Skip to content

Commit 4a7253a

Browse files
ryanofskypromag
andcommitted
Remove g_rpc_chain global
Replace with RPC request reference to new WalletContext struct similar to the existing NodeContext struct and reference. This PR is a followup to 25ad2c6 bitcoin/bitcoin#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 <[email protected]>
1 parent e783197 commit 4a7253a

File tree

7 files changed

+81
-20
lines changed

7 files changed

+81
-20
lines changed

src/Makefile.am

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ BITCOIN_CORE_H = \
241241
versionbitsinfo.h \
242242
walletinitinterface.h \
243243
wallet/coincontrol.h \
244+
wallet/context.h \
244245
wallet/crypter.h \
245246
wallet/db.h \
246247
wallet/feebumper.h \
@@ -350,6 +351,7 @@ libbitcoin_wallet_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
350351
libbitcoin_wallet_a_SOURCES = \
351352
interfaces/wallet.cpp \
352353
wallet/coincontrol.cpp \
354+
wallet/context.cpp \
353355
wallet/crypter.cpp \
354356
wallet/db.cpp \
355357
wallet/feebumper.cpp \

src/interfaces/wallet.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
#include <ui_interface.h>
1717
#include <uint256.h>
1818
#include <util/check.h>
19+
#include <util/ref.h>
1920
#include <util/system.h>
21+
#include <wallet/context.h>
2022
#include <wallet/feebumper.h>
2123
#include <wallet/fees.h>
2224
#include <wallet/ismine.h>
@@ -482,18 +484,21 @@ class WalletClientImpl : public ChainClient
482484
{
483485
public:
484486
WalletClientImpl(Chain& chain, std::vector<std::string> wallet_filenames)
485-
: m_chain(chain), m_wallet_filenames(std::move(wallet_filenames))
487+
: m_wallet_filenames(std::move(wallet_filenames))
486488
{
489+
m_context.chain = &chain;
487490
}
488491
void registerRpcs() override
489492
{
490-
g_rpc_chain = &m_chain;
491493
for (const CRPCCommand& command : GetWalletRPCCommands()) {
492-
m_rpc_handlers.emplace_back(m_chain.handleRpc(command));
494+
m_rpc_commands.emplace_back(command.category, command.name, [this, &command](const JSONRPCRequest& request, UniValue& result, bool last_handler) {
495+
return command.actor({request, m_context}, result, last_handler);
496+
}, command.argNames, command.unique_id);
497+
m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
493498
}
494499
}
495-
bool verify() override { return VerifyWallets(m_chain, m_wallet_filenames); }
496-
bool load() override { return LoadWallets(m_chain, m_wallet_filenames); }
500+
bool verify() override { return VerifyWallets(*m_context.chain, m_wallet_filenames); }
501+
bool load() override { return LoadWallets(*m_context.chain, m_wallet_filenames); }
497502
void start(CScheduler& scheduler) override { return StartWallets(scheduler); }
498503
void flush() override { return FlushWallets(); }
499504
void stop() override { return StopWallets(); }
@@ -508,9 +513,10 @@ class WalletClientImpl : public ChainClient
508513
}
509514
~WalletClientImpl() override { UnloadWallets(); }
510515

511-
Chain& m_chain;
516+
WalletContext m_context;
512517
std::vector<std::string> m_wallet_filenames;
513518
std::vector<std::unique_ptr<Handler>> m_rpc_handlers;
519+
std::list<CRPCCommand> m_rpc_commands;
514520
};
515521

516522
} // namespace

src/rpc/request.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ class JSONRPCRequest
4141
const util::Ref& context;
4242

4343
JSONRPCRequest(const util::Ref& context) : id(NullUniValue), params(NullUniValue), fHelp(false), context(context) {}
44+
45+
//! Initializes request information from another request object and the
46+
//! given context. The implementation should be updated if any members are
47+
//! added or removed above.
48+
JSONRPCRequest(const JSONRPCRequest& other, const util::Ref& context)
49+
: id(other.id), strMethod(other.strMethod), params(other.params), fHelp(other.fHelp), URI(other.URI),
50+
authUser(other.authUser), peerAddr(other.peerAddr), context(context)
51+
{
52+
}
53+
4454
void parse(const UniValue& valRequest);
4555
};
4656

src/wallet/context.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// Copyright (c) 2020 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <wallet/context.h>
6+
7+
WalletContext::WalletContext() {}
8+
WalletContext::~WalletContext() {}

src/wallet/context.h

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) 2020 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#ifndef BITCOIN_WALLET_CONTEXT_H
6+
#define BITCOIN_WALLET_CONTEXT_H
7+
8+
namespace interfaces {
9+
class Chain;
10+
} // namespace interfaces
11+
12+
//! WalletContext struct containing references to state shared between CWallet
13+
//! instances, like the reference to the chain interface, and the list of opened
14+
//! wallets.
15+
//!
16+
//! Future shared state can be added here as an alternative to adding global
17+
//! variables.
18+
//!
19+
//! The struct isn't intended to have any member functions. It should just be a
20+
//! collection of state pointers that doesn't pull in dependencies or implement
21+
//! behavior.
22+
struct WalletContext {
23+
interfaces::Chain* chain{nullptr};
24+
25+
//! Declare default constructor and destructor that are not inline, so code
26+
//! instantiating the WalletContext struct doesn't need to #include class
27+
//! definitions for smart pointer and container members.
28+
WalletContext();
29+
~WalletContext();
30+
};
31+
32+
#endif // BITCOIN_WALLET_CONTEXT_H

src/wallet/rpcwallet.cpp

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@
2121
#include <util/fees.h>
2222
#include <util/message.h> // For MessageSign()
2323
#include <util/moneystr.h>
24+
#include <util/ref.h>
2425
#include <util/string.h>
2526
#include <util/system.h>
2627
#include <util/translation.h>
2728
#include <util/url.h>
2829
#include <util/vector.h>
2930
#include <wallet/coincontrol.h>
31+
#include <wallet/context.h>
3032
#include <wallet/feebumper.h>
3133
#include <wallet/rpcwallet.h>
3234
#include <wallet/wallet.h>
@@ -121,6 +123,14 @@ void EnsureWalletIsUnlocked(const CWallet* pwallet)
121123
}
122124
}
123125

126+
WalletContext& EnsureWalletContext(const util::Ref& context)
127+
{
128+
if (!context.Has<WalletContext>()) {
129+
throw JSONRPCError(RPC_INTERNAL_ERROR, "Wallet context not found");
130+
}
131+
return context.Get<WalletContext>();
132+
}
133+
124134
// 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
125135
LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create)
126136
{
@@ -2584,6 +2594,7 @@ static UniValue loadwallet(const JSONRPCRequest& request)
25842594
},
25852595
}.Check(request);
25862596

2597+
WalletContext& context = EnsureWalletContext(request.context);
25872598
WalletLocation location(request.params[0].get_str());
25882599

25892600
if (!location.Exists()) {
@@ -2598,7 +2609,7 @@ static UniValue loadwallet(const JSONRPCRequest& request)
25982609

25992610
bilingual_str error;
26002611
std::vector<bilingual_str> warnings;
2601-
std::shared_ptr<CWallet> const wallet = LoadWallet(*g_rpc_chain, location, error, warnings);
2612+
std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, location, error, warnings);
26022613
if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original);
26032614

26042615
UniValue obj(UniValue::VOBJ);
@@ -2702,6 +2713,7 @@ static UniValue createwallet(const JSONRPCRequest& request)
27022713
},
27032714
}.Check(request);
27042715

2716+
WalletContext& context = EnsureWalletContext(request.context);
27052717
uint64_t flags = 0;
27062718
if (!request.params[1].isNull() && request.params[1].get_bool()) {
27072719
flags |= WALLET_FLAG_DISABLE_PRIVATE_KEYS;
@@ -2731,7 +2743,7 @@ static UniValue createwallet(const JSONRPCRequest& request)
27312743

27322744
bilingual_str error;
27332745
std::shared_ptr<CWallet> wallet;
2734-
WalletCreationStatus status = CreateWallet(*g_rpc_chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
2746+
WalletCreationStatus status = CreateWallet(*context.chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
27352747
switch (status) {
27362748
case WalletCreationStatus::CREATION_FAILED:
27372749
throw JSONRPCError(RPC_WALLET_ERROR, error.original);
@@ -4331,5 +4343,3 @@ static const CRPCCommand commands[] =
43314343
// clang-format on
43324344
return MakeSpan(commands);
43334345
}
4334-
4335-
interfaces::Chain* g_rpc_chain = nullptr;

src/wallet/rpcwallet.h

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,10 @@ class CWallet;
1616
class JSONRPCRequest;
1717
class LegacyScriptPubKeyMan;
1818
class UniValue;
19-
struct PartiallySignedTransaction;
2019
class CTransaction;
20+
struct PartiallySignedTransaction;
21+
struct WalletContext;
2122

22-
namespace interfaces {
23-
class Chain;
24-
}
25-
26-
//! Pointer to chain interface that needs to be declared as a global to be
27-
//! accessible loadwallet and createwallet methods. Due to limitations of the
28-
//! RPC framework, there's currently no direct way to pass in state to RPC
29-
//! methods without globals.
30-
extern interfaces::Chain* g_rpc_chain;
3123
Span<const CRPCCommand> GetWalletRPCCommands();
3224

3325
/**
@@ -40,6 +32,7 @@ std::shared_ptr<CWallet> GetWalletForJSONRPCRequest(const JSONRPCRequest& reques
4032

4133
void EnsureWalletIsUnlocked(const CWallet*);
4234
bool EnsureWalletIsAvailable(const CWallet*, bool avoidException);
35+
WalletContext& EnsureWalletContext(const util::Ref& context);
4336
LegacyScriptPubKeyMan& EnsureLegacyScriptPubKeyMan(CWallet& wallet, bool also_create = false);
4437

4538
UniValue getaddressinfo(const JSONRPCRequest& request);

0 commit comments

Comments
 (0)