Merge bitcoin/bitcoin#23521: test: refactor: dedup code by taking use of create_block parameters

e57c0eb test: refactor: replace OP_1/OP_TRUE magic numbers by constants (Sebastian Falbesoner)
df5d783 test: refactor: take use of `create_block` txlist parameter (Sebastian Falbesoner)
ae9df4e test: refactor: take use of `create_block` version parameter (or use default) (Sebastian Falbesoner)

Pull request description:

  The helper `create_block` offers two parameters `version` and `txlist` which set the `nVersion` field / extend the `vtx` array of the block, respectively. By taking use of those, we can remove a lot of code, including the recalculation of the merkle root. Both passing txs in string and `CTransaction` format is supported, i.e. we also save potential calls to `tx_from_hex`.
  The PR also contains another commit which replaces magic numbers for OP_TRUE/OP_1 (0x51) with the proper constant from the `script` module.

  Instances setting the block version of 4 explicitely after calling `create_block` are removed, as this is the default since #16333 got merged (see bitcoin/bitcoin#23521 (comment)).

ACKs for top commit:
  stratospher:
    tested ACK e57c0eb.

Tree-SHA512: a56965168d36b40b84e7f334b00472b82c31e8482c9e2651c97a791abd7fee3b40ca15e943a7acafa3acf172066fdace38bb13240084b789fd6ff4f6e510e23a

Author: MarcoFalke

test/functional/feature_assumevalid.py

@@ -122,10 +122,8 @@ def run_test(self):
         tx.vout.append(CTxOut(49 * 100000000, CScript([OP_TRUE])))
         tx.calc_sha256()
 
-        block102 = create_block(self.tip, create_coinbase(height), self.block_time)
+        block102 = create_block(self.tip, create_coinbase(height), self.block_time, txlist=[tx])
         self.block_time += 1
-        block102.vtx.extend([tx])
-        block102.hashMerkleRoot = block102.calc_merkle_root()
         block102.solve()
         self.blocks.append(block102)
         self.tip = block102.sha256
@@ -135,7 +133,6 @@ def run_test(self):
         # Bury the assumed valid block 2100 deep
         for _ in range(2100):
             block = create_block(self.tip, create_coinbase(height), self.block_time)
-            block.nVersion = 4
             block.solve()
             self.blocks.append(block)
             self.tip = block.sha256

test/functional/feature_bip68_sequence.py

@@ -388,9 +388,7 @@ def test_bip68_not_consensus(self):
         assert_raises_rpc_error(-26, NOT_FINAL_ERROR, self.nodes[0].sendrawtransaction, tx3.serialize().hex())
 
         # make a block that violates bip68; ensure that the tip updates
-        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 = create_block(tmpl=self.nodes[0].getblocktemplate(NORMAL_GBT_REQUEST_PARAMS), txlist=[tx1, tx2, tx3])
         add_witness_commitment(block)
         block.solve()
 

test/functional/feature_block.py

@@ -1361,11 +1361,10 @@ def next_block(self, number, spend=None, additional_coinbase_value=0, script=CSc
         else:
             coinbase.vout[0].nValue += spend.vout[0].nValue - 1  # all but one satoshi to fees
             coinbase.rehash()
-            block = create_block(base_block_hash, coinbase, block_time, version=version)
             tx = self.create_tx(spend, 0, 1, script)  # spend 1 satoshi
             self.sign_tx(tx, spend)
-            self.add_transactions_to_block(block, [tx])
-            block.hashMerkleRoot = block.calc_merkle_root()
+            tx.rehash()
+            block = create_block(base_block_hash, coinbase, block_time, version=version, txlist=[tx])
         # Block is created. Find a valid nonce.
         block.solve()
         self.tip = block

test/functional/feature_cltv.py

@@ -120,10 +120,7 @@ def run_test(self):
 
         tip = self.nodes[0].getbestblockhash()
         block_time = self.nodes[0].getblockheader(tip)['mediantime'] + 1
-        block = create_block(int(tip, 16), create_coinbase(CLTV_HEIGHT - 1), block_time)
-        block.nVersion = 3
-        block.vtx.extend(invalid_cltv_txs)
-        block.hashMerkleRoot = block.calc_merkle_root()
+        block = create_block(int(tip, 16), create_coinbase(CLTV_HEIGHT - 1), block_time, version=3, txlist=invalid_cltv_txs)
         block.solve()
 
         self.test_cltv_info(is_active=False)  # Not active as of current tip and next block does not need to obey rules
@@ -134,8 +131,7 @@ def run_test(self):
         self.log.info("Test that blocks must now be at least version 4")
         tip = block.sha256
         block_time += 1
-        block = create_block(tip, create_coinbase(CLTV_HEIGHT), block_time)
-        block.nVersion = 3
+        block = create_block(tip, create_coinbase(CLTV_HEIGHT), block_time, version=3)
         block.solve()
 
         with self.nodes[0].assert_debug_log(expected_msgs=[f'{block.hash}, bad-version(0x00000003)']):

test/functional/feature_csv_activation.py

@@ -173,10 +173,7 @@ def generate_blocks(self, number):
         return test_blocks
 
     def create_test_block(self, txs):
-        block = create_block(self.tip, create_coinbase(self.tipheight + 1), self.last_block_time + 600)
-        block.nVersion = 4
-        block.vtx.extend(txs)
-        block.hashMerkleRoot = block.calc_merkle_root()
+        block = create_block(self.tip, create_coinbase(self.tipheight + 1), self.last_block_time + 600, txlist=txs)
         block.solve()
         return block
 

test/functional/feature_dersig.py

@@ -85,9 +85,7 @@ def run_test(self):
 
         tip = self.nodes[0].getbestblockhash()
         block_time = self.nodes[0].getblockheader(tip)['mediantime'] + 1
-        block = create_block(int(tip, 16), create_coinbase(DERSIG_HEIGHT - 1), block_time)
-        block.vtx.append(spendtx)
-        block.hashMerkleRoot = block.calc_merkle_root()
+        block = create_block(int(tip, 16), create_coinbase(DERSIG_HEIGHT - 1), block_time, txlist=[spendtx])
         block.solve()
 
         assert_equal(self.nodes[0].getblockcount(), DERSIG_HEIGHT - 2)
@@ -100,8 +98,7 @@ def run_test(self):
         self.log.info("Test that blocks must now be at least version 3")
         tip = block.sha256
         block_time += 1
-        block = create_block(tip, create_coinbase(DERSIG_HEIGHT), block_time)
-        block.nVersion = 2
+        block = create_block(tip, create_coinbase(DERSIG_HEIGHT), block_time, version=2)
         block.solve()
 
         with self.nodes[0].assert_debug_log(expected_msgs=[f'{block.hash}, bad-version(0x00000002)']):

test/functional/feature_taproot.py

@@ -1266,12 +1266,8 @@ def block_submit(self, node, txs, msg, err_msg, cb_pubkey=None, fees=0, sigops_w
         # transactions.
         extra_output_script = CScript([OP_CHECKSIG]*((MAX_BLOCK_SIGOPS_WEIGHT - sigops_weight) // WITNESS_SCALE_FACTOR))
 
-        block = create_block(self.tip, create_coinbase(self.lastblockheight + 1, pubkey=cb_pubkey, extra_output_script=extra_output_script, fees=fees), self.lastblocktime + 1)
-        block.nVersion = 4
-        for tx in txs:
-            tx.rehash()
-            block.vtx.append(tx)
-        block.hashMerkleRoot = block.calc_merkle_root()
+        coinbase_tx = create_coinbase(self.lastblockheight + 1, pubkey=cb_pubkey, extra_output_script=extra_output_script, fees=fees)
+        block = create_block(self.tip, coinbase_tx, self.lastblocktime + 1, txlist=txs)
         witness and add_witness_commitment(block)
         block.solve()
         block_response = node.submitblock(block.serialize().hex())

test/functional/feature_versionbits_warning.py

@@ -48,8 +48,7 @@ def send_blocks_with_version(self, peer, numblocks, version):
         tip = int(tip, 16)
 
         for _ in range(numblocks):
-            block = create_block(tip, create_coinbase(height + 1), block_time)
-            block.nVersion = version
+            block = create_block(tip, create_coinbase(height + 1), block_time, version=version)
             block.solve()
             peer.send_message(msg_block(block))
             block_time += 1

test/functional/interface_zmq.py

@@ -18,7 +18,6 @@
 from test_framework.messages import (
     CTransaction,
     hash256,
-    tx_from_hex,
 )
 from test_framework.util import (
     assert_equal,
@@ -402,12 +401,8 @@ def test_sequence(self):
             raw_tx = self.nodes[0].getrawtransaction(orig_txid)
             bump_info = self.nodes[0].bumpfee(orig_txid)
             # Mine the pre-bump tx
-            block = create_block(int(self.nodes[0].getbestblockhash(), 16), create_coinbase(self.nodes[0].getblockcount()+1))
-            tx = tx_from_hex(raw_tx)
-            block.vtx.append(tx)
-            for txid in more_tx:
-                tx = tx_from_hex(self.nodes[0].getrawtransaction(txid))
-                block.vtx.append(tx)
+            txs_to_add = [raw_tx] + [self.nodes[0].getrawtransaction(txid) for txid in more_tx]
+            block = create_block(int(self.nodes[0].getbestblockhash(), 16), create_coinbase(self.nodes[0].getblockcount()+1), txlist=txs_to_add)
             add_witness_commitment(block)
             block.solve()
             assert_equal(self.nodes[0].submitblock(block.serialize().hex()), None)

test/functional/p2p_invalid_block.py

@@ -15,9 +15,14 @@
 import copy
 import time
 
-from test_framework.blocktools import create_block, create_coinbase, create_tx_with_script
+from test_framework.blocktools import (
+    create_block,
+    create_coinbase,
+    create_tx_with_script,
+)
 from test_framework.messages import COIN
 from test_framework.p2p import P2PDataStore
+from test_framework.script import OP_TRUE
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
@@ -66,15 +71,10 @@ def run_test(self):
         # For more information on merkle-root malleability see src/consensus/merkle.cpp.
         self.log.info("Test merkle root malleability.")
 
-        block2 = create_block(tip, create_coinbase(height), block_time)
+        tx1 = create_tx_with_script(block1.vtx[0], 0, script_sig=bytes([OP_TRUE]), amount=50 * COIN)
+        tx2 = create_tx_with_script(tx1, 0, script_sig=bytes([OP_TRUE]), amount=50 * COIN)
+        block2 = create_block(tip, create_coinbase(height), block_time, txlist=[tx1, tx2])
         block_time += 1
-
-        # b'0x51' is OP_TRUE
-        tx1 = create_tx_with_script(block1.vtx[0], 0, script_sig=b'\x51', amount=50 * COIN)
-        tx2 = create_tx_with_script(tx1, 0, script_sig=b'\x51', amount=50 * COIN)
-
-        block2.vtx.extend([tx1, tx2])
-        block2.hashMerkleRoot = block2.calc_merkle_root()
         block2.solve()
         orig_hash = block2.sha256
         block2_orig = copy.deepcopy(block2)
@@ -99,12 +99,8 @@ def run_test(self):
 
         self.log.info("Test very broken block.")
 
-        block3 = create_block(tip, create_coinbase(height), block_time)
+        block3 = create_block(tip, create_coinbase(height, nValue=100), block_time)
         block_time += 1
-        block3.vtx[0].vout[0].nValue = 100 * COIN  # Too high!
-        block3.vtx[0].sha256 = None
-        block3.vtx[0].calc_sha256()
-        block3.hashMerkleRoot = block3.calc_merkle_root()
         block3.solve()
 
         peer.send_blocks_and_test([block3], node, success=False, reject_reason='bad-cb-amount')
@@ -123,14 +119,10 @@ def run_test(self):
 
         # Complete testing of CVE-2018-17144, by checking for the inflation bug.
         # Create a block that spends the output of a tx in a previous block.
-        block4 = create_block(tip, create_coinbase(height), block_time)
-        tx3 = create_tx_with_script(tx2, 0, script_sig=b'\x51', amount=50 * COIN)
-
-        # Duplicates input
-        tx3.vin.append(tx3.vin[0])
+        tx3 = create_tx_with_script(tx2, 0, script_sig=bytes([OP_TRUE]), amount=50 * COIN)
+        tx3.vin.append(tx3.vin[0])  # Duplicates input
         tx3.rehash()
-        block4.vtx.append(tx3)
-        block4.hashMerkleRoot = block4.calc_merkle_root()
+        block4 = create_block(tip, create_coinbase(height), block_time, txlist=[tx3])
         block4.solve()
         self.log.info("Test inflation by duplicating input")
         peer.send_blocks_and_test([block4], node, success=False,  reject_reason='bad-txns-inputs-duplicate')
@@ -140,7 +132,6 @@ def run_test(self):
         node.setmocktime(t)
         # Set block time +1 second past max future validity
         block = create_block(tip, create_coinbase(height), t + MAX_FUTURE_BLOCK_TIME + 1)
-        block.hashMerkleRoot = block.calc_merkle_root()
         block.solve()
         # Need force_send because the block will get rejected without a getdata otherwise
         peer.send_blocks_and_test([block], node, force_send=True, success=False, reject_reason='time-too-new')