node/chainstate: Decouple from stringy errors

dongcarl · dongcarl · commit ae9121f958a4 · 2021-12-06T15:56:50.000-05:00
This allows us to separate the initialization code from translations and
error reporting.

This change changes the caller semantics of LoadChainstate quite
drastically.

To see that this change doesn't change behaviour, observe that:

1. Prior to this change, LoadChainstate returned false only in the "bad
   genesis block" failure case (by returning InitError()), indicating
   that the caller should immediately bail. After this change, the
   corresponding ERROR_BAD_GENESIS_BLOCK handler in src/init.cpp
   maintains behavioue by also bailing immediately.

2. The failed_* temporary booleans were only used to break out of the
   outer do/while(false) loop. They can therefore be safely removed.
diff --git a/src/init.cpp b/src/init.cpp
@@ -1418,11 +1418,8 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         bilingual_str strLoadError;
 
         uiInterface.InitMessage(_("Loading block index…").translated);
-
         const int64_t load_block_index_start_time = GetTimeMillis();
-        bool rv = LoadChainstate(fLoaded,
-                                 strLoadError,
-                                 fReset,
+        auto rv = LoadChainstate(fReset,
                                  chainman,
                                  node,
                                  fPruneMode,
@@ -1432,8 +1429,48 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
                                  nBlockTreeDBCache,
                                  nCoinDBCache,
                                  nCoinCacheUsage);
-        if (!rv) return false;
-        if (fLoaded) {
+        if (rv.has_value()) {
+            switch (rv.value()) {
+            case ChainstateLoadingError::ERROR_LOADING_BLOCK_DB:
+                strLoadError = _("Error loading block database");
+                break;
+            case ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK:
+                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:
+                strLoadError = _("Error upgrading chainstate database");
+                break;
+            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."),
+                                         chainparams.GetConsensus().SegwitHeight);
+                break;
+            case ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE:
+                strLoadError = _("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");
+                break;
+            case ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB:
+                strLoadError = _("Corrupted block database detected");
+                break;
+            case ChainstateLoadingError::SHUTDOWN_PROBED:
+                break;
+            }
+        } else {
+            fLoaded = true;
             LogPrintf(" block index %15dms\n", GetTimeMillis() - load_block_index_start_time);
         }
 
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
@@ -7,25 +7,23 @@
 #include <chainparams.h> // for CChainParams
 #include <rpc/blockchain.h> // for RPCNotifyBlockChange
 #include <util/time.h> // for GetTime
-#include <util/translation.h> // for bilingual_str
 #include <node/blockstorage.h> // for CleanupBlockRevFiles, fHavePruned, fReindex
 #include <node/context.h> // for NodeContext
 #include <node/ui_interface.h> // for InitError, uiInterface, and CClientUIInterface member access
 #include <shutdown.h> // for ShutdownRequested
 #include <validation.h> // for a lot of things
 
-bool LoadChainstate(bool& fLoaded,
-                    bilingual_str& strLoadError,
-                    bool fReset,
-                    ChainstateManager& chainman,
-                    NodeContext& node,
-                    bool fPruneMode,
-                    const CChainParams& chainparams,
-                    const ArgsManager& args,
-                    bool fReindexChainState,
-                    int64_t nBlockTreeDBCache,
-                    int64_t nCoinDBCache,
-                    int64_t nCoinCacheUsage) {
+std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
+                                                     ChainstateManager& chainman,
+                                                     NodeContext& node,
+                                                     bool fPruneMode,
+                                                     const CChainParams& chainparams,
+                                                     const ArgsManager& args,
+                                                     bool fReindexChainState,
+                                                     int64_t nBlockTreeDBCache,
+                                                     int64_t nCoinDBCache,
+                                                     int64_t nCoinCacheUsage)
+{
     auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
         return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull();
     };
@@ -52,46 +50,41 @@ bool LoadChainstate(bool& fLoaded,
                     CleanupBlockRevFiles();
             }
 
-            if (ShutdownRequested()) break;
+            if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED;
 
             // LoadBlockIndex will load fHavePruned if we've ever removed a
             // block file from disk.
             // Note that it also sets fReindex based on the disk flag!
             // From here on out fReindex and fReset mean something different!
             if (!chainman.LoadBlockIndex()) {
-                if (ShutdownRequested()) break;
-                strLoadError = _("Error loading block database");
-                break;
+                if (ShutdownRequested()) return ChainstateLoadingError::SHUTDOWN_PROBED;
+                return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB;
             }
 
             // If the loaded chain has a wrong genesis, bail out immediately
             // (we're likely using a testnet datadir, or the other way around).
             if (!chainman.BlockIndex().empty() &&
                     !chainman.m_blockman.LookupBlockIndex(chainparams.GetConsensus().hashGenesisBlock)) {
-                return InitError(_("Incorrect or no genesis block found. Wrong datadir for network?"));
+                return ChainstateLoadingError::ERROR_BAD_GENESIS_BLOCK;
             }
 
             // 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 (fHavePruned && !fPruneMode) {
-                strLoadError = _("You need to rebuild the database using -reindex to go back to unpruned mode.  This will redownload the entire blockchain");
-                break;
+                return ChainstateLoadingError::ERROR_PRUNED_NEEDS_REINDEX;
             }
 
             // At this point blocktree args are consistent with what's on disk.
             // If we're not mid-reindex (based on disk + args), add a genesis block on disk
             // (otherwise we use the one already on disk).
             // This is called again in ThreadImport after the reindex completes.
             if (!fReindex && !chainman.ActiveChainstate().LoadGenesisBlock()) {
-                strLoadError = _("Error initializing block database");
-                break;
+                return ChainstateLoadingError::ERROR_LOAD_GENESIS_BLOCK_FAILED;
             }
 
             // At this point we're either in reindex or we've loaded a useful
             // block tree into BlockIndex()!
 
-            bool failed_chainstate_init = false;
-
             for (CChainState* chainstate : chainman.GetAll()) {
                 chainstate->InitCoinsDB(
                     /* cache_size_bytes */ nCoinDBCache,
@@ -107,16 +100,12 @@ bool LoadChainstate(bool& fLoaded,
                 // If necessary, upgrade from older database format.
                 // This is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate
                 if (!chainstate->CoinsDB().Upgrade()) {
-                    strLoadError = _("Error upgrading chainstate database");
-                    failed_chainstate_init = true;
-                    break;
+                    return ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED;
                 }
 
                 // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate
                 if (!chainstate->ReplayBlocks()) {
-                    strLoadError = _("Unable to replay blocks. You will need to rebuild the database using -reindex-chainstate.");
-                    failed_chainstate_init = true;
-                    break;
+                    return ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED;
                 }
 
                 // The on-disk coinsdb is now in a good state, create the cache
@@ -126,36 +115,25 @@ bool LoadChainstate(bool& fLoaded,
                 if (!is_coinsview_empty(chainstate)) {
                     // LoadChainTip initializes the chain based on CoinsTip()'s best block
                     if (!chainstate->LoadChainTip()) {
-                        strLoadError = _("Error initializing block database");
-                        failed_chainstate_init = true;
-                        break; // out of the per-chainstate loop
+                        return ChainstateLoadingError::ERROR_LOADCHAINTIP_FAILED;
                     }
                     assert(chainstate->m_chain.Tip() != nullptr);
                 }
             }
-
-            if (failed_chainstate_init) {
-                break; // out of the chainstate activation do-while
-            }
         } catch (const std::exception& e) {
             LogPrintf("%s\n", e.what());
-            strLoadError = _("Error opening block database");
-            break;
+            return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED;
         }
 
         if (!fReset) {
             LOCK(cs_main);
             auto chainstates{chainman.GetAll()};
             if (std::any_of(chainstates.begin(), chainstates.end(),
                             [](const CChainState* cs) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return cs->NeedsRedownload(); })) {
-                strLoadError = strprintf(_("Witness data for blocks after height %d requires validation. Please restart with -reindex."),
-                                         chainparams.GetConsensus().SegwitHeight);
-                break;
+                return ChainstateLoadingError::ERROR_BLOCKS_WITNESS_INSUFFICIENTLY_VALIDATED;
             }
         }
 
-        bool failed_verification = false;
-
         try {
             LOCK(cs_main);
 
@@ -170,33 +148,21 @@ bool LoadChainstate(bool& fLoaded,
                     const CBlockIndex* tip = chainstate->m_chain.Tip();
                     RPCNotifyBlockChange(tip);
                     if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) {
-                        strLoadError = _("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");
-                        failed_verification = true;
-                        break;
+                        return ChainstateLoadingError::ERROR_BLOCK_FROM_FUTURE;
                     }
 
                     if (!CVerifyDB().VerifyDB(
                             *chainstate, chainparams, chainstate->CoinsDB(),
                             args.GetIntArg("-checklevel", DEFAULT_CHECKLEVEL),
                             args.GetIntArg("-checkblocks", DEFAULT_CHECKBLOCKS))) {
-                        strLoadError = _("Corrupted block database detected");
-                        failed_verification = true;
-                        break;
+                        return ChainstateLoadingError::ERROR_CORRUPTED_BLOCK_DB;
                     }
                 }
             }
         } catch (const std::exception& e) {
             LogPrintf("%s\n", e.what());
-            strLoadError = _("Error opening block database");
-            failed_verification = true;
-            break;
-        }
-
-        if (!failed_verification) {
-            fLoaded = true;
+            return ChainstateLoadingError::ERROR_GENERIC_BLOCKDB_OPEN_FAILED;
         }
     } while(false);
-    return true;
+    return std::nullopt;
 }
diff --git a/src/node/chainstate.h b/src/node/chainstate.h
@@ -6,13 +6,28 @@
 #define BITCOIN_NODE_CHAINSTATE_H
 
 #include <cstdint> // for int64_t
+#include <optional> // for std::optional
 
 class ArgsManager;
-struct bilingual_str;
 class CChainParams;
 class ChainstateManager;
 struct NodeContext;
 
+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,
+    ERROR_BLOCK_FROM_FUTURE,
+    ERROR_CORRUPTED_BLOCK_DB,
+    SHUTDOWN_PROBED,
+};
+
 /** This sequence can have 4 types of outcomes:
  *
  *  1. Success
@@ -24,25 +39,30 @@ struct NodeContext;
  *  4. Hard failure
  *    - a failure that definitively cannot be recovered from with a reindex
  *
- *  Currently, LoadChainstate returns a bool which:
- *      - if false
- *          - Definitely a "Hard failure"
- *      - if true
- *          - if fLoaded -> "Success"
- *          - if ShutdownRequested() -> "Shutdown requested"
- *          - else -> "Soft failure"
+ *  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 ShutdownRequested() at
+ *        arbitrary points, one of those calls returned true". Therefore, a
+ *        return value other than SHUTDOWN_PROBED does not guarantee that
+ *        ShutdownRequested() hasn't been called indirectly.
+ *  - else
+ *      - Success!
  */
-bool LoadChainstate(bool& fLoaded,
-                    bilingual_str& strLoadError,
-                    bool fReset,
-                    ChainstateManager& chainman,
-                    NodeContext& node,
-                    bool fPruneMode,
-                    const CChainParams& chainparams,
-                    const ArgsManager& args,
-                    bool fReindexChainState,
-                    int64_t nBlockTreeDBCache,
-                    int64_t nCoinDBCache,
-                    int64_t nCoinCacheUsage);
+std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
+                                                     ChainstateManager& chainman,
+                                                     NodeContext& node,
+                                                     bool fPruneMode,
+                                                     const CChainParams& chainparams,
+                                                     const ArgsManager& args,
+                                                     bool fReindexChainState,
+                                                     int64_t nBlockTreeDBCache,
+                                                     int64_t nCoinDBCache,
+                                                     int64_t nCoinCacheUsage);
 
 #endif // BITCOIN_NODE_CHAINSTATE_H
