Skip to content

Commit

Permalink
Merge bitcoin#26740: wallet: Migrate wallets that are not in a wallet…
Browse files Browse the repository at this point in the history
… dir

a1e6538 test: Add test for migrating default wallet and plain file wallet (Andrew Chow)
bdbe3fd wallet: Generated migrated wallet's path from walletdir and name (Andrew Chow)

Pull request description:

  This PR fixes an assertion error that is hit during the setup of the new database during migration of a wallet that was not contained in a wallet dir. Also added a test for this case as well as one for migrating the default wallet.

ACKs for top commit:
  ryanofsky:
    Code review ACK a1e6538
  furszy:
    ACK a1e6538

Tree-SHA512: 96b218c0de8567d8650ec96e1bf58b0f8ca4c4726f5efc6362453979b56b9d569baea0bb09befb3a5aed8d16d29bf75ed5cd8ffc432bbd4cbcad3ac5574bc479
  • Loading branch information
ryanofsky committed Jun 20, 2023
2 parents e4bbfb2 + a1e6538 commit ee22ca5
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3856,16 +3856,19 @@ bool CWallet::MigrateToSQLite(bilingual_str& error)

// Close this database and delete the file
fs::path db_path = fs::PathFromString(m_database->Filename());
fs::path db_dir = db_path.parent_path();
m_database->Close();
fs::remove(db_path);

// Generate the path for the location of the migrated wallet
// Wallets that are plain files rather than wallet directories will be migrated to be wallet directories.
const fs::path wallet_path = fsbridge::AbsPathJoin(GetWalletDir(), fs::PathFromString(m_name));

// Make new DB
DatabaseOptions opts;
opts.require_create = true;
opts.require_format = DatabaseFormat::SQLITE;
DatabaseStatus db_status;
std::unique_ptr<WalletDatabase> new_db = MakeDatabase(db_dir, opts, db_status, error);
std::unique_ptr<WalletDatabase> new_db = MakeDatabase(wallet_path, opts, db_status, error);
assert(new_db); // This is to prevent doing anything further with this wallet. The original file was deleted, but a backup exists.
m_database.reset();
m_database = std::move(new_db);
Expand Down
37 changes: 37 additions & 0 deletions test/functional/wallet_migration.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import os
import random
import shutil
from test_framework.descriptors import descsum_create
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
Expand Down Expand Up @@ -470,6 +471,40 @@ def test_unloaded_by_path(self):

assert_equal(bals, wallet.getbalances())

def test_default_wallet(self):
self.log.info("Test migration of the wallet named as the empty string")
wallet = self.create_legacy_wallet("")

wallet.migratewallet()
info = wallet.getwalletinfo()
assert_equal(info["descriptors"], True)
assert_equal(info["format"], "sqlite")

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")
wallet.unloadwallet()

wallets_dir = os.path.join(self.nodes[0].datadir, "regtest", "wallets")
wallet_path = os.path.join(wallets_dir, "plainfile")
wallet_dat_path = os.path.join(wallet_path, "wallet.dat")
shutil.copyfile(wallet_dat_path, os.path.join(wallets_dir, "plainfile.bak"))
shutil.rmtree(wallet_path)
shutil.move(os.path.join(wallets_dir, "plainfile.bak"), wallet_path)

self.nodes[0].loadwallet("plainfile")
info = wallet.getwalletinfo()
assert_equal(info["descriptors"], False)
assert_equal(info["format"], "bdb")

wallet.migratewallet()
info = wallet.getwalletinfo()
assert_equal(info["descriptors"], True)
assert_equal(info["format"], "sqlite")

assert os.path.isdir(wallet_path)
assert os.path.isfile(wallet_dat_path)

def run_test(self):
self.generate(self.nodes[0], 101)

Expand All @@ -482,6 +517,8 @@ def run_test(self):
self.test_encrypted()
self.test_unloaded()
self.test_unloaded_by_path()
self.test_default_wallet()
self.test_direct_file()

if __name__ == '__main__':
WalletMigrationTest().main()

0 comments on commit ee22ca5

Please sign in to comment.