Merge #21270: [Bundle 4/n] Prune g_chainman usage in validation-adjacent modules

a67983cd6d8e61565da4e03f3ba401d0148fe195 net_processing: Add review-only assertion to PeerManager (Carl Dong)
272d993e759e7fcfe883b84e9a2a3be3c75177ec scripted-diff: net_processing: Use existing chainman (Carl Dong)
021a04a46915468e7508a6ef44e7fbab1426343d net_processing: Move some static functions to PeerManager (Carl Dong)
91c5b68acd12cf7c2b4888d54d8fdd21837b2817 node/ifaces: ChainImpl: Use existing NodeContext member (Carl Dong)
8a1d580b2156268e3ab30f902b3fc9aa87bd2819 node/ifaces: NodeImpl: Use existing NodeContext member (Carl Dong)
4cde4a701b8856ac4ec9721b0226dbbfc52a71c3 node: Use existing NodeContext (Carl Dong)
106bcd4f390137904b5579cfef023fb8a5c8b4b5 node/coinstats: Pass in BlockManager to GetUTXOStats (Carl Dong)
2c3ba006930a5bbbf5a33bd530f3c1b2c4103c74 miner: Pass in blockman to ::RegenerateCommitments (Carl Dong)
2afcf24408b4453e4418ebfb326b141f6ea8647c miner: Remove old CreateNewBlock w/o chainstate param (Carl Dong)
46b7f29340acb399fbd2378508a204d8d8ee8fca scripted-diff: Invoke CreateNewBlock with chainstate (Carl Dong)
d0de61b764fc7e9c670b69d8210705da296dd245 miner: Pass in chainstate to BlockAssembler::CreateNewBlock (Carl Dong)
a04aac493fd564894166d58ed4cdfd9ad4f561cb validation: Remove extraneous LoadGenesisBlock function prototype (Carl Dong)

Pull request description:

  Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)

  Based on:
  - [x] #21055 | [Bundle 3/n] Prune g_chainman usage in mempool-related validation functions

  Note to reviewers:
  1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
  1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
  2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
  3. Remove `old_function`

ACKs for top commit:
  laanwj:
    Code review ACK a67983cd6d8e61565da4e03f3ba401d0148fe195
  ryanofsky:
    Code review ACK a67983cd6d8e61565da4e03f3ba401d0148fe195. Only change since last review new first commit fixing header declaration, and rebase
  glozow:
    code review ACK a67983cd6d8e61565da4e03f3ba401d0148fe195

Tree-SHA512: dce182a18b88be80cbf50978d4ba8fa6ab0f01e861d09bae0ae9364051bb78f9334859d164b185b07f1d70a583e739557fab6d820cac8c37b3855b85c2a6771b

5 files changed, 54 insertions, 29 deletions

diff --git a/src/node/coin.cpp b/src/node/coin.cpp
index f4f86cdbe9..263dcff657 100644
--- a/src/node/coin.cpp
+++ b/src/node/coin.cpp
@@ -12,7 +12,8 @@ void FindCoins(const NodeContext& node, std::map<COutPoint, Coin>& coins)
 {
     assert(node.mempool);
     LOCK2(cs_main, node.mempool->cs);
-    CCoinsViewCache& chain_view = ::ChainstateActive().CoinsTip();
+    assert(std::addressof(::ChainstateActive()) == std::addressof(node.chainman->ActiveChainstate()));
+    CCoinsViewCache& chain_view = node.chainman->ActiveChainstate().CoinsTip();
     CCoinsViewMemPool mempool_view(&chain_view, *node.mempool);
     for (auto& coin : coins) {
         if (!mempool_view.GetCoin(coin.first, coin.second)) {
diff --git a/src/node/coinstats.cpp b/src/node/coinstats.cpp
index 06fcc33725..88bdba5953 100644
--- a/src/node/coinstats.cpp
+++ b/src/node/coinstats.cpp
@@ -83,7 +83,7 @@ static void ApplyStats(CCoinsStats& stats, T& hash_obj, const uint256& hash, con
 
 //! Calculate statistics about the unspent transaction output set
 template <typename T>
-static bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point)
+static bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& stats, T hash_obj, const std::function<void()>& interruption_point)
 {
     stats = CCoinsStats();
     std::unique_ptr<CCoinsViewCursor> pcursor(view->Cursor());
@@ -92,7 +92,8 @@ static bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const
     stats.hashBlock = pcursor->GetBestBlock();
     {
         LOCK(cs_main);
-        stats.nHeight = g_chainman.m_blockman.LookupBlockIndex(stats.hashBlock)->nHeight;
+        assert(std::addressof(g_chainman.m_blockman) == std::addressof(blockman));
+        stats.nHeight = blockman.LookupBlockIndex(stats.hashBlock)->nHeight;
     }
 
     PrepareHash(hash_obj, stats);
@@ -126,19 +127,19 @@ static bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, T hash_obj, const
     return true;
 }
 
-bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, CoinStatsHashType hash_type, const std::function<void()>& interruption_point)
+bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& stats, CoinStatsHashType hash_type, const std::function<void()>& interruption_point)
 {
     switch (hash_type) {
     case(CoinStatsHashType::HASH_SERIALIZED): {
         CHashWriter ss(SER_GETHASH, PROTOCOL_VERSION);
-        return GetUTXOStats(view, stats, ss, interruption_point);
+        return GetUTXOStats(view, blockman, stats, ss, interruption_point);
     }
     case(CoinStatsHashType::MUHASH): {
         MuHash3072 muhash;
-        return GetUTXOStats(view, stats, muhash, interruption_point);
+        return GetUTXOStats(view, blockman, stats, muhash, interruption_point);
     }
     case(CoinStatsHashType::NONE): {
-        return GetUTXOStats(view, stats, nullptr, interruption_point);
+        return GetUTXOStats(view, blockman, stats, nullptr, interruption_point);
     }
     } // no default case, so the compiler can warn about missing cases
     assert(false);
diff --git a/src/node/coinstats.h b/src/node/coinstats.h
index f02b95235f..83f228aa7e 100644
--- a/src/node/coinstats.h
+++ b/src/node/coinstats.h
@@ -8,6 +8,7 @@
 
 #include <amount.h>
 #include <uint256.h>
+#include <validation.h>
 
 #include <cstdint>
 #include <functional>
@@ -36,6 +37,6 @@ struct CCoinsStats
 };
 
 //! Calculate statistics about the unspent transaction output set
-bool GetUTXOStats(CCoinsView* view, CCoinsStats& stats, const CoinStatsHashType hash_type, const std::function<void()>& interruption_point = {});
+bool GetUTXOStats(CCoinsView* view, BlockManager& blockman, CCoinsStats& stats, const CoinStatsHashType hash_type, const std::function<void()>& interruption_point = {});
 
 #endif // BITCOIN_NODE_COINSTATS_H
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 62ef72ef38..9a6cee6433 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -182,18 +182,21 @@ public:
     int getNumBlocks() override
     {
         LOCK(::cs_main);
-        return ::ChainActive().Height();
+        assert(std::addressof(::ChainActive()) == std::addressof(m_context->chainman->ActiveChain()));
+        return m_context->chainman->ActiveChain().Height();
     }
     uint256 getBestBlockHash() override
     {
-        const CBlockIndex* tip = WITH_LOCK(::cs_main, return ::ChainActive().Tip());
+        assert(std::addressof(::ChainActive()) == std::addressof(m_context->chainman->ActiveChain()));
+        const CBlockIndex* tip = WITH_LOCK(::cs_main, return m_context->chainman->ActiveChain().Tip());
         return tip ? tip->GetBlockHash() : Params().GenesisBlock().GetHash();
     }
     int64_t getLastBlockTime() override
     {
         LOCK(::cs_main);
-        if (::ChainActive().Tip()) {
-            return ::ChainActive().Tip()->GetBlockTime();
+        assert(std::addressof(::ChainActive()) == std::addressof(m_context->chainman->ActiveChain()));
+        if (m_context->chainman->ActiveChain().Tip()) {
+            return m_context->chainman->ActiveChain().Tip()->GetBlockTime();
         }
         return Params().GenesisBlock().GetBlockTime(); // Genesis block's time of current network
     }
@@ -202,11 +205,15 @@ public:
         const CBlockIndex* tip;
         {
             LOCK(::cs_main);
-            tip = ::ChainActive().Tip();
+            assert(std::addressof(::ChainActive()) == std::addressof(m_context->chainman->ActiveChain()));
+            tip = m_context->chainman->ActiveChain().Tip();
         }
         return GuessVerificationProgress(Params().TxData(), tip);
     }
-    bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); }
+    bool isInitialBlockDownload() override {
+        assert(std::addressof(::ChainstateActive()) == std::addressof(m_context->chainman->ActiveChainstate()));
+        return m_context->chainman->ActiveChainstate().IsInitialBlockDownload();
+    }
     bool getReindex() override { return ::fReindex; }
     bool getImporting() override { return ::fImporting; }
     void setNetworkActive(bool active) override
@@ -231,7 +238,8 @@ public:
     bool getUnspentOutput(const COutPoint& output, Coin& coin) override
     {
         LOCK(::cs_main);
-        return ::ChainstateActive().CoinsTip().GetCoin(output, coin);
+        assert(std::addressof(::ChainstateActive()) == std::addressof(m_context->chainman->ActiveChainstate()));
+        return m_context->chainman->ActiveChainstate().CoinsTip().GetCoin(output, coin);
     }
     WalletClient& walletClient() override
     {
@@ -441,13 +449,15 @@ public:
     bool checkFinalTx(const CTransaction& tx) override
     {
         LOCK(cs_main);
-        return CheckFinalTx(::ChainActive().Tip(), tx);
+        assert(std::addressof(::ChainActive()) == std::addressof(m_node.chainman->ActiveChain()));
+        return CheckFinalTx(m_node.chainman->ActiveChain().Tip(), tx);
     }
     Optional<int> findLocatorFork(const CBlockLocator& locator) override
     {
         LOCK(cs_main);
         const CChain& active = Assert(m_node.chainman)->ActiveChain();
-        if (CBlockIndex* fork = g_chainman.m_blockman.FindForkInGlobalIndex(active, locator)) {
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        if (CBlockIndex* fork = m_node.chainman->m_blockman.FindForkInGlobalIndex(active, locator)) {
             return fork->nHeight;
         }
         return nullopt;
@@ -456,7 +466,8 @@ public:
     {
         WAIT_LOCK(cs_main, lock);
         const CChain& active = Assert(m_node.chainman)->ActiveChain();
-        return FillBlock(g_chainman.m_blockman.LookupBlockIndex(hash), block, lock, active);
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        return FillBlock(m_node.chainman->m_blockman.LookupBlockIndex(hash), block, lock, active);
     }
     bool findFirstBlockWithTimeAndHeight(int64_t min_time, int min_height, const FoundBlock& block) override
     {
@@ -468,7 +479,8 @@ public:
     {
         WAIT_LOCK(cs_main, lock);
         const CChain& active = Assert(m_node.chainman)->ActiveChain();
-        if (const CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash)) {
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        if (const CBlockIndex* block = m_node.chainman->m_blockman.LookupBlockIndex(block_hash)) {
             if (const CBlockIndex* ancestor = block->GetAncestor(ancestor_height)) {
                 return FillBlock(ancestor, ancestor_out, lock, active);
             }
@@ -479,8 +491,10 @@ public:
     {
         WAIT_LOCK(cs_main, lock);
         const CChain& active = Assert(m_node.chainman)->ActiveChain();
-        const CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash);
-        const CBlockIndex* ancestor = g_chainman.m_blockman.LookupBlockIndex(ancestor_hash);
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        const CBlockIndex* block = m_node.chainman->m_blockman.LookupBlockIndex(block_hash);
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        const CBlockIndex* ancestor = m_node.chainman->m_blockman.LookupBlockIndex(ancestor_hash);
         if (block && ancestor && block->GetAncestor(ancestor->nHeight) != ancestor) ancestor = nullptr;
         return FillBlock(ancestor, ancestor_out, lock, active);
     }
@@ -488,8 +502,10 @@ public:
     {
         WAIT_LOCK(cs_main, lock);
         const CChain& active = Assert(m_node.chainman)->ActiveChain();
-        const CBlockIndex* block1 = g_chainman.m_blockman.LookupBlockIndex(block_hash1);
-        const CBlockIndex* block2 = g_chainman.m_blockman.LookupBlockIndex(block_hash2);
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        const CBlockIndex* block1 = m_node.chainman->m_blockman.LookupBlockIndex(block_hash1);
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        const CBlockIndex* block2 = m_node.chainman->m_blockman.LookupBlockIndex(block_hash2);
         const CBlockIndex* ancestor = block1 && block2 ? LastCommonAncestor(block1, block2) : nullptr;
         // Using & instead of && below to avoid short circuiting and leaving
         // output uninitialized.
@@ -499,7 +515,8 @@ public:
     double guessVerificationProgress(const uint256& block_hash) override
     {
         LOCK(cs_main);
-        return GuessVerificationProgress(Params().TxData(), g_chainman.m_blockman.LookupBlockIndex(block_hash));
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        return GuessVerificationProgress(Params().TxData(), m_node.chainman->m_blockman.LookupBlockIndex(block_hash));
     }
     bool hasBlocks(const uint256& block_hash, int min_height, Optional<int> max_height) override
     {
@@ -511,7 +528,8 @@ public:
         // used to limit the range, and passing min_height that's too low or
         // max_height that's too high will not crash or change the result.
         LOCK(::cs_main);
-        if (CBlockIndex* block = g_chainman.m_blockman.LookupBlockIndex(block_hash)) {
+        assert(std::addressof(g_chainman) == std::addressof(*m_node.chainman));
+        if (CBlockIndex* block = m_node.chainman->m_blockman.LookupBlockIndex(block_hash)) {
             if (max_height && block->nHeight >= *max_height) block = block->GetAncestor(*max_height);
             for (; block->nStatus & BLOCK_HAVE_DATA; block = block->pprev) {
                 // Check pprev to not segfault if min_height is too low
@@ -601,7 +619,10 @@ public:
         return ::fHavePruned;
     }
     bool isReadyToBroadcast() override { return !::fImporting && !::fReindex && !isInitialBlockDownload(); }
-    bool isInitialBlockDownload() override { return ::ChainstateActive().IsInitialBlockDownload(); }
+    bool isInitialBlockDownload() override {
+        assert(std::addressof(::ChainstateActive()) == std::addressof(m_node.chainman->ActiveChainstate()));
+        return m_node.chainman->ActiveChainstate().IsInitialBlockDownload();
+    }
     bool shutdownRequested() override { return ShutdownRequested(); }
     int64_t getAdjustedTime() override { return GetAdjustedTime(); }
     void initMessage(const std::string& message) override { ::uiInterface.InitMessage(message); }
diff --git a/src/node/transaction.cpp b/src/node/transaction.cpp
index 3b3fab7b6b..97763f4fa2 100644
--- a/src/node/transaction.cpp
+++ b/src/node/transaction.cpp
@@ -39,9 +39,10 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
 
     { // cs_main scope
     LOCK(cs_main);
+    assert(std::addressof(::ChainstateActive()) == std::addressof(node.chainman->ActiveChainstate()));
     // If the transaction is already confirmed in the chain, don't do anything
     // and return early.
-    CCoinsViewCache &view = ::ChainstateActive().CoinsTip();
+    CCoinsViewCache &view = node.chainman->ActiveChainstate().CoinsTip();
     for (size_t o = 0; o < tx->vout.size(); o++) {
         const Coin& existingCoin = view.AccessCoin(COutPoint(hashTx, o));
         // IsSpent doesn't mean the coin is spent, it means the output doesn't exist.
@@ -53,7 +54,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
         if (max_tx_fee > 0) {
             // First, call ATMP with test_accept and check the fee. If ATMP
             // fails here, return error immediately.
-            const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *node.mempool, tx, false /* bypass_limits */,
+            const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, false /* bypass_limits */,
                                                                   true /* test_accept */);
             if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
                 return HandleATMPError(result.m_state, err_string);
@@ -62,7 +63,7 @@ TransactionError BroadcastTransaction(NodeContext& node, const CTransactionRef t
             }
         }
         // Try to submit the transaction to the mempool.
-        const MempoolAcceptResult result = AcceptToMemoryPool(::ChainstateActive(), *node.mempool, tx, false /* bypass_limits */,
+        const MempoolAcceptResult result = AcceptToMemoryPool(node.chainman->ActiveChainstate(), *node.mempool, tx, false /* bypass_limits */,
                                                               false /* test_accept */);
         if (result.m_result_type != MempoolAcceptResult::ResultType::VALID) {
             return HandleATMPError(result.m_state, err_string);