From b9ff147c51dbcf18fccb67247b647468cf80aa75 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 1 May 2019 15:12:44 -0400 Subject: [PATCH] Add loadwallet and createwallet RPC load_on_startup options This maintains a persistent list of wallets stored in settings that will automatically be loaded on startup. Being able to load a wallet automatically on startup will be more useful in the GUI when the option to create wallets is added in #15006, but it's reasonable to expose this feature by RPC as well. --- doc/release-notes-15937.md | 12 +++++ src/interfaces/chain.cpp | 21 +++++++++ src/interfaces/chain.h | 7 +++ src/rpc/client.cpp | 3 ++ src/wallet/init.cpp | 11 ++++- src/wallet/load.cpp | 25 +++++++++++ src/wallet/load.h | 6 +++ src/wallet/rpcwallet.cpp | 38 +++++++++++++--- test/functional/test_framework/test_node.py | 4 +- test/functional/test_runner.py | 1 + test/functional/wallet_startup.py | 50 +++++++++++++++++++++ 11 files changed, 170 insertions(+), 8 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 0000000000000..b25049a327568 --- /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 d49e4454af591..313c1265dec28 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -372,6 +372,27 @@ class ChainImpl : public Chain RPCRunLater(name, std::move(fn), seconds); } int rpcSerializationFlags() override { return RPCSerializationFlags(); } + 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 bbeb0fa801357..053d40335f2af 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 @@ -269,6 +270,12 @@ class Chain //! Current RPC serialization flags. virtual int rpcSerializationFlags() = 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/rpc/client.cpp b/src/rpc/client.cpp index 66ace7263a742..c2d73e07bf11e 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -172,6 +172,9 @@ static const CRPCConvertParam vRPCConvertParams[] = { "createwallet", 2, "blank"}, { "createwallet", 4, "avoid_reuse"}, { "createwallet", 5, "descriptors"}, + { "createwallet", 6, "load_on_startup"}, + { "loadwallet", 1, "load_on_startup"}, + { "unloadwallet", 1, "load_on_startup"}, { "getnodeaddresses", 0, "count"}, { "stop", 0, "wait" }, }; diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 781920755cc64..0b5daa752a66d 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -118,6 +119,14 @@ void WalletInit::Construct(NodeContext& node) const LogPrintf("Wallet disabled!\n"); return; } - args.SoftSetArg("-wallet", ""); + // If there's no -wallet setting with a list of wallets to load, set it to + // load the default "" wallet. + if (!args.IsArgSet("wallet")) { + args.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, args, args.GetArgs("-wallet"))); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 2a81d30133dce..ae14769edbd9a 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -13,6 +13,8 @@ #include #include +#include + bool VerifyWallets(interfaces::Chain& chain, const std::vector& wallet_files) { if (gArgs.IsArgSet("-walletdir")) { @@ -120,3 +122,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 ff4f5b4b23da6..30f1a4c90d56d 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 9d334063c4c6f..973ffc54f4eed 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include @@ -229,6 +230,18 @@ static void SetFeeEstimateMode(const CWallet* pwallet, CCoinControl& cc, const U } } +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.")); + } +} + static UniValue getnewaddress(const JSONRPCRequest& request) { RPCHelpMan{"getnewaddress", @@ -2483,6 +2496,7 @@ static UniValue loadwallet(const JSONRPCRequest& request) "\napplied to the new wallet (eg -zapwallettxes, 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, "", "", @@ -2515,6 +2529,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); @@ -2599,6 +2615,7 @@ static UniValue createwallet(const JSONRPCRequest& request) {"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."}, {"descriptors", RPCArg::Type::BOOL, /* default */ "false", "Create a native descriptor wallet. The wallet will use descriptors internally to handle address creation"}, + {"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, "", "", @@ -2654,6 +2671,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); @@ -2668,8 +2687,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") @@ -2697,9 +2719,15 @@ static UniValue unloadwallet(const JSONRPCRequest& request) 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); - return NullUniValue; + UniValue result(UniValue::VOBJ); + result.pushKV("warning", Join(warnings, Untranslated("\n")).original); + return result; } static UniValue listunspent(const JSONRPCRequest& request) @@ -4137,7 +4165,7 @@ static const CRPCCommand commands[] = { "wallet", "addmultisigaddress", &addmultisigaddress, {"nrequired","keys","label","address_type"} }, { "wallet", "backupwallet", &backupwallet, {"destination"} }, { "wallet", "bumpfee", &bumpfee, {"txid", "options"} }, - { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "descriptors"} }, + { "wallet", "createwallet", &createwallet, {"wallet_name", "disable_private_keys", "blank", "passphrase", "avoid_reuse", "descriptors", "load_on_startup"} }, { "wallet", "dumpprivkey", &dumpprivkey, {"address"} }, { "wallet", "dumpwallet", &dumpwallet, {"filename"} }, { "wallet", "encryptwallet", &encryptwallet, {"passphrase"} }, @@ -4170,7 +4198,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"} }, @@ -4182,7 +4210,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", "upgradewallet", &upgradewallet, {"version"} }, { "wallet", "walletcreatefundedpsbt", &walletcreatefundedpsbt, {"inputs","outputs","locktime","options","bip32derivs"} }, { "wallet", "walletlock", &walletlock, {} }, diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 66bb2c89b56b7..8b30bdfa892ac 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -649,10 +649,10 @@ def __init__(self, rpc, cli=False, descriptors=False): def __getattr__(self, name): return getattr(self.rpc, name) - def createwallet(self, wallet_name, disable_private_keys=None, blank=None, passphrase='', avoid_reuse=None, descriptors=None): + def createwallet(self, wallet_name, disable_private_keys=None, blank=None, passphrase='', avoid_reuse=None, descriptors=None, load_on_startup=None): if descriptors is None: descriptors = self.descriptors - return self.__getattr__('createwallet')(wallet_name, disable_private_keys, blank, passphrase, avoid_reuse, descriptors) + return self.__getattr__('createwallet')(wallet_name, disable_private_keys, blank, passphrase, avoid_reuse, descriptors, load_on_startup) def importprivkey(self, privkey, label=None, rescan=None): wallet_info = self.getwalletinfo() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 95c2b7c5ec85c..6f1b9d308cdff 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -242,6 +242,7 @@ 'p2p_node_network_limited.py', 'p2p_permissions.py', 'feature_blocksdir.py', + 'wallet_startup.py', 'feature_config_args.py', 'feature_settings.py', 'rpc_getdescriptorinfo.py', diff --git a/test/functional/wallet_startup.py b/test/functional/wallet_startup.py new file mode 100755 index 0000000000000..7dfe987a996a1 --- /dev/null +++ b/test/functional/wallet_startup.py @@ -0,0 +1,50 @@ +#!/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 bitcoind 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.stop_nodes() + self.start_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='', load_on_startup=False) + self.stop_nodes() + self.start_node(0) + assert_equal(set(self.nodes[0].listwallets()), set(('w2', 'w3'))) + +if __name__ == '__main__': + WalletStartupTest().main()