lnwallet: don't use persistent pointer to funding tx within channel state machine

This commit fixes a lingering bug that could at times cause
incompatibilities with other implementations when attempting a
cooperative channel close. Before this commit, we would use a pointer
to the funding txin everywhere. As a result, each time we made a new
state, or verified one, we would modify the sequence field of the main
txin of the commitment transaction. Due to this if we updated the
channel, then went to do a cooperative channel closure, the sequence of
the txin would still be set to the value we used as the state hint.

To remedy this, we now copy the txin each time when making the
commitment transaction, and also the cooperative closure transaction.
This avoids accidentally mutating the txin itself.

Fixes #502.

htlcswitch/test_utils.go

@@ -154,7 +154,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte,
 
 	aliceCommitTx, bobCommitTx, err := lnwallet.CreateCommitmentTxns(aliceAmount,
 		bobAmount, &aliceCfg, &bobCfg, aliceCommitPoint, bobCommitPoint,
-		fundingTxIn)
+		*fundingTxIn)
 	if err != nil {
 		return nil, nil, nil, nil, err
 	}

lnwallet/channel.go

@@ -1171,7 +1171,7 @@ type LightningChannel struct {
 	// that opened the channel.
 	FundingWitnessScript []byte
 
-	fundingTxIn  *wire.TxIn
+	fundingTxIn  wire.TxIn
 	fundingP2WSH []byte
 
 	// ForceCloseSignal is a channel that is closed to indicate that a
@@ -1297,7 +1297,7 @@ func NewLightningChannel(signer Signer, events chainntnfs.ChainNotifier,
 	if err != nil {
 		return nil, err
 	}
-	lc.fundingTxIn = wire.NewTxIn(&state.FundingOutpoint, nil, nil)
+	lc.fundingTxIn = *wire.NewTxIn(&state.FundingOutpoint, nil, nil)
 	lc.fundingP2WSH = fundingPkScript
 	lc.signDesc = &SignDescriptor{
 		PubKey:        lc.localChanCfg.MultiSigKey,
@@ -4990,7 +4990,7 @@ func (lc *LightningChannel) generateRevocation(height uint64) (*lnwire.RevokeAnd
 // to the "owner" of the commitment transaction which can be spent after a
 // relative block delay or revocation event, and the other paying the
 // counterparty within the channel, which can be spent immediately.
-func CreateCommitTx(fundingOutput *wire.TxIn,
+func CreateCommitTx(fundingOutput wire.TxIn,
 	keyRing *commitmentKeyRing, csvTimeout uint32,
 	amountToSelf, amountToThem, dustLimit btcutil.Amount) (*wire.MsgTx, error) {
 
@@ -5020,7 +5020,7 @@ func CreateCommitTx(fundingOutput *wire.TxIn,
 	// the transaction itself. We use a transaction version of 2 since CSV
 	// will fail unless the tx version is >= 2.
 	commitTx := wire.NewMsgTx(2)
-	commitTx.AddTxIn(fundingOutput)
+	commitTx.AddTxIn(&fundingOutput)
 
 	// Avoid creating dust outputs within the commitment transaction.
 	if amountToSelf >= dustLimit {
@@ -5045,7 +5045,7 @@ func CreateCommitTx(fundingOutput *wire.TxIn,
 // constructing the channel is the initiator of the closure. Currently it is
 // expected that the initiator pays the transaction fees for the closing
 // transaction in full.
-func CreateCooperativeCloseTx(fundingTxIn *wire.TxIn,
+func CreateCooperativeCloseTx(fundingTxIn wire.TxIn,
 	localDust, remoteDust, ourBalance, theirBalance btcutil.Amount,
 	ourDeliveryScript, theirDeliveryScript []byte,
 	initiator bool) *wire.MsgTx {
@@ -5055,7 +5055,7 @@ func CreateCooperativeCloseTx(fundingTxIn *wire.TxIn,
 	// within the channel then a refund output for that particular side can
 	// be omitted.
 	closeTx := wire.NewMsgTx(2)
-	closeTx.AddTxIn(fundingTxIn)
+	closeTx.AddTxIn(&fundingTxIn)
 
 	// Create both cooperative closure outputs, properly respecting the
 	// dust limits of both parties.

lnwallet/channel_test.go

@@ -275,7 +275,7 @@ func createTestChannels(revocationWindow int) (*LightningChannel, *LightningChan
 
 	aliceCommitTx, bobCommitTx, err := CreateCommitmentTxns(channelBal,
 		channelBal, &aliceCfg, &bobCfg, aliceCommitPoint, bobCommitPoint,
-		fundingTxIn)
+		*fundingTxIn)
 	if err != nil {
 		return nil, nil, nil, err
 	}

lnwallet/script_utils_test.go

@@ -76,7 +76,7 @@ func TestCommitmentSpendValidation(t *testing.T) {
 		revocationKey: revokePubKey,
 		noDelayKey:    bobPayKey,
 	}
-	commitmentTx, err := CreateCommitTx(fakeFundingTxIn, keyRing, csvTimeout,
+	commitmentTx, err := CreateCommitTx(*fakeFundingTxIn, keyRing, csvTimeout,
 		channelBalance, channelBalance, DefaultDustLimit())
 	if err != nil {
 		t.Fatalf("unable to create commitment transaction: %v", nil)

lnwallet/wallet.go

@@ -666,7 +666,7 @@ func (l *LightningWallet) handleFundingCancelRequest(req *fundingReserveCancelMs
 func CreateCommitmentTxns(localBalance, remoteBalance btcutil.Amount,
 	ourChanCfg, theirChanCfg *channeldb.ChannelConfig,
 	localCommitPoint, remoteCommitPoint *btcec.PublicKey,
-	fundingTxIn *wire.TxIn) (*wire.MsgTx, *wire.MsgTx, error) {
+	fundingTxIn wire.TxIn) (*wire.MsgTx, *wire.MsgTx, error) {
 
 	localCommitmentKeys := deriveCommitmentKeys(localCommitPoint, true,
 		ourChanCfg, theirChanCfg)
@@ -819,7 +819,7 @@ func (l *LightningWallet) handleContributionMsg(req *addContributionMsg) {
 
 	// Create the txin to our commitment transaction; required to construct
 	// the commitment transactions.
-	fundingTxIn := &wire.TxIn{
+	fundingTxIn := wire.TxIn{
 		PreviousOutPoint: wire.OutPoint{
 			Hash:  fundingTxID,
 			Index: multiSigIndex,
@@ -1150,7 +1150,7 @@ func (l *LightningWallet) handleSingleFunderSigs(req *addSingleFunderSigsMsg) {
 		pendingReservation.theirContribution.ChannelConfig,
 		pendingReservation.ourContribution.FirstCommitmentPoint,
 		pendingReservation.theirContribution.FirstCommitmentPoint,
-		fundingTxIn,
+		*fundingTxIn,
 	)
 	if err != nil {
 		req.err <- err

test_utils.go

@@ -119,7 +119,7 @@ func createTestPeer(notifier chainntnfs.ChainNotifier,
 
 	aliceCommitTx, bobCommitTx, err := lnwallet.CreateCommitmentTxns(channelBal,
 		channelBal, &aliceCfg, &bobCfg, aliceCommitPoint, bobCommitPoint,
-		fundingTxIn)
+		*fundingTxIn)
 	if err != nil {
 		return nil, nil, nil, nil, err
 	}