node: Add reference to mempool in NodeContext

Currently it is an alias to the global ::mempool and should be used as
follows.

* Node code (validation and transaction relay) can use either ::mempool
  or node.mempool, whichever seems a better fit.
* RPC code should use the added convenience getter EnsureMempool, which
  makes sure the mempool exists before use. This prepares the RPC code
  to a future where the mempool might be disabled at runtime or compile
  time.
* Test code should use m_node.mempool directly, as the mempool is always
  initialized for tests.

Author: MarcoFalke

src/init.cpp

@@ -284,6 +284,7 @@ void Shutdown(NodeContext& node)
     GetMainSignals().UnregisterWithMempoolSignals(mempool);
     globalVerifyHandle.reset();
     ECC_Stop();
+    if (node.mempool) node.mempool = nullptr;
     LogPrintf("%s: done\n", __func__);
 }
 
@@ -1632,6 +1633,11 @@ bool AppInitMain(NodeContext& node)
         return false;
     }
 
+    // Now that the chain state is loaded, make mempool generally available in the node context. For example the
+    // connection manager, wallet, or RPC threads, which are all started after this, may use it from the node context.
+    assert(!node.mempool);
+    node.mempool = &::mempool;
+
     fs::path est_path = GetDataDir() / FEE_ESTIMATES_FILENAME;
     CAutoFile est_filein(fsbridge::fopen(est_path, "rb"), SER_DISK, CLIENT_VERSION);
     // Allowed to fail as this file IS missing on first startup.

src/node/context.h

@@ -10,6 +10,7 @@
 
 class BanMan;
 class CConnman;
+class CTxMemPool;
 class PeerLogicValidation;
 namespace interfaces {
 class Chain;
@@ -22,13 +23,13 @@ class ChainClient;
 //! This is used by init, rpc, and test code to pass object references around
 //! without needing to declare the same variables and parameters repeatedly, or
 //! to use globals. More variables could be added to this struct (particularly
-//! references to validation and mempool objects) to eliminate use of globals
+//! references to validation objects) to eliminate use of globals
 //! and make code more modular and testable. The struct isn't intended to have
 //! any member functions. It should just be a collection of references that can
 //! be used without pulling in unwanted dependencies or functionality.
-struct NodeContext
-{
+struct NodeContext {
     std::unique_ptr<CConnman> connman;
+    CTxMemPool* mempool{nullptr}; // Currently a raw pointer because the memory is not managed by this struct
     std::unique_ptr<PeerLogicValidation> peer_logic;
     std::unique_ptr<BanMan> banman;
     std::unique_ptr<interfaces::Chain> chain;

src/qt/test/rpcnestedtests.cpp

@@ -32,7 +32,6 @@ void RPCNestedTests::rpcNestedTests()
     // do some test setup
     // could be moved to a more generic place when we add more tests on QT level
     tableRPC.appendCommand("rpcNestedTest", &vRPCCommands[0]);
-    //mempool.setSanityCheck(1.0);
 
     TestingSetup test;
 

src/rpc/blockchain.cpp

@@ -15,6 +15,7 @@
 #include <hash.h>
 #include <index/blockfilterindex.h>
 #include <node/coinstats.h>
+#include <node/context.h>
 #include <node/utxo_snapshot.h>
 #include <policy/feerate.h>
 #include <policy/policy.h>
@@ -53,6 +54,15 @@ static Mutex cs_blockchange;
 static std::condition_variable cond_blockchange;
 static CUpdatedBlock latestblock;
 
+CTxMemPool& EnsureMemPool()
+{
+    CHECK_NONFATAL(g_rpc_node);
+    if (!g_rpc_node->mempool) {
+        throw JSONRPCError(RPC_CLIENT_MEMPOOL_DISABLED, "Mempool disabled or instance not found");
+    }
+    return *g_rpc_node->mempool;
+}
+
 /* Calculate the difficulty for a given block index.
  */
 double GetDifficulty(const CBlockIndex* blockindex)

src/rpc/blockchain.h

@@ -52,4 +52,6 @@ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES],
 //! direct way to pass in state to RPC methods without globals.
 extern NodeContext* g_rpc_node;
 
+CTxMemPool& EnsureMemPool();
+
 #endif

src/rpc/protocol.h

@@ -63,6 +63,9 @@ enum RPCErrorCode
     RPC_CLIENT_INVALID_IP_OR_SUBNET = -30, //!< Invalid IP/Subnet
     RPC_CLIENT_P2P_DISABLED         = -31, //!< No valid connection manager instance found
 
+    //! Chain errors
+    RPC_CLIENT_MEMPOOL_DISABLED     = -33, //!< No mempool instance found
+
     //! Wallet errors
     RPC_WALLET_ERROR                = -4,  //!< Unspecified problem with wallet (key not found etc.)
     RPC_WALLET_INSUFFICIENT_FUNDS   = -6,  //!< Not enough funds in wallet or account

src/test/util/setup_common.cpp

@@ -107,7 +107,6 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
     threadGroup.create_thread(std::bind(&CScheduler::serviceQueue, &scheduler));
     GetMainSignals().RegisterBackgroundSignalScheduler(scheduler);
 
-    mempool.setSanityCheck(1.0);
     pblocktree.reset(new CBlockTreeDB(1 << 20, true));
     g_chainstate = MakeUnique<CChainState>();
     ::ChainstateActive().InitCoinsDB(
@@ -131,6 +130,8 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
     }
     g_parallel_script_checks = true;
 
+    m_node.mempool = &::mempool;
+    m_node.mempool->setSanityCheck(1.0);
     m_node.banman = MakeUnique<BanMan>(GetDataDir() / "banlist.dat", nullptr, DEFAULT_MISBEHAVING_BANTIME);
     m_node.connman = MakeUnique<CConnman>(0x1337, 0x1337); // Deterministic randomness for tests.
 }
@@ -144,6 +145,7 @@ TestingSetup::~TestingSetup()
     g_rpc_node = nullptr;
     m_node.connman.reset();
     m_node.banman.reset();
+    m_node.mempool = nullptr;
     UnloadBlockIndex();
     g_chainstate.reset();
     pblocktree.reset();