test: add coverage for unnamed wallet migration failure
What the developer wrote
Authored by furszy
test: add coverage for unnamed wallet migration failure
Verifies that a failed migration of the unnamed (default) wallet
does not erase the main /wallets/ directory, and also that the
backup file exists.
What changed, and why it matters
This commit only adds a new automated test to Bitcoin Core. It does not change any production wallet code. The test checks that if migrating the default (unnamed) wallet fails, the main wallets directory is not deleted, a backup file is created, and the original wallet is restored. It is a regression test for a previously fixed bug, not a security fix itself.
No action required; this is a test-only addition. Reviewers may optionally confirm the underlying migration behavior it tests is already present in the production code path.
Security signals we found
Regression test for migration failure path of unnamed wallet
Asserts backup file creation and main wallets directory preservation
No change to runtime wallet migration logic
Evidence from the diff
The diff adds test_default_wallet_failure() in test/functional/wallet_migration.py. It simulates a failed migration of the legacy unnamed wallet by pre-creating a conflicting _watchonly directory, then asserts that migratewallet fails with ‘Failed to create database’, that /wallets/ still exists, that a default_wallet_<time>.legacy.bak backup exists, and that the original BDB wallet.dat is restored. No C++ or Python production code is modified.
Changed components
test/functional/wallet_migration.pyInspect captured patch +33 / −0
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index 89989fbf..27caab8f 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -675,6 +675,38 @@ class WalletMigrationTest(BitcoinTestFramework):
# migrate_and_get_rpc already checks for backup file existence
assert os.path.basename(res["backup_path"]).startswith("default_wallet")
+ wallet.unloadwallet()
+
+ def test_default_wallet_failure(self):
+ self.log.info("Test failure during unnamed (default) wallet migration")
+ master_wallet = self.master_node.get_wallet_rpc(self.default_wallet_name)
+ wallet = self.create_legacy_wallet("", blank=True)
+ wallet.importaddress(master_wallet.getnewaddress(address_type="legacy"))
+
+ # Create wallet directory with the watch-only name and a wallet file.
+ # Because the wallet dir exists, this will cause migration to fail.
+ watch_only_dir = self.master_node.wallets_path / "_watchonly"
+ os.mkdir(watch_only_dir)
+ shutil.copyfile(self.old_node.wallets_path / "wallet.dat", watch_only_dir / "wallet.dat")
+
+ mocked_time = int(time.time())
+ self.master_node.setmocktime(mocked_time)
+ assert_raises_rpc_error(-4, "Failed to create database", self.migrate_and_get_rpc, "")
+ self.master_node.setmocktime(0)
+
+ # Verify the /wallets/ path exists
+ assert self.master_node.wallets_path.exists()
+ # Check backup file exists. Because the wallet has no name, the backup is prefixed with 'default_wallet'
+ backup_path = self.master_node.wallets_path / f"default_wallet_{mocked_time}.legacy.bak"
+ assert backup_path.exists()
+ # Verify the original unnamed wallet was restored
+ assert (self.master_node.wallets_path / "wallet.dat").exists()
+ # And verify it is still a BDB wallet
+ self.assert_is_bdb("")
+
+ # Test cleanup: clear default wallet for next test
+ os.remove(self.old_node.wallets_path / "wallet.dat")
+
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")
@@ -1560,6 +1592,7 @@ class WalletMigrationTest(BitcoinTestFramework):
self.test_wallet_with_relative_path()
self.test_wallet_with_path("path/to/mywallet/")
self.test_wallet_with_path("path/that/ends/in/..")
+ self.test_default_wallet_failure()
self.test_default_wallet()
self.test_direct_file()
self.test_addressbook()
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.