lnwallet: include htlcs to settle in returned set of htlc's to forward

This commit fixes a slight bug in the channel state machine’s code
executed when processing a revocation messages. With this commit after
processing a revocation, log entries which we should forward to the
downstream or upstream peer for settling/adding HTLC’s are now properly
returned.

The testa have also been updated to ensure to correct htlc’s are
returned “for forwarding”.

lnwallet/channel.go

@@ -113,6 +113,12 @@ type PaymentDescriptor struct {
 	// remote party added, or one that we added locally.
 	Index uint32
 
+	// ParentIndex is the index of the log entry that this HTLC update
+	// settles or times out. If IsIncoming is false, then this refers to an
+	// index within our local log, otherwise this refers to an entry int he
+	// remote peer's log.
+	ParentIndex uint32
+
 	// Payload is an opaque blob which is used to complete multi-hop routing.
 	Payload []byte
 
@@ -135,7 +141,7 @@ type PaymentDescriptor struct {
 	addCommitHeightRemote uint64
 	addCommitHeightLocal  uint64
 
-	/// removeCommitHeight[Remote|Local] encodes the height of the
+	// removeCommitHeight[Remote|Local] encodes the height of the
 	//commitment which removed the parent pointer of this PaymentDescriptor
 	//either due to a timeout or a settle. Once both these heights are
 	//above the tail of both chains, the log entries can safely be removed.
@@ -942,25 +948,32 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.CommitRevocation) (
 		next = e.Next()
 		htlc := e.Value.(*PaymentDescriptor)
 
-		if htlc.entryType != Add {
+		if htlc.isForwarded {
-			// If this entry is either a timeout or settle, then we
+			continue
-			// can remove it from our log once the update it locked
+		}
-			// into both of our chains.
+
-			if remoteChainTail >= htlc.removeCommitHeightRemote &&
+		// If this entry is either a timeout or settle, then we
-				localChainTail >= htlc.removeCommitHeightLocal {
+		// can remove it from our log once the update it locked
-				parentLink := htlc.parent
+		// into both of our chains.
-				lc.stateUpdateLog.Remove(e)
+		if htlc.entryType != Add &&
-				lc.stateUpdateLog.Remove(parentLink)
+			remoteChainTail >= htlc.removeCommitHeightRemote &&
+			localChainTail >= htlc.removeCommitHeightLocal {
+
+			parentLink := htlc.parent
+			lc.stateUpdateLog.Remove(e)
+			lc.stateUpdateLog.Remove(parentLink)
+
+			if !htlc.IsIncoming {
+				htlc.ParentIndex = parentLink.Value.(*PaymentDescriptor).Index
+				htlcsToForward = append(htlcsToForward, htlc)
 			}
-		} else if !htlc.isForwarded {
+		} else if remoteChainTail >= htlc.addCommitHeightRemote &&
+			localChainTail >= htlc.addCommitHeightLocal {
+
 			// Once an HTLC has been fully locked into both of our
 			// chains, then we can safely forward it to the next
 			// hop.
-			// TODO(roasbeef): only incoming htcls?
+			if htlc.IsIncoming {
-			//  * re-visit once multi-hop from jcp
-			if remoteChainTail >= htlc.addCommitHeightRemote &&
-				localChainTail >= htlc.addCommitHeightLocal {
-
 				htlc.isForwarded = true
 				htlcsToForward = append(htlcsToForward, htlc)
 			}

lnwallet/channel_test.go

@@ -2,7 +2,6 @@ package lnwallet
 
 import (
 	"bytes"
-	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
@@ -355,13 +354,17 @@ func TestSimpleAddSettleWorkflow(t *testing.T) {
 	}
 	if htlcs, err := bobChannel.ReceiveRevocation(aliceRevocation2); err != nil {
 		t.Fatalf("bob unable to process alice's revocation: %v", err)
-	} else {
+	} else if len(htlcs) != 0 {
-		fmt.Println("bob forward htlcs: %v", htlcs)
+		t.Fatalf("bob shouldn't forward any HTLC's after outgoing settle, "+
+			"instead can forward: %v", spew.Sdump(htlcs))
 	}
 	if htlcs, err := aliceChannel.ReceiveRevocation(bobRevocation2); err != nil {
 		t.Fatalf("alice unable to process bob's revocation: %v", err)
-	} else {
+	} else if len(htlcs) != 1 {
-		fmt.Println("alice forward htlcs: %v", spew.Sdump(htlcs))
+		// Alice should now be able to forward the settlement HTLC to
+		// any down stream peers.
+		t.Fatalf("alice should be able to forward a single HTLC, "+
+			"instead can forward %v: %v", len(htlcs), spew.Sdump(htlcs))
 	}
 
 	// At this point, bob should have 6BTC settled, with Alice still having
@@ -416,3 +419,7 @@ func TestSimpleAddSettleWorkflow(t *testing.T) {
 			"instead", bobLogLen)
 	}
 }
+
+func TestCooperativeChannelClosure(t *testing.T) {
+	// * add validation of their sig
+}