init, validation: Improve handling if VerifyDB() fails due to insufficient dbcache

mzumsande · mzumsande · commit 0c7785bb2540 · 2023-02-16T17:58:52.000-05:00
The rpc command verifychain now fails if the dbcache was not sufficient
to complete the verification at the specified level and depth.

In the same situation, the VerifyDB check during Init will now fail (and lead to
an early shutdown) if the user has explicitly specified -checkblocks or
-checklevel but the check couldn't be executed because of the limited
cache. If the user didn't change any of the two and is using the defaults, log a warning
but don't prevent the node from starting up.
diff --git a/src/init.cpp b/src/init.cpp
@@ -1502,6 +1502,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         options.prune = chainman.m_blockman.IsPruneMode();
         options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
         options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);
+        options.require_full_verification = args.IsArgSet("-checkblocks") || args.IsArgSet("-checklevel");
         options.check_interrupt = ShutdownRequested;
         options.coins_error_cb = [] {
             uiInterface.ThreadSafeMessageBox(
@@ -1533,7 +1534,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
             }
         }
 
-        if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB) {
+        if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB || status == node::ChainstateLoadStatus::FAILURE_INSUFFICIENT_DBCACHE) {
             return InitError(error);
         }
 
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
@@ -197,6 +197,11 @@ ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const C
                 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
@@ -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>;
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
@@ -1110,7 +1110,7 @@ static RPCHelpMan verifychain()
                     {"nblocks", RPCArg::Type::NUM, RPCArg::DefaultHint{strprintf("%d, 0=all", DEFAULT_CHECKBLOCKS)}, "The number of blocks to check."},
                 },
                 RPCResult{
-                    RPCResult::Type::BOOL, "", "Verified or not"},
+                    RPCResult::Type::BOOL, "", "Verification finished successfully. If false, check debug.log for reason."},
                 RPCExamples{
                     HelpExampleCli("verifychain", "")
             + HelpExampleRpc("verifychain", "")
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -218,6 +218,7 @@ void TestingSetup::LoadVerifyActivateChainstate()
     options.prune = chainman.m_blockman.IsPruneMode();
     options.check_blocks = m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
     options.check_level = m_args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);
+    options.require_full_verification = m_args.IsArgSet("-checkblocks") || m_args.IsArgSet("-checklevel");
     auto [status, error] = LoadChainstate(chainman, m_cache_sizes, options);
     assert(status == node::ChainstateLoadStatus::SUCCESS);
 
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -4185,6 +4185,9 @@ VerifyDBResult CVerifyDB::VerifyDB(
 
     LogPrintf("Verification: No coin database inconsistencies in last %i blocks (%i transactions)\n", block_count, nGoodTransactions);
 
+    if (skipped_l3_checks) {
+        return VerifyDBResult::SKIPPED_L3_CHECKS;
+    }
     return VerifyDBResult::SUCCESS;
 }
 
diff --git a/src/validation.h b/src/validation.h
@@ -353,6 +353,7 @@ enum class VerifyDBResult {
     SUCCESS,
     CORRUPTED_BLOCK_DB,
     INTERRUPTED,
+    SKIPPED_L3_CHECKS,
 };
 
 /** RAII wrapper for VerifyDB: Verify consistency of the block and coin databases */

Original file line number	Diff line number	Diff line change
`@@ -1502,6 +1502,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)`
`1502`	`1502`	`options.prune = chainman.m_blockman.IsPruneMode();`
`1503`	`1503`	`options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);`
`1504`	`1504`	`options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);`
	`1505`	`+ options.require_full_verification = args.IsArgSet("-checkblocks") \|\| args.IsArgSet("-checklevel");`
`1505`	`1506`	`options.check_interrupt = ShutdownRequested;`
`1506`	`1507`	`options.coins_error_cb = [] {`
`1507`	`1508`	`uiInterface.ThreadSafeMessageBox(`
`@@ -1533,7 +1534,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)`
`1533`	`1534`	`}`
`1534`	`1535`	`}`
`1535`	`1536`
`1536`		`- if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB) {`
	`1537`	`+ if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB \|\| status == node::ChainstateLoadStatus::FAILURE_INSUFFICIENT_DBCACHE) {`
`1537`	`1538`	`return InitError(error);`
`1538`	`1539`	`}`
`1539`	`1540`
Original file line number	Diff line number	Diff line change
`@@ -197,6 +197,11 @@ ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const C`
`197`	`197`	`break;`
`198`	`198`	`case VerifyDBResult::CORRUPTED_BLOCK_DB:`
`199`	`199`	`return {ChainstateLoadStatus::FAILURE, _("Corrupted block database detected")};`
	`200`	`+ case VerifyDBResult::SKIPPED_L3_CHECKS:`
	`201`	`+ if (options.require_full_verification) {`
	`202`	`+ return {ChainstateLoadStatus::FAILURE_INSUFFICIENT_DBCACHE, _("Insufficient dbcache for block verification")};`
	`203`	`+ }`
	`204`	`+ break;`
`200`	`205`	`} // no default case, so the compiler can warn about missing cases`
`201`	`206`	`}`
`202`	`207`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4185,6 +4185,9 @@ VerifyDBResult CVerifyDB::VerifyDB(`
`4185`	`4185`
`4186`	`4186`	`LogPrintf("Verification: No coin database inconsistencies in last %i blocks (%i transactions)\n", block_count, nGoodTransactions);`
`4187`	`4187`
	`4188`	`+ if (skipped_l3_checks) {`
	`4189`	`+ return VerifyDBResult::SKIPPED_L3_CHECKS;`
	`4190`	`+ }`
`4188`	`4191`	`return VerifyDBResult::SUCCESS;`
`4189`	`4192`	`}`
`4190`	`4193`