diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index f3fe8a19c1..ec773fd177 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -132,6 +132,21 @@ public: /** Return path to main database filename */ std::string Filename() override { return fs::PathToString(env->Directory() / m_filename); } + std::vector Files() override + { + std::vector 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 e8790006a4..5f13ca29ff 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -170,6 +170,9 @@ public: /** 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 Files() = 0; + virtual std::string Format() = 0; std::atomic nUpdateCounter; diff --git a/src/wallet/dump.cpp b/src/wallet/dump.cpp index db2756e0ca..20aa5d453e 100644 --- a/src/wallet/dump.cpp +++ b/src/wallet/dump.cpp @@ -288,11 +288,17 @@ bool CreateFromDump(const ArgsManager& args, const std::string& name, const fs:: dump_file.close(); } + // On failure, gather the paths to remove + std::vector 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/src/wallet/migrate.h b/src/wallet/migrate.h index 16eadeb019..82359f9d4b 100644 --- a/src/wallet/migrate.h +++ b/src/wallet/migrate.h @@ -65,6 +65,7 @@ public: /** Return path to main database file for logs and error messages. */ std::string Filename() override { return fs::PathToString(m_filepath); } + std::vector Files() override { return {m_filepath}; } std::string Format() override { return "bdb_ro"; } diff --git a/src/wallet/salvage.cpp b/src/wallet/salvage.cpp index b924239073..443f80893f 100644 --- a/src/wallet/salvage.cpp +++ b/src/wallet/salvage.cpp @@ -63,6 +63,7 @@ public: void IncrementUpdateCounter() override { ++nUpdateCounter; } void ReloadDbEnv() override {} std::string Filename() override { return "dummy"; } + std::vector Files() override { return {}; } std::string Format() override { return "dummy"; } std::unique_ptr MakeBatch(bool flush_on_close = true) override { return std::make_unique(); } }; diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp index a8c9f8a8ab..896a2fc0f3 100644 --- a/src/wallet/sqlite.cpp +++ b/src/wallet/sqlite.cpp @@ -112,12 +112,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_write_semaphore(1), 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_write_semaphore(1), 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 @@ -253,7 +253,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 78a3accf89..c78cd29afc 100644 --- a/src/wallet/sqlite.h +++ b/src/wallet/sqlite.h @@ -105,7 +105,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; @@ -166,6 +166,14 @@ public: void IncrementUpdateCounter() override { ++nUpdateCounter; } std::string Filename() override { return m_file_path; } + /** Return paths to all database created files */ + std::vector Files() override + { + std::vector 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 b055c6c693..59e3a9c75f 100644 --- a/src/wallet/test/util.h +++ b/src/wallet/test/util.h @@ -123,6 +123,7 @@ public: void ReloadDbEnv() override {} std::string Filename() override { return "mockable"; } + std::vector Files() override { return {}; } std::string Format() override { return "mock"; } std::unique_ptr MakeBatch(bool flush_on_close = true) override { return std::make_unique(m_records, m_pass); } }; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 09eda0c28e..2397d84a6f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -501,6 +501,8 @@ std::shared_ptr 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 wallet; + bool wallet_file_copied = false; + bool created_parent_dir = false; try { if (!fs::exists(backup_file)) { @@ -509,13 +511,34 @@ std::shared_ptr 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; } fs::copy_file(backup_file, wallet_file, fs::copy_options::none); + wallet_file_copied = true; if (load_after_restore) { wallet = LoadWallet(context, wallet_name, load_on_start, options, status, error, warnings); @@ -528,7 +551,13 @@ std::shared_ptr RestoreWallet(WalletContext& context, const fs::path& b // Remove created wallet path only when loading fails if (load_after_restore && !wallet) { - fs::remove_all(wallet_path); + if (wallet_file_copied) fs::remove(wallet_file); + // Clean up the parent directory if we created it during restoration. + // As we have created it, it must be empty after deleting the wallet file. + if (created_parent_dir) { + Assume(fs::is_empty(wallet_path)); + fs::remove(wallet_path); + } } return wallet; @@ -4297,6 +4326,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 "_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); @@ -4328,7 +4366,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, DatabaseStatus status; std::vector warnings; - std::string wallet_name = wallet.GetName() + "_watchonly"; + std::string wallet_name = MigrationPrefixName(wallet) + "_watchonly"; std::unique_ptr database = MakeWalletDatabase(wallet_name, options, status, error); if (!database) { error = strprintf(_("Wallet file creation failed: %s"), error); @@ -4365,7 +4403,7 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, DatabaseStatus status; std::vector warnings; - std::string wallet_name = wallet.GetName() + "_solvables"; + std::string wallet_name = MigrationPrefixName(wallet) + "_solvables"; std::unique_ptr database = MakeWalletDatabase(wallet_name, options, status, error); if (!database) { error = strprintf(_("Wallet file creation failed: %s"), error); @@ -4486,7 +4524,12 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr 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. @@ -4499,7 +4542,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // Make a backup of the DB fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path(); - fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", (wallet_name.empty() ? "default_wallet" : wallet_name), GetTime())); + fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", MigrationPrefixName(*local_wallet), GetTime())); fs::path backup_path = this_wallet_dir / backup_filename; if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { if (was_loaded) { @@ -4542,26 +4585,44 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr } } - // 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 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 wallet_files_to_remove; + std::set 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); } } @@ -4569,7 +4630,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // Migration failed, cleanup // Before deleting the wallet's directory, copy the backup file to the top-level wallets 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); // Make list of wallets to cleanup std::vector> created_wallets; @@ -4578,8 +4639,8 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr if (res.solvables_wallet) created_wallets.push_back(std::move(res.solvables_wallet)); // Get the directories to remove after unloading - for (std::shared_ptr& w : created_wallets) { - wallet_dirs.emplace(fs::PathFromString(w->GetDatabase().Filename()).parent_path()); + for (std::shared_ptr& wallet : created_wallets) { + track_for_cleanup(*wallet); } // Unload the wallets @@ -4598,9 +4659,15 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr } } - // 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 @@ -4614,8 +4681,7 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr } // The wallet directory has been restored, but just in case, copy the previously created 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); // Verify that there is no dangling wallet: when the wallet wasn't loaded before, expect null. // This check is performed after restoration to avoid an early error before saving the backup. diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py index c7abc2da8d..979804a5fe 100755 --- a/test/functional/tool_wallet.py +++ b/test/functional/tool_wallet.py @@ -409,6 +409,18 @@ class ToolWalletTest(BitcoinTestFramework): 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.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") + 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") def test_chainless_conflicts(self): self.log.info("Test wallet tool when wallet contains conflicting transactions") diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py index 7c88f64dcf..3ff798d3d3 100755 --- a/test/functional/wallet_backup.py +++ b/test/functional/wallet_backup.py @@ -40,6 +40,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( assert_equal, assert_raises_rpc_error, + sha256sum_file, ) @@ -136,10 +137,71 @@ class WalletBackupTest(BitcoinTestFramework): backup_file = self.nodes[0].datadir_path / 'wallet.bak' wallet_name = "res0" wallet_file = node.wallets_path / wallet_name - error_message = "Failed to create database path '{}'. Database already exists.".format(wallet_file) + 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 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 test_pruned_wallet_backup(self): self.log.info("Test loading backup on a pruned node when the backup was created close to the prune height of the restoring node") node = self.nodes[3] @@ -159,6 +221,19 @@ class WalletBackupTest(BitcoinTestFramework): # the backup to load successfully this close to the prune height node.restorewallet('pruned', node.datadir_path / 'wallet_pruned.bak') + self.log.info("Test restore on a pruned node when the backup was beyond the pruning point") + 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 = "" + error_message = "Wallet loading failed. Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)" + assert_raises_rpc_error(-4, error_message, node.restorewallet, wallet_name, backup_file) + assert node.wallets_path.exists() # ensure the wallets dir exists + 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") self.generate(self.nodes[0], 1) @@ -227,6 +302,8 @@ class WalletBackupTest(BitcoinTestFramework): 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 ce8dc19460..8129baf438 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -4,6 +4,8 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test Migrating a wallet from legacy to descriptor.""" +import os +from pathlib import Path import random import shutil import struct @@ -24,6 +26,7 @@ from test_framework.script import hash160 from test_framework.script_util import key_to_p2pkh_script, key_to_p2pk_script, script_to_p2sh_script, script_to_p2wsh_script from test_framework.util import ( assert_equal, + assert_greater_than, assert_raises_rpc_error, find_vout_for_address, sha256sum_file, @@ -522,6 +525,14 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(bals, wallet.getbalances()) + def clear_default_wallet(self, backup_file): + # Test cleanup: Clear unnamed default wallet for subsequent tests + (self.old_node.wallets_path / "wallet.dat").unlink() + (self.master_node.wallets_path / "wallet.dat").unlink(missing_ok=True) + shutil.rmtree(self.master_node.wallets_path / "default_wallet_watchonly", ignore_errors=True) + shutil.rmtree(self.master_node.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("") @@ -548,6 +559,103 @@ class WalletMigrationTest(BitcoinTestFramework): self.master_node.setmocktime(0) + 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") + master_wallet = self.master_node.get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet("", blank=True) + wallet.importaddress(master_wallet.getnewaddress(address_type="legacy")) + + res, def_wallet = self.migrate_and_get_rpc("") + wallet = self.master_node.get_wallet_rpc("default_wallet_watchonly") + + info = 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.master_node.wallets_path / "wallet.dat").exists() + def_wallet_info = def_wallet.getwalletinfo() + assert_equal(def_wallet_info["descriptors"], True) + assert_equal(def_wallet_info["format"], "sqlite") + assert_equal(def_wallet_info["private_keys_enabled"], True) + assert_equal(def_wallet_info["walletname"], "") + assert_greater_than(def_wallet_info["keypoolsize"], 0) + + wallet.unloadwallet() + self.clear_default_wallet(backup_file=Path(res["backup_path"])) + + 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.master_node.get_wallet_rpc(self.default_wallet_name) + wallet = self.create_legacy_wallet(wallet_name, blank=True) + wallet.importaddress(master_wallet.getnewaddress(address_type="legacy")) + wallet.unloadwallet() + + if os.path.isabs(wallet_name): + old_path = master_path = Path(wallet_name) + else: + old_path = self.old_node.wallets_path / wallet_name + master_path = self.master_node.wallets_path / wallet_name + os.makedirs(master_path, exist_ok=True) + shutil.copyfile(old_path / "wallet.dat", master_path / "wallet.dat") + + # 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.master_node.wallets_path / wo_dirname + os.mkdir(watch_only_dir) + shutil.copyfile(old_path / "wallet.dat", watch_only_dir / "wallet.dat") + + # Make a file in the wallets dir that must still exist after migration + survive_path = self.master_node.wallets_path / "survive" + open(survive_path, "wb").close() + assert survive_path.exists() + + mocked_time = int(time.time()) + self.master_node.setmocktime(mocked_time) + assert_raises_rpc_error(-4, "Failed to create database", self.master_node.migratewallet, wallet_name) + self.master_node.setmocktime(0) + + # Verify the /wallets/ path exists. + assert self.master_node.wallets_path.exists() + + # Verify survive is still there + assert survive_path.exists() + # Verify both wallet paths exist. + assert Path(old_path / "wallet.dat").exists() + assert Path(master_path / "wallet.dat").exists() + + backup_prefix = "default_wallet" if is_default else wallet_name + backup_path = master_path / f"{backup_prefix}_{mocked_time}.legacy.bak" + assert backup_path.exists() + + with open(self.master_node.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) + + + # 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(master_path / "wallet.dat").unlink() + Path(old_path / "wallet.dat").unlink(missing_ok=True) + 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") @@ -1357,6 +1465,42 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(addr_info["solvable"], True) assert "hex" in addr_info + def unsynced_wallet_on_pruned_node_fails(self): + self.log.info("Test migration of an unsynced wallet on a pruned node fails gracefully") + wallet = self.create_legacy_wallet("", load_on_startup=False) + last_wallet_synced_block = wallet.getwalletinfo()['lastprocessedblock']['height'] + wallet.unloadwallet() + + shutil.copyfile(self.old_node.wallets_path / "wallet.dat", self.master_node.wallets_path / "wallet.dat") + + # Generate blocks just so the wallet best block is pruned + self.restart_node(0, ["-fastprune", "-prune=1", "-nowallet"]) + self.connect_nodes(0, 1) + self.generate(self.master_node, 450, sync_fun=self.no_op) + self.master_node.pruneblockchain(250) + # Ensure next block to sync is unavailable + assert_raises_rpc_error(-1, "Block not available (pruned data)", self.master_node.getblock, self.master_node.getblockhash(last_wallet_synced_block + 1)) + + # Check migration failure + mocked_time = int(time.time()) + self.master_node.setmocktime(mocked_time) + 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)", self.master_node.migratewallet, wallet_name="") + self.master_node.setmocktime(0) + + # Verify the /wallets/ path exists, the wallet is still BDB and the backup file is there. + assert self.master_node.wallets_path.exists() + + with open(self.master_node.wallets_path / "wallet.dat", "rb") as f: + data = f.read(16) + _, _, magic = struct.unpack("QII", data) + assert_equal(magic, BTREE_MAGIC) + + backup_path = self.master_node.wallets_path / f"default_wallet_{mocked_time}.legacy.bak" + assert backup_path.exists() + + self.clear_default_wallet(backup_path) + + def run_test(self): self.master_node = self.nodes[0] self.old_node = self.nodes[1] @@ -1372,7 +1516,18 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_encrypted() self.test_nonexistent() self.test_unloaded_by_path() + + migration_failure_cases = [ + "", + "../", + os.path.abspath(self.master_node.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() self.test_addressbook() self.test_migrate_raw_p2sh() @@ -1390,5 +1545,8 @@ class WalletMigrationTest(BitcoinTestFramework): self.test_taproot() self.test_solvable_no_privs() + # Note: After this test the first 250 blocks of 'master_node' are pruned + self.unsynced_wallet_on_pruned_node_fails() + if __name__ == '__main__': WalletMigrationTest(__file__).main()