From 7f3c8c399e18cc4e0d42408a401fce670f0a8b97 Mon Sep 17 00:00:00 2001 From: PastaPastaPasta Date: Tue, 21 Jun 2022 19:00:55 +0530 Subject: [PATCH] merge #15937: Add loadwallet and createwallet load_on_startup options Co-authored-by: UdjinM6 --- doc/release-notes-15937.md | 12 ++++++++ src/interfaces/chain.cpp | 21 ++++++++++++++ src/interfaces/chain.h | 7 +++++ src/qt/rpcconsole.cpp | 15 ++++++++-- src/rpc/client.cpp | 4 +++ src/wallet/init.cpp | 11 ++++++- src/wallet/load.cpp | 25 ++++++++++++++++ src/wallet/load.h | 6 ++++ src/wallet/rpcwallet.cpp | 40 ++++++++++++++++++++++---- test/functional/test_runner.py | 1 + test/functional/wallet_startup.py | 48 +++++++++++++++++++++++++++++++ 11 files changed, 181 insertions(+), 9 deletions(-) create mode 100644 doc/release-notes-15937.md create mode 100755 test/functional/wallet_startup.py diff --git a/doc/release-notes-15937.md b/doc/release-notes-15937.md new file mode 100644 index 0000000000..ec7d355dfa --- /dev/null +++ b/doc/release-notes-15937.md @@ -0,0 +1,12 @@ +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. + +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. diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 72148d1c5a..0ec655aa6e 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -360,6 +360,27 @@ public: { RPCRunLater(name, std::move(fn), seconds); } + util::SettingsValue getRwSetting(const std::string& name) override + { + util::SettingsValue result; + gArgs.LockSettings([&](const util::Settings& settings) { + if (const util::SettingsValue* value = util::FindKey(settings.rw_settings, name)) { + result = *value; + } + }); + return result; + } + bool updateRwSetting(const std::string& name, const util::SettingsValue& value) override + { + gArgs.LockSettings([&](util::Settings& settings) { + if (value.isNull()) { + settings.rw_settings.erase(name); + } else { + settings.rw_settings[name] = value; + } + }); + return gArgs.WriteSettingsFile(); + } void requestMempoolTransactions(Notifications& notifications) override { LOCK2(::cs_main, ::mempool.cs); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index bfa4a8e86c..1b417e7c57 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -7,6 +7,7 @@ #include // For Optional and nullopt #include // For CTransactionRef +#include // For util::SettingsValue #include #include @@ -244,6 +245,12 @@ public: //! Run function after given number of seconds. Cancel any previous calls with same name. virtual void rpcRunLater(const std::string& name, std::function fn, int64_t seconds) = 0; + //! Return /settings.json setting value. + virtual util::SettingsValue getRwSetting(const std::string& name) = 0; + + //! Write a setting to /settings.json. + virtual bool updateRwSetting(const std::string& name, const util::SettingsValue& value) = 0; + //! Synchronously send TransactionAddedToMempool notifications about all //! current mempool transactions to the specified handler and return after //! the last one is sent. These notifications aren't coordinated with async diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index d13489212c..8b0333d14d 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -27,6 +27,7 @@ #ifdef ENABLE_WALLET #include +#include #endif #include @@ -499,8 +500,18 @@ RPCConsole::RPCConsole(interfaces::Node& node, QWidget* parent, Qt::WindowFlags connect(ui->btn_reindex, &QPushButton::clicked, this, &RPCConsole::walletReindex); #ifdef ENABLE_WALLET - std::string walletPath = GetDataDir().string(); - walletPath += QDir::separator().toLatin1() + gArgs.GetArg("-wallet", "wallet.dat"); + // If there's no -wallet setting with a list of wallets to load, set it to + // load the default "" wallet. + if (!gArgs.IsArgSet("wallet")) { + gArgs.LockSettings([&](util::Settings& settings) { + util::SettingsValue wallets(util::SettingsValue::VARR); + wallets.push_back(""); // Default wallet name is "" + settings.rw_settings["wallet"] = wallets; + }); + } + std::string walletName = gArgs.GetArgs("-wallet").at(0); + std::string walletPath = GetWalletDir().string(); + walletPath += QDir::separator().toLatin1() + (walletName == "" ? "wallet.dat" : walletName); ui->wallet_path->setText(QString::fromStdString(walletPath)); #endif diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index a383d1c7dd..1914d02908 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -200,6 +200,10 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createwallet", 2, "blank"}, { "createwallet", 4, "avoid_reuse"}, { "upgradetohd", 3, "rescan"}, + { "createwallet", 5, "load_on_startup"}, + { "loadwallet", 1, "load_on_startup"}, + { "unloadwallet", 1, "load_on_startup"}, + { "upgradetohd", 3, "rescan"}, { "getnodeaddresses", 0, "count"}, { "stop", 0, "wait" }, }; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index d83ecd927e..4082cdf9d0 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -162,7 +163,15 @@ void WalletInit::Construct(NodeContext& node) const LogPrintf("Wallet disabled!\n"); return; } - gArgs.SoftSetArg("-wallet", ""); + // If there's no -wallet setting with a list of wallets to load, set it to + // load the default "" wallet. + if (!gArgs.IsArgSet("wallet")) { + gArgs.LockSettings([&](util::Settings& settings) { + util::SettingsValue wallets(util::SettingsValue::VARR); + wallets.push_back(""); // Default wallet name is "" + settings.rw_settings["wallet"] = wallets; + }); + } node.chain_clients.emplace_back(interfaces::MakeWalletClient(*node.chain, gArgs.GetArgs("-wallet"))); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 57d19ee83d..2dbbabde5d 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -16,6 +16,8 @@ #include #include +#include + bool VerifyWallets(interfaces::Chain& chain, const std::vector& wallet_files) { if (gArgs.IsArgSet("-walletdir")) { @@ -129,3 +131,26 @@ void UnloadWallets() 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 ddab317ce6..927efe5790 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -34,4 +34,10 @@ 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 895cd917c9..46c301c92a 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -31,6 +31,7 @@ #include #include #include +#include #include #include #include @@ -160,6 +161,18 @@ 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", @@ -2647,6 +2660,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) "\napplied to the new wallet (eg -upgradewallet, rescan, etc).\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."}, + {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -2679,6 +2693,8 @@ static UniValue loadwallet(const JSONRPCRequest& request) std::shared_ptr const wallet = LoadWallet(*context.chain, location, 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); @@ -2763,6 +2779,7 @@ static UniValue createwallet(const JSONRPCRequest& request) {"blank", RPCArg::Type::BOOL, /* default */ "false", "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed."}, {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Encrypt the wallet with this passphrase."}, {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "false", "Keep track of coin reuse, and treat dirty and clean coins differently with privacy considerations in mind."}, + {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -2814,6 +2831,8 @@ 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); @@ -2828,8 +2847,11 @@ static UniValue unloadwallet(const JSONRPCRequest& request) "Specifying the wallet name on a wallet endpoint is invalid.", { {"wallet_name", RPCArg::Type::STR, /* default */ "the wallet name from the RPC request", "The name of the wallet to unload."}, + {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."}, }, - RPCResult{RPCResult::Type::NONE, "", ""}, + RPCResult{RPCResult::Type::OBJ, "", "", { + {RPCResult::Type::STR, "warning", "Warning message if wallet was not unloaded cleanly."}, + }}, RPCExamples{ HelpExampleCli("unloadwallet", "wallet_name") + HelpExampleRpc("unloadwallet", "wallet_name") @@ -2857,9 +2879,15 @@ static UniValue unloadwallet(const JSONRPCRequest& request) throw JSONRPCError(RPC_MISC_ERROR, "Requested wallet already unloaded"); } - UnloadWallet(std::move(wallet)); + interfaces::Chain& chain = wallet->chain(); + std::vector warnings; - return NullUniValue; + UnloadWallet(std::move(wallet)); + UpdateWalletSetting(chain, wallet_name, request.params[1], warnings); + + UniValue result(UniValue::VOBJ); + result.pushKV("warning", Join(warnings, Untranslated("\n")).original); + return result; } static UniValue listunspent(const JSONRPCRequest& request) @@ -3932,7 +3960,7 @@ static const CRPCCommand commands[] = { "wallet", "abortrescan", &abortrescan, {} }, { "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","label"} }, { "wallet", "backupwallet", &backupwallet, {"destination"} }, - { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse"} }, + { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "load_on_startup"} }, { "wallet", "dumphdinfo", &dumphdinfo, {} }, { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, @@ -3966,7 +3994,7 @@ static const CRPCCommand commands[] = { "wallet", "listunspent", &listunspent, {"minconf","maxconf","addresses","include_unsafe","query_options"} }, { "wallet", "listwalletdir", &listwalletdir, {} }, { "wallet", "listwallets", &listwallets, {} }, - { "wallet", "loadwallet", &loadwallet, {"filename"} }, + { "wallet", "loadwallet", &loadwallet, {"filename", "load_on_startup"} }, { "wallet", "lockunspent", &lockunspent, {"unlock","transactions"} }, { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, @@ -3979,7 +4007,7 @@ static const CRPCCommand commands[] = { "wallet", "setwalletflag", &setwalletflag, {"flag","value"} }, { "wallet", "signmessage", &signmessage, {"address","message"} }, { "wallet", "signrawtransactionwithwallet", &signrawtransactionwithwallet, {"hexstring","prevtxs","sighashtype"} }, - { "wallet", "unloadwallet", &unloadwallet, {"wallet_name"} }, + { "wallet", "unloadwallet", &unloadwallet, {"wallet_name", "load_on_startup"} }, { "wallet", "upgradetohd", &upgradetohd, {"mnemonic", "mnemonicpassphrase", "walletpassphrase", "rescan"} }, { "wallet", "walletlock", &walletlock, {} }, { "wallet", "walletpassphrasechange", &walletpassphrasechange, {"oldpassphrase","newpassphrase"} }, diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 1909871dec..e8c17a1009 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -247,6 +247,7 @@ BASE_SCRIPTS = [ 'p2p_node_network_limited.py', 'p2p_permissions.py', 'feature_blocksdir.py', + 'wallet_startup.py', 'feature_config_args.py', 'feature_settings.py', 'rpc_help.py', diff --git a/test/functional/wallet_startup.py b/test/functional/wallet_startup.py new file mode 100755 index 0000000000..9de7457f17 --- /dev/null +++ b/test/functional/wallet_startup.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test wallet load on startup. + +Verify that a dashd node can maintain list of wallets loading on startup +""" +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, +) + + +class WalletStartupTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + self.supports_cli = True + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def setup_nodes(self): + self.add_nodes(self.num_nodes) + self.start_nodes() + + def run_test(self): + self.nodes[0].createwallet(wallet_name='w0', load_on_startup=True) + self.nodes[0].createwallet(wallet_name='w1', load_on_startup=False) + self.nodes[0].createwallet(wallet_name='w2', load_on_startup=True) + self.nodes[0].createwallet(wallet_name='w3', load_on_startup=False) + self.nodes[0].createwallet(wallet_name='w4', load_on_startup=False) + self.nodes[0].unloadwallet(wallet_name='w0', load_on_startup=False) + self.nodes[0].unloadwallet(wallet_name='w4', load_on_startup=False) + self.nodes[0].loadwallet(filename='w4', load_on_startup=True) + assert_equal(set(self.nodes[0].listwallets()), set(('', 'w1', 'w2', 'w3', 'w4'))) + self.restart_node(0) + assert_equal(set(self.nodes[0].listwallets()), set(('', 'w2', 'w4'))) + self.nodes[0].unloadwallet(wallet_name='', load_on_startup=False) + self.nodes[0].unloadwallet(wallet_name='w4', load_on_startup=False) + self.nodes[0].loadwallet(filename='w3', load_on_startup=True) + self.nodes[0].loadwallet(filename='') + self.restart_node(0) + assert_equal(set(self.nodes[0].listwallets()), set(('w2', 'w3'))) + +if __name__ == '__main__': + WalletStartupTest().main()