Merge #16839: Replace Connman and BanMan globals with NodeContext local

362ded410b8cb1104b7ef31ff8488fec4824a7d5 Avoid using g_rpc_node global in wallet code (Russell Yanofsky)
8922d7f6b751a3e6b3b9f6fb7961c442877fb65a scripted-diff: Remove g_connman, g_banman globals (Russell Yanofsky)
e6f4f895d5e42feaf7bfa5f41e80292aaa73cd7d Pass NodeContext, ConnMan, BanMan references more places (Russell Yanofsky)
4d5448c76b71c9d91399c31b043237091be2e5e7 MOVEONLY: Move NodeContext struct to node/context.h (Russell Yanofsky)
301bd41a2e6765b185bd55f4c541f9e27aeea29d scripted-diff: Rename InitInterfaces to NodeContext (Russell Yanofsky)

Pull request description:

  This change is mainly a naming / organization change intended to simplify #10102. It:

  - Renames struct InitInterfaces to struct NodeContext and moves it from
    src/init.h to src/node/context.h. This is a cosmetic change intended to make
    the point of the struct more obvious.

  - Gets rid of BanMan and ConnMan globals making them NodeContext members
    instead. Getting rid of these globals has been talked about in past as a way
    to implement testing and simulations. Making them NodeContext members is a
    way of keeping them accessible without the globals.

  - Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This
    better separates node and wallet rpc methods. Node RPC methods should have
    access NodeContext, while wallet RPC methods should only have indirect access
    to node functionality via interfaces::Chain.

  - Adds NodeContext& references to interfaces::Chain class and the
    interfaces::MakeChain() function. This is needed to access ConnMan and BanMan
    instances without the globals.

  - Gets rid of redundant Node and Chain instances in Qt tests. This is
    needed due to the previous MakeChain change, and also makes test setup a
    little more straightforward. More cleanup could be done in the future, but it
    will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init
    code.

ACKs for top commit:
  laanwj:
    ACK 362ded410b8cb1104b7ef31ff8488fec4824a7d5

Tree-SHA512: 9ae6ff1e33423291d1e52056bac95e0874538390892a6e83c4c115b3c73155a8827c0191b46eb3d14e3b3f6c23ccb08095490880fbc3188026319c71739f7db2

7 files changed, 63 insertions, 58 deletions

diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index fac6bcd60d..747113fbb4 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -2289,3 +2289,5 @@ void RegisterBlockchainRPCCommands(CRPCTable &t)
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         t.appendCommand(commands[vcidx].name, &commands[vcidx]);
 }
+
+NodeContext* g_rpc_node = nullptr;
diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h
index ff461fbcbc..8a1264f824 100644
--- a/src/rpc/blockchain.h
+++ b/src/rpc/blockchain.h
@@ -17,6 +17,7 @@ class CBlock;
 class CBlockIndex;
 class CTxMemPool;
 class UniValue;
+struct NodeContext;
 
 static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5;
 
@@ -46,4 +47,9 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex
 /** Used by getblockstats to get feerates at different percentiles by weight  */
 void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_weight);
 
+//! Pointer to node state that needs to be declared as a global to be accessible
+//! RPC methods. Due to limitations of the RPC framework, there's currently no
+//! direct way to pass in state to RPC methods without globals.
+extern NodeContext* g_rpc_node;
+
 #endif
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 07c2958635..bfa3e35d96 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -13,6 +13,7 @@
 #include <key_io.h>
 #include <miner.h>
 #include <net.h>
+#include <node/context.h>
 #include <policy/fees.h>
 #include <pow.h>
 #include <rpc/blockchain.h>
@@ -424,10 +425,10 @@ static UniValue getblocktemplate(const JSONRPCRequest& request)
     if (strMode != "template")
         throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid mode");
 
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
-    if (g_connman->GetNodeCount(CConnman::CONNECTIONS_ALL) == 0)
+    if (g_rpc_node->connman->GetNodeCount(CConnman::CONNECTIONS_ALL) == 0)
         throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, PACKAGE_NAME " is not connected!");
 
     if (::ChainstateActive().IsInitialBlockDownload())
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index 7b1507e4dc..f443f37c6d 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -11,7 +11,9 @@
 #include <net_processing.h>
 #include <net_permissions.h>
 #include <netbase.h>
+#include <node/context.h>
 #include <policy/settings.h>
+#include <rpc/blockchain.h>
 #include <rpc/protocol.h>
 #include <rpc/util.h>
 #include <sync.h>
@@ -38,10 +40,10 @@ static UniValue getconnectioncount(const JSONRPCRequest& request)
                 },
             }.Check(request);
 
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
-    return (int)g_connman->GetNodeCount(CConnman::CONNECTIONS_ALL);
+    return (int)g_rpc_node->connman->GetNodeCount(CConnman::CONNECTIONS_ALL);
 }
 
 static UniValue ping(const JSONRPCRequest& request)
@@ -58,11 +60,11 @@ static UniValue ping(const JSONRPCRequest& request)
                 },
             }.Check(request);
 
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
     // Request that each node send a ping during next message processing pass
-    g_connman->ForEachNode([](CNode* pnode) {
+    g_rpc_node->connman->ForEachNode([](CNode* pnode) {
         pnode->fPingQueued = true;
     });
     return NullUniValue;
@@ -131,11 +133,11 @@ static UniValue getpeerinfo(const JSONRPCRequest& request)
                 },
             }.Check(request);
 
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
     std::vector<CNodeStats> vstats;
-    g_connman->GetNodeStats(vstats);
+    g_rpc_node->connman->GetNodeStats(vstats);
 
     UniValue ret(UniValue::VARR);
 
@@ -234,7 +236,7 @@ static UniValue addnode(const JSONRPCRequest& request)
                 },
             }.ToString());
 
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
     std::string strNode = request.params[0].get_str();
@@ -242,18 +244,18 @@ static UniValue addnode(const JSONRPCRequest& request)
     if (strCommand == "onetry")
     {
         CAddress addr;
-        g_connman->OpenNetworkConnection(addr, false, nullptr, strNode.c_str(), false, false, true);
+        g_rpc_node->connman->OpenNetworkConnection(addr, false, nullptr, strNode.c_str(), false, false, true);
         return NullUniValue;
     }
 
     if (strCommand == "add")
     {
-        if(!g_connman->AddNode(strNode))
+        if(!g_rpc_node->connman->AddNode(strNode))
             throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: Node already added");
     }
     else if(strCommand == "remove")
     {
-        if(!g_connman->RemoveAddedNode(strNode))
+        if(!g_rpc_node->connman->RemoveAddedNode(strNode))
             throw JSONRPCError(RPC_CLIENT_NODE_NOT_ADDED, "Error: Node has not been added.");
     }
 
@@ -279,7 +281,7 @@ static UniValue disconnectnode(const JSONRPCRequest& request)
                 },
             }.Check(request);
 
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
     bool success;
@@ -288,11 +290,11 @@ static UniValue disconnectnode(const JSONRPCRequest& request)
 
     if (!address_arg.isNull() && id_arg.isNull()) {
         /* handle disconnect-by-address */
-        success = g_connman->DisconnectNode(address_arg.get_str());
+        success = g_rpc_node->connman->DisconnectNode(address_arg.get_str());
     } else if (!id_arg.isNull() && (address_arg.isNull() || (address_arg.isStr() && address_arg.get_str().empty()))) {
         /* handle disconnect-by-id */
         NodeId nodeid = (NodeId) id_arg.get_int64();
-        success = g_connman->DisconnectNode(nodeid);
+        success = g_rpc_node->connman->DisconnectNode(nodeid);
     } else {
         throw JSONRPCError(RPC_INVALID_PARAMS, "Only one of address and nodeid should be provided.");
     }
@@ -333,10 +335,10 @@ static UniValue getaddednodeinfo(const JSONRPCRequest& request)
                 },
             }.Check(request);
 
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
-    std::vector<AddedNodeInfo> vInfo = g_connman->GetAddedNodeInfo();
+    std::vector<AddedNodeInfo> vInfo = g_rpc_node->connman->GetAddedNodeInfo();
 
     if (!request.params[0].isNull()) {
         bool found = false;
@@ -399,21 +401,21 @@ static UniValue getnettotals(const JSONRPCRequest& request)
             + HelpExampleRpc("getnettotals", "")
                 },
             }.Check(request);
-    if(!g_connman)
+    if(!g_rpc_node->connman)
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
 
     UniValue obj(UniValue::VOBJ);
-    obj.pushKV("totalbytesrecv", g_connman->GetTotalBytesRecv());
-    obj.pushKV("totalbytessent", g_connman->GetTotalBytesSent());
+    obj.pushKV("totalbytesrecv", g_rpc_node->connman->GetTotalBytesRecv());
+    obj.pushKV("totalbytessent", g_rpc_node->connman->GetTotalBytesSent());
     obj.pushKV("timemillis", GetTimeMillis());
 
     UniValue outboundLimit(UniValue::VOBJ);
-    outboundLimit.pushKV("timeframe", g_connman->GetMaxOutboundTimeframe());
-    outboundLimit.pushKV("target", g_connman->GetMaxOutboundTarget());
-    outboundLimit.pushKV("target_reached", g_connman->OutboundTargetReached(false));
-    outboundLimit.pushKV("serve_historical_blocks", !g_connman->OutboundTargetReached(true));
-    outboundLimit.pushKV("bytes_left_in_cycle", g_connman->GetOutboundTargetBytesLeft());
-    outboundLimit.pushKV("time_left_in_cycle", g_connman->GetMaxOutboundTimeLeftInCycle());
+    outboundLimit.pushKV("timeframe", g_rpc_node->connman->GetMaxOutboundTimeframe());
+    outboundLimit.pushKV("target", g_rpc_node->connman->GetMaxOutboundTarget());
+    outboundLimit.pushKV("target_reached", g_rpc_node->connman->OutboundTargetReached(false));
+    outboundLimit.pushKV("serve_historical_blocks", !g_rpc_node->connman->OutboundTargetReached(true));
+    outboundLimit.pushKV("bytes_left_in_cycle", g_rpc_node->connman->GetOutboundTargetBytesLeft());
+    outboundLimit.pushKV("time_left_in_cycle", g_rpc_node->connman->GetMaxOutboundTimeLeftInCycle());
     obj.pushKV("uploadtarget", outboundLimit);
     return obj;
 }
@@ -492,16 +494,16 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
     obj.pushKV("version",       CLIENT_VERSION);
     obj.pushKV("subversion",    strSubVersion);
     obj.pushKV("protocolversion",PROTOCOL_VERSION);
-    if (g_connman) {
-        ServiceFlags services = g_connman->GetLocalServices();
+    if (g_rpc_node->connman) {
+        ServiceFlags services = g_rpc_node->connman->GetLocalServices();
         obj.pushKV("localservices", strprintf("%016x", services));
         obj.pushKV("localservicesnames", GetServicesNames(services));
     }
     obj.pushKV("localrelay", g_relay_txes);
     obj.pushKV("timeoffset",    GetTimeOffset());
-    if (g_connman) {
-        obj.pushKV("networkactive", g_connman->GetNetworkActive());
-        obj.pushKV("connections",   (int)g_connman->GetNodeCount(CConnman::CONNECTIONS_ALL));
+    if (g_rpc_node->connman) {
+        obj.pushKV("networkactive", g_rpc_node->connman->GetNetworkActive());
+        obj.pushKV("connections",   (int)g_rpc_node->connman->GetNodeCount(CConnman::CONNECTIONS_ALL));
     }
     obj.pushKV("networks",      GetNetworksInfo());
     obj.pushKV("relayfee",      ValueFromAmount(::minRelayTxFee.GetFeePerK()));
@@ -546,7 +548,7 @@ static UniValue setban(const JSONRPCRequest& request)
     if (request.fHelp || !help.IsValidNumArgs(request.params.size()) || (strCommand != "add" && strCommand != "remove")) {
         throw std::runtime_error(help.ToString());
     }
-    if (!g_banman) {
+    if (!g_rpc_node->banman) {
         throw JSONRPCError(RPC_DATABASE_ERROR, "Error: Ban database not loaded");
     }
 
@@ -570,7 +572,7 @@ static UniValue setban(const JSONRPCRequest& request)
 
     if (strCommand == "add")
     {
-        if (isSubnet ? g_banman->IsBanned(subNet) : g_banman->IsBanned(netAddr)) {
+        if (isSubnet ? g_rpc_node->banman->IsBanned(subNet) : g_rpc_node->banman->IsBanned(netAddr)) {
             throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: IP/Subnet already banned");
         }
 
@@ -583,20 +585,20 @@ static UniValue setban(const JSONRPCRequest& request)
             absolute = true;
 
         if (isSubnet) {
-            g_banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute);
-            if (g_connman) {
-                g_connman->DisconnectNode(subNet);
+            g_rpc_node->banman->Ban(subNet, BanReasonManuallyAdded, banTime, absolute);
+            if (g_rpc_node->connman) {
+                g_rpc_node->connman->DisconnectNode(subNet);
             }
         } else {
-            g_banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute);
-            if (g_connman) {
-                g_connman->DisconnectNode(netAddr);
+            g_rpc_node->banman->Ban(netAddr, BanReasonManuallyAdded, banTime, absolute);
+            if (g_rpc_node->connman) {
+                g_rpc_node->connman->DisconnectNode(netAddr);
             }
         }
     }
     else if(strCommand == "remove")
     {
-        if (!( isSubnet ? g_banman->Unban(subNet) : g_banman->Unban(netAddr) )) {
+        if (!( isSubnet ? g_rpc_node->banman->Unban(subNet) : g_rpc_node->banman->Unban(netAddr) )) {
             throw JSONRPCError(RPC_CLIENT_INVALID_IP_OR_SUBNET, "Error: Unban failed. Requested address/subnet was not previously banned.");
         }
     }
@@ -615,12 +617,12 @@ static UniValue listbanned(const JSONRPCRequest& request)
                 },
             }.Check(request);
 
-    if(!g_banman) {
+    if(!g_rpc_node->banman) {
         throw JSONRPCError(RPC_DATABASE_ERROR, "Error: Ban database not loaded");
     }
 
     banmap_t banMap;
-    g_banman->GetBanned(banMap);
+    g_rpc_node->banman->GetBanned(banMap);
 
     UniValue bannedAddresses(UniValue::VARR);
     for (const auto& entry : banMap)
@@ -649,11 +651,11 @@ static UniValue clearbanned(const JSONRPCRequest& request)
                             + HelpExampleRpc("clearbanned", "")
                 },
             }.Check(request);
-    if (!g_banman) {
+    if (!g_rpc_node->banman) {
         throw JSONRPCError(RPC_DATABASE_ERROR, "Error: Ban database not loaded");
     }
 
-    g_banman->ClearBanned();
+    g_rpc_node->banman->ClearBanned();
 
     return NullUniValue;
 }
@@ -669,13 +671,13 @@ static UniValue setnetworkactive(const JSONRPCRequest& request)
                 RPCExamples{""},
             }.Check(request);
 
-    if (!g_connman) {
+    if (!g_rpc_node->connman) {
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
     }
 
-    g_connman->SetNetworkActive(request.params[0].get_bool());
+    g_rpc_node->connman->SetNetworkActive(request.params[0].get_bool());
 
-    return g_connman->GetNetworkActive();
+    return g_rpc_node->connman->GetNetworkActive();
 }
 
 static UniValue getnodeaddresses(const JSONRPCRequest& request)
@@ -701,7 +703,7 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request)
             + HelpExampleRpc("getnodeaddresses", "8")
                 },
             }.Check(request);
-    if (!g_connman) {
+    if (!g_rpc_node->connman) {
         throw JSONRPCError(RPC_CLIENT_P2P_DISABLED, "Error: Peer-to-peer functionality missing or disabled");
     }
 
@@ -713,7 +715,7 @@ static UniValue getnodeaddresses(const JSONRPCRequest& request)
         }
     }
     // returns a shuffled list of CAddress
-    std::vector<CAddress> vAddr = g_connman->GetAddresses();
+    std::vector<CAddress> vAddr = g_rpc_node->connman->GetAddresses();
     UniValue ret(UniValue::VARR);
 
     int address_return_count = std::min<int>(count, vAddr.size());
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 487f74c3e1..74cd46080b 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -11,6 +11,7 @@
 #include <key_io.h>
 #include <merkleblock.h>
 #include <node/coin.h>
+#include <node/context.h>
 #include <node/psbt.h>
 #include <node/transaction.h>
 #include <policy/policy.h>
@@ -18,6 +19,7 @@
 #include <primitives/transaction.h>
 #include <psbt.h>
 #include <random.h>
+#include <rpc/blockchain.h>
 #include <rpc/rawtransaction_util.h>
 #include <rpc/server.h>
 #include <rpc/util.h>
@@ -817,7 +819,7 @@ static UniValue sendrawtransaction(const JSONRPCRequest& request)
 
     std::string err_string;
     AssertLockNotHeld(cs_main);
-    const TransactionError err = BroadcastTransaction(tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true);
+    const TransactionError err = BroadcastTransaction(*g_rpc_node, tx, err_string, max_raw_tx_fee, /*relay*/ true, /*wait_callback*/ true);
     if (TransactionError::OK != err) {
         throw JSONRPCTransactionError(err, err_string);
     }
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index adda90c104..653b287e97 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -13,8 +13,6 @@
 
 #include <tuple>
 
-InitInterfaces* g_rpc_interfaces = nullptr;
-
 void RPCTypeCheck(const UniValue& params,
                   const std::list<UniValueType>& typesExpected,
                   bool fAllowNull)
diff --git a/src/rpc/util.h b/src/rpc/util.h
index ec36956c95..221638aa9e 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -25,12 +25,6 @@
 class FillableSigningProvider;
 class CPubKey;
 class CScript;
-struct InitInterfaces;
-
-//! Pointers to interfaces that need to be accessible from RPC methods. Due to
-//! limitations of the RPC framework, there's currently no direct way to pass in
-//! state to RPC method implementations.
-extern InitInterfaces* g_rpc_interfaces;
 
 /** Wrapper for UniValue::VType, which includes typeAny:
  * Used to denote don't care type. */