diff --git a/src/node/miner.cpp b/src/node/miner.cpp index c24debd1e69..280736db2da 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -173,10 +173,10 @@ std::unique_ptr BlockAssembler::CreateNewBlock() pblock->nBits = GetNextWorkRequired(pindexPrev, pblock, chainparams.GetConsensus()); pblock->nNonce = 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; + std::string debug; + if (m_options.test_block_validity && !TestBlockValidity(m_chainstate, *pblock, /*check_pow=*/false, /*check_merkle_root=*/false, reason, debug)) { + throw std::runtime_error(strprintf("TestBlockValidity failed: %s - %s", reason, debug)); } const auto time_2{SteadyClock::now()}; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 95184cafee2..3e6d5e7ebfd 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -387,9 +387,10 @@ 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; + std::string debug; + if (!TestBlockValidity(chainman.ActiveChainstate(), block, /*check_pow=*/false, /*check_merkle_root=*/false, reason, debug)) { + throw JSONRPCError(RPC_VERIFY_ERROR, strprintf("TestBlockValidity failed: %s - %s", reason, debug)); } } @@ -741,13 +742,12 @@ 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; + std::string debug; + bool res{TestBlockValidity(chainman.ActiveChainstate(), block, /*check_pow=*/false, /*check_merkle_root=*/true, reason, debug)}; + if (res) return UniValue::VNULL; + LogDebug(BCLog::RPC, "Invalid block: %s", debug); + return UniValue{reason}; } const UniValue& aClientRules = oparam.find_value("rules"); diff --git a/src/validation.cpp b/src/validation.cpp index 1213d8be9f9..bd38f5b83c9 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4648,40 +4648,82 @@ MempoolAcceptResult ChainstateManager::ProcessTransaction(const CTransactionRef& return result; } -bool TestBlockValidity(BlockValidationState& state, - const CChainParams& chainparams, - Chainstate& chainstate, +bool TestBlockValidity(Chainstate& chainstate, const CBlock& block, - CBlockIndex* pindexPrev, - bool fCheckPOW, - bool fCheckMerkleRoot) + const bool check_pow, + const bool check_merkle_root, + std::string& reason, + std::string& debug) { - 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; + // Lock must be held throughout this function for two reasons: + // 1. We don't want the tip to change during several of the validation steps + // 2. To prevent a CheckBlock() race condition for fChecked, see ProcessNewBlock() + LOCK(chainstate.m_chainman.GetMutex()); - // 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()); + BlockValidationState state; + CBlockIndex* tip{Assert(chainstate.m_chain.Tip())}; + + if (block.hashPrevBlock != *Assert(tip->phashBlock)) { + reason = "inconclusive-not-best-prevblk"; return false; } - if (!CheckBlock(block, state, chainparams.GetConsensus(), fCheckPOW, fCheckMerkleRoot)) { - LogError("%s: Consensus::CheckBlock: %s\n", __func__, state.ToString()); + + // For signets CheckBlock() verifies the challenge iff fCheckPow is set. + if (!CheckBlock(block, state, chainstate.m_chainman.GetConsensus(), /*fCheckPow=*/check_pow, /*fCheckMerkleRoot=*/check_merkle_root)) { + reason = state.GetRejectReason(); + debug = state.GetDebugMessage(); return false; } - if (!ContextualCheckBlock(block, state, chainstate.m_chainman, pindexPrev)) { - LogError("%s: Consensus::ContextualCheckBlock: %s\n", __func__, state.ToString()); + + /** + * At this point ProcessNewBlock would call AcceptBlock(), but we + * don't want to store the block or its header. Run individual checks + * instead: + * - skip AcceptBlockHeader() because: + * - we don't want to update the block index + * - we do not care about duplicates + * - we already ran CheckBlockHeader() via CheckBlock() + * - we already checked for prev-blk-not-found + * - we know the tip is valid, so no need to check bad-prevblk + * - we already ran CheckBlock() + * - do run ContextualCheckBlockHeader() + * - do run ContextualCheckBlock() + */ + + if (!ContextualCheckBlockHeader(block, state, chainstate.m_blockman, chainstate.m_chainman, tip)) { + reason = state.GetRejectReason(); + debug = state.GetDebugMessage(); return false; } - if (!chainstate.ConnectBlock(block, state, &indexDummy, viewNew, true)) { + + if (!ContextualCheckBlock(block, state, chainstate.m_chainman, tip)) { + reason = state.GetRejectReason(); + debug = state.GetDebugMessage(); + return false; + } + + // We don't want ConnectBlock to update the actual chainstate, so create + // a cache on top of it, along with a dummy block index. + CBlockIndex index_dummy{block}; + uint256 block_hash(block.GetHash()); + index_dummy.pprev = tip; + index_dummy.nHeight = tip->nHeight + 1; + index_dummy.phashBlock = &block_hash; + CCoinsViewCache tip_view(&chainstate.CoinsTip()); + CCoinsView blockCoins; + CCoinsViewCache view(&blockCoins); + view.SetBackend(tip_view); + + // Set fJustCheck to true in order to update, and not clear, validation caches. + if(!chainstate.ConnectBlock(block, state, &index_dummy, view, /*fJustCheck=*/true)) { + reason = state.GetRejectReason(); + debug = state.GetDebugMessage(); + return false; + } + if (!Assume(state.IsValid())) { + LogError("Unexpected invalid validation state"); return false; } - assert(state.IsValid()); return true; } diff --git a/src/validation.h b/src/validation.h index e361c7af101..5a0f18c09f8 100644 --- a/src/validation.h +++ b/src/validation.h @@ -383,14 +383,26 @@ public: /** 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, +/** + * Verify a block, including transactions. + * + * @param[in] block The block we want to process. Must connect to the + * current tip. + * @param[in] chainstate The chainstate to connect to. + * @param[out] reason rejection reason (BIP22) + * @param[out] debug more detailed rejection reason + * @param[in] check_pow perform proof-of-work check, nBits in the header + * is always checked + * @param[in] check_merkle_root check the merkle root + * + * For signets the challenge verification is skipped when check_pow is false. + */ +bool TestBlockValidity(Chainstate& chainstate, const CBlock& block, - CBlockIndex* pindexPrev, - bool fCheckPOW = true, - bool fCheckMerkleRoot = true) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + const bool check_pow, + const bool check_merkle_root, + std::string& reason, + std::string& debug); /** Check with the proof of work on each blockheader matches the value in nBits */ bool HasValidProofOfWork(const std::vector& headers, const Consensus::Params& consensusParams);