From 1bee1e6269b76b52b1eab9112d39c245beaa27a2 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 22 Jun 2020 17:58:18 -0400 Subject: [PATCH 1/2] 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 | 6 +++++ src/interfaces/wallet.cpp | 11 ++++---- src/interfaces/wallet.h | 2 +- src/wallet/init.cpp | 11 +------- src/wallet/load.cpp | 25 ++++++++++++++++--- src/wallet/load.h | 4 +-- src/wallet/test/init_test_fixture.cpp | 2 +- src/wallet/test/wallet_test_fixture.h | 2 +- .../feature_backwards_compatibility.py | 4 +-- test/functional/feature_fee_estimation.py | 6 ++--- test/functional/feature_filelock.py | 4 +-- test/functional/mempool_compatibility.py | 2 +- test/functional/wallet_backup.py | 8 +++--- test/functional/wallet_dump.py | 2 +- test/functional/wallet_import_rescan.py | 4 +-- test/functional/wallet_multiwallet.py | 5 ++-- test/functional/wallet_startup.py | 10 ++++++++ test/functional/wallet_upgradewallet.py | 2 +- 18 files changed, 67 insertions(+), 43 deletions(-) create mode 100644 doc/release-notes-15454.md diff --git a/doc/release-notes-15454.md b/doc/release-notes-15454.md new file mode 100644 index 00000000000..00c847a8d29 --- /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 d19d0406b6a..94c63da84e3 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 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 MakeWallet(const std::shared_ptr& wallet) { return wallet ? MakeUnique(wallet) : nullptr; } -std::unique_ptr MakeWalletClient(Chain& chain, ArgsManager& args, std::vector wallet_filenames) +std::unique_ptr MakeWalletClient(Chain& chain, ArgsManager& args) { - return MakeUnique(chain, args, std::move(wallet_filenames)); + return MakeUnique(chain, args); } } // namespace interfaces diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index b1afbbfd7ce..6ccfd7fc205 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -411,7 +411,7 @@ std::unique_ptr MakeWallet(const std::shared_ptr& 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 MakeWalletClient(Chain& chain, ArgsManager& args, std::vector wallet_filenames); +std::unique_ptr MakeWalletClient(Chain& chain, ArgsManager& args); } // namespace interfaces diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 3910599ca7a..5d8c4fba292 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 c5d045e9ef6..1b057000d26 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -16,7 +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", ""); @@ -41,10 +41,27 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector& 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 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& wal 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; 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 ff4f5b4b23d..e12343de274 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& 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/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index c23dea1338b..c80310045ad 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 12ad13b49bc..ba8a5ff1f3a 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 m_chain = interfaces::MakeChain(m_node); - std::unique_ptr m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args), {}); + std::unique_ptr m_wallet_client = interfaces::MakeWalletClient(*m_chain, *Assert(m_node.args)); CWallet m_wallet; std::unique_ptr m_chain_notifications_handler; }; diff --git a/test/functional/feature_backwards_compatibility.py b/test/functional/feature_backwards_compatibility.py index dd17e888f93..daefb161ac7 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 702a1d99953..d89eeec400d 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 b56ffe179f7..e4ceb62c94c 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 ad29d389e2a..fd3dd47e2d0 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 47663553356..bcbac18d578 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 06f01ef1913..09581d864b9 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 4ff7f1d5259..87deaded095 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 d64d3dcb494..aaf050ebf79 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 cfc4edb8ee0..d3119925f75 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 69aa603c6be..031da8da818 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 ] From d26f0648f1c0d1115dcb8d76e57195032b88f400 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Wed, 20 Feb 2019 13:35:58 -0500 Subject: [PATCH 2/2] Tell users how to load or create a wallet when no wallet is loaded --- src/qt/bitcoingui.cpp | 5 +++++ src/qt/bitcoingui.h | 1 + src/qt/walletframe.cpp | 25 +++++++++++++++++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index aa58c0b10e7..0c2dcc3584e 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -660,6 +660,11 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller) } } +WalletController* BitcoinGUI::getWalletController() +{ + return m_wallet_controller; +} + void BitcoinGUI::addWallet(WalletModel* walletModel) { if (!walletFrame) return; diff --git a/src/qt/bitcoingui.h b/src/qt/bitcoingui.h index 4c55f286932..912297a74ee 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -79,6 +79,7 @@ public: void setClientModel(ClientModel *clientModel = nullptr, interfaces::BlockAndHeaderTipInfo* tip_info = nullptr); #ifdef ENABLE_WALLET void setWalletController(WalletController* wallet_controller); + WalletController* getWalletController(); #endif #ifdef ENABLE_WALLET diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index ec56f2755fc..f16761d6b2e 100644 --- a/src/qt/walletframe.cpp +++ b/src/qt/walletframe.cpp @@ -2,6 +2,8 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include +#include #include #include @@ -10,8 +12,11 @@ #include +#include #include #include +#include +#include WalletFrame::WalletFrame(const PlatformStyle *_platformStyle, BitcoinGUI *_gui) : QFrame(_gui), @@ -25,9 +30,25 @@ WalletFrame::WalletFrame(const PlatformStyle *_platformStyle, BitcoinGUI *_gui) walletFrameLayout->setContentsMargins(0,0,0,0); walletFrameLayout->addWidget(walletStack); - QLabel *noWallet = new QLabel(tr("No wallet has been loaded.")); + // hbox for no wallet + QGroupBox* no_wallet_group = new QGroupBox(walletStack); + QVBoxLayout* no_wallet_layout = new QVBoxLayout(no_wallet_group); + + QLabel *noWallet = new QLabel(tr("No wallet has been loaded.\nGo to File > Open Wallet to load a wallet.\n- OR -")); noWallet->setAlignment(Qt::AlignCenter); - walletStack->addWidget(noWallet); + no_wallet_layout->addWidget(noWallet, 0, Qt::AlignHCenter | Qt::AlignBottom); + + // A button for create wallet dialog + QPushButton* create_wallet_button = new QPushButton(tr("Create a new wallet"), walletStack); + connect(create_wallet_button, &QPushButton::clicked, [this] { + auto activity = new CreateWalletActivity(gui->getWalletController(), this); + connect(activity, &CreateWalletActivity::finished, activity, &QObject::deleteLater); + activity->create(); + }); + no_wallet_layout->addWidget(create_wallet_button, 0, Qt::AlignHCenter | Qt::AlignTop); + no_wallet_group->setLayout(no_wallet_layout); + + walletStack->addWidget(no_wallet_group); } WalletFrame::~WalletFrame()