refactor: Reduce number of LoadChainstate return values

2 files changed, 37 insertions, 56 deletions

diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index 47248953d2..bb6b2df1b7 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -23,8 +23,8 @@
 #include <vector>
 
 namespace node {
-std::optional<ChainstateLoadingError> LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
-                                                     const ChainstateLoadOptions& options)
+ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
+                                    const ChainstateLoadOptions& options)
 {
     auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
@@ -49,26 +49,28 @@ std::optional<ChainstateLoadingError> LoadChainstate(ChainstateManager& chainman
         }
     }
 
-    if (options.check_interrupt && options.check_interrupt()) 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 global based on the disk flag!
     // From here on, fReindex and options.reindex values may be different!
     if (!chainman.LoadBlockIndex()) {
-        if (options.check_interrupt && options.check_interrupt()) return ChainstateLoadingError::SHUTDOWN_PROBED;
-        return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB;
+        if (options.check_interrupt && options.check_interrupt()) return {ChainstateLoadStatus::INTERRUPTED, {}};
+        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 && !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.
@@ -76,7 +78,7 @@ std::optional<ChainstateLoadingError> LoadChainstate(ChainstateManager& chainman
     // (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
@@ -95,12 +97,14 @@ std::optional<ChainstateLoadingError> LoadChainstate(ChainstateManager& chainman
         // 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
@@ -110,7 +114,7 @@ std::optional<ChainstateLoadingError> LoadChainstate(ChainstateManager& chainman
         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);
         }
@@ -120,15 +124,15 @@ std::optional<ChainstateLoadingError> LoadChainstate(ChainstateManager& chainman
         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,
-                                                                const ChainstateLoadOptions& options)
+ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options)
 {
     auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         return options.reindex || options.reindex_chainstate || chainstate->CoinsTip().GetBestBlock().IsNull();
@@ -140,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(),
                     options.check_level,
                     options.check_blocks)) {
-                return ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB;
+                return {ChainstateLoadStatus::FAILURE, _("Corrupted block database detected")};
             }
         }
     }
 
-    return std::nullopt;
+    return {ChainstateLoadStatus::SUCCESS, {}};
 }
 } // namespace node
diff --git a/src/node/chainstate.h b/src/node/chainstate.h
index 4851ef1ae6..649cfb2526 100644
--- a/src/node/chainstate.h
+++ b/src/node/chainstate.h
@@ -31,18 +31,14 @@ struct ChainstateLoadOptions {
     std::function<void()> coins_error_cb;
 };
 
-enum class ChainstateLoadingError {
-    ERROR_LOADING_BLOCK_DB,
-    ERROR_BAD_GENESIS_BLOCK,
-    ERROR_PRUNED_NEEDS_REINDEX,
-    ERROR_LOAD_GENESIS_BLOCK_FAILED,
-    ERROR_CHAINSTATE_UPGRADE_FAILED,
-    ERROR_REPLAYBLOCKS_FAILED,
-    ERROR_LOADCHAINTIP_FAILED,
-    ERROR_GENERIC_BLOCKDB_OPEN_FAILED,
-    ERROR_BLOCKS_WITNESS_INSUFFICIENTLY_VALIDATED,
-    SHUTDOWN_PROBED,
-};
+//! 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:
  *
@@ -55,32 +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>
- *  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!
+ *  LoadChainstate returns a (status code, error string) tuple.
  */
-std::optional<ChainstateLoadingError> LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
-                                                     const ChainstateLoadOptions& options);
-
-enum class ChainstateLoadVerifyError {
-    ERROR_BLOCK_FROM_FUTURE,
-    ERROR_CORRUPTED_BLOCK_DB,
-    ERROR_GENERIC_FAILURE,
-};
-
-std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManager& chainman,
-                                                                const ChainstateLoadOptions& options);
+ChainstateLoadResult LoadChainstate(ChainstateManager& chainman, const CacheSizes& cache_sizes,
+                                    const ChainstateLoadOptions& options);
+ChainstateLoadResult VerifyLoadedChainstate(ChainstateManager& chainman, const ChainstateLoadOptions& options);
 } // namespace node
 
 #endif // BITCOIN_NODE_CHAINSTATE_H