Daily cherry pick

src/bench/block_assemble.cpp

@@ -30,7 +30,6 @@ 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/interfaces/mining.h

@@ -34,7 +34,8 @@ class BlockTemplate
     virtual ~BlockTemplate() = default;
 
     virtual CBlockHeader getBlockHeader() = 0;
-    // Block contains a dummy coinbase transaction that should not be used.
+    // Block contains a dummy coinbase transaction that should not be used and
+    // it may not match a transaction constructed from getCoinbaseTx().
     virtual CBlock getBlock() = 0;
 
     // Fees per transaction, not including coinbase transaction.
@@ -64,6 +65,10 @@ class BlockTemplate
      * @note unlike the submitblock RPC, this method does NOT add the
      *       coinbase witness automatically.
      *
+     * @note for heights <= 16, the BIP34 height push in getCoinbaseTx().script_sig_prefix
+     *       is only one byte long, so the coinbase scriptSig needs at least
+     *       one additional byte of data to avoid bad-cb-length.
+     *
      * @returns if the block was processed, does not necessarily indicate validity.
      *
      * @note Returns true if the block is already known, which can happen if

src/kernel/chainparams.cpp

@@ -537,6 +537,7 @@ class CRegTestParams : public CChainParams
         m_is_mockable_chain = true;
 
 
+
         chainTxData = ChainTxData{
             .nTime = 0,
             .tx_count = 0,

src/node/miner.cpp

@@ -184,14 +184,18 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
     // 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) {
+    // Set script_sig_prefix here, so IPC mining clients are not affected by
+    // the optional scriptSig padding below. They provide their own extraNonce,
+    // and in a typical setup a pool name or realistic extraNonce already makes
+    // the scriptSig long enough.
+    coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig;
+    if (nHeight <= 16) {
         // 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)
+        // bytes long (bad-cb-length), so an OP_0 is always appended at those
+        // heights.
         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);
     coinbase_tx.lock_time = coinbaseTx.nLockTime;
@@ -221,7 +225,6 @@ 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

@@ -72,10 +72,6 @@ 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 {
@@ -125,7 +121,9 @@ struct CoinbaseTx {
      * Prefix which needs to be placed at the beginning of the scriptSig.
      * Clients may append extra data to this as long as the overall scriptSig
      * size is 100 bytes or less, to avoid the block being rejected with
-     * "bad-cb-length" error.
+     * "bad-cb-length" error. At heights <= 16 the BIP 34 height push is only
+     * one byte long, so clients must append at least one additional byte to
+     * meet the consensus minimum scriptSig length of two bytes.
      *
      * Currently with BIP 34, the prefix is guaranteed to be less than 8 bytes,
      * but future soft forks could require longer prefixes.

src/test/fuzz/cmpctblock.cpp

@@ -123,7 +123,6 @@ void ResetChainmanAndMempool(TestingSetup& setup)
 
     node::BlockAssembler::Options options;
     options.coinbase_output_script = P2WSH_OP_TRUE;
-    options.include_dummy_extranonce = true;
 
     g_mature_coinbase.clear();
 

src/test/fuzz/package_eval.cpp

@@ -46,7 +46,6 @@ 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

@@ -43,7 +43,6 @@ void ResetChainman(TestingSetup& setup)
     setup.LoadVerifyActivateChainstate();
     for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
         node::BlockAssembler::Options options;
-        options.include_dummy_extranonce = true;
         MineBlock(setup.m_node, options);
     }
 }

src/test/fuzz/process_messages.cpp

@@ -37,7 +37,6 @@ void ResetChainman(TestingSetup& setup)
     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, options);
     }

src/test/fuzz/tx_pool.cpp

@@ -48,7 +48,6 @@ 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)};
@@ -98,7 +97,6 @@ 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

@@ -46,7 +46,6 @@ 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);
@@ -107,8 +106,12 @@ FUZZ_TARGET(utxo_total_supply)
     // Assuming that the fuzzer will mine relatively short chains (less than 200 blocks), we want the duplicate coinbase to be not too high.
     // Up to 300 seems reasonable.
     int64_t duplicate_coinbase_height = fuzzed_data_provider.ConsumeIntegralInRange(0, 300);
-    // Always pad with OP_0 at the end to avoid bad-cb-length error
-    const CScript duplicate_coinbase_script = CScript() << duplicate_coinbase_height << OP_0;
+    // Avoid bad-cb-length error at heights <= 16. Pad the BIP34-encoded height
+    // with OP_0 to satisfy the minimum 2-byte coinbase scriptSig length.
+    CScript duplicate_coinbase_script = CScript() << duplicate_coinbase_height;
+    if (duplicate_coinbase_height <= 16) {
+        duplicate_coinbase_script << OP_0;
+    }
     // Mine the first block with this duplicate
     current_block = PrepareNextBlock();
     StoreLastTxo();

src/test/miner_tests.cpp

@@ -117,7 +117,6 @@ 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);
@@ -336,7 +335,6 @@ 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()};
@@ -663,7 +661,6 @@ 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);
@@ -753,7 +750,6 @@ 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, /*cooldown=*/false);

src/test/peerman_tests.cpp

@@ -20,7 +20,6 @@ static void mineBlock(const node::NodeContext& node, std::chrono::seconds block_
 {
     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, options}.CreateNewBlock()->block;
     while (!CheckProofOfWork(block.GetHash(), block.nBits, node.chainman->GetConsensus())) ++block.nNonce;

src/test/testnet4_miner_tests.cpp

@@ -36,7 +36,6 @@ 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,7 +29,6 @@ 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);
 }
@@ -140,7 +139,6 @@ 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

@@ -380,7 +380,7 @@ TestChain100Setup::TestChain100Setup(
         LOCK(::cs_main);
         assert(
             m_node.chainman->ActiveChain().Tip()->GetBlockHash().ToString() ==
-            "0c8c5f79505775a0f6aed6aca2350718ceb9c6f2c878667864d5c7a6d8ffa2a6");
+            "0ee6e270d6594249e548110619f7bd690695beb219b915da4a2e84e2b61ed60f");
     }
 }
 
@@ -402,7 +402,6 @@ 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,7 +68,6 @@ 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;
@@ -337,7 +336,6 @@ 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;
 

src/test/validation_tests.cpp

@@ -127,6 +127,7 @@ BOOST_AUTO_TEST_CASE(signet_parse_tests)
     BOOST_CHECK(!CheckSignetBlockSolution(block, signet_params->GetConsensus()));
 }
 
+
 BOOST_AUTO_TEST_CASE(block_malleation)
 {
     // Test utilities that calls `IsBlockMutated` and then clears the validity