From abe7b3d3abe10e3554b770f40824174b3b217490 Mon Sep 17 00:00:00 2001 From: Suhas Daftuar Date: Wed, 8 Mar 2017 15:56:59 -0500 Subject: [PATCH] Don't require segwit in getblocktemplate for segwit signalling or mining Segwit's version bit will be signalled for all invocations of CreateNewBlock, and not specifying segwit only will cause CreateNewBlock to skip transactions with witness from being selected. --- qa/rpc-tests/p2p-segwit.py | 8 +++++--- qa/rpc-tests/segwit.py | 9 ++++++--- src/miner.cpp | 4 ++-- src/miner.h | 2 +- src/rpc/mining.cpp | 18 ++++++++++++++---- src/versionbits.cpp | 2 +- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/qa/rpc-tests/p2p-segwit.py b/qa/rpc-tests/p2p-segwit.py index 0f844883b..dcf2b9a7d 100755 --- a/qa/rpc-tests/p2p-segwit.py +++ b/qa/rpc-tests/p2p-segwit.py @@ -1698,9 +1698,11 @@ class SegWitTest(BitcoinTestFramework): for node in [self.nodes[0], self.nodes[2]]: gbt_results = node.getblocktemplate() block_version = gbt_results['version'] - # If we're not indicating segwit support, we should not be signalling - # for segwit activation, nor should we get a witness commitment. - assert_equal(block_version & (1 << VB_WITNESS_BIT), 0) + # If we're not indicating segwit support, we will still be + # signalling for segwit activation. + assert_equal((block_version & (1 << VB_WITNESS_BIT) != 0), node == self.nodes[0]) + # If we don't specify the segwit rule, then we won't get a default + # commitment. assert('default_witness_commitment' not in gbt_results) # Workaround: diff --git a/qa/rpc-tests/segwit.py b/qa/rpc-tests/segwit.py index 36eb0dbdc..50ace12aa 100755 --- a/qa/rpc-tests/segwit.py +++ b/qa/rpc-tests/segwit.py @@ -250,9 +250,12 @@ class SegWitTest(BitcoinTestFramework): assert(tmpl['transactions'][0]['txid'] == txid) assert(tmpl['transactions'][0]['sigops'] == 8) - self.log.info("Non-segwit miners are not able to use GBT response after activation.") - send_to_witness(1, self.nodes[0], find_unspent(self.nodes[0], 50), self.pubkey[0], False, Decimal("49.998")) - assert_raises_jsonrpc(-8, "Support for 'segwit' rule requires explicit client support", self.nodes[0].getblocktemplate, {}) + self.log.info("Non-segwit miners are able to use GBT response after activation.") + txid = send_to_witness(1, self.nodes[0], find_unspent(self.nodes[0], 50), self.pubkey[0], False, Decimal("49.998")) + #assert_raises_jsonrpc(-8, "Support for 'segwit' rule requires explicit client support", self.nodes[0].getblocktemplate, {}) + tmpl = self.nodes[0].getblocktemplate() + # TODO: add a transaction with witness to mempool, and verify it's not + # selected for mining. self.log.info("Verify behaviour of importaddress, addwitnessaddress and listunspent") diff --git a/src/miner.cpp b/src/miner.cpp index 67e7ff155..ff28a5680 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -137,7 +137,7 @@ void BlockAssembler::resetBlock() nFees = 0; } -std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) +std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx) { resetBlock(); @@ -175,7 +175,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc // -promiscuousmempoolflags is used. // TODO: replace this with a call to main to assess validity of a mempool // transaction (which in most cases can be a no-op). - fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()); + fIncludeWitness = IsWitnessEnabled(pindexPrev, chainparams.GetConsensus()) && fMineWitnessTx; addPackageTxs(); diff --git a/src/miner.h b/src/miner.h index c105d07c2..a159b0553 100644 --- a/src/miner.h +++ b/src/miner.h @@ -170,7 +170,7 @@ public: BlockAssembler(const CChainParams& params, const Options& options); /** Construct a new block template with coinbase to scriptPubKeyIn */ - std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn); + std::unique_ptr CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx=true); private: // utility functions diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index abdfa651a..4db8ffaa7 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -514,12 +514,22 @@ UniValue getblocktemplate(const JSONRPCRequest& request) // TODO: Maybe recheck connections/IBD and (if something wrong) send an expires-immediately template to stop miners? } + const struct BIP9DeploymentInfo& segwit_info = VersionBitsDeploymentInfo[Consensus::DEPLOYMENT_SEGWIT]; + // If the caller is indicating segwit support, then allow CreateNewBlock() + // to select witness transactions, after segwit activates (otherwise + // don't). + bool fSupportsSegwit = setClientRules.find(segwit_info.name) != setClientRules.end(); + // Update block static CBlockIndex* pindexPrev; static int64_t nStart; static std::unique_ptr pblocktemplate; + // Cache whether the last invocation was with segwit support, to avoid returning + // a segwit-block to a non-segwit caller. + static bool fLastTemplateSupportsSegwit = true; if (pindexPrev != chainActive.Tip() || - (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5)) + (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5) || + fLastTemplateSupportsSegwit != fSupportsSegwit) { // Clear pindexPrev so future calls make a new block, despite any failures from here on pindexPrev = nullptr; @@ -528,10 +538,11 @@ UniValue getblocktemplate(const JSONRPCRequest& request) nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); CBlockIndex* pindexPrevNew = chainActive.Tip(); nStart = GetTime(); + fLastTemplateSupportsSegwit = fSupportsSegwit; // Create new block CScript scriptDummy = CScript() << OP_TRUE; - pblocktemplate = BlockAssembler(Params()).CreateNewBlock(scriptDummy); + pblocktemplate = BlockAssembler(Params()).CreateNewBlock(scriptDummy, fSupportsSegwit); if (!pblocktemplate) throw JSONRPCError(RPC_OUT_OF_MEMORY, "Out of memory"); @@ -681,8 +692,7 @@ UniValue getblocktemplate(const JSONRPCRequest& request) result.push_back(Pair("bits", strprintf("%08x", pblock->nBits))); result.push_back(Pair("height", (int64_t)(pindexPrev->nHeight+1))); - const struct BIP9DeploymentInfo& segwit_info = VersionBitsDeploymentInfo[Consensus::DEPLOYMENT_SEGWIT]; - if (!pblocktemplate->vchCoinbaseCommitment.empty() && setClientRules.find(segwit_info.name) != setClientRules.end()) { + if (!pblocktemplate->vchCoinbaseCommitment.empty() && fSupportsSegwit) { result.push_back(Pair("default_witness_commitment", HexStr(pblocktemplate->vchCoinbaseCommitment.begin(), pblocktemplate->vchCoinbaseCommitment.end()))); } diff --git a/src/versionbits.cpp b/src/versionbits.cpp index d73f34051..8a7cce748 100644 --- a/src/versionbits.cpp +++ b/src/versionbits.cpp @@ -17,7 +17,7 @@ const struct BIP9DeploymentInfo VersionBitsDeploymentInfo[Consensus::MAX_VERSION }, { /*.name =*/ "segwit", - /*.gbt_force =*/ false, + /*.gbt_force =*/ true, } };