Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Any plans to add support for SLIP-0132 version bytes? #1797

Open
mflaxman opened this issue Aug 6, 2020 · 11 comments
Open

Any plans to add support for SLIP-0132 version bytes? #1797

mflaxman opened this issue Aug 6, 2020 · 11 comments

Comments

@mflaxman
Copy link

mflaxman commented Aug 6, 2020

https://github.com/satoshilabs/slips/blob/master/slip-0132.md

It appears you can pass in version bytes when constructing a master key and this will work for serializing private keys, but you cannot serialize public keys.

I was able to cobble together this hackey code, is there a better way to do this? Posting in case it helps others with the same issue.

package main

import (
	"bytes"
	"fmt"

	"github.com/btcsuite/btcd/chaincfg"
	"github.com/btcsuite/btcutil/hdkeychain"
)

func i32tob(val uint32) []byte {
	b := make([]byte, 4)
	// https://golang.org/pkg/encoding/binary/
	binary.BigEndian.PutUint32(b, val)
	return b
}

func NeuterWithVersion(xkey *hdkeychain.ExtendedKey, version []byte) (*hdkeychain.ExtendedKey, error) {
	// Already an extended public key
	if !xkey.IsPrivate() {
		return xkey, nil
	}

	pubkey, err := xkey.ECPubKey()
	if err != nil {
		return xkey, err
	}

	xfp := xkey.ParentFingerprint()
	xfp_bytes := i32tob(xfp)

	return hdkeychain.NewExtendedKey(version, pubkey.SerializeCompressed(), xkey.ChainCode(), xfp_bytes,
		xkey.Depth(), xkey.ChildIndex(), false), nil
}

func main() {
	net := chaincfg.MainNetParams
	// SLIP132 version bytes
	// https://github.com/satoshilabs/slips/blob/master/slip-0132.md
	net.HDPrivateKeyID = [4]byte{0x02, 0xaa, 0x7a, 0x99} // Zpriv
	net.HDPublicKeyID = [4]byte{0x02, 0xaa, 0x7e, 0xd3}  // Zpub

	// Insecure wallet:
	master, err := hdkeychain.NewMaster(bytes.Repeat([]byte{0x00}, 32), &net)
	if err != nil {
		panic(err)
	}
	fmt.Println("master", master)

	xpub, err := NeuterWithVersion(master, []byte{0x02, 0xaa, 0x7e, 0xd3}) // Zpub
	if err != nil {
		panic(err)
	}
	fmt.Println("xpub (NeuterWithVersion)", xpub)

	// This will fail
	xpub, err = master.Neuter()
	if err != nil {
		panic(err)
	}
	fmt.Println("xpub (regular)", xpub)

}
$ go run main.go 
master ZprvAhadJRUYsNgeANg593xPYvpJjBZoVyAfhMr43wqnhYRZieBp1JwiBkNDmfvvLmjfTs9zhZfVKaX3oHVMsyqDAiVPm4fTPADaUHKxM5cd47s
xpub (NeuterWithVersion) Zpub6vZyhw1ShkEwNrkYF5VPv4m3HDQHuRtX4amerLFQFsxYbSWxYrFxjYghcyYFUteKDEbbLUKq7o3m16BXww5ALJ9coC76L9PJpryReNjP2hi
panic: unknown hd private extended key bytes
...

You can compare this here to confirm it is indeed correct:
https://iancoleman.io/bip39/
Screen Shot 2020-08-05 at 11 43 03 PM

Note that I'm running github.com/btcsuite/btcutil v1.0.3-0.20200713135911-4649e4b73b34 as there were some recent changes that are not included in 1.0.2

@onyb
Copy link
Collaborator

onyb commented Aug 8, 2020

The cleanest approach I can think of is to provide a new method called RegisterHDKeyID, in the chaincfg package of btcd. Here's a rough implementation (untested):

// RegisterHDKeyID registers a public and private hierarchical deterministic
// extended key ID pair.
//
// Non-standard HD version bytes, such as the ones documented in SLIP-0132,
// should be registered using this method for library packages to lookup key
// IDs (aka HD version bytes).
// Reference:
//   SLIP-0132 : Registered HD version bytes for BIP-0032
//   https://github.com/satoshilabs/slips/blob/master/slip-0132.md
func RegisterHDKeyID(hdPublicKeyID []byte, hdPrivateKeyID []byte) error {
	if len(hdPublicKeyID) != 4 || len(hdPrivateKeyID) != 4 {
		return ErrInvalidHDKeyID
	}

	var keyID [4]byte
	copy(keyID[:], hdPrivateKeyID)

        hdPrivToPubKeyIDs[keyID] = hdPublicKeyID

        return nil
}

Using the above solution, you don't need to modify the chaincfg.MainNetParams in-place. Your code would then look something like this:

hdKeyIDZprv := []byte{0x02, 0xaa, 0x7a, 0x99}
hdKeyIDZpub := []byte{0x02, 0xaa, 0x7e, 0xd3}

if err := chaincfg.RegisterHDKeyID(hdKeyIDZprv, hdKeyIDZpub); err != nil {
        panic(err)
}

masterZpub, err = masterZprv.Neuter()  // assuming masterZprv is already defined
if err != nil {
        panic(err)
}

fmt.Println("serialized master Zpub", masterZpub.String())  // this should work!

I can do a PR if the above works for you.

@mflaxman
Copy link
Author

mflaxman commented Aug 9, 2020

That makes sense, thanks!

Would it be easy to switch version bytes on an existing MasterZprv or would this have to be registered at instantiation? There are use-cases where you might want to switch between (all of) them on the fly:

  1. Generate xpriv from some seed
  2. Calculate child xpub (using the standard MainNetParams)
  3. Display that xpub from step 2 with SLIP132 version bytes (say as a Ypub & zpub).

You can see there's some demand for step 3 as there exists a popular tool just to perform this step:
https://jlopp.github.io/xpub-converter/

If it's not easy to switch version bytes on an already-derived child key, I could still make this work in a hackey way: just do step 1 n times (once for each version byte I'd like to encode with), perform step 2 on each of those keys, and then calculate the .String() value of each of those instances of the key.

@onyb
Copy link
Collaborator

onyb commented Aug 13, 2020

You can see there's some demand for step 3 as there exists a popular tool just to perform this step:
https://jlopp.github.io/xpub-converter/

That's quite useful indeed. I don't think it's too hard to implement this feature. We just need to export the version field of ExtendedKey so that it's possible to set values as needed:

key, err := hdkeychain.NewKeyFromString("xpub67uA5wAUuv1ypp7rEY7jUZBZmwFSULFUArLBJrHr3amnymkUEYWzQJz13zLacZv33sSuxKVmerpZeFExapBNt8HpAqtTtWqDQRAgyqSKUHu")
if err != nil {
	panic(err)
}
	
key.Version = []byte{0x04, 0xb2, 0x47, 0x46}  // set the Version field for zpub from SLIP-0132
fmt.Println(key)  // zpub6mZghGWKDH6wXQW5uFgytjNa7sYLMaEU15Ncse5cobXZ5yNvjrr7eSJH6QFkcPDss9gXTGgtaBXfQpU62D1QUbf1uXHK4LUBwsHykyMwZ3t

I'll try to consolidate what we discussed in this issue, and do some PRs soon. :)

@mflaxman
Copy link
Author

That sounds awesome!

I want to open-source a CLI multisig and BIP39 RNG helper tool I've written which currently uses NeuterWithVersion and my hackey implementation at the beginning of this thread (ugh), so I'll just wait until you have this PR and switch to that much cleaner interface before publishing my confusing code.

Nit/Q: you're using key.Version in your latest example, but what if NewKeyFromString took in an xpriv (not an xpub), then how could I get both zpub and zpriv out of your code snippet?

key.Version seems ambiguous, because it's not clear if it applies to hdPrivateKeyID or hdPublicKeyID. I want to update key.Version to use 0x04b2430c for zprv and 0x04b24746 for zpub (at least in this case where we're P2WPKH, most of what I'm working on is Multi-signature P2WSH in P2SH or Multi-signature P2WSH, which have their own pub/private version bytes for mainnet/testnet).

@onyb
Copy link
Collaborator

onyb commented Aug 22, 2020

@mflaxman Sorry for the late response. I am looking forward to your multisig tool. 🙂

currently uses NeuterWithVersion and my hackey implementation at the beginning of this thread

The approach discussed here is indeed cleaner. The main issue with your original implementation was that you were modifying the chaincfg.MainNetParams in-place, which is never a good idea since it is a singleton. It will have side-effects in other parts of your code / btcd.

what if NewKeyFromString took in an xpriv (not an xpub), then how could I get both zpub and zpriv out of your code snippet?

You can find below a pseudo-code for the above. Let me know if this is what you want.

xprv, _ := hdkeychain.NewKeyFromString("xprv...")  // load the private extended key from a string
xpub, _ := xprv.Neuter()  // gives you the xpub

xprv.Version = []byte{0x04, 0xb2, 0x43, 0x0c}  // the xprv is now a zprv
xprv.String()  // gives you the serialized zprv
zpub, _ = xprv.Neuter()  // gives you the zpub

key.Version seems ambiguous, because it's not clear if it applies to hdPrivateKeyID or hdPublicKeyID

An ExtendedKey could be either public or private. If it is private, the Version field must refer to the private HD version bytes (and likewise for public).

It does not make sense for an ExtendedKey to be public but have private version bytes or both public and private version bytes. I would not say that it is ambiguous.

@mflaxman
Copy link
Author

+1 to everything you said.

Looking forward to being able to use that. Very clean & easy to read.

Thanks!

@onyb
Copy link
Collaborator

onyb commented Aug 27, 2020

@mflaxman I did two PRs to add support for this feature.

Check out the unit-tests of btcsuite/btcutil#181 for a sneak peek.

@mflaxman
Copy link
Author

Looks great!

Nit: could you add a test of xprv to zpub (and zprv to xpub)? It's not strictly needed for functionality, but will help others who use the tests as documentation.

@onyb
Copy link
Collaborator

onyb commented Aug 28, 2020

could you add a test of xprv to zpub (and zprv to xpub)?

We'll need #1617 for that, which won't be available in a stable btcd version until v0.22.0
(too late for v0.21.0, which will be out today). Provided the mappings have been registered in advance, it'll just be a matter of calling Neuter() after CloneWithVersion().

You should still be able to use it in your code by pointing to a development release of btcd, but I can only add a unit-test once btcd v0.22.0 is out.

@onyb
Copy link
Collaborator

onyb commented Sep 21, 2020

With #1617 and btcsuite/btcutil#181 merged, we now have a flexible API to support custom HD version bytes (including SLIP-0132)! 🚀

I let you close the issue after verifying the recent updates. Thanks for your constant feedback. 🙂

@Roasbeef Roasbeef transferred this issue from btcsuite/btcutil Jan 29, 2022
@Roasbeef
Copy link
Member

Transferred from the old btcutil repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants