merge bitcoin#24732: Remove buggy and confusing IncrementExtraNonce

kwvg · kwvg · commit e421753642c3 · 2025-06-19T17:47:47.000Z
`feature_blockfilterindex_prune.py` changes to `feature_index_prune.py` in bitcoin#21726 (backported as baf6e26, dash#6327), so changes are made there instead.
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
@@ -584,21 +584,3 @@ void BlockAssembler::addPackageTxs(const CTxMemPool& mempool, int& nPackagesSele
         nDescendantsUpdated += UpdatePackagesForAdded(mempool, ancestors, mapModifiedTx);
     }
 }
-
-void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce)
-{
-    // Update nExtraNonce
-    static uint256 hashPrevBlock;
-    if (hashPrevBlock != pblock->hashPrevBlock) {
-        nExtraNonce = 0;
-        hashPrevBlock = pblock->hashPrevBlock;
-    }
-    ++nExtraNonce;
-    unsigned int nHeight = pindexPrev->nHeight + 1; // Height first in coinbase required for block.version=2
-    CMutableTransaction txCoinbase(*pblock->vtx[0]);
-    txCoinbase.vin[0].scriptSig = (CScript() << nHeight << CScriptNum(nExtraNonce));
-    assert(txCoinbase.vin[0].scriptSig.size() <= 100);
-
-    pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase));
-    pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
-}
diff --git a/src/node/miner.h b/src/node/miner.h
@@ -226,8 +226,6 @@ class BlockAssembler
     void SortForBlock(const CTxMemPool::setEntries& package, std::vector<CTxMemPool::txiter>& sortedEntries);
 };
 
-/** Modify the extranonce in a block */
-void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned int& nExtraNonce);
 int64_t UpdateTime(CBlockHeader* pblock, const Consensus::Params& consensusParams, const CBlockIndex* pindexPrev);
 
 #endif // BITCOIN_NODE_MINER_H
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
@@ -8,6 +8,7 @@
 #include <chainparams.h>
 #include <consensus/amount.h>
 #include <consensus/consensus.h>
+#include <consensus/merkle.h>
 #include <consensus/params.h>
 #include <consensus/validation.h>
 #include <core_io.h>
@@ -121,14 +122,10 @@ static RPCHelpMan getnetworkhashps()
 }
 
 #if ENABLE_MINER
-static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& max_tries, unsigned int& extra_nonce, uint256& block_hash)
+static bool GenerateBlock(ChainstateManager& chainman, CBlock& block, uint64_t& max_tries, uint256& block_hash)
 {
     block_hash.SetNull();
-
-    {
-        LOCK(cs_main);
-        IncrementExtraNonce(&block, chainman.ActiveChain().Tip(), extra_nonce);
-    }
+    block.hashMerkleRoot = BlockMerkleRoot(block);
 
     CChainParams chainparams(Params());
 
@@ -157,30 +154,20 @@ static UniValue generateBlocks(ChainstateManager& chainman, const NodeContext& n
 {
     EnsureLLMQContext(node);
 
-    int nHeightEnd = 0;
-    int nHeight = 0;
-
-    {   // Don't keep cs_main locked
-        LOCK(cs_main);
-        nHeight = chainman.ActiveChain().Height();
-        nHeightEnd = nHeight+nGenerate;
-    }
-    unsigned int nExtraNonce = 0;
     UniValue blockHashes(UniValue::VARR);
-    while (nHeight < nHeightEnd && !ShutdownRequested())
-    {
+    while (nGenerate > 0 && !ShutdownRequested()) {
         std::unique_ptr<CBlockTemplate> pblocktemplate(BlockAssembler(chainman.ActiveChainstate(), node, &mempool, Params()).CreateNewBlock(coinbase_script));
         if (!pblocktemplate.get())
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
         CBlock *pblock = &pblocktemplate->block;
 
         uint256 block_hash;
-        if (!GenerateBlock(chainman, *pblock, nMaxTries, nExtraNonce, block_hash)) {
+        if (!GenerateBlock(chainman, *pblock, nMaxTries, block_hash)) {
             break;
         }
 
         if (!block_hash.IsNull()) {
-            ++nHeight;
+            --nGenerate;
             blockHashes.push_back(block_hash.GetHex());
         }
     }
@@ -400,9 +387,8 @@ static RPCHelpMan generateblock()
 
     uint256 block_hash;
     uint64_t max_tries{DEFAULT_MAX_TRIES};
-    unsigned int extra_nonce{0};
 
-    if (!GenerateBlock(chainman, block, max_tries, extra_nonce, block_hash) || block_hash.IsNull()) {
+    if (!GenerateBlock(chainman, block, max_tries, block_hash) || block_hash.IsNull()) {
         throw JSONRPCError(RPC_MISC_ERROR, "Failed to make block.");
     }
 
diff --git a/src/test/blockfilter_index_tests.cpp b/src/test/blockfilter_index_tests.cpp
@@ -4,6 +4,7 @@
 
 #include <blockfilter.h>
 #include <chainparams.h>
+#include <consensus/merkle.h>
 #include <consensus/validation.h>
 #include <index/blockfilterindex.h>
 #include <node/miner.h>
@@ -73,9 +74,12 @@ CBlock BuildChainTestingSetup::CreateBlock(const CBlockIndex* prev,
     for (const CMutableTransaction& tx : txns) {
         block.vtx.push_back(MakeTransactionRef(tx));
     }
-    // IncrementExtraNonce creates a valid coinbase and merkleRoot
-    unsigned int extraNonce = 0;
-    IncrementExtraNonce(&block, prev, extraNonce);
+    {
+        CMutableTransaction tx_coinbase{*block.vtx.at(0)};
+        tx_coinbase.vin.at(0).scriptSig = CScript{} << prev->nHeight + 1;
+        block.vtx.at(0) = MakeTransactionRef(std::move(tx_coinbase));
+        block.hashMerkleRoot = BlockMerkleRoot(block);
+    }
 
     while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;
 
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -8,6 +8,7 @@
 #include <banman.h>
 #include <chainparams.h>
 #include <consensus/consensus.h>
+#include <consensus/merkle.h>
 #include <consensus/params.h>
 #include <consensus/validation.h>
 #include <deploymentstatus.h>
@@ -502,11 +503,14 @@ CBlock TestChainSetup::CreateBlock(
         block.vtx[0] = MakeTransactionRef(tmpTx);
     }
 
-    // IncrementExtraNonce creates a valid coinbase and merkleRoot
+    // Create a valid coinbase and merkleRoot
     {
-        LOCK(cs_main);
-        unsigned int extraNonce = 0;
-        IncrementExtraNonce(&block, chainstate.m_chain.Tip(), extraNonce);
+        LOCK(::cs_main);
+        block.hashPrevBlock = chainstate.m_chain.Tip()->GetBlockHash();
+        CMutableTransaction tx_coinbase{*block.vtx[0]};
+        tx_coinbase.vin[0].scriptSig = CScript{} << (chainstate.m_chain.Height() + 1) << CScriptNum{1};
+        block.vtx[0] = MakeTransactionRef(std::move(tx_coinbase));
+        block.hashMerkleRoot = BlockMerkleRoot(block);
     }
 
     while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;
diff --git a/test/functional/feature_index_prune.py b/test/functional/feature_index_prune.py
@@ -78,7 +78,7 @@ def run_test(self):
                 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, 366)
+                assert_equal(pruneheight_new, 368)
 
         self.log.info("check if we can access the tips blockfilter and coinstats when we have pruned some blocks")
         tip = self.nodes[0].getbestblockhash()
@@ -95,8 +95,8 @@ def run_test(self):
             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], 298)
-        self.sync_index(height=998)
+        self.generate(self.nodes[0], 51)
+        self.sync_index(height=751)
 
         self.restart_without_indices()
 
@@ -108,12 +108,12 @@ def run_test(self):
             msg = "Querying specific block heights requires coinstatsindex"
             assert_raises_rpc_error(-8, msg, node.gettxoutsetinfo, "muhash", height_hash)
 
-        self.mine_batches(502)
+        self.mine_batches(749)
 
         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, 732)
+            assert_equal(pruneheight_2, 736)
             # Restart the nodes again with the indices activated
             self.restart_node(i, extra_args=self.extra_args[i], expected_stderr=EXPECTED_STDERR_NO_GOV_PRUNE)
 
@@ -148,7 +148,7 @@ def run_test(self):
         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, 2196)
+                assert_equal(pruneheight_new, 2208)
 
         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']):
diff --git a/test/functional/feature_utxo_set_hash.py b/test/functional/feature_utxo_set_hash.py
@@ -67,8 +67,8 @@ def test_muhash_implementation(self):
         assert_equal(finalized[::-1].hex(), node_muhash)
 
         self.log.info("Test deterministic UTXO set hash results")
-        assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "91ceba6561281d14db95afb48894fa6f764daa9f2190d05cb397e759634e10bf")
-        assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "c75ac7fcf057ef30cb5880fc800a99d3908fd0eb8593647512823ada6e359ef5")
+        assert_equal(node.gettxoutsetinfo()['hash_serialized_2'], "1d640be368b1d811619bc27bc435db672e3ce17f524c0f78def9f7398aef9eac")
+        assert_equal(node.gettxoutsetinfo("muhash")['muhash'], "c2ca3b5233cf7f4f9ba63ecd3166abf9a6b8c76fe050cf7d51acaeca0d52e78e")
 
     def run_test(self):
         self.test_muhash_implementation()
diff --git a/test/functional/rpc_dumptxoutset.py b/test/functional/rpc_dumptxoutset.py
@@ -37,16 +37,16 @@ def run_test(self):
         # Blockhash should be deterministic based on mocked time.
         assert_equal(
             out['base_hash'],
-            '0266fa3cd96867c40f2f3056da3b66daf8fb4c83ec4f3f5344fd3fe9755794b5')
+            '38619d68335f21ded08d6e1f4ef9572eaeb0bb04b9070b3e6be33c7c908564b4')
 
         with open(str(expected_path), 'rb') as f:
             digest = hashlib.sha256(f.read()).hexdigest()
             # UTXO snapshot hash should be deterministic based on mocked time.
             assert_equal(
-                digest, '32f1d4b7f643c97e88c540f431e8277fdd9332c3dea260b046c93787745e35b0')
+                digest, '4a34cf865938252bf0bef702989955824d886f595afab596b1edac4dd31cd89f')
 
         assert_equal(
-            out['txoutset_hash'], '970c1bc05f0d0920cc1eae5854860f90f85453ec815e710f40135ec149345ade')
+            out['txoutset_hash'], 'b2d7429106c96f5ab831843d5c96ba131ca8793111d0a0e30e7d7d8b4841e6cc')
         assert_equal(out['nchaintx'], 101)
 
         # Specifying a path to an existing file will fail.
