Merge bitcoin/bitcoin#34860: mining: always pad scriptSig at low heights, drop include_dummy_extranonce

8544537f41 mining: drop unused include_dummy_extranonce option (Sjors Provoost) 58eeab790d mining: only pad with OP_0 at heights <= 16 (Sjors Provoost) 00d22328b0 mining: pad coinbase to fix createNewBlock at heights <=16 (Sjors Provoost) 605ff37403 test: bad-cb-length for createNewBlock() at low heights (Sjors Provoost) 1966621b76 test: refactor IPC mining test to use script_BIP34_coinbase_height (Sjors Provoost) Pull request description: Blocks 0-16 on any new chain require mining code to be careful not to violate the `bad-cb-length` rule, which states the coinbase transaction scriptSig must be at least 2 bytes. Our mining code deals with that by padding the `scriptSig` with a 0 `extraNonce`. It does this for every height. As a result IPC clients would get an unnecessary `0` in the `scriptSigPrefix` field of `CoinbaseTx`. #32420 fixed that by introducing a `include_dummy_extranonce` option in `BlockCreateOptions` and turning that off for IPC clients. A minor issue was missed though: `createNewBlock()` now fails with `bad-cb-length`. An easy workaround is to use the `generate` RPC for the first 16 blocks, as demonstrated in the 2nd commit. The real fix is to have the miner code always pad the `scriptSig` at lower heights, but to _not_ include that in the `scriptSigPrefix` field of `CoinbaseTx` (introduced in #33819). This is what the 3rd commit implements. Now that we set `scriptSigPrefix` independent of what our internal miner code does - to get past `CheckBlock()` - the original motivation for `include_dummy_extranonce` goes away and we can just drop it entirely. The last commit drops it, while the 4th commit adjusts the tests and hardcoded block and assume utxo hashes. This last change does not break IPC clients, because `include_dummy_extranonce` was never exposed in `mining.capnp`. Instead of adjusting the hardcoded hashes, an alternative approach would be to just always pad the `scriptSig` internally, since we exclude the padding from `scriptSigPrefix` anyway. However, IPC clients can also call `getBlock()` to get the raw block and might be confused about the difference. The miner code is also easier to understand if we limit the exception (`coinbase_tx.script_sig_prefix != coinbaseTx.vin[0].scriptSig`) to `nHeight <= 16`, where the explanation is based purely on consensus rules rather than historical test suite reasons. The first two commits are preperation test changes: - extract `assert_capnp_failed` helper for macOS (also part of #34727) - use `script_BIP34_coinbase_height` in IPC mining test (existing code in `interface_ipc_mining.py` was incorrect for low height Fixes #35126 ACKs for top commit: ryanofsky: Code review ACK 8544537f41. Just rebased to fix silent conflict and applied some minor suggestions since last review. As part of rereviewing I left some more minor suggestions that are fine to ignore. sedited: Re-ACK 8544537f41 Tree-SHA512: a01d48842bf4bcc1a9c51a89ef9d750766db7d04edb4dcd6b3a8bf195c6b4fa07445256a49367ff0db00ab489a52a3d7ff6a5c3ab9290ecb1fcb82f532552e9b
2026-06-12 07:39:41 +02:00 · 2026-05-17 11:21:20 -04:00
parent ed1795aa17 8544537f41
commit 7802e578c3
30 changed files with 119 additions and 84 deletions
--- a/test/functional/feature_assumeutxo.py
+++ b/test/functional/feature_assumeutxo.py
@@ -139,12 +139,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, "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"\xff" * 32, 0, "43bb0fcc50f3789d50ce41891810bbc571d549a32e2bbaabfc036f73ae2a23f9", None],  # wrong outpoint hash
+            [(2).to_bytes(1, "little"), 32, None, "Bad snapshot data after deserializing 2 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, "9f562925721e4f97e6fde5b590dbfede51e2204a68639525062ad064545dd0ea", None],  # wrong outpoint index
-            [b"\x82", 34, "161393f07f8ad71760b3910a914f677f2cb166e5bcf5354e50d46b78c0422d15", None],  # wrong coin code VARINT
-            [b"\x80", 34, "e6fae191ef851554467b68acff01ca09ad0a2e48c9b3dfea46cf7d35a7fd0ad0", None],  # another wrong coin code
+            [b"\x01", 33, "0b0e96fd9a556157a68d9e6fe6e2c7562169c730347565ce0725032f824bea34", None],  # wrong outpoint index
+            [b"\x83", 34, "52084c732f760fbcfb804d854815a9145111e93d7c7f3f7bfcb0ade1ea4f6b99", None],  # wrong coin code VARINT
+            [b"\x80", 34, "c1a1308fcf04cbb14fc777467eb5e98e77a921b66eac4ad47877d0b6ed7b15b1", 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
@@ -163,12 +163,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 d2b051ff5e8eef46520350776f4100dd710a63447a8e01d917e92e79751a63e2, got {wrong_hash}."
+            msg = custom_message if custom_message is not None else f"Bad snapshot content hash: expected 106b2c56233e378a824cf0d5ff2be42ed32c72f1605c9be288d00942908a40ac, 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 (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) 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 (0c552ced4721c249a389eb9b08cb8da261cd46f0e7b5f9d064d48f3113406853) 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 (7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2) is part of an invalid chain."
+            msg = "Unable to load UTXO snapshot: The base block header (0c552ced4721c249a389eb9b08cb8da261cd46f0e7b5f9d064d48f3113406853) is part of an invalid chain."
            assert_raises_rpc_error(-32603, msg, node.loadtxoutset, dump_output_path)
            node.reconsiderblock(block_hash)

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

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

        # Use a hash instead of a height
--- a/test/functional/feature_index_prune.py
+++ b/test/functional/feature_index_prune.py
@@ -100,7 +100,7 @@ class FeatureIndexPruneTest(BitcoinTestFramework):
                pruneheight_new = node.pruneblockchain(400)
                # the prune heights used here and below are magic numbers that are determined by the
                # thresholds at which block files wrap, so they depend on disk serialization and default block file size.
-                assert_equal(pruneheight_new, 248)
+                assert_equal(pruneheight_new, 249)

        self.log.info("check if we can access the tips blockfilter and coinstats when we have pruned some blocks")
        tip = self.nodes[0].getbestblockhash()
@@ -117,8 +117,8 @@ class FeatureIndexPruneTest(BitcoinTestFramework):
            assert node.gettxoutsetinfo(hash_type="muhash", hash_or_height=height_hash)['muhash']

        # mine and sync index up to a height that will later be the pruneheight
-        self.generate(self.nodes[0], 51)
-        self.sync_index(height=751)
+        self.generate(self.nodes[0], 54)
+        self.sync_index(height=754)

        self.restart_without_indices()

@@ -130,12 +130,12 @@ class FeatureIndexPruneTest(BitcoinTestFramework):
            msg = "Querying specific block heights requires coinstatsindex"
            assert_raises_rpc_error(-8, msg, node.gettxoutsetinfo, "muhash", height_hash)

-        self.generate(self.nodes[0], 749)
+        self.generate(self.nodes[0], 746)

        self.log.info("prune exactly up to the indices best blocks while the indices are disabled")
        for i in range(3):
            pruneheight_2 = self.nodes[i].pruneblockchain(1000)
-            assert_equal(pruneheight_2, 750)
+            assert_equal(pruneheight_2, 753)
            # Restart the nodes again with the indices activated
            self.restart_node(i, extra_args=self.extra_args[i])

@@ -195,7 +195,7 @@ class FeatureIndexPruneTest(BitcoinTestFramework):
        for node in self.nodes[:2]:
            with node.assert_debug_log(['limited pruning to height 2489']):
                pruneheight_new = node.pruneblockchain(2500)
-                assert_equal(pruneheight_new, 2005)
+                assert_equal(pruneheight_new, 2013)

        self.log.info("ensure that prune locks don't prevent indices from failing in a reorg scenario")
        with self.nodes[0].assert_debug_log(['basic block filter index prune lock moved back to 2480']):
--- 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'], "e0b4c80f2880985fdf1adc331ed0735ac207588f986c91c7c05e8cf5fe6780f0")
-        assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "8739b878f23030ef39a5547edc7b57f88d50fdaaf47314ff0524608deb13067e")
+        assert_equal(node.gettxoutsetinfo()['hash_serialized_3'], "396058cfd7f3b4c05dcdfaf360be9986d859d2c8315082d3aadda6d2d16add25")
+        assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "97f341d4226cb4451dd9da2856759fb97659cfc238a0b8d5452e64af6032bd3d")

    def run_test(self):
        self.test_muhash_implementation()
--- a/test/functional/interface_ipc_mining.py
+++ b/test/functional/interface_ipc_mining.py
@@ -7,7 +7,7 @@ import asyncio
 import time
 from contextlib import AsyncExitStack
 from io import BytesIO
-from test_framework.blocktools import NULL_OUTPOINT
+from test_framework.blocktools import NULL_OUTPOINT, script_BIP34_coinbase_height
 from test_framework.messages import (
    MAX_BLOCK_WEIGHT,
    CBlockHeader,
@@ -20,10 +20,6 @@ from test_framework.messages import (
    from_hex,
    msg_headers,
 )
-from test_framework.script import (
-    CScript,
-    CScriptNum,
-)
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
    assert_equal,
@@ -67,7 +63,7 @@ class IPCMiningTest(BitcoinTestFramework):
        # as it is being called before knowing whether capnp is available).
        self.capnp_modules = load_capnp_modules(self.config)

-    async def build_coinbase_test(self, template, ctx, miniwallet):
+    async def build_coinbase_test(self, template, ctx, miniwallet, extra_nonce=b""):
        self.log.debug("Build coinbase transaction using getCoinbaseTx()")
        assert template is not None
        coinbase_res = await mining_get_coinbase_tx(template, ctx)
@@ -79,11 +75,11 @@ class IPCMiningTest(BitcoinTestFramework):

        # Verify there's no dummy extraNonce in the coinbase scriptSig
        current_block_height = self.nodes[0].getchaintips()[0]["height"]
-        expected_scriptsig = CScript([CScriptNum(current_block_height + 1)])
-        assert_equal(coinbase_res.scriptSigPrefix.hex(), expected_scriptsig.hex())
+        bip34_prefix = script_BIP34_coinbase_height(current_block_height + 1, padding=False)
+        assert_equal(coinbase_res.scriptSigPrefix, bip34_prefix)

        # Typically a mining pool appends its name and an extraNonce
-        coinbase_tx.vin[0].scriptSig = coinbase_res.scriptSigPrefix
+        coinbase_tx.vin[0].scriptSig = coinbase_res.scriptSigPrefix + extra_nonce

        # We currently always provide a coinbase witness, even for empty
        # blocks, but this may change, so always check:
@@ -407,6 +403,50 @@ class IPCMiningTest(BitcoinTestFramework):

        asyncio.run(capnp.run(async_routine()))

+    def run_low_height_test(self):
+        """Test that IPC createNewBlock() works at low block heights on a
+        clean chain, in particular with regard to bad-cb-length.
+
+        createNewBlock pads the scriptSig with a dummy extranonce to pass
+        its internal CheckBlock(). This dummy is omitted from the
+        getCoinbaseTx() script_sig_prefix field. The client provides its
+        own extraNonce via submitSolution()."""
+        self.log.info("Running low block height test")
+
+        node = self.nodes[0]
+        self.stop_node(0)
+        # Clear chain data to start from genesis
+        self.cleanup_folder(node.chain_path)
+        node.start()
+        node.wait_for_rpc_connection()
+        assert_equal(node.getblockcount(), 0)
+
+        miniwallet = MiniWallet(node)
+
+        async def async_routine():
+            ctx, mining = await make_mining_ctx(self)
+            opts = self.capnp_modules['mining'].BlockCreateOptions()
+
+            # Mine blocks 1-17 to exercise the boundary at height 16, where the
+            # internal scriptSig padding is no longer needed.
+            for height in range(1, 18):
+                async with AsyncExitStack() as stack:
+                    # Disable cooldown to avoid hanging in the IBD loop on a fresh chain
+                    template = await mining_create_block_template(mining, stack, ctx, opts, cooldown=False)
+                    assert template is not None
+                    block = await mining_get_block(template, ctx)
+                    # Heights <= 16 need extra nonce padding.
+                    extra_nonce = b'\xaa\xbb\xcc\xdd' if height <= 16 else b""
+                    coinbase = await self.build_coinbase_test(template, ctx, miniwallet, extra_nonce=extra_nonce)
+                    block.vtx[0] = coinbase
+                    block.hashMerkleRoot = block.calc_merkle_root()
+                    block.solve()
+                    submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result
+                    assert_equal(submitted, True)
+                    assert_equal(node.getblockcount(), height)
+
+        asyncio.run(capnp.run(async_routine()))
+
    def run_test(self):
        self.miniwallet = MiniWallet(self.nodes[0])
        self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions()
@@ -416,6 +456,9 @@ class IPCMiningTest(BitcoinTestFramework):
        self.run_coinbase_and_submission_test()
        self.run_ipc_option_override_test()

+        # Needs to run last because it resets the chain.
+        self.run_low_height_test()
+

 if __name__ == '__main__':
    IPCMiningTest(__file__).main()
--- a/test/functional/rpc_dumptxoutset.py
+++ b/test/functional/rpc_dumptxoutset.py
@@ -68,15 +68,15 @@ class DumptxoutsetTest(BitcoinTestFramework):
        # Blockhash should be deterministic based on mocked time.
        assert_equal(
            out['base_hash'],
-            '6885775faa46290bedfa071f22d0598c93f1d7e01f24607c4dedd69b9baa4a8f')
+            '220aee93f0f5409631f35488898258f0930952bd620063cb4d7d87f7c28a8f50')

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

        assert_equal(
-            out['txoutset_hash'], 'd4453995f4f20db7bb3a604afd10d7128e8ee11159cde56d5b2fd7f55be7c74c')
+            out['txoutset_hash'], '771d773b5c27b6f35f598ce764652a2cf28fbc268341eb1827844e416c629c7d')
        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
@@ -128,7 +128,7 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
        self.generate(self.nodes[0], 400, sync_fun=self.no_op)
        self.sync_blocks([self.nodes[0], pruned_node])
        pruneheight = pruned_node.pruneblockchain(300)
-        assert_equal(pruneheight, 248)
+        assert_equal(pruneheight, 249)
        # Ensure the block is actually pruned
        pruned_block = self.nodes[0].getblockhash(2)
        assert_raises_rpc_error(-1, "Block not available (pruned data)", pruned_node.getblock, pruned_block)
@@ -144,14 +144,14 @@ class GetBlockFromPeerTest(BitcoinTestFramework):
        self.log.info("Fetched block persists after next pruning event")
        self.generate(self.nodes[0], 250, sync_fun=self.no_op)
        self.sync_blocks([self.nodes[0], pruned_node])
-        pruneheight += 251
+        pruneheight += 252
        assert_equal(pruned_node.pruneblockchain(700), pruneheight)
        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)
        self.sync_blocks([self.nodes[0], pruned_node])
-        pruneheight += 250
+        pruneheight += 251
        assert_equal(pruned_node.pruneblockchain(1000), pruneheight)
        assert_raises_rpc_error(-1, "Block not available (pruned data)", pruned_node.getblock, pruned_block)

--- a/test/functional/test_framework/blocktools.py
+++ b/test/functional/test_framework/blocktools.py
@@ -161,11 +161,13 @@ def add_witness_commitment(block, nonce=0):
    block.hashMerkleRoot = block.calc_merkle_root()


-def script_BIP34_coinbase_height(height):
+def script_BIP34_coinbase_height(height, *, padding=True):
    if height <= 16:
        res = CScriptOp.encode_op_n(height)
-        # Append dummy extraNonce to increase scriptSig size to 2 (see bad-cb-length consensus rule)
-        return CScript([res, OP_0])
+        if padding:
+            # Append dummy extraNonce to increase scriptSig size to 2 (see bad-cb-length consensus rule)
+            return CScript([res, OP_0])
+        return CScript([res])
    return CScript([CScriptNum(height)])


--- a/test/functional/test_framework/ipc_util.py
+++ b/test/functional/test_framework/ipc_util.py
@@ -109,9 +109,9 @@ async def make_capnp_init_ctx(self):
    return ctx, init


-async def mining_create_block_template(mining, stack, ctx, opts):
+async def mining_create_block_template(mining, stack, ctx, *args, **kwargs):
    """Call mining.createNewBlock() and return template, then call template.destroy() when stack exits."""
-    response = await mining.createNewBlock(ctx, opts)
+    response = await mining.createNewBlock(ctx, *args, **kwargs)
    if not response._has("result"):
        return None
    return await stack.enter_async_context(destroying(response.result, ctx))
--- a/test/functional/tool_bitcoin_chainstate.py
+++ b/test/functional/tool_bitcoin_chainstate.py
@@ -20,7 +20,7 @@ from test_framework.wallet import MiniWallet
 START_HEIGHT = 199
 # Hardcoded in regtest chainparams
 SNAPSHOT_BASE_BLOCK_HEIGHT = 299
-SNAPSHOT_BASE_BLOCK_HASH = "7cc695046fec709f8c9394b6f928f81e81fd3ac20977bb68760fa1faa7916ea2"
+SNAPSHOT_BASE_BLOCK_HASH = "0c552ced4721c249a389eb9b08cb8da261cd46f0e7b5f9d064d48f3113406853"


 class BitcoinChainstateTest(BitcoinTestFramework):
--- a/test/functional/wallet_assumeutxo.py
+++ b/test/functional/wallet_assumeutxo.py
@@ -172,7 +172,7 @@ class AssumeutxoTest(BitcoinTestFramework):

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