Merge bitcoin/bitcoin#29586: wallet: default wallet migration, modify inconvenient backup filename

a951dba3a9 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 a951dba3a9
  brunoerg:
    utACK a951dba3a9
  vasild:
    ACK  a951dba3a9

Tree-SHA512: 6347bb12cfb50526a4baad96e4f1df9d82b493f79f0a0f7e0a1c8335a86a1e8e147c7b7f95cec6ede6f4507506a7eaf7972bd35131a2d5ed4cbbf38d94f0a9ca
This commit is contained in:
Ava Chow 2024-03-11 08:16:07 -04:00
commit b4a05751b6
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
2 changed files with 11 additions and 2 deletions

View File

@ -4336,7 +4336,7 @@ util::Result<MigrationResult> MigrateLegacyToDescriptor(const std::string& walle
// Make a backup of the DB
fs::path this_wallet_dir = fs::absolute(fs::PathFromString(local_wallet->GetDatabase().Filename())).parent_path();
fs::path backup_filename = fs::PathFromString(strprintf("%s-%d.legacy.bak", wallet_name, GetTime()));
fs::path backup_filename = fs::PathFromString(strprintf("%s_%d.legacy.bak", (wallet_name.empty() ? "default_wallet" : wallet_name), GetTime()));
fs::path backup_path = this_wallet_dir / backup_filename;
if (!local_wallet->BackupWallet(fs::PathToString(backup_path))) {
if (was_loaded) {

View File

@ -529,11 +529,20 @@ class WalletMigrationTest(BitcoinTestFramework):
self.log.info("Test migration of the wallet named as the empty string")
wallet = self.create_legacy_wallet("")
self.migrate_wallet(wallet)
# Set time to verify backup existence later
curr_time = int(time.time())
wallet.setmocktime(curr_time)
res = self.migrate_wallet(wallet)
info = wallet.getwalletinfo()
assert_equal(info["descriptors"], True)
assert_equal(info["format"], "sqlite")
# Check backup existence and its non-empty wallet filename
backup_path = self.nodes[0].wallets_path / f'default_wallet_{curr_time}.legacy.bak'
assert backup_path.exists()
assert_equal(str(backup_path), res['backup_path'])
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")