Bit-Wasp / bitcoin-php

Bitcoin implementation in PHP
The Unlicense
1.04k stars 417 forks source link

slip-0032 support? #709

Open dan-da opened 6 years ago

dan-da commented 6 years ago

I've been working on {y,Y,z,Z}{prv,pub} support in hd-wallet-derive and its complicated especially when considering all the other coins besides bitcoin and that some of them define their own special prefix bytes besides xprv, xpub. So it seems in the long run we would potentially have {x,y,Y,z,Z}{pub,prv} * n coins combinations. Or in other words, a big mess and very confusing for users and devs alike. imho.

slip-0032 proposes a conceptually simpler model that sticks to xpub/xprv that many are already familiar with and has several advantages.

Also, slip-0132, already supported by bitcoin-php, provides rationale/support for slip-0032 and is worth reading. Here's a snippet:

A final important motiviation for establishing a clearinghouse of HD version bytes is the fact that the extended serialization format does not encode the coin type. The SLIP-0032 proposal attempts a remedy by including the full BIP-0032 derivation path within the serialized key. Along with a human-readable prefix of xpub and Bech32 encoding, SLIP-0032 should greatly improve the wallet ecosystem. Until wallets begin implementation of SLIP-0032, however, this registry aims to alleviate the confusion.

I'm not sure if there is any planned or existing wallet adoption of slip-0032, but I do see there is an npm package for it already.

I figure that if bitwasp/bitcoin-php were to also implement it, that could help momentum for it, and I would certainly make use of it in my derivation tool.

edit: there was a discussion on bitcoin-dev about this last year, mostly positive.

At least, I wanted to start discussion about the idea...

afk11 commented 6 years ago

Hey!

slip-0032 looks interesting. In some cases does side step the whole issue of "supported" prefix bytes, and it's useful to have a data structure embedding the full path.

I'll happily accept a pull request, but meanwhile will try keep it in mind

afk11 commented 6 years ago

Been thinking about this. The data structure doesn't encapsulate HierarchicalKey fully as it no longer carries the fingerprint. So I think extracting out a common 'HdNode' type could be good, including properties ChainCode, Key at the least, which are required to do the BIP32 derivation.

I think the derive* methods belong on that class. Note, that the derive methods require access to sequence / depth, so we need the getters for those visible within the HdNode class (mandated via interface or abstract methods). To create a child key, the subtype can implement an (abstract) protected method taking simple parameters, and building the implementation in question.

The class HierarchicalKey will extend HdNode, and provide implementation specific access to getSequence and getDepth, and be the only implementation with getFingerprint, and so on.

The class NameTBDSlip32Key will extend HdNode also, and expose getPath. getDepth on this class works by checking the depth of the path. getSequence works by taking the last node in the path.

There should be another Factory and Serializer class for Slip32 keys.

Not sure what to do about toExtendedPrivateKey, toExtendedPublicKey, and toExtendedKey on HierarchicalKey. If they go on HdNode, people can swap key types with reasonably light effort (different factory, and most methods will work the same). Or if they should remain bip32-specific, we can extract those as a custom interface, implemented only by HierarchicalKey, and likewise for the Slip32Key.

dan-da commented 6 years ago

ah cool! @afk11 Seems you have a good handle on it. I'm not sure of best approach but def nice if toExtendedKey() and friends work in simple fashion for both cases.

afk11 commented 6 years ago

Also, forgot to post earlier, but the tool really looks nice!

I think I agree, toExtendedKey / etc can probably change with the implementation