Merge branch 'btc-sign-refactor'

Author: Beerosagos

backend/coins/btc/sign.go

@@ -33,7 +33,7 @@ type ProposedTransaction struct {
 	TXProposal *maketx.TxProposal
 	// List of signing configurations that might be used in the tx inputs.
 	AccountSigningConfigurations []*signing.Configuration
-	GetAddress                   func(blockchain.ScriptHashHex) *addresses.AccountAddress
+	GetAccountAddress            func(blockchain.ScriptHashHex) *addresses.AccountAddress
 	GetPrevTx                    func(chainhash.Hash) (*wire.MsgTx, error)
 	// Signatures collects the signatures, one per transaction input.
 	Signatures []*types.Signature
@@ -56,7 +56,7 @@ func (account *Account) signTransaction(
 	proposedTransaction := &ProposedTransaction{
 		TXProposal:                   txProposal,
 		AccountSigningConfigurations: signingConfigs,
-		GetAddress:                   account.getAddress,
+		GetAccountAddress:            account.getAddress,
 		GetPrevTx:                    getPrevTx,
 		Signatures:                   make([]*types.Signature, len(txProposal.Transaction.TxIn)),
 		SigHashes:                    txscript.NewTxSigHashes(txProposal.Transaction, previousOutputs),
@@ -73,7 +73,7 @@ func (account *Account) signTransaction(
 
 	for index, input := range txProposal.Transaction.TxIn {
 		spentOutput := previousOutputs[input.PreviousOutPoint]
-		address := proposedTransaction.GetAddress(spentOutput.ScriptHashHex())
+		address := proposedTransaction.GetAccountAddress(spentOutput.ScriptHashHex())
 		signature := proposedTransaction.Signatures[index]
 		if signature == nil {
 			return errp.New("Signature missing")

backend/devices/bitbox/keystore.go

@@ -160,7 +160,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 			keystore.log.Error("There needs to be exactly one output being spent per input.")
 			return errp.New("There needs to be exactly one output being spent per input.")
 		}
-		address := btcProposedTx.GetAddress(spentOutput.ScriptHashHex())
+		address := btcProposedTx.GetAccountAddress(spentOutput.ScriptHashHex())
 		isSegwit, subScript := address.ScriptForHashToSign()
 		var signatureHash []byte
 		if isSegwit {

backend/devices/bitbox02/keystore.go

@@ -293,6 +293,8 @@ func (keystore *keystore) ExtendedPublicKey(
 func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransaction) error {
 	tx := btcProposedTx.TXProposal.Transaction
 
+	// scriptConfigs represent the script configurations of a specific account and include the
+	// script type (e.g. p2wpkh, p2tr..) and the account keypath
 	scriptConfigs := []*messages.BTCScriptConfigWithKeypath{}
 	// addScriptConfig returns the index of the scriptConfig in scriptConfigs, adding it if it isn't
 	// present. Must be a Simple configuration.
@@ -312,6 +314,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 		return errp.Newf("coin not supported: %s", coin.Code())
 	}
 
+	// iterate over the tx inputs to add the related scriptconfigs and translate into `firmware.BTCTxInput` format.
 	inputs := make([]*firmware.BTCTxInput, len(tx.TxIn))
 	for inputIndex, txIn := range tx.TxIn {
 		prevOut, ok := btcProposedTx.TXProposal.PreviousOutputs[txIn.PreviousOutPoint]
@@ -320,7 +323,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 			return errp.New("There needs to be exactly one output being spent per input.")
 		}
 
-		inputAddress := btcProposedTx.GetAddress(prevOut.ScriptHashHex())
+		inputAddress := btcProposedTx.GetAccountAddress(prevOut.ScriptHashHex())
 
 		accountConfiguration := inputAddress.AccountConfiguration
 		msgScriptType, ok := btcMsgScriptTypeMap[accountConfiguration.ScriptType()]
@@ -344,14 +347,18 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 		}
 	}
 
+	txChangeAddress := btcProposedTx.TXProposal.ChangeAddress
+
+	// iterate over tx outputs to add the related scriptconfigs, flag internal addresses and build
+	// the output signing requests.
 	outputs := make([]*messages.BTCSignOutputRequest, len(tx.TxOut))
 	for index, txOut := range tx.TxOut {
-		address, err := util.AddressFromPkScript(txOut.PkScript, coin.Net())
+		outputAddress, err := util.AddressFromPkScript(txOut.PkScript, coin.Net())
 		if err != nil {
 			return err
 		}
 		var msgOutputType messages.BTCOutputType
-		switch address.(type) {
+		switch outputAddress.(type) {
 		case *btcutil.AddressPubKeyHash:
 			msgOutputType = messages.BTCOutputType_P2PKH
 		case *btcutil.AddressScriptHash:
@@ -363,18 +370,20 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 		case *btcutil.AddressTaproot:
 			msgOutputType = messages.BTCOutputType_P2TR
 		default:
-			return errp.Newf("unsupported output type: %v", address)
+			return errp.Newf("unsupported output type: %v", outputAddress)
 		}
-		outputAddress := btcProposedTx.GetAddress(blockchain.NewScriptHashHex(txOut.PkScript))
 
-		changeAddress := btcProposedTx.TXProposal.ChangeAddress
 		// Could also determine change using `outputAddress != nil AND second-to-last keypath element of outputAddress is 1`.
-		isChange := changeAddress != nil && bytes.Equal(
-			changeAddress.PubkeyScript(),
+		isChange := txChangeAddress != nil && bytes.Equal(
+			txChangeAddress.PubkeyScript(),
 			txOut.PkScript,
 		)
 
-		isOurs := outputAddress != nil
+		// outputAccountAddress represents the same address as outputAddress, but embeds the account configuration.
+		// It is nil if the address is external.
+		outputAccountAddress := btcProposedTx.GetAccountAddress(blockchain.NewScriptHashHex(txOut.PkScript))
+
+		isOurs := outputAccountAddress != nil
 		if !isChange && !keystore.device.Version().AtLeast(semver.NewSemVer(9, 15, 0)) {
 			// For firmware older than 9.15.0, non-change outputs cannot be marked internal.
 			isOurs = false
@@ -383,8 +392,8 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 		var keypath []uint32
 		var scriptConfigIndex int
 		if isOurs {
-			keypath = outputAddress.Configuration.AbsoluteKeypath().ToUInt32()
-			accountConfiguration := outputAddress.AccountConfiguration
+			keypath = outputAccountAddress.Configuration.AbsoluteKeypath().ToUInt32()
+			accountConfiguration := outputAccountAddress.AccountConfiguration
 			msgScriptType, ok := btcMsgScriptTypeMap[accountConfiguration.ScriptType()]
 			if !ok {
 				return errp.Newf("Unsupported script type %s", accountConfiguration.ScriptType())
@@ -399,7 +408,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 			Ours:              isOurs,
 			Type:              msgOutputType,
 			Value:             uint64(txOut.Value),
-			Payload:           address.ScriptAddress(),
+			Payload:           outputAddress.ScriptAddress(),
 			Keypath:           keypath,
 			ScriptConfigIndex: uint32(scriptConfigIndex),
 		}
@@ -438,6 +447,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 		}
 	}
 
+	// Handle displaying formatting in btc or sats.
 	formatUnit := messages.BTCSignInitRequest_DEFAULT
 	if btcProposedTx.FormatUnit == coinpkg.BtcUnitSats {
 		formatUnit = messages.BTCSignInitRequest_SAT

backend/keystore/software/software.go

@@ -197,7 +197,7 @@ func (keystore *Keystore) SignTransaction(
 			keystore.log.Error("There needs to be exactly one output being spent per input.")
 			return errp.New("There needs to be exactly one output being spent per input.")
 		}
-		address := btcProposedTx.GetAddress(spentOutput.ScriptHashHex())
+		address := btcProposedTx.GetAccountAddress(spentOutput.ScriptHashHex())
 
 		xprv, err := address.Configuration.AbsoluteKeypath().Derive(keystore.master)
 		if err != nil {