Skip to content

Commit

Permalink
wallet: Reload watchonly and solvables wallets after migration
Browse files Browse the repository at this point in the history 
When migrating, create the watchonly and solvables wallets without a
context. Then unload and reload them after migration completes, as we do
for the actual wallet.

There is also additional handling for a failed reload.
  • Loading branch information
@achow101
achow101 committed Oct 17, 2023
1 parent 5ae2af5 commit 197a625
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 24 deletions.
81 changes: 58 additions & 23 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4079,6 +4079,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;
Expand All @@ -4094,8 +4098,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;
}
Expand Down Expand Up @@ -4125,8 +4135,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;
}
Expand Down Expand Up @@ -4229,47 +4245,66 @@ 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
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.
const auto& reload_wallet = [&](std::shared_ptr<CWallet>& to_reload) {
assert(to_reload.use_count() == 1);
std::string name = to_reload->GetName();
wallet_dirs.insert(fs::PathFromString(to_reload->GetDatabase().Filename()).parent_path());
to_reload.reset();
to_reload = LoadWallet(context, name, /*load_on_start=*/std::nullopt, options, status, error, warnings);
return to_reload != nullptr;
};
// Reload the main wallet
success = reload_wallet(local_wallet);
res.wallet = local_wallet;
res.wallet_name = wallet_name;
} else {
if (success && res.watchonly_wallet) {
// Reload watchonly
success = reload_wallet(res.watchonly_wallet);
}
if (success && res.solvables_wallet) {
// Reload solvables
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();

// 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);
}

Expand Down
2 changes: 1 addition & 1 deletion test/functional/wallet_migration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ def test_failed_migration_cleanup(self):
wallet.addmultisigaddress(2, [wallet.getnewaddress(), get_generate_key().pubkey])
wallet.importaddress(get_generate_key().p2pkh_addr)

assert_raises_rpc_error(-4, "Failed to create new watchonly wallet", wallet.migratewallet)
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()
Expand Down

0 comments on commit 197a625

Please sign in to comment.