rc6 crashes trying to rewrite wallet #644

I've been testing the release candidates. rc1 through rc3 seem OK, but rc4 through rc6 all crash when I try to start them with my existing excrypted wallet:

$ ./bitcoin-qt QGtkStyle was unable to detect the current GTK+ theme.

EXCEPTION: 11DbException
Db::open: Invalid argument
bitcoin in Runaway exception

terminate called after throwing an instance of 'DbException' what(): Db::open: Invalid argument Aborted $

The exception is thrown by the "ret = pdbCopy->open(NULL, ...) in CDB::Rewrite() in db.cpp

I'm running a 64 bit archlinux, and am linking against package Berkeley DB ('db') version 5.2.36-2 because I couldn't find a version 4.

Is that likely to be causing this problem?

I'm pretty sure I had rc4 manage to open my wallet one time, but can't get it to work now.

I found a version 4.8 of db, so will try building against that.

It's here:

http://aur.archlinux.org/packages.php?ID=43907

Cannot reproduce using Ubuntu 10.10 server.

I've been able to reproduce it several times, but not reliably.

It does seem to trash my wallet when I do, however. Luckily I made a backup before running rc6 for the first time so didn't lose any funds. I wonder if it would be good to always have bitcoin keep a copy of the previous version of the wallet in case this or other bugs cause wallet corruption. If we only update the backup each time we successfully load a wallet we should never have the user be without any working copy of their wallet.

I'll try again to find a reliable way to reproduce this problem.

OK, this seems to do it:

/bin/rm -fr ~/.bitcoin; rc1-db4/bitcoin-qt -keypool=3; rc6-db4/bitcoin-qt -keypool=3; rc6-db4/bitcoin-qt -keypool=3

Remove .bitcoind, create it using rc1 and immediately encrypt the wallet, then quit. Then run rc6, it will rewrite the wallet and exit. Run it again, and it fails to start. Here's the output. I put some debugging printf()s in db4.8.

chris@chris:~/Programs/bitcoin$ /bin/rm -fr ~/.bitcoin; rc1-db4/bitcoin-qt -keypool=3; rc6-db4/bitcoin-qt -keypool=3; rc6-db4/bitcoin-qt -keypool=3 QGtkStyle was unable to detect the current GTK+ theme. __db_open_pp('addr.dat') __db_open_pp('addr.dat') returning 0 __db_open_pp('blkindex.dat') __db_open_pp('blkindex.dat') returning 0 __db_open_pp('wallet.dat') __db_open_pp('wallet.dat') returning 0 QGtkStyle was unable to detect the current GTK+ theme. subdb exists __db_open_pp('addr.dat') subdb exists __db_open_pp('addr.dat') returning 0 __db_open_pp('blkindex.dat') subdb exists __db_open_pp('blkindex.dat') returning 0 __db_open_pp('wallet.dat') subdb exists __db_open_pp('wallet.dat') returning 0 __db_open_pp('wallet.dat') subdb exists __db_open_pp('wallet.dat') returning 0 __db_open_pp('wallet.dat.rewrite') __db_open_pp('wallet.dat.rewrite') returning 0 QGtkStyle was unable to detect the current GTK+ theme. __db_open_pp('addr.dat') subdb exists __db_open_pp('addr.dat') returning 0 __db_open_pp('blkindex.dat') subdb exists __db_open_pp('blkindex.dat') returning 0 __db_open_pp('wallet.dat') __db_open_pp('wallet.dat') returning 22

EXCEPTION: 11DbException
Db::open: Invalid argument
bitcoin in Runaway exception

terminate called after throwing an instance of 'DbException' what(): Db::open: Invalid argument Aborted chris@chris:~/Programs/bitcoin$

I've uploaded the resulting .bitcoin folder here: http://replay.marpirc.net/chris/bitcoin.tgz

It fails to load up in rc6 for me. How about you?

I did some more testing of this.

If I create and encrypt the wallet using rc3 and rewrite it using rc4, the wallet got corrupted only 1 in 5 trials.

Rather than writing this out in full for each case, here's a summary:

(encrypt using -> rewrite using = corrupted) 3 -> 4 = 1 of 5 1 -> 4 = 1 of 5 3 -> 5 = 0 of 3 1 -> 5 = 0 of 3 1 -> 6 = 6 of 6 3 -> 6 = 6 of 6

so it seems that creating the wallet with rc1 or rc3 then loading it into rc6 is the least safe rc4 and rc5 mostly are able to rewrite old wallets correctly, but rc6 always corrupts them.

If I comment out line 34 of db.cpp (rc6): // fRemoveLogFiles = fIn; then creating and encrypting a wallet in rc3 and rewriting it in rc6 doesn't corrupt the wallet (0 of 2 trials caused corruption).

This has allowed me to load my original wallet backup. Just commenting line 1157 of wallet.cpp (rc6) fixes the problem for me: // RemoveLogFilesOnShutdown(true);

Also, would it be possible to load the wallet first? Having to wait several minutes for the addresses and blockchain to load only to have the wallet be rewritten causing bitcoin to need restarting is annoying:

Loading addresses... dbenv.open strLogDir=/home/chris/.bitcoin/database strErrorFile=/home/chris/.bitcoin/db.log Loaded 150885 addresses addresses 44139ms Loading block index... LoadBlockIndex(): hashBestChain=0000000000000ac25d05 height=153983 block index 87077ms Loading wallet...

http://pybsddb.sourceforge.net/ref/transapp/logfile.html tells me:

"you cannot remove the current log files simply because you have created a database snapshot"

"Log files may be removed at any time, as long as:

the log file is not involved in an active transaction. at least two checkpoints have been written subsequent to the log file's creation. the log file is not the only log file in the environment."

In my test cases, bitcoin is removing the only log file in the environment. Is this the problem?

Could be, although the log file is only removed after the new wallet is successfully written and the environment has been closed.

The problem is the log file can contain old, unencrypted private keys; suggestions on how to get bdb to completely clean up after itself very welcome...

If I comment out the actual 'delete log file' line only, I can create a wallet in rc3 and have rc6 rewrite it, then load it successfully. If I have it rewrite the wallet, then wait a minute and delete the logfile from .bitcoin/databases while no bitcoin process is running, it won't later load up the wallet again.

So it seems that the log files can contain something necessary even when the process is stopped.

This file: http://replay.marpirc.net/chris/bitcoin2.tgz contains just 2 files: wallet.dat and database/log.0000000001

If you extract it, the wallet loads. If you blow it away, extract it again, and "rm .bitcoin/database/log.0000000001", it doesn't load. If you blow it away, extract again, run bitcoin and quit, the log.001 has gone, but log.002 has appeared, which also breaks things if you delete it after closing bitcoin.

It's as if there's something in the logfiles that makes the wallet work. Deleting them willy-nilly seems kind of dangerous.

OK... Plan E : Leave the log file alone.

That means encrypting your wallet gets you a good wallet.dat file, but old private keys will hang around in the database/log.* file for a while, but will eventually be removed when you cleanly shutdown bitcoin.

What will be removed when you shutdown bitcoin?

The logfiles aren't removed when I shut down. And I don't know if it's safe to assume the keys will be removed from the logfile either.

BDB closes the old logfile and opens a new one when it get close to 10megabytes big.

When bitcoin shuts down cleanly, it asks BDB to remove any unused log files, and BDB will remove all but one file from database/log.*

So: if you encrypt your wallet, unencrypted private keys will be in the old part of the log file. But after running for a day or two, BDB will close that old log file and open a new one. Then, the next time you restart bitcoin, the old log file containing the unencrypted keys is removed.

This seems like a reasonable compromise between security and safety for now; a better wallet encryption solution for the next version of bitcoin (one that doesn't require shutting down and restarting after encrypting the wallet) is a good idea, but out of scope for this release.

OK. Thanks for explaining that.

Thanks very much for helping test! rc7 binaries are available now.

For the fun of it I just tested sipa's 'resilver' change. It fixes this whole issue. So I think you can go back to deleting the logfiles.

To be clear, git revision f47dfa5a works for me.

So this issue can be closed?