Merge pull request #51 from bitcoin-sv/feature/sign-unsigned

add Transaction.SignUnsigned

Author: rohenaz

CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file. The format
 ## Table of Contents
 
 - [Unreleased](#unreleased)
+- [1.1.7 - 2024-09-10](#117---2024-09-10)
 - [1.1.6 - 2024-09-09](#116---2024-09-09)
 - [1.1.5 - 2024-09-06](#115---2024-09-06)
 - [1.1.4 - 2024-09-05](#114---2024-09-05)
@@ -14,6 +15,19 @@ All notable changes to this project will be documented in this file. The format
 - [1.1.0 - 2024-08-19](#110---2024-08-19)
 - [1.0.0 - 2024-06-06](#100---2024-06-06)
 
+
+## [1.1.7] - 2024-09-10
+  - Rework `tx.Clone()` to be more efficient
+  - Introduce SignUnsigned to sign only inputs that have not already been signed
+  - Added tests
+  - Other minor performance improvements.
+
+  ### Added
+  - New method `Transaction.SignUnsigned()`
+
+  ### Changed
+  - `Transaction.Clone()` does not reconstitute the source transaction from bytes. Creates a new transaction.
+
 ## [1.1.6] - 2024-09-09
   - Optimize handling of source transaction inputs. Avoid mocking up entire transaction when adding source inputs.
   - Minor alignment in ECIES helper function

script/interpreter/operations.go

@@ -2016,7 +2016,7 @@ func opcodeCheckSig(op *ParsedOpcode, t *thread) error {
 		return err
 	}
 
-	txCopy := t.tx.ShallowClone()
+	txCopy := t.tx.Clone()
 	sourceTxOut := txCopy.Inputs[t.inputIdx].SourceTxOutput()
 	sourceTxOut.LockingScript = up
 
@@ -2282,7 +2282,7 @@ func opcodeCheckMultiSig(op *ParsedOpcode, t *thread) error {
 		}
 
 		// Generate the signature hash based on the signature hash type.
-		txCopy := t.tx.ShallowClone()
+		txCopy := t.tx.Clone()
 		input := txCopy.Inputs[t.inputIdx]
 		sourceOut := input.SourceTxOutput()
 		if sourceOut != nil {

transaction/signaturehash.go

@@ -188,7 +188,7 @@ func (tx *Transaction) CalcInputPreimageLegacy(inputNumber uint32, shf sighash.F
 		return defaultHex, nil
 	}
 
-	txCopy := tx.ShallowClone()
+	txCopy := tx.Clone()
 
 	for i := range txCopy.Inputs {
 		if i == int(inputNumber) {

transaction/transaction.go

@@ -5,13 +5,13 @@ import (
 	"encoding/binary"
 	"encoding/hex"
 	"io"
-	"log"
 	"slices"
 
 	"github.com/bitcoin-sv/go-sdk/chainhash"
 	crypto "github.com/bitcoin-sv/go-sdk/primitives/hash"
 	"github.com/bitcoin-sv/go-sdk/script"
 	"github.com/bitcoin-sv/go-sdk/util"
+	"github.com/pkg/errors"
 )
 
 type Transaction struct {
@@ -313,26 +313,7 @@ func (tx *Transaction) BytesWithClearedInputs(index int, lockingScript []byte) [
 	return tx.toBytesHelper(index, lockingScript, false)
 }
 
-// Clone returns a clone of the tx
 func (tx *Transaction) Clone() *Transaction {
-	// Ignore err as byte slice passed in is created from valid tx
-	clone, err := NewTransactionFromBytes(tx.Bytes())
-	if err != nil {
-		log.Fatal(err)
-	}
-
-	for i, input := range tx.Inputs {
-		if input.SourceTransaction != nil {
-			clone.Inputs[i].SourceTransaction = input.SourceTransaction.Clone()
-		}
-		// clone.Inputs[i].SourceTransaction = input.SourceTransaction
-		clone.Inputs[i].sourceOutput = input.sourceOutput
-	}
-
-	return clone
-}
-
-func (tx *Transaction) ShallowClone() *Transaction {
 	// Creating a new Tx from scratch is much faster than cloning from bytes
 	// ~ 420ns/op vs 2200ns/op of the above function in benchmarking
 	// this matters as we clone txs a couple of times when verifying signatures
@@ -345,9 +326,10 @@ func (tx *Transaction) ShallowClone() *Transaction {
 
 	for i, input := range tx.Inputs {
 		clone.Inputs[i] = &TransactionInput{
-			SourceTXID:       (*chainhash.Hash)(input.SourceTXID[:]),
-			SourceTxOutIndex: input.SourceTxOutIndex,
-			SequenceNumber:   input.SequenceNumber,
+			SourceTXID:              (*chainhash.Hash)(input.SourceTXID[:]),
+			SourceTxOutIndex:        input.SourceTxOutIndex,
+			SequenceNumber:          input.SequenceNumber,
+			UnlockingScriptTemplate: input.UnlockingScriptTemplate,
 		}
 		if input.UnlockingScript != nil {
 			clone.Inputs[i].UnlockingScript = input.UnlockingScript
@@ -436,7 +418,12 @@ func (tx *Transaction) AddMerkleProof(bump *MerklePath) error {
 	return nil
 }
 
+// Fee returns the fee of the transaction.
 func (tx *Transaction) Sign() error {
+	err := tx.checkFeeComputed()
+	if err != nil {
+		return err
+	}
 	for vin, i := range tx.Inputs {
 		if i.UnlockingScriptTemplate != nil {
 			unlock, err := i.UnlockingScriptTemplate.Sign(tx, uint32(vin))
@@ -448,3 +435,32 @@ func (tx *Transaction) Sign() error {
 	}
 	return nil
 }
+
+// SignUnsigned signs the transaction without the unlocking script.
+func (tx *Transaction) SignUnsigned() error {
+	err := tx.checkFeeComputed()
+	if err != nil {
+		return err
+	}
+	for vin, i := range tx.Inputs {
+		if i.UnlockingScript == nil {
+			if i.UnlockingScriptTemplate != nil {
+				unlock, err := i.UnlockingScriptTemplate.Sign(tx, uint32(vin))
+				if err != nil {
+					return err
+				}
+				i.UnlockingScript = unlock
+			}
+		}
+	}
+	return nil
+}
+
+func (tx *Transaction) checkFeeComputed() error {
+	for _, out := range tx.Outputs {
+		if out.Satoshis == 0 && out.Change {
+			return errors.New("fee not computed")
+		}
+	}
+	return nil
+}

transaction/transaction_test.go

@@ -1,8 +1,10 @@
 package transaction_test
 
 import (
+	"encoding/hex"
 	"testing"
 
+	"github.com/bitcoin-sv/go-sdk/chainhash"
 	ec "github.com/bitcoin-sv/go-sdk/primitives/ec"
 	"github.com/bitcoin-sv/go-sdk/script"
 	"github.com/bitcoin-sv/go-sdk/transaction"
@@ -57,6 +59,19 @@ func TestNewTransaction(t *testing.T) {
 	})
 }
 
+func TestIsCoinbase(t *testing.T) {
+	tx, err := transaction.NewTransactionFromHex("01000000010000000000000000000000000000000000000000000000000000000000000000ffffffff17033f250d2f43555656452f2c903fb60859897700d02700ffffffff01d864a012000000001976a914d648686cf603c11850f39600e37312738accca8f88ac00000000")
+	require.NoError(t, err)
+	require.True(t, tx.IsCoinbase())
+}
+
+func TestIsValidTxID(t *testing.T) {
+	valid, _ := hex.DecodeString("fe77aa03d5563d3ec98455a76655ea3b58e19a4eb102baf7b2a47af37e94b295")
+	require.True(t, transaction.IsValidTxID(valid))
+	invalid, _ := hex.DecodeString("fe77aa03d5563d3ec98455a76655ea3b58e19a4eb102baf7b2a47af37e94b2")
+	require.False(t, transaction.IsValidTxID(invalid))
+}
+
 func TestBEEF(t *testing.T) {
 	t.Parallel()
 	t.Run("deserialize and serialize", func(t *testing.T) {
@@ -79,13 +94,97 @@ func TestEF(t *testing.T) {
 	})
 }
 
-func Benchmark_ShallowClone(b *testing.B) {
+func TestClone(t *testing.T) {
+	tx, err := transaction.NewTransactionFromBEEFHex(BRC62Hex)
+	require.NoError(t, err)
+
+	clone := tx.Clone()
+	require.Equal(t, tx.Bytes(), clone.Bytes())
+}
+
+func BenchmarkClone(b *testing.B) {
 	tx, _ := transaction.NewTransactionFromHex("0200000003a9bc457fdc6a54d99300fb137b23714d860c350a9d19ff0f571e694a419ff3a0010000006b48304502210086c83beb2b2663e4709a583d261d75be538aedcafa7766bd983e5c8db2f8b2fc02201a88b178624ab0ad1748b37c875f885930166237c88f5af78ee4e61d337f935f412103e8be830d98bb3b007a0343ee5c36daa48796ae8bb57946b1e87378ad6e8a090dfeffffff0092bb9a47e27bf64fc98f557c530c04d9ac25e2f2a8b600e92a0b1ae7c89c20010000006b483045022100f06b3db1c0a11af348401f9cebe10ae2659d6e766a9dcd9e3a04690ba10a160f02203f7fbd7dfcfc70863aface1a306fcc91bbadf6bc884c21a55ef0d32bd6b088c8412103e8be830d98bb3b007a0343ee5c36daa48796ae8bb57946b1e87378ad6e8a090dfeffffff9d0d4554fa692420a0830ca614b6c60f1bf8eaaa21afca4aa8c99fb052d9f398000000006b483045022100d920f2290548e92a6235f8b2513b7f693a64a0d3fa699f81a034f4b4608ff82f0220767d7d98025aff3c7bd5f2a66aab6a824f5990392e6489aae1e1ae3472d8dffb412103e8be830d98bb3b007a0343ee5c36daa48796ae8bb57946b1e87378ad6e8a090dfeffffff02807c814a000000001976a9143a6bf34ebfcf30e8541bbb33a7882845e5a29cb488ac76b0e60e000000001976a914bd492b67f90cb85918494767ebb23102c4f06b7088ac67000000")
 
 	b.Run("clone", func(b *testing.B) {
 		for i := 0; i < b.N; i++ {
-			clone := tx.ShallowClone()
+			clone := tx.Clone()
 			_ = clone
 		}
 	})
 }
+
+func TestUncomputedFee(t *testing.T) {
+	tx, _ := transaction.NewTransactionFromBEEFHex(BRC62Hex)
+
+	tx.AddOutput(&transaction.TransactionOutput{
+		Change:        true,
+		LockingScript: tx.Outputs[0].LockingScript,
+	})
+
+	err := tx.Sign()
+	require.Error(t, err)
+
+	err = tx.SignUnsigned()
+	require.Error(t, err)
+}
+
+func TestSignUnsigned(t *testing.T) {
+	tx, err := transaction.NewTransactionFromBEEFHex(BRC62Hex)
+	require.NoError(t, err)
+
+	cloneTx := tx.Clone()
+	pk, _ := ec.NewPrivateKey()
+
+	// Adding a script template with random key so sigs will be different
+	for i := range tx.Inputs {
+		cloneTx.Inputs[i].UnlockingScriptTemplate, err = p2pkh.Unlock(pk, nil)
+		require.NoError(t, err)
+	}
+
+	// This should do nothing because the inputs from hex are already signed
+	err = cloneTx.SignUnsigned()
+	require.NoError(t, err)
+	for i := range cloneTx.Inputs {
+		require.Equal(t, tx.Inputs[i].UnlockingScript, cloneTx.Inputs[i].UnlockingScript)
+	}
+
+	// This should sign the inputs with the incorrect key which should change the sigs
+	cloneTx.Sign()
+	for i := range tx.Inputs {
+		require.NotEqual(t, tx.Inputs[i].UnlockingScript, cloneTx.Inputs[i].UnlockingScript)
+	}
+}
+
+func TestSignUnsignedNew(t *testing.T) {
+	pk, _ := ec.PrivateKeyFromWif("L1y6DgX4TuonxXzRPuk9reK2TD2THjwQReNUwVrvWN3aRkjcbauB")
+	address, _ := script.NewAddressFromPublicKey(pk.PubKey(), true)
+	tx := transaction.NewTransaction()
+	lockingScript, err := p2pkh.Lock(address)
+	require.NoError(t, err)
+	sourceTxID, _ := chainhash.NewHashFromHex("fe77aa03d5563d3ec98455a76655ea3b58e19a4eb102baf7b2a47af37e94b295")
+	unlockingScript, _ := p2pkh.Unlock(pk, nil)
+	tx.AddInput(&transaction.TransactionInput{
+		SourceTransaction: &transaction.Transaction{
+			Outputs: []*transaction.TransactionOutput{
+				{
+					Satoshis:      1,
+					LockingScript: lockingScript,
+				},
+			},
+		},
+		SourceTXID:              sourceTxID,
+		UnlockingScriptTemplate: unlockingScript,
+	})
+
+	tx.AddOutput(&transaction.TransactionOutput{
+		Satoshis:      1,
+		LockingScript: lockingScript,
+	})
+
+	err = tx.SignUnsigned()
+	require.NoError(t, err)
+
+	for _, input := range tx.Inputs {
+		require.Positive(t, len(input.UnlockingScript.Bytes()))
+	}
+}