diff options
author | Andrew Chow <achow101-github@achow101.com> | 2020-06-22 17:58:18 -0400 |
---|---|---|
committer | Andrew Chow <achow101-github@achow101.com> | 2020-09-08 21:02:53 -0400 |
commit | 1bee1e6269b76b52b1eab9112d39c245beaa27a2 (patch) | |
tree | 1b437fa47cf89317a767c7e5ae25087283d5d28a | |
parent | 78cb45d72251e85db07e8500bbdd2e9460b132b2 (diff) | |
download | bitcoin-1bee1e6269b76b52b1eab9112d39c245beaa27a2.tar.xz |
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.
-rw-r--r-- | doc/release-notes-15454.md | 6 | ||||
-rw-r--r-- | src/interfaces/wallet.cpp | 11 | ||||
-rw-r--r-- | src/interfaces/wallet.h | 2 | ||||
-rw-r--r-- | src/wallet/init.cpp | 11 | ||||
-rw-r--r-- | src/wallet/load.cpp | 25 | ||||
-rw-r--r-- | src/wallet/load.h | 4 | ||||
-rw-r--r-- | src/wallet/test/init_test_fixture.cpp | 2 | ||||
-rw-r--r-- | src/wallet/test/wallet_test_fixture.h | 2 | ||||
-rwxr-xr-x | test/functional/feature_backwards_compatibility.py | 4 | ||||
-rwxr-xr-x | test/functional/feature_fee_estimation.py | 6 | ||||
-rwxr-xr-x | test/functional/feature_filelock.py | 4 | ||||
-rwxr-xr-x | test/functional/mempool_compatibility.py | 2 | ||||
-rwxr-xr-x | test/functional/wallet_backup.py | 8 | ||||
-rwxr-xr-x | test/functional/wallet_dump.py | 2 | ||||
-rwxr-xr-x | test/functional/wallet_import_rescan.py | 4 | ||||
-rwxr-xr-x | test/functional/wallet_multiwallet.py | 5 | ||||
-rwxr-xr-x | test/functional/wallet_startup.py | 10 | ||||
-rwxr-xr-x | test/functional/wallet_upgradewallet.py | 2 |
18 files changed, 67 insertions, 43 deletions
diff --git a/doc/release-notes-15454.md b/doc/release-notes-15454.md new file mode 100644 index 0000000000..00c847a8d2 --- /dev/null +++ b/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. diff --git a/src/interfaces/wallet.cpp b/src/interfaces/wallet.cpp index d19d0406b6..94c63da84e 100644 --- a/src/interfaces/wallet.cpp +++ b/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 diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index b1afbbfd7c..6ccfd7fc20 100644 --- a/src/interfaces/wallet.h +++ b/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 diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 3910599ca7..5d8c4fba29 100644 --- a/src/wallet/init.cpp +++ b/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)); } diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index c5d045e9ef..1b057000d2 100644 --- a/src/wallet/load.cpp +++ b/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() diff --git a/src/wallet/load.h b/src/wallet/load.h index ff4f5b4b23..e12343de27 100644 --- a/src/wallet/load.h +++ b/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); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index c23dea1338..c80310045a 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/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; diff --git a/src/wallet/test/wallet_test_fixture.h b/src/wallet/test/wallet_test_fixture.h index 12ad13b49b..ba8a5ff1f3 100644 --- a/src/wallet/test/wallet_test_fixture.h +++ b/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; }; diff --git a/test/functional/feature_backwards_compatibility.py b/test/functional/feature_backwards_compatibility.py index dd17e888f9..daefb161ac 100755 --- a/test/functional/feature_backwards_compatibility.py +++ b/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): diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 702a1d9995..d89eeec400 100755 --- a/test/functional/feature_fee_estimation.py +++ b/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): diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index b56ffe179f..e4ceb62c94 100755 --- a/test/functional/feature_filelock.py +++ b/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() diff --git a/test/functional/mempool_compatibility.py b/test/functional/mempool_compatibility.py index ad29d389e2..fd3dd47e2d 100755 --- a/test/functional/mempool_compatibility.py +++ b/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): diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 4766355335..bcbac18d57 100755 --- a/test/functional/wallet_backup.py +++ b/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 diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index 06f01ef191..09581d864b 100755 --- a/test/functional/wallet_dump.py +++ b/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): diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 4ff7f1d525..87deaded09 100755 --- a/test/functional/wallet_import_rescan.py +++ b/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() diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index d64d3dcb49..aaf050ebf7 100755 --- a/test/functional/wallet_multiwallet.py +++ b/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) diff --git a/test/functional/wallet_startup.py b/test/functional/wallet_startup.py index cfc4edb8ee..d3119925f7 100755 --- a/test/functional/wallet_startup.py +++ b/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) diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py index 69aa603c6b..031da8da81 100755 --- a/test/functional/wallet_upgradewallet.py +++ b/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 ] |