Do not create default wallet

No longer create a default wallet. The default wallet will still be
loaded if it exists and not other wallets were specified (anywhere,
including settings.json, bitcoin.conf, and command line).

Tests are updated to be started with -wallet= if they need the default
wallet.

Added test to wallet_startup.py testing that no default wallet is
created and that it is loaded if it exists and no other wallets were
specified.

doc/release-notes-15454.md

@@ -0,0 +1,6 @@
+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.

src/interfaces/wallet.cpp

@@ -488,8 +488,7 @@ public:
 class WalletClientImpl : public WalletClient
 {
 public:
-    WalletClientImpl(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames)
-        : m_wallet_filenames(std::move(wallet_filenames))
+    WalletClientImpl(Chain& chain, ArgsManager& args)
     {
         m_context.chain = &chain;
         m_context.args = &args;
@@ -506,8 +505,8 @@ public:
             m_rpc_handlers.emplace_back(m_context.chain->handleRpc(m_rpc_commands.back()));
         }
     }
-    bool verify() override { return VerifyWallets(*m_context.chain, m_wallet_filenames); }
-    bool load() override { return LoadWallets(*m_context.chain, m_wallet_filenames); }
+    bool verify() override { return VerifyWallets(*m_context.chain); }
+    bool load() override { return LoadWallets(*m_context.chain); }
     void start(CScheduler& scheduler) override { return StartWallets(scheduler, *Assert(m_context.args)); }
     void flush() override { return FlushWallets(); }
     void stop() override { return StopWallets(); }
@@ -566,9 +565,9 @@ public:
 
 std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet) { return wallet ? MakeUnique<WalletImpl>(wallet) : nullptr; }
 
-std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames)
+std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args)
 {
-    return MakeUnique<WalletClientImpl>(chain, args, std::move(wallet_filenames));
+    return MakeUnique<WalletClientImpl>(chain, args);
 }
 
 } // namespace interfaces

src/interfaces/wallet.h

@@ -411,7 +411,7 @@ std::unique_ptr<Wallet> MakeWallet(const std::shared_ptr<CWallet>& wallet);
 
 //! Return implementation of ChainClient interface for a wallet client. This
 //! function will be undefined in builds where ENABLE_WALLET is false.
-std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args, std::vector<std::string> wallet_filenames);
+std::unique_ptr<WalletClient> MakeWalletClient(Chain& chain, ArgsManager& args);
 
 } // namespace interfaces
 

src/wallet/init.cpp

@@ -107,16 +107,7 @@ void WalletInit::Construct(NodeContext& node) const
         LogPrintf("Wallet disabled!\n");
         return;
     }
-    // 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;
-        });
-    }
-    auto wallet_client = interfaces::MakeWalletClient(*node.chain, args, args.GetArgs("-wallet"));
+    auto wallet_client = interfaces::MakeWalletClient(*node.chain, args);
     node.wallet_client = wallet_client.get();
     node.chain_clients.emplace_back(std::move(wallet_client));
 }

src/wallet/load.cpp

@@ -16,7 +16,7 @@
 
 #include <univalue.h>
 
-bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
+bool VerifyWallets(interfaces::Chain& chain)
 {
     if (gArgs.IsArgSet("-walletdir")) {
         fs::path wallet_dir = gArgs.GetArg("-walletdir", "");
@@ -41,10 +41,27 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
 
     chain.initMessage(_("Verifying wallet(s)...").translated);
 
+    // For backwards compatibility if an unnamed top level wallet exists in the
+    // wallets directory, include it in the default list of wallets to load.
+    if (!gArgs.IsArgSet("wallet")) {
+        DatabaseOptions options;
+        DatabaseStatus status;
+        bilingual_str error_string;
+        options.require_existing = true;
+        options.verify = false;
+        if (MakeWalletDatabase("", options, status, error_string)) {
+            gArgs.LockSettings([&](util::Settings& settings) {
+                util::SettingsValue wallets(util::SettingsValue::VARR);
+                wallets.push_back(""); // Default wallet name is ""
+                settings.rw_settings["wallet"] = wallets;
+            });
+        }
+    }
+
     // Keep track of each wallet absolute path to detect duplicates.
     std::set<fs::path> wallet_paths;
 
-    for (const auto& wallet_file : wallet_files) {
+    for (const auto& wallet_file : gArgs.GetArgs("-wallet")) {
         const fs::path path = fs::absolute(wallet_file, GetWalletDir());
 
         if (!wallet_paths.insert(path).second) {
@@ -65,10 +82,10 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
     return true;
 }
 
-bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
+bool LoadWallets(interfaces::Chain& chain)
 {
     try {
-        for (const std::string& name : wallet_files) {
+        for (const std::string& name : gArgs.GetArgs("-wallet")) {
             DatabaseOptions options;
             DatabaseStatus status;
             options.verify = false; // No need to verify, assuming verified earlier in VerifyWallets()

src/wallet/load.h

@@ -17,10 +17,10 @@ class Chain;
 } // namespace interfaces
 
 //! Responsible for reading and validating the -wallet arguments and verifying the wallet database.
-bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files);
+bool VerifyWallets(interfaces::Chain& chain);
 
 //! Load wallet databases.
-bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files);
+bool LoadWallets(interfaces::Chain& chain);
 
 //! Complete startup of wallets.
 void StartWallets(CScheduler& scheduler, const ArgsManager& args);

src/wallet/test/init_test_fixture.cpp

@@ -10,7 +10,7 @@
 
 InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName) : BasicTestingSetup(chainName)
 {
-    m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args), {});
+    m_wallet_client = MakeWalletClient(*m_chain, *Assert(m_node.args));
 
     std::string sep;
     sep += fs::path::preferred_separator;

src/wallet/test/wallet_test_fixture.h

@@ -21,7 +21,7 @@ struct WalletTestingSetup : public TestingSetup {
     explicit WalletTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
 
     std::unique_ptr<interfaces::Chain> m_chain = interfaces::MakeChain(m_node);
-    std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {});
+    std::unique_ptr<interfaces::WalletClient> m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args));
     CWallet m_wallet;
     std::unique_ptr<interfaces::Handler> m_chain_notifications_handler;
 };

test/functional/feature_backwards_compatibility.py

@@ -36,12 +36,12 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         self.num_nodes = 6
         # Add new version after each release:
         self.extra_args = [
-            ["-addresstype=bech32"], # Pre-release: use to mine blocks
+            ["-addresstype=bech32", "-wallet="], # Pre-release: use to mine blocks
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # Pre-release: use to receive coins, swap wallets, etc
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.19.1
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.18.1
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.17.2
-            ["-nowallet", "-walletrbf=1", "-addresstype=bech32"], # v0.16.3
+            ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-wallet=wallet.dat"], # v0.16.3
         ]
 
     def skip_test_if_missing_module(self):

test/functional/feature_fee_estimation.py

@@ -145,9 +145,9 @@ class EstimateFeeTest(BitcoinTestFramework):
         # mine non-standard txs (e.g. txs with "dust" outputs)
         # Force fSendTrickle to true (via whitelist.noban)
         self.extra_args = [
-            ["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1"],
-            ["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=68000"],
-            ["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=32000"],
+            ["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-wallet="],
+            ["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=68000", "-wallet="],
+            ["-acceptnonstdtxn", "-whitelist=noban@127.0.0.1", "-blockmaxweight=32000", "-wallet="],
         ]
 
     def skip_test_if_missing_module(self):

test/functional/feature_filelock.py

@@ -15,7 +15,7 @@ class FilelockTest(BitcoinTestFramework):
 
     def setup_network(self):
         self.add_nodes(self.num_nodes, extra_args=None)
-        self.nodes[0].start([])
+        self.nodes[0].start(['-wallet='])
         self.nodes[0].wait_for_rpc_connection()
 
     def run_test(self):
@@ -30,7 +30,7 @@ class FilelockTest(BitcoinTestFramework):
             wallet_dir = os.path.join(datadir, 'wallets')
             self.log.info("Check that we can't start a second bitcoind instance using the same wallet")
             expected_msg = "Error: Error initializing wallet database environment"
-            self.nodes[1].assert_start_raises_init_error(extra_args=['-walletdir={}'.format(wallet_dir), '-noserver'], expected_msg=expected_msg, match=ErrorMatch.PARTIAL_REGEX)
+            self.nodes[1].assert_start_raises_init_error(extra_args=['-walletdir={}'.format(wallet_dir), '-wallet=', '-noserver'], expected_msg=expected_msg, match=ErrorMatch.PARTIAL_REGEX)
 
 if __name__ == '__main__':
     FilelockTest().main()

test/functional/mempool_compatibility.py

@@ -31,7 +31,7 @@ class MempoolCompatibilityTest(BitcoinTestFramework):
             150200, # oldest version supported by the test framework
             None,
         ])
-        self.start_nodes()
+        self.start_nodes([[], ["-wallet="]])
         self.import_deterministic_coinbase_privkeys()
 
     def run_test(self):

test/functional/wallet_backup.py

@@ -50,10 +50,10 @@ class WalletBackupTest(BitcoinTestFramework):
         # nodes 1, 2,3 are spenders, let's give them a keypool=100
         # whitelist all peers to speed up tx relay / mempool sync
         self.extra_args = [
-            ["-whitelist=noban@127.0.0.1", "-keypool=100"],
-            ["-whitelist=noban@127.0.0.1", "-keypool=100"],
-            ["-whitelist=noban@127.0.0.1", "-keypool=100"],
-            ["-whitelist=noban@127.0.0.1"],
+            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
+            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
+            ["-whitelist=noban@127.0.0.1", "-keypool=100", "-wallet="],
+            ["-whitelist=noban@127.0.0.1", "-wallet="],
         ]
         self.rpc_timeout = 120
 

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"]]
+        self.extra_args = [["-keypool=90", "-addresstype=legacy", "-wallet=dump"]]
         self.rpc_timeout = 120
 
     def skip_test_if_missing_module(self):

test/functional/wallet_import_rescan.py

@@ -151,7 +151,7 @@ class ImportRescanTest(BitcoinTestFramework):
         self.skip_if_no_wallet()
 
     def setup_network(self):
-        self.extra_args = [[] for _ in range(self.num_nodes)]
+        self.extra_args = [["-wallet="] for _ in range(self.num_nodes)]
         for i, import_node in enumerate(IMPORT_NODES, 2):
             if import_node.prune:
                 self.extra_args[i] += ["-prune=1"]
@@ -159,7 +159,7 @@ class ImportRescanTest(BitcoinTestFramework):
         self.add_nodes(self.num_nodes, extra_args=self.extra_args)
 
         # Import keys with pruning disabled
-        self.start_nodes(extra_args=[[]] * self.num_nodes)
+        self.start_nodes(extra_args=[["-wallet="]] * self.num_nodes)
         for n in self.nodes:
             n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase')
         self.stop_nodes()

test/functional/wallet_multiwallet.py

@@ -43,6 +43,7 @@ class MultiWalletTest(BitcoinTestFramework):
         self.setup_clean_chain = True
         self.num_nodes = 2
         self.rpc_timeout = 120
+        self.extra_args = [["-wallet="], ["-wallet="]]
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
@@ -82,7 +83,7 @@ class MultiWalletTest(BitcoinTestFramework):
         os.rename(wallet_dir("wallet.dat"), wallet_dir("w8"))
 
         # create another dummy wallet for use in testing backups later
-        self.start_node(0, [])
+        self.start_node(0, ["-wallet="])
         self.stop_nodes()
         empty_wallet = os.path.join(self.options.tmpdir, 'empty.dat')
         os.rename(wallet_dir("wallet.dat"), empty_wallet)
@@ -152,7 +153,7 @@ class MultiWalletTest(BitcoinTestFramework):
 
         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, ['-walletdir=' + competing_wallet_dir, '-wallet='])
         exp_stderr = r"Error: Error initializing wallet database environment \"\S+competing_walletdir\"!"
         self.nodes[1].assert_start_raises_init_error(['-walletdir=' + competing_wallet_dir], exp_stderr, match=ErrorMatch.PARTIAL_REGEX)
 

test/functional/wallet_startup.py

@@ -26,6 +26,16 @@ class WalletStartupTest(BitcoinTestFramework):
         self.start_nodes()
 
     def run_test(self):
+        self.log.info('Should start without any wallets')
+        assert_equal(self.nodes[0].listwallets(), [])
+        assert_equal(self.nodes[0].listwalletdir(), {'wallets': []})
+
+        self.log.info('New default wallet should load by default when there are no other wallets')
+        self.nodes[0].createwallet(wallet_name='', load_on_startup=False)
+        self.restart_node(0)
+        assert_equal(self.nodes[0].listwallets(), [''])
+
+        self.log.info('Test load on startup behavior')
         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)

test/functional/wallet_upgradewallet.py

@@ -27,7 +27,7 @@ class UpgradeWalletTest(BitcoinTestFramework):
         self.setup_clean_chain = True
         self.num_nodes = 3
         self.extra_args = [
-            ["-addresstype=bech32"], # current wallet version
+            ["-addresstype=bech32", "-wallet="], # current wallet version
             ["-usehd=1"],            # v0.16.3 wallet
             ["-usehd=0"]             # v0.15.2 wallet
         ]