Skip to content

Commit acddb7b

Browse files
glozowvijaydasmp
authored andcommitted
Merge bitcoin#30265: wallet: Fix listwalletdir listing of migrated default wallets and generated backup files
6b2dcba wallet: List sqlite wallets with empty string name (Ava Chow) 3ddbdd1 wallet: Ignore .bak files when listing wallet files (Ava Chow) Pull request description: When the default wallet is migrated, we do not rename the wallet so we end up having a descriptor wallet with the empty string as its name and the wallet.dat file in the root of the walletdir. This is supposed to be an unsupported configuration and there is no other way to achieve this (other than file copying), but the wallet loading code does not disallow loading such wallets. However `listwalletdir` does not currently list the default wallet if it is sqlite. This is confusing to users, so change `listwalletdir` to include these wallets. Additionally, the migration of the default wallet, and of any plain wallet files in the walletdir, produces a backup file in the walletdir itself. Since these backups are a BDB file, `listwalletdir` will detect them as being another wallet that we could open, but this is erroneous and could lead to confusion and potentially funds loss if both the backup and the migrated wallet are in use simultaneously. To reduce the likelihood of this issue, don't list these wallets in `listwalletdir`. *** Possibly we could have more stringent checks on loading to resolve these issues, but I'm concerned that that will just confuse users and gratuitously break things that already worked. Since the original intent was to disallow default wallets for sqlite/descriptors, a possible alternative would be to prevent people from loading such wallets and change migration to rename those wallets. However, given that this behavior with migrating default wallets has existed since default wallet migration was fixed, I think that making such a change would be confusing and break things for no good reason. Although perhaps we should still do the renaming. For the backups, we could also change loading to refuse to load any wallet named with `.bak` (or `.legacy.bak`) as such wallets can still be loaded by giving the path to them directly, which some users may do to "restore" the backup. However restricting what can be loaded based on filename seems a little heavyhanded. It wouldn't be funds loss though since the correct way to restore the backup is with `restorewallet`. ACKs for top commit: fjahr: Code review ACK 6b2dcba furszy: Code ACK 6b2dcba glozow: ACK 6b2dcba Tree-SHA512: 0b033f6ed55830f8a054afea3fb2cf1fa82a94040053ebfaf123bda36c99f45d3f01a2aec4ed02fed9c61bb3d320b047ed892d7f6644b5a356a7bc5974b10cff
1 parent 03a8abe commit acddb7b

File tree

1 file changed

+2
-2
lines changed

1 file changed

+2
-2
lines changed

src/wallet/db.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ std::vector<fs::path> ListDatabases(const fs::path& wallet_dir)
3737
(IsBDBFile(BDBDataFile(it->path())) || IsSQLiteFile(SQLiteDataFile(it->path())))) {
3838
// Found a directory which contains wallet.dat btree file, add it as a wallet.
3939
paths.emplace_back(path);
40-
} else if (it.depth() == 0 && it->symlink_status().type() == fs::file_type::regular && IsBDBFile(it->path())) {
40+
} else if (it.depth() == 0 && it->symlink_status().type() == fs::file_type::regular && it->path().extension() != ".bak") {
4141
if (it->path().filename() == "wallet.dat") {
4242
// Found top-level wallet.dat btree file, add top level directory ""
4343
// as a wallet.
4444
paths.emplace_back();
45-
} else {
45+
} else if (IsBDBFile(it->path())) {
4646
// Found top-level btree file not called wallet.dat. Current bitcoin
4747
// software will never create these files but will allow them to be
4848
// opened in a shared database environment for backwards compatibility.

0 commit comments

Comments
 (0)