From 362ded410b8cb1104b7ef31ff8488fec4824a7d5 Mon Sep 17 00:00:00 2001
From: Russell Yanofsky <russ@yanofsky.org>
Date: Tue, 17 Sep 2019 19:05:26 -0400
Subject: Avoid using g_rpc_node global in wallet code

Wallet code should use interfaces::Chain and not directly access to node state.

Add a g_rpc_chain replacement global for wallet code to use, and move
g_rpc_node definition to a libbitcoin_server source file so there are link
errors if wallet code tries to access it.
---
 src/interfaces/wallet.cpp  | 6 +++++-
 src/rpc/blockchain.cpp     | 2 ++
 src/rpc/blockchain.h       | 6 ++++++
 src/rpc/net.cpp            | 1 +
 src/rpc/rawtransaction.cpp | 2 +-
 src/rpc/util.cpp           | 2 --
 src/rpc/util.h             | 6 ------
 src/test/setup_common.cpp  | 3 +++
 src/wallet/rpcwallet.cpp   | 6 ++++--
 src/wallet/rpcwallet.h     | 6 ++++++
 10 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp
index 9b0a8b64c9..bdfb17801d 100644
--- a/src/interfaces/wallet.cpp
+++ b/src/interfaces/wallet.cpp
@@ -489,7 +489,11 @@ public:
         : m_chain(chain), m_wallet_filenames(std::move(wallet_filenames))
     {
     }
-    void registerRpcs() override { return RegisterWalletRPCCommands(m_chain, m_rpc_handlers); }
+    void registerRpcs() override
+    {
+        g_rpc_chain = &m_chain;
+        return RegisterWalletRPCCommands(m_chain, m_rpc_handlers);
+    }
     bool verify() override { return VerifyWallets(m_chain, m_wallet_filenames); }
     bool load() override { return LoadWallets(m_chain, m_wallet_filenames); }
     void start(CScheduler& scheduler) override { return StartWallets(scheduler); }
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/net.cpp b/src/rpc/net.cpp
index ebc3947856..f443f37c6d 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -13,6 +13,7 @@
 #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>
diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp
index 9b71b70b22..5ce23226b7 100644
--- a/src/rpc/rawtransaction.cpp
+++ b/src/rpc/rawtransaction.cpp
@@ -10,7 +10,6 @@
 #include <index/txindex.h>
 #include <key_io.h>
 #include <merkleblock.h>
-#include <net.h>
 #include <node/coin.h>
 #include <node/context.h>
 #include <node/psbt.h>
@@ -20,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>
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp
index 26252e2b8a..653b287e97 100644
--- a/src/rpc/util.cpp
+++ b/src/rpc/util.cpp
@@ -13,8 +13,6 @@
 
 #include <tuple>
 
-NodeContext* g_rpc_node = 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 872ef5ecaa..221638aa9e 100644
--- a/src/rpc/util.h
+++ b/src/rpc/util.h
@@ -25,12 +25,6 @@
 class FillableSigningProvider;
 class CPubKey;
 class CScript;
-struct NodeContext;
-
-//! 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 NodeContext* g_rpc_node;
 
 /** Wrapper for UniValue::VType, which includes typeAny:
  * Used to denote don't care type. */
diff --git a/src/test/setup_common.cpp b/src/test/setup_common.cpp
index 4efd3d42c4..89a19e172d 100644
--- a/src/test/setup_common.cpp
+++ b/src/test/setup_common.cpp
@@ -15,6 +15,7 @@
 #include <net.h>
 #include <noui.h>
 #include <pow.h>
+#include <rpc/blockchain.h>
 #include <rpc/register.h>
 #include <rpc/server.h>
 #include <script/sigcache.h>
@@ -76,6 +77,7 @@ TestingSetup::TestingSetup(const std::string& chainName) : BasicTestingSetup(cha
     const CChainParams& chainparams = Params();
     // Ideally we'd move all the RPC tests to the functional testing framework
     // instead of unit tests, but for now we need these here.
+    g_rpc_node = &m_node;
     RegisterAllCoreRPCCommands(tableRPC);
 
     // We have to run a scheduler thread to prevent ActivateBestChain
@@ -114,6 +116,7 @@ TestingSetup::~TestingSetup()
     threadGroup.join_all();
     GetMainSignals().FlushBackgroundCallbacks();
     GetMainSignals().UnregisterBackgroundSignalScheduler();
+    g_rpc_node = nullptr;
     m_node.connman.reset();
     m_node.banman.reset();
     UnloadBlockIndex();
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 45c6e340c0..9050553a0e 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2574,7 +2574,7 @@ static UniValue loadwallet(const JSONRPCRequest& request)
 
     std::string error;
     std::vector<std::string> warning;
-    std::shared_ptr<CWallet> const wallet = LoadWallet(*g_rpc_node->chain, location, error, warning);
+    std::shared_ptr<CWallet> const wallet = LoadWallet(*g_rpc_chain, location, error, warning);
     if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error);
 
     UniValue obj(UniValue::VOBJ);
@@ -2700,7 +2700,7 @@ static UniValue createwallet(const JSONRPCRequest& request)
 
     std::string error;
     std::shared_ptr<CWallet> wallet;
-    WalletCreationStatus status = CreateWallet(*g_rpc_node->chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
+    WalletCreationStatus status = CreateWallet(*g_rpc_chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
     switch (status) {
         case WalletCreationStatus::CREATION_FAILED:
             throw JSONRPCError(RPC_WALLET_ERROR, error);
@@ -4231,3 +4231,5 @@ void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique
     for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
         handlers.emplace_back(chain.handleRpc(commands[vcidx]));
 }
+
+interfaces::Chain* g_rpc_chain = nullptr;
diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h
index 1c0523c90b..31d3f7a5f9 100644
--- a/src/wallet/rpcwallet.h
+++ b/src/wallet/rpcwallet.h
@@ -21,6 +21,12 @@ class Chain;
 class Handler;
 }
 
+//! Pointer to chain interface that needs to be declared as a global to be
+//! accessible loadwallet and createwallet methods. Due to limitations of the
+//! RPC framework, there's currently no direct way to pass in state to RPC
+//! methods without globals.
+extern interfaces::Chain* g_rpc_chain;
+
 void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers);
 
 /**
-- 
cgit v1.2.3