From 012424702e9f5ddcd8459d04f25eb6bceb859207 Mon Sep 17 00:00:00 2001 From: Jonathan Chappelow Date: Thu, 28 Oct 2021 16:31:22 -0500 Subject: [PATCH] server/asset/btc: require zpub, but offer xpub conversion For a server operator to accept BTC for the registration fee, it is currently necessary to provide an extended public key with the "xpub" prefix. This is because the btcutil/hdkeychain package only recognizes such xpub strings as being from a given network, otherwise the net/version bytes are unknown. However, the derived fee addresses are P2WPKH. The potential trouble is that wallets that generate native segwit addresses tend to return "zpub"-prefixed extended public key encodings in following BIP-84 (vpub for testnet and regnet). I have verified that Electrum does this if the wallet was created as native segwit, and Exodus actually exports both an xpub and zpub that do not seem to correspond to the same extended key (two separate derivation paths) making it potentially ill-advised to use the xpub for segwit addresses! Prior to this PR, an operator with a wallet that provides a zpub would have to convert to an xpub. This is not hard (see here), but it is confusing and unnecessary since the dex software can do it. Further, if they got an xpub from a non-segwit wallet or non-segwit path, it's possibly they will have to go to some trouble to claim funds sent to segwit addresses derived from this extended key. To address this, this PR now requires the key to be with a "zpub" prefix. Also, if an "xpub" is provided, it will convert it to "zpub" and return an error with this string so the operator is aware that segwit addresses will be generated if they decide to use that zpub. --- dex/testing/dcrdex/harness.sh | 2 +- server/asset/btc/addresser.go | 133 +++++++++++++++++++++++++-- server/asset/btc/addresser_test.go | 138 +++++++++++++++++++++++++++++ 3 files changed, 265 insertions(+), 8 deletions(-) create mode 100644 server/asset/btc/addresser_test.go diff --git a/dex/testing/dcrdex/harness.sh b/dex/testing/dcrdex/harness.sh index c90facd25d..fde84e38e3 100755 --- a/dex/testing/dcrdex/harness.sh +++ b/dex/testing/dcrdex/harness.sh @@ -135,7 +135,7 @@ cat << EOF >> "./markets.json" "configPath": "${TEST_ROOT}/btc/alpha/alpha.conf", "regConfs": 2, "regFee": 20000000, - "regXPub": "tpubD6NzVbkrYhZ4XgiXtGrdW5XDAPFCL9h7we1vwNCpn8tGbBcgfVYjXyhWo4E1xkh56hjod1RhGjxbaTLV3X4FyWuejifB9jusQ46QzG87VKp" + "regXPub": "vpub5SLqN2bLY4WeZJ9SmNJHsyzqVKreTXD4ZnPC22MugDNcjhKX5xNX9QiQWcE4SSRzVWyHWUihpKRT7hckDGNzVc69wSX2JPcfGeNiT5c2XZy" EOF if [ $LTC_ON -eq 0 ]; then diff --git a/server/asset/btc/addresser.go b/server/asset/btc/addresser.go index 744857da0b..002834dceb 100644 --- a/server/asset/btc/addresser.go +++ b/server/asset/btc/addresser.go @@ -4,12 +4,14 @@ package btc import ( + "encoding/binary" "errors" "fmt" "sync" "decred.org/dcrdex/server/asset" "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/wire" "github.com/btcsuite/btcutil" "github.com/btcsuite/btcutil/hdkeychain" ) @@ -26,18 +28,57 @@ type AddressDeriver struct { // NewAddressDeriver creates a new AddressDeriver for the provided extended // public key, KeyIndexer, and network parameters. Note that if the source -// wallet has accounts, the extended key should be for an account. +// wallet has accounts (e.g. BIP44), the extended key should be for an account. +// Since AddressDeriver returns P2WPKH addresses, the extended public key should +// be for a P2WPKH derivation path (e.g. zpub for mainnet, and vpub for regnet +// and testnet). However, if a key for a different address encoding is provided +// that is otherwise valid for the specified network, the error message will +// suggest the equivalent extended key for P2WPKH addresses, but the operator +// must be certain they can redeem payments to P2WPKH addresses. This is done +// instead of returning different encodings depending on the extended key format +// to prevent accidental reuse of keys. +// +// Refs: +// https://github.com/bitcoin/bips/blob/master/bip-0084.mediawiki#extended-key-version +// https://github.com/satoshilabs/slips/blob/master/slip-0132.md +// +// See TestAddressDeriver to double check your own keys and the first addresses. func NewAddressDeriver(xpub string, keyIndexer asset.KeyIndexer, chainParams *chaincfg.Params) (*AddressDeriver, uint32, error) { key, err := hdkeychain.NewKeyFromString(xpub) if err != nil { return nil, 0, fmt.Errorf("error parsing master pubkey: %w", err) } - if !key.IsForNet(chainParams) { - return nil, 0, fmt.Errorf("key is for the wrong network, wanted %s", chainParams.Name) - } if key.IsPrivate() { return nil, 0, errors.New("private key provided") } + + // Get the desired version bytes for a p2wpkh extended key for this network. + var wantVer uint32 + switch chainParams.Net { + case wire.MainNet: + wantVer = mainnetWitnessHashVer + case wire.TestNet, wire.TestNet3: // regnet and testnet3 have the same version bytes + wantVer = testnetWitnessHashVer + default: + return nil, 0, fmt.Errorf("unsupported network %v", chainParams.Name) + } + + // See if we recognize the version bytes for the given network. key.IsForNet + // will only verify against chainParams.HDPublicKeyID, which btcsuite sets + // for the legacy (non-segwit) P2PKH derivation path. + ver := key.Version() + prefix := pubVerPrefix(ver, chainParams.Net) + if prefix == "" { + return nil, 0, fmt.Errorf("key type is not recognized for network %s", chainParams.Name) + } + version := binary.BigEndian.Uint32(ver) + if version != wantVer { + // Patch the public key version bytes to suggest the p2wpkh equivalent. + key.SetNet(&chaincfg.Params{HDPublicKeyID: verBytes(wantVer)}) + return nil, 0, fmt.Errorf("provided extended key (prefix %q) not for p2wpkh, equivalent is %q", + prefix, key.String()) + } + external, _, err := getChild(key, 0) // derive from the external branch (not change addresses) if err != nil { return nil, 0, fmt.Errorf("unexpected key derivation error: %w", err) @@ -72,9 +113,9 @@ func getChild(xkey *hdkeychain.ExtendedKey, i uint32) (*hdkeychain.ExtendedKey, } } -// NextAddress retrieves the pkh address for the next pubkey. While this should -// always return a valid address, an empty string may be returned in the event -// of an unexpected internal error. +// NextAddress retrieves the p2wpkh address for the next pubkey. While this +// should always return a valid address, an empty string may be returned in the +// event of an unexpected internal error. func (ap *AddressDeriver) NextAddress() (string, error) { ap.mtx.Lock() defer ap.mtx.Unlock() @@ -104,3 +145,81 @@ func (ap *AddressDeriver) NextAddress() (string, error) { } return addrP2WPKH.String(), nil } + +func verBytes(ver uint32) [4]byte { + var vers [4]byte + binary.BigEndian.PutUint32(vers[:], ver) + return vers +} + +// HD version bytes and prefixes +// https://github.com/satoshilabs/slips/blob/master/slip-0132.md + +const ( + mainnetWitnessHashVer uint32 = 0x04b24746 + testnetWitnessHashVer uint32 = 0x045f1cf6 // also regtest/regnet +) + +var pubVersionPrefixesMainnet = map[uint32]string{ + 0x0488b21e: "xpub", // P2PKH or P2SH + 0x049d7cb2: "ypub", // P2WPKH *in* P2SH + 0x04b24746: "zpub", // P2WPKH + 0x0295b43f: "Ypub", // Multi-signature P2WSH in P2SH + 0x02aa7ed3: "Zpub", // Multi-signature P2WSH +} +var pubPrefixVersionsMainnet map[string]uint32 + +var pubVersionPrefixesTestnet = map[uint32]string{ + 0x043587cf: "tpub", // P2PKH or P2SH + 0x044a5262: "upub", // P2WPKH *in* P2SH + 0x045f1cf6: "vpub", // P2WPKH + 0x024289ef: "Upub", // Multi-signature P2WSH in P2SH + 0x02575483: "Vpub", // Multi-signature P2WSH +} +var pubPrefixVersionsTestnet map[string]uint32 + +func init() { + pubPrefixVersionsMainnet = make(map[string]uint32, len(pubVersionPrefixesMainnet)) + for ver, pref := range pubVersionPrefixesMainnet { + pubPrefixVersionsMainnet[pref] = ver + } + pubPrefixVersionsTestnet = make(map[string]uint32, len(pubVersionPrefixesTestnet)) + for ver, pref := range pubVersionPrefixesTestnet { + pubPrefixVersionsTestnet[pref] = ver + } +} + +func pubVerPrefix(vers []byte, net wire.BitcoinNet) string { + if len(vers) != 4 { + return "" + } + ver := binary.BigEndian.Uint32(vers) + switch net { + case wire.MainNet: + return pubVersionPrefixesMainnet[ver] + case wire.TestNet, wire.TestNet3: // regnet and testnet3 have same version bytes + return pubVersionPrefixesTestnet[ver] + default: + return "" + } +} + +func pubPrefixVer(prefix string, net wire.BitcoinNet) []byte { + if len(prefix) != 4 { + return nil + } + var ver uint32 + var found bool + switch net { + case wire.MainNet: + ver, found = pubPrefixVersionsMainnet[prefix] + case wire.TestNet, wire.TestNet3: // regnet and testnet3 have same version bytes + ver, found = pubPrefixVersionsTestnet[prefix] + } + if !found { + return nil + } + version := make([]byte, 4) + binary.BigEndian.PutUint32(version, ver) + return version +} diff --git a/server/asset/btc/addresser_test.go b/server/asset/btc/addresser_test.go new file mode 100644 index 0000000000..d3edd0b90f --- /dev/null +++ b/server/asset/btc/addresser_test.go @@ -0,0 +1,138 @@ +// This code is available on the terms of the project LICENSE.md file, +// also available online at https://blueoakcouncil.org/license/1.0.0. + +package btc + +import ( + "bytes" + "encoding/binary" + "strings" + "testing" + + "decred.org/dcrdex/server/asset" + "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil/hdkeychain" +) + +// Dummy keyIndexer since there is no DB or need to coordinate withother +// goroutines. +type keyIndexer struct { + idx map[string]uint32 +} + +func newKeyIndexer() *keyIndexer { + return &keyIndexer{make(map[string]uint32)} +} + +func (ki *keyIndexer) KeyIndex(xpub string) (uint32, error) { + return ki.idx[xpub], nil +} + +func (ki *keyIndexer) SetKeyIndex(idx uint32, xpub string) error { + ki.idx[xpub] = idx + return nil +} + +var _ asset.KeyIndexer = (*keyIndexer)(nil) + +func TestAddressDeriver(t *testing.T) { + var x uint32 = 0x0488b21e + x2 := binary.BigEndian.Uint32([]byte{0x04, 0x88, 0xb2, 0x1e}) + if x != x2 { + t.Fatalf("%d != %d", x, x2) + } + + params := &chaincfg.MainNetParams + // zpub := "zpub6nKbQwE3qUJUhehpUf2sETXY1v2mm1VM5ou4xdM3PPMudQfuo8XC3DPM3esiGCRhT1JBubsydS2UFM2eX8Uh2tSx6FhDY3GKQGESmbARSmR" + // xpubWant := "xpub68f4obtDY7DX14KaowTcpHLXfyjssmWMFardPqZGdNc9XD3THpC4o6551ExYGP7rdj4aQegri7KNUmoX5jefSR5kMaJNNDdLrp79zQBSGaK" + zpub := "zpub6r6v49qBjybMzg17LsqFDYDG3oYssB5GkQ4BdQtjCDkSeBPAgXxHBYXgNGy3LxfHwf5JwvkJTBqHHjkXKdwheZdtLKdcXCU1SMkzryt1evp" + xpubWant := "xpub6CSPSpVMScWQJ5csgAFzoN2FhsFyyw6GvB1k4d6xSCzgXykiBDd9wRDQKs3sM9MT8NqhSyZBXs8BXAXPtF7g46GgbeEmMNq2tudi5k68MjJ" + // params := &chaincfg.TestNet3Params + // zpub := "vpub5UpDTWU6Nuj9uuU5TZnxjS97SAjKwY4UiLvvnYNRC6vmvbDa5toZRG1BkqCnLSRmNYuLuqLDPtdDq6YvELoMUSPjVSFTsX1H42kabkJDwWD" + // xpubWant := "tpubD8rNb5dcPYmZtJ3AaUMJMXfV7E7spAYY6CZfhtDLJ2SRn5WjfRymopzJ3HCjrkgqyjfs2N3CrKANHrGf4bUcxMDEHiPcisJVBSUH8wNhdoA" + + net := params.Net + + // P2PWKH extended key prefix is: zpub on mainnet, vpub on regnet/testnet3 + var xpubPrefix, zpubPrefix string + switch net { + case wire.MainNet: + xpubPrefix = "xpub" + zpubPrefix = "zpub" + case wire.TestNet, wire.TestNet3: + xpubPrefix = "tpub" + zpubPrefix = "vpub" + } + + key, err := hdkeychain.NewKeyFromString(zpub) + if err != nil { + t.Fatalf("error parsing master pubkey: %v", err) + } + if key.IsPrivate() { + t.Fatal("that's a private key") + } + if key.IsForNet(params) { + t.Fatal("btcsuite's hdkeychain recognized our zpub!") + } + + vers := key.Version() + prefix := pubVerPrefix(vers, net) + if prefix == "" { + t.Fatalf("invalid prefix for version bytes %x", vers) + } + if prefix != zpubPrefix { + t.Fatalf("unexpected prefix %q for version bytes %x", prefix, vers) + } + if v := pubPrefixVer(prefix, net); !bytes.Equal(v, vers) { + t.Fatal("inconsistent version") + } + + key.SetNet(params) // params uses xpub version bytes + + vers = key.Version() + prefix = pubVerPrefix(vers, net) + if prefix == "" { + t.Fatalf("bad prefix for version bytes %x", vers) + } + if prefix != xpubPrefix { + t.Fatalf("unexpected prefix %q for version bytes %x", prefix, vers) + } + if v := pubPrefixVer(prefix, net); !bytes.Equal(v, vers) { + t.Fatal("inconsistent version") + } + + xpub := key.String() + if xpub != xpubWant { + t.Fatal(xpub, " != ", xpubWant) + } + + ki := newKeyIndexer() + + // NewAddressDeriver should not like the xpub, and suggest the zpub. + _, _, err = NewAddressDeriver(xpub, ki, params) + if err == nil { + t.Fatal("no error from NewAddressDeriver for an xpub prefixed key") + } + if !strings.Contains(err.Error(), zpub) { + t.Fatalf("NewAddressDeriver should have suggested the corresponding zpub, said: %q", err.Error()) + } + + // Should work with the zpub. + ad, _, err := NewAddressDeriver(zpub, ki, params) + if err != nil { + t.Fatal(err) + } + + for i := 0; i < 10; i++ { + addr, err := ad.NextAddress() + if err != nil { + t.Fatal(err) + } + t.Log(addr) + } + + if ad.next != 10 { + t.Fatal(ad.next) + } +}