Merge #7935: Versionbits: GBT support

12c708a getblocktemplate: Use version/force mutation to support pre-BIP9 clients (Luke Dashjr)
9879060 getblocktemplate: Explicitly handle the distinction between GBT-affecting softforks vs not (Luke Dashjr)
72cd6b2 qa/rpc-tests: bip9-softforks: Add tests for getblocktemplate versionbits updates (Luke Dashjr)
d3df40e Implement BIP 9 GBT changes (Luke Dashjr)

7 files changed, 150 insertions, 14 deletions

diff --git a/qa/rpc-tests/bip9-softforks.py b/qa/rpc-tests/bip9-softforks.py
index aae258315e..d7e8e5e5a5 100755
--- a/qa/rpc-tests/bip9-softforks.py
+++ b/qa/rpc-tests/bip9-softforks.py
@@ -80,7 +80,7 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         info = self.nodes[0].getblockchaininfo()
         return info['bip9_softforks'][key]
 
-    def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignature):
+    def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignature, bitno):
         # generate some coins for later
         self.coinbase_blocks = self.nodes[0].generate(2)
         self.height = 3  # height of the next block to build
@@ -89,6 +89,11 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         self.last_block_time = int(time.time())
 
         assert_equal(self.get_bip9_status(bipName)['status'], 'defined')
+        tmpl = self.nodes[0].getblocktemplate({})
+        assert(bipName not in tmpl['rules'])
+        assert(bipName not in tmpl['vbavailable'])
+        assert_equal(tmpl['vbrequired'], 0)
+        assert_equal(tmpl['version'], 0x20000000)
 
         # Test 1
         # Advance from DEFINED to STARTED
@@ -96,6 +101,11 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         yield TestInstance(test_blocks, sync_every_block=False)
 
         assert_equal(self.get_bip9_status(bipName)['status'], 'started')
+        tmpl = self.nodes[0].getblocktemplate({})
+        assert(bipName not in tmpl['rules'])
+        assert_equal(tmpl['vbavailable'][bipName], bitno)
+        assert_equal(tmpl['vbrequired'], 0)
+        assert(tmpl['version'] & activated_version)
 
         # Test 2
         # Fail to achieve LOCKED_IN 100 out of 144 signal bit 1
@@ -107,6 +117,11 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         yield TestInstance(test_blocks, sync_every_block=False)
 
         assert_equal(self.get_bip9_status(bipName)['status'], 'started')
+        tmpl = self.nodes[0].getblocktemplate({})
+        assert(bipName not in tmpl['rules'])
+        assert_equal(tmpl['vbavailable'][bipName], bitno)
+        assert_equal(tmpl['vbrequired'], 0)
+        assert(tmpl['version'] & activated_version)
 
         # Test 3
         # 108 out of 144 signal bit 1 to achieve LOCKED_IN
@@ -118,6 +133,8 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         yield TestInstance(test_blocks, sync_every_block=False)
 
         assert_equal(self.get_bip9_status(bipName)['status'], 'locked_in')
+        tmpl = self.nodes[0].getblocktemplate({})
+        assert(bipName not in tmpl['rules'])
 
         # Test 4
         # 143 more version 536870913 blocks (waiting period-1)
@@ -125,6 +142,8 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         yield TestInstance(test_blocks, sync_every_block=False)
 
         assert_equal(self.get_bip9_status(bipName)['status'], 'locked_in')
+        tmpl = self.nodes[0].getblocktemplate({})
+        assert(bipName not in tmpl['rules'])
 
         # Test 5
         # Check that the new rule is enforced
@@ -148,6 +167,11 @@ class BIP9SoftForksTest(ComparisonTestFramework):
         yield TestInstance([[block, True]])
 
         assert_equal(self.get_bip9_status(bipName)['status'], 'active')
+        tmpl = self.nodes[0].getblocktemplate({})
+        assert(bipName in tmpl['rules'])
+        assert(bipName not in tmpl['vbavailable'])
+        assert_equal(tmpl['vbrequired'], 0)
+        assert(not (tmpl['version'] & (1 << bitno)))
 
         # Test 6
         # Check that the new sequence lock rules are enforced
@@ -183,9 +207,9 @@ class BIP9SoftForksTest(ComparisonTestFramework):
 
     def get_tests(self):
         for test in itertools.chain(
-                self.test_BIP('csv', 536870913, self.sequence_lock_invalidate, self.donothing),
-                self.test_BIP('csv', 536870913, self.mtp_invalidate, self.donothing),
-                self.test_BIP('csv', 536870913, self.donothing, self.csv_invalidate)
+                self.test_BIP('csv', 0x20000001, self.sequence_lock_invalidate, self.donothing, 0),
+                self.test_BIP('csv', 0x20000001, self.mtp_invalidate, self.donothing, 0),
+                self.test_BIP('csv', 0x20000001, self.donothing, self.csv_invalidate, 0)
         ):
             yield test
 
diff --git a/src/consensus/params.h b/src/consensus/params.h
index 4f3480b89b..6c4cc49479 100644
--- a/src/consensus/params.h
+++ b/src/consensus/params.h
@@ -16,6 +16,7 @@ enum DeploymentPos
 {
     DEPLOYMENT_TESTDUMMY,
     DEPLOYMENT_CSV, // Deployment of BIP68, BIP112, and BIP113.
+    // NOTE: Also add new deployments to VersionBitsDeploymentInfo in versionbits.cpp
     MAX_VERSION_BITS_DEPLOYMENTS
 };
 
diff --git a/src/main.cpp b/src/main.cpp
index bd028fefba..84edaebb82 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -2181,7 +2181,7 @@ void PartitionCheck(bool (*initialDownloadCheck)(), CCriticalSection& cs, const
 }
 
 // Protected by cs_main
-static VersionBitsCache versionbitscache;
+VersionBitsCache versionbitscache;
 
 int32_t ComputeBlockVersion(const CBlockIndex* pindexPrev, const Consensus::Params& params)
 {
diff --git a/src/main.h b/src/main.h
index 4e93c084f4..9b99ae7c87 100644
--- a/src/main.h
+++ b/src/main.h
@@ -482,6 +482,8 @@ extern CBlockTreeDB *pblocktree;
  */
 int GetSpendHeight(const CCoinsViewCache& inputs);
 
+extern VersionBitsCache versionbitscache;
+
 /**
  * Determine what nVersion a new block should use.
  */
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 9a7d9d53a0..2bd52eadbc 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -8,6 +8,7 @@
 #include "chain.h"
 #include "chainparams.h"
 #include "consensus/consensus.h"
+#include "consensus/params.h"
 #include "consensus/validation.h"
 #include "core_io.h"
 #include "init.h"
@@ -303,6 +304,15 @@ static UniValue BIP22ValidationResult(const CValidationState& state)
     return "valid?";
 }
 
+std::string gbt_vb_name(const Consensus::DeploymentPos pos) {
+    const struct BIP9DeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos];
+    std::string s = vbinfo.name;
+    if (!vbinfo.gbt_force) {
+        s.insert(s.begin(), '!');
+    }
+    return s;
+}
+
 UniValue getblocktemplate(const UniValue& params, bool fHelp)
 {
     if (fHelp || params.size() > 1)
@@ -310,7 +320,9 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp)
             "getblocktemplate ( \"jsonrequestobject\" )\n"
             "\nIf the request parameters include a 'mode' key, that is used to explicitly select between the default 'template' request or a 'proposal'.\n"
             "It returns data needed to construct a block to work on.\n"
-            "See https://en.bitcoin.it/wiki/BIP_0022 for full specification.\n"
+            "For full specification, see BIPs 22 and 9:\n"
+            "    https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki\n"
+            "    https://github.com/bitcoin/bips/blob/master/bip-0009.mediawiki#getblocktemplate_changes\n"
 
             "\nArguments:\n"
             "1. \"jsonrequestobject\"       (string, optional) A json object in the following spec\n"
@@ -326,6 +338,12 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp)
             "\nResult:\n"
             "{\n"
             "  \"version\" : n,                    (numeric) The block version\n"
+            "  \"rules\" : [ \"rulename\", ... ],    (array of strings) specific block rules that are to be enforced\n"
+            "  \"vbavailable\" : {                 (json object) set of pending, supported versionbit (BIP 9) softfork deployments\n"
+            "      \"rulename\" : bitnumber        (numeric) identifies the bit number as indicating acceptance and readiness for the named softfork rule\n"
+            "      ,...\n"
+            "  },\n"
+            "  \"vbrequired\" : n,                 (numeric) bit mask of versionbits the server requires set in submissions\n"
             "  \"previousblockhash\" : \"xxxx\",    (string) The hash of current highest block\n"
             "  \"transactions\" : [                (array) contents of non-coinbase transactions that should be included in the next block\n"
             "      {\n"
@@ -369,6 +387,8 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp)
 
     std::string strMode = "template";
     UniValue lpval = NullUniValue;
+    std::set<std::string> setClientRules;
+    int64_t nMaxVersionPreVB = -1;
     if (params.size() > 0)
     {
         const UniValue& oparam = params[0].get_obj();
@@ -412,6 +432,20 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp)
             TestBlockValidity(state, Params(), block, pindexPrev, false, true);
             return BIP22ValidationResult(state);
         }
+
+        const UniValue& aClientRules = find_value(oparam, "rules");
+        if (aClientRules.isArray()) {
+            for (unsigned int i = 0; i < aClientRules.size(); ++i) {
+                const UniValue& v = aClientRules[i];
+                setClientRules.insert(v.get_str());
+            }
+        } else {
+            // NOTE: It is important that this NOT be read if versionbits is supported
+            const UniValue& uvMaxVersion = find_value(oparam, "maxversion");
+            if (uvMaxVersion.isNum()) {
+                nMaxVersionPreVB = uvMaxVersion.get_int64();
+            }
+        }
     }
 
     if (strMode != "template")
@@ -501,9 +535,10 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp)
         pindexPrev = pindexPrevNew;
     }
     CBlock* pblock = &pblocktemplate->block; // pointer for convenience
+    const Consensus::Params& consensusParams = Params().GetConsensus();
 
     // Update nTime
-    UpdateTime(pblock, Params().GetConsensus(), pindexPrev);
+    UpdateTime(pblock, consensusParams, pindexPrev);
     pblock->nNonce = 0;
 
     UniValue aCaps(UniValue::VARR); aCaps.push_back("proposal");
@@ -544,17 +579,69 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp)
 
     arith_uint256 hashTarget = arith_uint256().SetCompact(pblock->nBits);
 
-    static UniValue aMutable(UniValue::VARR);
-    if (aMutable.empty())
-    {
-        aMutable.push_back("time");
-        aMutable.push_back("transactions");
-        aMutable.push_back("prevblock");
-    }
+    UniValue aMutable(UniValue::VARR);
+    aMutable.push_back("time");
+    aMutable.push_back("transactions");
+    aMutable.push_back("prevblock");
 
     UniValue result(UniValue::VOBJ);
     result.push_back(Pair("capabilities", aCaps));
+
+    UniValue aRules(UniValue::VARR);
+    UniValue vbavailable(UniValue::VOBJ);
+    for (int i = 0; i < (int)Consensus::MAX_VERSION_BITS_DEPLOYMENTS; ++i) {
+        Consensus::DeploymentPos pos = Consensus::DeploymentPos(i);
+        ThresholdState state = VersionBitsState(pindexPrev, consensusParams, pos, versionbitscache);
+        switch (state) {
+            case THRESHOLD_DEFINED:
+            case THRESHOLD_FAILED:
+                // Not exposed to GBT at all
+                break;
+            case THRESHOLD_LOCKED_IN:
+                // Ensure bit is set in block version
+                pblock->nVersion |= VersionBitsMask(consensusParams, pos);
+                // FALL THROUGH to get vbavailable set...
+            case THRESHOLD_STARTED:
+            {
+                const struct BIP9DeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos];
+                vbavailable.push_back(Pair(gbt_vb_name(pos), consensusParams.vDeployments[pos].bit));
+                if (setClientRules.find(vbinfo.name) == setClientRules.end()) {
+                    if (!vbinfo.gbt_force) {
+                        // If the client doesn't support this, don't indicate it in the [default] version
+                        pblock->nVersion &= ~VersionBitsMask(consensusParams, pos);
+                    }
+                }
+                break;
+            }
+            case THRESHOLD_ACTIVE:
+            {
+                // Add to rules only
+                const struct BIP9DeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos];
+                aRules.push_back(gbt_vb_name(pos));
+                if (setClientRules.find(vbinfo.name) == setClientRules.end()) {
+                    // Not supported by the client; make sure it's safe to proceed
+                    if (!vbinfo.gbt_force) {
+                        // If we do anything other than throw an exception here, be sure version/force isn't sent to old clients
+                        throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Support for '%s' rule requires explicit client support", vbinfo.name));
+                    }
+                }
+                break;
+            }
+        }
+    }
     result.push_back(Pair("version", pblock->nVersion));
+    result.push_back(Pair("rules", aRules));
+    result.push_back(Pair("vbavailable", vbavailable));
+    result.push_back(Pair("vbrequired", int(0)));
+
+    if (nMaxVersionPreVB >= 2) {
+        // If VB is supported by the client, nMaxVersionPreVB is -1, so we won't get here
+        // Because BIP 34 changed how the generation transaction is serialised, we can only use version/force back to v2 blocks
+        // This is safe to do [otherwise-]unconditionally only because we are throwing an exception above if a non-force deployment gets activated
+        // Note that this can probably also be removed entirely after the first BIP9 non-force deployment (ie, probably segwit) gets activated
+        aMutable.push_back("version/force");
+    }
+
     result.push_back(Pair("previousblockhash", pblock->hashPrevBlock.GetHex()));
     result.push_back(Pair("transactions", transactions));
     result.push_back(Pair("coinbaseaux", aux));
diff --git a/src/versionbits.cpp b/src/versionbits.cpp
index 78feb8ab0c..043819c654 100644
--- a/src/versionbits.cpp
+++ b/src/versionbits.cpp
@@ -4,6 +4,19 @@
 
 #include "versionbits.h"
 
+#include "consensus/params.h"
+
+const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] = {
+    {
+        /*.name =*/ "testdummy",
+        /*.gbt_force =*/ true,
+    },
+    {
+        /*.name =*/ "csv",
+        /*.gbt_force =*/ true,
+    }
+};
+
 ThresholdState AbstractThresholdConditionChecker::GetStateFor(const CBlockIndex* pindexPrev, const Consensus::Params& params, ThresholdConditionCache& cache) const
 {
     int nPeriod = Period(params);
diff --git a/src/versionbits.h b/src/versionbits.h
index 04f4738272..ede2dcdda8 100644
--- a/src/versionbits.h
+++ b/src/versionbits.h
@@ -30,6 +30,15 @@ enum ThresholdState {
 // will either be NULL or a block with (height + 1) % Period() == 0.
 typedef std::map<const CBlockIndex*, ThresholdState> ThresholdConditionCache;
 
+struct BIP9DeploymentInfo {
+    /** Deployment name */
+    const char *name;
+    /** Whether GBT clients can safely ignore this rule in simplified usage */
+    bool gbt_force;
+};
+
+extern const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[];
+
 /**
  * Abstract class that implements BIP9-style threshold logic, and caches results.
  */