diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index cc76d5aef..97e6085ef 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -9,6 +9,7 @@ import ( "time" "github.com/lightningnetwork/lnd/clock" + "github.com/lightningnetwork/lnd/fn/v2" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/queue" @@ -1086,17 +1087,36 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked( updateSubscribers bool ) callback := func(inv *Invoice) (*InvoiceUpdateDesc, error) { - updateDesc, res, err := updateInvoice(ctx, inv) + // First check if this is a replayed htlc and resolve it + // according to its current state. We cannot decide differently + // once the HTLC has already been processed before. + isReplayed, res, err := resolveReplayedHtlc(ctx, inv) if err != nil { return nil, err } + if isReplayed { + resolution = res + return nil, nil + } - // Only send an update if the invoice state was changed. - updateSubscribers = updateDesc != nil && - updateDesc.State != nil - - // Assign resolution to outer scope variable. + // In case the HTLC interceptor cancels the HTLC set, we do NOT + // cancel the invoice however we cancel the complete HTLC set. if cancelSet { + // If the invoice is not open, something is wrong, we + // fail just the HTLC with the specific error. + if inv.State != ContractOpen { + log.Errorf("Invoice state (%v) is not OPEN, "+ + "cancelling HTLC set not allowed by "+ + "external source", inv.State) + + resolution = NewFailResolution( + ctx.circuitKey, ctx.currentHeight, + ResultInvoiceNotOpen, + ) + + return nil, nil + } + // If a cancel signal was set for the htlc set, we set // the resolution as a failure with an underpayment // indication. Something was wrong with this htlc, so @@ -1105,10 +1125,43 @@ func (i *InvoiceRegistry) notifyExitHopHtlcLocked( ctx.circuitKey, ctx.currentHeight, ResultAmountTooLow, ) - } else { - resolution = res + + // We cancel all HTLCs which are in the accepted state. + // + // NOTE: The current HTLC is not included because it + // was never accepted in the first place. + htlcs := inv.HTLCSet(ctx.setID(), HtlcStateAccepted) + htlcKeys := fn.KeySet[CircuitKey](htlcs) + + // The external source did cancel the htlc set, so we + // cancel all HTLCs in the set. We however keep the + // invoice in the open state. + // + // NOTE: The invoice event loop will still call the + // `cancelSingleHTLC` method for MPP payments, however + // because the HTLCs are already cancled back it will be + // a NOOP. + update := &InvoiceUpdateDesc{ + UpdateType: CancelHTLCsUpdate, + CancelHtlcs: htlcKeys, + SetID: setID, + } + + return update, nil } + updateDesc, res, err := updateInvoice(ctx, inv) + if err != nil { + return nil, err + } + + // Set resolution in outer scope only after successful update. + resolution = res + + // Only send an update if the invoice state was changed. + updateSubscribers = updateDesc != nil && + updateDesc.State != nil + return updateDesc, nil } diff --git a/invoices/invoices.go b/invoices/invoices.go index 32164cbe1..d6d59b4a0 100644 --- a/invoices/invoices.go +++ b/invoices/invoices.go @@ -765,6 +765,9 @@ type InvoiceStateUpdateDesc struct { // InvoiceUpdateCallback is a callback used in the db transaction to update the // invoice. +// TODO(ziggie): Add the option of additional return values to the callback +// for example the resolution which is currently assigned via an outer scope +// variable. type InvoiceUpdateCallback = func(invoice *Invoice) (*InvoiceUpdateDesc, error) // ValidateInvoice assures the invoice passes the checks for all the relevant diff --git a/invoices/update.go b/invoices/update.go index 3137bbbfa..6f7a34f4c 100644 --- a/invoices/update.go +++ b/invoices/update.go @@ -109,41 +109,52 @@ func (i invoiceUpdateCtx) acceptRes( return newAcceptResolution(i.circuitKey, outcome) } +// resolveReplayedHtlc returns the HTLC resolution for a replayed HTLC. The +// returned boolean indicates whether the HTLC was replayed or not. +func resolveReplayedHtlc(ctx *invoiceUpdateCtx, inv *Invoice) (bool, + HtlcResolution, error) { + + // Don't update the invoice when this is a replayed htlc. + htlc, replayedHTLC := inv.Htlcs[ctx.circuitKey] + if !replayedHTLC { + return false, nil, nil + } + + switch htlc.State { + case HtlcStateCanceled: + return true, ctx.failRes(ResultReplayToCanceled), nil + + case HtlcStateAccepted: + return true, ctx.acceptRes(resultReplayToAccepted), nil + + case HtlcStateSettled: + pre := inv.Terms.PaymentPreimage + + // Terms.PaymentPreimage will be nil for AMP invoices. + // Set it to the HTLCs AMP Preimage instead. + if pre == nil { + pre = htlc.AMP.Preimage + } + + return true, ctx.settleRes( + *pre, + ResultReplayToSettled, + ), nil + + default: + return true, nil, errors.New("unknown htlc state") + } +} + // updateInvoice is a callback for DB.UpdateInvoice that contains the invoice // settlement logic. It returns a HTLC resolution that indicates what the // outcome of the update was. +// +// NOTE: Make sure replayed HTLCs are always considered before calling this +// function. func updateInvoice(ctx *invoiceUpdateCtx, inv *Invoice) ( *InvoiceUpdateDesc, HtlcResolution, error) { - // Don't update the invoice when this is a replayed htlc. - htlc, ok := inv.Htlcs[ctx.circuitKey] - if ok { - switch htlc.State { - case HtlcStateCanceled: - return nil, ctx.failRes(ResultReplayToCanceled), nil - - case HtlcStateAccepted: - return nil, ctx.acceptRes(resultReplayToAccepted), nil - - case HtlcStateSettled: - pre := inv.Terms.PaymentPreimage - - // Terms.PaymentPreimage will be nil for AMP invoices. - // Set it to the HTLCs AMP Preimage instead. - if pre == nil { - pre = htlc.AMP.Preimage - } - - return nil, ctx.settleRes( - *pre, - ResultReplayToSettled, - ), nil - - default: - return nil, nil, errors.New("unknown htlc state") - } - } - // If no MPP payload was provided, then we expect this to be a keysend, // or a payment to an invoice created before we started to require the // MPP payload. diff --git a/itest/lnd_invoice_acceptor_test.go b/itest/lnd_invoice_acceptor_test.go index e94d81551..9195b102e 100644 --- a/itest/lnd_invoice_acceptor_test.go +++ b/itest/lnd_invoice_acceptor_test.go @@ -116,6 +116,7 @@ func testInvoiceHtlcModifierBasic(ht *lntest.HarnessTest) { &invoicesrpc.HtlcModifyResponse{ CircuitKey: modifierRequest.ExitHtlcCircuitKey, AmtPaid: &amtPaid, + CancelSet: tc.cancelSet, }, ) require.NoError(ht, err, "failed to send request") @@ -128,7 +129,9 @@ func testInvoiceHtlcModifierBasic(ht *lntest.HarnessTest) { require.Fail(ht, "timeout waiting for payment send") } - ht.Log("Ensure invoice status is settled") + ht.Logf("Ensure invoice status is expected state %v", + tc.finalInvoiceState) + require.Eventually(ht, func() bool { updatedInvoice := carol.RPC.LookupInvoice( tc.invoice.RHash, @@ -141,6 +144,13 @@ func testInvoiceHtlcModifierBasic(ht *lntest.HarnessTest) { tc.invoice.RHash, ) + // If the HTLC modifier canceled the incoming HTLC set, we don't + // expect any HTLCs in the invoice. + if tc.cancelSet { + require.Len(ht, updatedInvoice.Htlcs, 0) + return + } + require.Len(ht, updatedInvoice.Htlcs, 1) require.Equal( ht, lntest.CustomRecordsWithUnendorsed( @@ -231,6 +241,10 @@ type acceptorTestCase struct { // invoice is the invoice that will be paid. invoice *lnrpc.Invoice + + // cancelSet is a boolean which indicates whether the HTLC modifier + // canceled the incoming HTLC set. + cancelSet bool } // acceptorTestScenario is a helper struct to hold the test context and provides @@ -282,6 +296,12 @@ func (c *acceptorTestScenario) prepareTestCases() []*acceptorTestCase { lnwire.MinCustomRecordsTlvType: {1, 2, 3}, }, }, + { + invoiceAmountMsat: 9000, + sendAmountMsat: 1000, + finalInvoiceState: lnrpc.Invoice_OPEN, + cancelSet: true, + }, } for _, t := range cases {