From ddd8c01d705df5526260c3804f89aa9ca9987fac Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 23 Apr 2016 23:30:20 +0000 Subject: [PATCH 1/4] Implement BIP 9 GBT changes - BIP9DeploymentInfo struct for static deployment info - VersionBitsDeploymentInfo: Avoid C++11ism by commenting parameter names - getblocktemplate: Make sure to set deployments in the version if it is LOCKED_IN - In this commit, all rules are considered required for clients to support --- src/consensus/params.h | 1 + src/main.cpp | 2 +- src/main.h | 2 ++ src/rpcmining.cpp | 50 ++++++++++++++++++++++++++++++++++++++++-- src/versionbits.cpp | 11 ++++++++++ src/versionbits.h | 7 ++++++ 6 files changed, 70 insertions(+), 3 deletions(-) diff --git a/src/consensus/params.h b/src/consensus/params.h index 4f3480b89ba69..6c4cc49479ba1 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 14f70cdf5f514..5cf9b45532dc9 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2137,7 +2137,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 05ae9ea450e72..2705bd8eda7fc 100644 --- a/src/main.h +++ b/src/main.h @@ -532,6 +532,8 @@ extern CBlockTreeDB *pblocktree; */ int GetSpendHeight(const CCoinsViewCache& inputs); +extern VersionBitsCache versionbitscache; + /** * Determine what nVersion a new block should use. */ diff --git a/src/rpcmining.cpp b/src/rpcmining.cpp index 1048344e00920..6befd0a575323 100644 --- a/src/rpcmining.cpp +++ b/src/rpcmining.cpp @@ -7,6 +7,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" @@ -320,6 +321,13 @@ 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; + s.insert(s.begin(), '!'); + return s; +} + UniValue getblocktemplate(const UniValue& params, bool fHelp) { if (fHelp || params.size() > 1) @@ -327,7 +335,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" @@ -343,6 +353,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" @@ -518,9 +534,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"); @@ -571,7 +588,36 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) 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: + // Add to vbavailable (and it's presumably in version already) + vbavailable.push_back(Pair(gbt_vb_name(pos), consensusParams.vDeployments[pos].bit)); + break; + case THRESHOLD_ACTIVE: + // Add to rules only + aRules.push_back(gbt_vb_name(pos)); + 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))); + 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 fbb60c0fc598e..041ca2adbcafc 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -4,6 +4,17 @@ #include "versionbits.h" +#include "consensus/params.h" + +const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] = { + { + /*.name =*/ "testdummy", + }, + { + /*.name =*/ "csv", + } +}; + 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 04f4738272796..d80594202884b 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -30,6 +30,13 @@ enum ThresholdState { // will either be NULL or a block with (height + 1) % Period() == 0. typedef std::map ThresholdConditionCache; +struct BIP9DeploymentInfo { + /** Deployment name */ + const char *name; +}; + +extern const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[]; + /** * Abstract class that implements BIP9-style threshold logic, and caches results. */ From 40e81f5f57a9f317b534b12ecb851845a91b3276 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 21 May 2016 08:39:34 +0000 Subject: [PATCH 2/4] qa/rpc-tests: bip9-softforks: Add tests for getblocktemplate versionbits updates --- qa/rpc-tests/bip9-softforks.py | 32 ++++++++++++++++++++++++++++---- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/qa/rpc-tests/bip9-softforks.py b/qa/rpc-tests/bip9-softforks.py index cbb1b7d4cee89..1b1f5dd0dcce4 100755 --- a/qa/rpc-tests/bip9-softforks.py +++ b/qa/rpc-tests/bip9-softforks.py @@ -85,7 +85,7 @@ def get_bip9_status(self, key): raise IndexError ('key:"%s" not found' % 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 @@ -94,6 +94,11 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu self.last_block_time = 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 @@ -101,6 +106,11 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu 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 @@ -112,6 +122,11 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu 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 @@ -123,6 +138,8 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu 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) @@ -130,6 +147,8 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu 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 @@ -153,6 +172,11 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu 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 @@ -187,9 +211,9 @@ def test_BIP(self, bipName, activated_version, invalidate, invalidatePostSignatu 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 From 65ee3324fbd6799241acada1320b684651bad892 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 1 Jun 2016 16:47:36 +0000 Subject: [PATCH 3/4] getblocktemplate: Explicitly handle the distinction between GBT-affecting softforks vs not --- src/rpcmining.cpp | 32 ++++++++++++++++++++++++++++++-- src/versionbits.cpp | 2 ++ src/versionbits.h | 2 ++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/rpcmining.cpp b/src/rpcmining.cpp index 6befd0a575323..527514887050d 100644 --- a/src/rpcmining.cpp +++ b/src/rpcmining.cpp @@ -324,7 +324,9 @@ static UniValue BIP22ValidationResult(const CValidationState& state) std::string gbt_vb_name(const Consensus::DeploymentPos pos) { const struct BIP9DeploymentInfo& vbinfo = VersionBitsDeploymentInfo[pos]; std::string s = vbinfo.name; - s.insert(s.begin(), '!'); + if (!vbinfo.gbt_force) { + s.insert(s.begin(), '!'); + } return s; } @@ -402,6 +404,7 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) std::string strMode = "template"; UniValue lpval = NullUniValue; + std::set setClientRules; if (params.size() > 0) { const UniValue& oparam = params[0].get_obj(); @@ -445,6 +448,14 @@ 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()); + } + } } if (strMode != "template") @@ -604,13 +615,30 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) pblock->nVersion |= VersionBitsMask(consensusParams, pos); // FALL THROUGH to get vbavailable set... case THRESHOLD_STARTED: - // Add to vbavailable (and it's presumably in version already) + { + 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) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Support for '%s' rule requires explicit client support", vbinfo.name)); + } + } break; + } } } result.push_back(Pair("version", pblock->nVersion)); diff --git a/src/versionbits.cpp b/src/versionbits.cpp index 041ca2adbcafc..c06c9907be944 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -9,9 +9,11 @@ const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION_BITS_DEPLOYMENTS] = { { /*.name =*/ "testdummy", + /*.gbt_force =*/ true, }, { /*.name =*/ "csv", + /*.gbt_force =*/ true, } }; diff --git a/src/versionbits.h b/src/versionbits.h index d80594202884b..ede2dcdda8a7a 100644 --- a/src/versionbits.h +++ b/src/versionbits.h @@ -33,6 +33,8 @@ typedef std::map 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[]; From db4bacf59092ae4aa923a58f379c36757edad932 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Wed, 1 Jun 2016 16:51:54 +0000 Subject: [PATCH 4/4] getblocktemplate: Use version/force mutation to support pre-BIP9 clients --- src/rpcmining.cpp | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/rpcmining.cpp b/src/rpcmining.cpp index 527514887050d..18c3b3a511730 100644 --- a/src/rpcmining.cpp +++ b/src/rpcmining.cpp @@ -405,6 +405,7 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) std::string strMode = "template"; UniValue lpval = NullUniValue; std::set setClientRules; + int64_t nMaxVersionPreVB = -1; if (params.size() > 0) { const UniValue& oparam = params[0].get_obj(); @@ -455,6 +456,12 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) 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(); + } } } @@ -589,13 +596,10 @@ 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)); @@ -634,6 +638,7 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) 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)); } } @@ -646,6 +651,14 @@ UniValue getblocktemplate(const UniValue& params, bool fHelp) 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));