Unit test keystore

.github/workflows/ci.yml

@@ -288,3 +288,19 @@ jobs:
           TEXT: |
             **Oh no! [${{ steps.vars.outputs.git_sha_short }}](https://github.com/${{ github.repository }}/commit/${{ github.sha }}) failed to build.**
             See [run](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) for more details.
+  bitbox02-simulator:
+    # This is a separate job because the simulator runs on Ubuntu 22.04+, while the
+    # Docker CI image is still on Ubuntu 20.04 (the min supported version for BitBoxApp).
+    runs-on: ubuntu-22.04
+    steps:
+      - name: Clone the repo
+        uses: actions/checkout@v4
+        with:
+          submodules: recursive
+      - name: Install Go
+        uses: actions/setup-go@v5
+        with:
+          go-version-file: 'go.mod'
+      - name: Run simulator tests
+        run: |
+          go test -mod=vendor -tags=bitbox02_simulator ./... -count=1 -v -run 'TestSimulator*'

backend/coins/btc/maketx/maketx.go

@@ -24,7 +24,6 @@ import (
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/accounts"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/accounts/errors"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/addresses"
-	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/btc/transactions"
 	coinpkg "github.com/BitBoxSwiss/bitbox-wallet-app/backend/coins/coin"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/backend/signing"
 	"github.com/BitBoxSwiss/bitbox-wallet-app/util/errp"
@@ -36,7 +35,7 @@ import (
 )
 
 // PreviousOutputs represents a UTXO set. It also implements `txscript.PrevOutputFetcher`.
-type PreviousOutputs map[wire.OutPoint]*transactions.SpendableOutput
+type PreviousOutputs map[wire.OutPoint]UTXO
 
 // FetchPrevOutput implements `txscript.PrevOutputFetcher`.
 func (p PreviousOutputs) FetchPrevOutput(op wire.OutPoint) *wire.TxOut {
@@ -75,8 +74,8 @@ func (txProposal *TxProposal) Total() btcutil.Amount {
 
 // UTXO contains the data needed of a spendable UTXO in a new tx.
 type UTXO struct {
-	TxOut         *wire.TxOut
-	Configuration *signing.Configuration
+	TxOut   *wire.TxOut
+	Address *addresses.AccountAddress
 }
 
 type byValue struct {
@@ -128,7 +127,7 @@ func toInputConfigurations(
 ) []*signing.Configuration {
 	inputConfigurations := make([]*signing.Configuration, len(selectedOutPoints))
 	for i, outPoint := range selectedOutPoints {
-		inputConfigurations[i] = spendableOutputs[outPoint].Configuration
+		inputConfigurations[i] = spendableOutputs[outPoint].Address.Configuration
 	}
 	return inputConfigurations
 }
@@ -182,15 +181,11 @@ func NewTxSpendAll(
 ) (*TxProposal, error) {
 	selectedOutPoints := []wire.OutPoint{}
 	inputs := []*wire.TxIn{}
-	previousOutputs := make(PreviousOutputs, len(spendableOutputs))
 	outputsSum := btcutil.Amount(0)
 	for outPoint, output := range spendableOutputs {
 		selectedOutPoints = append(selectedOutPoints, outPoint)
 		outputsSum += btcutil.Amount(output.TxOut.Value)
 		inputs = append(inputs, wire.NewTxIn(&outPoint, nil, nil))
-		previousOutputs[outPoint] = &transactions.SpendableOutput{
-			TxOut: spendableOutputs[outPoint].TxOut,
-		}
 	}
 	txSize := estimateTxSize(
 		toInputConfigurations(spendableOutputs, selectedOutPoints),
@@ -219,7 +214,7 @@ func NewTxSpendAll(
 		Amount:               btcutil.Amount(output.Value),
 		Fee:                  maxRequiredFee,
 		Transaction:          unsignedTransaction,
-		PreviousOutputs:      previousOutputs,
+		PreviousOutputs:      spendableOutputs,
 		SilentPaymentAddress: outputInfo.silentPaymentAddress,
 		// Only one output in send-all
 		OutIndex: 0,
@@ -272,9 +267,7 @@ func NewTx(
 		previousOutputs := make(PreviousOutputs, len(selectedOutPoints))
 		for i, outPoint := range selectedOutPoints {
 			inputs[i] = wire.NewTxIn(&outPoint, nil, nil)
-			previousOutputs[outPoint] = &transactions.SpendableOutput{
-				TxOut: spendableOutputs[outPoint].TxOut,
-			}
+			previousOutputs[outPoint] = spendableOutputs[outPoint]
 		}
 		unsignedTransaction := &wire.MsgTx{
 			Version:  wire.TxVersion,

backend/coins/btc/maketx/maketx_test.go

@@ -121,8 +121,8 @@ func (s *newTxSuite) buildUTXO(satoshis ...int64) map[wire.OutPoint]maketx.UTXO
 	utxo := map[wire.OutPoint]maketx.UTXO{}
 	for i, satoshi := range satoshis {
 		utxo[s.outpoint(i)] = maketx.UTXO{
-			TxOut:         wire.NewTxOut(satoshi, s.someAddresses[0].PubkeyScript()),
-			Configuration: s.inputConfiguration,
+			TxOut:   wire.NewTxOut(satoshi, s.someAddresses[0].PubkeyScript()),
+			Address: s.someAddresses[0],
 		}
 	}
 	return utxo

backend/coins/btc/sign.go

@@ -39,6 +39,20 @@ type ProposedTransaction struct {
 	FormatUnit coin.BtcUnit
 }
 
+// Finalize adds the signatureScript/witness for each input based on the available signatures and
+// input address configurations.
+func (p *ProposedTransaction) Finalize() error {
+	for index, input := range p.TXProposal.Transaction.TxIn {
+		address := p.TXProposal.PreviousOutputs[input.PreviousOutPoint].Address
+		signature := p.Signatures[index]
+		if signature == nil {
+			return errp.New("Signature missing")
+		}
+		input.SignatureScript, input.Witness = address.SignatureScript(*signature)
+	}
+	return nil
+}
+
 // signTransaction signs all inputs. It assumes all outputs spent belong to this
 // wallet. previousOutputs must contain all outputs which are spent by the transaction.
 func (account *Account) signTransaction(
@@ -68,34 +82,30 @@ func (account *Account) signTransaction(
 		return err
 	}
 
-	for index, input := range txProposal.Transaction.TxIn {
-		spentOutput := previousOutputs[input.PreviousOutPoint]
-		address := proposedTransaction.GetAccountAddress(spentOutput.ScriptHashHex())
-		signature := proposedTransaction.Signatures[index]
-		if signature == nil {
-			return errp.New("Signature missing")
-		}
-		input.SignatureScript, input.Witness = address.SignatureScript(*signature)
+	// Insert signatureScripts/witnesses.
+	if err := proposedTransaction.Finalize(); err != nil {
+		return err
 	}
 
 	// Sanity check: see if the created transaction is valid.
-	if err := txValidityCheck(txProposal.Transaction, previousOutputs,
+	if err := TxValidityCheck(txProposal.Transaction, previousOutputs,
 		txProposal.SigHashes()); err != nil {
 		account.log.WithError(err).Panic("Failed to pass transaction validity check.")
 	}
 
 	return nil
 }
 
-func txValidityCheck(transaction *wire.MsgTx, previousOutputs maketx.PreviousOutputs,
+// TxValidityCheck checks if the transaction is valid, including signature/witness checks.
+func TxValidityCheck(transaction *wire.MsgTx, previousOutputs maketx.PreviousOutputs,
 	sigHashes *txscript.TxSigHashes) error {
 	for index, txIn := range transaction.TxIn {
 		spentOutput, ok := previousOutputs[txIn.PreviousOutPoint]
 		if !ok {
 			return errp.New("There needs to be exactly one output being spent per input!")
 		}
-		engine, err := txscript.NewEngine(spentOutput.PkScript, transaction, index,
-			txscript.StandardVerifyFlags, nil, sigHashes, spentOutput.Value, previousOutputs)
+		engine, err := txscript.NewEngine(spentOutput.TxOut.PkScript, transaction, index,
+			txscript.StandardVerifyFlags, nil, sigHashes, spentOutput.TxOut.Value, previousOutputs)
 		if err != nil {
 			return errp.WithStack(err)
 		}

backend/coins/btc/transaction.go

@@ -102,7 +102,7 @@ func (account *Account) pickChangeAddress(utxos map[wire.OutPoint]maketx.UTXO) (
 	if p2trIndex >= 0 {
 		// Check if there is at least one taproot UTXO.
 		for _, utxo := range utxos {
-			if utxo.Configuration.ScriptType() == signing.ScriptTypeP2TR {
+			if utxo.Address.Configuration.ScriptType() == signing.ScriptTypeP2TR {
 				// Found a taproot UTXO.
 				unusedAddresses, err := account.subaccounts[p2trIndex].changeAddresses.GetUnused()
 				if err != nil {
@@ -162,8 +162,8 @@ func (account *Account) newTx(args *accounts.TxProposalArgs) (
 		}
 		wireUTXO[outPoint] = maketx.UTXO{
 			TxOut: txOut.TxOut,
-			Configuration: account.getAddress(
-				blockchain.NewScriptHashHex(txOut.TxOut.PkScript)).Configuration,
+			Address: account.getAddress(
+				blockchain.NewScriptHashHex(txOut.TxOut.PkScript)),
 		}
 	}
 	feeRatePerKb, err := account.getFeePerKb(args)

backend/devices/bitbox/keystore.go

@@ -161,13 +161,13 @@ 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.GetAccountAddress(spentOutput.ScriptHashHex())
+		address := spentOutput.Address
 		isSegwit, subScript := address.ScriptForHashToSign()
 		var signatureHash []byte
 		if isSegwit {
 			var err error
 			signatureHash, err = txscript.CalcWitnessSigHash(subScript, sigHashes,
-				txscript.SigHashAll, transaction, index, spentOutput.Value)
+				txscript.SigHashAll, transaction, index, spentOutput.TxOut.Value)
 			if err != nil {
 				return errp.Wrap(err, "Failed to calculate SegWit signature hash")
 			}

backend/devices/bitbox02/keystore.go

@@ -324,7 +324,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 			return errp.New("There needs to be exactly one output being spent per input.")
 		}
 
-		inputAddress := btcProposedTx.GetAccountAddress(prevOut.ScriptHashHex())
+		inputAddress := prevOut.Address
 
 		accountConfiguration := inputAddress.AccountConfiguration
 		msgScriptType, ok := btcMsgScriptTypeMap[accountConfiguration.ScriptType()]
@@ -348,7 +348,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 			Input: &messages.BTCSignInputRequest{
 				PrevOutHash:       txIn.PreviousOutPoint.Hash[:],
 				PrevOutIndex:      txIn.PreviousOutPoint.Index,
-				PrevOutValue:      uint64(prevOut.Value),
+				PrevOutValue:      uint64(prevOut.TxOut.Value),
 				Sequence:          txIn.Sequence,
 				Keypath:           inputAddress.Configuration.AbsoluteKeypath().ToUInt32(),
 				ScriptConfigIndex: uint32(scriptConfigIndex),
@@ -393,7 +393,7 @@ func (keystore *keystore) signBTCTransaction(btcProposedTx *btc.ProposedTransact
 			payload = outputAddress.ScriptAddress()
 		}
 
-		// Could also determine change using `outputAddress != nil AND second-to-last keypath element of outputAddress is 1`.
+		// Could also determine change using `outputAccountAddress != nil AND second-to-last keypath element of outputAddress is 1`.
 		isChange := txChangeAddress != nil && bytes.Equal(
 			txChangeAddress.PubkeyScript(),
 			txOut.PkScript,