Skip to content

Commit 6378eef

Browse files
laanwjlaanwj
committed
Merge #13063: Use shared pointer to retain wallet instance
80b4910 wallet: Use shared pointer to retain wallet instance (João Barbosa) Pull request description: Currently there are 3 places where it makes sense to retain a wallet shared pointer: - `vpwallets`; - `interfaces::Wallet` interface instance - used by the UI; - wallet RPC functions - given by `GetWalletForJSONRPCRequest`. The way it is now it is possible to have, for instance, listunspent RPC and in parallel unload the wallet (once #13111 is merged) without blocking. Once the RPC finishes, the shared pointer will release the wallet. It is also possible to get all existing wallets without blocking because the caller keeps a local list of shared pointers. This is mostly relevant for wallet unloading. This PR replaces #11402. Tree-SHA512: b7e37c7e1ab56626085afe2d40b1628e8d4f0dbda08df01b7e618ecd2d894ce9b83d4219443f444ba889096286eff002f163cb0a48f37063b62e9ba4ccfa6cce
2 parents 5c41b60 + 80b4910 commit 6378eef

File tree

15 files changed

+239
-139
lines changed

15 files changed

+239
-139
lines changed

‎src/interfaces/node.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,8 @@ class NodeImpl : public Node
222222
{
223223
#ifdef ENABLE_WALLET
224224
std::vector<std::unique_ptr<Wallet>> wallets;
225-
for (CWallet* wallet : GetWallets()) {
226-
wallets.emplace_back(MakeWallet(*wallet));
225+
for (const std::shared_ptr<CWallet>& wallet : GetWallets()) {
226+
wallets.emplace_back(MakeWallet(wallet));
227227
}
228228
return wallets;
229229
#else
@@ -249,7 +249,7 @@ class NodeImpl : public Node
249249
std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) override
250250
{
251251
CHECK_WALLET(
252-
return MakeHandler(::uiInterface.LoadWallet.connect([fn](CWallet* wallet) { fn(MakeWallet(*wallet)); })));
252+
return MakeHandler(::uiInterface.LoadWallet.connect([fn](std::shared_ptr<CWallet> wallet) { fn(MakeWallet(wallet)); })));
253253
}
254254
std::unique_ptr<Handler> handleNotifyNumConnectionsChanged(NotifyNumConnectionsChangedFn fn) override
255255
{

‎src/interfaces/wallet.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ WalletTxOut MakeWalletTxOut(CWallet& wallet, const CWalletTx& wtx, int n, int de
118118
class WalletImpl : public Wallet
119119
{
120120
public:
121-
WalletImpl(CWallet& wallet) : m_wallet(wallet) {}
121+
WalletImpl(const std::shared_ptr<CWallet>& wallet) : m_shared_wallet(wallet), m_wallet(*wallet.get()) {}
122122

123123
bool encryptWallet(const SecureString& wallet_passphrase) override
124124
{
@@ -453,11 +453,12 @@ class WalletImpl : public Wallet
453453
return MakeHandler(m_wallet.NotifyWatchonlyChanged.connect(fn));
454454
}
455455

456+
std::shared_ptr<CWallet> m_shared_wallet;
456457
CWallet& m_wallet;
457458
};
458459

459460
} // namespace
460461

461-
std::unique_ptr<Wallet> MakeWallet(CWallet& wallet) { return MakeUnique<WalletImpl>(wallet); }
462+
std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return MakeUnique<WalletImpl>(wallet); }
462463

463464
} // namespace interfaces

‎src/interfaces/wallet.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ struct WalletTxOut
363363

364364
//! Return implementation of Wallet interface. This function will be undefined
365365
//! in builds where ENABLE_WALLET is false.
366-
std::unique_ptr<Wallet> MakeWallet(CWallet& wallet);
366+
std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet);
367367

368368
} // namespace interfaces
369369

‎src/qt/test/addressbooktests.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,15 @@ void EditAddressAndSubmit(
5656
void TestAddAddressesToSendBook()
5757
{
5858
TestChain100Setup test;
59-
CWallet wallet("mock", WalletDatabase::CreateMock());
59+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>("mock", WalletDatabase::CreateMock());
6060
bool firstRun;
61-
wallet.LoadWallet(firstRun);
61+
wallet->LoadWallet(firstRun);
6262

6363
auto build_address = [&wallet]() {
6464
CKey key;
6565
key.MakeNewKey(true);
6666
CTxDestination dest(GetDestinationForKey(
67-
key.GetPubKey(), wallet.m_default_address_type));
67+
key.GetPubKey(), wallet->m_default_address_type));
6868

6969
return std::make_pair(dest, QString::fromStdString(EncodeDestination(dest)));
7070
};
@@ -87,13 +87,13 @@ void TestAddAddressesToSendBook()
8787
std::tie(std::ignore, new_address) = build_address();
8888

8989
{
90-
LOCK(wallet.cs_wallet);
91-
wallet.SetAddressBook(r_key_dest, r_label.toStdString(), "receive");
92-
wallet.SetAddressBook(s_key_dest, s_label.toStdString(), "send");
90+
LOCK(wallet->cs_wallet);
91+
wallet->SetAddressBook(r_key_dest, r_label.toStdString(), "receive");
92+
wallet->SetAddressBook(s_key_dest, s_label.toStdString(), "send");
9393
}
9494

9595
auto check_addbook_size = [&wallet](int expected_size) {
96-
QCOMPARE(static_cast<int>(wallet.mapAddressBook.size()), expected_size);
96+
QCOMPARE(static_cast<int>(wallet->mapAddressBook.size()), expected_size);
9797
};
9898

9999
// We should start with the two addresses we added earlier and nothing else.
@@ -103,9 +103,9 @@ void TestAddAddressesToSendBook()
103103
std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other"));
104104
auto node = interfaces::MakeNode();
105105
OptionsModel optionsModel(*node);
106-
AddWallet(&wallet);
106+
AddWallet(wallet);
107107
WalletModel walletModel(std::move(node->getWallets()[0]), *node, platformStyle.get(), &optionsModel);
108-
RemoveWallet(&wallet);
108+
RemoveWallet(wallet);
109109
EditAddressDialog editAddressDialog(EditAddressDialog::NewSendingAddress);
110110
editAddressDialog.setModel(walletModel.getAddressTableModel());
111111

‎src/qt/test/wallettests.cpp

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -144,39 +144,39 @@ void TestGUI()
144144
for (int i = 0; i < 5; ++i) {
145145
test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
146146
}
147-
CWallet wallet("mock", WalletDatabase::CreateMock());
147+
std::shared_ptr<CWallet> wallet = std::make_shared<CWallet>("mock", WalletDatabase::CreateMock());
148148
bool firstRun;
149-
wallet.LoadWallet(firstRun);
149+
wallet->LoadWallet(firstRun);
150150
{
151-
LOCK(wallet.cs_wallet);
152-
wallet.SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet.m_default_address_type), "", "receive");
153-
wallet.AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey());
151+
LOCK(wallet->cs_wallet);
152+
wallet->SetAddressBook(GetDestinationForKey(test.coinbaseKey.GetPubKey(), wallet->m_default_address_type), "", "receive");
153+
wallet->AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey());
154154
}
155155
{
156156
LOCK(cs_main);
157-
WalletRescanReserver reserver(&wallet);
157+
WalletRescanReserver reserver(wallet.get());
158158
reserver.reserve();
159-
wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, true);
159+
wallet->ScanForWalletTransactions(chainActive.Genesis(), nullptr, reserver, true);
160160
}
161-
wallet.SetBroadcastTransactions(true);
161+
wallet->SetBroadcastTransactions(true);
162162

163163
// Create widgets for sending coins and listing transactions.
164164
std::unique_ptr<const PlatformStyle> platformStyle(PlatformStyle::instantiate("other"));
165165
SendCoinsDialog sendCoinsDialog(platformStyle.get());
166166
TransactionView transactionView(platformStyle.get());
167167
auto node = interfaces::MakeNode();
168168
OptionsModel optionsModel(*node);
169-
AddWallet(&wallet);
169+
AddWallet(wallet);
170170
WalletModel walletModel(std::move(node->getWallets().back()), *node, platformStyle.get(), &optionsModel);
171-
RemoveWallet(&wallet);
171+
RemoveWallet(wallet);
172172
sendCoinsDialog.setModel(&walletModel);
173173
transactionView.setModel(&walletModel);
174174

175175
// Send two transactions, and verify they are added to transaction list.
176176
TransactionTableModel* transactionTableModel = walletModel.getTransactionTableModel();
177177
QCOMPARE(transactionTableModel->rowCount({}), 105);
178-
uint256 txid1 = SendCoins(wallet, sendCoinsDialog, CKeyID(), 5 * COIN, false /* rbf */);
179-
uint256 txid2 = SendCoins(wallet, sendCoinsDialog, CKeyID(), 10 * COIN, true /* rbf */);
178+
uint256 txid1 = SendCoins(*wallet.get(), sendCoinsDialog, CKeyID(), 5 * COIN, false /* rbf */);
179+
uint256 txid2 = SendCoins(*wallet.get(), sendCoinsDialog, CKeyID(), 10 * COIN, true /* rbf */);
180180
QCOMPARE(transactionTableModel->rowCount({}), 107);
181181
QVERIFY(FindTx(*transactionTableModel, txid1).isValid());
182182
QVERIFY(FindTx(*transactionTableModel, txid2).isValid());

‎src/rpc/rawtransaction.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,8 @@ static UniValue signrawtransactionwithkey(const JSONRPCRequest& request)
988988
UniValue signrawtransaction(const JSONRPCRequest& request)
989989
{
990990
#ifdef ENABLE_WALLET
991-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
991+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
992+
CWallet* const pwallet = wallet.get();
992993
#endif
993994

994995
if (request.fHelp || request.params.size() < 1 || request.params.size() > 4)

‎src/ui_interface.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#ifndef BITCOIN_UI_INTERFACE_H
77
#define BITCOIN_UI_INTERFACE_H
88

9+
#include <memory>
910
#include <stdint.h>
1011
#include <string>
1112

@@ -92,7 +93,7 @@ class CClientUIInterface
9293
boost::signals2::signal<void ()> NotifyAlertChanged;
9394

9495
/** A wallet has been loaded. */
95-
boost::signals2::signal<void (CWallet* wallet)> LoadWallet;
96+
boost::signals2::signal<void (std::shared_ptr<CWallet> wallet)> LoadWallet;
9697

9798
/**
9899
* Show progress e.g. for verifychain.

‎src/wallet/init.cpp

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ bool WalletInit::Open() const
226226
}
227227

228228
for (const std::string& walletFile : gArgs.GetArgs("-wallet")) {
229-
CWallet * const pwallet = CWallet::CreateWalletFromFile(walletFile, fs::absolute(walletFile, GetWalletDir()));
229+
std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(walletFile, fs::absolute(walletFile, GetWalletDir()));
230230
if (!pwallet) {
231231
return false;
232232
}
@@ -238,7 +238,7 @@ bool WalletInit::Open() const
238238

239239
void WalletInit::Start(CScheduler& scheduler) const
240240
{
241-
for (CWallet* pwallet : GetWallets()) {
241+
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
242242
pwallet->postInitProcess();
243243
}
244244

@@ -248,22 +248,21 @@ void WalletInit::Start(CScheduler& scheduler) const
248248

249249
void WalletInit::Flush() const
250250
{
251-
for (CWallet* pwallet : GetWallets()) {
251+
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
252252
pwallet->Flush(false);
253253
}
254254
}
255255

256256
void WalletInit::Stop() const
257257
{
258-
for (CWallet* pwallet : GetWallets()) {
258+
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
259259
pwallet->Flush(true);
260260
}
261261
}
262262

263263
void WalletInit::Close() const
264264
{
265-
for (CWallet* pwallet : GetWallets()) {
265+
for (const std::shared_ptr<CWallet>& pwallet : GetWallets()) {
266266
RemoveWallet(pwallet);
267-
delete pwallet;
268267
}
269268
}

‎src/wallet/rpcdump.cpp

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ static std::string DecodeDumpString(const std::string &str) {
5656
for (unsigned int pos = 0; pos < str.length(); pos++) {
5757
unsigned char c = str[pos];
5858
if (c == '%' && pos+2 < str.length()) {
59-
c = (((str[pos+1]>>6)*9+((str[pos+1]-'0')&15)) << 4) |
59+
c = (((str[pos+1]>>6)*9+((str[pos+1]-'0')&15)) << 4) |
6060
((str[pos+2]>>6)*9+((str[pos+2]-'0')&15));
6161
pos += 2;
6262
}
@@ -89,7 +89,8 @@ static bool GetWalletAddressesForKey(CWallet * const pwallet, const CKeyID &keyi
8989

9090
UniValue importprivkey(const JSONRPCRequest& request)
9191
{
92-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
92+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
93+
CWallet* const pwallet = wallet.get();
9394
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
9495
return NullUniValue;
9596
}
@@ -185,7 +186,8 @@ UniValue importprivkey(const JSONRPCRequest& request)
185186

186187
UniValue abortrescan(const JSONRPCRequest& request)
187188
{
188-
CWallet* const pwallet = GetWalletForJSONRPCRequest(request);
189+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
190+
CWallet* const pwallet = wallet.get();
189191
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
190192
return NullUniValue;
191193
}
@@ -246,7 +248,8 @@ static void ImportAddress(CWallet* const pwallet, const CTxDestination& dest, co
246248

247249
UniValue importaddress(const JSONRPCRequest& request)
248250
{
249-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
251+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
252+
CWallet* const pwallet = wallet.get();
250253
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
251254
return NullUniValue;
252255
}
@@ -330,7 +333,8 @@ UniValue importaddress(const JSONRPCRequest& request)
330333

331334
UniValue importprunedfunds(const JSONRPCRequest& request)
332335
{
333-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
336+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
337+
CWallet* const pwallet = wallet.get();
334338
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
335339
return NullUniValue;
336340
}
@@ -392,7 +396,8 @@ UniValue importprunedfunds(const JSONRPCRequest& request)
392396

393397
UniValue removeprunedfunds(const JSONRPCRequest& request)
394398
{
395-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
399+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
400+
CWallet* const pwallet = wallet.get();
396401
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
397402
return NullUniValue;
398403
}
@@ -430,7 +435,8 @@ UniValue removeprunedfunds(const JSONRPCRequest& request)
430435

431436
UniValue importpubkey(const JSONRPCRequest& request)
432437
{
433-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
438+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
439+
CWallet* const pwallet = wallet.get();
434440
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
435441
return NullUniValue;
436442
}
@@ -506,7 +512,8 @@ UniValue importpubkey(const JSONRPCRequest& request)
506512

507513
UniValue importwallet(const JSONRPCRequest& request)
508514
{
509-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
515+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
516+
CWallet* const pwallet = wallet.get();
510517
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
511518
return NullUniValue;
512519
}
@@ -640,7 +647,8 @@ UniValue importwallet(const JSONRPCRequest& request)
640647

641648
UniValue dumpprivkey(const JSONRPCRequest& request)
642649
{
643-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
650+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
651+
CWallet* const pwallet = wallet.get();
644652
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
645653
return NullUniValue;
646654
}
@@ -683,7 +691,8 @@ UniValue dumpprivkey(const JSONRPCRequest& request)
683691

684692
UniValue dumpwallet(const JSONRPCRequest& request)
685693
{
686-
CWallet * const pwallet = GetWalletForJSONRPCRequest(request);
694+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
695+
CWallet* const pwallet = wallet.get();
687696
if (!EnsureWalletIsAvailable(pwallet, request.fHelp)) {
688697
return NullUniValue;
689698
}
@@ -1127,7 +1136,8 @@ static int64_t GetImportTimestamp(const UniValue& data, int64_t now)
11271136

11281137
UniValue importmulti(const JSONRPCRequest& mainRequest)
11291138
{
1130-
CWallet * const pwallet = GetWalletForJSONRPCRequest(mainRequest);
1139+
std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(mainRequest);
1140+
CWallet* const pwallet = wallet.get();
11311141
if (!EnsureWalletIsAvailable(pwallet, mainRequest.fHelp)) {
11321142
return NullUniValue;
11331143
}

0 commit comments

Comments
 (0)