diff --git a/doc/release-notes-15937.md b/doc/release-notes-15937.md index ec7d355dfa..1ab817b0e5 100644 --- a/doc/release-notes-15937.md +++ b/doc/release-notes-15937.md @@ -1,12 +1,15 @@ Configuration ------------- -The `createwallet`, `loadwallet`, and `unloadwallet` RPCs now accept -`load_on_startup` options that modify bitcoin's dynamic configuration in -`\/settings.json`, and can add or remove a wallet from the list of -wallets automatically loaded at startup. Unless these options are explicitly -set to true or false, the load on startup wallet list is not modified, so this -change is backwards compatible. +Wallets created or loaded in the GUI will now be automatically loaded on +startup, so they don't need to be manually reloaded next time Bitcoin is +started. The list of wallets to load on startup is stored in +`\/settings.json` and augments any command line or `bitcoin.conf` +`-wallet=` settings that specify more wallets to load. Wallets that are +unloaded in the GUI get removed from the settings list so they won't load again +automatically next startup. (#19754) -In the future, the GUI will start updating the same startup wallet list as the -RPCs to automatically reopen wallets previously opened in the GUI. +The `createwallet`, `loadwallet`, and `unloadwallet` RPCs now accept +`load_on_startup` options to modify the settings list. Unless these options are +explicitly set to true or false, the list is not modified, so the RPC methods +remain backwards compatible. (#15937) diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index 73287f6939..66ceed7445 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -506,7 +506,7 @@ public: CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; } void remove() override { - RemoveWallet(m_wallet); + RemoveWallet(m_wallet, false /* load_on_start */); } std::unique_ptr handleUnload(UnloadFn fn) override { @@ -586,12 +586,12 @@ public: std::unique_ptr createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, WalletCreationStatus& status, bilingual_str& error, std::vector& warnings) override { std::shared_ptr wallet; - status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet); + status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, true /* load_on_start */, error, warnings, wallet); return MakeWallet(std::move(wallet)); } std::unique_ptr loadWallet(const std::string& name, bilingual_str& error, std::vector& warnings) override { - return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), error, warnings)); + return MakeWallet(LoadWallet(*m_context.chain, WalletLocation(name), true /* load_on_start */, error, warnings)); } std::string getWalletDir() override { diff --git a/src/qt/test/addressbooktests.cpp b/src/qt/test/addressbooktests.cpp index af644ed110..31dad56d1a 100644 --- a/src/qt/test/addressbooktests.cpp +++ b/src/qt/test/addressbooktests.cpp @@ -106,7 +106,7 @@ void TestAddAddressesToSendBook(interfaces::Node& node) OptionsModel optionsModel(node); AddWallet(wallet); WalletModel walletModel(interfaces::MakeWallet(wallet), node, &optionsModel); - RemoveWallet(wallet); + RemoveWallet(wallet, nullopt); EditAddressDialog editAddressDialog(EditAddressDialog::NewSendingAddress); editAddressDialog.setModel(walletModel.getAddressTableModel()); diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp index 7aaf4b24f1..2a903e5fb5 100644 --- a/src/qt/test/wallettests.cpp +++ b/src/qt/test/wallettests.cpp @@ -209,7 +209,7 @@ void TestGUI(interfaces::Node& node) QPushButton* removeRequestButton = receiveCoinsDialog.findChild("removeRequestButton"); removeRequestButton->click(); QCOMPARE(requestTableModel->rowCount({}), currentRowCount-1); - RemoveWallet(wallet); + RemoveWallet(wallet, nullopt); } } // namespace diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 2dbbabde5d..dc45e0a347 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -127,30 +127,8 @@ void UnloadWallets() while (!wallets.empty()) { auto wallet = wallets.back(); wallets.pop_back(); - RemoveWallet(wallet); + std::vector warnings; + RemoveWallet(wallet, nullopt, warnings); UnloadWallet(std::move(wallet)); } } - -bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) -{ - util::SettingsValue setting_value = chain.getRwSetting("wallet"); - if (!setting_value.isArray()) setting_value.setArray(); - for (const util::SettingsValue& value : setting_value.getValues()) { - if (value.isStr() && value.get_str() == wallet_name) return true; - } - setting_value.push_back(wallet_name); - return chain.updateRwSetting("wallet", setting_value); -} - -bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) -{ - util::SettingsValue setting_value = chain.getRwSetting("wallet"); - if (!setting_value.isArray()) return true; - util::SettingsValue new_value(util::SettingsValue::VARR); - for (const util::SettingsValue& value : setting_value.getValues()) { - if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value); - } - if (new_value.size() == setting_value.size()) return true; - return chain.updateRwSetting("wallet", new_value); -} diff --git a/src/wallet/load.h b/src/wallet/load.h index 927efe5790..ddab317ce6 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -34,10 +34,4 @@ void StopWallets(); //! Close all wallets. void UnloadWallets(); -//! Add wallet name to persistent configuration so it will be loaded on startup. -bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); - -//! Remove wallet name from persistent configuration so it will not be loaded on startup. -bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); - #endif // BITCOIN_WALLET_LOAD_H diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 46c301c92a..84a5321274 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -161,18 +161,6 @@ static std::string LabelFromValue(const UniValue& value) return label; } -static void UpdateWalletSetting(interfaces::Chain& chain, - const std::string& wallet_name, - const UniValue& load_on_startup, - std::vector& warnings) -{ - if (load_on_startup.isTrue() && !AddWalletSetting(chain, wallet_name)) { - warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may not be loaded next node startup.")); - } else if (load_on_startup.isFalse() && !RemoveWalletSetting(chain, wallet_name)) { - warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may still be loaded next node startup.")); - } -} - UniValue getnewaddress(const JSONRPCRequest& request) { RPCHelpMan{"getnewaddress", @@ -2690,11 +2678,10 @@ static UniValue loadwallet(const JSONRPCRequest& request) bilingual_str error; std::vector warnings; - std::shared_ptr const wallet = LoadWallet(*context.chain, location, error, warnings); + Optional load_on_start = request.params[1].isNull() ? nullopt : Optional(request.params[1].get_bool()); + std::shared_ptr const wallet = LoadWallet(*context.chain, location, load_on_start, error, warnings); if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original); - UpdateWalletSetting(*context.chain, location.GetName(), request.params[1], warnings); - UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); @@ -2820,7 +2807,8 @@ static UniValue createwallet(const JSONRPCRequest& request) bilingual_str error; std::shared_ptr wallet; - WalletCreationStatus status = CreateWallet(*context.chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet); + Optional load_on_start = request.params[5].isNull() ? nullopt : Optional(request.params[5].get_bool()); + WalletCreationStatus status = CreateWallet(*context.chain, passphrase, flags, request.params[0].get_str(), load_on_start, error, warnings, wallet); switch (status) { case WalletCreationStatus::CREATION_FAILED: throw JSONRPCError(RPC_WALLET_ERROR, error.original); @@ -2831,8 +2819,6 @@ static UniValue createwallet(const JSONRPCRequest& request) // no default case, so the compiler can warn about missing cases } - UpdateWalletSetting(*context.chain, request.params[0].get_str(), request.params[6], warnings); - UniValue obj(UniValue::VOBJ); obj.pushKV("name", wallet->GetName()); obj.pushKV("warning", Join(warnings, Untranslated("\n")).original); @@ -2875,15 +2861,13 @@ static UniValue unloadwallet(const JSONRPCRequest& request) // Release the "main" shared pointer and prevent further notifications. // Note that any attempt to load the same wallet would fail until the wallet // is destroyed (see CheckUniqueFileid). - if (!RemoveWallet(wallet)) { + std::vector warnings; + Optional load_on_start = request.params[1].isNull() ? nullopt : Optional(request.params[1].get_bool()); + if (!RemoveWallet(wallet, load_on_start, warnings)) { throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); } - interfaces::Chain& chain = wallet->chain(); - std::vector warnings; - UnloadWallet(std::move(wallet)); - UpdateWalletSetting(chain, wallet_name, request.params[1], warnings); UniValue result(UniValue::VOBJ); result.pushKV("warning", Join(warnings, Untranslated("\n")).original); diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index 790a79e4f8..a135e6ff60 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -148,7 +148,7 @@ public: ~CTransactionBuilderTestSetup() { - RemoveWallet(wallet); + RemoveWallet(wallet, nullopt); } std::shared_ptr chain; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index db0c676957..615c337253 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -192,7 +192,7 @@ BOOST_FIXTURE_TEST_CASE(importmulti_rescan, TestChain100Setup) "downloading and rescanning the relevant blocks (see -reindex and -rescan " "options).\"}},{\"success\":true}]", 0, oldTip->GetBlockTimeMax(), TIMESTAMP_WINDOW)); - RemoveWallet(wallet); + RemoveWallet(wallet, nullopt); } } @@ -233,7 +233,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.push_back(backup_file); AddWallet(wallet); ::dumpwallet(request); - RemoveWallet(wallet); + RemoveWallet(wallet, nullopt); } // Call importwallet RPC and verify all blocks with timestamps >= BLOCK_TIME @@ -247,7 +247,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) request.params.push_back(backup_file); AddWallet(wallet); ::importwallet(request); - RemoveWallet(wallet); + RemoveWallet(wallet, nullopt); LOCK(wallet->cs_wallet); BOOST_CHECK_EQUAL(wallet->mapWallet.size(), 3U); @@ -534,7 +534,7 @@ public: ~CreateTransactionTestSetup() { - RemoveWallet(wallet); + RemoveWallet(wallet, nullopt); } std::shared_ptr wallet; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b51d71430a..d390b97bfb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -44,11 +44,12 @@ #include #include +#include + #include #include #include - #include #include @@ -67,6 +68,42 @@ static CCriticalSection cs_wallets; static std::vector> vpwallets GUARDED_BY(cs_wallets); static std::list g_load_wallet_fns GUARDED_BY(cs_wallets); +bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) +{ + util::SettingsValue setting_value = chain.getRwSetting("wallet"); + if (!setting_value.isArray()) setting_value.setArray(); + for (const util::SettingsValue& value : setting_value.getValues()) { + if (value.isStr() && value.get_str() == wallet_name) return true; + } + setting_value.push_back(wallet_name); + return chain.updateRwSetting("wallet", setting_value); +} + +bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name) +{ + util::SettingsValue setting_value = chain.getRwSetting("wallet"); + if (!setting_value.isArray()) return true; + util::SettingsValue new_value(util::SettingsValue::VARR); + for (const util::SettingsValue& value : setting_value.getValues()) { + if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value); + } + if (new_value.size() == setting_value.size()) return true; + return chain.updateRwSetting("wallet", new_value); +} + +static void UpdateWalletSetting(interfaces::Chain& chain, + const std::string& wallet_name, + Optional load_on_startup, + std::vector& warnings) +{ + if (load_on_startup == nullopt) return; + if (load_on_startup.get() && !AddWalletSetting(chain, wallet_name)) { + warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may not be loaded next node startup.")); + } else if (!load_on_startup.get() && !RemoveWalletSetting(chain, wallet_name)) { + warnings.emplace_back(Untranslated("Wallet load on startup setting could not be updated, so wallet may still be loaded next node startup.")); + } +} + bool AddWallet(const std::shared_ptr& wallet) { LOCK(cs_wallets); @@ -78,9 +115,13 @@ bool AddWallet(const std::shared_ptr& wallet) return true; } -bool RemoveWallet(const std::shared_ptr& wallet) +bool RemoveWallet(const std::shared_ptr& wallet, Optional load_on_start, std::vector& warnings) { assert(wallet); + + interfaces::Chain& chain = wallet->chain(); + std::string name = wallet->GetName(); + // Unregister with the validation interface which also drops shared ponters. wallet->m_chain_notifications_handler.reset(); LOCK(cs_wallets); @@ -89,9 +130,19 @@ bool RemoveWallet(const std::shared_ptr& wallet) vpwallets.erase(i); auto it = coinJoinClientManagers.find(wallet->GetName()); coinJoinClientManagers.erase(it); + + // Write the wallet setting + UpdateWalletSetting(chain, name, load_on_start, warnings); + return true; } +bool RemoveWallet(const std::shared_ptr& wallet, Optional load_on_start) +{ + std::vector warnings; + return RemoveWallet(wallet, load_on_start, warnings); +} + std::vector> GetWallets() { LOCK(cs_wallets); @@ -163,7 +214,7 @@ void UnloadWallet(std::shared_ptr&& wallet) } namespace { -std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings) +std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const WalletLocation& location, Optional load_on_start, bilingual_str& error, std::vector& warnings) { try { if (!CWallet::Verify(chain, location, error, warnings)) { @@ -178,6 +229,10 @@ std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const Wall } AddWallet(wallet); wallet->postInitProcess(); + + // Write the wallet setting + UpdateWalletSetting(chain, location.GetName(), load_on_start, warnings); + return wallet; } catch (const std::runtime_error& e) { error = Untranslated(e.what()); @@ -186,19 +241,19 @@ std::shared_ptr LoadWalletInternal(interfaces::Chain& chain, const Wall } } // namespace -std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings) +std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, Optional load_on_start, bilingual_str& error, std::vector& warnings) { auto result = WITH_LOCK(g_loading_wallet_mutex, return g_loading_wallet_set.insert(location.GetName())); if (!result.second) { error = Untranslated("Wallet already being loading."); return nullptr; } - auto wallet = LoadWalletInternal(chain, location, error, warnings); + auto wallet = LoadWalletInternal(chain, location, load_on_start, error, warnings); WITH_LOCK(g_loading_wallet_mutex, g_loading_wallet_set.erase(result.first)); return wallet; } -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector& warnings, std::shared_ptr& result) +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, Optional load_on_start, bilingual_str& error, std::vector& warnings, std::shared_ptr& result) { // Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted bool create_blank = (wallet_creation_flags & WALLET_FLAG_BLANK_WALLET); @@ -265,6 +320,10 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& AddWallet(wallet); wallet->postInitProcess(); result = wallet; + + // Write the wallet settings + UpdateWalletSetting(chain, name, load_on_start, warnings); + return WalletCreationStatus::SUCCESS; } @@ -5069,7 +5128,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, std::shared_ptr walletInstance(new CWallet(&chain, location, CreateWalletDatabase(location.GetPath())), ReleaseWallet); AddWallet(walletInstance); auto unload_wallet = [&](const bilingual_str& strError) { - RemoveWallet(walletInstance); + RemoveWallet(walletInstance, nullopt); error = strError; return nullptr; }; @@ -5077,7 +5136,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, try { nLoadWalletRet = walletInstance->LoadWallet(fFirstRun); } catch (const std::exception& e) { - RemoveWallet(walletInstance); + RemoveWallet(walletInstance, nullopt); throw; } if (nLoadWalletRet != DBErrors::LOAD_OK) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 77ecf85aed..fc6d870539 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -50,10 +50,11 @@ using LoadWalletFn = std::function wall void UnloadWallet(std::shared_ptr&& wallet); bool AddWallet(const std::shared_ptr& wallet); -bool RemoveWallet(const std::shared_ptr& wallet); +bool RemoveWallet(const std::shared_ptr& wallet, Optional load_on_start, std::vector& warnings); +bool RemoveWallet(const std::shared_ptr& wallet, Optional load_on_start); std::vector> GetWallets(); std::shared_ptr GetWallet(const std::string& name); -std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, bilingual_str& error, std::vector& warnings); +std::shared_ptr LoadWallet(interfaces::Chain& chain, const WalletLocation& location, Optional load_on_start, bilingual_str& error, std::vector& warnings); std::unique_ptr HandleLoadWallet(LoadWalletFn load_wallet); enum class WalletCreationStatus { @@ -62,7 +63,7 @@ enum class WalletCreationStatus { ENCRYPTION_FAILED }; -WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector& warnings, std::shared_ptr& result); +WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, Optional load_on_start, bilingual_str& error, std::vector& warnings, std::shared_ptr& result); static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; //! -paytxfee default @@ -1532,4 +1533,11 @@ public: // be IsAllFromMe). int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, bool use_max_sig = false) EXCLUSIVE_LOCKS_REQUIRED(wallet->cs_wallet); int64_t CalculateMaximumSignedTxSize(const CTransaction &tx, const CWallet *wallet, const std::vector& txouts, bool use_max_sig = false); + +//! Add wallet name to persistent configuration so it will be loaded on startup. +bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); + +//! Remove wallet name from persistent configuration so it will not be loaded on startup. +bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name); + #endif // BITCOIN_WALLET_WALLET_H