Merge #19401: QA: Use GBT to get block versions correct

d438d60 QA: Use GBT to get block versions correct (Luke Dashjr)
1df2cd1 QA: blocktools: Accept block template to create_block (Luke Dashjr)

Pull request description:

  The goal here is to decouple unrelated tests from the details of block versions.

  Currently, these tests are forcing specific versions of blocks for no real reason.

ACKs for top commit:
  fjahr:
    re-ACK d438d60
  benthecarman:
    ACK d438d60

Tree-SHA512: 523b1cd4dac8d65c88432e126ce7f60df96ca4b94f7ecc8e83ba4ffbade23e2afe7055fdf586ce3c195a533f2004e63fff83add4267b39473a581c9f1c6d5340

test/functional/feature_bip68_sequence.py

@@ -6,7 +6,7 @@
 
 import time
 
-from test_framework.blocktools import create_block, create_coinbase, add_witness_commitment
+from test_framework.blocktools import create_block, NORMAL_GBT_REQUEST_PARAMS, add_witness_commitment
 from test_framework.messages import COIN, COutPoint, CTransaction, CTxIn, CTxOut, FromHex, ToHex
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
@@ -275,6 +275,8 @@ def test_nonzero_locks(orig_tx, node, relayfee, use_height_lock):
 
         # Advance the time on the node so that we can test timelocks
         self.nodes[0].setmocktime(cur_time+600)
+        # Save block template now to use for the reorg later
+        tmpl = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
         self.nodes[0].generate(1)
         assert tx2.hash not in self.nodes[0].getrawmempool()
 
@@ -318,16 +320,15 @@ def test_nonzero_locks(orig_tx, node, relayfee, use_height_lock):
         # diagram above).
         # This would cause tx2 to be added back to the mempool, which in turn causes
         # tx3 to be removed.
-        tip = int(self.nodes[0].getblockhash(self.nodes[0].getblockcount()-1), 16)
-        height = self.nodes[0].getblockcount()
         for i in range(2):
-            block = create_block(tip, create_coinbase(height), cur_time)
-            block.nVersion = 3
+            block = create_block(tmpl=tmpl, ntime=cur_time)
             block.rehash()
             block.solve()
             tip = block.sha256
-            height += 1
             assert_equal(None if i == 1 else 'inconclusive', self.nodes[0].submitblock(ToHex(block)))
+            tmpl = self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
+            tmpl['previousblockhash'] = '%x' % tip
+            tmpl['transactions'] = []
             cur_time += 1
 
         mempool = self.nodes[0].getrawmempool()
@@ -375,9 +376,7 @@ def test_bip68_not_consensus(self):
         assert_raises_rpc_error(-26, NOT_FINAL_ERROR, self.nodes[0].sendrawtransaction, ToHex(tx3))
 
         # make a block that violates bip68; ensure that the tip updates
-        tip = int(self.nodes[0].getbestblockhash(), 16)
-        block = create_block(tip, create_coinbase(self.nodes[0].getblockcount()+1))
-        block.nVersion = 3
+        block = create_block(tmpl=self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS))
         block.vtx.extend([tx1, tx2, tx3])
         block.hashMerkleRoot = block.calc_merkle_root()
         block.rehash()

test/functional/feature_nulldummy.py

@@ -14,7 +14,7 @@
 """
 import time
 
-from test_framework.blocktools import create_coinbase, create_block, create_transaction, add_witness_commitment
+from test_framework.blocktools import NORMAL_GBT_REQUEST_PARAMS, create_block, create_transaction, add_witness_commitment
 from test_framework.messages import CTransaction
 from test_framework.script import CScript
 from test_framework.test_framework import BitcoinTestFramework
@@ -37,14 +37,15 @@ def trueDummy(tx):
 class NULLDUMMYTest(BitcoinTestFramework):
 
     def set_test_params(self):
-        self.num_nodes = 1
+        # Need two nodes only so GBT doesn't complain that it's not connected
+        self.num_nodes = 2
         self.setup_clean_chain = True
         # This script tests NULLDUMMY activation, which is part of the 'segwit' deployment, so we go through
         # normal segwit activation here (and don't use the default always-on behaviour).
         self.extra_args = [[
             '-segwitheight=432',
             '-addresstype=legacy',
-        ]]
+        ]] * 2
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -61,7 +62,6 @@ def run_test(self):
             coinbase_txid.append(self.nodes[0].getblock(i)['tx'][0])
         self.nodes[0].generate(427)  # Block 429
         self.lastblockhash = self.nodes[0].getbestblockhash()
-        self.tip = int("0x" + self.lastblockhash, 0)
         self.lastblockheight = 429
         self.lastblocktime = int(time.time()) + 429
 
@@ -102,8 +102,10 @@ def run_test(self):
         self.block_submit(self.nodes[0], test6txs, True, True)
 
     def block_submit(self, node, txs, witness=False, accept=False):
-        block = create_block(self.tip, create_coinbase(self.lastblockheight + 1), self.lastblocktime + 1)
-        block.nVersion = 4
+        tmpl = node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS)
+        assert_equal(tmpl['previousblockhash'], self.lastblockhash)
+        assert_equal(tmpl['height'], self.lastblockheight + 1)
+        block = create_block(tmpl=tmpl, ntime=self.lastblocktime + 1)
         for tx in txs:
             tx.rehash()
             block.vtx.append(tx)
@@ -114,7 +116,6 @@ def block_submit(self, node, txs, witness=False, accept=False):
         assert_equal(None if accept else 'block-validation-failed', node.submitblock(block.serialize().hex()))
         if (accept):
             assert_equal(node.getbestblockhash(), block.hash)
-            self.tip = block.sha256
             self.lastblockhash = block.hash
             self.lastblocktime += 1
             self.lastblockheight += 1

test/functional/feature_segwit.py

@@ -61,14 +61,12 @@ def set_test_params(self):
             ],
             [
                 "-acceptnonstdtxn=1",
-                "-blockversion=4",
                 "-rpcserialversion=1",
                 "-segwitheight=432",
                 "-addresstype=legacy",
             ],
             [
                 "-acceptnonstdtxn=1",
-                "-blockversion=536870915",
                 "-segwitheight=432",
                 "-addresstype=legacy",
             ],

test/functional/p2p_compactblocks.py

@@ -9,7 +9,7 @@
 """
 import random
 
-from test_framework.blocktools import create_block, create_coinbase, add_witness_commitment
+from test_framework.blocktools import create_block, NORMAL_GBT_REQUEST_PARAMS, add_witness_commitment
 from test_framework.messages import BlockTransactions, BlockTransactionsRequest, calculate_shortid, CBlock, CBlockHeader, CInv, COutPoint, CTransaction, CTxIn, CTxInWitness, CTxOut, FromHex, HeaderAndShortIDs, msg_no_witness_block, msg_no_witness_blocktxn, msg_cmpctblock, msg_getblocktxn, msg_getdata, msg_getheaders, msg_headers, msg_inv, msg_sendcmpct, msg_sendheaders, msg_tx, msg_block, msg_blocktxn, MSG_BLOCK, MSG_CMPCT_BLOCK, MSG_WITNESS_FLAG, NODE_NETWORK, P2PHeaderAndShortIDs, PrefilledTransaction, ser_uint256, ToHex
 from test_framework.p2p import p2p_lock, P2PInterface
 from test_framework.script import CScript, OP_TRUE, OP_DROP
@@ -104,11 +104,7 @@ def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
 
     def build_block_on_tip(self, node, segwit=False):
-        height = node.getblockcount()
-        tip = node.getbestblockhash()
-        mtp = node.getblockheader(tip)['mediantime']
-        block = create_block(int(tip, 16), create_coinbase(height + 1), mtp + 1)
-        block.nVersion = 4
+        block = create_block(tmpl=node.getblocktemplate(NORMAL_GBT_REQUEST_PARAMS))
         if segwit:
             add_witness_commitment(block)
         block.solve()
@@ -769,6 +765,9 @@ def announce_cmpct_block(node, peer):
         assert_equal(int(node.getbestblockhash(), 16), block.sha256)
 
     def run_test(self):
+        # Get the nodes out of IBD
+        self.nodes[0].generate(1)
+
         # Setup the p2p connections
         self.segwit_node = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=2))
         self.old_node = self.nodes[0].add_p2p_connection(TestP2PConn(cmpct_version=1), services=NODE_NETWORK)

test/functional/test_framework/blocktools.py

@@ -4,6 +4,10 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Utilities for manipulating blocks and transactions."""
 
+from binascii import a2b_hex
+import io
+import struct
+import time
 import unittest
 
 from .address import (
@@ -53,19 +57,31 @@
 # From BIP141
 WITNESS_COMMITMENT_HEADER = b"\xaa\x21\xa9\xed"
 
+NORMAL_GBT_REQUEST_PARAMS = {"rules": ["segwit"]}
 
-def create_block(hashprev, coinbase, ntime=None, *, version=1):
+
+def create_block(hashprev=None, coinbase=None, ntime=None, *, version=None, tmpl=None, txlist=None):
     """Create a block (with regtest difficulty)."""
     block = CBlock()
-    block.nVersion = version
-    if ntime is None:
-        import time
-        block.nTime = int(time.time() + 600)
+    if tmpl is None:
+        tmpl = {}
+    block.nVersion = version or tmpl.get('version') or 1
+    block.nTime = ntime or tmpl.get('curtime') or int(time.time() + 600)
+    block.hashPrevBlock = hashprev or int(tmpl['previousblockhash'], 0x10)
+    if tmpl and not tmpl.get('bits') is None:
+        block.nBits = struct.unpack('>I', a2b_hex(tmpl['bits']))[0]
     else:
-        block.nTime = ntime
-    block.hashPrevBlock = hashprev
-    block.nBits = 0x207fffff  # difficulty retargeting is disabled in REGTEST chainparams
+        block.nBits = 0x207fffff  # difficulty retargeting is disabled in REGTEST chainparams
+    if coinbase is None:
+        coinbase = create_coinbase(height=tmpl['height'])
     block.vtx.append(coinbase)
+    if txlist:
+        for tx in txlist:
+            if not hasattr(tx, 'calc_sha256'):
+                txo = CTransaction()
+                txo.deserialize(io.BytesIO(tx))
+                tx = txo
+            block.vtx.append(tx)
     block.hashMerkleRoot = block.calc_merkle_root()
     block.calc_sha256()
     return block