backend/btc/address: be explicit about chainIndex/addressIndex

The relative keypath always has two elements, the first element
indicating change and the second indicating the address index.

A relative keypath of arbitrary length is harder to handle. E.g. it is
not compatible with output descriptors with multipath
indices (`xpub/<0;1>/*`), where also two elements are required to
derive an address (the multipath index selecting between `<X;Y>` and the address index).

The unit tests change because the GetAddress test function used an
empty relative path instead of a chainIndex and addressIndex, which
now was forced to change with the stricter types.

I considered keeping `ChainIndex uint32` instead of `Change bool`,
because in multipaths, one could have more than two,
e.g. `xpub/<X;Y;Z>/*`, but I prefer the stricter type and it's not
likely we will ever need more than two (receive & change).

Author: benma

backend/backend_test.go

@@ -151,7 +151,7 @@ func MockBtcAccount(t *testing.T, config *accounts.AccountConfig, coin *btc.Coin
 			for _, signingConfig := range config.Config.SigningConfigurations {
 				addressChain := addresses.NewAddressChain(
 					signingConfig,
-					coin.Net(), 20, 0,
+					coin.Net(), 20, false,
 					func(*addresses.AccountAddress) (bool, error) {
 						return false, nil
 					},

backend/coins/btc/account.go

@@ -331,9 +331,9 @@ func (account *Account) Initialize() error {
 		account.log.Infof("gap limits: receive=%d, change=%d", gapLimits.Receive, gapLimits.Change)
 
 		subacc.receiveAddresses = addresses.NewAddressChain(
-			signingConfiguration, account.coin.Net(), int(gapLimits.Receive), 0, account.isAddressUsed, account.log)
+			signingConfiguration, account.coin.Net(), int(gapLimits.Receive), false, account.isAddressUsed, account.log)
 		subacc.changeAddresses = addresses.NewAddressChain(
-			signingConfiguration, account.coin.Net(), int(gapLimits.Change), 1, account.isAddressUsed, account.log)
+			signingConfiguration, account.coin.Net(), int(gapLimits.Change), true, account.isAddressUsed, account.log)
 
 		account.subaccounts = append(account.subaccounts, subacc)
 	}

backend/coins/btc/addresses/address.go

@@ -47,22 +47,28 @@ type AccountAddress struct {
 }
 
 // NewAccountAddress creates a new account address.
+// The chainIndex is 0 for receive and 1 for change.
 func NewAccountAddress(
 	accountConfiguration *signing.Configuration,
-	keyPath signing.RelativeKeypath,
+	derivation types.Derivation,
 	net *chaincfg.Params,
 	log *logrus.Entry,
 ) *AccountAddress {
 
 	var address btcutil.Address
 	var redeemScript []byte
-	configuration, err := accountConfiguration.Derive(keyPath)
+	configuration, err := accountConfiguration.Derive(
+		signing.NewEmptyRelativeKeypath().
+			Child(derivation.SimpleChainIndex(), signing.NonHardened).
+			Child(derivation.AddressIndex, signing.NonHardened),
+	)
 	if err != nil {
 		log.WithError(err).Panic("Failed to derive the configuration.")
 	}
 	log = log.WithFields(logrus.Fields{
-		"key-path":      configuration.AbsoluteKeypath().Encode(),
-		"configuration": configuration.String(),
+		"accountConfiguration": accountConfiguration.String(),
+		"change":               derivation.Change,
+		"addressIndex":         derivation.AddressIndex,
 	})
 	log.Debug("Creating new account address")
 

backend/coins/btc/addresses/address_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/addresses"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/addresses/test"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/blockchain"
+	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/types"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/signing"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/util/logging"
 	testlog "github.com/BitBoxSwiss/bitbox-wallet-app/util/test"
@@ -38,8 +39,6 @@ func TestMain(m *testing.M) {
 
 var net = &chaincfg.TestNet3Params
 
-var absoluteKeypath = signing.NewEmptyAbsoluteKeypath().Child(0, false).Child(10, false)
-
 type addressTestSuite struct {
 	suite.Suite
 
@@ -53,21 +52,24 @@ func TestAddressTestSuite(t *testing.T) {
 }
 
 func (s *addressTestSuite) TestNewAddress() {
-	s.Require().Equal(absoluteKeypath, s.address.Configuration.AbsoluteKeypath())
-	s.Require().Equal("n2gAErwJCuPmnQuhzPkkWi2haGz9oQxjnX", s.address.EncodeAddress())
+	expectedKeypath := signing.NewEmptyAbsoluteKeypath().
+		Child(0, false).
+		Child(10, false).
+		Child(0, false).
+		Child(0, false)
+	s.Require().Equal(expectedKeypath, s.address.Configuration.AbsoluteKeypath())
+	s.Require().Equal("moTM88EgqzATgCjSrcNfahXaT9uCy3FHh3", s.address.EncodeAddress())
 	s.Require().True(s.address.IsForNet(net))
 }
 
 func (s *addressTestSuite) TestPubkeyScript() {
-	payToAddrScript := []byte{
-		0x76, 0xa9, 0x14, 0xe8, 0x18, 0x5b, 0x34, 0x52, 0x22, 0xbe, 0x2b, 0x77, 0x2f,
-		0x7a, 0xef, 0x16, 0x2c, 0x11, 0x85, 0x73, 0x2, 0x9d, 0xf4, 0x88, 0xac}
+	payToAddrScript := []byte{0x76, 0xa9, 0x14, 0x57, 0x12, 0x66, 0x22, 0x63, 0x8b, 0x8d, 0xb3, 0x25, 0xa6, 0x1, 0x35, 0xe2, 0xfd, 0x88, 0x53, 0x74, 0x91, 0xa1, 0xe0, 0x88, 0xac}
 	s.Require().Equal(payToAddrScript, s.address.PubkeyScript())
 }
 
 func (s *addressTestSuite) TestScriptHashHex() {
 	s.Require().Equal(
-		blockchain.ScriptHashHex("0466d0029406f583feadaccb91c7b5b855eb5d6782316cafa4f390b7c784436b"),
+		blockchain.ScriptHashHex("1f4444773ff74188b4d8ccff2a2efec0cae61efce152cafef97b6fadb96382b5"),
 		s.address.PubkeyScriptHashHex())
 }
 
@@ -79,27 +81,30 @@ func TestAddressP2TR(t *testing.T) {
 	keypath, err := signing.NewAbsoluteKeypath("m/86'/0'/0'")
 	require.NoError(t, err)
 	for _, test := range []struct {
-		path             string
+		change           bool
+		addressIndex     uint32
 		expectedAddress  string
 		expectedPkScript string
 	}{
 		{
-			path:             "0/0",
+			change:           false,
+			addressIndex:     0,
 			expectedAddress:  "bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr",
 			expectedPkScript: "5120a60869f0dbcf1dc659c9cecbaf8050135ea9e8cdc487053f1dc6880949dc684c",
 		},
 		{
-			path:             "0/1",
+			change:           false,
+			addressIndex:     1,
 			expectedAddress:  "bc1p4qhjn9zdvkux4e44uhx8tc55attvtyu358kutcqkudyccelu0was9fqzwh",
 			expectedPkScript: "5120a82f29944d65b86ae6b5e5cc75e294ead6c59391a1edc5e016e3498c67fc7bbb",
 		},
 		{
-			path:             "1/0",
+			change:           true,
+			addressIndex:     0,
 			expectedAddress:  "bc1p3qkhfews2uk44qtvauqyr2ttdsw7svhkl9nkm9s9c3x4ax5h60wqwruhk7",
 			expectedPkScript: "5120882d74e5d0572d5a816cef0041a96b6c1de832f6f9676d9605c44d5e9a97d3dc",
 		},
 	} {
-		relKeypath, err := signing.NewRelativeKeypath(test.path)
 		require.NoError(t, err)
 		addr := addresses.NewAccountAddress(
 			signing.NewBitcoinConfiguration(
@@ -108,7 +113,7 @@ func TestAddressP2TR(t *testing.T) {
 				keypath,
 				extendedPublicKey,
 			),
-			relKeypath,
+			types.Derivation{Change: test.change, AddressIndex: test.addressIndex},
 			&chaincfg.MainNetParams,
 			logging.Get().WithGroup("addresses_test"),
 		)

backend/coins/btc/addresses/addresschain.go

@@ -16,6 +16,7 @@ package addresses
 
 import (
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/blockchain"
+	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/types"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/signing"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/util/errp"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/util/locker"
@@ -28,7 +29,7 @@ type AddressChain struct {
 	accountConfiguration *signing.Configuration
 	net                  *chaincfg.Params
 	gapLimit             int
-	chainIndex           uint32
+	change               bool
 	addresses            []*AccountAddress
 	addressesLookup      map[blockchain.ScriptHashHex]*AccountAddress
 	addressesLock        locker.Locker
@@ -41,20 +42,20 @@ func NewAddressChain(
 	accountConfiguration *signing.Configuration,
 	net *chaincfg.Params,
 	gapLimit int,
-	chainIndex uint32,
+	change bool,
 	isAddressUsed func(*AccountAddress) (bool, error),
 	log *logrus.Entry,
 ) *AddressChain {
 	return &AddressChain{
 		accountConfiguration: accountConfiguration,
 		net:                  net,
 		gapLimit:             gapLimit,
-		chainIndex:           chainIndex,
+		change:               change,
 		addresses:            []*AccountAddress{},
 		addressesLookup:      map[blockchain.ScriptHashHex]*AccountAddress{},
 		isAddressUsed:        isAddressUsed,
 		log: log.WithFields(logrus.Fields{"group": "addresses", "net": net.Name,
-			"gap-limit": gapLimit, "chain-index": chainIndex,
+			"gap-limit": gapLimit, "change": change,
 			"configuration": accountConfiguration.String()}),
 	}
 }
@@ -79,7 +80,7 @@ func (addresses *AddressChain) addAddress() *AccountAddress {
 	index := uint32(len(addresses.addresses))
 	address := NewAccountAddress(
 		addresses.accountConfiguration,
-		signing.NewEmptyRelativeKeypath().Child(addresses.chainIndex, signing.NonHardened).Child(index, signing.NonHardened),
+		types.Derivation{Change: addresses.change, AddressIndex: index},
 		addresses.net,
 		addresses.log,
 	)

backend/coins/btc/addresses/addresschain_test.go

@@ -32,7 +32,7 @@ type addressChainTestSuite struct {
 	addresses     *addresses.AddressChain
 	xpub          *hdkeychain.ExtendedKey
 	gapLimit      int
-	chainIndex    uint32
+	change        bool
 	isAddressUsed func(*addresses.AccountAddress) bool
 	log           *logrus.Entry
 }
@@ -46,14 +46,14 @@ func (s *addressChainTestSuite) SetupTest() {
 		panic(err)
 	}
 	s.gapLimit = 6
-	s.chainIndex = 1
+	s.change = true
 	s.xpub = xpub
 	s.addresses = addresses.NewAddressChain(
 		signing.NewBitcoinConfiguration(
 			signing.ScriptTypeP2PKH, []byte{1, 2, 3, 4}, signing.NewEmptyAbsoluteKeypath(), xpub),
 		net,
 		s.gapLimit,
-		s.chainIndex,
+		s.change,
 		func(address *addresses.AccountAddress) (bool, error) {
 			return s.isAddressUsed(address), nil
 		},
@@ -122,7 +122,11 @@ func (s *addressChainTestSuite) TestEnsureAddresses() {
 	s.Require().Len(newAddresses, s.gapLimit)
 	// Check that the pubkeys behind the new addresses are derived in sequence from the root xpub.
 	getPubKey := func(index int) *btcec.PublicKey {
-		chain, err := s.xpub.Derive(s.chainIndex)
+		chainIndex := uint32(0)
+		if s.change {
+			chainIndex = 1
+		}
+		chain, err := s.xpub.Derive(chainIndex)
 		if err != nil {
 			panic(err)
 		}

backend/coins/btc/addresses/test/test.go

@@ -16,6 +16,7 @@ package test
 
 import (
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/addresses"
+	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/types"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/signing"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/util/logging"
 	"github.com/btcsuite/btcd/btcutil/hdkeychain"
@@ -48,7 +49,7 @@ func NewAddressChain(
 	}
 	configuration := signing.NewBitcoinConfiguration(
 		signing.ScriptTypeP2PKH, []byte{1, 2, 3, 4}, derivationPath, xpub)
-	return configuration, addresses.NewAddressChain(configuration, net, 20, 0, isAddressUsed, log)
+	return configuration, addresses.NewAddressChain(configuration, net, 20, false, isAddressUsed, log)
 }
 
 // GetAddress returns a dummy address for a given address type.
@@ -61,7 +62,7 @@ func GetAddress(scriptType signing.ScriptType) *addresses.AccountAddress {
 		scriptType, []byte{1, 2, 3, 4}, absoluteKeypath, extendedPublicKey)
 	return addresses.NewAccountAddress(
 		configuration,
-		signing.NewEmptyRelativeKeypath(),
+		types.Derivation{Change: false, AddressIndex: 0},
 		net,
 		logging.Get().WithGroup("addresses_test"),
 	)

backend/coins/btc/types/types.go

@@ -52,3 +52,21 @@ func (s *Signature) SerializeCompact() []byte {
 	s.S.FillBytes(result[32:])
 	return result
 }
+
+// Derivation contains the derivation information to derive address-level information from a BTC
+// account descriptor.
+type Derivation struct {
+	// Change is true to derive change address details and false to derive receive address details.
+	Change       bool
+	AddressIndex uint32
+}
+
+// SimpleChainIndex returns 0 for receive and 1 or change. This applies to standard BIP-44
+// derivations, but not to multipath descriptors.
+func (d Derivation) SimpleChainIndex() uint32 {
+	if d.Change {
+		return 1
+	}
+
+	return 0
+}

backend/devices/bitbox02/keystore_simulator_test.go

@@ -335,10 +335,10 @@ func makeTx(t *testing.T, device *Device, recipient *maketx.OutputInfo) *btc.Pro
 		makeConfig(t, device, signing.ScriptTypeP2WPKH, mustKeypath("m/84'/0'/0'")),
 		makeConfig(t, device, signing.ScriptTypeP2WPKHP2SH, mustKeypath("m/49'/0'/0'")),
 	}
-	inputAddress0 := addresses.NewAccountAddress(configurations[0], signing.NewEmptyRelativeKeypath().Child(0, false).Child(0, false), network, log)
-	inputAddress1 := addresses.NewAccountAddress(configurations[1], signing.NewEmptyRelativeKeypath().Child(0, false).Child(0, false), network, log)
-	inputAddress2 := addresses.NewAccountAddress(configurations[2], signing.NewEmptyRelativeKeypath().Child(0, false).Child(0, false), network, log)
-	changeAddress := addresses.NewAccountAddress(configurations[0], signing.NewEmptyRelativeKeypath().Child(1, false).Child(0, false), network, log)
+	inputAddress0 := addresses.NewAccountAddress(configurations[0], types.Derivation{Change: false, AddressIndex: 0}, network, log)
+	inputAddress1 := addresses.NewAccountAddress(configurations[1], types.Derivation{Change: false, AddressIndex: 0}, network, log)
+	inputAddress2 := addresses.NewAccountAddress(configurations[2], types.Derivation{Change: false, AddressIndex: 0}, network, log)
+	changeAddress := addresses.NewAccountAddress(configurations[0], types.Derivation{Change: true, AddressIndex: 1}, network, log)
 
 	prevTx := &wire.MsgTx{
 		Version: 2,
@@ -449,7 +449,7 @@ func TestSimulatorSignBTCTransactionSendSelfSameAccount(t *testing.T) {
 		cfg := makeConfig(t, device, signing.ScriptTypeP2TR, mustKeypath("m/86'/0'/0'"))
 		selfAddress := addresses.NewAccountAddress(
 			cfg,
-			signing.NewEmptyRelativeKeypath().Child(0, false).Child(0, false),
+			types.Derivation{Change: false, AddressIndex: 0},
 			network,
 			log)
 		proposedTransaction := makeTx(t, device, maketx.NewOutputInfo(selfAddress.PubkeyScript()))