From 37c73dcd608a5ce6321058a3beada12c7b63dfb5 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Mon, 30 Dec 2024 20:45:38 +0100 Subject: [PATCH] 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. --- src/node/interfaces.cpp | 2 +- src/node/miner.cpp | 7 +++---- src/rpc/mining.cpp | 16 ++++++---------- src/validation.cpp | 40 +--------------------------------------- src/validation.h | 11 +---------- 5 files changed, 12 insertions(+), 64 deletions(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 6f28acfd4c240..4bbf1e80ebd3e 100644 --- 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 index 5d7304b597ea3..0c920ff56eba9 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -167,10 +167,9 @@ std::unique_ptr 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 index a59ec5df42bcb..9f5d533cd4726 100644 --- 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 index 59bf71be97bfc..c4c880e3cf5f9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4589,7 +4589,7 @@ bool ChainstateManager::AcceptBlock(const std::shared_ptr& 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 index 5b39746e45305..0e10f2cc854e0 100644 --- 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& 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