Wallet encryption is broken! #2779

Fresh build of bitcoind, fresh wallet.

• ran ./bitcoind encryptwallet ......
• restarted bitcoind
• ran ./bitcoind walletpassphrase ....
• received error: {"code":-15,"message":"Error: running with an unencrypted wallet, but walletpassphrase was called."}

Also can dumpprivkey without unlocking the wallet.

Client: v0.8.2-111-gd9ff1cd-beta Os; Ubuntu 12.04.2 LTS Machine: VirtualBox 64bit 2gb

No libs manually installed.

Confirmed on Ubuntu 13.04 x86_64, v0.8.2-123-gb153ee6-beta

1. had a valid, unencrypted wallet
2. ran 'encryptwallet' under HEAD
3. restarted with HEAD, reproduced problem
4. restarted with 0.8.2, yielded the following output

2013-06-19 19:29:57 init message: Loading wallet...
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
2013-06-19 19:29:57 Error reading wallet database: CPrivKey pubkey inconsistency
[...continues...]
2013-06-19 19:29:57 Error loading wallet.dat: Wallet corrupted
 wallet                  442ms
2013-06-19 19:29:57 init message: Loading addresses...
2013-06-19 19:29:57 Loaded 13808 addresses from peers.dat  30ms
2013-06-19 19:29:57 Error: Error loading wallet.dat: Wallet corrupted

Version 0.8.2 release appears to be OK. Bisecting...

git bisect points the finger at #1863 and commit 3869fb89b60091281b43a35921057ba3f43c18f0 ...

3869fb89b60091281b43a35921057ba3f43c18f0 is the first bad commit
commit 3869fb89b60091281b43a35921057ba3f43c18f0
Author: Jeff Garzik <jgarzik@bitpay.com>
Date:   Mon Jun 10 09:36:29 2013 -0400

    Wallet: store key creation time. Calculate whole-wallet birthday.

    This also encapsulate wallet-read state information into CWalletScanState.

:040000 040000 95eb86835b8a3d9801cd9eafb5e2311515a3c8cb f72f6d6008efc40a803182799c55955e272ffc61 M  src

@jgarzik CWallet::AddCryptedKey overrode CKeyStore::AddCryptedKey; since #1863 it does no longer, so the newly encrypted keys never get written. The easy solution is adding the timestamp to CKeyStore::AddCryptedKey, but that is sorta ugly... (it's data the keystore shouldn't care about).

@jgarzik Thinking a bit more, also about how CKeyMeta will get used for more than just birthdates (BIP32 in particular), how about this:

• Key metadata is kept in a CWallet::mapKeyMetaData (std::map<CKeyId,CKeyMeta)
• When generating a new key, time is put in that map, and new key is written
• AddKey and AddCryptedKey do not take a creation time argument, but instead pull it from that map, if it exists there.

This will make it easier to store key metadata along with the key itself in a wallet format update (avoiding repeating the pubkeys 3 times - once for the key, once for the keypool, once for the metadata).