diff options
author | Andrew Chow <github@achow101.com> | 2023-02-22 14:10:18 -0500 |
---|---|---|
committer | Andrew Chow <github@achow101.com> | 2023-02-22 14:19:44 -0500 |
commit | 832fa2d238006765edbabb66ec7c453175713ed3 (patch) | |
tree | a3aaf5eca9b0ed21be4fbfcc3a85dbf2979b0a22 /src/node | |
parent | 9f6ef0c156ac26fb043387b915002e10f44c80b8 (diff) | |
parent | 0af16e7134459e0820ab95d751093876c1ec4c6d (diff) |
Merge bitcoin/bitcoin#25574: validation: Improve error handling when VerifyDB dosn't finish successfully
0af16e7134459e0820ab95d751093876c1ec4c6d doc: add release note for #25574 (Martin Zumsande)
57ef2a4812f443b2d734f43cebf3ef5038da83f2 validation: report if pruning prevents completion of verification (Martin Zumsande)
0c7785bb2540b69564104767d38342704230cbc2 init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache (Martin Zumsande)
d6f781f1cfcbc2c2ad5ee289a0642ed00386d013 validation: return VerifyDBResult::INTERRUPTED if verification was interrupted (Martin Zumsande)
6360b5302d2675788de5c4a28ea77d823f6d809e validation: Change return value of VerifyDB to enum type (Martin Zumsande)
Pull request description:
`VerifyDB()` can fail to complete due to insufficient dbcache at the level 3 checks. This PR improves the error handling in this case in the following ways:
- The rpc `-verifychain` now returns false if the check can't be completed due to insufficient cache
- During init, we only log a warning if the default values for `-checkblocks` and `-checklevel` are taken and the check doesn't complete. However, if the user actively specifies one of these args, we return with an InitError if we can't complete the check.
This PR also changes `-verifychain` RPC to return `false` if the verification didn't finish due to missing block data (pruning) or due to being interrupted by the node being shutdown.
Previously, this PR also included a fix for a possible assert during verification - this was done in #27009 (now merged).
ACKs for top commit:
achow101:
ACK 0af16e7134459e0820ab95d751093876c1ec4c6d
ryanofsky:
Code review ACK 0af16e7134459e0820ab95d751093876c1ec4c6d. Only small suggested changes since the last review, like renaming some of the enum values. I did leave more suggestions, but they are not very important and could be followups
john-moffett:
ACK 0af16e7134459e0820ab95d751093876c1ec4c6d
MarcoFalke:
lgtm re-ACK 0af16e7134459e0820ab95d751093876c1ec4c6d 🎚
Tree-SHA512: 84b4f767cf9bfbafef362312757c9bf765b41ae3977f4ece840e40c52a2266b1457832df0cdf70440be0aac2168d9b58fc817238630b0b6812f3836ca950bc0e
Diffstat (limited to 'src/node')
-rw-r--r-- | src/node/chainstate.cpp | 21 | ||||
-rw-r--r-- | src/node/chainstate.h | 9 |
2 files changed, 24 insertions, 6 deletions
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp index 4741c4c421..41c0ff2118 100644 --- a/src/node/chainstate.cpp +++ b/src/node/chainstate.cpp @@ -192,12 +192,23 @@ 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: + case VerifyDBResult::INTERRUPTED: + case VerifyDBResult::SKIPPED_MISSING_BLOCKS: + break; + case VerifyDBResult::CORRUPTED_BLOCK_DB: return {ChainstateLoadStatus::FAILURE, _("Corrupted block database detected")}; - } + case VerifyDBResult::SKIPPED_L3_CHECKS: + if (options.require_full_verification) { + return {ChainstateLoadStatus::FAILURE_INSUFFICIENT_DBCACHE, _("Insufficient dbcache for block verification")}; + } + break; + } // no default case, so the compiler can warn about missing cases } } diff --git a/src/node/chainstate.h b/src/node/chainstate.h index d3c7656bf2..7838a62d0c 100644 --- a/src/node/chainstate.h +++ b/src/node/chainstate.h @@ -25,6 +25,7 @@ struct ChainstateLoadOptions { bool reindex{false}; bool reindex_chainstate{false}; bool prune{false}; + bool require_full_verification{true}; int64_t check_blocks{DEFAULT_CHECKBLOCKS}; int64_t check_level{DEFAULT_CHECKLEVEL}; std::function<bool()> check_interrupt; @@ -35,7 +36,13 @@ struct ChainstateLoadOptions { //! case, and treat other cases as errors. More complex applications may want to //! try reindexing in the generic failure case, and pass an interrupt callback //! and exit cleanly in the interrupted case. -enum class ChainstateLoadStatus { SUCCESS, FAILURE, FAILURE_INCOMPATIBLE_DB, INTERRUPTED }; +enum class ChainstateLoadStatus { + SUCCESS, + FAILURE, + FAILURE_INCOMPATIBLE_DB, + FAILURE_INSUFFICIENT_DBCACHE, + INTERRUPTED, +}; //! Chainstate load status code and optional error string. using ChainstateLoadResult = std::tuple<ChainstateLoadStatus, bilingual_str>; |