utils: wallet_dump can create a `database` directory, cross-pollinating records

laanwj commented at 7:20 pm on April 15, 2024: member

Is there an existing issue for this?

I have searched the existing issues

Current behaviour

I created a large directory with legacy wallet dat files for testing #26606. Ran into a weird issue where the output mismatches, but after investigating deeper, this was not due to a bug in that PR.

So I created a script that iterates over all wallets, dumps them with the internal bdb and external bdb, diffs the textual output, stopping on the first wallet which mismatches. Mind that the wallets are loose .dat files, not themselves subdirectories.

 0#!/bin/bash
 1set -e
 2DATADIR="$HOME/.../2024-04-testwallets"
 3for WALLET in a.dat b.dat c.dat d.dat ...; do
 4    echo $WALLET
 5    rm -f /tmp/dump /tmp/dump2
 6    src/bitcoin-wallet -datadir="$DATADIR" -withinternalbdb -wallet="$WALLET" -dumpfile=/tmp/dump dump
 7    src/bitcoin-wallet -datadir="$DATADIR" -wallet="$WALLET" -dumpfile=/tmp/dump2 dump
 8
 9    diff -q /tmp/dump /tmp/dump2
10
11    ### Uncomment to following to make it work:
12    # rm -rf $DATADIR/database. 
13done
14rm -f /tmp/dump /tmp/dump2

So far so good.

However, I was seeing some huge divergences. Not small differences that could be explained to differences in interpretation, but e.g. the output would have wildly different blockheight. A kind of cross-pollination.

What it turned out to be is that the bitcoin-wallet dump leaves behind a database/ subdirectory on close containing log files (log.0000000001 etc). When opening the next wallet in the same directory, this subdirectory is used, and affects the dumped contents of the new wallet.

Deleting the database directory between wallets solves the problem. If this is still worth fixing, the wallet should probably compact after close to incorporate the log files. You wouldn’t expect modification on a read-only operation in the first place, but I suppose this is due to an inherent bdb limitation.

Expected behaviour

Successively dumped wallets don’t affect each other.

Steps to reproduce

See script in “current behavior”.

Relevant log output

No response

How did you obtain Bitcoin Core

Compiled from source

What version of Bitcoin Core are you using?

7f8d4c963c6a12da0e8df462a3f25ccda4a39a62 (#26606)

Operating system and version

Ubuntu 24.04

Machine specifications

No response

laanwj added the label Wallet on Apr 15, 2024

laanwj commented at 8:03 pm on April 15, 2024: member

Oh, i think part of the trigger here is that some of the wallets are backups of the same database at different times, so the logs can get re-used because the unique id matches (?).

achow101 commented at 10:34 pm on April 15, 2024: member

Hmm, the contents of the log files shouldn’t matter since the database files have their LSNs reset when the wallet is closed. Is it possible that the wallet files were last opened in an unclean shutdown, or maybe with older versions that didn’t always flush on shutdown?

Also, yes, having duplicate files will result in the same unique id which means that the log files can be applied to the wrong file, which will really mess things up.

laanwj commented at 4:41 am on April 16, 2024: member

Is it possible that the wallet files were last opened in an unclean shutdown, or maybe with older versions that didn’t always flush on shutdown?

Yes, this is possible, it’s a random grab bag of wallet database files (not even sure there’s not a litecoin or dogecoin one in there).

Also, yes, having duplicate files will result in the same unique id which means that the log files can be applied to the wrong file, which will really mess things up.

Honestly after this train wreck i’m at the point where i am more confident about your bdb parsing code to get things right than bdb itself.

laanwj commented at 6:07 am on April 16, 2024: member

You were right: with the LSN check, one of the wallets (a backup) from 2015 trips up:

0terminate called after throwing an instance of 'std::runtime_error'
1  what():  LSNs are not reset, this database is not completely flushed. Please reopen then close the database with a version that has BDB support

bitcoin deleted a comment on May 11, 2024

utils: wallet_dump can create a database directory, cross-pollinating records #29883