From a10c96a63b208c1d2a08ddcd402ef7499aee1b29 Mon Sep 17 00:00:00 2001
From: Juan Pablo Civile <elementohb@gmail.com>
Date: Thu, 6 May 2021 19:17:49 -0300
Subject: [PATCH 1/2] htlcswitch: set sourceRef when resolving packets through
 interceptor

Having it set to nil caused https://github.com/lightningnetwork/lnd/issues/5115

The problem was several layers removed from the fix. The link decides to
clean up a `fwdPkg` only if it's completed, otherwise it renotifies the
HTLCs. A package is only set to complete if it's `addAck` and
`settleFail` filters are full. For forwarded HTLCs, the `addAck` was
never being set so it would never be considered complete under this
criteria.

`addAck` is set for an HTLC when signing the next commitment TX in the
`LightningChannel`. The path for this is:
* `LightningChannel#SettleHtlc` adds the HTLC to `localUpdates`
* `LightningChannel#SignNextCommitment` builds the `ackAddRef` for all
updates with `SourceRef != nil`.
* `LightningChannel#SignNextCommitment` then passes the list of
`ackAddRef` to `OpenChannel#AppendRemoteCommitChain` to persist the new
acks in the filter

Since `SourceRef` was nil for interceptor packages, `SignNextCommitment`
ignored it and the ack was never persisted.
---
 htlcswitch/interceptable_switch.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/htlcswitch/interceptable_switch.go b/htlcswitch/interceptable_switch.go
index f6d0d2ef..1e340534 100644
--- a/htlcswitch/interceptable_switch.go
+++ b/htlcswitch/interceptable_switch.go
@@ -179,6 +179,7 @@ func (f *interceptedForward) resolve(message lnwire.Message) error {
 		circuit:        f.packet.circuit,
 		htlc:           message,
 		obfuscator:     f.packet.obfuscator,
+		sourceRef:      f.packet.sourceRef,
 	}
 	return f.htlcSwitch.mailOrchestrator.Deliver(pkt.incomingChanID, pkt)
 }

From 6e4528515e265d8946b35e4d8f9215f1330fbe7d Mon Sep 17 00:00:00 2001
From: Juan Pablo Civile <elementohb@gmail.com>
Date: Fri, 25 Jun 2021 09:50:50 -0300
Subject: [PATCH 2/2] lntest/itest: verify HTLC interceptor renotification bug

---
 lntest/itest/lnd_forward_interceptor_test.go | 45 ++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/lntest/itest/lnd_forward_interceptor_test.go b/lntest/itest/lnd_forward_interceptor_test.go
index 46fdadf3..6275c981 100644
--- a/lntest/itest/lnd_forward_interceptor_test.go
+++ b/lntest/itest/lnd_forward_interceptor_test.go
@@ -13,6 +13,7 @@ import (
 	"github.com/lightningnetwork/lnd/lnrpc"
 	"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
 	"github.com/lightningnetwork/lnd/lntest"
+	"github.com/lightningnetwork/lnd/lntest/wait"
 	"github.com/lightningnetwork/lnd/routing/route"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/codes"
@@ -224,6 +225,50 @@ func testForwardInterceptor(net *lntest.NetworkHarness, t *harnessTest) {
 	// that tests the payment final status for the held payment.
 	cancelInterceptor()
 	wg.Wait()
+
+	// Verify that we don't get notified about already completed HTLCs
+	// We do that by restarting alice, the sender the HTLCs. Under
+	// https://github.com/lightningnetwork/lnd/issues/5115
+	// this should cause all HTLCs settled or failed by the interceptor to renotify.
+	restartAlice, err := net.SuspendNode(alice)
+	require.NoError(t.t, err, "failed to suspend alice")
+
+	ctx = context.Background()
+	ctxt, cancelInterceptor = context.WithTimeout(ctx, defaultTimeout)
+	defer cancelInterceptor()
+	interceptor, err = testContext.bob.RouterClient.HtlcInterceptor(ctxt)
+	require.NoError(t.t, err, "failed to create HtlcInterceptor")
+
+	err = restartAlice()
+	require.NoError(t.t, err, "failed to restart alice")
+
+	go func() {
+		request, err := interceptor.Recv()
+		if err != nil {
+			// If it is  just the error result of the context cancellation
+			// the we exit silently.
+			status, ok := status.FromError(err)
+			if ok && status.Code() == codes.Canceled {
+				return
+			}
+			// Otherwise it an unexpected error, we fail the test.
+			require.NoError(
+				t.t, err, "unexpected error in interceptor.Recv()",
+			)
+			return
+		}
+
+		require.Nil(t.t, request, "no more intercepts should arrive")
+	}()
+
+	err = wait.Predicate(func() bool {
+		channels, err := bob.ListChannels(ctx, &lnrpc.ListChannelsRequest{
+			ActiveOnly: true, Peer: alice.PubKey[:],
+		})
+		return err == nil && len(channels.Channels) > 0
+	}, defaultTimeout)
+	require.NoError(t.t, err, "alice <> bob channel didnt re-activate")
+
 }
 
 // interceptorTestContext is a helper struct to hold the test context and