From 3ddbdd1815c676a88345b3b0e55a551d2a569e28 Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 10 Jun 2024 17:13:07 -0400 Subject: [PATCH 1/2] wallet: Ignore .bak files when listing wallet files Migration creates backup files in the wallet directory with .bak as the extension. This pollutes the output of listwalletdir with backup files that most users should not need to care about. --- src/wallet/db.cpp | 2 +- test/functional/wallet_migration.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index a5a5f8ec6f2..f0c78e72032 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -42,7 +42,7 @@ std::vector ListDatabases(const fs::path& wallet_dir) (IsBDBFile(BDBDataFile(it->path())) || IsSQLiteFile(SQLiteDataFile(it->path())))) { // Found a directory which contains wallet.dat btree file, add it as a wallet. paths.emplace_back(path); - } else if (it.depth() == 0 && it->symlink_status().type() == fs::file_type::regular && IsBDBFile(it->path())) { + } else if (it.depth() == 0 && it->symlink_status().type() == fs::file_type::regular && IsBDBFile(it->path()) && it->path().extension() != ".bak") { if (it->path().filename() == "wallet.dat") { // Found top-level wallet.dat btree file, add top level directory "" // as a wallet. diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 890b6a5c1bb..88a1a0cae58 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -538,10 +538,14 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(info["descriptors"], True) assert_equal(info["format"], "sqlite") + walletdir_list = wallet.listwalletdir() + # Check backup existence and its non-empty wallet filename - backup_path = self.nodes[0].wallets_path / f'default_wallet_{curr_time}.legacy.bak' + backup_filename = f"default_wallet_{curr_time}.legacy.bak" + backup_path = self.nodes[0].wallets_path / backup_filename assert backup_path.exists() assert_equal(str(backup_path), res['backup_path']) + assert {"name": backup_filename} not in walletdir_list["wallets"] def test_direct_file(self): self.log.info("Test migration of a wallet that is not in a wallet directory") From 6b2dcba07670f04f32c0dc3a2c86fd805c85f12d Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Mon, 10 Jun 2024 17:23:55 -0400 Subject: [PATCH 2/2] wallet: List sqlite wallets with empty string name Although it is not explicitly possible to create a default wallet with descriptors, it is possible to migrate a default wallet and have it end up being a default wallet with descriptors. These wallets should be listed by ListDatabases so that it appears in wallet directory listings to avoid user confusion. --- src/wallet/db.cpp | 4 ++-- test/functional/wallet_migration.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index f0c78e72032..1523b7d6133 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -42,12 +42,12 @@ std::vector ListDatabases(const fs::path& wallet_dir) (IsBDBFile(BDBDataFile(it->path())) || IsSQLiteFile(SQLiteDataFile(it->path())))) { // Found a directory which contains wallet.dat btree file, add it as a wallet. paths.emplace_back(path); - } else if (it.depth() == 0 && it->symlink_status().type() == fs::file_type::regular && IsBDBFile(it->path()) && it->path().extension() != ".bak") { + } else if (it.depth() == 0 && it->symlink_status().type() == fs::file_type::regular && it->path().extension() != ".bak") { if (it->path().filename() == "wallet.dat") { // Found top-level wallet.dat btree file, add top level directory "" // as a wallet. paths.emplace_back(); - } else { + } else if (IsBDBFile(it->path())) { // Found top-level btree file not called wallet.dat. Current bitcoin // software will never create these files but will allow them to be // opened in a shared database environment for backwards compatibility. diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 88a1a0cae58..e2c8671e1cc 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -539,6 +539,7 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(info["format"], "sqlite") walletdir_list = wallet.listwalletdir() + assert {"name": info["walletname"]} in walletdir_list["wallets"] # Check backup existence and its non-empty wallet filename backup_filename = f"default_wallet_{curr_time}.legacy.bak"