From 8c002a08a7191a7fef5307663aa397f143724cb0 Mon Sep 17 00:00:00 2001
From: nsa <elzeigel@gmail.com>
Date: Mon, 13 Jul 2020 15:34:47 -0400
Subject: [PATCH] lnwallet: properly restore removeCommitHeightRemote

Previously, we could sign a pending commitment for the remote party,
disconnect, and not restore these signed remote updates as having been
removed at the pending commitment height. This commit fixes that to
look up whether the update under the unsigned acked updates key is
present on the pending commitment or not and appropriately set
the remove commit heights.
---
 lnwallet/channel.go | 53 +++++++++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/lnwallet/channel.go b/lnwallet/channel.go
index 7c6a861c..a0ce102a 100644
--- a/lnwallet/channel.go
+++ b/lnwallet/channel.go
@@ -1867,6 +1867,7 @@ func (lc *LightningChannel) restoreStateLogs(
 	// in our next signature.
 	err := lc.restorePendingRemoteUpdates(
 		unsignedAckedUpdates, localCommitment.height,
+		pendingRemoteCommit,
 	)
 	if err != nil {
 		return err
@@ -1879,7 +1880,8 @@ func (lc *LightningChannel) restoreStateLogs(
 // haven't yet signed for.
 func (lc *LightningChannel) restorePendingRemoteUpdates(
 	unsignedAckedUpdates []channeldb.LogUpdate,
-	localCommitmentHeight uint64) error {
+	localCommitmentHeight uint64,
+	pendingRemoteCommit *commitment) error {
 
 	lc.log.Debugf("Restoring %v dangling remote updates",
 		len(unsignedAckedUpdates))
@@ -1894,12 +1896,38 @@ func (lc *LightningChannel) restorePendingRemoteUpdates(
 			return err
 		}
 
+		logIdx := payDesc.LogIndex
+
 		// Sanity check that we are not restoring a remote log update
 		// that we haven't received a sig for.
-		if payDesc.LogIndex >= lc.remoteUpdateLog.logIndex {
+		if logIdx >= lc.remoteUpdateLog.logIndex {
 			return fmt.Errorf("attempted to restore an "+
 				"unsigned remote update: log_index=%v",
-				payDesc.LogIndex)
+				logIdx)
+		}
+
+		// We previously restored Adds along with all the other upates,
+		// but this Add restoration was a no-op as every single one of
+		// these Adds was already restored since they're all incoming
+		// htlcs on the local commitment.
+		if payDesc.EntryType == Add {
+			continue
+		}
+
+		var (
+			height    uint64
+			heightSet bool
+		)
+
+		// If we have a pending commitment for them, and this update
+		// is included in that commit, then we'll use this commitment
+		// height as this commitment will include these updates for
+		// their new remote commitment.
+		if pendingRemoteCommit != nil {
+			if logIdx < pendingRemoteCommit.theirMessageIndex {
+				height = pendingRemoteCommit.height
+				heightSet = true
+			}
 		}
 
 		// Insert the update into the log. The log update index doesn't
@@ -1907,23 +1935,20 @@ func (lc *LightningChannel) restorePendingRemoteUpdates(
 		// final value was properly persisted with the last local
 		// commitment update.
 		switch payDesc.EntryType {
-		case Add:
-			lc.remoteUpdateLog.restoreHtlc(payDesc)
-
-			// Sanity check to be sure that we are not restoring an
-			// add update that the remote hasn't signed for yet.
-			if payDesc.HtlcIndex >= lc.remoteUpdateLog.htlcCounter {
-				return fmt.Errorf("attempted to restore an "+
-					"unsigned remote htlc: htlc_index=%v",
-					payDesc.HtlcIndex)
+		case FeeUpdate:
+			if heightSet {
+				payDesc.addCommitHeightRemote = height
+				payDesc.removeCommitHeightRemote = height
 			}
 
-		case FeeUpdate:
 			lc.remoteUpdateLog.restoreUpdate(payDesc)
 
 		default:
-			lc.remoteUpdateLog.restoreUpdate(payDesc)
+			if heightSet {
+				payDesc.removeCommitHeightRemote = height
+			}
 
+			lc.remoteUpdateLog.restoreUpdate(payDesc)
 			lc.localUpdateLog.markHtlcModified(payDesc.ParentIndex)
 		}
 	}