From d511adb664edcfb97be44bc0738f49b679240504 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 14 Jan 2026 10:02:33 +0100 Subject: [PATCH] [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 297465be80f..702f2c09366 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 58b147ca2ea..b5073054eb2 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 6930672f02e..deab1faacb8 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 9c357f87cba..d0efa84fb71 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 e970ae9cbb7..d7d10dfb1ae 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 1cc2caa396e..93cd8ad6bbd 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 ef8cb686cee..7a24c1de348 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 f36f528b0e3..28bee67d37e 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 f70dd710b35..bb15552723a 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 d27ca3470b4..cac9d4ce703 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 a1f5a6d08de..3b4beebeff5 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 64b13fa3cc1..e391e8b9c3f 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 5b3582ac795..33e028a5c9d 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 05f1be77ee2..c4823bce351 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 4e8866399e9..e479168a48e 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 dfa66bb8899..f91b30a307c 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 47589cbcece..6d007e88304 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