Skip to content

Commit

Permalink
server/asset/btc: require zpub, but offer xpub conversion
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chappjc committed Oct 28, 2021
1 parent ebfcff7 commit 0124247
Show file tree
Hide file tree
Showing 3 changed files with 265 additions and 8 deletions.
2 changes: 1 addition & 1 deletion dex/testing/dcrdex/harness.sh
Expand Up @@ -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
Expand Down
133 changes: 126 additions & 7 deletions server/asset/btc/addresser.go
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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
}
138 changes: 138 additions & 0 deletions 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)
}
}

0 comments on commit 0124247

Please sign in to comment.