dcrutil: Fix NewTxDeepTxIns implementation.

This fixes NewTxDeepTxIns implementation. The current implementation
of NewTxDeepTxIns is wrong and has many issues. This commit fixes
this issues and adds tests for ensuring the API works correctly.

dcrutil/tx.go

@@ -1,5 +1,5 @@
 // Copyright (c) 2013-2016 The btcsuite developers
-// Copyright (c) 2015-2017 The Decred developers
+// Copyright (c) 2015-2019 The Decred developers
 // Use of this source code is governed by an ISC
 // license that can be found in the LICENSE file.
 
@@ -154,41 +154,43 @@ func NewTxDeepTxIns(msgTx *wire.MsgTx) *Tx {
 		return nil
 	}
 
-	newMsgTx := new(wire.MsgTx)
+	txIns := make([]*wire.TxIn, len(msgTx.TxIn))
+	txOuts := make([]*wire.TxOut, len(msgTx.TxOut))
 
-	// Copy the fixed fields.
-	newMsgTx.Version = msgTx.Version
-	newMsgTx.LockTime = msgTx.LockTime
-	newMsgTx.Expiry = msgTx.Expiry
+	for i, txin := range msgTx.TxIn {
+		sigScript := make([]byte, len(txin.SignatureScript))
+		copy(sigScript[:], txin.SignatureScript[:])
 
-	// Copy the TxIns deeply.
-	for _, txIn := range msgTx.TxIn {
-		sigScrLen := len(txIn.SignatureScript)
-		sigScrCopy := make([]byte, sigScrLen)
-
-		txInCopy := new(wire.TxIn)
-		txInCopy.PreviousOutPoint.Hash = txIn.PreviousOutPoint.Hash
-		txInCopy.PreviousOutPoint.Index = txIn.PreviousOutPoint.Index
-		txInCopy.PreviousOutPoint.Tree = txIn.PreviousOutPoint.Tree
-
-		txInCopy.Sequence = txIn.Sequence
-		txInCopy.ValueIn = txIn.ValueIn
-		txInCopy.BlockHeight = txIn.BlockHeight
-		txInCopy.BlockIndex = txIn.BlockIndex
-
-		txInCopy.SignatureScript = sigScrCopy
-
-		newMsgTx.AddTxIn(txIn)
+		txIns[i] = &wire.TxIn{
+			PreviousOutPoint: wire.OutPoint{
+				Hash:  txin.PreviousOutPoint.Hash,
+				Index: txin.PreviousOutPoint.Index,
+				Tree:  txin.PreviousOutPoint.Tree,
+			},
+			Sequence:        txin.Sequence,
+			ValueIn:         txin.ValueIn,
+			BlockHeight:     txin.BlockHeight,
+			BlockIndex:      txin.BlockIndex,
+			SignatureScript: sigScript,
+		}
 	}
 
 	// Shallow copy the TxOuts.
-	for _, txOut := range msgTx.TxOut {
-		newMsgTx.AddTxOut(txOut)
+	copy(txOuts, msgTx.TxOut)
+
+	newTxMsg := &wire.MsgTx{
+		CachedHash: nil,
+		SerType:    msgTx.SerType,
+		Version:    msgTx.Version,
+		TxIn:       txIns,
+		TxOut:      txOuts,
+		LockTime:   msgTx.LockTime,
+		Expiry:     msgTx.Expiry,
 	}
 
 	return &Tx{
-		hash:    newMsgTx.TxHash(),
-		msgTx:   newMsgTx,
+		hash:    newTxMsg.TxHash(),
+		msgTx:   newTxMsg,
 		txTree:  wire.TxTreeUnknown,
 		txIndex: TxIndexUnknown,
 	}

dcrutil/tx_test.go

@@ -171,3 +171,50 @@ func TestNewTxDeep(t *testing.T) {
 		}
 	}
 }
+
+// TestNewTxDeepTxIns tests the API for creation of a Tx with deep TxIn copy.
+func TestNewTxDeepTxIns(t *testing.T) {
+	tx := Block100000.Transactions[0]
+	copyTxDeep := NewTxDeepTxIns(tx)
+	cpTx := copyTxDeep.MsgTx()
+
+	// Ensure original and copied transactions has equal values.
+	if !reflect.DeepEqual(tx, cpTx) {
+		t.Fatalf("MsgTx is not equal - got %v, want %v",
+			spew.Sdump(cpTx), spew.Sdump(&tx))
+	}
+
+	// Ensure original and copied transactions refer to different allocations.
+	if tx == cpTx {
+		t.Fatal("MsgTx is referring to the same allocation")
+	}
+
+	// Compare each original and copied input transaction allocations.
+	for i := 0; i < len(tx.TxIn); i++ {
+		// Ensure input transactions refer to different allocations.
+		if tx.TxIn[i] == cpTx.TxIn[i] {
+			t.Errorf("TxIn #%d is referring to the same allocation", i)
+		}
+
+		// Ensure previous transaction output points refer to different
+		// allocations.
+		if &tx.TxIn[i].PreviousOutPoint == &cpTx.TxIn[i].PreviousOutPoint {
+			t.Errorf("PreviousOutPoint #%d is referring to the same"+
+				" allocation", i)
+		}
+
+		// Ensure signature scripts refer to different allocations.
+		if &tx.TxIn[i].SignatureScript[0] == &cpTx.TxIn[i].SignatureScript[0] {
+			t.Errorf("SignatureScript #%d is referring to the same"+
+				" allocation", i)
+		}
+	}
+
+	// Compare each original and copied output transaction allocations.
+	for i := 0; i < len(tx.TxOut); i++ {
+		// Ensure output transactions refer to same allocation.
+		if tx.TxOut[i] != cpTx.TxOut[i] {
+			t.Errorf("TxOut #%d is not referring to same allocation", i)
+		}
+	}
+}