validation: replace TestBlockValidity

Delete the original TestBlockValidity and rename CheckNewBlock. Have the generateblock and getblocktemplate RPC calls, as well as BlockAssembler::CreateNewBlock use the new method.
Sjors · Jan 7, 2025 · 37c73dc · 37c73dc
1 parent 27d67e6
commit 37c73dc
Show file tree

Hide file tree

Showing 5 changed files with 12 additions and 64 deletions.
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
@@ -990,7 +990,7 @@ class MinerImpl : public Mining
 
     bool checkBlock(const CBlock& block, const node::BlockCheckOptions& options, std::string& reason) override
     {
-        return chainman().CheckNewBlock(block, reason, options.check_merkle_root, options.check_pow, options.target);
+        return chainman().TestBlockValidity(block, reason, options.check_merkle_root, options.check_pow, options.target);
     }
 
     NodeContext* context() override { return &m_node; }

diff --git a/src/node/miner.cpp b/src/node/miner.cpp
@@ -167,10 +167,9 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
     pblock->nNonce         = 0;
     pblocktemplate->vTxSigOpsCost[0] = WITNESS_SCALE_FACTOR * GetLegacySigOpCount(*pblock->vtx[0]);
 
-    BlockValidationState state;
-    if (m_options.test_block_validity && !TestBlockValidity(state, chainparams, m_chainstate, *pblock, pindexPrev,
-                                                            /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) {
-        throw std::runtime_error(strprintf("%s: TestBlockValidity failed: %s", __func__, state.ToString()));
+    std::string reason;
+    if (m_options.test_block_validity && !m_chainstate.m_chainman.TestBlockValidity(*pblock, reason, /*check_merkle_root=*/false, /*check_pow=*/false)) {
+        throw std::runtime_error(strprintf("TestBlockValidity failed: %s", reason));
     }
     const auto time_2{SteadyClock::now()};
 

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
@@ -385,9 +385,9 @@ static RPCHelpMan generateblock()
         block.vtx.insert(block.vtx.end(), txs.begin(), txs.end());
         RegenerateCommitments(block, chainman);
 
-        BlockValidationState state;
-        if (!TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/false)) {
-            throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", state.ToString()));
+        std::string reason;
+        if (!miner.checkBlock(block, {.check_merkle_root = false, .check_pow = false}, reason)) {
+            throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s", reason));
         }
     }
 
@@ -716,13 +716,9 @@ static RPCHelpMan getblocktemplate()
                 return "duplicate-inconclusive";
             }
 
-            // TestBlockValidity only supports blocks built on the current Tip
-            if (block.hashPrevBlock != tip) {
-                return "inconclusive-not-best-prevblk";
-            }
-            BlockValidationState state;
-            TestBlockValidity(state, chainman.GetParams(), chainman.ActiveChainstate(), block, chainman.m_blockman.LookupBlockIndex(block.hashPrevBlock), /*fCheckPOW=*/false, /*fCheckMerkleRoot=*/true);
-            return BIP22ValidationResult(state);
+            std::string reason;
+            bool res{miner.checkBlock(block, {.check_pow = false}, reason)};
+            return res ? UniValue::VNULL : UniValue{reason};
         }
 
         const UniValue& aClientRules = oparam.find_value("rules");

diff --git a/src/validation.cpp b/src/validation.cpp
@@ -4589,7 +4589,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr<const CBlock>& pblock,
     return true;
 }
 
-bool ChainstateManager::CheckNewBlock(const CBlock& block, std::string& reason, const bool check_merkle_root, const bool check_pow, const uint256 target)
+bool ChainstateManager::TestBlockValidity(const CBlock& block, std::string& reason, const bool check_merkle_root, const bool check_pow, const uint256 target)
 {
     ChainstateManager& chainman{ActiveChainstate().m_chainman};
 
@@ -4760,44 +4760,6 @@ MempoolAcceptResult ChainstateManager::ProcessTransaction(const CTransactionRef&
     return result;
 }
 
-bool TestBlockValidity(BlockValidationState& state,
-                       const CChainParams& chainparams,
-                       Chainstate& chainstate,
-                       const CBlock& block,
-                       CBlockIndex* pindexPrev,
-                       bool fCheckPOW,
-                       bool fCheckMerkleRoot)
-{
-    AssertLockHeld(cs_main);
-    assert(pindexPrev && pindexPrev == chainstate.m_chain.Tip());
-    CCoinsViewCache viewNew(&chainstate.CoinsTip());
-    uint256 block_hash(block.GetHash());
-    CBlockIndex indexDummy(block);
-    indexDummy.pprev = pindexPrev;
-    indexDummy.nHeight = pindexPrev->nHeight + 1;
-    indexDummy.phashBlock = &block_hash;
-
-    // NOTE: CheckBlockHeader is called by CheckBlock
-    if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, pindexPrev)) {
-        LogError("%s: Consensus::ContextualCheckBlockHeader: %s\n", __func__, state.ToString());
-        return false;
-    }
-    if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) {
-        LogError("%s: Consensus::CheckBlock: %s\n", __func__, state.ToString());
-        return false;
-    }
-    if (!ContextualCheckBlock(block, state, chainstate.m_chainman, pindexPrev)) {
-        LogError("%s: Consensus::ContextualCheckBlock: %s\n", __func__, state.ToString());
-        return false;
-    }
-    if (!chainstate.ConnectBlock(block, state, &indexDummy, viewNew, true)) {
-        return false;
-    }
-    assert(state.IsValid());
-
-    return true;
-}
-
 /* This function is called from the RPC code for pruneblockchain */
 void PruneBlockFilesManual(Chainstate& active_chainstate, int nManualPruneHeight)
 {

diff --git a/src/validation.h b/src/validation.h
@@ -383,15 +383,6 @@ class ValidationCache
 /** Context-independent validity checks */
 bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensus::Params& consensusParams, bool fCheckPOW = true, bool fCheckMerkleRoot = true);
 
-/** Check a block is completely valid from start to finish (only works on top of our current best block) */
-bool TestBlockValidity(BlockValidationState& state,
-                       const CChainParams& chainparams,
-                       Chainstate& chainstate,
-                       const CBlock& block,
-                       CBlockIndex* pindexPrev,
-                       bool fCheckPOW = true,
-                       bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
-
 /** Check with the proof of work on each blockheader matches the value in nBits */
 bool HasValidProofOfWork(const std::vector<CBlockHeader>& headers, const Consensus::Params& consensusParams);
 
@@ -1197,7 +1188,7 @@ class ChainstateManager
      * For signets the challenge verification is skipped when check_pow is false or
      * a higher target is provided.
      */
-    bool CheckNewBlock(const CBlock& block, std::string& reason, const bool check_merkle_root = true, const bool check_pow = true, const uint256 target = uint256::ZERO);
+    bool TestBlockValidity(const CBlock& block, std::string& reason, const bool check_merkle_root = true, const bool check_pow = true, const uint256 target = uint256::ZERO);
 
     /**
      * Process an incoming block. This only returns after the best known valid