BIP-178: Extend WIF format with key type #12869

BIP pull request: https://github.com/bitcoin/bips/pull/673

This PR does not change the behavior. It only adds underlying support for formats beyond 'compressed pubkey' and 'uncompressed pubkey'. It partially implements BIP-178.

The key types are based on the key types in Ethereum Electrum 3.0: https://github.com/spesmilo/electrum/blob/82e88cb89df35288b80dfdbe071da74247351251/RELEASE-NOTES#L95-L108

The legacy types KEY_P2PKH_{UN}COMPRESSED map to the current WIF format. If used/seen, they indicate that the corresponding public key type is unknown, and when imported, they will simply iterate over all types and watch all of them (P2PKH, P2SH-P2WPKH, P2WPKH (bech32), and so on). (Current behavior.)

When one of the other types is used, the current code will behave exactly the same as above. In a future PR, the code will be changed to only import the corresponding type when importing a private key.

This is related to #12705, see in particular #12705 (comment).

Ping @sipa & @promag

The key types are based on the key types in Ethereum 3.0: 

😯

TIL Ethereum has Segwit.

Oops.. fixed.

Ideally we would first draft/spec the WIF "standard" in the form of a BIP.

General Concpet ACK

Ideally we would first draft/spec the WIF "standard" in the form of a BIP.

I did. The PR proposal* is here: https://github.com/kallewoof/bips/blob/bip-typed-wif/bip-0178.mediawiki

I posted it in the ML but as a reply to the existing thread, here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-April/015870.html

(* It is not actually a PR yet.)

@kallewoof This link https://github.com/kallewoof/bips/blob/bip-typed-wif/bip-extended-privkey.mediawiki is broken. I can't find this BIP-178 proposal anywhere else than in the mailing list. What is the plan? @kallewoof can you add it to your repo? Thanks

@Janaka-Steph I updated the link: https://github.com/kallewoof/bips/blob/bip-typed-wif/bip-0178.mediawiki

The PR is here: https://github.com/bitcoin/bips/pull/673

Needs rebase due to merge of #12924

Rebased.

It feels wrong to store the address type in CKey. CKey is a representation of a private key, not an address. Address derivation and knowledge about that belongs in the wallet code, not the ECDSA wrapper.

@sipa The compressed byte is in the private key right now, and BIP-178 extends that so it feels like it makes sense to keep it in the key class. Where else would it go?

@kallewoof The compressed flag affects the public key, not the address. Different private keys (including compressed flag) correspond to different public keys (when looking at the bytes, which is what matters to us).

Where should it go? Right now, nowhere - we can't use it, as the IsMine can't take it into account, it just looks at what we can sign. importmulti can use it to automatically figure out which redeemscript/witnessscript to compute, but it will inevitably overshoot what it matches again because of the IsMine logic. Later (see https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2) it should go into the ismine record data. But it's not necessarily something associated with a single key, but with an entire record (so it can apply to a whole HD chain or even more complex structures).

I understand it seems easiest to store it in CKey for now, but that's just due to the code historically conflating keys with addresses. I really would try to avoid continuing that.

That makes sense. Maybe I should just close this PR, then. I could always rework it to split out the WIF stuff from the key stuff. Would that make sense or just a waste of time, you think?