Repository: bitcoin/bitcoin (language: cpp)

Issue:
migratewallet: backup file has inconvenient name
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

The `migratewallet` RPC can create a backup named (something like) `-998239328.legacy.bak`. Files starting with a `-` are somewhat inconvenient to work with in UNIX, because commands confuse them with a flag, especally when using `*`. It's usually possible to work around this with `--` but still, it would be more convenient to use positive numbers only.

### Expected behaviour

It would be more convenient to use positive numbers only.

### Steps to reproduce

`bitcoin-cli migratewallet ""`

### Relevant log output

N/A

### How did you obtain Bitcoin Core

Compiled from source

### What version of Bitcoin Core are you using?

v26.0

### Operating system and version

Debian Linux

### Machine specifications

_No response_


Discussion / hints:
> more convenient to use positive numbers only

It isn't a negative number. It is just a separator. The file format is "<wallet_name>-<current_time>.legacy.bak" (guess that you migrated the default wallet, thus the empty wallet_name). But sure, it could easily be replaced for an underscore separator.
Yes, I migrated the default wallet. In that case, instead of changing the seperator, substituting something else instead of "" would work too. It's fine to have `-` in a filename, just not as first character.
> Yes, I migrated the default wallet. In that case, instead of changing the seperator, substituting something else instead of "" would work too. It's fine to have `-` in a filename, just not as first character.

Yep sure. Already working on it.

Fix patch:
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 3ac09430d83c3..9c15c2a82756c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -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) {


Test patch:
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
index f9919716be655..890b6a5c1bb7b 100755
--- a/test/functional/wallet_migration.py
+++ b/test/functional/wallet_migration.py
@@ -529,11 +529,20 @@ def test_default_wallet(self):
         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")