[WIP] [wallet] keypool restore #10830

1. 
   jnewbery commented at 8:30 pm on July 14, 2017: member

   This is #10240 rebased on master, with some of the comments addressed. I’ve tidied the code up to current style (braces, variable names, etc) and also squashed/split/re-arranged the commits to help reviewers.

   Not ready for merge yet, but I’m happy to receive early feedback.

   Unaddressed feedback from #10240:

   • make this work with multiwallet. Perhaps use a counter instead of a bool for pausing block requests/tip udpates.
   • Increase the size of the keypool
   • enable keypool restore for non-HD wallets.

   Thanks to @jonasschnelli for his work on #10240. Paging @ryanofsky @gmaxwell @sipa since they provided feedback on 10240.

2. jnewbery force-pushed on Jul 14, 2017
3. jnewbery force-pushed on Jul 14, 2017
4. jnewbery force-pushed on Jul 14, 2017
5. [wallet] MOVEONLY move CAffectedKeysVisitor b8bc69dbd7
6. [wallet] add keypool restore functionality 92a4d6a105
7. [wallet] add option to pause block requests (is/setBlockRequestsPaused()) ca315d52ca
8. [wallet] add option to pause tip updates (is/setTipUpdatesPaused()) c653ca24fb
9. [wallet] add request-halt flag to BlockConnected signal 877452b06a
10. [wallet] pause wallet sync when the keypool requires extension ad970bef54
11. [wallet] check the keypool min size during startup and when retriving a key 031eba3631
12. [wallet] add -keypoolrestoremin option 4abdce4a48
13. [wallet] [tests] add keypool restore functional test cac1824e33
14. jnewbery force-pushed on Jul 14, 2017
15. 
    jnewbery commented at 10:24 pm on July 14, 2017: member

    I’ve done my best to address the comments from #10240, but the history on that PR is rather convoluted, so I may have missed some. @ryanofsky - do you mind checking this PR for any that I may have missed?

    Obviously I still need to address the three main points in the PR description.

16. 
    ryanofsky commented at 10:49 pm on July 14, 2017: member

    @ryanofsky - do you mind checking this PR for any that I may have missed?

    Yeah, planning to look at this first thing next week. Do you know if this needs to be tagged for 0.15? It wasn’t clear from earlier if this fixes a new problem introduced in this release by #9294.

17. 
    TheBlueMatt commented at 11:58 pm on July 14, 2017: member

    Hmm, it would be nice to pull in a cut-down version of this for 0.15 - skip the pause-if-wallet-falls-behind stuff and jack up the keypool default a ton (with associated performance fixes for too-often-wallet-flushing). This should get us to a place that +/- doesnt have any of the issues folks were complaining about at the meeting and will only pretty-rarely miss transactions (ie if you run out of keypool, an issue which clearly would be worse pre-hd, and we can just set keypool very large).
18. fanquake added the label Wallet on Jul 15, 2017
19. 
    gmaxwell commented at 2:11 am on July 15, 2017: contributor

    @TheBlueMatt since you listed a list of what could be left out– the most important feature of this is marking all keys up to a used key as used.

    If we want to cut down the pause stuff for now, we could couple that with something that prolongs keeping an encrypted wallet unlocked while syncing/rescanning is running. Then at least an encrypted wallet recovery is a matter of unlock then rescan. Not as elegant as pausing but workable.

20. 
    instagibbs commented at 12:56 pm on July 17, 2017: member

    needs rebase
21. 
    ryanofsky commented at 3:28 pm on July 17, 2017: member

    It wasn’t clear from earlier if this fixes a new problem introduced in this release by #9294.

    Still curious about the relationship with #9294 (if any).

22. 
    ryanofsky commented at 4:45 pm on July 17, 2017: member

    Still curious about the relationship with #9294 (if any).

    Talking offline, seems there isn’t really a relationship with #9294 (#9294 might even make the problem less bad) and that isn’t a new problem since 0.14. Holding off on review since most of my comments from #10240 were about the pause mechanism and John was going to work on a simpler version of this change without pausing to target 0.15.

23. sipa added this to the milestone 0.15.0 on Jul 17, 2017
24. 
    jnewbery commented at 6:37 pm on July 19, 2017: member

    Closing. Not required for 0.15, although may revisit later.

    #10882 is a simpler alternative which doesn’t include the block sync pause stuff.

25. jnewbery closed this on Jul 19, 2017

    ---

26. DrahtBot locked this on Sep 8, 2021

Contributors
jnewbery ryanofsky TheBlueMatt gmaxwell instagibbs

Labels
Wallet

Milestone
0.15.0