Merge bitcoin/bitcoin#32155: miner: timelock the coinbase to the mined block's height

a58cb3b1c1 qa: sanity check mined block have their coinbase timelocked to height (Antoine Poinsot) 8f2078af6a miner: timelock coinbase transactions (Antoine Poinsot) 788aeebf34 qa: use prev height as nLockTime for coinbase txs created in unit tests (Antoine Poinsot) c76dbe9b8b qa: timelock coinbase transactions created in fuzz targets (Antoine Poinsot) 9c94069d8b contrib: timelock coinbase transactions in signet miner (Antoine Poinsot) a5f52cfcc4 qa: timelock coinbase transactions created in functional tests (Antoine Poinsot) Pull request description: The Consensus Cleanup soft fork proposal includes enforcing that coinbase transactions set their nLockTime field to the block height minus 1, as well as their nSequence such as to not disable the timelock. If such a fork were to be activated by Bitcoin users, miners need to be ready to produce compliant blocks at the risk of losing substantial amounts mining would-be invalid blocks. As miners are unfamously slow to upgrade, it's good to make this change as early as possible. Although Bitcoin Core's GBT implementation does not provide the `coinbasetxn` field, and mining pool software crafts the coinbase on its own, updating the Bitcoin Core mining code is a first step toward convincing pools to update their (often closed source) code. A possible followup is also to introduce new fields to GBT. In addition, this first step also makes it possible to test future Consensus Cleanup changes. The commit making the change also updates a bunch of seemingly-unrelated tests. This is because those tests were asserting error messages based on the txid of transactions involved, and changing the coinbase transaction structure necessarily changes the txid of all tests' transactions. ACKs for top commit: Sjors: Code review ACK a58cb3b1c1 achow101: ACK a58cb3b1c1 TheCharlatan: Re-ACK a58cb3b1c1 Tree-SHA512: a2aae009a187eb760d34435f518a895ee76c6b02a667eb030ddf6bd584da6e8eae2737d974dbf81a928d60c07bcb4820f055adc067e18d8819640db0240bb513
2025-08-30 00:51:08 +02:00 · 2025-05-09 15:09:27 -07:00
parent 6c6ef58b0b a58cb3b1c1
commit 19b1e177d6
21 changed files with 88 additions and 58 deletions
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -140,12 +140,12 @@ class AssumeutxoTest(BitcoinTestFramework):
        self.log.info("  - snapshot file with alternated but parsable UTXO data results in different hash")
        cases = [
            # (content, offset, wrong_hash, custom_message)
-            [b"\xff" * 32, 0, "7d52155c9a9fdc4525b637ef6170568e5dad6fabd0b1fdbb9432010b8453095b", None],  # wrong outpoint hash
-            [(2).to_bytes(1, "little"), 32, None, "Bad snapshot data after deserializing 1 coins."],  # wrong txid coins count
+            [b"\xff" * 32, 0, "77874d48d932a5cb7a7f770696f5224ff05746fdcf732a58270b45da0f665934", None],  # wrong outpoint hash
+            [(2).to_bytes(1, "little"), 32, None, "Bad snapshot format or truncated snapshot after deserializing 1 coins."],  # wrong txid coins count
            [b"\xfd\xff\xff", 32, None, "Mismatch in coins count in snapshot metadata and actual snapshot data"],  # txid coins count exceeds coins left
-            [b"\x01", 33, "9f4d897031ab8547665b4153317ae2fdbf0130c7840b66427ebc48b881cb80ad", None],  # wrong outpoint index
-            [b"\x81", 34, "3da966ba9826fb6d2604260e01607b55ba44e1a5de298606b08704bc62570ea8", None],  # wrong coin code VARINT
-            [b"\x80", 34, "091e893b3ccb4334378709578025356c8bcb0a623f37c7c4e493133c988648e5", None],  # another wrong coin code
+            [b"\x01", 33, "9f562925721e4f97e6fde5b590dbfede51e2204a68639525062ad064545dd0ea", None],  # wrong outpoint index
+            [b"\x82", 34, "161393f07f8ad71760b3910a914f677f2cb166e5bcf5354e50d46b78c0422d15", None],  # wrong coin code VARINT
+            [b"\x80", 34, "e6fae191ef851554467b68acff01ca09ad0a2e48c9b3dfea46cf7d35a7fd0ad0", None],  # another wrong coin code
            [b"\x84\x58", 34, None, "Bad snapshot data after deserializing 0 coins"],  # wrong coin case with height 364 and coinbase 0
            [
                # compressed txout value + scriptpubkey
@@ -164,12 +164,12 @@ class AssumeutxoTest(BitcoinTestFramework):
                f.write(content)
                f.write(valid_snapshot_contents[(5 + 2 + 4 + 32 + 8 + offset + len(content)):])

-            msg = custom_message if custom_message is not None else f"Bad snapshot content hash: expected a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27, got {wrong_hash}."
+            msg = custom_message if custom_message is not None else f"Bad snapshot content hash: expected d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2, got {wrong_hash}."
            expected_error(msg)

    def test_headers_not_synced(self, valid_snapshot_path):
        for node in self.nodes[1:]:
-            msg = "Unable to load UTXO snapshot: The base block header (3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again."
+            msg = "Unable to load UTXO snapshot: The base block header (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) must appear in the headers chain. Make sure all headers are syncing, and call loadtxoutset again."
            assert_raises_rpc_error(-32603, msg, node.loadtxoutset, valid_snapshot_path)

    def test_invalid_chainstate_scenarios(self):
@@ -228,7 +228,7 @@ class AssumeutxoTest(BitcoinTestFramework):
            block_hash = node.getblockhash(height)
            node.invalidateblock(block_hash)
            assert_equal(node.getblockcount(), height - 1)
-            msg = "Unable to load UTXO snapshot: The base block header (3bb7ce5eba0be48939b7a521ac1ba9316afee2c7bada3a0cca24188e6d7d96c0) is part of an invalid chain."
+            msg = "Unable to load UTXO snapshot: The base block header (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) is part of an invalid chain."
            assert_raises_rpc_error(-32603, msg, node.loadtxoutset, dump_output_path)
            node.reconsiderblock(block_hash)

@@ -446,7 +446,7 @@ class AssumeutxoTest(BitcoinTestFramework):
        def check_dump_output(output):
            assert_equal(
                output['txoutset_hash'],
-                "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27")
+                "d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2")
            assert_equal(output["nchaintx"], blocks[SNAPSHOT_BASE_HEIGHT].chain_tx)

        check_dump_output(dump_output)
@@ -476,7 +476,7 @@ class AssumeutxoTest(BitcoinTestFramework):
        dump_output4 = n0.dumptxoutset(path='utxos4.dat', rollback=prev_snap_height)
        assert_equal(
            dump_output4['txoutset_hash'],
-            "8a1db0d6e958ce0d7c963bc6fc91ead596c027129bacec68acc40351037b09d7")
+            "45ac2777b6ca96588210e2a4f14b602b41ec37b8b9370673048cc0af434a1ec8")
        assert_not_equal(sha256sum_file(dump_output['path']), sha256sum_file(dump_output4['path']))

        # Use a hash instead of a height
--- a/test/functional/feature_block.py
+++ b/test/functional/feature_block.py
@@ -831,6 +831,7 @@ class FullBlockTest(BitcoinTestFramework):
        self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)")
        self.move_tip(60)
        b61 = self.next_block(61)
+        b61.vtx[0].nLockTime = 0
        b61.vtx[0].vin[0].scriptSig = DUPLICATE_COINBASE_SCRIPT_SIG
        b61.vtx[0].rehash()
        b61 = self.update_block(61, [])
@@ -853,6 +854,7 @@ class FullBlockTest(BitcoinTestFramework):
        b_spend_dup_cb = self.update_block('spend_dup_cb', [tx])

        b_dup_2 = self.next_block('dup_2')
+        b_dup_2.vtx[0].nLockTime = 0
        b_dup_2.vtx[0].vin[0].scriptSig = DUPLICATE_COINBASE_SCRIPT_SIG
        b_dup_2.vtx[0].rehash()
        b_dup_2 = self.update_block('dup_2', [])
--- a/test/functional/feature_utxo_set_hash.py
+++ b/test/functional/feature_utxo_set_hash.py
@@ -67,8 +67,8 @@ class UTXOSetHashTest(BitcoinTestFramework):
        assert_equal(finalized[::-1].hex(), node_muhash)

        self.log.info("Test deterministic UTXO set hash results")
-        assert_equal(node.gettxoutsetinfo()['hash_serialized_3'], "d1c7fec1c0623f6793839878cbe2a531eb968b50b27edd6e2a57077a5aed6094")
-        assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "d1725b2fe3ef43e55aa4907480aea98d406fc9e0bf8f60169e2305f1fbf5961b")
+        assert_equal(node.gettxoutsetinfo()['hash_serialized_3'], "e0b4c80f2880985fdf1adc331ed0735ac207588f986c91c7c05e8cf5fe6780f0")
+        assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "8739b878f23030ef39a5547edc7b57f88d50fdaaf47314ff0524608deb13067e")

    def run_test(self):
        self.test_muhash_implementation()
--- a/test/functional/mempool_package_rbf.py
+++ b/test/functional/mempool_package_rbf.py
@@ -384,7 +384,7 @@ class PackageRBFTest(BitcoinTestFramework):
        # Now make conflicting packages for each coin
        package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
        package_result = node.submitpackage(package_hex1)
-        assert_equal(f"package RBF failed: {child_result['tx'].rehash()} has 2 ancestors, max 1 allowed", package_result["package_msg"])
+        assert_equal(f"package RBF failed: {parent1_result['tx'].rehash()} is not the only parent of child {child_result['tx'].rehash()}", package_result["package_msg"])

        package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
        package_result = node.submitpackage(package_hex2)
@@ -438,7 +438,7 @@ class PackageRBFTest(BitcoinTestFramework):
        # Now make conflicting packages for each coin
        package_hex1, _package_txns1 = self.create_simple_package(coin1, DEFAULT_FEE, DEFAULT_CHILD_FEE)
        package_result = node.submitpackage(package_hex1)
-        assert_equal(f"package RBF failed: {child2_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"])
+        assert_equal(f"package RBF failed: {child1_result['tx'].rehash()} is not the only child of parent {parent_result['tx'].rehash()}", package_result["package_msg"])

        package_hex2, _package_txns2 = self.create_simple_package(coin2, DEFAULT_FEE, DEFAULT_CHILD_FEE)
        package_result = node.submitpackage(package_hex2)
--- a/test/functional/mining_basic.py
+++ b/test/functional/mining_basic.py
@@ -28,9 +28,10 @@ from test_framework.messages import (
    COIN,
    DEFAULT_BLOCK_RESERVED_WEIGHT,
    MAX_BLOCK_WEIGHT,
+    MAX_SEQUENCE_NONFINAL,
    MINIMUM_BLOCK_RESERVED_WEIGHT,
    ser_uint256,
-    WITNESS_SCALE_FACTOR
+    WITNESS_SCALE_FACTOR,
 )
 from test_framework.p2p import P2PDataStore
 from test_framework.test_framework import BitcoinTestFramework
@@ -362,6 +363,12 @@ class MiningTest(BitcoinTestFramework):
            expected_msg=f"Error: Specified -blockmaxweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})",
        )

+    def test_height_in_locktime(self):
+        self.log.info("Sanity check generated blocks have their coinbase timelocked to their height.")
+        self.generate(self.nodes[0], 1, sync_fun=self.no_op)
+        block = self.nodes[0].getblock(self.nodes[0].getbestblockhash(), 2)
+        assert_equal(block["tx"][0]["locktime"], block["height"] - 1)
+        assert_equal(block["tx"][0]["vin"][0]["sequence"], MAX_SEQUENCE_NONFINAL)

    def run_test(self):
        node = self.nodes[0]
@@ -592,6 +599,7 @@ class MiningTest(BitcoinTestFramework):
        self.test_block_max_weight()
        self.test_timewarp()
        self.test_pruning()
+        self.test_height_in_locktime()


 if __name__ == '__main__':
--- a/test/functional/mining_mainnet.py
+++ b/test/functional/mining_mainnet.py
@@ -30,6 +30,7 @@ from test_framework.blocktools import (

 from test_framework.messages import (
    CBlock,
+    SEQUENCE_FINAL,
 )

 import json
@@ -61,6 +62,10 @@ class MiningMainnetTest(BitcoinTestFramework):
        block.nBits = DIFF_1_N_BITS
        block.nNonce = blocks['nonces'][height - 1]
        block.vtx = [create_coinbase(height=height, script_pubkey=bytes.fromhex(COINBASE_SCRIPT_PUBKEY), retarget_period=2016)]
+        # The alternate mainnet chain was mined with non-timelocked coinbase txs.
+        block.vtx[0].nLockTime = 0
+        block.vtx[0].vin[0].nSequence = SEQUENCE_FINAL
+        block.vtx[0].rehash()
        block.hashMerkleRoot = block.calc_merkle_root()
        block.rehash()
        block_hex = block.serialize(with_witness=False).hex()
--- a/test/functional/rpc_dumptxoutset.py
+++ b/test/functional/rpc_dumptxoutset.py
@@ -49,15 +49,15 @@ class DumptxoutsetTest(BitcoinTestFramework):
        # Blockhash should be deterministic based on mocked time.
        assert_equal(
            out['base_hash'],
-            '09abf0e7b510f61ca6cf33bab104e9ee99b3528b371d27a2d4b39abb800fba7e')
+            '6885775faa46290bedfa071f22d0598c93f1d7e01f24607c4dedd69b9baa4a8f')

        # UTXO snapshot hash should be deterministic based on mocked time.
        assert_equal(
            sha256sum_file(str(expected_path)).hex(),
-            '31fcdd0cf542a4b1dfc13c3c05106620ce48951ef62907dd8e5e8c15a0aa993b')
+            'd9506d541437f5e2892d6b6ea173f55233de11601650c157a27d8f2b9d08cb6f')

        assert_equal(
-            out['txoutset_hash'], 'a0b7baa3bf5ccbd3279728f230d7ca0c44a76e9923fca8f32dbfd08d65ea496a')
+            out['txoutset_hash'], 'd4453995f4f20db7bb3a604afd10d7128e8ee11159cde56d5b2fd7f55be7c74c')
        assert_equal(out['nchaintx'], 101)

        # Specifying a path to an existing or invalid file will fail.
--- a/test/functional/rpc_getblockfrompeer.py
+++ b/test/functional/rpc_getblockfrompeer.py
@@ -143,7 +143,7 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
        self.sync_blocks([self.nodes[0], pruned_node])
        pruneheight += 251
        assert_equal(pruned_node.pruneblockchain(700), pruneheight)
-        assert_equal(pruned_node.getblock(pruned_block)["hash"], "36c56c5b5ebbaf90d76b0d1a074dcb32d42abab75b7ec6fa0ffd9b4fbce8f0f7")
+        assert_equal(pruned_node.getblock(pruned_block)["hash"], "196ee3a1a6db2353965081c48ef8e6b031cb2115d084bec6fec937e91a2c6277")

        self.log.info("Fetched block can be pruned again when prune height exceeds the height of the tip at the time when the block was fetched")
        self.generate(self.nodes[0], 250, sync_fun=self.no_op)
--- a/test/functional/test_framework/blocktools.py
+++ b/test/functional/test_framework/blocktools.py
@@ -29,6 +29,7 @@ from .messages import (
    tx_from_hex,
    uint256_from_compact,
    WITNESS_SCALE_FACTOR,
+    MAX_SEQUENCE_NONFINAL,
 )
 from .script import (
    CScript,
@@ -151,7 +152,8 @@ def create_coinbase(height, pubkey=None, *, script_pubkey=None, extra_output_scr
    If extra_output_script is given, make a 0-value output to that
    script. This is useful to pad block weight/sigops as needed. """
    coinbase = CTransaction()
-    coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff), script_BIP34_coinbase_height(height), SEQUENCE_FINAL))
+    coinbase.nLockTime = height - 1
+    coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff), script_BIP34_coinbase_height(height), MAX_SEQUENCE_NONFINAL))
    coinbaseoutput = CTxOut()
    coinbaseoutput.nValue = nValue * COIN
    if nValue == 50:
--- a/test/functional/wallet_assumeutxo.py
+++ b/test/functional/wallet_assumeutxo.py
@@ -118,7 +118,7 @@ class AssumeutxoTest(BitcoinTestFramework):

        assert_equal(
            dump_output['txoutset_hash'],
-            "a4bf3407ccb2cc0145c49ebba8fa91199f8a3903daf0883875941497d2493c27")
+            "d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2")
        assert_equal(dump_output["nchaintx"], 334)
        assert_equal(n0.getblockchaininfo()["blocks"], SNAPSHOT_BASE_HEIGHT)