One warning while compiling

  CXX      qt/qt_libbitcoinqt_a-walletmodeltransaction.o
qt/walletmodeltransaction.cpp:20:5: warning: delete called on 'CReserveKey' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
    delete keyChange;
    ^
1 warning generated.

hdgetaddress doesn't seem to work?

tinyformat: Not enough conversion specifiers in format string (code -1)

@fanquake: thanks for the tests. Will have a look at it. Before calling hdgetaddress did you call hdaddchain?

can this be used to import master seeds? If not, would this require much additional work?

@rebroad: right. This PR would enabled HD features in the main bitcoin-core wallet. It won't affect the current ways of generating keys and sending coins. It's totally independent form the rest of the wallet features. So it is relatively save to use/test this.

You can import hd seeds, but only in raw hex. There is no support for Bip39 word lists. I'm pretty sure you'll find a tool to convert bip39 wordlists into 32byte master seed represented in hex.

I haven't gotten to testing this yet, but why did you not decide to accept a base58 encoded key?

The first reason against accepting raw hex is there's no way to validate the key without visually comparing. And extended keys can often look similar at the start. xpub/xprv's include a checksum, so the user can be sure they entered the right thing.

The other reason is extensibility, since adding watch-only chains implies there won't be any master hex around to use.

@afk11: The lack of base58check encoded master keys is only because it's not implemented yet (didn't find time to do it). But it's very trivial to add. I decided to export the master seed hex to potentially allow to create a bip39 mnemonic (there could be a tool hex-seed->bip39mnemonic). IIRC, you can't create a bip39 mnemonic from the master private key (not sure). Somehow i feel to store a hd chains of keys in the very root, the seed and not in the first derivation, the master private key.
Indeed, you can't visually verify a hex master seed. At the moment i'm not sure what's best practice here. I'd like to avoid Bip39 for security reasons and so im stuck now with hex encoding for the 256bit entropy.

But right. Especially for watch-only and HDM wallets, base58check encoded public key importing and usage as external chain root is extremely helpful. I also wrote that in the PR description (see "whats next").

Rebased and added the base58check encoded master pub/priv key to the hdaddchain RPC output.

Added some commits on top to address nits and add a feature for creating a hd chain of keys with a existing base58check encoded extended master private key (xpriv...).
Supporting watch only wallets with a given base58check encoded pub key of the external key chain will follow.

Most of the bip32 work is also included in CoreWallet (https://github.com/jonasschnelli/bitcoin/tree/2015/05/corewallet), which aims to be a replacement/twin for the current wallet with the option of completely split it of, of the bitcoin-core repository.

The PR is an option for quicker adding bip32 supporting to bitcoin-core (for the ones who can compile bitcoin-core by them self) or for the case where CoreWallet could fail to reach a stable level.

concept ACK

@jonasschnelli oops, missed the notification on this.. You're right you can't go from hex to bip39 mnemonic, as mnemonics have pbkdf2 applied to them. +1 on base58 also.

I noticed you have a hard limit of 32 bytes of hex for the provided seed. This isn't correct as BIP32 doesn't specify a size, but rather states the binary seed may be short, or as long as 512 bits (128-256 recommended), since it's hashed with HMAC.

Small PR against your branch for this: jonasschnelli#8

Also, chalk it up to not reading the docs carefully, but I didn't expect hdaddchain to create a random key when I started testing. Maybe explicitly mention this will happen if no hex seed / master private key is provided?

@afk11: Meh. Right. I see your point. So you are saying once you have created a bip32 chain with a entropy over GetRandBytes() you can't build a bip39 wordlist (sorry, not familiar with bip39)?

This would definitively mean using base58c for the seed output, or even better, just the master private key in base58c. Or do you see any reason to keep the seed?

Maybe it should also support importing a bip39 wordlist to create a bip32 chain.

Yes, that is correct - the entropy used to produce the bip39 mnemonic is different to the seed that is produced.

I would suggest only displaying the base58 master key. It would keep the API consistent, since in certain situations the initial seed used to yield the master key won't be available, and in all other dumphd... cases it'll probably just be the xprv/xpub.

There isn't much reason to keep the hex seed - the same can be said for BIP39 mnemonics (and their passphrases) I guess. You could have separate RPC methods to create a new master via both methods.

I'm not sure if anything is lost in doing this. If someone dumps their master xprv, they can import it using hdaddchain just the same as if they had the mnemonic / hex.

Could you point me at some background on the concerns about BIP 39 and security please?

Seed interoperability with other wallets would seem like one of the major advantages to HD wallet support, although being about to import is IMHO the most important part of that use case, so reference client can be used to recover where another wallet fails.

@rnicoll: Mostly because Bip39 can harm your security:

• https://bitcointalk.org/index.php?topic=812894.msg10907427#msg10907427
• https://bitcointalk.org/index.php?topic=1138063.msg12013904#msg12013904

The 2048 kdf rounds might be okay for embedded USB hardware wallet. But IMO nothing that i'd like to have on my full-node wallet.

But i'm pretty sure you can export/convert a bip39 memonic into a master xpriv key (maybe on bip32.org)?
Also feel free to patch the RPC hdaddchain with import support for bip39.

something to keep in mind with BIP39 is that because seed = hash(mnemonic, pass) it doesn't allow for changing passwords :/

@rubensayshi, at least, not without transferring all the BTC.

Needs rebase

Rebased and updated.

• Not the default keypath is m/c' = (m/0'/0' for first external key and m/1'/0' for first internal/change key).
• Support for private child key derivation (default).

This PR is relatively complex and are meant for user who are seeking full control over their hd structure. I have plans to also open a smaller, simpler HD patch.

Closing in favor of #8035.
Some parts of this PR could be used to extend HD functionality if and once #8035 has been merged.