diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8a79cf730b..ea7928ab4f 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2923,7 +2923,7 @@ bool CWallet::EraseAddressReceiveRequest(WalletBatch& batch, const CTxDestinatio return true; } -std::unique_ptr MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error_string) +static util::Result GetWalletPath(const std::string& name) { // Do some checking on wallet path. It should be either a: // @@ -2936,15 +2936,24 @@ std::unique_ptr MakeWalletDatabase(const std::string& name, cons if (!(path_type == fs::file_type::not_found || path_type == fs::file_type::directory || (path_type == fs::file_type::symlink && fs::is_directory(wallet_path)) || (path_type == fs::file_type::regular && fs::PathFromString(name).filename() == fs::PathFromString(name)))) { - error_string = Untranslated(strprintf( + return util::Error{Untranslated(strprintf( "Invalid -wallet path '%s'. -wallet path should point to a directory where wallet.dat and " "database/log.?????????? files can be stored, a location where such a directory could be created, " "or (for backwards compatibility) the name of an existing data file in -walletdir (%s)", - name, fs::quoted(fs::PathToString(GetWalletDir())))); + name, fs::quoted(fs::PathToString(GetWalletDir()))))}; + } + return wallet_path; +} + +std::unique_ptr MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error_string) +{ + const auto& wallet_path = GetWalletPath(name); + if (!wallet_path) { + error_string = util::ErrorString(wallet_path); status = DatabaseStatus::FAILED_BAD_PATH; return nullptr; } - return MakeDatabase(wallet_path, options, status, error_string); + return MakeDatabase(*wallet_path, options, status, error_string); } std::shared_ptr CWallet::Create(WalletContext& context, const std::string& name, std::unique_ptr database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector& warnings) @@ -4346,11 +4355,24 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle // 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; 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")}; } UnloadWallet(std::move(wallet)); was_loaded = true; + } else { + // Check if the wallet is BDB + const auto& wallet_path = GetWalletPath(wallet_name); + if (!wallet_path) { + return util::Error{util::ErrorString(wallet_path)}; + } + if (!IsBDBFile(BDBDataFile(*wallet_path))) { + return util::Error{_("Error: This wallet is already a descriptor wallet")}; + } } // Load the wallet but only in the context of this function. @@ -4359,6 +4381,7 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle empty_context.args = context.args; DatabaseOptions options; options.require_existing = true; + options.require_format = DatabaseFormat::BERKELEY_RO; DatabaseStatus status; std::unique_ptr database = MakeWalletDatabase(wallet_name, options, status, error); if (!database) { @@ -4373,6 +4396,8 @@ util::Result MigrateLegacyToDescriptor(const std::string& walle // Helper to reload as normal for some of our exit scenarios const auto& reload_wallet = [&](std::shared_ptr& to_reload) { + // Reset options.require_format as wallets of any format may be reloaded. + options.require_format = std::nullopt; assert(to_reload.use_count() == 1); std::string name = to_reload->GetName(); to_reload.reset(); diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 890b6a5c1b..aa0aefa7b5 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -205,9 +205,13 @@ class WalletMigrationTest(BitcoinTestFramework): self.assert_list_txs_equal(basic2.listtransactions(), basic2_txs) # Now test migration on a descriptor wallet - self.log.info("Test \"nothing to migrate\" when the user tries to migrate a wallet with no legacy data") + self.log.info("Test \"nothing to migrate\" when the user tries to migrate a loaded wallet with no legacy data") assert_raises_rpc_error(-4, "Error: This wallet is already a descriptor wallet", basic2.migratewallet) + self.log.info("Test \"nothing to migrate\" when the user tries to migrate an unloaded wallet with no legacy data") + basic2.unloadwallet() + assert_raises_rpc_error(-4, "Error: This wallet is already a descriptor wallet", self.nodes[0].migratewallet, "basic2") + def test_multisig(self): default = self.nodes[0].get_wallet_rpc(self.default_wallet_name)