Merge #15937: Add loadwallet and createwallet load_on_startup options

642ad31b41 Add loadwallet and createwallet RPC load_on_startup options (Russell Yanofsky)

Pull request description:

  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, but it's reasonable to expose this feature by RPC as well.

ACKs for top commit:
  achow101:
    re-ACK 642ad31b41 Only change is the test
  meshcollider:
    re-utACK 642ad31b41

Tree-SHA512: cca0b71bf1a83ad071830e6c459f1cd979b4add7144e899ec560da72b5910dd9bf9426e5c7d125ae96fad8990fbf81a76bc83c0459486c16086ada6cbde5eaa3

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
+`\<datadir\>/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.

src/interfaces/chain.cpp

@@ -372,6 +372,27 @@ public:
         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);

src/interfaces/chain.h

@@ -7,6 +7,7 @@
 
 #include <optional.h>               // For Optional and nullopt
 #include <primitives/transaction.h> // For CTransactionRef
+#include <util/settings.h>          // For util::SettingsValue
 
 #include <functional>
 #include <memory>
@@ -269,6 +270,12 @@ public:
     //! Current RPC serialization flags.
     virtual int rpcSerializationFlags() = 0;
 
+    //! Return <datadir>/settings.json setting value.
+    virtual util::SettingsValue getRwSetting(const std::string& name) = 0;
+
+    //! Write a setting to <datadir>/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

src/rpc/client.cpp

@@ -173,6 +173,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"},
     { "addpeeraddress", 1, "port"},
     { "stop", 0, "wait" },

src/wallet/init.cpp

@@ -9,6 +9,7 @@
 #include <node/context.h>
 #include <node/ui_interface.h>
 #include <outputtype.h>
+#include <univalue.h>
 #include <util/check.h>
 #include <util/moneystr.h>
 #include <util/system.h>
@@ -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")));
 }

src/wallet/load.cpp

@@ -13,6 +13,8 @@
 #include <wallet/wallet.h>
 #include <wallet/walletdb.h>
 
+#include <univalue.h>
+
 bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& 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);
+}

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

src/wallet/rpcwallet.cpp

@@ -30,6 +30,7 @@
 #include <wallet/coincontrol.h>
 #include <wallet/context.h>
 #include <wallet/feebumper.h>
+#include <wallet/load.h>
 #include <wallet/rpcwallet.h>
 #include <wallet/wallet.h>
 #include <wallet/walletdb.h>
@@ -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<bilingual_str>& 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",
@@ -2484,6 +2497,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, "", "",
@@ -2516,6 +2530,8 @@ static UniValue loadwallet(const JSONRPCRequest& request)
     std::shared_ptr<CWallet> 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);
@@ -2600,6 +2616,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, "", "",
@@ -2655,6 +2672,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);
@@ -2669,8 +2688,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")
@@ -2698,9 +2720,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<bilingual_str> 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)
@@ -4158,7 +4186,7 @@ static const CRPCCommand commands[] =
     { "wallet",             "backupwallet",                     &backupwallet,                  {"destination"} },
     { "wallet",             "bumpfee",                          &bumpfee,                       {"txid", "options"} },
     { "wallet",             "psbtbumpfee",                      &psbtbumpfee,                   {"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"} },
@@ -4191,7 +4219,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"} },
@@ -4203,7 +4231,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,                    {} },

test/functional/test_framework/test_node.py

@@ -650,10 +650,10 @@ class RPCOverloadWrapper():
     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()

test/functional/test_runner.py

@@ -244,6 +244,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_getdescriptorinfo.py',

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 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.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()