Change MigrateLegacyToDescriptor to reopen wallet as BERKELEY_RO

When we reopen the wallet to do the migration, instead of opening using
BDB, open it using the BerkeleyRO implementation.
This commit is contained in:
Ava Chow 2024-01-05 18:36:19 -05:00
parent a786fd2041
commit 517e204bac
2 changed files with 34 additions and 5 deletions

View File

@ -2923,7 +2923,7 @@ bool CWallet::EraseAddressReceiveRequest(WalletBatch& batch, const CTxDestinatio
return true;
}
std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error_string)
static util::Result<fs::path> GetWalletPath(const std::string& name)
{
// Do some checking on wallet path. It should be either a:
//
@ -2936,15 +2936,24 @@ std::unique_ptr<WalletDatabase> 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<WalletDatabase> 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> CWallet::Create(WalletContext& context, const std::string& name, std::unique_ptr<WalletDatabase> database, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings)
@ -4346,11 +4355,24 @@ util::Result<MigrationResult> 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<MigrationResult> 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<WalletDatabase> database = MakeWalletDatabase(wallet_name, options, status, error);
if (!database) {
@ -4373,6 +4396,8 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
// Helper to reload as normal for some of our exit scenarios
const auto& reload_wallet = [&](std::shared_ptr<CWallet>& 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();

View File

@ -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)