Merge bitcoin/bitcoin#25308: refactor: Reduce number of LoadChainstate parameters and return values

1e761a0169 ci: Enable IWYU in src/kernel directory (Ryan Ofsky)
6db6552377 refactor: Reduce number of SanityChecks return values (Ryan Ofsky)
b3e7de7ee6 refactor: Reduce number of LoadChainstate return values (Russell Yanofsky)
3b91d4b994 refactor: Reduce number of LoadChainstate parameters (Russell Yanofsky)

Pull request description:

  Replace long LoadChainstate parameters list with options struct. Replace long list of return values with simpler error strings.

  No changes in behavior. Motivation is just to make libbitcoin_kernel API easier to use and more future-proof, and make internal code clearer and more maintainable.

ACKs for top commit:
  MarcoFalke:
    ACK 1e761a0169 🕚

Tree-SHA512: 86f251ab820ca6664ade87ccac8330f79b0e48e26b98082f022f592ed1380f8eefc3cce260b85d5eea5d2f5f2531602e03d641e579c15684ecd9093b2aebcc58

ci/test/06_script_b.sh

@@ -42,7 +42,7 @@ if [ "${RUN_TIDY}" = "true" ]; then
           " src/compat"\
           " src/dbwrapper.cpp"\
           " src/init"\
-          " src/kernel/mempool_persist.cpp"\
+          " src/kernel"\
           " src/node/chainstate.cpp"\
           " src/policy/feerate.cpp"\
           " src/policy/packages.cpp"\

src/bitcoin-chainstate.cpp

@@ -18,6 +18,7 @@
 #include <consensus/validation.h>
 #include <core_io.h>
 #include <node/blockstorage.h>
+#include <node/caches.h>
 #include <node/chainstate.h>
 #include <scheduler.h>
 #include <script/sigcache.h>
@@ -83,27 +84,19 @@ int main(int argc, char* argv[])
     };
     ChainstateManager chainman{chainman_opts};
 
-    auto rv = node::LoadChainstate(false,
+    node::CacheSizes cache_sizes;
-                                   std::ref(chainman),
+    cache_sizes.block_tree_db = 2 << 20;
-                                   nullptr,
+    cache_sizes.coins_db = 2 << 22;
-                                   false,
+    cache_sizes.coins = (450 << 20) - (2 << 20) - (2 << 22);
-                                   false,
+    node::ChainstateLoadOptions options;
-                                   2 << 20,
+    options.check_interrupt = [] { return false; };
-                                   2 << 22,
+    auto [status, error] = node::LoadChainstate(chainman, cache_sizes, options);
-                                   (450 << 20) - (2 << 20) - (2 << 22),
+    if (status != node::ChainstateLoadStatus::SUCCESS) {
-                                   false,
-                                   false,
-                                   []() { return false; });
-    if (rv.has_value()) {
         std::cerr << "Failed to load Chain state from your datadir." << std::endl;
         goto epilogue;
     } else {
-        auto maybe_verify_error = node::VerifyLoadedChainstate(std::ref(chainman),
+        std::tie(status, error) = node::VerifyLoadedChainstate(chainman, options);
-                                                               false,
+        if (status != node::ChainstateLoadStatus::SUCCESS) {
-                                                               false,
-                                                               DEFAULT_CHECKBLOCKS,
-                                                               DEFAULT_CHECKLEVEL);
-        if (maybe_verify_error.has_value()) {
             std::cerr << "Failed to verify loaded Chain state from your datadir." << std::endl;
             goto epilogue;
         }

src/init.cpp

@@ -108,8 +108,6 @@ using kernel::DumpMempool;
 
 using node::CacheSizes;
 using node::CalculateCacheSizes;
-using node::ChainstateLoadVerifyError;
-using node::ChainstateLoadingError;
 using node::DEFAULT_PERSIST_MEMPOOL;
 using node::DEFAULT_PRINTPRIORITY;
 using node::DEFAULT_STOPAFTERBLOCKIMPORT;
@@ -1098,21 +1096,8 @@ static bool LockDataDirectory(bool probeOnly)
 bool AppInitSanityChecks(const kernel::Context& kernel)
 {
     // ********************************************************* Step 4: sanity checks
-    auto maybe_error = kernel::SanityChecks(kernel);
+    if (auto error = kernel::SanityChecks(kernel)) {
-
+        InitError(*error);
-    if (maybe_error.has_value()) {
-        switch (maybe_error.value()) {
-        case kernel::SanityCheckError::ERROR_ECC:
-            InitError(Untranslated("Elliptic curve cryptography sanity check failure. Aborting."));
-            break;
-        case kernel::SanityCheckError::ERROR_RANDOM:
-            InitError(Untranslated("OS cryptographic RNG sanity check failure. Aborting."));
-            break;
-        case kernel::SanityCheckError::ERROR_CHRONO:
-            InitError(Untranslated("Clock epoch mismatch. Aborting."));
-            break;
-        } // no default case, so the compiler can warn about missing cases
-
         return InitError(strprintf(_("Initialization sanity check failed. %s is shutting down."), PACKAGE_NAME));
     }
 
@@ -1452,112 +1437,54 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         node.chainman = std::make_unique<ChainstateManager>(chainman_opts);
         ChainstateManager& chainman = *node.chainman;
 
-        const bool fReset = fReindex;
+        node::ChainstateLoadOptions options;
-        bilingual_str strLoadError;
+        options.mempool = Assert(node.mempool.get());
+        options.reindex = node::fReindex;
+        options.reindex_chainstate = fReindexChainState;
+        options.prune = node::fPruneMode;
+        options.check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
+        options.check_level = args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);
+        options.check_interrupt = ShutdownRequested;
+        options.coins_error_cb = [] {
+            uiInterface.ThreadSafeMessageBox(
+                _("Error reading from database, shutting down."),
+                "", CClientUIInterface::MSG_ERROR);
+        };
 
         uiInterface.InitMessage(_("Loading block index…").translated);
         const int64_t load_block_index_start_time = GetTimeMillis();
-        std::optional<ChainstateLoadingError> maybe_load_error;
+        auto catch_exceptions = [](auto&& f) {
-        try {
-            maybe_load_error = LoadChainstate(fReset,
-                                              chainman,
-                                              Assert(node.mempool.get()),
-                                              fPruneMode,
-                                              fReindexChainState,
-                                              cache_sizes.block_tree_db,
-                                              cache_sizes.coins_db,
-                                              cache_sizes.coins,
-                                              /*block_tree_db_in_memory=*/false,
-                                              /*coins_db_in_memory=*/false,
-                                              /*shutdown_requested=*/ShutdownRequested,
-                                              /*coins_error_cb=*/[]() {
-                                                  uiInterface.ThreadSafeMessageBox(
-                                                                                   _("Error reading from database, shutting down."),
-                                                                                   "", CClientUIInterface::MSG_ERROR);
-                                              });
-        } catch (const std::exception& e) {
-            LogPrintf("%s\n", e.what());
-            maybe_load_error = ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED;
-        }
-        if (maybe_load_error.has_value()) {
-            switch (maybe_load_error.value()) {
-            case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB:
-                strLoadError = _("Error loading block database");
-                break;
-            case ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK:
-                // If the loaded chain has a wrong genesis, bail out immediately
-                // (we're likely using a testnet datadir, or the other way around).
-                return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?"));
-            case ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX:
-                strLoadError = _("You need to rebuild the database using -reindex to go back to unpruned mode.  This will redownload the entire blockchain");
-                break;
-            case ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED:
-                strLoadError = _("Error initializing block database");
-                break;
-            case ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED:
-                return InitError(_("Unsupported chainstate database format found. "
-                                   "Please restart with -reindex-chainstate. This will "
-                                   "rebuild the chainstate database."));
-            case ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED:
-                strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.");
-                break;
-            case ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED:
-                strLoadError = _("Error initializing block database");
-                break;
-            case ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED:
-                strLoadError = _("Error opening block database");
-                break;
-            case ChainstateLoadingError::ERROR_BLOCKS_WITNESS_INSUFFICIENTLY_VALIDATED:
-                strLoadError = strprintf(_("Witness data for blocks after height %d requires validation. Please restart with -reindex."),
-                                         chainman.GetConsensus().SegwitHeight);
-                break;
-            case ChainstateLoadingError::SHUTDOWN_PROBED:
-                break;
-            }
-        } else {
-            std::optional<ChainstateLoadVerifyError> maybe_verify_error;
             try {
-                uiInterface.InitMessage(_("Verifying blocks…").translated);
+                return f();
-                auto check_blocks = args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
-                if (chainman.m_blockman.m_have_pruned && check_blocks > MIN_BLOCKS_TO_KEEP) {
-                    LogPrintfCategory(BCLog::PRUNE, "pruned datadir may not have more than %d blocks; only checking available blocks\n",
-                                      MIN_BLOCKS_TO_KEEP);
-                }
-                maybe_verify_error = VerifyLoadedChainstate(chainman,
-                                                            fReset,
-                                                            fReindexChainState,
-                                                            check_blocks,
-                                                            args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL));
             } catch (const std::exception& e) {
                 LogPrintf("%s\n", e.what());
-                maybe_verify_error = ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE;
+                return std::make_tuple(node::ChainstateLoadStatus::FAILURE, _("Error opening block database"));
             }
-            if (maybe_verify_error.has_value()) {
+        };
-                switch (maybe_verify_error.value()) {
+        auto [status, error] = catch_exceptions([&]{ return LoadChainstate(chainman, cache_sizes, options); });
-                case ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE:
+        if (status == node::ChainstateLoadStatus::SUCCESS) {
-                    strLoadError = _("The block database contains a block which appears to be from the future. "
+            uiInterface.InitMessage(_("Verifying blocks…").translated);
-                                     "This may be due to your computer's date and time being set incorrectly. "
+            if (chainman.m_blockman.m_have_pruned && options.check_blocks > MIN_BLOCKS_TO_KEEP) {
-                                     "Only rebuild the block database if you are sure that your computer's date and time are correct");
+                LogPrintfCategory(BCLog::PRUNE, "pruned datadir may not have more than %d blocks; only checking available blocks\n",
-                    break;
+                                  MIN_BLOCKS_TO_KEEP);
-                case ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB:
+            }
-                    strLoadError = _("Corrupted block database detected");
+            std::tie(status, error) = catch_exceptions([&]{ return VerifyLoadedChainstate(chainman, options);});
-                    break;
+            if (status == node::ChainstateLoadStatus::SUCCESS) {
-                case ChainstateLoadVerifyError::ERROR_GENERIC_FAILURE:
-                    strLoadError = _("Error opening block database");
-                    break;
-                }
-            } else {
                 fLoaded = true;
                 LogPrintf(" block index %15dms\n", GetTimeMillis() - load_block_index_start_time);
             }
         }
 
+        if (status == node::ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB) {
+            return InitError(error);
+        }
+
         if (!fLoaded && !ShutdownRequested()) {
             // first suggest a reindex
-            if (!fReset) {
+            if (!options.reindex) {
                 bool fRet = uiInterface.ThreadSafeQuestion(
-                    strLoadError + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"),
+                    error + Untranslated(".\n\n") + _("Do you want to rebuild the block database now?"),
-                    strLoadError.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.",
+                    error.original + ".\nPlease restart with -reindex or -reindex-chainstate to recover.",
                     "", CClientUIInterface::MSG_ERROR | CClientUIInterface::BTN_ABORT);
                 if (fRet) {
                     fReindex = true;
@@ -1567,7 +1494,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                     return false;
                 }
             } else {
-                return InitError(strLoadError);
+                return InitError(error);
             }
         }
     }

src/kernel/checks.cpp

@@ -7,21 +7,24 @@
 #include <key.h>
 #include <random.h>
 #include <util/time.h>
+#include <util/translation.h>
+
+#include <memory>
 
 namespace kernel {
 
-std::optional<SanityCheckError> SanityChecks(const Context&)
+std::optional<bilingual_str> SanityChecks(const Context&)
 {
     if (!ECC_InitSanityCheck()) {
-        return SanityCheckError::ERROR_ECC;
+        return Untranslated("Elliptic curve cryptography sanity check failure. Aborting.");
     }
 
     if (!Random_SanityCheck()) {
-        return SanityCheckError::ERROR_RANDOM;
+        return Untranslated("OS cryptographic RNG sanity check failure. Aborting.");
     }
 
     if (!ChronoSanityCheck()) {
-        return SanityCheckError::ERROR_CHRONO;
+        return Untranslated("Clock epoch mismatch. Aborting.");
     }
 
     return std::nullopt;

src/kernel/checks.h

@@ -7,20 +7,16 @@
 
 #include <optional>
 
+struct bilingual_str;
+
 namespace kernel {
 
 struct Context;
 
-enum class SanityCheckError {
-    ERROR_ECC,
-    ERROR_RANDOM,
-    ERROR_CHRONO,
-};
-
 /**
  *  Ensure a usable environment with all necessary library support.
  */
-std::optional<SanityCheckError> SanityChecks(const Context&);
+std::optional<bilingual_str> SanityChecks(const Context&);
 
 }
 

src/kernel/coinstats.cpp

@@ -4,16 +4,32 @@
 
 #include <kernel/coinstats.h>
 
+#include <chain.h>
 #include <coins.h>
 #include <crypto/muhash.h>
 #include <hash.h>
+#include <node/blockstorage.h>
+#include <primitives/transaction.h>
+#include <script/script.h>
 #include <serialize.h>
+#include <span.h>
+#include <streams.h>
+#include <sync.h>
+#include <tinyformat.h>
 #include <uint256.h>
+#include <util/check.h>
 #include <util/overflow.h>
 #include <util/system.h>
 #include <validation.h>
+#include <version.h>
 
+#include <cassert>
+#include <iosfwd>
+#include <iterator>
 #include <map>
+#include <memory>
+#include <string>
+#include <utility>
 
 namespace kernel {
 

src/kernel/coinstats.h

@@ -5,16 +5,18 @@
 #ifndef BITCOIN_KERNEL_COINSTATS_H
 #define BITCOIN_KERNEL_COINSTATS_H
 
-#include <chain.h>
-#include <coins.h>
 #include <consensus/amount.h>
 #include <streams.h>
 #include <uint256.h>
 
 #include <cstdint>
 #include <functional>
+#include <optional>
 
 class CCoinsView;
+class Coin;
+class COutPoint;
+class CScript;
 namespace node {
 class BlockManager;
 } // namespace node

src/node/chainstate.cpp

@@ -8,6 +8,7 @@
 #include <coins.h>
 #include <consensus/params.h>
 #include <node/blockstorage.h>
+#include <node/caches.h>
 #include <sync.h>
 #include <threadsafety.h>
 #include <txdb.h>
@@ -22,61 +23,54 @@
 #include <vector>
 
 namespace node {
-std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
+ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
-                                                     ChainstateManager& chainman,
+                                    const ChainstateLoadOptions& options)
-                                                     CTxMemPool* mempool,
-                                                     bool fPruneMode,
-                                                     bool fReindexChainState,
-                                                     int64_t nBlockTreeDBCache,
-                                                     int64_t nCoinDBCache,
-                                                     int64_t nCoinCacheUsage,
-                                                     bool block_tree_db_in_memory,
-                                                     bool coins_db_in_memory,
-                                                     std::function<bool()> shutdown_requested,
-                                                     std::function<void()> coins_error_cb)
 {
     auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull();
+        return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
     };
 
     LOCK(cs_main);
-    chainman.InitializeChainstate(mempool);
+    chainman.InitializeChainstate(options.mempool);
-    chainman.m_total_coinstip_cache = nCoinCacheUsage;
+    chainman.m_total_coinstip_cache = cache_sizes.coins;
-    chainman.m_total_coinsdb_cache = nCoinDBCache;
+    chainman.m_total_coinsdb_cache = cache_sizes.coins_db;
 
     auto& pblocktree{chainman.m_blockman.m_block_tree_db};
     // new CBlockTreeDB tries to delete the existing file, which
     // fails if it's still open from the previous loop. Close it first:
     pblocktree.reset();
-    pblocktree.reset(new CBlockTreeDB(nBlockTreeDBCache, block_tree_db_in_memory, fReset));
+    pblocktree.reset(new CBlockTreeDB(cache_sizes.block_tree_db, options.block_tree_db_in_memory, options.reindex));
 
-    if (fReset) {
+    if (options.reindex) {
         pblocktree->WriteReindexing(true);
         //If we're reindexing in prune mode, wipe away unusable block files and all undo data files
-        if (fPruneMode)
+        if (options.prune) {
             CleanupBlockRevFiles();
+        }
     }
 
-    if (shutdown_requested && shutdown_requested()) return ChainstateLoadingError::SHUTDOWN_PROBED;
+    if (options.check_interrupt && options.check_interrupt()) return {ChainstateLoadStatus::INTERRUPTED, {}};
 
     // LoadBlockIndex will load m_have_pruned if we've ever removed a
     // block file from disk.
-    // Note that it also sets fReindex based on the disk flag!
+    // Note that it also sets fReindex global based on the disk flag!
-    // From here on out fReindex and fReset mean something different!
+    // From here on, fReindex and options.reindex values may be different!
     if (!chainman.LoadBlockIndex()) {
-        if (shutdown_requested && shutdown_requested()) return ChainstateLoadingError::SHUTDOWN_PROBED;
+        if (options.check_interrupt && options.check_interrupt()) return {ChainstateLoadStatus::INTERRUPTED, {}};
-        return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB;
+        return {ChainstateLoadStatus::FAILURE, _("Error loading block database")};
     }
 
     if (!chainman.BlockIndex().empty() &&
             !chainman.m_blockman.LookupBlockIndex(chainman.GetConsensus().hashGenesisBlock)) {
-        return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK;
+        // If the loaded chain has a wrong genesis, bail out immediately
+        // (we're likely using a testnet datadir, or the other way around).
+        return {ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB, _("Incorrect or no genesis block found. Wrong datadir for network?")};
     }
 
     // Check for changed -prune state.  What we are concerned about is a user who has pruned blocks
     // in the past, but is now trying to run unpruned.
-    if (chainman.m_blockman.m_have_pruned && !fPruneMode) {
+    if (chainman.m_blockman.m_have_pruned && !options.prune) {
-        return ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX;
+        return {ChainstateLoadStatus::FAILURE, _("You need to rebuild the database using -reindex to go back to unpruned mode.  This will redownload the entire blockchain")};
     }
 
     // At this point blocktree args are consistent with what's on disk.
@@ -84,7 +78,7 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
     // (otherwise we use the one already on disk).
     // This is called again in ThreadImport after the reindex completes.
     if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) {
-        return ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED;
+        return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
     }
 
     // At this point we're either in reindex or we've loaded a useful
@@ -92,57 +86,56 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
 
     for (CChainState* chainstate : chainman.GetAll()) {
         chainstate->InitCoinsDB(
-            /*cache_size_bytes=*/nCoinDBCache,
+            /*cache_size_bytes=*/cache_sizes.coins_db,
-            /*in_memory=*/coins_db_in_memory,
+            /*in_memory=*/options.coins_db_in_memory,
-            /*should_wipe=*/fReset || fReindexChainState);
+            /*should_wipe=*/options.reindex || options.reindex_chainstate);
 
-        if (coins_error_cb) {
+        if (options.coins_error_cb) {
-            chainstate->CoinsErrorCatcher().AddReadErrCallback(coins_error_cb);
+            chainstate->CoinsErrorCatcher().AddReadErrCallback(options.coins_error_cb);
         }
 
         // Refuse to load unsupported database format.
         // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate
         if (chainstate->CoinsDB().NeedsUpgrade()) {
-            return ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED;
+            return {ChainstateLoadStatus::FAILURE_INCOMPATIBLE_DB, _("Unsupported chainstate database format found. "
+                                                                     "Please restart with -reindex-chainstate. This will "
+                                                                     "rebuild the chainstate database.")};
         }
 
         // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate
         if (!chainstate->ReplayBlocks()) {
-            return ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED;
+            return {ChainstateLoadStatus::FAILURE, _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.")};
         }
 
         // The on-disk coinsdb is now in a good state, create the cache
-        chainstate->InitCoinsCache(nCoinCacheUsage);
+        chainstate->InitCoinsCache(cache_sizes.coins);
         assert(chainstate->CanFlushToDisk());
 
         if (!is_coinsview_empty(chainstate)) {
             // LoadChainTip initializes the chain based on CoinsTip()'s best block
             if (!chainstate->LoadChainTip()) {
-                return ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED;
+                return {ChainstateLoadStatus::FAILURE, _("Error initializing block database")};
             }
             assert(chainstate->m_chain.Tip() != nullptr);
         }
     }
 
-    if (!fReset) {
+    if (!options.reindex) {
         auto chainstates{chainman.GetAll()};
         if (std::any_of(chainstates.begin(), chainstates.end(),
                         [](const CChainState* cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
-            return ChainstateLoadingError::ERROR_BLOCKS_WITNESS_INSUFFICIENTLY_VALIDATED;
+            return {ChainstateLoadStatus::FAILURE, strprintf(_("Witness data for blocks after height %d requires validation. Please restart with -reindex."),
-        }
+                                                             chainman.GetConsensus().SegwitHeight)};
+        };
     }
 
-    return std::nullopt;
+    return {ChainstateLoadStatus::SUCCESS, {}};
 }
 
-std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManager& chainman,
+ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options)
-                                                                bool fReset,
-                                                                bool fReindexChainState,
-                                                                int check_blocks,
-                                                                int check_level)
 {
     auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
-        return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull();
+        return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
     };
 
     LOCK(cs_main);
@@ -151,18 +144,20 @@ std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManage
         if (!is_coinsview_empty(chainstate)) {
             const CBlockIndex* tip = chainstate->m_chain.Tip();
             if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) {
-                return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE;
+                return {ChainstateLoadStatus::FAILURE, _("The block database contains a block which appears to be from the future. "
+                                                         "This may be due to your computer's date and time being set incorrectly. "
+                                                         "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(),
-                    check_level,
+                    options.check_level,
-                    check_blocks)) {
+                    options.check_blocks)) {
-                return ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB;
+                return {ChainstateLoadStatus::FAILURE, _("Corrupted block database detected")};
             }
         }
     }
 
-    return std::nullopt;
+    return {ChainstateLoadStatus::SUCCESS, {}};
 }
 } // namespace node

src/node/chainstate.h

@@ -5,6 +5,8 @@
 #ifndef BITCOIN_NODE_CHAINSTATE_H
 #define BITCOIN_NODE_CHAINSTATE_H
 
+#include <validation.h>
+
 #include <cstdint>
 #include <functional>
 #include <optional>
@@ -13,19 +15,31 @@ class ChainstateManager;
 class CTxMemPool;
 
 namespace node {
-enum class ChainstateLoadingError {
+
-    ERROR_LOADING_BLOCK_DB,
+struct CacheSizes;
-    ERROR_BAD_GENESIS_BLOCK,
+
-    ERROR_PRUNED_NEEDS_REINDEX,
+struct ChainstateLoadOptions {
-    ERROR_LOAD_GENESIS_BLOCK_FAILED,
+    CTxMemPool* mempool{nullptr};
-    ERROR_CHAINSTATE_UPGRADE_FAILED,
+    bool block_tree_db_in_memory{false};
-    ERROR_REPLAYBLOCKS_FAILED,
+    bool coins_db_in_memory{false};
-    ERROR_LOADCHAINTIP_FAILED,
+    bool reindex{false};
-    ERROR_GENERIC_BLOCKDB_OPEN_FAILED,
+    bool reindex_chainstate{false};
-    ERROR_BLOCKS_WITNESS_INSUFFICIENTLY_VALIDATED,
+    bool prune{false};
-    SHUTDOWN_PROBED,
+    int64_t check_blocks{DEFAULT_CHECKBLOCKS};
+    int64_t check_level{DEFAULT_CHECKLEVEL};
+    std::function<bool()> check_interrupt;
+    std::function<void()> coins_error_cb;
 };
 
+//! Chainstate load status. Simple applications can just check for the success
+//! 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 };
+
+//! Chainstate load status code and optional error string.
+using ChainstateLoadResult = std::tuple<ChainstateLoadStatus, bilingual_str>;
+
 /** This sequence can have 4 types of outcomes:
  *
  *  1. Success
@@ -37,45 +51,11 @@ enum class ChainstateLoadingError {
  *  4. Hard failure
  *    - a failure that definitively cannot be recovered from with a reindex
  *
- *  Currently, LoadChainstate returns a std::optional<ChainstateLoadingError>
+ *  LoadChainstate returns a (status code, error string) tuple.
- *  which:
- *
- *  - if has_value()
- *      - Either "Soft failure", "Hard failure", or "Shutdown requested",
- *        differentiable by the specific enumerator.
- *
- *        Note that a return value of SHUTDOWN_PROBED means ONLY that "during
- *        this sequence, when we explicitly checked shutdown_requested() at
- *        arbitrary points, one of those calls returned true". Therefore, a
- *        return value other than SHUTDOWN_PROBED does not guarantee that
- *        shutdown hasn't been called indirectly.
- *  - else
- *      - Success!
  */
-std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
+ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
-                                                     ChainstateManager& chainman,
+                                    const ChainstateLoadOptions& options);
-                                                     CTxMemPool* mempool,
+ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options);
-                                                     bool fPruneMode,
-                                                     bool fReindexChainState,
-                                                     int64_t nBlockTreeDBCache,
-                                                     int64_t nCoinDBCache,
-                                                     int64_t nCoinCacheUsage,
-                                                     bool block_tree_db_in_memory,
-                                                     bool coins_db_in_memory,
-                                                     std::function<bool()> shutdown_requested = nullptr,
-                                                     std::function<void()> coins_error_cb = nullptr);
-
-enum class ChainstateLoadVerifyError {
-    ERROR_BLOCK_FROM_FUTURE,
-    ERROR_CORRUPTED_BLOCK_DB,
-    ERROR_GENERIC_FAILURE,
-};
-
-std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManager& chainman,
-                                                                bool fReset,
-                                                                bool fReindexChainState,
-                                                                int check_blocks,
-                                                                int check_level);
 } // namespace node
 
 #endif // BITCOIN_NODE_CHAINSTATE_H

src/test/util/setup_common.cpp

@@ -54,8 +54,6 @@
 
 using node::BlockAssembler;
 using node::CalculateCacheSizes;
-using node::fPruneMode;
-using node::fReindex;
 using node::LoadChainstate;
 using node::NodeContext;
 using node::RegenerateCommitments;
@@ -218,25 +216,20 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
     // instead of unit tests, but for now we need these here.
     RegisterAllCoreRPCCommands(tableRPC);
 
-    auto maybe_load_error = LoadChainstate(fReindex.load(),
+    node::ChainstateLoadOptions options;
-                                           *Assert(m_node.chainman.get()),
+    options.mempool = Assert(m_node.mempool.get());
-                                           Assert(m_node.mempool.get()),
+    options.block_tree_db_in_memory = true;
-                                           fPruneMode,
+    options.coins_db_in_memory = true;
-                                           m_args.GetBoolArg("-reindex-chainstate", false),
+    options.reindex = node::fReindex;
-                                           m_cache_sizes.block_tree_db,
+    options.reindex_chainstate = m_args.GetBoolArg("-reindex-chainstate", false);
-                                           m_cache_sizes.coins_db,
+    options.prune = node::fPruneMode;
-                                           m_cache_sizes.coins,
+    options.check_blocks = m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS);
-                                           /*block_tree_db_in_memory=*/true,
+    options.check_level = m_args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL);
-                                           /*coins_db_in_memory=*/true);
+    auto [status, error] = LoadChainstate(*Assert(m_node.chainman), m_cache_sizes, options);
-    assert(!maybe_load_error.has_value());
+    assert(status == node::ChainstateLoadStatus::SUCCESS);
 
-    auto maybe_verify_error = VerifyLoadedChainstate(
+    std::tie(status, error) = VerifyLoadedChainstate(*Assert(m_node.chainman), options);
-        *Assert(m_node.chainman),
+    assert(status == node::ChainstateLoadStatus::SUCCESS);
-        fReindex.load(),
-        m_args.GetBoolArg("-reindex-chainstate", false),
-        m_args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS),
-        m_args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL));
-    assert(!maybe_verify_error.has_value());
 
     BlockValidationState state;
     if (!m_node.chainman->ActiveChainstate().ActivateBestChain(state)) {