From 5921b863e39e5c3997895ffee1c87159e37a5d6f Mon Sep 17 00:00:00 2001
From: Carl Dong <contact@carldong.me>
Date: Mon, 13 Sep 2021 17:02:55 -0400
Subject: [PATCH] init: Reset mempool and chainman via reconstruction

Fixes https://github.com/bitcoin/bitcoin/issues/22964

Previously, we used UnloadBlockIndex() in order to reset node.mempool
and node.chainman. However, that has proven to be fragile (see
https://github.com/bitcoin/bitcoin/issues/22964), and requires
UnloadBlockIndex and its callees to be updated manually for each member
that's introduced to the mempool and chainman classes.

In this commit, we stop using the UnloadBlockIndex function and we
simply reconstruct node.mempool and node.chainman.

Since PeerManager needs a valid reference to both node.mempool and
node.chainman, we also move PeerManager's construction via `::make` to
after the chainstate activation sequence is complete.

There are no more callers to UnloadBlockIndex after this commit, so it
and its sole callees can be pruned.
---
 src/init.cpp            | 29 ++++++++++++++++-------------
 src/node/chainstate.cpp |  2 --
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index cccb088eec7..11d2b61da87 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1288,19 +1288,6 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     // as they would never get updated.
     if (!ignores_incoming_txs) node.fee_estimator = std::make_unique<CBlockPolicyEstimator>();
 
-    assert(!node.mempool);
-    int check_ratio = std::min<int>(std::max<int>(args.GetIntArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000);
-    node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get(), check_ratio);
-
-    assert(!node.chainman);
-    node.chainman = std::make_unique<ChainstateManager>();
-    ChainstateManager& chainman = *node.chainman;
-
-    assert(!node.peerman);
-    node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(),
-                                     chainman, *node.mempool, ignores_incoming_txs);
-    RegisterValidationInterface(node.peerman.get());
-
     // sanitize comments per BIP-0014, format user agent and check total size
     std::vector<std::string> uacomments;
     for (const std::string& cmt : args.GetArgs("-uacomment")) {
@@ -1429,8 +1416,17 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
     LogPrintf("* Using %.1f MiB for chain state database\n", cache_sizes.coins_db * (1.0 / 1024 / 1024));
     LogPrintf("* Using %.1f MiB for in-memory UTXO set (plus up to %.1f MiB of unused mempool space)\n", cache_sizes.coins * (1.0 / 1024 / 1024), nMempoolSizeMax * (1.0 / 1024 / 1024));
 
+    assert(!node.mempool);
+    assert(!node.chainman);
+    int check_ratio = std::min<int>(std::max<int>(args.GetIntArg("-checkmempool", chainparams.DefaultConsistencyChecks() ? 1 : 0), 0), 1000000);
+
     bool fLoaded = false;
     while (!fLoaded && !ShutdownRequested()) {
+        node.mempool = std::make_unique<CTxMemPool>(node.fee_estimator.get(), check_ratio);
+
+        node.chainman = std::make_unique<ChainstateManager>();
+        ChainstateManager& chainman = *node.chainman;
+
         const bool fReset = fReindex;
         bilingual_str strLoadError;
 
@@ -1562,6 +1558,13 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
         return false;
     }
 
+    ChainstateManager& chainman = *Assert(node.chainman);
+
+    assert(!node.peerman);
+    node.peerman = PeerManager::make(chainparams, *node.connman, *node.addrman, node.banman.get(),
+                                     chainman, *node.mempool, ignores_incoming_txs);
+    RegisterValidationInterface(node.peerman.get());
+
     // ********************************************************* Step 8: start indexers
     if (args.GetBoolArg("-txindex", DEFAULT_TXINDEX)) {
         if (const auto error{WITH_LOCK(cs_main, return CheckLegacyTxindex(*Assert(chainman.m_blockman.m_block_tree_db)))}) {
diff --git a/src/node/chainstate.cpp b/src/node/chainstate.cpp
index e43211402ca..99615dea692 100644
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -32,8 +32,6 @@ std::optional<ChainstateLoadingError> LoadChainstate(bool fReset,
     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: