From 6360b5302d2675788de5c4a28ea77d823f6d809e Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Thu, 6 Oct 2022 15:51:09 -0400 Subject: [PATCH] validation: Change return value of VerifyDB to enum type This does not change behavior. It is in preparation for special handling of the case where VerifyDB doesn't finish for various reasons, but doesn't fail. --- src/node/chainstate.cpp | 14 +++++++++----- src/rpc/blockchain.cpp | 2 +- src/validation.cpp | 39 ++++++++++++++++++++++++--------------- src/validation.h | 7 ++++++- 4 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index ba1024d22e1..245dfee14fc 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -187,12 +187,16 @@ ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const C "Only rebuild the block database if you are sure that your computer's date and time are correct")}; } - if (!CVerifyDB().VerifyDB( - *chainstate, chainman.GetConsensus(), chainstate->CoinsDB(), - options.check_level, - options.check_blocks)) { + VerifyDBResult result = CVerifyDB().VerifyDB( + *chainstate, chainman.GetConsensus(), chainstate->CoinsDB(), + options.check_level, + options.check_blocks); + switch (result) { + case VerifyDBResult::SUCCESS: + break; + case VerifyDBResult::CORRUPTED_BLOCK_DB: return {ChainstateLoadStatus::FAILURE, _("Corrupted block database detected")}; - } + } // no default case, so the compiler can warn about missing cases } } diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 8bee066ab80..e2a1efea55a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1125,7 +1125,7 @@ static RPCHelpMan verifychain() Chainstate& active_chainstate = chainman.ActiveChainstate(); return CVerifyDB().VerifyDB( - active_chainstate, chainman.GetParams().GetConsensus(), active_chainstate.CoinsTip(), check_level, check_depth); + active_chainstate, chainman.GetParams().GetConsensus(), active_chainstate.CoinsTip(), check_level, check_depth) == VerifyDBResult::SUCCESS; }, }; } diff --git a/src/validation.cpp b/src/validation.cpp index f0ffb748dd6..29299a8eb3a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4057,7 +4057,7 @@ CVerifyDB::~CVerifyDB() uiInterface.ShowProgress("", 100, false); } -bool CVerifyDB::VerifyDB( +VerifyDBResult CVerifyDB::VerifyDB( Chainstate& chainstate, const Consensus::Params& consensus_params, CCoinsView& coinsview, @@ -4066,7 +4066,7 @@ bool CVerifyDB::VerifyDB( AssertLockHeld(cs_main); if (chainstate.m_chain.Tip() == nullptr || chainstate.m_chain.Tip()->pprev == nullptr) { - return true; + return VerifyDBResult::SUCCESS; } // Verify blocks in the best chain @@ -4106,19 +4106,22 @@ bool CVerifyDB::VerifyDB( CBlock block; // check level 0: read from disk if (!ReadBlockFromDisk(block, pindex, consensus_params)) { - return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); + LogPrintf("Verification error: ReadBlockFromDisk failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + return VerifyDBResult::CORRUPTED_BLOCK_DB; } // check level 1: verify block validity if (nCheckLevel >= 1 && !CheckBlock(block, state, consensus_params)) { - return error("%s: *** found bad block at %d, hash=%s (%s)\n", __func__, - pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); + LogPrintf("Verification error: found bad block at %d, hash=%s (%s)\n", + pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); + return VerifyDBResult::CORRUPTED_BLOCK_DB; } // check level 2: verify undo validity if (nCheckLevel >= 2 && pindex) { CBlockUndo undo; if (!pindex->GetUndoPos().IsNull()) { if (!UndoReadFromDisk(undo, pindex)) { - return error("VerifyDB(): *** found bad undo data at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + LogPrintf("Verification error: found bad undo data at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + return VerifyDBResult::CORRUPTED_BLOCK_DB; } } } @@ -4130,7 +4133,8 @@ bool CVerifyDB::VerifyDB( assert(coins.GetBestBlock() == pindex->GetBlockHash()); DisconnectResult res = chainstate.DisconnectBlock(block, pindex, coins); if (res == DISCONNECT_FAILED) { - return error("VerifyDB(): *** irrecoverable inconsistency in block data at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); + LogPrintf("Verification error: irrecoverable inconsistency in block data at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + return VerifyDBResult::CORRUPTED_BLOCK_DB; } if (res == DISCONNECT_UNCLEAN) { nGoodTransactions = 0; @@ -4142,14 +4146,16 @@ bool CVerifyDB::VerifyDB( skipped_l3_checks = true; } } - if (ShutdownRequested()) return true; + if (ShutdownRequested()) return VerifyDBResult::SUCCESS; } if (pindexFailure) { - return error("VerifyDB(): *** coin database inconsistencies found (last %i blocks, %i good transactions before that)\n", chainstate.m_chain.Height() - pindexFailure->nHeight + 1, nGoodTransactions); + LogPrintf("Verification error: coin database inconsistencies found (last %i blocks, %i good transactions before that)\n", chainstate.m_chain.Height() - pindexFailure->nHeight + 1, nGoodTransactions); + return VerifyDBResult::CORRUPTED_BLOCK_DB; } if (skipped_l3_checks) { LogPrintf("Skipped verification of level >=3 (insufficient database cache size). Consider increasing -dbcache.\n"); } + // store block count as we move pindex at check level >= 4 int block_count = chainstate.m_chain.Height() - pindex->nHeight; @@ -4165,18 +4171,21 @@ bool CVerifyDB::VerifyDB( uiInterface.ShowProgress(_("Verifying blocks…").translated, percentageDone, false); pindex = chainstate.m_chain.Next(pindex); CBlock block; - if (!ReadBlockFromDisk(block, pindex, consensus_params)) - return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); - if (!chainstate.ConnectBlock(block, state, pindex, coins)) { - return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); + if (!ReadBlockFromDisk(block, pindex, consensus_params)) { + LogPrintf("Verification error: ReadBlockFromDisk failed at %d, hash=%s\n", pindex->nHeight, pindex->GetBlockHash().ToString()); + return VerifyDBResult::CORRUPTED_BLOCK_DB; } - if (ShutdownRequested()) return true; + if (!chainstate.ConnectBlock(block, state, pindex, coins)) { + LogPrintf("Verification error: found unconnectable block at %d, hash=%s (%s)\n", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); + return VerifyDBResult::CORRUPTED_BLOCK_DB; + } + if (ShutdownRequested()) return VerifyDBResult::SUCCESS; } } LogPrintf("Verification: No coin database inconsistencies in last %i blocks (%i transactions)\n", block_count, nGoodTransactions); - return true; + return VerifyDBResult::SUCCESS; } /** Apply the effects of a block on the utxo cache, ignoring that it may already have been applied. */ diff --git a/src/validation.h b/src/validation.h index 7170467b006..7e30fc95de4 100644 --- a/src/validation.h +++ b/src/validation.h @@ -349,12 +349,17 @@ bool HasValidProofOfWork(const std::vector& headers, const Consens /** Return the sum of the work on a given set of headers */ arith_uint256 CalculateHeadersWork(const std::vector& headers); +enum class VerifyDBResult { + SUCCESS, + CORRUPTED_BLOCK_DB, +}; + /** RAII wrapper for VerifyDB: Verify consistency of the block and coin databases */ class CVerifyDB { public: CVerifyDB(); ~CVerifyDB(); - bool VerifyDB( + [[nodiscard]] VerifyDBResult VerifyDB( Chainstate& chainstate, const Consensus::Params& consensus_params, CCoinsView& coinsview,