diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -12,6 +12,12 @@ - The `-zapwallettxes` startup option has been removed and its functionality removed from the wallet. This functionality has been superseded with the abandon transaction feature. +- Bitcoin ABC 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. Configuration ------------- diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -411,9 +411,7 @@ //! Return implementation of ChainClient interface for a wallet client. This //! function will be undefined in builds where ENABLE_WALLET is false. -std::unique_ptr -MakeWalletClient(Chain &chain, ArgsManager &args, - std::vector wallet_filenames); +std::unique_ptr MakeWalletClient(Chain &chain, ArgsManager &args); } // namespace interfaces diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -464,9 +464,7 @@ class WalletClientImpl : public WalletClient { public: - WalletClientImpl(Chain &chain, ArgsManager &args, - std::vector wallet_filenames) - : m_wallet_filenames(std::move(wallet_filenames)) { + WalletClientImpl(Chain &chain, ArgsManager &args) { m_context.chain = &chain; m_context.args = &args; } @@ -493,12 +491,8 @@ registerRpcs(GetWalletRPCCommands()); registerRpcs(GetWalletDumpRPCCommands()); } - 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)); } @@ -565,11 +559,9 @@ return wallet ? std::make_unique(wallet) : nullptr; } -std::unique_ptr -MakeWalletClient(Chain &chain, ArgsManager &args, - std::vector wallet_filenames) { - return std::make_unique(chain, args, - std::move(wallet_filenames)); +std::unique_ptr MakeWalletClient(Chain &chain, + ArgsManager &args) { + return std::make_unique(chain, args); } } // namespace interfaces diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -205,17 +205,7 @@ 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)); } diff --git a/src/wallet/load.h b/src/wallet/load.h --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -18,12 +18,10 @@ //! Responsible for reading and validating the -wallet arguments and verifying //! the wallet database. -bool VerifyWallets(interfaces::Chain &chain, - const std::vector &wallet_files); +bool VerifyWallets(interfaces::Chain &chain); //! Load wallet databases. -bool LoadWallets(interfaces::Chain &chain, - const std::vector &wallet_files); +bool LoadWallets(interfaces::Chain &chain); //! Complete startup of wallets. void StartWallets(CScheduler &scheduler, const ArgsManager &args); diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -16,8 +16,7 @@ #include -bool VerifyWallets(interfaces::Chain &chain, - const std::vector &wallet_files) { +bool VerifyWallets(interfaces::Chain &chain) { if (gArgs.IsArgSet("-walletdir")) { fs::path wallet_dir = gArgs.GetArg("-walletdir", ""); boost::system::error_code error; @@ -49,10 +48,28 @@ 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); + // Default wallet name is "" + wallets.push_back(""); + settings.rw_settings["wallet"] = wallets; + }); + } + } + // Keep track of each wallet absolute path to detect duplicates. std::set 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) { @@ -75,10 +92,9 @@ return true; } -bool LoadWallets(interfaces::Chain &chain, - const std::vector &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; // No need to verify, assuming verified earlier in VerifyWallets() diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -13,7 +13,7 @@ const std::string &chainName) : BasicTestingSetup(chainName) { m_chain = interfaces::MakeChain(m_node, Params()); - 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; diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h --- a/src/wallet/test/wallet_test_fixture.h +++ b/src/wallet/test/wallet_test_fixture.h @@ -24,7 +24,7 @@ std::unique_ptr m_chain; std::unique_ptr m_wallet_client = - interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); + interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args)); CWallet m_wallet; std::unique_ptr m_chain_notifications_handler; }; diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py --- a/test/functional/feature_filelock.py +++ b/test/functional/feature_filelock.py @@ -16,7 +16,7 @@ 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): @@ -38,6 +38,7 @@ self.nodes[1].assert_start_raises_init_error( extra_args=[ '-walletdir={}'.format(wallet_dir), + '-wallet=', '-noserver'], expected_msg=expected_msg, match=ErrorMatch.PARTIAL_REGEX) diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -50,10 +50,10 @@ # 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 diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -84,7 +84,7 @@ class WalletDumpTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - self.extra_args = [["-keypool=90"]] + self.extra_args = [["-keypool=90", "-wallet=dump"]] self.rpc_timeout = 120 def skip_test_if_missing_module(self): diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -152,7 +152,7 @@ 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"] @@ -160,7 +160,7 @@ 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, diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -45,6 +45,7 @@ def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 2 + self.extra_args = [["-wallet="], ["-wallet="]] def skip_test_if_missing_module(self): self.skip_if_no_wallet() @@ -91,7 +92,7 @@ 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) @@ -178,7 +179,8 @@ 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) diff --git a/test/functional/wallet_startup.py b/test/functional/wallet_startup.py --- a/test/functional/wallet_startup.py +++ b/test/functional/wallet_startup.py @@ -24,6 +24,17 @@ 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)