From a74b0f93efa1d9eaf5abc2f6591c44a632aec6ed Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Wed, 26 Jun 2024 13:42:55 +0200 Subject: Have testBlockValidity hold cs_main instead of caller The goal of interfaces is to eventually run in their own process, so we can't use EXCLUSIVE_LOCKS_REQUIRED in their declaration. However TestBlockValidaty will crash (in its call to ConnectBlock) if the tip changes from under the proposed block. Have the testBlockValidity implementation hold the lock instead, and non-fatally check for this condition. --- src/interfaces/mining.h | 2 +- src/node/interfaces.cpp | 11 +++++++++-- src/rpc/mining.cpp | 4 ---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index bc98448833..7d71a01450 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -67,7 +67,7 @@ public: * @param[in] block the block to validate * @param[in] check_merkle_root call CheckMerkleRoot() * @param[out] state details of why a block failed to validate - * @returns false if any of the checks fail + * @returns false if it does not build on the current tip, or any of the checks fail */ virtual bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 5e87bf234a..fa151407fa 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -872,8 +872,15 @@ public: bool testBlockValidity(const CBlock& block, bool check_merkle_root, BlockValidationState& state) override { - LOCK(::cs_main); - return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, chainman().ActiveChain().Tip(), /*fCheckPOW=*/false, check_merkle_root); + LOCK(cs_main); + CBlockIndex* tip{chainman().ActiveChain().Tip()}; + // Fail if the tip updated before the lock was taken + if (block.hashPrevBlock != tip->GetBlockHash()) { + state.Error("Block does not connect to current chain tip."); + return false; + } + + return TestBlockValidity(state, chainman().GetParams(), chainman().ActiveChainstate(), block, tip, /*fCheckPOW=*/false, check_merkle_root); } std::unique_ptr createNewBlock(const CScript& script_pub_key, bool use_mempool) override diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index d9fd82d708..7e420dcd9b 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -371,8 +371,6 @@ static RPCHelpMan generateblock() ChainstateManager& chainman = EnsureChainman(node); { - LOCK(cs_main); - std::unique_ptr blocktemplate{miner.createNewBlock(coinbase_script, /*use_mempool=*/false)}; if (!blocktemplate) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block"); @@ -387,8 +385,6 @@ static RPCHelpMan generateblock() RegenerateCommitments(block, chainman); { - LOCK(cs_main); - BlockValidationState state; if (!miner.testBlockValidity(block, /*check_merkle_root=*/false, state)) { throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("testBlockValidity failed: %s", state.ToString())); -- cgit v1.2.3