[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 <aj@erisian.com.au>

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};

src/node/miner.cpp

@@ -177,11 +177,17 @@ std::unique_ptr<CBlockTemplate> 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
+    // Mining clients are expected to append extra data to this prefix, so
-    // could be removed without a consensus change. Mining clients are expected
+    // increasing its length would reduce the space they can use and may break
-    // to append extra data to this prefix, so increasing its length would reduce
+    // existing clients.
-    // the space they can use and may break existing clients.
+    coinbaseTx.vin[0].scriptSig = CScript() << nHeight;
-    coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0;
+    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<uint32_t>(nHeight - 1);
@@ -212,6 +218,7 @@ std::unique_ptr<CBlockTemplate> 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()));
         }

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 {

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<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script }));
+        std::unique_ptr<BlockTemplate> block_template(miner.createNewBlock({ .coinbase_output_script = coinbase_output_script, .include_dummy_extranonce = true }));
         CHECK_NONFATAL(block_template);
 
         std::shared_ptr<const CBlock> block_out;
@@ -376,7 +376,7 @@ static RPCHelpMan generateblock()
     {
         LOCK(chainman.GetMutex());
         {
-            std::unique_ptr<BlockTemplate> block_template{miner.createNewBlock({.use_mempool = false, .coinbase_output_script = coinbase_output_script})};
+            std::unique_ptr<BlockTemplate> 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);
 
 

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<CBlockTemplate> pblocktemplate = BlockAssembler{m_node.chainman->ActiveChainstate(), m_node.mempool.get(), options}.CreateNewBlock();
     CBlock& block = pblocktemplate->block;
     block.hashPrevBlock = prev->GetBlockHash();

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)};

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();
 }

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();
 }

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);

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);

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<BlockTemplate> block_template = mining->createNewBlock(options);

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<std::chrono::seconds>();
+    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

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<BlockTemplate> block_template;
 
     // Set node time a few minutes past the testnet4 genesis block

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<CBlock> 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);
 }

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);

src/test/validation_block_tests.cpp

@@ -68,6 +68,7 @@ std::shared_ptr<CBlock> 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<CBlock>(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;
 

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