From 8a922f10f628c73d263b8b83e3086db971eb7d5a Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Mon, 23 Oct 2023 16:55:57 -0400
Subject: [PATCH 1/7] Merge bitcoin/bitcoin#28609: wallet: Reload watchonly and
 solvables wallets after migration

4814e4063e674ad9b0a5c7e56059cd6a2bf9b764 test: Check tx metadata is migrated to watchonly (Andrew Chow)
d616d30ea5fdfb897f8375ffd8b9f4536ae7835b wallet: Reload watchonly and solvables wallets after migration (Andrew Chow)
118f2d7d70b584eee7b89e58b5cd2d61c59a9bbf wallet: Copy all tx metadata to watchonly wallet (Andrew Chow)
9af87cf3485ce3fac553a284cde37a35d1085c25 test: Check that a failed wallet migration is cleaned up (Andrew Chow)

Pull request description:

  Some incomplete/incorrect state as a result of migration can be mitigated/cleaned up by simply restarting the migrated wallets. We already do this for a wallet when it is migrated, but we do not for the new watchonly and solvables wallets that may be created. This PR introduces this behavior, in addition to creating those wallets initially without an attached chain.

  While implementing this, I noticed that not all `CWalletTx` metadata was being copied over to the watchonly wallet and so some data, such as time received, was being lost. This PR fixes this as a side effect of not having a chain attached to the watchonly wallet. A test has also been added.

ACKs for top commit:
  ishaanam:
    light code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764
  ryanofsky:
    Code review ACK 4814e4063e674ad9b0a5c7e56059cd6a2bf9b764. Just implemented the suggested orderpos, copyfrom, and path set comments since last review
  furszy:
    ACK 4814e406

Tree-SHA512: 0b992430df9f452cb252c2212df8e876613f43564fcd1dc00c6c31fa497adb84dfff6b5ef597590f9b288c5f64cb455f108fcc9b6c9d1fe9eb2c39e7f2c12a89

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/wallet/transaction.cpp          |   5 ++
 src/wallet/transaction.h            |   8 ++-
 src/wallet/wallet.cpp               | 107 +++++++++++++++++++++-------
 test/functional/wallet_migration.py |  58 ++++++++++++++-
 4 files changed, 149 insertions(+), 29 deletions(-)

diff --git a/src/wallet/transaction.cpp b/src/wallet/transaction.cpp
index a46846c1d4a3..4f78fe752054 100644
--- a/src/wallet/transaction.cpp
+++ b/src/wallet/transaction.cpp
@@ -24,4 +24,9 @@ int64_t CWalletTx::GetTxTime() const
     int64_t n = nTimeSmart;
     return n ? n : nTimeReceived;
 }
+
+void CWalletTx::CopyFrom(const CWalletTx& _tx)
+{
+    *this = _tx;
+}
 } // namespace wallet
diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h
index db823f8c0dbe..cf9eb10ed1b1 100644
--- a/src/wallet/transaction.h
+++ b/src/wallet/transaction.h
@@ -341,11 +341,15 @@ class CWalletTx
     bool IsCoinBase() const { return tx->IsCoinBase(); }
     bool IsPlatformTransfer() const { return tx->IsPlatformTransfer(); }
 
+private:
     // Disable copying of CWalletTx objects to prevent bugs where instances get
     // copied in and out of the mapWallet map, and fields are updated in the
     // wrong copy.
-    CWalletTx(CWalletTx const &) = delete;
-    void operator=(CWalletTx const &x) = delete;
+    CWalletTx(const CWalletTx&) = default;
+    CWalletTx& operator=(const CWalletTx&) = default;
+public:
+    // Instead have an explicit copy function
+    void CopyFrom(const CWalletTx&);
 };
 
 struct WalletTxOrderComparator {
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 671d5a987687..fa23c9989008 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4838,6 +4838,14 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
     // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet.
     // We need to go through these in the tx insertion order so that lookups to spends works.
     std::vector<uint256> txids_to_delete;
+    std::unique_ptr<WalletBatch> watchonly_batch;
+    if (data.watchonly_wallet) {
+        watchonly_batch = std::make_unique<WalletBatch>(data.watchonly_wallet->GetDatabase());
+        // Copy the next tx order pos to the watchonly wallet
+        LOCK(data.watchonly_wallet->cs_wallet);
+        data.watchonly_wallet->nOrderPosNext = nOrderPosNext;
+        watchonly_batch->WriteOrderPosNext(data.watchonly_wallet->nOrderPosNext);
+    }
     for (const auto& [_pos, wtx] : wtxOrdered) {
         // Check it is the watchonly wallet's
         // solvable_wallet doesn't need to be checked because transactions for those scripts weren't being watched for
@@ -4846,13 +4854,21 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
             LOCK(data.watchonly_wallet->cs_wallet);
             if (data.watchonly_wallet->IsMine(*wtx->tx) || data.watchonly_wallet->IsFromMe(*wtx->tx)) {
                 // Add to watchonly wallet
-                if (!data.watchonly_wallet->AddToWallet(wtx->tx, wtx->m_state)) {
-                    error = _("Error: Could not add watchonly tx to watchonly wallet");
+                const uint256& hash = wtx->GetHash();
+                const CWalletTx& to_copy_wtx = *wtx;
+                if (!data.watchonly_wallet->LoadToWallet(hash, [&](CWalletTx& ins_wtx, bool new_tx) EXCLUSIVE_LOCKS_REQUIRED(data.watchonly_wallet->cs_wallet) {
+                    if (!new_tx) return false;
+                    ins_wtx.SetTx(to_copy_wtx.tx);
+                    ins_wtx.CopyFrom(to_copy_wtx);
+                    return true;
+                })) {
+                    error = strprintf(_("Error: Could not add watchonly tx %s to watchonly wallet"), wtx->GetHash().GetHex());
                     return false;
                 }
+                watchonly_batch->WriteTx(data.watchonly_wallet->mapWallet.at(hash));
                 // Mark as to remove from the migrated wallet only if it does not also belong to it
                 if (!is_mine) {
-                    txids_to_delete.push_back(wtx->GetHash());
+                    txids_to_delete.push_back(hash);
                 }
                 continue;
             }
@@ -4863,6 +4879,7 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
             return false;
         }
     }
+    watchonly_batch.reset(); // Flush
     // Do the removes
     if (txids_to_delete.size() > 0) {
         std::vector<uint256> deleted_txids;
@@ -4999,6 +5016,10 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
         DatabaseOptions options;
         options.require_existing = false;
         options.require_create = true;
+        options.require_format = DatabaseFormat::SQLITE;
+
+        WalletContext empty_context;
+        empty_context.args = context.args;
 
         // Make the wallets
         options.create_flags = WALLET_FLAG_DISABLE_PRIVATE_KEYS | WALLET_FLAG_BLANK_WALLET | WALLET_FLAG_DESCRIPTORS;
@@ -5014,8 +5035,14 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
             DatabaseStatus status;
             std::vector<bilingual_str> warnings;
             std::string wallet_name = wallet.GetName() + "_watchonly";
-            data->watchonly_wallet = CreateWallet(context, wallet_name, std::nullopt, options, status, error, warnings);
-            if (status != DatabaseStatus::SUCCESS) {
+            std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(wallet_name, options, status, error);
+            if (!database) {
+                error = strprintf(_("Wallet file creation failed: %s"), error);
+                return false;
+            }
+
+            data->watchonly_wallet = CWallet::Create(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings);
+            if (!data->watchonly_wallet) {
                 error = _("Error: Failed to create new watchonly wallet");
                 return false;
             }
@@ -5045,8 +5072,14 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
             DatabaseStatus status;
             std::vector<bilingual_str> warnings;
             std::string wallet_name = wallet.GetName() + "_solvables";
-            data->solvable_wallet = CreateWallet(context, wallet_name, std::nullopt, options, status, error, warnings);
-            if (status != DatabaseStatus::SUCCESS) {
+            std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(wallet_name, options, status, error);
+            if (!database) {
+                error = strprintf(_("Wallet file creation failed: %s"), error);
+                return false;
+            }
+
+            data->solvable_wallet = CWallet::Create(empty_context, wallet_name, std::move(database), options.create_flags, error, warnings);
+            if (!data->solvable_wallet) {
                 error = _("Error: Failed to create new watchonly wallet");
                 return false;
             }
@@ -5183,47 +5216,71 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         success = DoMigration(*local_wallet, context, error, res);
     }
 
+    // In case of reloading failure, we need to remember the wallet dirs to remove
+    // Set is used as it may be populated with the same wallet directory paths multiple times,
+    // both before and after reloading. This ensures the set is complete even if one of the wallets
+    // fails to reload.
+    std::set<fs::path> wallet_dirs;
     if (success) {
-        // Migration successful, unload the wallet locally, then reload it.
-        assert(local_wallet.use_count() == 1);
-        local_wallet.reset();
-        res.wallet = LoadWallet(context, wallet_name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
+        // Migration successful, unload all wallets locally, then reload them.
+        // Reload the main wallet
+        wallet_dirs.insert(fs::PathFromString(local_wallet->GetDatabase().Filename()).parent_path());
+        success = reload_wallet(local_wallet);
+        res.wallet = local_wallet;
         res.wallet_name = wallet_name;
-    } else {
+        if (success && res.watchonly_wallet) {
+            // Reload watchonly
+            wallet_dirs.insert(fs::PathFromString(res.watchonly_wallet->GetDatabase().Filename()).parent_path());
+            success = reload_wallet(res.watchonly_wallet);
+        }
+        if (success && res.solvables_wallet) {
+            // Reload solvables
+            wallet_dirs.insert(fs::PathFromString(res.solvables_wallet->GetDatabase().Filename()).parent_path());
+            success = reload_wallet(res.solvables_wallet);
+        }
+    }
+    if (!success) {
         // Migration failed, cleanup
         // Copy the backup to the actual wallet dir
         fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename);
         fs::copy_file(backup_path, temp_backup_location, fs::copy_options::none);
 
-        // Remember this wallet's walletdir to remove after unloading
-        std::vector<fs::path> wallet_dirs;
-        wallet_dirs.emplace_back(fs::PathFromString(local_wallet->GetDatabase().Filename()).parent_path());
-
-        // Unload the wallet locally
-        assert(local_wallet.use_count() == 1);
-        local_wallet.reset();
+        // Drop the second reference to the reloaded main wallet: UnloadWallet() below
+        // waits until the last reference is gone, and this one would only die on return.
+        // Dash-only: upstream 28.x/29.x carry the same latent hang when a watch-only or
+        // solvables reload fails after the main wallet reloaded, and master only moves
+        // the pointer into res.wallet, which then skips the rollback instead.
+        res.wallet.reset();
 
         // Make list of wallets to cleanup
         std::vector<std::shared_ptr<CWallet>> created_wallets;
+        if (local_wallet) created_wallets.push_back(std::move(local_wallet));
         if (res.watchonly_wallet) created_wallets.push_back(std::move(res.watchonly_wallet));
         if (res.solvables_wallet) created_wallets.push_back(std::move(res.solvables_wallet));
 
         // Get the directories to remove after unloading
         for (std::shared_ptr<CWallet>& w : created_wallets) {
-            wallet_dirs.emplace_back(fs::PathFromString(w->GetDatabase().Filename()).parent_path());
+            wallet_dirs.emplace(fs::PathFromString(w->GetDatabase().Filename()).parent_path());
         }
 
         // Unload the wallets
         for (std::shared_ptr<CWallet>& w : created_wallets) {
-            if (!RemoveWallet(context, w, /*load_on_start=*/false)) {
-                error += _("\nUnable to cleanup failed migration");
-                return util::Error{error};
+            if (w->HaveChain()) {
+                // Unloading for wallets that were loaded for normal use
+                if (!RemoveWallet(context, w, /*load_on_start=*/false)) {
+                    error += _("\nUnable to cleanup failed migration");
+                    return util::Error{error};
+                }
+                UnloadWallet(std::move(w));
+            } else {
+                // Unloading for wallets in local context
+                assert(w.use_count() == 1);
+                w.reset();
             }
-            UnloadWallet(std::move(w));
         }
 
         // Delete the wallet directories
-        for (fs::path& dir : wallet_dirs) {
+        for (const fs::path& dir : wallet_dirs) {
             fs::remove_all(dir);
         }
 
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index 419772242846..78910199b0bb 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -7,7 +7,11 @@
 import os
 import random
 import shutil
+import struct
+import time
+
 from test_framework.address import key_to_p2pkh, script_to_p2sh
+from test_framework.bdb import BTREE_MAGIC
 from test_framework.descriptors import descsum_create
 from test_framework.key import ECPubKey
 from test_framework.test_framework import BitcoinTestFramework
@@ -17,6 +21,10 @@
     assert_equal,
     assert_raises_rpc_error,
     find_vout_for_address,
+    sha256sum_file,
+)
+from test_framework.wallet_util import (
+    get_generate_key,
 )
 
 
@@ -350,12 +358,17 @@ def test_other_watchonly(self):
         watchonly_spendable_txid = default.send(outputs=[{received_addr: 1}, {import_addr:1}])["txid"]
 
         self.generate(self.nodes[0], 2)
+        received_watchonly_tx_info = imports0.gettransaction(received_watchonly_txid, True)
+        received_sent_watchonly_tx_info = imports0.gettransaction(received_sent_watchonly_txid, True)
 
         balances = imports0.getbalances()
         spendable_bal = balances["mine"]["trusted"]
         watchonly_bal = balances["watchonly"]["trusted"]
         assert_equal(len(imports0.listtransactions(include_watchonly=True)), 6)
 
+        # Mock time forward a bit so we can check that tx metadata is preserved
+        self.bump_mocktime(100)
+
         # Migrate
         self.migrate_wallet(imports0)
         assert_equal(imports0.getwalletinfo()["descriptors"], True)
@@ -374,8 +387,12 @@ def test_other_watchonly(self):
         assert_equal(watchonly_info["descriptors"], True)
         self.assert_is_sqlite("imports0_watchonly")
         assert_equal(watchonly_info["private_keys_enabled"], False)
-        watchonly.gettransaction(received_watchonly_txid)
-        watchonly.gettransaction(received_sent_watchonly_txid)
+        received_migrated_watchonly_tx_info = watchonly.gettransaction(received_watchonly_txid)
+        assert_equal(received_watchonly_tx_info["time"], received_migrated_watchonly_tx_info["time"])
+        assert_equal(received_watchonly_tx_info["timereceived"], received_migrated_watchonly_tx_info["timereceived"])
+        received_sent_migrated_watchonly_tx_info = watchonly.gettransaction(received_sent_watchonly_txid)
+        assert_equal(received_sent_watchonly_tx_info["time"], received_sent_migrated_watchonly_tx_info["time"])
+        assert_equal(received_sent_watchonly_tx_info["timereceived"], received_sent_migrated_watchonly_tx_info["timereceived"])
         watchonly.gettransaction(sent_watchonly_txid)
         watchonly.gettransaction(watchonly_spendable_txid)
         assert_equal(watchonly.getbalance(), watchonly_bal)
@@ -877,6 +894,42 @@ def check(info, node):
         for addr_info in [addr_external, addr_external_with_label]:
             check(addr_info, wallet_solvables)
 
+    def test_failed_migration_cleanup(self):
+        self.log.info("Test that a failed migration is cleaned up")
+        wallet = self.create_legacy_wallet("failed")
+
+        # Make a copy of the wallet with the solvables wallet name so that we are unable
+        # to create the solvables wallet when migrating, thus failing to migrate
+        wallet.unloadwallet()
+        solvables_path = self.nodes[0].wallets_path / "failed_solvables"
+        shutil.copytree(self.nodes[0].wallets_path / "failed", solvables_path)
+        original_shasum = sha256sum_file(solvables_path / "wallet.dat")
+
+        self.nodes[0].loadwallet("failed")
+
+        # Add a multisig so that a solvables wallet is created
+        wallet.addmultisigaddress(2, [wallet.getnewaddress(), get_generate_key().pubkey])
+        wallet.importaddress(get_generate_key().p2pkh_addr)
+
+        assert_raises_rpc_error(-4, "Failed to create database", wallet.migratewallet)
+
+        assert "failed" in self.nodes[0].listwallets()
+        assert "failed_watchonly" not in self.nodes[0].listwallets()
+        assert "failed_solvables" not in self.nodes[0].listwallets()
+
+        assert not (self.nodes[0].wallets_path / "failed_watchonly").exists()
+        # Since the file in failed_solvables is one that we put there, migration shouldn't touch it
+        assert solvables_path.exists()
+        new_shasum = sha256sum_file(solvables_path / "wallet.dat")
+        assert_equal(original_shasum, new_shasum)
+
+        wallet.unloadwallet()
+        # Check the wallet we tried to migrate is still BDB
+        with open(self.nodes[0].wallets_path / "failed" / "wallet.dat", "rb") as f:
+            data = f.read(16)
+            _, _, magic = struct.unpack("QII", data)
+            assert_equal(magic, BTREE_MAGIC)
+
     def run_test(self):
         self.generate(self.nodes[0], 101)
 
@@ -896,6 +949,7 @@ def run_test(self):
         self.test_hybrid_pubkey()
         self.test_conflict_txs()
         self.test_addressbook()
+        self.test_failed_migration_cleanup()
 
 if __name__ == '__main__':
     WalletMigrationTest().main()

From 3fd819a41846e569b1449d201f67ff1b937597a6 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Fri, 2 Feb 2024 21:17:14 -0500
Subject: [PATCH 2/7] Merge bitcoin/bitcoin#28868: wallet: Fix migration of
 wallets with txs that have both spendable and watchonly outputs

BACKPORT NOTE:
Only 9332c7edda (write bestblock to watchonly and solvable wallets) is taken here.
The other commits of this PR were already backported in dashpay/dash#7793. Needed now
because bitcoin#28609 reloads the watch-only and solvables wallets after migration, and
without a bestblock record they rescan from genesis on that reload.

4da76ca24725eb9ba8122317e04a6e1ee14ac846 test: Test migration of tx with both spendable and watchonly (Ava Chow)
c62a8d03a862fb124b4f4b88efd61978e46605f8 wallet: Keep txs that belong to both watchonly and migrated wallets (Ava Chow)
71cb28ea8cb579ac04cefc47a57557c94080d1af test: Make sure that migration test does not rescan on reloading (Ava Chow)
78ba0e6748d2a519a96c41dea851e7c43b82f251 wallet: Reload the wallet if migration exited early (Ava Chow)
9332c7edda79a39bb729b71b6f8db6a9d37343bb wallet: Write bestblock to watchonly and solvable wallets (Ava Chow)

Pull request description:

  A transaction does not necessarily have to belong to either the migrated wallet (with the private keys) and the watchonly wallet (with watchonly things), it could have multiple outputs with each isminetype. So we should be putting such transactions in one or the other wallet, but rather putting it in both.

  I've added a test for this behavior, however the test also revealed a few other issues. Notably, it revealed that `migratewallet` would have the watchonly wallet rescan from genesis when it is reloaded at the end of migration. This could be a cause for migration appearing to be very slow. This is resolved by first writing best block records to the watchonly and solvable wallets, as well as updating the test to make sure that rescans don't happen.

  The change to avoid rescans also found an issue where some of our early exits would result in unloading the wallet even though nothing happened. So there is also a commit to reload the wallet for such early exits.

ACKs for top commit:
  ryanofsky:
    Code review ACK 4da76ca24725eb9ba8122317e04a6e1ee14ac846. This looks great. The code is actually cleaner than before, two bugs are fixed, and the test checking for rescanning is pretty clever and broadens test coverage.
  furszy:
    Code review ACK 4da76ca2

Tree-SHA512: 5fc210cff16ca6720d7b2d0616d7e3f295c974147854abc704cf99a3bfaad17572ada084859e7a1b1ca94da647ad130303219678f429b7995f85e040236db35c

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
---
 src/wallet/wallet.cpp | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index fa23c9989008..ba76e246f8ba 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -4835,6 +4835,13 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
         }
     }
 
+    // Get best block locator so that we can copy it to the watchonly and solvables
+    CBlockLocator best_block_locator;
+    if (!WalletBatch(GetDatabase()).ReadBestBlock(best_block_locator)) {
+        error = _("Error: Unable to read wallet's best block locator record");
+        return false;
+    }
+
     // Check if the transactions in the wallet are still ours. Either they belong here, or they belong in the watchonly wallet.
     // We need to go through these in the tx insertion order so that lookups to spends works.
     std::vector<uint256> txids_to_delete;
@@ -4845,6 +4852,18 @@ bool CWallet::ApplyMigrationData(MigrationData& data, bilingual_str& error)
         LOCK(data.watchonly_wallet->cs_wallet);
         data.watchonly_wallet->nOrderPosNext = nOrderPosNext;
         watchonly_batch->WriteOrderPosNext(data.watchonly_wallet->nOrderPosNext);
+        // Write the best block locator to avoid rescanning on reload
+        if (!watchonly_batch->WriteBestBlock(best_block_locator)) {
+            error = _("Error: Unable to write watchonly wallet best block locator record");
+            return false;
+        }
+    }
+    if (data.solvable_wallet) {
+        // Write the best block locator to avoid rescanning on reload
+        if (!WalletBatch(data.solvable_wallet->GetDatabase()).WriteBestBlock(best_block_locator)) {
+            error = _("Error: Unable to write solvable wallet best block locator record");
+            return false;
+        }
     }
     for (const auto& [_pos, wtx] : wtxOrdered) {
         // Check it is the watchonly wallet's

From c8f51bb8205b8796679cb77799b175f86526031b Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Wed, 2 Jul 2025 13:25:33 -0700
Subject: [PATCH 3/7] partial Merge bitcoin/bitcoin#31423: wallet: migration,
 avoid creating spendable wallet from a watch-only legacy wallet

BACKPORT NOTE:
Only the two commits needed by bitcoin#34156 are taken (1de423e0a0, d04f6a97ba); the
migration behaviour change (b789907346) and the reload dedup (e86d71b749) need the
BERKELEY_RO-based migration path and are not backported. BerkeleyDatabase::Files() and
the DummyDatabase override come from the 28.x adaptation (bitcoin#34223, 5f07b93d7f).

b78990734621b8fe46c68a6e7edaf1fbd2f7d351 wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet (furszy)
e86d71b749c08bde6002b9aa2baee824975a518a wallet: refactor, dedup wallet re-loading code (furszy)
1de423e0a08bbc63eed36c8772e9ef8b48e80fb8 wallet: introduce method to return all db created files (furszy)
d04f6a97ba9a55aa9455e1a805feeed4d630f59a refactor: remove sqlite dir path back-and-forth conversion (furszy)

Pull request description:

  Currently, the migration process creates a brand-new descriptor wallet with no
  connection to the user's legacy wallet when the legacy wallet lacks key material
  and contains only watch-only scripts. This behavior is not aligned with user
  expectations. If the legacy wallet contains only watch-only scripts, the migration
  process should only generate a watch-only wallet instead.

  TODO List:
  * Explain that `migratewallet` renames the watch-only after migration, and
  also that the wallet will not have keys enabled.

ACKs for top commit:
  achow101:
    ACK b78990734621b8fe46c68a6e7edaf1fbd2f7d351
  pablomartin4btc:
    tACK b78990734621b8fe46c68a6e7edaf1fbd2f7d351
  rkrux:
    LGTM ACK b78990734621b8fe46c68a6e7edaf1fbd2f7d351

Tree-SHA512: 1d583ac4b206fb477e9727daf4b5ad9c3e18b12d40e1ab4a61e8565da44c3d0327c892b51cf47b4894405d122e414cefb6b6366c357e02a74a7ca96e06762d83

Co-authored-by: Ava Chow <github@achow101.com>
---
 src/wallet/bdb.h       | 15 +++++++++++++++
 src/wallet/db.h        |  3 +++
 src/wallet/salvage.cpp |  1 +
 src/wallet/sqlite.cpp  |  6 +++---
 src/wallet/sqlite.h    | 10 +++++++++-
 src/wallet/test/util.h |  1 +
 6 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
index 51b0d7fd7987..47f02f7112bb 100644
--- a/src/wallet/bdb.h
+++ b/src/wallet/bdb.h
@@ -129,6 +129,21 @@ class BerkeleyDatabase : public WalletDatabase
     /** Return path to main database filename */
     std::string Filename() override { return fs::PathToString(env->Directory() / m_filename); }
 
+    std::vector<fs::path> Files() override
+    {
+        std::vector<fs::path> files;
+        files.emplace_back(env->Directory() / m_filename);
+        if (env->m_databases.size() == 1) {
+            files.emplace_back(env->Directory() / "db.log");
+            files.emplace_back(env->Directory() / ".walletlock");
+            files.emplace_back(env->Directory() / "database" / "log.0000000001");
+            files.emplace_back(env->Directory() / "database");
+            // Note that this list is not exhaustive as BDB may create more log files, and possibly other ones too
+            // However it should be good enough for the only calls to Files()
+        }
+        return files;
+    }
+
     std::string Format() override { return "bdb"; }
     /**
      * Pointer to shared database environment.
diff --git a/src/wallet/db.h b/src/wallet/db.h
index 323c506970cb..55558ab87efe 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -162,6 +162,9 @@ class WalletDatabase
     /** Return path to main database file for logs and error messages. */
     virtual std::string Filename() = 0;
 
+    /** Return paths to all database created files */
+    virtual std::vector<fs::path> Files() = 0;
+
     virtual std::string Format() = 0;
 
     std::atomic<unsigned int> nUpdateCounter;
diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp
index 8bd79b5fbd9d..804813097a9d 100644
--- a/src/wallet/salvage.cpp
+++ b/src/wallet/salvage.cpp
@@ -66,6 +66,7 @@ class DummyDatabase : public WalletDatabase
     void IncrementUpdateCounter() override { ++nUpdateCounter; }
     void ReloadDbEnv() override {}
     std::string Filename() override { return "dummy"; }
+    std::vector<fs::path> Files() override { return {}; }
     std::string Format() override { return "dummy"; }
     std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<DummyBatch>(); }
 };
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index c534693af164..84ab061e4138 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -109,12 +109,12 @@ Mutex SQLiteDatabase::g_sqlite_mutex;
 int SQLiteDatabase::g_sqlite_count = 0;
 
 SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock)
-    : WalletDatabase(), m_mock(mock), m_dir_path(fs::PathToString(dir_path)), m_file_path(fs::PathToString(file_path)), m_use_unsafe_sync(options.use_unsafe_sync)
+    : WalletDatabase(), m_mock(mock), m_dir_path(dir_path), m_file_path(fs::PathToString(file_path)), m_use_unsafe_sync(options.use_unsafe_sync)
 {
     {
         LOCK(g_sqlite_mutex);
         LogPrintf("Using SQLite Version %s\n", SQLiteDatabaseVersion());
-        LogPrintf("Using wallet %s\n", m_dir_path);
+        LogPrintf("Using wallet %s\n", fs::PathToString(m_dir_path));
 
         if (++g_sqlite_count == 1) {
             // Setup logging
@@ -250,7 +250,7 @@ void SQLiteDatabase::Open()
 
     if (m_db == nullptr) {
         if (!m_mock) {
-            TryCreateDirectories(fs::PathFromString(m_dir_path));
+            TryCreateDirectories(m_dir_path);
         }
         int ret = sqlite3_open_v2(m_file_path.c_str(), &m_db, flags, nullptr);
         if (ret != SQLITE_OK) {
diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h
index a5350b1ecec4..b5d142434b59 100644
--- a/src/wallet/sqlite.h
+++ b/src/wallet/sqlite.h
@@ -76,7 +76,7 @@ class SQLiteDatabase : public WalletDatabase
 private:
     const bool m_mock{false};
 
-    const std::string m_dir_path;
+    const fs::path m_dir_path;
 
     const std::string m_file_path;
 
@@ -133,6 +133,14 @@ class SQLiteDatabase : public WalletDatabase
     void IncrementUpdateCounter() override { ++nUpdateCounter; }
 
     std::string Filename() override { return m_file_path; }
+    /** Return paths to all database created files */
+    std::vector<fs::path> Files() override
+    {
+        std::vector<fs::path> files;
+        files.emplace_back(m_dir_path / fs::PathFromString(m_file_path));
+        files.emplace_back(m_dir_path / fs::PathFromString(m_file_path + "-journal"));
+        return files;
+    }
     std::string Format() override { return "sqlite"; }
 
     /** Make a SQLiteBatch connected to this database */
diff --git a/src/wallet/test/util.h b/src/wallet/test/util.h
index 52ef7553f7fc..fa58f061f785 100644
--- a/src/wallet/test/util.h
+++ b/src/wallet/test/util.h
@@ -117,6 +117,7 @@ class MockableDatabase : public WalletDatabase
     void ReloadDbEnv() override {}
 
     std::string Filename() override { return "mockable"; }
+    std::vector<fs::path> Files() override { return {}; }
     std::string Format() override { return "mock"; }
     std::unique_ptr<DatabaseBatch> MakeBatch(bool flush_on_close = true) override { return std::make_unique<MockableBatch>(m_records, m_pass); }
 };

From cfd04125bc33f65b2dc40c2b0b276ddacad32d4c Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Wed, 7 Jan 2026 14:32:01 +0000
Subject: [PATCH 4/7] Merge bitcoin/bitcoin#34215: wallettool: fix unnamed
 createfromdump failure walletsdir deletion

f78f6f1dc8e16d5a8a23749e77bc3bf17c91ae42 wallettool: do not use fs::remove_all in createfromdump cleanup (Ava Chow)

Pull request description:

  As pointed out in https://github.com/bitcoin/bitcoin/pull/34156#issuecomment-3716728670, it is possible for `createfromdump` to also accidentally delete the entire wallets directory if the wallet name is the empty string and the dumpfile contains a checksum error.

  This is also fixed by removing the files created by only removing the directory for named wallets, and avoiding the use of `fs::remove_all`.

ACKs for top commit:
  waketraindev:
    lgtm ACK f78f6f1dc8e16d5a8a23749e77bc3bf17c91ae42
  polespinasa:
    code review and tACK f78f6f1dc8e16d5a8a23749e77bc3bf17c91ae42
  rkrux:
    Code review and tACK f78f6f1dc8e16d5a8a23749e77bc3bf17c91ae42
  willcl-ark:
    ACK f78f6f1dc8e16d5a8a23749e77bc3bf17c91ae42
  pablomartin4btc:
    ACK f78f6f1dc8e16d5a8a23749e77bc3bf17c91ae42

Tree-SHA512: ff1e7668131ec3632c67d990c99e8fddff28605e7e553c7e20695e61017c88476c3636e22f2007e763a00d527e80e4d1d3d45409f6678d28729b8397430bfe7a

Co-authored-by: merge-script <fanquake@gmail.com>
---
 src/wallet/dump.cpp            | 8 +++++++-
 test/functional/tool_wallet.py | 6 ++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp
index b55b64859c09..2b13e5d93d4c 100644
--- a/src/wallet/dump.cpp
+++ b/src/wallet/dump.cpp
@@ -280,11 +280,17 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs::
 
         dump_file.close();
     }
+    // On failure, gather the paths to remove
+    std::vector<fs::path> paths_to_remove = wallet->GetDatabase().Files();
+    if (!name.empty()) paths_to_remove.push_back(wallet_path);
+
     wallet.reset(); // The pointer deleter will close the wallet for us.
 
     // Remove the wallet dir if we have a failure
     if (!ret) {
-        fs::remove_all(wallet_path);
+        for (const auto& p : paths_to_remove) {
+            fs::remove(p);
+        }
     }
 
     return ret;
diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
index b4bb58a0ac77..98a1b6bf6bf4 100755
--- a/test/functional/tool_wallet.py
+++ b/test/functional/tool_wallet.py
@@ -401,6 +401,12 @@ def test_dump_createfromdump(self):
         self.write_dump(dump_data, bad_sum_wallet_dump)
         self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
         assert not (self.nodes[0].wallets_path / "badload").is_dir()
+        self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
+        assert self.nodes[0].wallets_path.exists()
+        assert not (self.nodes[0].wallets_path / "wallet.dat").exists()
+
+        self.log.info('Checking createfromdump with an unnamed wallet')
+        self.do_tool_createfromdump("", "wallet.dump")
 
     def test_chainless_conflicts(self):
         self.log.info("Test wallet tool when wallet contains conflicting transactions")

From 7b7d0c64e04543d7db3c58ee3cae2860ca47f5e1 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Wed, 7 Jan 2026 11:08:57 +0000
Subject: [PATCH 5/7] partial Merge bitcoin/bitcoin#34156: wallet: fix unnamed
 legacy wallet migration failure

BACKPORT NOTE:
Code taken from the 28.x adaptation (bitcoin#34223: e1e9d71da9, e47af69222,
1d4662441f, 75b59e5aba, fb4406e63a, 86eaf71e60, 77622e000b) because master's
commits target the BERKELEY_RO-only migration path. Dropped test_pruned_wallet_backup
(needs bitcoin#30678). The backup filename keeps Dash's basename reduction for
path-style wallet names (see the TODO for bitcoin#32273) and only adopts
MigrationPrefixName for the unnamed wallet.

b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb test: coverage for migration failure when last sync is beyond prune height (furszy)
82caa8193a3e36f248dcc949e0cd41def191efac wallet: migration, fix watch-only and solvables wallets names (furszy)
d70b159c42008ac3b63d1c43d99d4f1316d2f1ef wallet: improve post-migration logging (furszy)
f011e0f0680a8c39988ae57dae57eb86e92dd449 test: restorewallet, coverage for existing dirs, unnamed wallet and prune failure (furszy)
36093bde63286e19821a9e62cdff1712b6245dc7 test: add coverage for unnamed wallet migration failure (furszy)
f4c7e28e80bf9af50b03a770b641fd309a801589 wallet: fix unnamed wallet migration failure (furszy)
4ed0693a3f2a427ef9e7ad016930ec29fa244995 wallet: RestoreWallet failure, erase only what was created (furszy)

Pull request description:

  Minimal fix for #34128.

  The issue occurs during the migration of a legacy unnamed wallet
  (the legacy "default" wallet). When the migration fails, the cleanup
  logic is triggered to roll back the state, which involves erasing the
  newly created descriptor wallets directories. Normally, this only
  affects the parent directories of named wallets, since they each
  reside in their own directories. However, because the unnamed
  wallet resides directly in the top-level `/wallets/` folder, this
  logic accidentally deletes the main directory.

  The fix ensures that only the wallet.dat file of the unnamed wallet
  is touched and restored, preserving the wallet in BDB format and
  leaving the main `/wallets/` directory intact.

  #### Story Line:
  #32273 fixed a different set of issues and, in doing so, uncovered
  this one.
  Before the mentioned PR, backups were stored in the same directory
  as the wallet.dat file. On a migration failure, the backup was then
  copied to the top-level `/wallets/` directory. For the unnamed legacy
  wallet, the wallet directory is the `/wallets/` directory, so the source
  and destination paths were identical. As a result, we threw early in the
  `fs::copy_file` call ([here](https://github.com/bitcoin/bitcoin/blob/29.x/src/wallet/wallet.cpp#L4572)) because the file already existed, as we
  were trying to copy the file onto itself. This caused the cleanup logic
  to abort early on and never reach the removal line.

  #### Testing Notes:
  Cherry-pick the test commit on top of master and run it. You will
  see the failure and realize the reason by reading the test code.

ACKs for top commit:
  achow101:
    ACK b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb
  davidgumberg:
    crACK https://github.com/bitcoin/bitcoin/commit/b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb
  w0xlt:
    ACK https://github.com/bitcoin/bitcoin/commit/b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb
  willcl-ark:
    ACK b7c34d08dd9549a95cffc6ec1ffa4bb4f81e35eb

Tree-SHA512: d0be14c0ed6417f999c3f2f429652c2407097d0cc18453c91653e57ae4b5375b327ad3b2553d9ea6ff46a3ae00cdbd5ab325b94eba763072c4fc5a773b85618b

Co-authored-by: merge-script <fanquake@gmail.com>
---
 src/wallet/wallet.cpp               | 138 +++++++++++++++++++++-------
 test/functional/wallet_backup.py    |  83 ++++++++++++++++-
 test/functional/wallet_migration.py | 111 +++++++++++++++++++++-
 3 files changed, 289 insertions(+), 43 deletions(-)

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index ba76e246f8ba..65b8246664cb 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -517,6 +517,7 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& b
     const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::u8path(wallet_name));
     auto wallet_file = wallet_path / "wallet.dat";
     std::shared_ptr<CWallet> wallet;
+    bool wallet_file_copied = false;
     bool created_parent_dir = false;
 
     try {
@@ -526,14 +527,34 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& b
             return nullptr;
         }
 
-        if (fs::exists(wallet_path) || !TryCreateDirectories(wallet_path)) {
-            error = Untranslated(strprintf("Failed to create database path '%s'. Database already exists.", fs::PathToString(wallet_path)));
-            status = DatabaseStatus::FAILED_ALREADY_EXISTS;
-            return nullptr;
+        // Wallet directories are allowed to exist, but must not contain a .dat file.
+        // Any existing wallet database is treated as a hard failure to prevent overwriting.
+        if (fs::exists(wallet_path)) {
+            // If this is a file, it is the db and we don't want to overwrite it.
+            if (!fs::is_directory(wallet_path)) {
+                error = Untranslated(strprintf("Failed to restore wallet. Database file exists '%s'.", fs::PathToString(wallet_path)));
+                status = DatabaseStatus::FAILED_ALREADY_EXISTS;
+                return nullptr;
+            }
+
+            // Check we are not going to overwrite an existing db file
+            if (fs::exists(wallet_file)) {
+                error = Untranslated(strprintf("Failed to restore wallet. Database file exists in '%s'.", fs::PathToString(wallet_file)));
+                status = DatabaseStatus::FAILED_ALREADY_EXISTS;
+                return nullptr;
+            }
+        } else {
+            // The directory doesn't exist, create it
+            if (!TryCreateDirectories(wallet_path)) {
+                error = Untranslated(strprintf("Failed to restore database path '%s'.", fs::PathToString(wallet_path)));
+                status = DatabaseStatus::FAILED_ALREADY_EXISTS;
+                return nullptr;
+            }
+            created_parent_dir = true;
         }
 
-        created_parent_dir = true;
         fs::copy_file(backup_file, wallet_file, fs::copy_options::none);
+        wallet_file_copied = true;
 
         wallet = LoadWallet(context, wallet_name, load_on_start, options, status, error, warnings);
     } catch (const std::exception& e) {
@@ -542,8 +563,19 @@ std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& b
         error += strprintf(Untranslated("Unexpected exception: %s"), e.what());
         status = DatabaseStatus::FAILED_LOAD;
     }
-    if (!wallet && created_parent_dir) {
-        fs::remove_all(wallet_path);
+
+    // Remove created wallet path only when loading fails
+    if (!wallet) {
+        if (wallet_file_copied) fs::remove(wallet_file);
+        // Clean up the parent directory if we created it during restoration.
+        // Verifying a corrupt BDB file leaves its environment files (db.log,
+        // .walletlock, database/) next to the wallet file, so the directory is
+        // not necessarily empty. It did not exist before this call, so removing
+        // it recursively only touches what this restore created.
+        // TODO(bitcoin#28710): once BDB is gone nothing but wallet.dat can be
+        //       created here; restore upstream's `Assume(fs::is_empty(wallet_path));
+        //       fs::remove(wallet_path);` (bitcoin#34156, 4ed0693a3f).
+        if (created_parent_dir) fs::remove_all(wallet_path);
     }
 
     return wallet;
@@ -5022,6 +5054,15 @@ bool CWallet::CanGrindR() const
     return !IsWalletFlagSet(WALLET_FLAG_EXTERNAL_SIGNER);
 }
 
+// Returns wallet prefix for migration.
+// Used to name the backup file and newly created wallets.
+// E.g. a watch-only wallet is named "<prefix>_watchonly".
+static std::string MigrationPrefixName(CWallet& wallet)
+{
+    const std::string& name{wallet.GetName()};
+    return name.empty() ? "default_wallet" : name;
+}
+
 bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, MigrationResult& res) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet)
 {
     AssertLockHeld(wallet.cs_wallet);
@@ -5053,7 +5094,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
 
             DatabaseStatus status;
             std::vector<bilingual_str> warnings;
-            std::string wallet_name = wallet.GetName() + "_watchonly";
+            std::string wallet_name = MigrationPrefixName(wallet) + "_watchonly";
             std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(wallet_name, options, status, error);
             if (!database) {
                 error = strprintf(_("Wallet file creation failed: %s"), error);
@@ -5090,7 +5131,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error,
 
             DatabaseStatus status;
             std::vector<bilingual_str> warnings;
-            std::string wallet_name = wallet.GetName() + "_solvables";
+            std::string wallet_name = MigrationPrefixName(wallet) + "_solvables";
             std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(wallet_name, options, status, error);
             if (!database) {
                 error = strprintf(_("Wallet file creation failed: %s"), error);
@@ -5171,7 +5212,12 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         std::string name = to_reload->GetName();
         to_reload.reset();
         to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
-        return to_reload != nullptr;
+        if (!to_reload) {
+            LogError("Failed to load wallet '%s' after migration. Rolling back migration to preserve consistency. "
+                     "Error cause: %s\n", wallet_name, error.original);
+            return false;
+        }
+        return true;
     };
 
     // Before anything else, check if there is something to migrate.
@@ -5184,10 +5230,13 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
 
     // Make a backup of the DB in the wallet's directory with a unique filename
     // based on the wallet name and current timestamp. The backup filename uses
-    // the basename of the wallet path (or "wallet" if the name is blank) so that
-    // path-style wallet names cannot escape the wallet directory.
-    // TODO: backport bitcoin/bitcoin#32273 to also move the backup into the
-    //       top-level walletdir and unify with upstream.
+    // the basename of the wallet path (or "default_wallet" if the name is blank)
+    // so that path-style wallet names cannot escape the wallet directory.
+    // TODO(bitcoin#32273): backups move to the top-level walletdir and are named
+    //       from the wallet path. When backported, drop the basename reduction
+    //       below, replace test_wallet_name_with_slashes with upstream's path
+    //       tests, re-derive the bitcoin#34226 path cases from master, then
+    //       take bitcoin#33104 and bitcoin#33122.
     fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path();
     std::string backup_prefix;
     if (!wallet_name.empty()) {
@@ -5196,9 +5245,9 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         backup_prefix = fs::PathToString(legacy_wallet_path.filename());
     }
     // Fall back when the name was blank or canonicalizes to an empty filename
-    // (e.g. "/", "."), so we never emit a file like "-1748432384.legacy.bak".
-    if (backup_prefix.empty()) backup_prefix = "wallet";
-    fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", backup_prefix, GetTime()));
+    // (e.g. "/", "."), so we never emit a file like "_1748432384.legacy.bak".
+    if (backup_prefix.empty()) backup_prefix = MigrationPrefixName(*local_wallet);
+    fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", backup_prefix, GetTime()));
     fs::path backup_path = this_wallet_dir / backup_filename;
     if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) {
         if (was_loaded) {
@@ -5235,26 +5284,44 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         success = DoMigration(*local_wallet, context, error, res);
     }
 
-    // In case of reloading failure, we need to remember the wallet dirs to remove
-    // Set is used as it may be populated with the same wallet directory paths multiple times,
-    // both before and after reloading. This ensures the set is complete even if one of the wallets
-    // fails to reload.
-    std::set<fs::path> wallet_dirs;
+    // In case of loading failure, we need to remember the wallet files we have created to remove.
+    // A `set` is used as it may be populated with the same wallet directory paths multiple times,
+    // both before and after loading. This ensures the set is complete even if one of the wallets
+    // fails to load.
+    std::set<fs::path> wallet_files_to_remove;
+    std::set<fs::path> wallet_empty_dirs_to_remove;
+
+    // Helper to track wallet files and directories for cleanup on failure.
+    // Only directories of wallets created during migration (not the main wallet) are tracked.
+    auto track_for_cleanup = [&](const CWallet& wallet) {
+        const auto files = wallet.GetDatabase().Files();
+        wallet_files_to_remove.insert(files.begin(), files.end());
+        if (wallet.GetName() != wallet_name) {
+            // If this isn’t the main wallet, mark its directory for removal.
+            // This applies to the watch-only and solvable wallets.
+            // Wallets stored directly as files in the top-level directory
+            // (e.g. default unnamed wallets) don’t have a removable parent directory.
+            wallet_empty_dirs_to_remove.insert(fs::PathFromString(wallet.GetDatabase().Filename()).parent_path());
+        }
+    };
+
+
     if (success) {
         // Migration successful, unload all wallets locally, then reload them.
         // Reload the main wallet
-        wallet_dirs.insert(fs::PathFromString(local_wallet->GetDatabase().Filename()).parent_path());
+        LogInfo("Loading new wallets after migration...\n");
+        track_for_cleanup(*local_wallet);
         success = reload_wallet(local_wallet);
         res.wallet = local_wallet;
         res.wallet_name = wallet_name;
         if (success && res.watchonly_wallet) {
             // Reload watchonly
-            wallet_dirs.insert(fs::PathFromString(res.watchonly_wallet->GetDatabase().Filename()).parent_path());
+            track_for_cleanup(*res.watchonly_wallet);
             success = reload_wallet(res.watchonly_wallet);
         }
         if (success && res.solvables_wallet) {
             // Reload solvables
-            wallet_dirs.insert(fs::PathFromString(res.solvables_wallet->GetDatabase().Filename()).parent_path());
+            track_for_cleanup(*res.solvables_wallet);
             success = reload_wallet(res.solvables_wallet);
         }
     }
@@ -5262,7 +5329,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         // Migration failed, cleanup
         // Copy the backup to the actual wallet dir
         fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename);
-        fs::copy_file(backup_path, temp_backup_location, fs::copy_options::none);
+        fs::rename(backup_path, temp_backup_location);
 
         // Drop the second reference to the reloaded main wallet: UnloadWallet() below
         // waits until the last reference is gone, and this one would only die on return.
@@ -5278,8 +5345,8 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         if (res.solvables_wallet) created_wallets.push_back(std::move(res.solvables_wallet));
 
         // Get the directories to remove after unloading
-        for (std::shared_ptr<CWallet>& w : created_wallets) {
-            wallet_dirs.emplace(fs::PathFromString(w->GetDatabase().Filename()).parent_path());
+        for (std::shared_ptr<CWallet>& wallet : created_wallets) {
+            track_for_cleanup(*wallet);
         }
 
         // Unload the wallets
@@ -5298,9 +5365,15 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
             }
         }
 
-        // Delete the wallet directories
-        for (const fs::path& dir : wallet_dirs) {
-            fs::remove_all(dir);
+        // First, delete the db files we have created throughout this process and nothing else
+        for (const fs::path& file : wallet_files_to_remove) {
+            fs::remove(file);
+        }
+
+        // Second, delete the created wallet directories and nothing else. They must be empty at this point.
+        for (const fs::path& dir : wallet_empty_dirs_to_remove) {
+            Assume(fs::is_empty(dir));
+            fs::remove(dir);
         }
 
         // Restore the backup
@@ -5312,8 +5385,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         }
 
         // Move the backup to the wallet dir
-        fs::copy_file(temp_backup_location, backup_path, fs::copy_options::none);
-        fs::remove(temp_backup_location);
+        fs::rename(temp_backup_location, backup_path);
 
         return util::Error{error};
     }
diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py
index 8f8c2da40ea5..9048ec24d210 100755
--- a/test/functional/wallet_backup.py
+++ b/test/functional/wallet_backup.py
@@ -37,7 +37,11 @@
 
 from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal, assert_raises_rpc_error
+from test_framework.util import (
+    assert_equal,
+    assert_raises_rpc_error,
+    sha256sum_file,
+)
 
 class WalletBackupTest(BitcoinTestFramework):
     def add_options(self, parser):
@@ -155,17 +159,84 @@ def restore_preserves_existing_directory(self):
             with open(sentinel, encoding='utf8') as f:
                 assert_equal(f.read(), 'preserve existing data')
             assert_raises_rpc_error(-8, 'Backup file does not exist', node.restorewallet, wallet_name, backup_file + '.missing')
-            assert_raises_rpc_error(-36, 'Database already exists', node.restorewallet, wallet_name, backup_file)
+            existing_db = os.path.join(destination, 'wallet.dat')
+            with open(existing_db, 'w', encoding='utf8') as f:
+                f.write('not a wallet')
+            assert_raises_rpc_error(-36, 'Database file exists', node.restorewallet, wallet_name, backup_file)
             assert os.path.isfile(sentinel)
+            with open(existing_db, encoding='utf8') as f:
+                assert_equal(f.read(), 'not a wallet')
 
     def restore_wallet_existent_name(self):
         node = self.nodes[3]
         backup_file = os.path.join(self.nodes[0].datadir, 'wallet.bak')
         wallet_name = "res0"
-        wallet_file = os.path.join(node.wallets_path, wallet_name)
-        error_message = "Failed to create database path '{}'. Database already exists.".format(wallet_file)
+        wallet_file = node.wallets_path / wallet_name
+        error_message = "Failed to restore wallet. Database file exists in '{}'.".format(wallet_file / "wallet.dat")
         assert_raises_rpc_error(-36, error_message, node.restorewallet, wallet_name, backup_file)
-        assert os.path.exists(wallet_file)
+        assert wallet_file.exists()
+
+    def test_restore_existent_dir(self):
+        self.log.info("Test restore on an existent empty directory")
+        node = self.nodes[3]
+        backup_file = self.nodes[0].datadir_path / 'wallet.bak'
+        wallet_name = "restored_wallet"
+        wallet_dir = node.wallets_path / wallet_name
+        os.mkdir(wallet_dir)
+        res = node.restorewallet(wallet_name, backup_file)
+        assert_equal(res['name'], wallet_name)
+        node.unloadwallet(wallet_name)
+
+        self.log.info("Test restore succeeds when the target directory contains non-wallet files")
+        wallet_file = node.wallets_path / wallet_name / "wallet.dat"
+        os.remove(wallet_file)
+        extra_file = node.wallets_path / wallet_name / "not_a_wallet.txt"
+        extra_file.touch()
+        res = node.restorewallet(wallet_name, backup_file)
+        assert_equal(res['name'], wallet_name)
+        assert extra_file.exists() # extra file was not removed by mistake
+        node.unloadwallet(wallet_name)
+
+        self.log.info("Test restore failure due to existing db file in the destination directory")
+        original_shasum = sha256sum_file(wallet_file)
+        error_message = "Failed to restore wallet. Database file exists in '{}'.".format(wallet_dir / "wallet.dat")
+        assert_raises_rpc_error(-36, error_message, node.restorewallet, wallet_name, backup_file)
+        # Ensure the wallet file remains untouched
+        assert wallet_dir.exists()
+        assert_equal(original_shasum, sha256sum_file(wallet_file))
+
+        self.log.info("Test restore succeeds when the .dat file in the destination has a different name")
+        second_wallet = wallet_dir / "hidden_storage.dat"
+        os.rename(wallet_dir / "wallet.dat", second_wallet)
+        original_shasum = sha256sum_file(second_wallet)
+        res = node.restorewallet(wallet_name, backup_file)
+        assert_equal(res['name'], wallet_name)
+        assert (wallet_dir / "hidden_storage.dat").exists()
+        assert_equal(original_shasum, sha256sum_file(second_wallet))
+        node.unloadwallet(wallet_name)
+
+        # Clean for follow-up tests
+        os.remove(wallet_file)
+
+    def test_restore_into_unnamed_wallet(self):
+        self.log.info("Test restore into a default unnamed wallet")
+        # This is also useful to test the migration recovery after failure logic
+        node = self.nodes[3]
+        if not self.options.descriptors:
+            node.unloadwallet("")
+            os.rename(node.wallets_path / "wallet.dat", node.wallets_path / "default.wallet.dat")
+        backup_file = self.nodes[0].datadir_path / 'wallet.bak'
+        wallet_name = ""
+        res = node.restorewallet(wallet_name, backup_file)
+        assert_equal(res['name'], "")
+        assert (node.wallets_path / "wallet.dat").exists()
+        # Clean for follow-up tests
+        node.unloadwallet("")
+        os.remove(node.wallets_path / "wallet.dat")
+        if not self.options.descriptors:
+            os.rename(node.wallets_path / "default.wallet.dat", node.wallets_path / "wallet.dat")
+            node.loadwallet("")
+
 
     def run_test(self):
         self.log.info("Generating initial blockchain")
@@ -243,6 +314,8 @@ def run_test(self):
         assert_equal(res2_rpc.getbalance(), balance2)
 
         self.restore_wallet_existent_name()
+        self.test_restore_existent_dir()
+        self.test_restore_into_unnamed_wallet()
 
         if not self.options.descriptors:
             self.log.info("Restoring using dumped wallet")
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index 78910199b0bb..61fa5ca44b80 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -4,11 +4,11 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test Migrating a wallet from legacy to descriptor."""
 
+from pathlib import Path
 import os
 import random
 import shutil
 import struct
-import time
 
 from test_framework.address import key_to_p2pkh, script_to_p2sh
 from test_framework.bdb import BTREE_MAGIC
@@ -19,6 +19,7 @@
 from test_framework.script_util import key_to_p2pkh_script, script_to_p2sh_script
 from test_framework.util import (
     assert_equal,
+    assert_greater_than,
     assert_raises_rpc_error,
     find_vout_for_address,
     sha256sum_file,
@@ -50,8 +51,8 @@ def assert_is_sqlite(self, wallet_name):
             assert_equal(file_magic, b'SQLite format 3\x00')
         assert_equal(self.nodes[0].get_wallet_rpc(wallet_name).getwalletinfo()["format"], "sqlite")
 
-    def create_legacy_wallet(self, wallet_name, disable_private_keys=False, blank=False):
-        self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=False, disable_private_keys=disable_private_keys, blank=blank)
+    def create_legacy_wallet(self, wallet_name, disable_private_keys=False, blank=False, **kwargs):
+        self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=False, disable_private_keys=disable_private_keys, blank=blank, **kwargs)
         wallet = self.nodes[0].get_wallet_rpc(wallet_name)
         info = wallet.getwalletinfo()
         assert_equal(info["descriptors"], False)
@@ -574,20 +575,92 @@ def test_wallet_name_with_slashes(self):
         # Basename must use the sanitized prefix derived from the wallet
         # name's last component, not the full "nested/..." path.
         backup_basename = os.path.basename(backup_path)
-        assert backup_basename.startswith("subdir_wallet-"), backup_basename
+        assert backup_basename.startswith("subdir_wallet_"), backup_basename
         assert backup_basename.endswith(".legacy.bak"), backup_basename
 
         self.assert_is_sqlite(nested_name)
 
+    def clear_default_wallet(self, backup_file):
+        # Test cleanup: Clear unnamed default wallet for subsequent tests
+        (self.nodes[0].wallets_path / "wallet.dat").unlink()
+        shutil.rmtree(self.nodes[0].wallets_path / "default_wallet_watchonly", ignore_errors=True)
+        shutil.rmtree(self.nodes[0].wallets_path / "default_wallet_solvables", ignore_errors=True)
+        backup_file.unlink()
+
     def test_default_wallet(self):
         self.log.info("Test migration of the wallet named as the empty string")
         wallet = self.create_legacy_wallet("")
 
-        self.migrate_wallet(wallet)
+        res = self.migrate_wallet(wallet)
         info = wallet.getwalletinfo()
         assert_equal(info["descriptors"], True)
         assert_equal(info["format"], "sqlite")
 
+        wallet.unloadwallet()
+        self.clear_default_wallet(backup_file=Path(res["backup_path"]))
+
+    def test_default_wallet_watch_only(self):
+        self.log.info("Test unnamed (default) watch-only wallet migration")
+        def_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+        wallet = self.create_legacy_wallet("")
+        wallet.importaddress(def_wallet.getnewaddress())
+
+        res = self.migrate_wallet(wallet)
+        wallet = self.nodes[0].get_wallet_rpc("")
+        watchonly_wallet = self.nodes[0].get_wallet_rpc("default_wallet_watchonly")
+
+        info = watchonly_wallet.getwalletinfo()
+        assert_equal(info["descriptors"], True)
+        assert_equal(info["format"], "sqlite")
+        assert_equal(info["private_keys_enabled"], False)
+        assert_equal(info["walletname"], "default_wallet_watchonly")
+
+        # The default wallet will still exist and have newly generated descriptors
+        assert (self.nodes[0].wallets_path / "wallet.dat").exists()
+        info = wallet.getwalletinfo()
+        assert_equal(info["descriptors"], True)
+        assert_equal(info["format"], "sqlite")
+        assert_equal(info["private_keys_enabled"], True)
+        assert_equal(info["walletname"], "")
+        assert_greater_than(info["keypoolsize"], 0)
+
+        wallet.unloadwallet()
+        watchonly_wallet.unloadwallet()
+        self.clear_default_wallet(backup_file=Path(res["backup_path"]))
+
+    def test_default_wallet_failure(self):
+        self.log.info("Test failure during unnamed (default) wallet migration")
+        master_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
+        wallet = self.create_legacy_wallet("", blank=True)
+        wallet.importaddress(master_wallet.getnewaddress())
+
+        # Create wallet directory with the watch-only name and a wallet file.
+        # Because the wallet dir exists, this will cause migration to fail.
+        watch_only_dir = self.nodes[0].wallets_path / "default_wallet_watchonly"
+        os.mkdir(watch_only_dir)
+        shutil.copyfile(self.nodes[0].wallets_path / "wallet.dat", watch_only_dir / "wallet.dat")
+
+        mocked_time = int(time.time())
+        self.nodes[0].setmocktime(mocked_time)
+        assert_raises_rpc_error(-4, "Wallet file creation failed", self.nodes[0].migratewallet, "")
+        self.nodes[0].setmocktime(0)
+
+        # Verify the /wallets/ path exists
+        assert self.nodes[0].wallets_path.exists()
+        # Check backup file exists. Because the wallet has no name, the backup is prefixed with 'default_wallet'
+        backup_path = self.nodes[0].wallets_path / f"default_wallet_{mocked_time}.legacy.bak"
+        assert backup_path.exists()
+        # Verify the original unnamed wallet was restored
+        assert (self.nodes[0].wallets_path / "wallet.dat").exists()
+        # And verify it is still a BDB wallet
+        with open(self.nodes[0].wallets_path / "wallet.dat", "rb") as f:
+            data = f.read(16)
+            _, _, magic = struct.unpack("QII", data)
+            assert_equal(magic, BTREE_MAGIC)
+
+        wallet.unloadwallet()
+        self.clear_default_wallet(backup_path)
+
     def test_direct_file(self):
         self.log.info("Test migration of a wallet that is not in a wallet directory")
         wallet = self.create_legacy_wallet("plainfile")
@@ -930,6 +1003,29 @@ def test_failed_migration_cleanup(self):
             _, _, magic = struct.unpack("QII", data)
             assert_equal(magic, BTREE_MAGIC)
 
+    def unsynced_wallet_on_pruned_node_fails(self):
+        self.log.info("Test migration of an unsynced wallet on a pruned node fails gracefully")
+        shutil.rmtree(self.nodes[0].wallets_path / "database", ignore_errors=True)
+        wallet = self.create_legacy_wallet("", load_on_startup=False)
+        last_wallet_synced_block = wallet.getwalletinfo()['lastprocessedblock']['height']
+
+        # Generate blocks just so the wallet best block is pruned.
+        # Dash's small early regtest blocks fill the first -fastprune block file up
+        # to height ~320, so mine and prune further than upstream to remove it.
+        self.restart_node(0, ["-fastprune", "-prune=1", "-nowallet"])
+        self.generate(self.nodes[0], 600, sync_fun=self.no_op)
+        self.nodes[0].pruneblockchain(400)
+        # Ensure next block to sync is unavailable
+        assert_raises_rpc_error(-1, "Block not available (pruned data)", self.nodes[0].getblock, self.nodes[0].getblockhash(last_wallet_synced_block + 1))
+
+        # Check migration failure
+        mocked_time = self.mocktime
+        assert_raises_rpc_error(-4, "last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)\nUnable to restore backup of wallet.", self.nodes[0].migratewallet, wallet_name="")
+
+        # The wallet will not have been restored, and the backup will be in the wallets directory.
+        backup_path = self.nodes[0].wallets_path / f"default_wallet_{mocked_time}.legacy.bak"
+        assert backup_path.exists()
+
     def run_test(self):
         self.generate(self.nodes[0], 101)
 
@@ -943,7 +1039,9 @@ def run_test(self):
         self.test_unloaded()
         self.test_unloaded_by_path()
         self.test_wallet_name_with_slashes()
+        self.test_default_wallet_failure()
         self.test_default_wallet()
+        self.test_default_wallet_watch_only()
         self.test_direct_file()
         self.test_migrate_raw_p2sh()
         self.test_hybrid_pubkey()
@@ -951,5 +1049,8 @@ def run_test(self):
         self.test_addressbook()
         self.test_failed_migration_cleanup()
 
+        # Note: After this test the first block file of 'nodes[0]' is pruned
+        self.unsynced_wallet_on_pruned_node_fails()
+
 if __name__ == '__main__':
     WalletMigrationTest().main()

From 9857f7386b404a360ed634e1cc0b8c7ca0cd068d Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Fri, 9 Jan 2026 10:02:21 +0000
Subject: [PATCH 6/7] Merge bitcoin/bitcoin#34226: wallet: test: Relative
 wallet failed migration cleanup

BACKPORT NOTE:
Single-node test layout as on 28.x (4d219725a7). The expected backup path uses the
last component of the resolved wallet path because Dash keeps the basename rule
for path-style wallet names (see the TODO for bitcoin#32273 in wallet.cpp).

eeaf28dbe0e09819ab0e95bb7762b29536bdeef6 wallet: test: Failed migration cleanup (David Gumberg)

Pull request description:

  Prior to https://github.com/bitcoin/bitcoin/pull/34156, an issue existed where if migration of a wallet with a relative pathname failed, the relatively specified path where the legacy wallet is would be deleted. This issue predates #32273, because the relative pathnames get stacked together, e.g. "../../", the copy conflict bug that caused migration to abort early instead of getting far enough to attempt clean-up that was fixed in #32273 is avoided.

  This is a functional test demonstrating that we handle failed migration clean-up correctly for relatively-named wallets. To see the issue, you can backport this test onto 29.x: https://github.com/davidgumberg/bitcoin/tree/2026-01-07-rel-migration-test-backport

  I've also added an absolute path failed migration cleanup test. WRT this and #34156, absolute paths exhibit similar behavior to unnamed wallets. Because of the name-conflict bug prior to #32273 an absolute-path migration would fail no matter what because migration would attempt to copy a file to a destination that already exists. But after #32273, absolute-path migration gets past there, and if it fails for some other reason, the same behavior that's fixed in #34156 occurs where the directory containing the wallet file is deleted.

ACKs for top commit:
  achow101:
    ACK eeaf28dbe0e09819ab0e95bb7762b29536bdeef6
  furszy:
    ACK eeaf28dbe0e09819ab0e95bb7762b29536bdeef6
  rkrux:
    lgtm ACK eeaf28dbe0e09819ab0e95bb7762b29536bdeef6

Tree-SHA512: ee366fe526d0328654a86c2e9e6f228ca81554c8f8a78c259fa7aab90f024f9e5694ecf3f1d188938355f4e6d351c5a6a8ad236701bdd0ce63005e5d42c15e15

Co-authored-by: merge-script <fanquake@gmail.com>
---
 test/functional/wallet_migration.py | 84 +++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 21 deletions(-)

diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index 61fa5ca44b80..3e11e8d27836 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -628,38 +628,71 @@ def test_default_wallet_watch_only(self):
         watchonly_wallet.unloadwallet()
         self.clear_default_wallet(backup_file=Path(res["backup_path"]))
 
-    def test_default_wallet_failure(self):
-        self.log.info("Test failure during unnamed (default) wallet migration")
+    def test_migration_failure(self, wallet_name):
+        is_default = wallet_name == ""
+        wallet_pretty_name = "unnamed (default)" if is_default else f'"{wallet_name}"'
+        self.log.info(f"Test failure during migration of wallet named: {wallet_pretty_name}")
+        # Preface, set up legacy wallet and unload it
         master_wallet = self.nodes[0].get_wallet_rpc(self.default_wallet_name)
-        wallet = self.create_legacy_wallet("", blank=True)
+        wallet = self.create_legacy_wallet(wallet_name, blank=True)
         wallet.importaddress(master_wallet.getnewaddress())
+        wallet.unloadwallet()
 
-        # Create wallet directory with the watch-only name and a wallet file.
-        # Because the wallet dir exists, this will cause migration to fail.
-        watch_only_dir = self.nodes[0].wallets_path / "default_wallet_watchonly"
+        if os.path.isabs(wallet_name):
+            wallet_path = Path(wallet_name)
+        else:
+            wallet_path = self.nodes[0].wallets_path / wallet_name
+
+        # This will be the watch-only directory the migration tries to create,
+        # we make migration fail by placing a wallet.dat file there.
+        wo_prefix = wallet_name or "default_wallet"
+        # wo_prefix might have path characters in it, this corresponds with
+        # DoMigration().
+        wo_dirname = f"{wo_prefix}_watchonly"
+        watch_only_dir = self.nodes[0].wallets_path / wo_dirname
         os.mkdir(watch_only_dir)
-        shutil.copyfile(self.nodes[0].wallets_path / "wallet.dat", watch_only_dir / "wallet.dat")
+        shutil.copyfile(wallet_path / "wallet.dat", watch_only_dir / "wallet.dat")
 
-        mocked_time = int(time.time())
-        self.nodes[0].setmocktime(mocked_time)
-        assert_raises_rpc_error(-4, "Wallet file creation failed", self.nodes[0].migratewallet, "")
-        self.nodes[0].setmocktime(0)
+        # Make a file in the wallets dir that must still exist after migration
+        survive_path = self.nodes[0].wallets_path / "survive"
+        open(survive_path, "wb").close()
+        assert survive_path.exists()
+
+        # The backup is named after the node's time, which is the framework's mocktime
+        mocked_time = self.mocktime
+        assert_raises_rpc_error(-4, "Failed to create database", self.nodes[0].migratewallet, wallet_name)
+        wallet.unloadwallet()
 
-        # Verify the /wallets/ path exists
+        # Verify the /wallets/ path exists.
         assert self.nodes[0].wallets_path.exists()
-        # Check backup file exists. Because the wallet has no name, the backup is prefixed with 'default_wallet'
-        backup_path = self.nodes[0].wallets_path / f"default_wallet_{mocked_time}.legacy.bak"
+
+        # Verify survive is still there
+        assert survive_path.exists()
+        # Verify the wallet path exists.
+        assert Path(wallet_path / "wallet.dat").exists()
+
+        # The backup is named after the last component of the wallet's path, so
+        # path-style wallet names cannot escape the wallet directory.
+        backup_prefix = "default_wallet" if is_default else wallet_path.resolve().name
+        backup_path = wallet_path / f"{backup_prefix}_{mocked_time}.legacy.bak"
         assert backup_path.exists()
-        # Verify the original unnamed wallet was restored
-        assert (self.nodes[0].wallets_path / "wallet.dat").exists()
-        # And verify it is still a BDB wallet
-        with open(self.nodes[0].wallets_path / "wallet.dat", "rb") as f:
+
+        with open(self.nodes[0].wallets_path / wallet_name / self.wallet_data_filename, "rb") as f:
             data = f.read(16)
             _, _, magic = struct.unpack("QII", data)
             assert_equal(magic, BTREE_MAGIC)
 
-        wallet.unloadwallet()
-        self.clear_default_wallet(backup_path)
+        survive_path.unlink()
+
+        # Cleanup
+        if is_default:
+            self.clear_default_wallet(backup_path)
+        else:
+            backup_path.unlink()
+            Path(watch_only_dir / "wallet.dat").unlink()
+            Path(watch_only_dir).rmdir()
+            Path(wallet_path / "wallet.dat").unlink()
+
 
     def test_direct_file(self):
         self.log.info("Test migration of a wallet that is not in a wallet directory")
@@ -1039,7 +1072,16 @@ def run_test(self):
         self.test_unloaded()
         self.test_unloaded_by_path()
         self.test_wallet_name_with_slashes()
-        self.test_default_wallet_failure()
+
+        migration_failure_cases = [
+            "",
+            "../",
+            os.path.abspath(self.nodes[0].datadir_path / "absolute_path"),
+            "normallynamedwallet"
+        ]
+        for wallet_name in migration_failure_cases:
+            self.test_migration_failure(wallet_name=wallet_name)
+
         self.test_default_wallet()
         self.test_default_wallet_watch_only()
         self.test_direct_file()

From 1574eb042f2f47568f870d50298d1a528c16a802 Mon Sep 17 00:00:00 2001
From: Konstantin Akimov <github@knstqq.com>
Date: Thu, 29 Jan 2026 17:48:10 +0100
Subject: [PATCH 7/7] Merge bitcoin/bitcoin#34370: [29.x] Fix #34222 backport
 bugs

BACKPORT NOTE:
These five commits have no master counterpart (they fix the BDB-era backport
bitcoin#34222 / bitcoin#34223). The 28.x copies in bitcoin#34620 (29abedc97b,
619480bd78, 6494072295, c5d9f75c4b, 242b9ae3f3) were used as the code reference.
Dash has no load_after_restore (bitcoin#31451) and no blank-wallet branch
(bitcoin#28976); see the TODOs in MigrateLegacyToDescriptor.

65173944ed60df3b9cffca95932aed8720921478 QA: tool_wallet: Check that db.log is deleted with a lone legacy wallet, but not with a shared db environment (Luke Dashjr)
69a6b9b1152ba0bb3edab6d2a54509fd416b24c8 Bugfix: Wallet/Migration: Move backup into wallet directory when migrating from non-directory (Luke Dashjr)
cef01d0be5223e9d33efc897d7fbe5d0a08692c0 Wallet/Migration: Skip moving the backup file back and forth for no reason (Luke Dashjr)
60f529027c6eacbdc298fab50192f8c60d7082a1 Wallet/Migration: If loading the new watchonly or solvables wallet fails, log the correct wallet name in error message (Luke Dashjr)
7475d134f6a3a6039ab6b9d39706ade47c764aa8 Wallet/bdb: Safely and correctly list files only used by the single wallet (Luke Dashjr)

Pull request description:

ACKs for top commit:
  achow101:
    ACK 65173944ed60df3b9cffca95932aed8720921478
  furszy:
    light ACK 65173944ed60df3b9cffca95932aed8720921478

Tree-SHA512: c10fe00dde512ca78cd6939a748b3875d0b40e9714997aedfd939a1dffdc7eaa2fd1779f3972a34b1c1d9a97d8f1ee1e082c970de15ac0e2ef5d9bbf3dc1d89a

Co-authored-by: merge-script <seb.kung@gmail.com>
---
 src/wallet/bdb.cpp             | 48 ++++++++++++++++++++++++++++++++++
 src/wallet/bdb.h               | 15 +----------
 src/wallet/wallet.cpp          | 32 +++++++++++++++--------
 test/functional/tool_wallet.py | 19 ++++++++++++++
 4 files changed, 89 insertions(+), 25 deletions(-)

diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 58488fdbfaa4..c1b7bff8d880 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -13,6 +13,7 @@
 #include <util/strencodings.h>
 #include <util/translation.h>
 
+#include <set>
 #include <stdint.h>
 
 #include <db_cxx.h>
@@ -331,6 +332,53 @@ bool BerkeleyDatabase::Verify(bilingual_str& errorStr)
     return true;
 }
 
+std::vector<fs::path> BerkeleyDatabase::Files()
+{
+    std::vector<fs::path> files;
+    // If the wallet is the *only* file, clean up the entire BDB environment
+    constexpr auto build_files_list = [](std::vector<fs::path>& files, const std::shared_ptr<BerkeleyEnvironment>& env, const fs::path& filename) {
+        if (env->m_databases.size() != 1) return false;
+
+        const auto env_dir = env->Directory();
+        const auto db_subdir = env_dir / "database";
+        if (fs::exists(db_subdir)) {
+            if (!fs::is_directory(db_subdir)) return false;
+            for (const auto& entry : fs::directory_iterator(db_subdir)) {
+                const auto& path = entry.path().filename();
+                if (!fs::PathToString(path).starts_with("log.")) {
+                    return false;
+                }
+                files.emplace_back(entry.path());
+            }
+        }
+        const std::set<fs::path> allowed_paths = {
+            filename,
+            "db.log",
+            ".walletlock",
+            "database"
+        };
+        for (const auto& entry : fs::directory_iterator(env_dir)) {
+            const auto& path = entry.path().filename();
+            if (allowed_paths.contains(path)) {
+                files.emplace_back(entry.path());
+            } else if (fs::is_directory(entry.path())) {
+                // Subdirectories can't possibly be using this db env, and is expected if this is a non-directory wallet
+                // Do not include them in Files, but still allow the env cleanup
+            } else {
+                return false;
+            }
+        }
+        return true;
+    };
+    try {
+        if (build_files_list(files, env, m_filename)) return files;
+    } catch (...) {
+        // Give up building the comprehensive file list if any error occurs
+    }
+    // Otherwise, it's only really safe to delete the one wallet file
+    return {env->Directory() / m_filename};
+}
+
 void BerkeleyEnvironment::CheckpointLSN(const std::string& strFile)
 {
     dbenv->txn_checkpoint(0, 0, 0);
diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h
index 47f02f7112bb..2d9bf4ef0e33 100644
--- a/src/wallet/bdb.h
+++ b/src/wallet/bdb.h
@@ -129,20 +129,7 @@ class BerkeleyDatabase : public WalletDatabase
     /** Return path to main database filename */
     std::string Filename() override { return fs::PathToString(env->Directory() / m_filename); }
 
-    std::vector<fs::path> Files() override
-    {
-        std::vector<fs::path> files;
-        files.emplace_back(env->Directory() / m_filename);
-        if (env->m_databases.size() == 1) {
-            files.emplace_back(env->Directory() / "db.log");
-            files.emplace_back(env->Directory() / ".walletlock");
-            files.emplace_back(env->Directory() / "database" / "log.0000000001");
-            files.emplace_back(env->Directory() / "database");
-            // Note that this list is not exhaustive as BDB may create more log files, and possibly other ones too
-            // However it should be good enough for the only calls to Files()
-        }
-        return files;
-    }
+    std::vector<fs::path> Files() override;
 
     std::string Format() override { return "bdb"; }
     /**
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 65b8246664cb..7378090c1050 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -5214,7 +5214,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
         if (!to_reload) {
             LogError("Failed to load wallet '%s' after migration. Rolling back migration to preserve consistency. "
-                     "Error cause: %s\n", wallet_name, error.original);
+                     "Error cause: %s\n", name, error.original);
             return false;
         }
         return true;
@@ -5280,6 +5280,15 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         // First change to using SQLite
         if (!local_wallet->MigrateToSQLite(error)) return util::Error{error};
 
+        // In case we're migrating from file to directory, move the backup into it
+        // TODO(bitcoin#32273): this block exists only on the 28.x/29.x branches. Remove
+        //       it together with the backup naming TODO above: once backups live in
+        //       the top-level walletdir they never need to be moved.
+        this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path();
+        backup_path = this_wallet_dir / backup_filename;
+        fs::rename(res.backup_path, backup_path);
+        res.backup_path = backup_path;
+
         // Do the migration, and cleanup if it fails
         success = DoMigration(*local_wallet, context, error, res);
     }
@@ -5327,9 +5336,6 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
     }
     if (!success) {
         // Migration failed, cleanup
-        // Copy the backup to the actual wallet dir
-        fs::path temp_backup_location = fsbridge::AbsPathJoin(GetWalletDir(), backup_filename);
-        fs::rename(backup_path, temp_backup_location);
 
         // Drop the second reference to the reloaded main wallet: UnloadWallet() below
         // waits until the last reference is gone, and this one would only die on return.
@@ -5377,16 +5383,20 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
         }
 
         // Restore the backup
-        DatabaseStatus status;
-        std::vector<bilingual_str> warnings;
-        if (!RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, error, warnings)) {
-            error += _("\nUnable to restore backup of wallet.");
+        // Convert the backup file to the wallet db file by renaming it and moving it into the wallet's directory.
+        // Reload it into memory if the wallet was previously loaded.
+        // TODO(bitcoin#31451): RestoreWallet loads the wallet even when it was not loaded
+        //       before migration. When backporting bitcoin#31451, add its load_after_restore
+        //       parameter to RestoreWallet, pass was_loaded here, assert that the wallet is
+        //       reloaded only if was_loaded, and re-derive this function from the 29.x commits
+        //       d91f56e1e3 and a074d36254 (bitcoin#34222), which already have that shape.
+        bilingual_str restore_error;
+        const auto& ptr_wallet = RestoreWallet(context, backup_path, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings);
+        if (!restore_error.empty()) {
+            error += restore_error + _("\nUnable to restore backup of wallet.");
             return util::Error{error};
         }
 
-        // Move the backup to the wallet dir
-        fs::rename(temp_backup_location, backup_path);
-
         return util::Error{error};
     }
     return res;
diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
index 98a1b6bf6bf4..b4c61be7846b 100755
--- a/test/functional/tool_wallet.py
+++ b/test/functional/tool_wallet.py
@@ -401,12 +401,31 @@ def test_dump_createfromdump(self):
         self.write_dump(dump_data, bad_sum_wallet_dump)
         self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=badload', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
         assert not (self.nodes[0].wallets_path / "badload").is_dir()
+        if not self.options.descriptors:
+            os.rename(self.nodes[0].wallets_path / "wallet.dat", self.nodes[0].wallets_path / "../default.wallet.dat")
+            (self.nodes[0].wallets_path / "db.log").unlink(missing_ok=True)
         self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
         assert self.nodes[0].wallets_path.exists()
         assert not (self.nodes[0].wallets_path / "wallet.dat").exists()
+        if not self.options.descriptors:
+            assert not (self.nodes[0].wallets_path / "db.log").exists()
 
         self.log.info('Checking createfromdump with an unnamed wallet')
         self.do_tool_createfromdump("", "wallet.dump")
+        assert (self.nodes[0].wallets_path / "wallet.dat").exists()
+        os.unlink(self.nodes[0].wallets_path / "wallet.dat")
+        if not self.options.descriptors:
+            os.rename(self.nodes[0].wallets_path / "../default.wallet.dat", self.nodes[0].wallets_path / "wallet.dat")
+
+            self.log.info('Checking createfromdump with multiple non-directory wallets')
+            assert not (self.nodes[0].wallets_path / "wallet.dat").is_dir()
+            assert (self.nodes[0].wallets_path / "db.log").exists()
+            os.rename(self.nodes[0].wallets_path / "wallet.dat", self.nodes[0].wallets_path / "test.dat")
+            self.assert_raises_tool_error('Error: Checksum is not the correct size', '-wallet=', '-dumpfile={}'.format(bad_sum_wallet_dump), 'createfromdump')
+            assert not (self.nodes[0].wallets_path / "wallet.dat").exists()
+            assert (self.nodes[0].wallets_path / "test.dat").exists()
+            assert (self.nodes[0].wallets_path / "db.log").exists()
+            os.rename(self.nodes[0].wallets_path / "test.dat", self.nodes[0].wallets_path / "wallet.dat")
 
     def test_chainless_conflicts(self):
         self.log.info("Test wallet tool when wallet contains conflicting transactions")