[Wallet] Add support for flexible BIP32/HD keypath-scheme #8723

1. 
   jonasschnelli commented at 1:22 pm on September 14, 2016: contributor

   This adds the startup argument -hdkeypath which allows to set the BIP32 keypath scheme during the creation of a wallet.

   This PR would allow to use keypath-scheme after BIP44, etc. to be compatible with other wallets.

   This PR does not change the keypool mechanism. Even if the keypath would allow public-key-derivation, we still derive all keys with private-key-derivation and fill up the keypool. Though, a PR that would enable public-key-derivation would be “in a reviewable size” once this gets merged.

2. jonasschnelli added the label Wallet on Sep 14, 2016
3. jonasschnelli force-pushed on Sep 14, 2016
4. dcousens approved
5. 
   dcousens commented at 2:04 am on September 15, 2016: contributor

   concept ACK, has there been any though for adhering to the scheme of 2 accounts, 1 for receiving and 1 for change?
6. 
   jonasschnelli commented at 6:27 am on September 15, 2016: contributor

   @dcousens: Yes. I have thought about the split of the two key-chains (internal/external). But the current keypool concept make it a bit more difficult. I guess it could be combined with the feature to allow public key derivation.
7. in src/wallet/wallet.cpp: in bb87e2d841 outdated

   3384@@ -3360,6 +3385,8 @@ bool CWallet::InitLoadWallet()
   3385         bool useHD = GetBoolArg("-usehd", DEFAULT_USE_HD_WALLET);
   3386         if (walletInstance->IsHDEnabled() && !useHD)
   3387             return InitError(strprintf(_("Error loading %s: You can't disable HD on a already existing HD wallet"), walletFile));
   3388+        if (walletInstance->IsHDEnabled() && walletInstance->GetHDChain().keypathScheme != GetArg("-hdkeypath", DEFAULT_HD_KEYPATH_SCHEME))
   

   ---

   ---

   

   luke-jr commented at 8:54 am on September 22, 2016:

   Maybe the default here should be the current scheme?

   ---

   

   jonasschnelli commented at 7:52 am on October 20, 2016:

   The default is the current scheme IMO (DEFAULT_HD_KEYPATH_SCHEME == "m/0'/0'/k'" == current scheme)
8. 
   jonasschnelli commented at 8:01 am on October 20, 2016: contributor

   Rebased. I hope we can make some progress regarding the flexibility of our HD wallet feature as well as with pub-child-key-derivation.
9. jonasschnelli closed this on Oct 20, 2016

   ---

10. jonasschnelli force-pushed on Oct 20, 2016
11. [Wallet] Add support for flexible BIP32/HD keypath-scheme fdfd58b0d2
12. 
    jonasschnelli commented at 8:02 am on October 20, 2016: contributor

    Accidentally closed, reopen.
13. jonasschnelli reopened this on Oct 20, 2016

    ---

14. jonasschnelli added this to the milestone 0.14.0 on Nov 25, 2016
15. 
    jonasschnelli commented at 7:41 am on November 25, 2016: contributor

    Added the 0.14 milestone label. IMO we should support flexible keypaths as well as wallet creation with an initial xpriv.
16. 
    jmcorgan commented at 5:20 pm on December 2, 2016: contributor

    ACK. It works as is, however, I would like to see either a separate keypath scheme for change keys, or a way of specifying both in the existing one. Of course, you’d need to actually implement the split in the wallet code.

    Wallet creation with an xpriv/keypathscheme would be very useful, however, it brings up the question of restoring a wallet from only those (scanning, gap policy, etc.), vs. a full wallet.dat file.

17. 
    jtimon commented at 6:09 pm on January 10, 2017: contributor

    Concept ACK, needs rebase.
18. jonasschnelli removed this from the milestone 0.14.0 on Jan 12, 2017
19. in qa/rpc-tests/wallet-hd.py:84 in fdfd58b0d2

    80@@ -82,6 +81,16 @@ def run_test (self):
    81         #connect_nodes_bi(self.nodes, 0, 1)
    82         assert_equal(self.nodes[1].getbalance(), num_hd_adds + 1)
    83 
    84+        print("Testing flexible keypath scheme ...")
    

    ---

    ---

    

    Sjors commented at 5:51 pm on February 8, 2018:

    self.logger.info("Testing flexible keypath scheme ...")
20. 
    Sjors commented at 5:56 pm on February 8, 2018: member

    Concept ACK

    I’ll try this against some other wallets after rebase, both importing and exporting.

    I suspect wallet compatibility is going to be somewhat painful with SegWit, but let’s deal with that in another PR.

21. MarcoFalke added the label Needs rebase on Jun 6, 2018
22. 
    DrahtBot commented at 11:20 pm on November 8, 2018: member
23. DrahtBot added the label Up for grabs on Nov 8, 2018
24. DrahtBot closed this on Nov 8, 2018

    ---

25. 
    Sjors commented at 8:45 am on November 10, 2018: member

    I think this should be revisited after adding descriptor support to import_multi #14491 and the ambition to turn wallets into descriptor bags. Once that’s done this should be a simple matter of adding optional descriptor arguments to createwallet so users can pick whatever derivation scheme they prefer.
26. laanwj removed the label Needs rebase on Oct 24, 2019
27. DrahtBot locked this on Dec 16, 2021

Contributors
jonasschnelli dcousens luke-jr jmcorgan jtimon Sjors DrahtBot

Labels
Wallet Up for grabs