process: allow outgoing channel not found for failed htlcs

Update error handling for outgoing channel not found to catch the case
where an outgoing channel was not found for a failed HTLC. Unlike
incoming HTLCs, where the HTLC arrived on the channel so we know it
exists, we have not yet performed any existence validation on the
outgoing channel (because interception happens before we check that it
exists).

Since we only need the outgoing channel for record keeping, we just
log the case where a HTLC was failed back and we don't know the channel
(since this is just a bogus channel). We don't store this HTLC in the
DB, as it will be instantly failed back.

peer_controller.go

@@ -88,7 +88,7 @@ type peerInterceptEvent struct {
 
 type peerResolvedEvent struct {
 	resolvedEvent
-	outgoingPeer route.Vertex
+	outgoingPeer *route.Vertex
 }
 
 type peerState struct {
@@ -433,6 +433,13 @@ func (p *peerController) markHtlcComplete(ctx context.Context, key circuitKey,
 		return
 	}
 
+	// If we couldn't look up an outgoing peer for the HTLC, either:
+	// 1. The outgoing channel never existed (since this is not validated on intercept)
+	// 2. The outgoing channel is pending close at time of resolution (edge case)
+	if resolution.outgoingPeer == nil {
+		return
+	}
+
 	// Track available HTLC information and report to handler.
 	htlcInfo := &HtlcInfo{
 		addTime:         inFlight.addedTs,
@@ -443,7 +450,7 @@ func (p *peerController) markHtlcComplete(ctx context.Context, key circuitKey,
 		incomingCircuit: key,
 		outgoingCircuit: resolution.outgoingCircuitKey,
 		incomingPeer:    p.pubKey,
-		outgoingPeer:    resolution.outgoingPeer,
+		outgoingPeer:    *resolution.outgoingPeer,
 	}
 
 	if err := p.htlcCompleted(ctx, htlcInfo); err != nil {

process.go

@@ -337,18 +337,35 @@ func (p *process) eventLoop(ctx context.Context, group *errgroup.Group) error {
 
 			ctrl := p.getPeerController(ctx, chanInfo.peer, group.Go)
 
-			// Lookup the outgoing peer to supplement the
-			// information on the resolved event.
+			// Lookup the outgoing peer to supplement the information on the
+			// resolved event. Here we handle a channel lookup error
+			// differently to the incoming channel, because it's possible
+			// we were forwarded a HTLC with a bogus outgoing channel. If
+			// this is the case, LND would have failed the HTLC back even if
+			// we let it through. We catch and log that error, rather than
+			// exiting like we do with incoming channels (where we reasonably
+			// expect to find the channel). We still enforce channel lookup
+			// for successful HTLCs, because then we know that the channel
+			// does exist and should be found.
+			var outgoingPeer *route.Vertex
 			chanInfo, err = p.getChanInfo(
 				resolvedEvent.outgoingCircuitKey.channel,
 			)
-			if err != nil {
+			switch {
+			case errors.Is(err, errChannelNotFound) && !resolvedEvent.settled:
+				log.Debugf("Channel not found for failed htlc: %v",
+					resolvedEvent.outgoingCircuitKey.channel)
+
+			case err != nil:
 				return err
+
+			default:
+				outgoingPeer = &chanInfo.peer
 			}
 
 			if err := ctrl.resolved(ctx, peerResolvedEvent{
 				resolvedEvent: resolvedEvent,
-				outgoingPeer:  chanInfo.peer,
+				outgoingPeer:  outgoingPeer,
 			}); err != nil {
 				return err
 			}

process_test.go

@@ -408,6 +408,115 @@ func TestChannelNotFound(t *testing.T) {
 	}
 }
 
+// TestOutgoingChannelNotFound tests the case where the outgoing channel for a htlc is
+// not found in two cases:
+// 1. The HTLC was settled: the channel must exist, so we fail if it's not found
+// 2. The HTLC was failed: the outgoing channel could be bogus, so we handle the error
+func TestOutgoingChannelNotFound(t *testing.T) {
+	tests := []struct {
+		name          string
+		settled       bool
+		outgoingFound bool
+		err           error
+	}{
+		{
+			name:          "outgoing found, settled",
+			settled:       true,
+			outgoingFound: true,
+		},
+		{
+			name:          "outgoing found, not settled",
+			settled:       false,
+			outgoingFound: true,
+		},
+		{
+			name:          "outgoing not found, settled",
+			settled:       true,
+			outgoingFound: false,
+			err:           errChannelNotFound,
+		},
+		{
+			name:          "outgoing not found, not settled",
+			settled:       false,
+			outgoingFound: false,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			testLookupOutgoingChannel(
+				t, test.settled, test.outgoingFound, test.err,
+			)
+		})
+	}
+}
+
+func testLookupOutgoingChannel(t *testing.T, settled, outgoingFound bool,
+	exitErr error) {
+
+	client := newLndclientMock(testChannels, nil)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	db, cleanup := setupTestDb(t, defaultFwdHistoryLimit)
+	defer cleanup()
+
+	log := zaptest.NewLogger(t).Sugar()
+
+	cfg := &Limits{}
+
+	p := NewProcess(client, log, cfg, db)
+
+	resolved := make(chan struct{})
+	p.resolvedCallback = func() {
+		close(resolved)
+	}
+
+	exit := make(chan error)
+	go func() {
+		exit <- p.Run(ctx)
+	}()
+
+	// Send a htlc with a known incoming channel.
+	key := circuitKey{
+		channel: 2,
+		htlc:    5,
+	}
+	client.htlcInterceptorRequests <- &interceptedEvent{
+		circuitKey: key,
+	}
+
+	resp := <-client.htlcInterceptorResponses
+	require.True(t, resp.resume)
+
+	// Set the outgoing channel based on whether we want it to be found by our
+	// lookup or not.
+	outgoingKey := outgoingKey
+	if !outgoingFound {
+		outgoingKey.channel = 9999
+	}
+
+	htlcEvent := &resolvedEvent{
+		incomingCircuitKey: key,
+		outgoingCircuitKey: outgoingKey,
+		settled:            settled,
+	}
+
+	client.htlcEvents <- htlcEvent
+
+	// If expected, assert that we exit with an error, otherwise ensure that the htlc
+	// is settled and we exit cleanly.
+	if exitErr != nil {
+		require.ErrorIs(t, <-exit, exitErr)
+	} else {
+		<-resolved
+
+		cancel()
+		require.ErrorIs(t, <-exit, context.Canceled)
+	}
+}
+
 // TestClosedChannelHtlc tests that we can handle intercepted htlcs that are associated
 // with closed channels.
 func TestClosedChannelHtlc(t *testing.T) {