Added new config option noautofillkeypool. #2841

One of the most annoying things about trying to maintain wallet backups is the fact that every time you unlock a wallet the key pool gets automatically refilled, thus making any existing backups of the wallet keys obsolete. If you call getnewaddress on a locked wallet, no problem - but the moment you unlock it, your wallet backups are out of date...no warning is given to user. Sooner or later that backed up wallet will expire, by which time we better hope the user had the foresight, understanding, discipline, and memory to have made a new backup.

As things currently stand, the user must either keep track of when the backed up keypool is exhausted and remember to make a backup before the new keys are used - or must make a backup after every few transactions to be certain the backed up keypool is never exhausted. And let's be honest about it - very few users do either of these things. They are tedious and annoying. I only do them because they're far less tedious or annoying than attempting to recover lost keys.

The user now has the choice to disable automatic refilling of the keypool by adding the following line to bitcoin.conf:

noautofillkeypool=1

This effectively disables all automatic key generation, requiring the user to manually run keypoolrefill to replenish the wallet when the key pool runs low or a new wallet is created.The keypoolrefill command has been conveniently exposed in the GUI as well. This ensures that all generated keys can be easily and conveniently backed up in a timely fashion, with confidence that the backup has been made properly and will never expire without warning (at least as far as keys - synchronizing history and account labels are separate issues that this pull request does not address).

We probably shouldn't be allowing users to generate new keys without prompting them to make backups (or automatically making the backups for them); certainly not in the GUI and probably not in the RPC either (at least not without requiring an override flag or an additional call). It obviously makes more sense to do backups in batch rather than after every single new transaction; after all, that's exactly the purpose of pregenerating a key pool in the first place. I believe hiding this from users only does them a disservice. Anyone who is smart enough to use Bitcoin-Qt should have no problem understanding how to do this - and anyone who is incapable of doing this probably shouldn't be managing their own bitcoin keys in the first place. EVERYONE should be making proper backups of their signing keys. Exposing a wallet backup function without also giving users control over their keypool is giving them a tool that requires two hands to use properly but tying one of their hands behind their back.

This pull request simply makes the keypool feature which already exists properly usable. Having said that, with the expectation that some may disagree with my position, this new behavior is 100% opt-in - if noautofillkeypool is not set in bitcoin.conf (or omitted), the app defaults to its old behavior thus ensuring full backward compatibility.

View of Bitcoin-Qt interface with keypoolrefill feature exposed.

TODO:

• General: Automate backups when keypool runs low by, for instance, allowing the user to add one or more autobackupwallet=<destination> lines to the config file. The destination field should support protocols like sftp as well as removable media. Will require mechanism to prompt user or return an error if connection cannot be established, removable media is not present, or specified path is not found/invalid.
• Bitcoin-Qt: Prompt user to refill pool when key pool is empty, prompt/remind user more aggressively to make backups, check key pool status before attempting operations that require new keys, add indicator to show how many keys are left in pool, add the ability to easily change the pool size from GUI, make it clear when errors are due to an empty key pool and show user proper procedure for refilling it and making a backup.
• Bitcoin-Qt: Icon attribution for , taken from http://i1-win.softpedia-static.com/screenshots/32x32-Free-Design-Icons_1.png?1358761381

1. Feature request seems sane. 30 second glance did not turn up any bugaboos.

2. Mild conflict with #2776 but easily resolved.

3. Waiting to see if pulltesters likes these new commits. A bit worried that the first commit breaks git bisect?

needs rebase

Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/4ec0d1d99554e4109bfcef1970b1354e89cc6a97 for binaries and test log.

This could happen for one of several reasons:

1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in qa/pull-tester)
2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
4. The test suite fails on either Linux i386 or Win32
5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

needs rebase again

I ain't never seen a double-negative I didn't disapprove of.

Option should be -autofillkeypool, default should be 1, set it to 0 to turn it off.

And needs rebase and a test plan.

I like this feature, as I'd like to have more control over when and whether it refills the key pool, so that I know exactly when there are new keys in the wallet and I need to make a new backup. It just feels safer.

Maybe I/someone should just implement very basic BIP32 key generation instead :)

That'd be a good option too. But as long as we have a keypool option (and I don't think it's wise to completely do away with the keypool as soon as introducing deterministic wallet support) having more explicit control over it would be useful. And this code is already written...

I wonder if it should be -generatekeys=0/1 instead, so it covers any future cases where keys might be created.

@CodeShark I've rebased it: https://github.com/laanwj/bitcoin/tree/2013_10_noautorefillkeypool_rebase @luke-jr yes, it should block all the cases in which keys are added to the wallet automatically. But in the (future) case of HD wallets it could "generate" new keys from the seed even with this option on.

Closing this, even though I personally would find this a useful safety feature as long as deterministic wallets haven't landed, but the consensus is against.