From 4a7253ab6c3bb323581cea54573529c2f823f035 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Thu, 28 May 2020 02:13:19 -0400 Subject: Remove g_rpc_chain global MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace with RPC request reference to new WalletContext struct similar to the existing NodeContext struct and reference. This PR is a followup to 25ad2c623af30056ffb36dcd203a52edda2b170f https://github.com/bitcoin/bitcoin/pull/18740 removing the g_rpc_node global. Some later PRs will follow this up and move more wallet globals to the WalletContext struct. Co-authored-by: João Barbosa --- src/Makefile.am | 2 ++ src/interfaces/wallet.cpp | 18 ++++++++++++------ src/rpc/request.h | 10 ++++++++++ src/wallet/context.cpp | 8 ++++++++ src/wallet/context.h | 32 ++++++++++++++++++++++++++++++++ src/wallet/rpcwallet.cpp | 18 ++++++++++++++---- src/wallet/rpcwallet.h | 13 +++---------- 7 files changed, 81 insertions(+), 20 deletions(-) create mode 100644 src/wallet/context.cpp create mode 100644 src/wallet/context.h diff --git a/src/Makefile.am b/src/Makefile.am index 2b004691fd..7a280a67a7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -241,6 +241,7 @@ BITCOIN_CORE_H = \ versionbitsinfo.h \ walletinitinterface.h \ wallet/coincontrol.h \ + wallet/context.h \ wallet/crypter.h \ wallet/db.h \ wallet/feebumper.h \ @@ -350,6 +351,7 @@ libbitcoin_wallet_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libbitcoin_wallet_a_SOURCES = \ interfaces/wallet.cpp \ wallet/coincontrol.cpp \ + wallet/context.cpp \ wallet/crypter.cpp \ wallet/db.cpp \ wallet/feebumper.cpp \ diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 9e5b357f69..7b907b2a0b 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -16,7 +16,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -482,18 +484,21 @@ class WalletClientImpl : public ChainClient { public: WalletClientImpl(Chain& chain, std::vector 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; for (const CRPCCommand& command : GetWalletRPCCommands()) { - m_rpc_handlers.emplace_back(m_chain.handleRpc(command)); + 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(); } @@ -508,9 +513,10 @@ public: } ~WalletClientImpl() override { UnloadWallets(); } - Chain& m_chain; + WalletContext m_context; std::vector m_wallet_filenames; std::vector> m_rpc_handlers; + std::list m_rpc_commands; }; } // namespace diff --git a/src/rpc/request.h b/src/rpc/request.h index 02ec5393a7..4761e9e371 100644 --- a/src/rpc/request.h +++ b/src/rpc/request.h @@ -41,6 +41,16 @@ public: 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); }; diff --git a/src/wallet/context.cpp b/src/wallet/context.cpp new file mode 100644 index 0000000000..09b2f30467 --- /dev/null +++ b/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 + +WalletContext::WalletContext() {} +WalletContext::~WalletContext() {} diff --git a/src/wallet/context.h b/src/wallet/context.h new file mode 100644 index 0000000000..3c8fdd1c59 --- /dev/null +++ b/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 diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index c4052d67b0..ae2b19e923 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -21,12 +21,14 @@ #include #include // For MessageSign() #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -121,6 +123,14 @@ void EnsureWalletIsUnlocked(const CWallet* pwallet) } } +WalletContext& EnsureWalletContext(const util::Ref& context) +{ + if (!context.Has()) { + throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet context not found"); + } + return context.Get(); +} + // also_create should only be set to true only when the RPC is expected to add things to a blank wallet and make it no longer blank LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create) { @@ -2584,6 +2594,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) }, }.Check(request); + WalletContext& context = EnsureWalletContext(request.context); WalletLocation location(request.params[0].get_str()); if (!location.Exists()) { @@ -2598,7 +2609,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) bilingual_str error; std::vector warnings; - std::shared_ptr const wallet = LoadWallet(*g_rpc_chain, location, error, warnings); + std::shared_ptr const wallet = LoadWallet(*context.chain, location, error, warnings); if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original); UniValue obj(UniValue::VOBJ); @@ -2702,6 +2713,7 @@ static UniValue createwallet(const JSONRPCRequest& request) }, }.Check(request); + 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; @@ -2731,7 +2743,7 @@ static UniValue createwallet(const JSONRPCRequest& request) bilingual_str error; std::shared_ptr 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); @@ -4331,5 +4343,3 @@ static const CRPCCommand commands[] = // clang-format on return MakeSpan(commands); } - -interfaces::Chain* g_rpc_chain = nullptr; diff --git a/src/wallet/rpcwallet.h b/src/wallet/rpcwallet.h index fd8a41ed14..d00221a04c 100644 --- a/src/wallet/rpcwallet.h +++ b/src/wallet/rpcwallet.h @@ -16,18 +16,10 @@ class CWallet; class JSONRPCRequest; class LegacyScriptPubKeyMan; class UniValue; -struct PartiallySignedTransaction; class CTransaction; +struct PartiallySignedTransaction; +struct WalletContext; -namespace interfaces { -class Chain; -} - -//! 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; Span GetWalletRPCCommands(); /** @@ -40,6 +32,7 @@ std::shared_ptr GetWalletForJSONRPCRequest(const JSONRPCRequest& reques void EnsureWalletIsUnlocked(const CWallet*); bool EnsureWalletIsAvailable(const CWallet*, bool avoidException); +WalletContext& EnsureWalletContext(const util::Ref& context); LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false); UniValue getaddressinfo(const JSONRPCRequest& request); -- cgit v1.2.3