From b71ee6cbeeabcfc2b5fabb5f1a754e4c8cdb174f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com> Date: Mon, 22 Jun 2020 17:58:18 -0400 Subject: [PATCH] merge bitcoin#15454: Remove the automatic creation and loading of the default wallet Co-authored-by: UdjinM6 --- doc/release-notes-15454.md | 6 ++++++ src/interfaces/wallet.cpp | 11 +++++----- src/interfaces/wallet.h | 2 +- src/qt/bitcoingui.cpp | 5 +++++ src/qt/bitcoingui.h | 1 + src/qt/guiutil.cpp | 2 +- src/qt/walletframe.cpp | 25 +++++++++++++++++++++-- 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 +- src/wallet/wallet.cpp | 3 +++ test/functional/feature_fee_estimation.py | 6 +++--- test/functional/feature_filelock.py | 4 ++-- 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 +++++++++ 20 files changed, 96 insertions(+), 42 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 0000000000..9cb0368084 --- /dev/null +++ b/doc/release-notes-15454.md @@ -0,0 +1,6 @@ +Wallet +------ + +Dash 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 ce354daeaa..acef6a75fc 100644 --- a/src/interfaces/wallet.cpp +++ b/src/interfaces/wallet.cpp @@ -582,8 +582,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; @@ -600,8 +599,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(); } @@ -660,9 +659,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 b217dc857c..768e9f9d63 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -449,7 +449,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/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp index c30bb725ee..acff2f2331 100644 --- a/src/qt/bitcoingui.cpp +++ b/src/qt/bitcoingui.cpp @@ -870,6 +870,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 754b45572a..bf8e280086 100644 --- a/src/qt/bitcoingui.h +++ b/src/qt/bitcoingui.h @@ -80,6 +80,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/guiutil.cpp b/src/qt/guiutil.cpp index 79497599f6..3d4b39df8c 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -1455,7 +1455,7 @@ void updateFonts() std::vector vecIgnoreClasses{ "QWidget", "QDialog", "QFrame", "QStackedWidget", "QDesktopWidget", "QDesktopScreenWidget", "QTipLabel", "QMessageBox", "QMenu", "QComboBoxPrivateScroller", "QComboBoxPrivateContainer", - "QScrollBar", "QListView", "BitcoinGUI", "WalletView", "WalletFrame" + "QScrollBar", "QListView", "BitcoinGUI", "WalletView", "WalletFrame", "QVBoxLayout", "QGroupBox" }; std::vector vecIgnoreObjects{ "messagesWidget" diff --git a/src/qt/walletframe.cpp b/src/qt/walletframe.cpp index 77d116f2c8..5a478109d4 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 @@ -12,8 +14,11 @@ #include +#include #include #include +#include +#include WalletFrame::WalletFrame(BitcoinGUI* _gui) : QFrame(_gui), @@ -26,9 +31,25 @@ WalletFrame::WalletFrame(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); masternodeListPage = new MasternodeList(); walletStack->addWidget(masternodeListPage); diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 0cbcdf4546..80457866eb 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -166,16 +166,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 485dfadfeb..57e5de3405 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -19,7 +19,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", ""); @@ -44,10 +44,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) { @@ -68,10 +85,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 5d9dfd03d0..7f7787c64b 100644 --- a/src/wallet/load.h +++ b/src/wallet/load.h @@ -18,10 +18,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 704ba7371c..8f8a197d9f 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 0faeee4b4a..c35ec2eede 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/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 95edf6e73c..5086159110 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -37,6 +37,7 @@ #include #include #include +#include #include #include @@ -112,6 +113,7 @@ bool AddWallet(const std::shared_ptr& wallet) if (i != vpwallets.end()) return false; coinJoinClientManagers.emplace(std::make_pair(wallet->GetName(), std::make_shared(*wallet))); vpwallets.push_back(wallet); + g_wallet_init_interface.InitCoinJoinSettings(); return true; } @@ -130,6 +132,7 @@ bool RemoveWallet(const std::shared_ptr& wallet, Optional load_on vpwallets.erase(i); auto it = coinJoinClientManagers.find(wallet->GetName()); coinJoinClientManagers.erase(it); + g_wallet_init_interface.InitCoinJoinSettings(); // Write the wallet setting UpdateWalletSetting(chain, name, load_on_start, warnings); diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index 0007447947..a307b29fc3 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -126,9 +126,9 @@ class EstimateFeeTest(BitcoinTestFramework): self.num_nodes = 3 # mine non-standard txs (e.g. txs with "dust" outputs) self.extra_args = [ - ["-acceptnonstdtxn=1", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], - ["-acceptnonstdtxn=1", "-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"], - ["-acceptnonstdtxn=1", "-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1"] + ["-acceptnonstdtxn=1", "-maxorphantxsize=1000", "-whitelist=127.0.0.1", "-wallet="], + ["-acceptnonstdtxn=1", "-blockmaxsize=17000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1", "-wallet="], + ["-acceptnonstdtxn=1", "-blockmaxsize=8000", "-maxorphantxsize=1000", "-whitelist=127.0.0.1", "-wallet="] ] def skip_test_if_missing_module(self): diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py index 748e0792a4..5bad9639ed 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 dashd 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/wallet_backup.py b/test/functional/wallet_backup.py index d8e1f37886..e9b07db3b6 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -45,10 +45,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 = [ - ["-keypool=100", "-whitelist=127.0.0.1"], - ["-keypool=100", "-whitelist=127.0.0.1"], - ["-keypool=100", "-whitelist=127.0.0.1"], - ["-whitelist=127.0.0.1"] + ["-keypool=100", "-whitelist=127.0.0.1", "-wallet="], + ["-keypool=100", "-whitelist=127.0.0.1", "-wallet="], + ["-keypool=100", "-whitelist=127.0.0.1", "-wallet="], + ["-whitelist=127.0.0.1", "-wallet="] ] self.rpc_timeout = 120 diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index a37a81c88e..098d375dff 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -79,7 +79,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", "-usehd=1"]] + self.extra_args = [["-keypool=90", "-usehd=1", "-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 d1969026bb..5bd364308f 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -126,7 +126,7 @@ class ImportRescanTest(BitcoinTestFramework): self.skip_if_no_wallet() def setup_network(self): - extra_args = [[] for _ in range(self.num_nodes)] + extra_args = [["-wallet="] for _ in range(self.num_nodes)] for i, import_node in enumerate(IMPORT_NODES, 2): if import_node.prune: # txindex is enabled by default in Dash and needs to be disabled for import-rescan.py @@ -135,7 +135,7 @@ class ImportRescanTest(BitcoinTestFramework): self.add_nodes(self.num_nodes, extra_args=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 4abcf2e6d1..a39751d921 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() @@ -83,7 +84,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) @@ -160,7 +161,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 9de7457f17..7310252158 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)