multi: replace errInvoiceNotFound with resolution result

This commit moves handling of invoice not found
errors into NotifyExitHopHtlc and exposes a
resolution result to the calling functions. The
intention of this change is to make calling
functions as naive of the invoice registry's
mechanics as possible.

When NotifyExitHopHtlc is called and an invoice
is not found, calling functions can take action
based on the HtlcResolution's InvoiceNotFound
outcome rather than having to add a special error
check on every call to handle the error.

contractcourt/htlc_incoming_contest_resolver.go

@@ -201,23 +201,26 @@ func (h *htlcIncomingContestResolver) Resolve() (ContractResolver, error) {
 		HtlcID: h.htlc.HtlcIndex,
 	}
 
-	event, err := h.Registry.NotifyExitHopHtlc(
+	resolution, err := h.Registry.NotifyExitHopHtlc(
 		h.htlc.RHash, h.htlc.Amt, h.htlcExpiry, currentHeight,
 		circuitKey, hodlChan, payload,
 	)
-	switch err {
-	case channeldb.ErrInvoiceNotFound:
-	case nil:
-		defer h.Registry.HodlUnsubscribeAll(hodlChan)
-
-		// Resolve the htlc directly if possible.
-		if event != nil {
-			return processHtlcResolution(*event)
-		}
-	default:
+	if err != nil {
 		return nil, err
 	}
 
+	defer h.Registry.HodlUnsubscribeAll(hodlChan)
+
+	// If the resolution is non-nil (indicating that a settle or cancel has
+	// occurred), and the invoice is known to the registry (indicating that
+	// the htlc is paying one of our invoices and is not a forward), try to
+	// resolve it directly.
+	if resolution != nil &&
+		resolution.Outcome != invoices.ResultInvoiceNotFound {
+
+		return processHtlcResolution(*resolution)
+	}
+
 	// With the epochs and preimage subscriptions initialized, we'll query
 	// to see if we already know the preimage.
 	preimage, ok := h.PreimageDB.LookupPreimage(h.htlc.RHash)

contractcourt/htlc_incoming_resolver_test.go

@@ -35,7 +35,10 @@ func TestHtlcIncomingResolverFwdPreimageKnown(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyErr = channeldb.ErrInvoiceNotFound
+	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+		testResCircuitKey, testHtlcExpiry,
+		invoices.ResultInvoiceNotFound,
+	)
 	ctx.witnessBeacon.lookupPreimage[testResHash] = testResPreimage
 	ctx.resolve()
 	ctx.waitForResult(true)
@@ -49,7 +52,10 @@ func TestHtlcIncomingResolverFwdContestedSuccess(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyErr = channeldb.ErrInvoiceNotFound
+	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+		testResCircuitKey, testHtlcExpiry,
+		invoices.ResultInvoiceNotFound,
+	)
 	ctx.resolve()
 
 	// Simulate a new block coming in. HTLC is not yet expired.
@@ -66,7 +72,10 @@ func TestHtlcIncomingResolverFwdContestedTimeout(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-	ctx.registry.notifyErr = channeldb.ErrInvoiceNotFound
+	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+		testResCircuitKey, testHtlcExpiry,
+		invoices.ResultInvoiceNotFound,
+	)
 	ctx.resolve()
 
 	// Simulate a new block coming in. HTLC expires.
@@ -82,8 +91,10 @@ func TestHtlcIncomingResolverFwdTimeout(t *testing.T) {
 	defer timeout(t)()
 
 	ctx := newIncomingResolverTestContext(t)
-
-	ctx.registry.notifyErr = channeldb.ErrInvoiceNotFound
+	ctx.registry.notifyResolution = invoices.NewFailureResolution(
+		testResCircuitKey, testHtlcExpiry,
+		invoices.ResultInvoiceNotFound,
+	)
 	ctx.witnessBeacon.lookupPreimage[testResHash] = testResPreimage
 	ctx.resolver.htlcExpiry = 90
 	ctx.resolve()

htlcswitch/link.go

@@ -2823,21 +2823,7 @@ func (l *channelLink) processExitHop(pd *lnwallet.PaymentDescriptor,
 		invoiceHash, pd.Amount, pd.Timeout, int32(heightNow),
 		circuitKey, l.hodlQueue.ChanIn(), payload,
 	)
-
-	switch err {
-
-	// Cancel htlc if we don't have an invoice for it.
-	case channeldb.ErrInvoiceNotFound:
-		failure := lnwire.NewFailIncorrectDetails(pd.Amount, heightNow)
-		l.sendHTLCError(pd.HtlcIndex, failure, obfuscator, pd.SourceRef)
-
-		return nil
-
-	// No error.
-	case nil:
-
-	// Pass error to caller.
-	default:
+	if err != nil {
 		return err
 	}
 

invoices/invoiceregistry.go

@@ -758,11 +758,21 @@ func (i *InvoiceRegistry) NotifyExitHopHtlc(rHash lntypes.Hash,
 			return updateDesc, nil
 		},
 	)
-	if err != nil {
-		debugLog(err.Error())
+	switch err {
+	case channeldb.ErrInvoiceNotFound:
+		// If the invoice was not found, return a failure resolution
+		// with an invoice not found result.
+		return NewFailureResolution(
+			circuitKey, currentHeight, ResultInvoiceNotFound,
+		), nil
 
+	case nil:
+
+	default:
+		debugLog(err.Error())
 		return nil, err
 	}
+
 	debugLog(result.String())
 
 	if updateSubscribers {

invoices/invoiceregistry_test.go

@@ -585,12 +585,16 @@ func TestUnknownInvoice(t *testing.T) {
 	// succeed.
 	hodlChan := make(chan interface{})
 	amt := lnwire.MilliSatoshi(100000)
-	_, err := ctx.registry.NotifyExitHopHtlc(
+	result, err := ctx.registry.NotifyExitHopHtlc(
 		testInvoicePaymentHash, amt, testHtlcExpiry, testCurrentHeight,
 		getCircuitKey(0), hodlChan, testPayload,
 	)
-	if err != channeldb.ErrInvoiceNotFound {
-		t.Fatal("expected invoice not found error")
+	if err != nil {
+		t.Fatal("unexpected error")
+	}
+	if result.Outcome != ResultInvoiceNotFound {
+		t.Fatalf("expected ResultInvoiceNotFound, got: %v",
+			result.Outcome)
 	}
 }
 

invoices/update.go

@@ -81,6 +81,10 @@ const (
 
 	// ResultHtlcSetOverpayment is returned when a mpp set is overpaid.
 	ResultHtlcSetOverpayment
+
+	// ResultInvoiceNotFound is returned when an attempt is made to pay an
+	// invoice that is unknown to us.
+	ResultInvoiceNotFound
 )
 
 // String returns a human-readable representation of the invoice update result.