From 78df9003d63414e4a17b686af7647aeefd706ec5 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Sat, 1 Nov 2025 04:46:17 +1000 Subject: [PATCH 1/3] [doc] Update comments on dummy extraNonces in tests --- src/test/util/mining.cpp | 1 + src/test/validation_block_tests.cpp | 2 +- test/functional/test_framework/blocktools.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index da15e6789a..05f1be77ee 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -49,6 +49,7 @@ std::vector> CreateBlockChain(size_t total_height, const coinbase_tx.vout.resize(1); coinbase_tx.vout[0].scriptPubKey = P2WSH_OP_TRUE; coinbase_tx.vout[0].nValue = GetBlockSubsidy(height + 1, params.GetConsensus()); + // Always include OP_0 as a dummy extraNonce. coinbase_tx.vin[0].scriptSig = CScript() << (height + 1) << OP_0; block.vtx = {MakeTransactionRef(std::move(coinbase_tx))}; diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index 2303fd4cc7..dfa66bb889 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -82,7 +82,7 @@ std::shared_ptr MinerTestingSetup::Block(const uint256& prev_hash) txCoinbase.vout[1].nValue = txCoinbase.vout[0].nValue; txCoinbase.vout[0].nValue = 0; txCoinbase.vin[0].scriptWitness.SetNull(); - // Always pad with OP_0 at the end to avoid bad-cb-length error + // Always pad with OP_0 as dummy extraNonce (also avoids bad-cb-length error for block <=16) const int prev_height{WITH_LOCK(::cs_main, return m_node.chainman->m_blockman.LookupBlockIndex(prev_hash)->nHeight)}; txCoinbase.vin[0].scriptSig = CScript{} << prev_height + 1 << OP_0; txCoinbase.nLockTime = static_cast(prev_height); diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 8ad8ea8a52..e1453e3a99 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -164,7 +164,7 @@ def add_witness_commitment(block, nonce=0): def script_BIP34_coinbase_height(height): if height <= 16: res = CScriptOp.encode_op_n(height) - # Append dummy to increase scriptSig size to 2 (see bad-cb-length consensus rule) + # Append dummy extraNonce to increase scriptSig size to 2 (see bad-cb-length consensus rule) return CScript([res, OP_0]) return CScript([CScriptNum(height)]) From bf3b5d6d069a0bbb39af0c487fd597257f862f31 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Tue, 27 Jan 2026 14:45:04 +0100 Subject: [PATCH 2/3] test: clarify getCoinbaseRawTx() comparison The code comment mistakingly referred to "the deprecated getCoinbaseTx()", instead of getCoinbaseRawTx. This was missed in d59b4cdb5772917ee13e48552d51662160104b62. Also rename parse_and_deserialize_coinbase_tx to make it more clear it refers to the deprecated method. Finally, this commit drops the getCoinbaseRawTx() call when testing template inspectors. The coinbase input check here is already covered by build_coinbase_test. --- test/functional/interface_ipc.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py index c7183c1bcb..47589cbcec 100755 --- a/test/functional/interface_ipc.py +++ b/test/functional/interface_ipc.py @@ -144,7 +144,7 @@ class IPCInterfaceTest(BitcoinTestFramework): block.deserialize(block_data) return block - async def parse_and_deserialize_coinbase_tx(self, block_template, ctx): + async def get_coinbase_raw_tx(self, block_template, ctx): assert block_template is not None coinbase_data = BytesIO((await block_template.getCoinbaseRawTx(ctx)).result) tx = CTransaction() @@ -224,8 +224,9 @@ class IPCInterfaceTest(BitcoinTestFramework): coinbase_tx.nLockTime = coinbase_res.lockTime - # Compare to dummy coinbase provided by the deprecated getCoinbaseTx() - coinbase_legacy = await self.parse_and_deserialize_coinbase_tx(template, ctx) + # Compare to dummy coinbase transaction provided by the deprecated + # getCoinbaseRawTx() + coinbase_legacy = await self.get_coinbase_raw_tx(template, ctx) assert_equal(coinbase_legacy.vout[0].nValue, coinbase_res.blockRewardRemaining) # Swap dummy output for our own coinbase_legacy.vout[0].scriptPubKey = coinbase_tx.vout[0].scriptPubKey @@ -282,10 +283,6 @@ class IPCInterfaceTest(BitcoinTestFramework): assert_equal(len(txfees.result), 0) txsigops = await template.getTxSigops(ctx) assert_equal(len(txsigops.result), 0) - coinbase_data = BytesIO((await template.getCoinbaseRawTx(ctx)).result) - coinbase = CTransaction() - coinbase.deserialize(coinbase_data) - assert_equal(coinbase.vin[0].prevout.hash, 0) self.log.debug("Wait for a new template") waitoptions = self.capnp_modules['mining'].BlockWaitOptions() From d511adb664edcfb97be44bc0738f49b679240504 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 14 Jan 2026 10:02:33 +0100 Subject: [PATCH 3/3] [miner] omit dummy extraNonce via IPC Previously the coinbase transaction generated by our miner code was not used downstream, because the getblocktemplate RPC excludes it. Since the Mining IPC interface was introduced in #30200 we do expose this dummy coinbase transaction. In Stratum v2 several parts of it are communicated downstream, including the scriptSig. This commit removes the dummy extraNonce from the coinbase scriptSig in block templates requested via IPC. This limits the scriptSig to what is essential for consensus (BIP34) and removes the need for external mining software to remove the dummy, or even ignore the scriptSig we provide and generate it some other way. This could cause problems if a future soft fork requires additional data to be committed here. A test is added to verify the new IPC behavior. It achieves this by introducing an include_dummy_extranonce option which defaults to false with all test code updated to set it to true. Because this option is not exposed via IPC, callers will no longer see it. The caller needs to ensure that for blocks 1 through 16 they pad the scriptSig in order to avoid bad-cb-length. Co-authored-by: Anthony Towns --- src/bench/block_assemble.cpp | 1 + src/node/miner.cpp | 17 ++++++++++++----- src/node/types.h | 4 ++++ src/rpc/mining.cpp | 6 +++--- src/test/blockfilter_index_tests.cpp | 1 + src/test/fuzz/package_eval.cpp | 1 + src/test/fuzz/process_message.cpp | 4 +++- src/test/fuzz/process_messages.cpp | 4 +++- src/test/fuzz/tx_pool.cpp | 2 ++ src/test/fuzz/utxo_total_supply.cpp | 1 + src/test/miner_tests.cpp | 4 ++++ src/test/peerman_tests.cpp | 4 +++- src/test/testnet4_miner_tests.cpp | 1 + src/test/util/mining.cpp | 2 ++ src/test/util/setup_common.cpp | 1 + src/test/validation_block_tests.cpp | 2 ++ test/functional/interface_ipc.py | 12 +++++++++++- 17 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/bench/block_assemble.cpp b/src/bench/block_assemble.cpp index 297465be80..702f2c0936 100644 --- a/src/bench/block_assemble.cpp +++ b/src/bench/block_assemble.cpp @@ -30,6 +30,7 @@ static void AssembleBlock(benchmark::Bench& bench) witness.stack.push_back(WITNESS_STACK_ELEM_OP_TRUE); BlockAssembler::Options options; options.coinbase_output_script = P2WSH_OP_TRUE; + options.include_dummy_extranonce = true; // Collect some loose transactions that spend the coinbases of our mined blocks constexpr size_t NUM_BLOCKS{200}; diff --git a/src/node/miner.cpp b/src/node/miner.cpp index 58b147ca2e..b5073054eb 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -177,11 +177,17 @@ std::unique_ptr BlockAssembler::CreateNewBlock() coinbase_tx.block_reward_remaining = block_reward; // Start the coinbase scriptSig with the block height as required by BIP34. - // The trailing OP_0 (historically an extranonce) is optional padding and - // could be removed without a consensus change. Mining clients are expected - // to append extra data to this prefix, so increasing its length would reduce - // the space they can use and may break existing clients. - coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0; + // Mining clients are expected to append extra data to this prefix, so + // increasing its length would reduce the space they can use and may break + // existing clients. + coinbaseTx.vin[0].scriptSig = CScript() << nHeight; + if (m_options.include_dummy_extranonce) { + // For blocks at heights <= 16, the BIP34-encoded height alone is only + // one byte. Consensus requires coinbase scriptSigs to be at least two + // bytes long (bad-cb-length), so tests and regtest include a dummy + // extraNonce (OP_0) + coinbaseTx.vin[0].scriptSig << OP_0; + } coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig; Assert(nHeight > 0); coinbaseTx.nLockTime = static_cast(nHeight - 1); @@ -212,6 +218,7 @@ std::unique_ptr BlockAssembler::CreateNewBlock() pblock->nNonce = 0; if (m_options.test_block_validity) { + // if nHeight <= 16, and include_dummy_extranonce=false this will fail due to bad-cb-length. if (BlockValidationState state{TestBlockValidity(m_chainstate, *pblock, /*check_pow=*/false, /*check_merkle_root=*/false)}; !state.IsValid()) { throw std::runtime_error(strprintf("TestBlockValidity failed: %s", state.ToString())); } diff --git a/src/node/types.h b/src/node/types.h index 6930672f02..deab1faacb 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -67,6 +67,10 @@ struct BlockCreateOptions { * coinbase_max_additional_weight and coinbase_output_max_additional_sigops. */ CScript coinbase_output_script{CScript() << OP_TRUE}; + /** + * Whether to include an OP_0 as a dummy extraNonce in the template's coinbase + */ + bool include_dummy_extranonce{false}; }; struct BlockWaitOptions { diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 9c357f87cb..d0efa84fb7 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -165,7 +165,7 @@ static UniValue generateBlocks(ChainstateManager& chainman, Mining& miner, const { UniValue blockHashes(UniValue::VARR); while (nGenerate > 0 && !chainman.m_interrupt) { - std::unique_ptr block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script })); + std::unique_ptr block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true })); CHECK_NONFATAL(block_template); std::shared_ptr block_out; @@ -376,7 +376,7 @@ static RPCHelpMan generateblock() { LOCK(chainman.GetMutex()); { - std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script})}; + std::unique_ptr block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true})}; CHECK_NONFATAL(block_template); block = block_template->getBlock(); @@ -871,7 +871,7 @@ static RPCHelpMan getblocktemplate() time_start = GetTime(); // Create new block - block_template = miner.createNewBlock(); + block_template = miner.createNewBlock({.include_dummy_extranonce = true}); CHECK_NONFATAL(block_template); diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp index e970ae9cbb..d7d10dfb1a 100644 --- a/src/test/blockfilter_index_tests.cpp +++ b/src/test/blockfilter_index_tests.cpp @@ -69,6 +69,7 @@ CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev, { BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; + options.include_dummy_extranonce = true; std::unique_ptr pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(); CBlock& block = pblocktemplate->block; block.hashPrevBlock = prev->GetBlockHash(); diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp index 1cc2caa396..93cd8ad6bb 100644 --- a/src/test/fuzz/package_eval.cpp +++ b/src/test/fuzz/package_eval.cpp @@ -46,6 +46,7 @@ void initialize_tx_pool() BlockAssembler::Options options; options.coinbase_output_script = P2WSH_EMPTY; + options.include_dummy_extranonce = true; for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) { COutPoint prevout{MineBlock(g_setup->m_node, options)}; diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index ef8cb686ce..7a24c1de34 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -41,7 +41,9 @@ void ResetChainman(TestingSetup& setup) setup.m_make_chainman(); setup.LoadVerifyActivateChainstate(); for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { - MineBlock(setup.m_node, {}); + node::BlockAssembler::Options options; + options.include_dummy_extranonce = true; + MineBlock(setup.m_node, options); } setup.m_node.validation_signals->SyncWithValidationInterfaceQueue(); } diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index f36f528b0e..28bee67d37 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -35,8 +35,10 @@ void ResetChainman(TestingSetup& setup) setup.m_node.chainman.reset(); setup.m_make_chainman(); setup.LoadVerifyActivateChainstate(); + node::BlockAssembler::Options options; + options.include_dummy_extranonce = true; for (int i = 0; i < 2 * COINBASE_MATURITY; i++) { - MineBlock(setup.m_node, {}); + MineBlock(setup.m_node, options); } setup.m_node.validation_signals->SyncWithValidationInterfaceQueue(); } diff --git a/src/test/fuzz/tx_pool.cpp b/src/test/fuzz/tx_pool.cpp index f70dd710b3..bb15552723 100644 --- a/src/test/fuzz/tx_pool.cpp +++ b/src/test/fuzz/tx_pool.cpp @@ -48,6 +48,7 @@ void initialize_tx_pool() BlockAssembler::Options options; options.coinbase_output_script = P2WSH_OP_TRUE; + options.include_dummy_extranonce = true; for (int i = 0; i < 2 * COINBASE_MATURITY; ++i) { COutPoint prevout{MineBlock(g_setup->m_node, options)}; @@ -97,6 +98,7 @@ void Finish(FuzzedDataProvider& fuzzed_data_provider, MockedTxPool& tx_pool, Cha BlockAssembler::Options options; options.nBlockMaxWeight = fuzzed_data_provider.ConsumeIntegralInRange(0U, MAX_BLOCK_WEIGHT); options.blockMinFeeRate = CFeeRate{ConsumeMoney(fuzzed_data_provider, /*max=*/COIN)}; + options.include_dummy_extranonce = true; auto assembler = BlockAssembler{chainstate, &tx_pool, options}; auto block_template = assembler.CreateNewBlock(); Assert(block_template->block.vtx.size() >= 1); diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp index d27ca3470b..cac9d4ce70 100644 --- a/src/test/fuzz/utxo_total_supply.cpp +++ b/src/test/fuzz/utxo_total_supply.cpp @@ -45,6 +45,7 @@ FUZZ_TARGET(utxo_total_supply) }; BlockAssembler::Options options; options.coinbase_output_script = CScript() << OP_FALSE; + options.include_dummy_extranonce = true; const auto PrepareNextBlock = [&]() { // Use OP_FALSE to avoid BIP30 check from hitting early auto block = PrepareBlock(node, options); diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index a1f5a6d08d..3b4beebeff 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -116,6 +116,7 @@ void MinerTestingSetup::TestPackageSelection(const CScript& scriptPubKey, const auto mining{MakeMining()}; BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; + options.include_dummy_extranonce = true; LOCK(tx_mempool.cs); BOOST_CHECK(tx_mempool.size() == 0); @@ -334,6 +335,7 @@ void MinerTestingSetup::TestBasicMining(const CScript& scriptPubKey, const std:: BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; + options.include_dummy_extranonce = true; { CTxMemPool& tx_mempool{MakeMempool()}; @@ -660,6 +662,7 @@ void MinerTestingSetup::TestPrioritisedMining(const CScript& scriptPubKey, const BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; + options.include_dummy_extranonce = true; CTxMemPool& tx_mempool{MakeMempool()}; LOCK(tx_mempool.cs); @@ -749,6 +752,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) CScript scriptPubKey = CScript() << "04678afdb0fe5548271967f1a67130b7105cd6a828e03909a67962e0ea1f61deb649f6bc3f4cef38c4f35504e51ec112de5c384df7ba0b8d578a4c702b6bf11d5f"_hex << OP_CHECKSIG; BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; + options.include_dummy_extranonce = true; // Create and check a simple template std::unique_ptr block_template = mining->createNewBlock(options); diff --git a/src/test/peerman_tests.cpp b/src/test/peerman_tests.cpp index 64b13fa3cc..e391e8b9c3 100644 --- a/src/test/peerman_tests.cpp +++ b/src/test/peerman_tests.cpp @@ -19,8 +19,10 @@ static constexpr int64_t NODE_NETWORK_LIMITED_ALLOW_CONN_BLOCKS = 144; static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_time) { auto curr_time = GetTime(); + node::BlockAssembler::Options options; + options.include_dummy_extranonce = true; SetMockTime(block_time); // update time so the block is created with it - CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr, {}}.CreateNewBlock()->block; + CBlock block = node::BlockAssembler{node.chainman->ActiveChainstate(), nullptr, options}.CreateNewBlock()->block; while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce; block.fChecked = true; // little speedup SetMockTime(curr_time); // process block at current time diff --git a/src/test/testnet4_miner_tests.cpp b/src/test/testnet4_miner_tests.cpp index 5b3582ac79..33e028a5c9 100644 --- a/src/test/testnet4_miner_tests.cpp +++ b/src/test/testnet4_miner_tests.cpp @@ -35,6 +35,7 @@ BOOST_AUTO_TEST_CASE(MiningInterface) BOOST_REQUIRE(mining); BlockAssembler::Options options; + options.include_dummy_extranonce = true; std::unique_ptr block_template; // Set node time a few minutes past the testnet4 genesis block diff --git a/src/test/util/mining.cpp b/src/test/util/mining.cpp index 05f1be77ee..c4823bce35 100644 --- a/src/test/util/mining.cpp +++ b/src/test/util/mining.cpp @@ -29,6 +29,7 @@ COutPoint generatetoaddress(const NodeContext& node, const std::string& address) assert(IsValidDestination(dest)); BlockAssembler::Options assembler_options; assembler_options.coinbase_output_script = GetScriptForDestination(dest); + assembler_options.include_dummy_extranonce = true; return MineBlock(node, assembler_options); } @@ -139,6 +140,7 @@ std::shared_ptr PrepareBlock(const NodeContext& node, const CScript& coi { BlockAssembler::Options assembler_options; assembler_options.coinbase_output_script = coinbase_scriptPubKey; + assembler_options.include_dummy_extranonce = true; ApplyArgsManOptions(*node.args, assembler_options); return PrepareBlock(node, assembler_options); } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 4e8866399e..e479168a48 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -404,6 +404,7 @@ CBlock TestChain100Setup::CreateBlock( { BlockAssembler::Options options; options.coinbase_output_script = scriptPubKey; + options.include_dummy_extranonce = true; CBlock block = BlockAssembler{chainstate, nullptr, options}.CreateNewBlock()->block; Assert(block.vtx.size() == 1); diff --git a/src/test/validation_block_tests.cpp b/src/test/validation_block_tests.cpp index dfa66bb889..f91b30a307 100644 --- a/src/test/validation_block_tests.cpp +++ b/src/test/validation_block_tests.cpp @@ -68,6 +68,7 @@ std::shared_ptr MinerTestingSetup::Block(const uint256& prev_hash) BlockAssembler::Options options; options.coinbase_output_script = CScript{} << i++ << OP_TRUE; + options.include_dummy_extranonce = true; auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(); auto pblock = std::make_shared(ptemplate->block); pblock->hashPrevBlock = prev_hash; @@ -336,6 +337,7 @@ BOOST_AUTO_TEST_CASE(witness_commitment_index) pubKey << 1 << OP_TRUE; BlockAssembler::Options options; options.coinbase_output_script = pubKey; + options.include_dummy_extranonce = true; auto ptemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock(); CBlock pblock = ptemplate->block; diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py index 47589cbcec..6d007e8830 100755 --- a/test/functional/interface_ipc.py +++ b/test/functional/interface_ipc.py @@ -20,10 +20,14 @@ from test_framework.messages import ( ser_uint256, COIN, ) +from test_framework.script import ( + CScript, + CScriptNum, +) from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, - assert_not_equal + assert_not_equal, ) from test_framework.wallet import MiniWallet from typing import Optional @@ -194,6 +198,12 @@ class IPCInterfaceTest(BitcoinTestFramework): coinbase_tx.vin = [CTxIn()] coinbase_tx.vin[0].prevout = NULL_OUTPOINT coinbase_tx.vin[0].nSequence = coinbase_res.sequence + + # 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()) + # Typically a mining pool appends its name and an extraNonce coinbase_tx.vin[0].scriptSig = coinbase_res.scriptSigPrefix