merge bitcoin#19096: Remove g_rpc_chain global

Author: kwvg

src/Makefile.am

@@ -317,6 +317,7 @@ BITCOIN_CORE_H = \
   walletinitinterface.h \
   wallet/bdb.h \
   wallet/coincontrol.h \
+  wallet/context.h \
   wallet/crypter.h \
   wallet/db.h \
   wallet/fees.h \
@@ -480,6 +481,7 @@ libdash_wallet_a_SOURCES = \
   interfaces/wallet.cpp \
   wallet/bdb.cpp \
   wallet/coincontrol.cpp \
+  wallet/context.cpp \
   wallet/crypter.cpp \
   wallet/db.cpp \
   wallet/fees.cpp \

src/interfaces/wallet.cpp

@@ -10,13 +10,16 @@
 #include <interfaces/handler.h>
 #include <policy/fees.h>
 #include <primitives/transaction.h>
+#include <rpc/server.h>
 #include <script/standard.h>
 #include <support/allocators/secure.h>
 #include <sync.h>
 #include <ui_interface.h>
 #include <uint256.h>
 #include <util/system.h>
+#include <util/ref.h>
 #include <validation.h>
+#include <wallet/context.h>
 #include <wallet/fees.h>
 #include <wallet/ismine.h>
 #include <wallet/rpcwallet.h>
@@ -572,16 +575,21 @@ class WalletClientImpl : public ChainClient
 {
 public:
     WalletClientImpl(Chain& chain, std::vector<std::string> wallet_filenames)
-        : m_chain(chain), m_wallet_filenames(std::move(wallet_filenames))
+        : m_wallet_filenames(std::move(wallet_filenames))
     {
+        m_context.chain = &chain;
     }
     void registerRpcs() override
     {
-        g_rpc_chain = &m_chain;
-        return RegisterWalletRPCCommands(m_chain, m_rpc_handlers);
+        for (const CRPCCommand& command : GetWalletRPCCommands()) {
+            m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) {
+                return command.actor({request, m_context}, result, last_handler);
+            }, command.argNames, command.unique_id);
+            m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
+        }
     }
-    bool verify() override { return VerifyWallets(m_chain, m_wallet_filenames); }
-    bool load() override { return LoadWallets(m_chain, m_wallet_filenames); }
+    bool verify() override { return VerifyWallets(*m_context.chain, m_wallet_filenames); }
+    bool load() override { return LoadWallets(*m_context.chain, m_wallet_filenames); }
     void start(CScheduler& scheduler) override { return StartWallets(scheduler); }
     void flush() override { return FlushWallets(); }
     void stop() override { return StopWallets(); }
@@ -596,9 +604,10 @@ class WalletClientImpl : public ChainClient
     }
     ~WalletClientImpl() override { UnloadWallets(); }
 
-    Chain& m_chain;
+    WalletContext m_context;
     std::vector<std::string> m_wallet_filenames;
     std::vector<std::unique_ptr<Handler>> m_rpc_handlers;
+    std::list<CRPCCommand> m_rpc_commands;
 };
 
 } // namespace

src/rpc/request.h

@@ -41,6 +41,16 @@ class JSONRPCRequest
     const util::Ref& context;
 
     JSONRPCRequest(const util::Ref& context) : id(NullUniValue), params(NullUniValue), fHelp(false), context(context) {}
+
+    //! Initializes request information from another request object and the
+    //! given context. The implementation should be updated if any members are
+    //! added or removed above.
+    JSONRPCRequest(const JSONRPCRequest& other, const util::Ref& context)
+        : id(other.id), strMethod(other.strMethod), params(other.params), fHelp(other.fHelp), URI(other.URI),
+          authUser(other.authUser), peerAddr(other.peerAddr), context(context)
+    {
+    }
+
     void parse(const UniValue& valRequest);
 };
 

src/wallet/context.cpp

@@ -0,0 +1,8 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <wallet/context.h>
+
+WalletContext::WalletContext() {}
+WalletContext::~WalletContext() {}

src/wallet/context.h

@@ -0,0 +1,32 @@
+// Copyright (c) 2020 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_WALLET_CONTEXT_H
+#define BITCOIN_WALLET_CONTEXT_H
+
+namespace interfaces {
+class Chain;
+} // namespace interfaces
+
+//! WalletContext struct containing references to state shared between CWallet
+//! instances, like the reference to the chain interface, and the list of opened
+//! wallets.
+//!
+//! Future shared state can be added here as an alternative to adding global
+//! variables.
+//!
+//! The struct isn't intended to have any member functions. It should just be a
+//! collection of state pointers that doesn't pull in dependencies or implement
+//! behavior.
+struct WalletContext {
+    interfaces::Chain* chain{nullptr};
+
+    //! Declare default constructor and destructor that are not inline, so code
+    //! instantiating the WalletContext struct doesn't need to #include class
+    //! definitions for smart pointer and container members.
+    WalletContext();
+    ~WalletContext();
+};
+
+#endif // BITCOIN_WALLET_CONTEXT_H

src/wallet/rpcwallet.cpp

@@ -21,13 +21,15 @@
 #include <util/fees.h>
 #include <util/system.h>
 #include <util/moneystr.h>
+#include <util/ref.h>
 #include <util/string.h>
 #include <util/translation.h>
 #include <util/url.h>
 #include <util/validation.h>
 #include <util/vector.h>
 #include <validation.h>
 #include <wallet/coincontrol.h>
+#include <wallet/context.h>
 #include <wallet/psbtwallet.h>
 #include <wallet/rpcwallet.h>
 #include <wallet/wallet.h>
@@ -93,6 +95,14 @@ void EnsureWalletIsUnlocked(CWallet * const pwallet)
     }
 }
 
+WalletContext& EnsureWalletContext(const util::Ref& context)
+{
+    if (!context.Has<WalletContext>()) {
+        throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet context not found");
+    }
+    return context.Get<WalletContext>();
+}
+
 static void WalletTxToJSON(interfaces::Chain& chain, interfaces::Chain::Lock& locked_chain, const CWalletTx& wtx, UniValue& entry)
 {
     AssertLockHeld(cs_main); // for mapBlockIndex
@@ -2714,6 +2724,7 @@ static UniValue loadwallet(const JSONRPCRequest& request)
                 },
             }.ToString());
 
+    WalletContext& context = EnsureWalletContext(request.context);
     WalletLocation location(request.params[0].get_str());
 
     if (!location.Exists()) {
@@ -2728,7 +2739,7 @@ static UniValue loadwallet(const JSONRPCRequest& request)
 
     bilingual_str error;
     std::vector<bilingual_str> warnings;
-    std::shared_ptr<CWallet> const wallet = LoadWallet(*g_rpc_chain, location, error, warnings);
+    std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, location, error, warnings);
     if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original);
 
     UniValue obj(UniValue::VOBJ);
@@ -2766,6 +2777,7 @@ static UniValue createwallet(const JSONRPCRequest& request)
         throw std::runtime_error(help.ToString());
     }
 
+    WalletContext& context = EnsureWalletContext(request.context);
     uint64_t flags = 0;
     if (!request.params[1].isNull() && request.params[1].get_bool()) {
         flags |= WALLET_FLAG_DISABLE_PRIVATE_KEYS;
@@ -2787,7 +2799,7 @@ static UniValue createwallet(const JSONRPCRequest& request)
 
     bilingual_str error;
     std::shared_ptr<CWallet> wallet;
-    WalletCreationStatus status = CreateWallet(*g_rpc_chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
+    WalletCreationStatus status = CreateWallet(*context.chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
     switch (status) {
         case WalletCreationStatus::CREATION_FAILED:
             throw JSONRPCError(RPC_WALLET_ERROR, error.original);
@@ -3990,10 +4002,7 @@ static const CRPCCommand commands[] =
 };
 // clang-format on
 
-void RegisterWalletRPCCommands(interfaces::Chain& chain, std::vector<std::unique_ptr<interfaces::Handler>>& handlers)
+Span<const CRPCCommand> GetWalletRPCCommands()
 {
-    for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
-        handlers.emplace_back(chain.handleRpc(commands[vcidx]));
+    return MakeSpan(commands);
 }
-
-interfaces::Chain* g_rpc_chain = nullptr;

src/wallet/rpcwallet.h

@@ -5,32 +5,25 @@
 #ifndef BITCOIN_WALLET_RPCWALLET_H
 #define BITCOIN_WALLET_RPCWALLET_H
 
+#include <span.h>
+
 #include <memory>
 #include <string>
 #include <vector>
 
-class CRPCTable;
+class CRPCCommand;
 class CWallet;
 class JSONRPCRequest;
 class UniValue;
-struct PartiallySignedTransaction;
 class CTransaction;
+struct PartiallySignedTransaction;
+struct WalletContext;
 
-namespace interfaces {
-class Chain;
-class Handler;
-}
 namespace util {
 class Ref;
 } // namespace util
 
-//! 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);
+Span<const CRPCCommand> GetWalletRPCCommands();
 
 /**
  * Figures out what wallet, if any, to use for a JSONRPCRequest.
@@ -42,6 +35,7 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques
 
 std::string HelpRequiringPassphrase();
 void EnsureWalletIsUnlocked(CWallet *);
+WalletContext& EnsureWalletContext(const util::Ref& context);
 
 UniValue getaddressinfo(const JSONRPCRequest& request);
 UniValue signrawtransactionwithwallet(const JSONRPCRequest& request);