Merge #16923: wallet: Handle duplicate fileid exception

9eefc6e92f gui: Delete progress dialog instead of hidding it (João Barbosa)
ee9e88ba27 wallet: Handle duplicate fileid exception (João Barbosa)

Pull request description:

  Handle the duplicate fileid exception thrown at `CheckUniqueFileid` in tow cases:
   - when duplicate wallets are set on the command line - catch in `LoadWallets`;
   - when a duplicate wallet is loaded dynamically - catch in `LoadWallet`.

  Fixes #16776.

ACKs for top commit:
  jonatack:
    Re-ACK 9eefc6e92f no change since last review 68e0ff0e1f530c942721aab49cf67ffc07104628
  hebasto:
    re-ACK 9eefc6e92f

Tree-SHA512: 46e3c1cd6708b54e2d1c4973a74c8d5428822e04cecbc147cf200eb034efa385e867bd749c7c639020e83c9813fae8fed64a851bdd99abf60c33b07e0363f5d5

src/qt/walletcontroller.cpp

@@ -166,6 +166,7 @@ WalletControllerActivity::~WalletControllerActivity()
 
 void WalletControllerActivity::showProgressDialog(const QString& label_text)
 {
+    assert(!m_progress_dialog);
     m_progress_dialog = new QProgressDialog(m_parent_widget);
 
     m_progress_dialog->setLabelText(label_text);
@@ -175,6 +176,13 @@ void WalletControllerActivity::showProgressDialog(const QString& label_text)
     GUIUtil::PolishProgressDialog(m_progress_dialog);
 }
 
+void WalletControllerActivity::destroyProgressDialog()
+{
+    assert(m_progress_dialog);
+    delete m_progress_dialog;
+    m_progress_dialog = nullptr;
+}
+
 CreateWalletActivity::CreateWalletActivity(WalletController* wallet_controller, QWidget* parent_widget)
     : WalletControllerActivity(wallet_controller, parent_widget)
 {
@@ -229,7 +237,7 @@ void CreateWalletActivity::createWallet()
 
 void CreateWalletActivity::finish()
 {
-    m_progress_dialog->hide();
+    destroyProgressDialog();
 
     if (!m_error_message.empty()) {
         QMessageBox::critical(m_parent_widget, tr("Create wallet failed"), QString::fromStdString(m_error_message));
@@ -270,7 +278,7 @@ OpenWalletActivity::OpenWalletActivity(WalletController* wallet_controller, QWid
 
 void OpenWalletActivity::finish()
 {
-    m_progress_dialog->hide();
+    destroyProgressDialog();
 
     if (!m_error_message.empty()) {
         QMessageBox::critical(m_parent_widget, tr("Open wallet failed"), QString::fromStdString(m_error_message));

src/qt/walletcontroller.h

@@ -96,6 +96,7 @@ protected:
     QObject* worker() const { return m_wallet_controller->m_activity_worker; }
 
     void showProgressDialog(const QString& label_text);
+    void destroyProgressDialog();
 
     WalletController* const m_wallet_controller;
     QWidget* const m_parent_widget;

src/wallet/load.cpp

@@ -66,6 +66,7 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
 
 bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
 {
+    try {
         for (const std::string& walletFile : wallet_files) {
             std::string error;
             std::vector<std::string> warnings;
@@ -77,8 +78,11 @@ bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& walle
             }
             AddWallet(pwallet);
         }
-
         return true;
+    } catch (const std::runtime_error& e) {
+        chain.initError(e.what());
+        return false;
+    }
 }
 
 void StartWallets(CScheduler& scheduler)

src/wallet/wallet.cpp

@@ -148,6 +148,7 @@ void UnloadWallet(std::shared_ptr<CWallet>&& wallet)
 
 std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::vector<std::string>& warnings)
 {
+    try {
         if (!CWallet::Verify(chain, location, false, error, warnings)) {
             error = "Wallet file verification failed: " + error;
             return nullptr;
@@ -161,6 +162,10 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocati
         AddWallet(wallet);
         wallet->postInitProcess();
         return wallet;
+    } catch (const std::runtime_error& e) {
+        error = e.what();
+        return nullptr;
+    }
 }
 
 std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::vector<std::string>& warnings)

test/functional/wallet_multiwallet.py

@@ -236,10 +236,10 @@ class MultiWalletTest(BitcoinTestFramework):
         assert_raises_rpc_error(-4, "Wallet file verification failed: Error loading wallet wallet.dat. Duplicate -wallet filename specified.", self.nodes[0].loadwallet, 'wallet.dat')
 
         # Fail to load if one wallet is a copy of another
-        assert_raises_rpc_error(-1, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy')
+        assert_raises_rpc_error(-4, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy')
 
         # Fail to load if one wallet is a copy of another, test this twice to make sure that we don't re-introduce #14304
-        assert_raises_rpc_error(-1, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy')
+        assert_raises_rpc_error(-4, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy')
 
 
         # Fail to load if wallet file is a symlink