Merge bitcoin/bitcoin#23280: init: Coalesce Chainstate loading sequence between {,non-}unittest codepaths

7f15eff style-only: Remove redundant scope in *Chainstate (Carl Dong)
89bec82 Collapse the 2 cs_main locks in LoadChainstate (Carl Dong)
3b1584b Remove all #include // for * comments (Carl Dong)
9a5a5a3 test/setup: Use LoadChainstate (Carl Dong)
c541da0 node/chainstate: Add options for in-memory DBs (Carl Dong)
ceb9790 node/caches: Remove intermediate variables (Carl Dong)
ac4bf13 node/caches: Extract cache calculation logic (Carl Dong)
15f2e33 validation: VerifyDB only needs Consensus::Params (Carl Dong)
4da9c07 node/chainstate: Decouple from ShutdownRequested (Carl Dong)
05441c2 node/chainstate: Decouple from GetTime (Carl Dong)
2414ebc init: Delay RPC block notif until warmup finished (Carl Dong)
8d466a8 Move -checkblocks LogPrintf to AppInitMain (Carl Dong)
aad8d59 node/chainstate: Reduce coupling of LogPrintf (Carl Dong)
b345979 node/chainstate: Decouple from concept of uiInterface (Carl Dong)
ca7c0b9 Split off VerifyLoadedChainstate (Carl Dong)
adf4912 node/chainstate: Remove do/while loop (Carl Dong)
975235c Move init logistics message for BAD_GENESIS_BLOCK to init.cpp (Carl Dong)
8715658 Move mempool nullptr Assert out of LoadChainstate (Carl Dong)
9162a4f node/chainstate: Decouple from concept of NodeContext (Carl Dong)
c7a5c46 node/chainstate: Decouple from ArgsManager (Carl Dong)
ae9121f node/chainstate: Decouple from stringy errors (Carl Dong)
cbac28b node/chainstate: Decouple from GetTimeMillis (Carl Dong)
cb64af9 node: Extract chainstate loading sequence (Carl Dong)

Pull request description:

  This PR:
  1. Coalesce the Chainstate loading sequence between `AppInitMain` and `*TestingSetup` (which makes it more tested)
  2. Makes the Chainstate loading sequence reusable in preparation for future work extracting out our consensus engine.

  Code-wise, this PR:
  1. Extracts `AppInitMain`'s Chainstate loading sequence into a `::LoadChainstateSequence` function
  2. Makes this `::LoadChainstateSequence` function reusable by
      1. Decoupling it from various concepts (`ArgsManager`, `uiInterface`, etc)
      2. Making it report errors using an `enum` rather than by setting a `bilingual_str`
  3. Makes `*TestingSetup` use this new `::LoadChainstateSequence`

  Reviewers: Aside from commentary, I've also included `git diff` flags of interest in the commit messages which I hope will aid review!

ACKs for top commit:
  ryanofsky:
    Code review ACK 7f15eff. Thanks for updates!
  MarcoFalke:
    review ACK 7f15eff 💳

Tree-SHA512: fb9a6cbd1c511a52b477c62a5e68e53a8be5dec2fff0e44a279966afb91efbab44bf1fe7c6b1519f8464ecc25f42dd4bae8e1efbf55ee91fc90fa0b92e3a83e2

Author: MarcoFalke

src/Makefile.am

@@ -174,6 +174,8 @@ BITCOIN_CORE_H = \
   netbase.h \
   netmessagemaker.h \
   node/blockstorage.h \
+  node/caches.h \
+  node/chainstate.h \
   node/coin.h \
   node/coinstats.h \
   node/context.h \
@@ -339,6 +341,8 @@ libbitcoin_server_a_SOURCES = \
   net.cpp \
   net_processing.cpp \
   node/blockstorage.cpp \
+  node/caches.cpp \
+  node/chainstate.cpp \
   node/coin.cpp \
   node/coinstats.cpp \
   node/context.cpp \

src/init.cpp

@@ -0,0 +1,32 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <node/caches.h>
+
+#include <txdb.h>
+#include <util/system.h>
+#include <validation.h>
+
+CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes)
+{
+    int64_t nTotalCache = (args.GetIntArg("-dbcache", nDefaultDbCache) << 20);
+    nTotalCache = std::max(nTotalCache, nMinDbCache << 20); // total cache cannot be less than nMinDbCache
+    nTotalCache = std::min(nTotalCache, nMaxDbCache << 20); // total cache cannot be greater than nMaxDbcache
+    CacheSizes sizes;
+    sizes.block_tree_db = std::min(nTotalCache / 8, nMaxBlockDBCache << 20);
+    nTotalCache -= sizes.block_tree_db;
+    sizes.tx_index = std::min(nTotalCache / 8, args.GetBoolArg("-txindex", DEFAULT_TXINDEX) ? nMaxTxIndexCache << 20 : 0);
+    nTotalCache -= sizes.tx_index;
+    sizes.filter_index = 0;
+    if (n_indexes > 0) {
+        int64_t max_cache = std::min(nTotalCache / 8, max_filter_index_cache << 20);
+        sizes.filter_index = max_cache / n_indexes;
+        nTotalCache -= sizes.filter_index * n_indexes;
+    }
+    sizes.coins_db = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache
+    sizes.coins_db = std::min(sizes.coins_db, nMaxCoinsDBCache << 20); // cap total coins db cache
+    nTotalCache -= sizes.coins_db;
+    sizes.coins = nTotalCache; // the rest goes to in-memory cache
+    return sizes;
+}

src/node/caches.cpp

@@ -0,0 +1,22 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_NODE_CACHES_H
+#define BITCOIN_NODE_CACHES_H
+
+#include <cstddef>
+#include <cstdint>
+
+class ArgsManager;
+
+struct CacheSizes {
+    int64_t block_tree_db;
+    int64_t coins_db;
+    int64_t coins;
+    int64_t tx_index;
+    int64_t filter_index;
+};
+CacheSizes CalculateCacheSizes(const ArgsManager& args, size_t n_indexes = 0);
+
+#endif // BITCOIN_NODE_CACHES_H

src/node/caches.h

@@ -0,0 +1,158 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <node/chainstate.h>
+
+#include <consensus/params.h>
+#include <node/blockstorage.h>
+#include <validation.h>
+
+std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
+                                                     ChainstateManager& chainman,
+                                                     CTxMemPool* mempool,
+                                                     bool fPruneMode,
+                                                     const Consensus::Params& consensus_params,
+                                                     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();
+    };
+
+    LOCK(cs_main);
+    chainman.InitializeChainstate(mempool);
+    chainman.m_total_coinstip_cache = nCoinCacheUsage;
+    chainman.m_total_coinsdb_cache = nCoinDBCache;
+
+    UnloadBlockIndex(mempool, chainman);
+
+    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));
+
+    if (fReset) {
+        pblocktree->WriteReindexing(true);
+        //If we're reindexing in prune mode, wipe away unusable block files and all undo data files
+        if (fPruneMode)
+            CleanupBlockRevFiles();
+    }
+
+    if (shutdown_requested && shutdown_requested()) 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 (shutdown_requested && shutdown_requested()) return ChainstateLoadingError::SHUTDOWN_PROBED;
+        return ChainstateLoadingError::ERROR_LOADING_BLOCK_DB;
+    }
+
+    if (!chainman.BlockIndex().empty() &&
+            !chainman.m_blockman.LookupBlockIndex(consensus_params.hashGenesisBlock)) {
+        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) {
+        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()) {
+        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()!
+
+    for (CChainState* chainstate : chainman.GetAll()) {
+        chainstate->InitCoinsDB(
+            /* cache_size_bytes */ nCoinDBCache,
+            /* in_memory */ coins_db_in_memory,
+            /* should_wipe */ fReset || fReindexChainState);
+
+        if (coins_error_cb) {
+            chainstate->CoinsErrorCatcher().AddReadErrCallback(coins_error_cb);
+        }
+
+        // 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()) {
+            return ChainstateLoadingError::ERROR_CHAINSTATE_UPGRADE_FAILED;
+        }
+
+        // ReplayBlocks is a no-op if we cleared the coinsviewdb with -reindex or -reindex-chainstate
+        if (!chainstate->ReplayBlocks()) {
+            return ChainstateLoadingError::ERROR_REPLAYBLOCKS_FAILED;
+        }
+
+        // The on-disk coinsdb is now in a good state, create the cache
+        chainstate->InitCoinsCache(nCoinCacheUsage);
+        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;
+            }
+            assert(chainstate->m_chain.Tip() != nullptr);
+        }
+    }
+
+    if (!fReset) {
+        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 std::nullopt;
+}
+
+std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManager& chainman,
+                                                                bool fReset,
+                                                                bool fReindexChainState,
+                                                                const Consensus::Params& consensus_params,
+                                                                unsigned int check_blocks,
+                                                                unsigned int check_level,
+                                                                std::function<int64_t()> get_unix_time_seconds)
+{
+    auto is_coinsview_empty = [&](CChainState* chainstate) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
+        return fReset || fReindexChainState || chainstate->CoinsTip().GetBestBlock().IsNull();
+    };
+
+    LOCK(cs_main);
+
+    for (CChainState* chainstate : chainman.GetAll()) {
+        if (!is_coinsview_empty(chainstate)) {
+            const CBlockIndex* tip = chainstate->m_chain.Tip();
+            if (tip && tip->nTime > get_unix_time_seconds() + 2 * 60 * 60) {
+                return ChainstateLoadVerifyError::ERROR_BLOCK_FROM_FUTURE;
+            }
+
+            if (!CVerifyDB().VerifyDB(
+                    *chainstate, consensus_params, chainstate->CoinsDB(),
+                    check_level,
+                    check_blocks)) {
+                return ChainstateLoadVerifyError::ERROR_CORRUPTED_BLOCK_DB;
+            }
+        }
+    }
+
+    return std::nullopt;
+}

src/node/chainstate.cpp

@@ -0,0 +1,85 @@
+// Copyright (c) 2021 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_NODE_CHAINSTATE_H
+#define BITCOIN_NODE_CHAINSTATE_H
+
+#include <cstdint>
+#include <functional>
+#include <optional>
+
+class ChainstateManager;
+namespace Consensus {
+    struct Params;
+}
+class CTxMemPool;
+
+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,
+};
+
+/** This sequence can have 4 types of outcomes:
+ *
+ *  1. Success
+ *  2. Shutdown requested
+ *    - nothing failed but a shutdown was triggered in the middle of the
+ *      sequence
+ *  3. Soft failure
+ *    - a failure that might be recovered from with a reindex
+ *  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_requested() hasn't been called indirectly.
+ *  - else
+ *      - Success!
+ */
+std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
+                                                     ChainstateManager& chainman,
+                                                     CTxMemPool* mempool,
+                                                     bool fPruneMode,
+                                                     const Consensus::Params& consensus_params,
+                                                     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,
+                                                                const Consensus::Params& consensus_params,
+                                                                unsigned int check_blocks,
+                                                                unsigned int check_level,
+                                                                std::function<int64_t()> get_unix_time_seconds);
+
+#endif // BITCOIN_NODE_CHAINSTATE_H

src/node/chainstate.h

@@ -1387,7 +1387,7 @@ static RPCHelpMan verifychain()
 
     CChainState& active_chainstate = chainman.ActiveChainstate();
     return CVerifyDB().VerifyDB(
-        active_chainstate, Params(), active_chainstate.CoinsTip(), check_level, check_depth);
+        active_chainstate, Params().GetConsensus(), active_chainstate.CoinsTip(), check_level, check_depth);
 },
     };
 }

src/rpc/blockchain.cpp

@@ -17,13 +17,16 @@
 #include <net_processing.h>
 #include <node/miner.h>
 #include <noui.h>
+#include <node/blockstorage.h>
+#include <node/chainstate.h>
 #include <policy/fees.h>
 #include <pow.h>
 #include <rpc/blockchain.h>
 #include <rpc/register.h>
 #include <rpc/server.h>
 #include <scheduler.h>
 #include <script/sigcache.h>
+#include <shutdown.h>
 #include <streams.h>
 #include <txdb.h>
 #include <util/strencodings.h>
@@ -143,8 +146,10 @@ ChainTestingSetup::ChainTestingSetup(const std::string& chainName, const std::ve
     m_node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
     m_node.mempool = std::make_unique<CTxMemPool>(m_node.fee_estimator.get(), 1);
 
+    m_cache_sizes = CalculateCacheSizes(m_args);
+
     m_node.chainman = std::make_unique<ChainstateManager>();
-    m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(1 << 20, true);
+    m_node.chainman->m_blockman.m_block_tree_db = std::make_unique<CBlockTreeDB>(m_cache_sizes.block_tree_db, true);
 
     // Start script-checking threads. Set g_parallel_script_checks to true so they are used.
     constexpr int script_check_threads = 2;
@@ -177,15 +182,18 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const
     // instead of unit tests, but for now we need these here.
     RegisterAllCoreRPCCommands(tableRPC);
 
-    m_node.chainman->InitializeChainstate(m_node.mempool.get());
-    m_node.chainman->ActiveChainstate().InitCoinsDB(
-        /*cache_size_bytes=*/1 << 23, /*in_memory=*/true, /*should_wipe=*/false);
-    assert(!m_node.chainman->ActiveChainstate().CanFlushToDisk());
-    m_node.chainman->ActiveChainstate().InitCoinsCache(1 << 23);
-    assert(m_node.chainman->ActiveChainstate().CanFlushToDisk());
-    if (!m_node.chainman->ActiveChainstate().LoadGenesisBlock()) {
-        throw std::runtime_error("LoadGenesisBlock failed.");
-    }
+    auto rv = LoadChainstate(fReindex.load(),
+                             *Assert(m_node.chainman.get()),
+                             Assert(m_node.mempool.get()),
+                             fPruneMode,
+                             chainparams.GetConsensus(),
+                             m_args.GetBoolArg("-reindex-chainstate", false),
+                             m_cache_sizes.block_tree_db,
+                             m_cache_sizes.coins_db,
+                             m_cache_sizes.coins,
+                             true,
+                             true);
+    assert(!rv.has_value());
 
     BlockValidationState state;
     if (!m_node.chainman->ActiveChainstate().ActivateBestChain(state)) {