Increase DEFAULT_KEYPOOL_SIZE to 10000. #8025

Reduce risk of backups being incomplete.

That seems like a lot, at least the way we're currently populating the keypool. Key generation holds up everything in the initial startup, and my laptop generates about 5 keys/sec. If we're going to generate keys for half an hour, I think we ought to do it in a separate thread from the initial sync.

Forgot that I hadn't actually fixed that.

Whatever happened to deterministic wallets, anyway (#3250 / #6265)? That would be the ideal solution to this problem...

In principle I agree with this change but there is also a sense in which it makes things worse: with 100 keys, people need to be active with backups. With 10,000 they could just back up once, sit back and assume they'll never hit the 10,000 mark. Until they do and are entirely screwed and there won't even be a warning.

I think if you want to do thi, this should be accompanied by disabling keypool refill by default, and making that a manual operation (could be as easy as clicking a button in the GUI) that should be accompanied by a new backup.

ConceptMEH. I would much prefer the few line version of switching to hardened BIP32 keys for filling the keypool.

We should really make a decision here. This same back and forth has been going on for years, with the same people giving the same replies, and as a result the keypool logic is still the same as in 2012. At least what @pstratem proposes (increasing the default key pool size) is a small improvement.

So is there any drawback in moving Bitcoin Core's wallet from a keypool system to hardened BIP32? If this is just a few-line change, why hasn't this done yet? One reason could be that a few line change doesn't mean it's easy to review - do people trust it? maybe it's hard to assure that it is correct. I'm not sure.

It would reduce the need for secure randomness enormously (reduced the throughput necessary in e.g. #7891), to just generating of seeds.

There is a PR where the keypool gets filled with Bip32 hardened keys: #7273 (needs overhaul, finalizing which I'm ready to do if we agree on Bip32 support) #7273 is a very simple solution (mostly just fill the keypool deterministic) There is also a more complex and flexible solution: #6265

The Bip32 pro/cons:

• - slightly different security (stuff like losing an ext. pub key together with a child priv key)
• - migration from non HD to HD wallets
• + Backups are more robust
• + Better support for hardware wallets
• + Possible support for Bip45 like multisig wallets

Default keypool size of 10'000 could be an intermediate step. What about the performance impacts on slower systems (RPi, etc.) during first start?

I think that we should then invent new shortcut: IKR (Initial Keypool Refill)... On RPi, it will take longer than IBD.

I think that we should then invent new shortcut: IKR (Initial Keypool Refill)... On RPi, it will take longer than IBD

Also there is a kind of perverse incentive here: Generating secure randomness, especially if it needs input from hardware sources, is usually slow. If you want to generate 10,000 truly random keys (~320,000 bytes of entropy) it should take a while. If you speed it up you can do worse than BIP32 hardened derivation. So then, why not use it in the first place.

I definitely agree on this. This will also motivate people using slow systems to work on BIP32 ;-)

@jonasschnelli working on fixing the performance issues here in #8026

@pstratem But getting 10'000*32byte secure entropy (and deriving 10'000 pubkeys including a sign/verify to check the key integrity) will be the bottleneck IMO.

@jonasschnelli fair point

i guess i'll have to fix this correctly by making the keypool refill happen in the background with a trapdoor

"But getting 10'000*32byte secure entropy" that shouldn't be slow... maybe it is, but there isn't a fundamental reason it should be.

There are a bunch of overheads with wallet key management which last I check completely eclipsed key generation: it was flushing and syncing the wallet for each and every key. Running on one core, the generation should operate at about, say, 3000/sec (it's only that slow because it does a generate, sign, and verify-- otherwise it would be over 10000/sec on a typical desktop).

I think we should work towards bip32 instead. IMO the risks and drawbacks of a default 10'000 keys keypool are higher then reviewing and merging an adequate bip32 keypool refill PR.

@jonasschnelli your patches are far more than the minimal change required. I believe the first time I derandomized the wallet (back before BIP32 was written), it was a 15 line patch, not counting the KDF itself-- though I @don't doubt it was inadequate #7273 is a 1605 line patch which changes the UI and such.

I agree we should do this, but we should work to make it the smallest sensible change; otherwise its just too much work to review considering the criticality of getting it right.

@gmaxwell: IMO the minimal patch should include:

• store the master key (seed) in a extra record (& support for encrypted wallets)
• derive keys by a given keypath (to support different keypath schemes)
• getnewaddress and getaddressesbyaccount (and co.) should report the addresses bip32 keypath
• the wallet needs to keep track of the last used child index so it can derive the next child (#7273 does that by using a data object called hdchain)

I don't see simple ways to reduce the diff size of #7273 without starting to reduce its flexibility and the overall software design.

I'm happy to write a simpler solution if we agree on the minimal spec for a first bip32 support.

@jonasschnelli A minimal patch that makes backups safe doesn't need any RPC or UI changes, and just 1) generate/store a master key 2) derive newly generated keys from that master 3) remember how many have been generated already. You wouldn't be able to really call that "bip32 support", but it would give all advantages deterministic keys have regardless.

@sipa one step above the bare minimum is to keep separate change and non-change pools so they don't get scrambled up on an emergency recovery.

"But getting 10'000*32byte secure entropy" that shouldn't be slow... maybe it is, but there isn't a fundamental reason it should be.

Slow is a matter of degree, of course. What is an acceptable speed for generating good "true" randomness on a typical PC?

I mean I'm pretty sure that if you read a block of 320,000 bytes from urandom at once, you'll get randomness generated from a 'seed' (that happens to be the kernel's RNG state) just as much as you were to use BIP32-or-similar. The only difference would be the lack of determinism, similar to throwing away the seed after use.

i guess i'll have to fix this correctly by making the keypool refill happen in the background with a trapdoor

True, there is no inherent reason why initial sync and keypool generation could not be done in parallel. Though it would complicate things more than this minimal change, of course.

I'll try to write a more minimal PR without API impacts.

Alternative solution: #8035

Can one of the admins verify this patch?

Closing in favor of #8035

m/0'/0'