BIP 32/SLIP 132 y,z...pub/priv types with generics#469
BIP 32/SLIP 132 y,z...pub/priv types with generics#469dr-orlovsky wants to merge 3 commits intorust-bitcoin:masterfrom BP-WG:feat/yzpub
Conversation
|
I came here, because the released version of this lib could not decode a ypub from an electrum wallet. |
Well, that's kind of the point of hardened derivation paths, you can only derive them using an xpriv. So you'd need to use an |
|
This might be a bit overkill to the solution, but it actually seems quite good. Could you perhaps rebase? |
|
@stevenroose rebased and isolated from other PRs |
|
@apoelstra can you add "API Break" label to this pls? |
src/util/bip32.rs
Outdated
| pub struct ExtendedPubKey { | ||
| /// The network this key is to be used on | ||
| pub network: Network, | ||
| pub struct ExtendedPubKey<R: VersionResolver> { |
There was a problem hiding this comment.
I'm not a fan of the fact that this type becomes generic. I think it must be possible to have this functionality without this type being generic.
|
Agree with @apoelstra in #279 (comment) that SLIP's should not be a part of rust-bitcoin |
|
Re-opening the PR for another consideration round since zpub/ypubs are important part of existing Bitcoin ecosystem (for instance Electrum always provide them for SegWit enabled wallets, and there is no trivial way of converting them into xpub) |
|
Since this kind of is an unofficial standard and has annoyed me a few times already I'd be fine with having a module to parse these into our current xpubs/xprivs and some additional key usage enum. But I would not like to make it a part of our types, I don't want to have to support that standard, I'd often rather throw away the extra parsed info, e.g. when exporting xpubs from electrum to use with miniscript. "I know what I'm doing, don't bother me with different usage encodings!" 😆 In that sense such a lenient parser would be useful. If @apoelstra doesn't like it in rust-bitcoin I'd recommend making it a separate crate. That should be easy for a version that doesn't change the xpub type. |
|
@sgeisler just in case it's already in https://github.com/LNP-BP/rust-lnpbp/ (specifically in https://github.com/LNP-BP/rust-lnpbp/blob/master/src/bp/slip32.rs). If you use |
|
concept NACK. I could be sold on making this more generic for the sake of letting people externally add decoding code for ypubs, zpubs, if it weren't too big a diff. But this code is pretty-much a perfect example of why these standards are terrible and not something we want in the library. Hundreds of lines of special-purpose code that do a tiny fraction of what descriptors do, and which require constant updating for every new output type. |
|
@apoelstra seems like your review comment was cut to a single letter Ok, so the problem is how to deal with y/zpubs exported from Electrum: many pieces of software has to do this while using rust-bitcoin. |
|
I ended up writing an |
|
I think it makes sense as an external crate which just converts them to descriptors. |
|
BTW - github does not let you NACK things, only "request changes", and it does not let you do so without a non-zero-length message. Hence my "f" comment. |
|
Slip132 is done as a separate crate: https://crates.io/crates/slip132 |
|
@apoelstra sorry with brining this topic again, but I have occasionally found that ypub/zpubs are defined in BIP-49 and BIP-84, so this is not some "custom" SLIP-132 and the final bitcoin specs - widely used in the industry (for instance you can't export Electrum SegWit keys w/o supporting these standards). |
|
Lots of things have BIP numbers, and lots of things are done by Electrum. This doesn't make them a good idea and doesn't make them a good fit for rust-bitcoin. I think descriptors are strictly better than ypubs and zpubs and descriptors are still not a part of rust-bitcoin. |
ae0f3d9 desc: check for mismatch in the number of derivation paths between multipath keys (Antoine Poinsot) 131bb5b descriptors: BIP389 multipath descriptors support (Antoine Poinsot) cf401b6 desc keys: add a method to get single-path keys from multipath keys (Antoine Poinsot) b5eb317 desc keys: unit tests for multipath key expressions (Antoine Poinsot) 34472bb desc keys: implement BIP389 multipath key expressions (Antoine Poinsot) e122255 desc keys: make 'at_derivation_index' return an error instead of panicing (Antoine Poinsot) ccd7ef1 desc keys: make parse_xkey_deriv a standalone function (Antoine Poinsot) fb3cb90 desc keys: rename parse_xkey_origin to parse_key_origin (Antoine Poinsot) 60732fb desc keys: make origin key parsing a standalone function (Antoine Poinsot) Pull request description: This implements [BIP389 multipath descriptors](bitcoin/bips#1354). Fixes rust-bitcoin#469. ACKs for top commit: sanket1729: ACK ae0f3d9. Tree-SHA512: c7791b02b7bfef964db586eb962289c0058af9a80ad2243cdf4309d6ac1ce65db5cfbfd9b5aa86d733088aeca691449b9777f2d03b969e41ee988d11fe02dd1e
An alternative to #279 ypub and other extended pub/priv key support with generics. Will add more tests once this will get concept acks.
I have started working on this issue b/c of my other work on adding missed keys to PSBTs in #471, which required BIP 32 binary serialization of extended public key (#470). However it happened that this serialization is non-trivial b/c not of public key types are supported, so I ended up with this PR
Unlike the other alternative, this version allows to extend public key types by defining other concrete implementations of
VersionResolverand using it with extended public and private keys.