diff --git a/qa/rpc-tests/p2p-segwit.py b/qa/rpc-tests/p2p-segwit.py index 2f339bb54..479f1c679 100755 --- a/qa/rpc-tests/p2p-segwit.py +++ b/qa/rpc-tests/p2p-segwit.py @@ -1701,9 +1701,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 d6831e00e..c814399f9 100755 --- a/qa/rpc-tests/segwit.py +++ b/qa/rpc-tests/segwit.py @@ -11,9 +11,9 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import * from test_framework.mininode import sha256, ripemd160, CTransaction, CTxIn, COutPoint, CTxOut from test_framework.address import script_to_p2sh, key_to_p2pkh -from test_framework.script import CScript, OP_HASH160, OP_CHECKSIG, OP_0, hash160, OP_EQUAL, OP_DUP, OP_EQUALVERIFY, OP_1, OP_2, OP_CHECKMULTISIG +from test_framework.script import CScript, OP_HASH160, OP_CHECKSIG, OP_0, hash160, OP_EQUAL, OP_DUP, OP_EQUALVERIFY, OP_1, OP_2, OP_CHECKMULTISIG, OP_TRUE from io import BytesIO -from test_framework.mininode import FromHex +from test_framework.mininode import ToHex, FromHex, COIN NODE_0 = 0 NODE_1 = 1 @@ -251,20 +251,54 @@ class SegWitTest(BitcoinTestFramework): assert(tmpl['transactions'][0]['txid'] == txid) assert(tmpl['transactions'][0]['sigops'] == 8) - print("Verify non-segwit miners get a valid GBT response after the fork") - send_to_witness(1, self.nodes[0], find_unspent(self.nodes[0], 50), self.pubkey[0], False, Decimal("49.998")) - try: - tmpl = self.nodes[0].getblocktemplate({}) - assert(len(tmpl['transactions']) == 1) # Doesn't include witness tx - assert(tmpl['sizelimit'] == 1000000) - assert('weightlimit' not in tmpl) - assert(tmpl['sigoplimit'] == 20000) - assert(tmpl['transactions'][0]['hash'] == txid) - assert(tmpl['transactions'][0]['sigops'] == 2) - assert(('!segwit' in tmpl['rules']) or ('segwit' not in tmpl['rules'])) - except JSONRPCException: - # This is an acceptable outcome - pass + self.nodes[0].generate(1) # Mine a block to clear the gbt cache + + print("Non-segwit miners are able to use GBT response after activation.") + # Create a 3-tx chain: tx1 (non-segwit input, paying to a segwit output) -> + # tx2 (segwit input, paying to a non-segwit output) -> + # tx3 (non-segwit input, paying to a non-segwit output). + # tx1 is allowed to appear in the block, but no others. + txid1 = send_to_witness(1, self.nodes[0], find_unspent(self.nodes[0], 50), self.pubkey[0], False, Decimal("49.996")) + hex_tx = self.nodes[0].gettransaction(txid)['hex'] + tx = FromHex(CTransaction(), hex_tx) + assert(tx.wit.is_null()) # This should not be a segwit input + assert(txid1 in self.nodes[0].getrawmempool()) + + # Now create tx2, which will spend from txid1. + tx = CTransaction() + tx.vin.append(CTxIn(COutPoint(int(txid1, 16), 0), b'')) + tx.vout.append(CTxOut(int(49.99*COIN), CScript([OP_TRUE]))) + tx2_hex = self.nodes[0].signrawtransaction(ToHex(tx))['hex'] + txid2 = self.nodes[0].sendrawtransaction(tx2_hex) + tx = FromHex(CTransaction(), tx2_hex) + assert(not tx.wit.is_null()) + + # Now create tx3, which will spend from txid2 + tx = CTransaction() + tx.vin.append(CTxIn(COutPoint(int(txid2, 16), 0), b"")) + tx.vout.append(CTxOut(int(49.95*COIN), CScript([OP_TRUE]))) # Huge fee + tx.calc_sha256() + txid3 = self.nodes[0].sendrawtransaction(ToHex(tx)) + assert(tx.wit.is_null()) + assert(txid3 in self.nodes[0].getrawmempool()) + + # Now try calling getblocktemplate() without segwit support. + template = self.nodes[0].getblocktemplate() + + # Check that tx1 is the only transaction of the 3 in the template. + template_txids = [ t['txid'] for t in template['transactions'] ] + assert(txid2 not in template_txids and txid3 not in template_txids) + assert(txid1 in template_txids) + + # Check that running with segwit support results in all 3 being included. + template = self.nodes[0].getblocktemplate({"rules": ["segwit"]}) + template_txids = [ t['txid'] for t in template['transactions'] ] + assert(txid1 in template_txids) + assert(txid2 in template_txids) + assert(txid3 in template_txids) + + # Mine a block to clear the gbt cache again. + self.nodes[0].generate(1) print("Verify behaviour of importaddress, addwitnessaddress and listunspent") diff --git a/src/miner.cpp b/src/miner.cpp index d01edd93b..7b3d94d0e 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -127,7 +127,7 @@ void BlockAssembler::resetBlock() blockFinished = false; } -std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn) +std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& scriptPubKeyIn, bool fMineWitnessTx) { resetBlock(); @@ -165,7 +165,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; addPriorityTxs(); addPackageTxs(); diff --git a/src/miner.h b/src/miner.h index 3ba92b16b..29013c3bc 100644 --- a/src/miner.h +++ b/src/miner.h @@ -165,7 +165,7 @@ private: public: BlockAssembler(const CChainParams& chainparams); /** 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 77cd282a3..38d7b1eb1 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -519,12 +519,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; @@ -533,10 +543,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"); @@ -686,8 +697,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, } };