From d2e395d5f2eb8db93d466c1f62b718fe063a29cf Mon Sep 17 00:00:00 2001 From: carla Date: Fri, 20 Dec 2019 12:25:08 +0200 Subject: [PATCH] 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. --- .../htlc_incoming_contest_resolver.go | 25 +++++++++++-------- contractcourt/htlc_incoming_resolver_test.go | 21 ++++++++++++---- htlcswitch/link.go | 16 +----------- invoices/invoiceregistry.go | 14 +++++++++-- invoices/invoiceregistry_test.go | 10 +++++--- invoices/update.go | 4 +++ 6 files changed, 54 insertions(+), 36 deletions(-) diff --git a/contractcourt/htlc_incoming_contest_resolver.go b/contractcourt/htlc_incoming_contest_resolver.go index 9d026cd07..98e52500a 100644 --- a/contractcourt/htlc_incoming_contest_resolver.go +++ b/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) diff --git a/contractcourt/htlc_incoming_resolver_test.go b/contractcourt/htlc_incoming_resolver_test.go index cce6343bc..773b22c00 100644 --- a/contractcourt/htlc_incoming_resolver_test.go +++ b/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() diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 596b13eef..005b2e3b0 100644 --- a/htlcswitch/link.go +++ b/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 } diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index 0ee0364c3..0b3163242 100644 --- a/invoices/invoiceregistry.go +++ b/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 { diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 27a63c255..2750e942d 100644 --- a/invoices/invoiceregistry_test.go +++ b/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) } } diff --git a/invoices/update.go b/invoices/update.go index fcf7a8e1d..2d512dbe3 100644 --- a/invoices/update.go +++ b/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.