From a3fd738491be1d669ebb448715887bd5e9b9303d Mon Sep 17 00:00:00 2001
From: Olaoluwa Osuntokun <laolu32@gmail.com>
Date: Tue, 11 Apr 2017 21:31:22 -0700
Subject: [PATCH] lnwallet: fix HTLC mutation bug in commitment chain
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This commit fixes a class of bug that can arise in the channel state
machine when a very high throughput workflow is attempted. Since the
PaymentDescriptor’s within a commitment pointed directly into the log,
any changes to a payment descriptor would also be reflected in all
other ones. Due to this mutation possibility, at times, the
locateOutputIndex method would fail since the HTLC’s pkScript was
modified, causing the channel to fail.

We fix this class of bug by simply ensure that once an HTLC has been
associated with a particular commitment, then it becomes immutable.
---
 lnwallet/channel.go | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index 2a2e0d29..a4660197 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -236,8 +236,8 @@ type commitment struct {
 
 	// htlcs is the set of HTLCs which remain unsettled within this
 	// commitment.
-	outgoingHTLCs []*PaymentDescriptor
-	incomingHTLCs []*PaymentDescriptor
+	outgoingHTLCs []PaymentDescriptor
+	incomingHTLCs []PaymentDescriptor
 }
 
 // toChannelDelta converts the target commitment into a format suitable to be
@@ -1302,17 +1302,28 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool,
 	// ordering. This lets us skip sending the entire transaction over,
 	// instead we'll just send signatures.
 	txsort.InPlaceSort(commitTx)
-
-	return &commitment{
+	c := &commitment{
 		txn:               commitTx,
 		height:            nextHeight,
 		ourBalance:        ourBalance,
 		ourMessageIndex:   ourLogIndex,
 		theirMessageIndex: theirLogIndex,
 		theirBalance:      theirBalance,
-		outgoingHTLCs:     filteredHTLCView.ourUpdates,
-		incomingHTLCs:     filteredHTLCView.theirUpdates,
-	}, nil
+	}
+
+	// In order to ensure _none_ of the HTLC's associated with this new
+	// commitment are mutated, we'll manually copy over each HTLC to its
+	// respective slice.
+	c.outgoingHTLCs = make([]PaymentDescriptor, len(filteredHTLCView.ourUpdates))
+	for i, htlc := range filteredHTLCView.ourUpdates {
+		c.outgoingHTLCs[i] = *htlc
+	}
+	c.incomingHTLCs = make([]PaymentDescriptor, len(filteredHTLCView.theirUpdates))
+	for i, htlc := range filteredHTLCView.theirUpdates {
+		c.incomingHTLCs[i] = *htlc
+	}
+
+	return c, nil
 }
 
 // evaluateHTLCView processes all update entries in both HTLC update logs,