Skip to content

Commit f1ee373

Browse files
achow101achow101
committed
wallet: Reload previously loaded wallets on GUI startup
Enable the GUI to also use the load_on_startup feature. Wallets loaded in the GUI always have load_on_startup=true. When they are unloaded from the GUI, load_on_startup=false. To facilitate this change, UpdateWalletSetting is moved into the wallet module and called from within LoadWallet, RemoveWallet, and Createwallet. This change does not actually touch the GUI code but rather the wallet functions that are shared between the GUI and RPC.
1 parent 89a8299 commit f1ee373

File tree

10 files changed

+104
-77
lines changed

10 files changed

+104
-77
lines changed

‎doc/release-notes-15937.md

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
Configuration
22
-------------
33

4-
The `createwallet`, `loadwallet`, and `unloadwallet` RPCs now accept
5-
`load_on_startup` options that modify bitcoin's dynamic configuration in
6-
`\<datadir\>/settings.json`, and can add or remove a wallet from the list of
7-
wallets automatically loaded at startup. Unless these options are explicitly
8-
set to true or false, the load on startup wallet list is not modified, so this
9-
change is backwards compatible.
4+
Wallets created or loaded in the GUI will now be automatically loaded on
5+
startup, so they don't need to be manually reloaded next time Bitcoin is
6+
started. The list of wallets to load on startup is stored in
7+
`\<datadir\>/settings.json` and augments any command line or `bitcoin.conf`
8+
`-wallet=` settings that specify more wallets to load. Wallets that are
9+
unloaded in the GUI get removed from the settings list so they won't load again
10+
automatically next startup. (#19754)
1011

11-
In the future, the GUI will start updating the same startup wallet list as the
12-
RPCs to automatically reopen wallets previously opened in the GUI.
12+
The `createwallet`, `loadwallet`, and `unloadwallet` RPCs now accept
13+
`load_on_startup` options to modify the settings list. Unless these options are
14+
explicitly set to true or false, the list is not modified, so the RPC methods
15+
remain backwards compatible. (#15937)

‎src/interfaces/wallet.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ class WalletImpl : public Wallet
446446
CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; }
447447
void remove() override
448448
{
449-
RemoveWallet(m_wallet);
449+
RemoveWallet(m_wallet, false /* load_on_start */);
450450
}
451451
bool isLegacy() override { return m_wallet->IsLegacy(); }
452452
std::unique_ptr<Handler> handleUnload(UnloadFn fn) override
@@ -517,12 +517,12 @@ class WalletClientImpl : public WalletClient
517517
std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) override
518518
{
519519
std::shared_ptr<CWallet> wallet;
520-
status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet);
520+
status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, true /* load_on_start */, error, warnings, wallet);
521521
return MakeWallet(std::move(wallet));
522522
}
523523
std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
524524
{
525-
return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), error, warnings));
525+
return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), true /* load_on_start */, error, warnings));
526526
}
527527
std::string getWalletDir() override
528528
{

‎src/qt/test/addressbooktests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node)
112112
ClientModel clientModel(node, &optionsModel);
113113
AddWallet(wallet);
114114
WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel, platformStyle.get());
115-
RemoveWallet(wallet);
115+
RemoveWallet(wallet, nullopt);
116116
EditAddressDialog editAddressDialog(EditAddressDialog::NewSendingAddress);
117117
editAddressDialog.setModel(walletModel.getAddressTableModel());
118118

‎src/qt/test/wallettests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ void TestGUI(interfaces::Node& node)
167167
ClientModel clientModel(node, &optionsModel);
168168
AddWallet(wallet);
169169
WalletModel walletModel(interfaces::MakeWallet(wallet), clientModel, platformStyle.get());
170-
RemoveWallet(wallet);
170+
RemoveWallet(wallet, nullopt);
171171
sendCoinsDialog.setModel(&walletModel);
172172
transactionView.setModel(&walletModel);
173173

‎src/wallet/load.cpp

Lines changed: 2 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -118,30 +118,8 @@ void UnloadWallets()
118118
while (!wallets.empty()) {
119119
auto wallet = wallets.back();
120120
wallets.pop_back();
121-
RemoveWallet(wallet);
121+
std::vector<bilingual_str> warnings;
122+
RemoveWallet(wallet, nullopt, warnings);
122123
UnloadWallet(std::move(wallet));
123124
}
124125
}
125-
126-
bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name)
127-
{
128-
util::SettingsValue setting_value = chain.getRwSetting("wallet");
129-
if (!setting_value.isArray()) setting_value.setArray();
130-
for (const util::SettingsValue& value : setting_value.getValues()) {
131-
if (value.isStr() && value.get_str() == wallet_name) return true;
132-
}
133-
setting_value.push_back(wallet_name);
134-
return chain.updateRwSetting("wallet", setting_value);
135-
}
136-
137-
bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name)
138-
{
139-
util::SettingsValue setting_value = chain.getRwSetting("wallet");
140-
if (!setting_value.isArray()) return true;
141-
util::SettingsValue new_value(util::SettingsValue::VARR);
142-
for (const util::SettingsValue& value : setting_value.getValues()) {
143-
if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value);
144-
}
145-
if (new_value.size() == setting_value.size()) return true;
146-
return chain.updateRwSetting("wallet", new_value);
147-
}

‎src/wallet/load.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,4 @@ void StopWallets();
3434
//! Close all wallets.
3535
void UnloadWallets();
3636

37-
//! Add wallet name to persistent configuration so it will be loaded on startup.
38-
bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
39-
40-
//! Remove wallet name from persistent configuration so it will not be loaded on startup.
41-
bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
42-
4337
#endif // BITCOIN_WALLET_LOAD_H

‎src/wallet/rpcwallet.cpp

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -230,18 +230,6 @@ static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const U
230230
}
231231
}
232232

233-
static void UpdateWalletSetting(interfaces::Chain& chain,
234-
const std::string& wallet_name,
235-
const UniValue& load_on_startup,
236-
std::vector<bilingual_str>& warnings)
237-
{
238-
if (load_on_startup.isTrue() && !AddWalletSetting(chain, wallet_name)) {
239-
warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may not be loaded next node startup."));
240-
} else if (load_on_startup.isFalse() && !RemoveWalletSetting(chain, wallet_name)) {
241-
warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may still be loaded next node startup."));
242-
}
243-
}
244-
245233
static UniValue getnewaddress(const JSONRPCRequest& request)
246234
{
247235
RPCHelpMan{"getnewaddress",
@@ -2528,11 +2516,10 @@ static UniValue loadwallet(const JSONRPCRequest& request)
25282516

25292517
bilingual_str error;
25302518
std::vector<bilingual_str> warnings;
2531-
std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, location, error, warnings);
2519+
Optional<bool> load_on_start = request.params[1].isNull() ? nullopt : Optional<bool>(request.params[1].get_bool());
2520+
std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, location, load_on_start, error, warnings);
25322521
if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original);
25332522

2534-
UpdateWalletSetting(*context.chain, location.GetName(), request.params[1], warnings);
2535-
25362523
UniValue obj(UniValue::VOBJ);
25372524
obj.pushKV("name", wallet->GetName());
25382525
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
@@ -2662,7 +2649,8 @@ static UniValue createwallet(const JSONRPCRequest& request)
26622649

26632650
bilingual_str error;
26642651
std::shared_ptr<CWallet> wallet;
2665-
WalletCreationStatus status = CreateWallet(*context.chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
2652+
Optional<bool> load_on_start = request.params[6].isNull() ? nullopt : Optional<bool>(request.params[6].get_bool());
2653+
WalletCreationStatus status = CreateWallet(*context.chain, passphrase, flags, request.params[0].get_str(), load_on_start, error, warnings, wallet);
26662654
switch (status) {
26672655
case WalletCreationStatus::CREATION_FAILED:
26682656
throw JSONRPCError(RPC_WALLET_ERROR, error.original);
@@ -2673,8 +2661,6 @@ static UniValue createwallet(const JSONRPCRequest& request)
26732661
// no default case, so the compiler can warn about missing cases
26742662
}
26752663

2676-
UpdateWalletSetting(*context.chain, request.params[0].get_str(), request.params[6], warnings);
2677-
26782664
UniValue obj(UniValue::VOBJ);
26792665
obj.pushKV("name", wallet->GetName());
26802666
obj.pushKV("warning", Join(warnings, Untranslated("\n")).original);
@@ -2717,15 +2703,13 @@ static UniValue unloadwallet(const JSONRPCRequest& request)
27172703
// Release the "main" shared pointer and prevent further notifications.
27182704
// Note that any attempt to load the same wallet would fail until the wallet
27192705
// is destroyed (see CheckUniqueFileid).
2720-
if (!RemoveWallet(wallet)) {
2706+
std::vector<bilingual_str> warnings;
2707+
Optional<bool> load_on_start = request.params[1].isNull() ? nullopt : Optional<bool>(request.params[1].get_bool());
2708+
if (!RemoveWallet(wallet, load_on_start, warnings)) {
27212709
throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded");
27222710
}
27232711

2724-
interfaces::Chain& chain = wallet->chain();
2725-
std::vector<bilingual_str> warnings;
2726-
27272712
UnloadWallet(std::move(wallet));
2728-
UpdateWalletSetting(chain, wallet_name, request.params[1], warnings);
27292713

27302714
UniValue result(UniValue::VOBJ);
27312715
result.pushKV("warning", Join(warnings, Untranslated("\n")).original);

‎src/wallet/test/wallet_tests.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup)
229229
"downloading and rescanning the relevant blocks (see -reindex and -rescan "
230230
"options).\"}},{\"success\":true}]",
231231
0, oldTip->GetBlockTimeMax(), TIMESTAMP_WINDOW));
232-
RemoveWallet(wallet);
232+
RemoveWallet(wallet, nullopt);
233233
}
234234
}
235235

@@ -275,7 +275,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
275275
request.params.push_back(backup_file);
276276

277277
::dumpwallet().HandleRequest(request);
278-
RemoveWallet(wallet);
278+
RemoveWallet(wallet, nullopt);
279279
}
280280

281281
// Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME
@@ -292,7 +292,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
292292
AddWallet(wallet);
293293
wallet->SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash());
294294
::importwallet().HandleRequest(request);
295-
RemoveWallet(wallet);
295+
RemoveWallet(wallet, nullopt);
296296

297297
BOOST_CHECK_EQUAL(wallet->mapWallet.size(), 3U);
298298
BOOST_CHECK_EQUAL(m_coinbase_txns.size(), 103U);

‎src/wallet/wallet.cpp

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
#include <wallet/coincontrol.h>
3434
#include <wallet/fees.h>
3535

36+
#include <univalue.h>
37+
3638
#include <algorithm>
3739
#include <assert.h>
3840

@@ -54,6 +56,42 @@ static RecursiveMutex cs_wallets;
5456
static std::vector<std::shared_ptr<CWallet>> vpwallets GUARDED_BY(cs_wallets);
5557
static std::list<LoadWalletFn> g_load_wallet_fns GUARDED_BY(cs_wallets);
5658

59+
bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name)
60+
{
61+
util::SettingsValue setting_value = chain.getRwSetting("wallet");
62+
if (!setting_value.isArray()) setting_value.setArray();
63+
for (const util::SettingsValue& value : setting_value.getValues()) {
64+
if (value.isStr() && value.get_str() == wallet_name) return true;
65+
}
66+
setting_value.push_back(wallet_name);
67+
return chain.updateRwSetting("wallet", setting_value);
68+
}
69+
70+
bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name)
71+
{
72+
util::SettingsValue setting_value = chain.getRwSetting("wallet");
73+
if (!setting_value.isArray()) return true;
74+
util::SettingsValue new_value(util::SettingsValue::VARR);
75+
for (const util::SettingsValue& value : setting_value.getValues()) {
76+
if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value);
77+
}
78+
if (new_value.size() == setting_value.size()) return true;
79+
return chain.updateRwSetting("wallet", new_value);
80+
}
81+
82+
static void UpdateWalletSetting(interfaces::Chain& chain,
83+
const std::string& wallet_name,
84+
Optional<bool> load_on_startup,
85+
std::vector<bilingual_str>& warnings)
86+
{
87+
if (load_on_startup == nullopt) return;
88+
if (load_on_startup.get() && !AddWalletSetting(chain, wallet_name)) {
89+
warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may not be loaded next node startup."));
90+
} else if (!load_on_startup.get() && !RemoveWalletSetting(chain, wallet_name)) {
91+
warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may still be loaded next node startup."));
92+
}
93+
}
94+
5795
bool AddWallet(const std::shared_ptr<CWallet>& wallet)
5896
{
5997
LOCK(cs_wallets);
@@ -65,18 +103,32 @@ bool AddWallet(const std::shared_ptr<CWallet>& wallet)
65103
return true;
66104
}
67105

68-
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet)
106+
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, Optional<bool> load_on_start, std::vector<bilingual_str>& warnings)
69107
{
70108
assert(wallet);
109+
110+
interfaces::Chain& chain = wallet->chain();
111+
std::string name = wallet->GetName();
112+
71113
// Unregister with the validation interface which also drops shared ponters.
72114
wallet->m_chain_notifications_handler.reset();
73115
LOCK(cs_wallets);
74116
std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(vpwallets.begin(), vpwallets.end(), wallet);
75117
if (i == vpwallets.end()) return false;
76118
vpwallets.erase(i);
119+
120+
// Write the wallet setting
121+
UpdateWalletSetting(chain, name, load_on_start, warnings);
122+
77123
return true;
78124
}
79125

126+
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, Optional<bool> load_on_start)
127+
{
128+
std::vector<bilingual_str> warnings;
129+
return RemoveWallet(wallet, load_on_start, warnings);
130+
}
131+
80132
std::vector<std::shared_ptr<CWallet>> GetWallets()
81133
{
82134
LOCK(cs_wallets);
@@ -148,7 +200,7 @@ void UnloadWallet(std::shared_ptr<CWallet>&& wallet)
148200
}
149201

150202
namespace {
151-
std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings)
203+
std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, Optional<bool> load_on_start, bilingual_str& error, std::vector<bilingual_str>& warnings)
152204
{
153205
try {
154206
if (!CWallet::Verify(chain, location, error, warnings)) {
@@ -163,6 +215,10 @@ std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const Wall
163215
}
164216
AddWallet(wallet);
165217
wallet->postInitProcess();
218+
219+
// Write the wallet setting
220+
UpdateWalletSetting(chain, location.GetName(), load_on_start, warnings);
221+
166222
return wallet;
167223
} catch (const std::runtime_error& e) {
168224
error = Untranslated(e.what());
@@ -171,19 +227,19 @@ std::shared_ptr<CWallet> LoadWalletInternal(interfaces::Chain& chain, const Wall
171227
}
172228
} // namespace
173229

174-
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings)
230+
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, Optional<bool> load_on_start, bilingual_str& error, std::vector<bilingual_str>& warnings)
175231
{
176232
auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(location.GetName()));
177233
if (!result.second) {
178234
error = Untranslated("Wallet already being loading.");
179235
return nullptr;
180236
}
181-
auto wallet = LoadWalletInternal(chain, location, error, warnings);
237+
auto wallet = LoadWalletInternal(chain, location, load_on_start, error, warnings);
182238
WITH_LOCK(g_loading_wallet_mutex, g_loading_wallet_set.erase(result.first));
183239
return wallet;
184240
}
185241

186-
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result)
242+
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, Optional<bool> load_on_start, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result)
187243
{
188244
// Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted
189245
bool create_blank = (wallet_creation_flags & WALLET_FLAG_BLANK_WALLET);
@@ -254,6 +310,10 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString&
254310
AddWallet(wallet);
255311
wallet->postInitProcess();
256312
result = wallet;
313+
314+
// Write the wallet settings
315+
UpdateWalletSetting(chain, name, load_on_start, warnings);
316+
257317
return WalletCreationStatus::SUCCESS;
258318
}
259319

‎src/wallet/wallet.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,11 @@ struct bilingual_str;
5050
void UnloadWallet(std::shared_ptr<CWallet>&& wallet);
5151

5252
bool AddWallet(const std::shared_ptr<CWallet>& wallet);
53-
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet);
53+
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, Optional<bool> load_on_start, std::vector<bilingual_str>& warnings);
54+
bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, Optional<bool> load_on_start);
5455
std::vector<std::shared_ptr<CWallet>> GetWallets();
5556
std::shared_ptr<CWallet> GetWallet(const std::string& name);
56-
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector<bilingual_str>& warnings);
57+
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, Optional<bool> load_on_start, bilingual_str& error, std::vector<bilingual_str>& warnings);
5758
std::unique_ptr<interfaces::Handler> HandleLoadWallet(LoadWalletFn load_wallet);
5859

5960
enum class WalletCreationStatus {
@@ -62,7 +63,7 @@ enum class WalletCreationStatus {
6263
ENCRYPTION_FAILED
6364
};
6465

65-
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result);
66+
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, Optional<bool> load_on_start, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result);
6667

6768
//! -paytxfee default
6869
constexpr CAmount DEFAULT_PAY_TX_FEE = 0;
@@ -1340,4 +1341,11 @@ class WalletRescanReserver
13401341
// be IsAllFromMe).
13411342
int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet);
13421343
int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector<CTxOut>& txouts, bool use_max_sig = false);
1344+
1345+
//! Add wallet name to persistent configuration so it will be loaded on startup.
1346+
bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
1347+
1348+
//! Remove wallet name from persistent configuration so it will not be loaded on startup.
1349+
bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name);
1350+
13431351
#endif // BITCOIN_WALLET_WALLET_H

0 commit comments

Comments
 (0)