merge bitcoin#15454: Remove the automatic creation and loading of the default wallet

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

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.

src/interfaces/wallet.cpp

@@ -582,8 +582,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;
@@ -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<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

@@ -449,7 +449,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/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;

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

src/qt/guiutil.cpp

@@ -1455,7 +1455,7 @@ void updateFonts()
         std::vector<QString> 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<QString> vecIgnoreObjects{
             "messagesWidget"

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 <qt/createwalletdialog.h>
+#include <qt/walletcontroller.h>
 #include <qt/walletframe.h>
 #include <qt/walletmodel.h>
 
@@ -12,8 +14,11 @@
 
 #include <cassert>
 
+#include <QGroupBox>
 #include <QHBoxLayout>
 #include <QLabel>
+#include <QPushButton>
+#include <QVBoxLayout>
 
 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);

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));
 }

src/wallet/load.cpp

@@ -19,7 +19,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", "");
@@ -44,10 +44,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) {
@@ -68,10 +85,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

@@ -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<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;
 };

src/wallet/wallet.cpp

@@ -37,6 +37,7 @@
 #include <wallet/coincontrol.h>
 #include <wallet/coinselection.h>
 #include <wallet/fees.h>
+#include <walletinitinterface.h>
 #include <warnings.h>
 
 #include <coinjoin/client.h>
@@ -112,6 +113,7 @@ bool AddWallet(const std::shared_ptr<CWallet>& wallet)
     if (i != vpwallets.end()) return false;
     coinJoinClientManagers.emplace(std::make_pair(wallet->GetName(), std::make_shared<CCoinJoinClientManager>(*wallet)));
     vpwallets.push_back(wallet);
+    g_wallet_init_interface.InitCoinJoinSettings();
     return true;
 }
 
@@ -130,6 +132,7 @@ bool RemoveWallet(const std::shared_ptr<CWallet>& wallet, Optional<bool> 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);

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):

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()

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
 

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):

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()

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)
 

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)