Skip to content

Commit b4a0575

Browse files
committed
Merge bitcoin/bitcoin#29586: wallet: default wallet migration, modify inconvenient backup filename
a951dba wallet: default wallet migration, modify inconvenient backup filename (furszy) Pull request description: Fixes #29584 On default legacy wallets, the backup filename starts with an "-" due to the wallet name being empty. This is inconvenient for systems who treat what follows the initial "-" character as flags. Note: As the user can freely set the wallet name to anything, we could also guard the backup filename against other inconvenient characters in the future (we need to be careful here, because the wallet name could also be a path). ACKs for top commit: achow101: ACK a951dba brunoerg: utACK a951dba vasild: ACK a951dba Tree-SHA512: 6347bb12cfb50526a4baad96e4f1df9d82b493f79f0a0f7e0a1c8335a86a1e8e147c7b7f95cec6ede6f4507506a7eaf7972bd35131a2d5ed4cbbf38d94f0a9ca
2 parents 4a90374 + a951dba commit b4a0575

File tree

2 files changed

+11
-2
lines changed

2 files changed

+11
-2
lines changed

src/wallet/wallet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4336,7 +4336,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
43364336

43374337
// Make a backup of the DB
43384338
fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path();
4339-
fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", wallet_name, GetTime()));
4339+
fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", (wallet_name.empty() ? "default_wallet" : wallet_name), GetTime()));
43404340
fs::path backup_path = this_wallet_dir / backup_filename;
43414341
if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) {
43424342
if (was_loaded) {

test/functional/wallet_migration.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,11 +529,20 @@ def test_default_wallet(self):
529529
self.log.info("Test migration of the wallet named as the empty string")
530530
wallet = self.create_legacy_wallet("")
531531

532-
self.migrate_wallet(wallet)
532+
# Set time to verify backup existence later
533+
curr_time = int(time.time())
534+
wallet.setmocktime(curr_time)
535+
536+
res = self.migrate_wallet(wallet)
533537
info = wallet.getwalletinfo()
534538
assert_equal(info["descriptors"], True)
535539
assert_equal(info["format"], "sqlite")
536540

541+
# Check backup existence and its non-empty wallet filename
542+
backup_path = self.nodes[0].wallets_path / f'default_wallet_{curr_time}.legacy.bak'
543+
assert backup_path.exists()
544+
assert_equal(str(backup_path), res['backup_path'])
545+
537546
def test_direct_file(self):
538547
self.log.info("Test migration of a wallet that is not in a wallet directory")
539548
wallet = self.create_legacy_wallet("plainfile")

0 commit comments

Comments
 (0)