wallet: Make -wallet setting not create wallets

doc/release-notes.md

@@ -292,15 +292,18 @@ Wallet
   changed from `-32601` (method not found) to `-18` (wallet not found).
   (#20101)
 
-### Default Wallet
-
-Bitcoin Core will no longer create an unnamed `""` wallet by default when no
-wallet is specified on the command line or in the configuration files. For
-backwards compatibility, if an unnamed `""` wallet already exists and would
-have been loaded previously, then it will still be loaded. Users without an
-unnamed `""` wallet and without any other wallets to be loaded on startup will
-be prompted to either choose a wallet to load, or to create a new wallet.
-(#15454)
+### Automatic wallet creation removed
+
+Bitcoin Core will no longer automatically create new wallets on startup. It will
+load existing wallets specified by `-wallet` options on the command line or in
+`bitcoin.conf` or `settings.json` files. And by default it will also load a
+top-level unnamed ("") wallet. However, if specified wallets don't exist,
+Bitcoin Core will now just log warnings instead of creating new wallets with
+new keys and addresses like previous releases did.
+
+New wallets can be created through the GUI (which has a more prominent create
+wallet option), through the `bitcoin-cli createwallet` or `bitcoin-wallet
+create` commands, or the `createwallet` RPC. (#15454)
 
 ### Experimental Descriptor Wallets
 

src/wallet/init.cpp

@@ -60,7 +60,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddArg("-rescan", "Rescan the block chain for missing wallet transactions on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-spendzeroconfchange", strprintf("Spend unconfirmed change when sending transactions (default: %u)", DEFAULT_SPEND_ZEROCONF_CHANGE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-txconfirmtarget=<n>", strprintf("If paytxfee is not set, include enough fee so transactions begin confirmation on average within n blocks (default: %u)", DEFAULT_TX_CONFIRM_TARGET), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
-    argsman.AddArg("-wallet=<path>", "Specify wallet database path. Can be specified multiple times to load multiple wallets. Path is interpreted relative to <walletdir> if it is not absolute, and will be created if it does not exist (as a directory containing a wallet.dat file and log files). For backwards compatibility this will also accept names of existing data files in <walletdir>.)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET);
+    argsman.AddArg("-wallet=<path>", "Specify wallet path to load at startup. Can be used multiple times to load multiple wallets. Path is to a directory containing wallet data and log files. If the path is not absolute, it is interpreted relative to <walletdir>. This only loads existing wallets and does not create new ones. For backwards compatibility this also accepts names of existing top-level data files in <walletdir>.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET);
     argsman.AddArg("-walletbroadcast",  strprintf("Make the wallet broadcast transactions (default: %u)", DEFAULT_WALLETBROADCAST), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-walletdir=<dir>", "Specify directory to hold wallets (default: <datadir>/wallets if it exists, otherwise <datadir>)", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET);
 #if HAVE_SYSTEM

src/wallet/load.cpp

@@ -71,11 +71,16 @@ bool VerifyWallets(interfaces::Chain& chain)
 
         DatabaseOptions options;
         DatabaseStatus status;
+        options.require_existing = true;
         options.verify = true;
         bilingual_str error_string;
         if (!MakeWalletDatabase(wallet_file, options, status, error_string)) {
-            chain.initError(error_string);
-            return false;
+            if (status == DatabaseStatus::FAILED_NOT_FOUND) {
+                chain.initWarning(Untranslated(strprintf("Skipping -wallet path that doesn't exist. %s\n", error_string.original)));
+            } else {
+                chain.initError(error_string);
+                return false;
+            }
         }
     }
 
@@ -88,10 +93,14 @@ bool LoadWallets(interfaces::Chain& chain)
         for (const std::string& name : gArgs.GetArgs("-wallet")) {
             DatabaseOptions options;
             DatabaseStatus status;
+            options.require_existing = true;
             options.verify = false; // No need to verify, assuming verified earlier in VerifyWallets()
             bilingual_str error;
             std::vector<bilingual_str> warnings;
             std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(name, options, status, error);
+            if (!database && status == DatabaseStatus::FAILED_NOT_FOUND) {
+                continue;
+            }
             std::shared_ptr<CWallet> pwallet = database ? CWallet::Create(chain, name, std::move(database), options.create_flags, error, warnings) : nullptr;
             if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n")));
             if (!pwallet) {

test/functional/feature_config_args.py

@@ -179,19 +179,15 @@ def run_test(self):
 
         # Create the directory and ensure the config file now works
         os.mkdir(new_data_dir)
-        self.start_node(0, ['-conf='+conf_file, '-wallet=w1'])
+        self.start_node(0, ['-conf='+conf_file])
         self.stop_node(0)
         assert os.path.exists(os.path.join(new_data_dir, self.chain, 'blocks'))
-        if self.is_wallet_compiled():
-            assert os.path.exists(os.path.join(new_data_dir, self.chain, 'wallets', 'w1'))
 
         # Ensure command line argument overrides datadir in conf
         os.mkdir(new_data_dir_2)
         self.nodes[0].datadir = new_data_dir_2
-        self.start_node(0, ['-datadir='+new_data_dir_2, '-conf='+conf_file, '-wallet=w2'])
+        self.start_node(0, ['-datadir='+new_data_dir_2, '-conf='+conf_file])
         assert os.path.exists(os.path.join(new_data_dir_2, self.chain, 'blocks'))
-        if self.is_wallet_compiled():
-            assert os.path.exists(os.path.join(new_data_dir_2, self.chain, 'wallets', 'w2'))
 
 
 if __name__ == '__main__':

test/functional/rpc_scantxoutset.py

@@ -55,7 +55,8 @@ def run_test(self):
         self.log.info("Stop node, remove wallet, mine again some blocks...")
         self.stop_node(0)
         shutil.rmtree(os.path.join(self.nodes[0].datadir, self.chain, 'wallets'))
-        self.start_node(0)
+        self.start_node(0, ['-nowallet'])
+        self.import_deterministic_coinbase_privkeys()
         self.nodes[0].generate(110)
 
         scan = self.nodes[0].scantxoutset("start", [])

test/functional/test_framework/test_framework.py

@@ -111,6 +111,7 @@ def __init__(self):
         # are not imported.
         self.wallet_names = None
         self.set_test_params()
+        assert self.wallet_names is None or len(self.wallet_names) <= self.num_nodes
         if self.options.timeout_factor == 0 :
             self.options.timeout_factor = 99999
         self.rpc_timeout = int(self.rpc_timeout * self.options.timeout_factor) # optionally, increase timeout by a factor
@@ -390,9 +391,13 @@ def setup_nodes(self):
                 assert_equal(chain_info["initialblockdownload"], False)
 
     def import_deterministic_coinbase_privkeys(self):
-        wallet_names = [self.default_wallet_name] * len(self.nodes) if self.wallet_names is None else self.wallet_names
-        assert len(wallet_names) <= len(self.nodes)
-        for wallet_name, n in zip(wallet_names, self.nodes):
+        for i in range(self.num_nodes):
+            self.init_wallet(i)
+
+    def init_wallet(self, i):
+        wallet_name = self.default_wallet_name if self.wallet_names is None else self.wallet_names[i] if i < len(self.wallet_names) else False
+        if wallet_name is not False:
+            n = self.nodes[i]
             if wallet_name is not None:
                 n.createwallet(wallet_name=wallet_name, descriptors=self.options.descriptors, load_on_startup=True)
             n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase')

test/functional/tool_wallet.py

@@ -218,7 +218,8 @@ def test_getwalletinfo_on_different_wallet(self):
     def test_salvage(self):
         # TODO: Check salvage actually salvages and doesn't break things. https://github.com/bitcoin/bitcoin/issues/7463
         self.log.info('Check salvage')
-        self.start_node(0, ['-wallet=salvage'])
+        self.start_node(0)
+        self.nodes[0].createwallet("salvage")
         self.stop_node(0)
 
         self.assert_tool_output('', '-wallet=salvage', 'salvage')

test/functional/wallet_backup.py

@@ -91,10 +91,10 @@ def do_one_round(self):
         self.sync_blocks()
 
     # As above, this mirrors the original bash test.
-    def start_three(self):
-        self.start_node(0)
-        self.start_node(1)
-        self.start_node(2)
+    def start_three(self, args=()):
+        self.start_node(0, self.extra_args[0] + list(args))
+        self.start_node(1, self.extra_args[1] + list(args))
+        self.start_node(2, self.extra_args[2] + list(args))
         self.connect_nodes(0, 3)
         self.connect_nodes(1, 3)
         self.connect_nodes(2, 3)
@@ -110,6 +110,11 @@ def erase_three(self):
         os.remove(os.path.join(self.nodes[1].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename))
         os.remove(os.path.join(self.nodes[2].datadir, self.chain, 'wallets', self.default_wallet_name, self.wallet_data_filename))
 
+    def init_three(self):
+        self.init_wallet(0)
+        self.init_wallet(1)
+        self.init_wallet(2)
+
     def run_test(self):
         self.log.info("Generating initial blockchain")
         self.nodes[0].generate(1)
@@ -193,7 +198,8 @@ def run_test(self):
             shutil.rmtree(os.path.join(self.nodes[2].datadir, self.chain, 'blocks'))
             shutil.rmtree(os.path.join(self.nodes[2].datadir, self.chain, 'chainstate'))
 
-            self.start_three()
+            self.start_three(["-nowallet"])
+            self.init_three()
 
             assert_equal(self.nodes[0].getbalance(), 0)
             assert_equal(self.nodes[1].getbalance(), 0)

test/functional/wallet_dump.py

@@ -95,7 +95,7 @@ def read_dump(file_name, addrs, script_addrs, hd_master_addr_old):
 class WalletDumpTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
-        self.extra_args = [["-keypool=90", "-addresstype=legacy", "-wallet=dump"]]
+        self.extra_args = [["-keypool=90", "-addresstype=legacy"]]
         self.rpc_timeout = 120
 
     def skip_test_if_missing_module(self):
@@ -106,6 +106,8 @@ def setup_network(self):
         self.start_nodes()
 
     def run_test(self):
+        self.nodes[0].createwallet("dump")
+
         wallet_unenc_dump = os.path.join(self.nodes[0].datadir, "wallet.unencrypted.dump")
         wallet_enc_dump = os.path.join(self.nodes[0].datadir, "wallet.encrypted.dump")
 
@@ -190,7 +192,8 @@ def run_test(self):
         assert_raises_rpc_error(-8, "already exists", lambda: self.nodes[0].dumpwallet(wallet_enc_dump))
 
         # Restart node with new wallet, and test importwallet
-        self.restart_node(0, ['-wallet=w2'])
+        self.restart_node(0)
+        self.nodes[0].createwallet("w2")
 
         # Make sure the address is not IsMine before import
         result = self.nodes[0].getaddressinfo(multisig_addr)

test/functional/wallet_multiwallet.py

@@ -41,6 +41,7 @@ def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 2
         self.rpc_timeout = 120
+        self.extra_args = [["-nowallet"], []]
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -80,7 +81,9 @@ def wallet_file(name):
         # rename wallet.dat to make sure plain wallet file paths (as opposed to
         # directory paths) can be loaded
         # create another dummy wallet for use in testing backups later
-        self.start_node(0, ["-nowallet", "-wallet=empty", "-wallet=plain"])
+        self.start_node(0)
+        node.createwallet("empty", descriptors=False)
+        node.createwallet("plain", descriptors=False)
         node.createwallet("created")
         self.stop_nodes()
         empty_wallet = os.path.join(self.options.tmpdir, 'empty.dat')
@@ -101,21 +104,23 @@ def wallet_file(name):
         #   w8         - to verify existing wallet file is loaded correctly
         #   ''         - to verify default wallet file is created correctly
         wallet_names = ['w1', 'w2', 'w3', 'w', 'sub/w5', os.path.join(self.options.tmpdir, 'extern/w6'), 'w7_symlink', 'w8', self.default_wallet_name]
-        extra_args = ['-nowallet'] + ['-wallet={}'.format(n) for n in wallet_names]
-        self.start_node(0, extra_args)
+        self.start_node(0)
+        for wallet_name in wallet_names[:-2]:
+            self.nodes[0].createwallet(wallet_name, descriptors=False)
+        for wallet_name in wallet_names[-2:]:
+            self.nodes[0].loadwallet(wallet_name)
         assert_equal(sorted(map(lambda w: w['name'], self.nodes[0].listwalletdir()['wallets'])), [self.default_wallet_name, os.path.join('sub', 'w5'), 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink', 'w8'])
 
         assert_equal(set(node.listwallets()), set(wallet_names))
 
+        # should raise rpc error if wallet path can't be created
+        assert_raises_rpc_error(-1, "boost::filesystem::create_directory:", self.nodes[0].createwallet, "w8/bad", descriptors=False)
+
         # check that all requested wallets were created
         self.stop_node(0)
         for wallet_name in wallet_names:
             assert_equal(os.path.isfile(wallet_file(wallet_name)), True)
 
-        # should not initialize if wallet path can't be created
-        exp_stderr = "boost::filesystem::create_directory:"
-        self.nodes[0].assert_start_raises_init_error(['-wallet=w8/bad'], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
-
         self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
         self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
         self.nodes[0].assert_start_raises_init_error(['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())
@@ -142,26 +147,33 @@ def wallet_file(name):
         # if wallets/ doesn't exist, datadir should be the default wallet dir
         wallet_dir2 = data_dir('walletdir')
         os.rename(wallet_dir(), wallet_dir2)
-        self.start_node(0, ['-nowallet', '-wallet=w4', '-wallet=w5'])
+        self.start_node(0)
+        self.nodes[0].createwallet("w4")
+        self.nodes[0].createwallet("w5")
         assert_equal(set(node.listwallets()), {"w4", "w5"})
         w5 = wallet("w5")
         node.generatetoaddress(nblocks=1, address=w5.getnewaddress())
 
         # now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded
         os.rename(wallet_dir2, wallet_dir())
-        self.restart_node(0, ['-nowallet', '-wallet=w4', '-wallet=w5', '-walletdir=' + data_dir()])
+        self.restart_node(0, ['-nowallet', '-walletdir=' + data_dir()])
+        self.nodes[0].loadwallet("w4")
+        self.nodes[0].loadwallet("w5")
         assert_equal(set(node.listwallets()), {"w4", "w5"})
         w5 = wallet("w5")
         w5_info = w5.getwalletinfo()
         assert_equal(w5_info['immature_balance'], 50)
 
         competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir')
         os.mkdir(competing_wallet_dir)
-        self.restart_node(0, ['-walletdir=' + competing_wallet_dir])
+        self.restart_node(0, ['-nowallet', '-walletdir=' + competing_wallet_dir])
+        self.nodes[0].createwallet(self.default_wallet_name, descriptors=False)
         exp_stderr = r"Error: Error initializing wallet database environment \"\S+competing_walletdir\S*\"!"
         self.nodes[1].assert_start_raises_init_error(['-walletdir=' + competing_wallet_dir], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
 
-        self.restart_node(0, extra_args)
+        self.restart_node(0)
+        for wallet_name in wallet_names:
+            self.nodes[0].loadwallet(wallet_name)
 
         assert_equal(sorted(map(lambda w: w['name'], self.nodes[0].listwalletdir()['wallets'])), [self.default_wallet_name, os.path.join('sub', 'w5'), 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink', 'w8', 'w8_copy'])