diff --git a/src/bench/wallet_migration.cpp b/src/bench/wallet_migration.cpp index 52da05367f2..0ce23a49b4e 100644 --- a/src/bench/wallet_migration.cpp +++ b/src/bench/wallet_migration.cpp @@ -66,7 +66,7 @@ static void WalletMigration(benchmark::Bench& bench) bench.epochs(/*numEpochs=*/1).epochIterations(/*numIters=*/1) // run the migration exactly once .run([&] { - auto res{MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", *loader->context(), /*was_loaded=*/false)}; + auto res{MigrateLegacyToDescriptor(std::move(wallet), /*passphrase=*/"", *loader->context())}; assert(res); assert(res->wallet); assert(res->watchonly_wallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3f2d1a38f00..ca73486aeee 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -4174,18 +4174,10 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle std::vector warnings; bilingual_str error; - // If the wallet is still loaded, unload it so that nothing else tries to use it while we're changing it - bool was_loaded = false; + // The only kind of wallets that could be loaded are descriptor ones, which don't need to be migrated. if (auto wallet = GetWallet(context, wallet_name)) { - if (wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { - return util::Error{_("Error: This wallet is already a descriptor wallet")}; - } - - if (!RemoveWallet(context, wallet, /*load_on_start=*/std::nullopt, warnings)) { - return util::Error{_("Unable to unload the wallet before migrating")}; - } - WaitForDeleteWallet(std::move(wallet)); - was_loaded = true; + assert(wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); + return util::Error{_("Error: This wallet is already a descriptor wallet")}; } else { // Check if the wallet is BDB const auto& wallet_path = GetWalletPath(wallet_name); @@ -4219,10 +4211,10 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle return util::Error{Untranslated("Wallet loading failed.") + Untranslated(" ") + error}; } - return MigrateLegacyToDescriptor(std::move(local_wallet), passphrase, context, was_loaded); + return MigrateLegacyToDescriptor(std::move(local_wallet), passphrase, context); } -util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded) +util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context) { MigrationResult res; bilingual_str error; @@ -4245,9 +4237,6 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // Before anything else, check if there is something to migrate. if (local_wallet->IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) { - if (was_loaded) { - reload_wallet(local_wallet); - } return util::Error{_("Error: This wallet is already a descriptor wallet")}; } @@ -4256,9 +4245,6 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", (wallet_name.empty() ? "default_wallet" : wallet_name), GetTime())); fs::path backup_path = this_wallet_dir / backup_filename; if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) { - if (was_loaded) { - reload_wallet(local_wallet); - } return util::Error{_("Error: Unable to make a backup of your wallet")}; } res.backup_path = backup_path; @@ -4267,9 +4253,6 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // Unlock the wallet if needed if (local_wallet->IsLocked() && !local_wallet->Unlock(passphrase)) { - if (was_loaded) { - reload_wallet(local_wallet); - } if (passphrase.find('\0') == std::string::npos) { return util::Error{Untranslated("Error: Wallet decryption failed, the wallet passphrase was not provided or was incorrect.")}; } else { @@ -4379,23 +4362,19 @@ util::Result MigrateLegacyToDescriptor(std::shared_ptr // Restore the backup // 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. bilingual_str restore_error; - const auto& ptr_wallet = RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings, /*load_after_restore=*/was_loaded); + const auto& ptr_wallet = RestoreWallet(context, temp_backup_location, wallet_name, /*load_on_start=*/std::nullopt, status, restore_error, warnings, /*load_after_restore=*/false); if (!restore_error.empty()) { error += restore_error + _("\nUnable to restore backup of wallet."); return util::Error{error}; } + // Verify that the legacy wallet is not loaded after restoring from the backup. + assert(!ptr_wallet); // 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); - // 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. - bool wallet_reloaded = ptr_wallet != nullptr; - assert(was_loaded == wallet_reloaded); - return util::Error{error}; } return res; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 2db522c4f4d..0d5d1fba1db 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1137,7 +1137,7 @@ struct MigrationResult { //! Do all steps to migrate a legacy wallet to a descriptor wallet [[nodiscard]] util::Result MigrateLegacyToDescriptor(const std::string& wallet_name, const SecureString& passphrase, WalletContext& context); //! Requirement: The wallet provided to this function must be isolated, with no attachment to the node's context. -[[nodiscard]] util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context, bool was_loaded); +[[nodiscard]] util::Result MigrateLegacyToDescriptor(std::shared_ptr local_wallet, const SecureString& passphrase, WalletContext& context); } // namespace wallet #endif // BITCOIN_WALLET_WALLET_H