From 2aa680ede2ac2552ec466a00fadc28e4df87a3ab Mon Sep 17 00:00:00 2001 From: Andras Banki-Horvath Date: Tue, 4 Aug 2020 18:28:15 +0200 Subject: [PATCH] invoices: optionally garbage collect invoices on the fly This commit extends invoice garbage collection to also remove invoices which are canceled when LND is already up and running. When the option GcCanceledInvoicesOnTheFly is false (default) then invoices are kept and the behavior is unchanged. --- config.go | 2 ++ invoices/invoiceregistry.go | 30 ++++++++++++++++++ invoices/invoiceregistry_test.go | 53 +++++++++++++++++++++++++++----- server.go | 1 + 4 files changed, 78 insertions(+), 8 deletions(-) diff --git a/config.go b/config.go index 89f3fa89f..511e012a1 100644 --- a/config.go +++ b/config.go @@ -244,6 +244,8 @@ type Config struct { GcCanceledInvoicesOnStartup bool `long:"gc-canceled-invoices-on-startup" description:"If true, we'll attempt to garbage collect canceled invoices upon start."` + GcCanceledInvoicesOnTheFly bool `long:"gc-canceled-invoices-on-the-fly" description:"If true, we'll delete newly canceled invoices on the fly."` + Routing *routing.Conf `group:"routing" namespace:"routing"` Workers *lncfg.Workers `group:"workers" namespace:"workers"` diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index bc40168a8..c827f1445 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -61,6 +61,10 @@ type RegistryConfig struct { // all canceled invoices upon start. GcCanceledInvoicesOnStartup bool + // GcCanceledInvoicesOnTheFly if set, we'll garbage collect all newly + // canceled invoices on the fly. + GcCanceledInvoicesOnTheFly bool + // KeysendHoldTime indicates for how long we want to accept and hold // spontaneous keysend payments. KeysendHoldTime time.Duration @@ -1124,6 +1128,32 @@ func (i *InvoiceRegistry) cancelInvoiceImpl(payHash lntypes.Hash, } i.notifyClients(payHash, invoice, channeldb.ContractCanceled) + // Attempt to also delete the invoice if requested through the registry + // config. + if i.cfg.GcCanceledInvoicesOnTheFly { + // Assemble the delete reference and attempt to delete through + // the invocice from the DB. + deleteRef := channeldb.InvoiceDeleteRef{ + PayHash: payHash, + AddIndex: invoice.AddIndex, + SettleIndex: invoice.SettleIndex, + } + if invoice.Terms.PaymentAddr != channeldb.BlankPayAddr { + deleteRef.PayAddr = &invoice.Terms.PaymentAddr + } + + err = i.cdb.DeleteInvoice( + []channeldb.InvoiceDeleteRef{deleteRef}, + ) + // If by any chance deletion failed, then log it instead of + // returning the error, as the invoice itsels has already been + // canceled. + if err != nil { + log.Warnf("Invoice%v could not be deleted: %v", + ref, err) + } + } + return nil } diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index 6e5ca2212..cb916aeab 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -220,11 +220,14 @@ func TestSettleInvoice(t *testing.T) { } } -// TestCancelInvoice tests cancelation of an invoice and related notifications. -func TestCancelInvoice(t *testing.T) { +func testCancelInvoice(t *testing.T, gc bool) { ctx := newTestContext(t) defer ctx.cleanup() + // If set to true, then also delete the invoice from the DB after + // cancellation. + ctx.registry.cfg.GcCanceledInvoicesOnTheFly = gc + allSubscriptions, err := ctx.registry.SubscribeNotifications(0, 0) assert.Nil(t, err) defer allSubscriptions.Cancel() @@ -299,13 +302,26 @@ func TestCancelInvoice(t *testing.T) { t.Fatal("no update received") } + if gc { + // Check that the invoice has been deleted from the db. + _, err = ctx.cdb.LookupInvoice( + channeldb.InvoiceRefByHash(testInvoicePaymentHash), + ) + require.Error(t, err) + } + // We expect no cancel notification to be sent to all invoice // subscribers (backwards compatibility). - // Try to cancel again. + // Try to cancel again. Expect that we report ErrInvoiceNotFound if the + // invoice has been garbage collected (since the invoice has been + // deleted when it was canceled), and no error otherwise. err = ctx.registry.CancelInvoice(testInvoicePaymentHash) - if err != nil { - t.Fatal("expected cancelation of a canceled invoice to succeed") + + if gc { + require.Error(t, err, channeldb.ErrInvoiceNotFound) + } else { + require.NoError(t, err) } // Notify arrival of a new htlc paying to this invoice. This should @@ -327,12 +343,33 @@ func TestCancelInvoice(t *testing.T) { t.Fatalf("expected acceptHeight %v, but got %v", testCurrentHeight, failResolution.AcceptHeight) } - if failResolution.Outcome != ResultInvoiceAlreadyCanceled { - t.Fatalf("expected expiry too soon, got: %v", - failResolution.Outcome) + + // If the invoice has been deleted (or not present) then we expect the + // outcome to be ResultInvoiceNotFound instead of when the invoice is + // in our database in which case we expect ResultInvoiceAlreadyCanceled. + if gc { + require.Equal(t, failResolution.Outcome, ResultInvoiceNotFound) + } else { + require.Equal(t, + failResolution.Outcome, + ResultInvoiceAlreadyCanceled, + ) } } +// TestCancelInvoice tests cancelation of an invoice and related notifications. +func TestCancelInvoice(t *testing.T) { + // Test cancellation both with garbage collection (meaning that canceled + // invoice will be deleted) and without (meain it'll be kept). + t.Run("garbage collect", func(t *testing.T) { + testCancelInvoice(t, true) + }) + + t.Run("no garbage collect", func(t *testing.T) { + testCancelInvoice(t, false) + }) +} + // TestSettleHoldInvoice tests settling of a hold invoice and related // notifications. func TestSettleHoldInvoice(t *testing.T) { diff --git a/server.go b/server.go index d3de88ef9..0f2b59d0a 100644 --- a/server.go +++ b/server.go @@ -401,6 +401,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, chanDB *channeldb.DB, Clock: clock.NewDefaultClock(), AcceptKeySend: cfg.AcceptKeySend, GcCanceledInvoicesOnStartup: cfg.GcCanceledInvoicesOnStartup, + GcCanceledInvoicesOnTheFly: cfg.GcCanceledInvoicesOnTheFly, KeysendHoldTime: cfg.KeysendHoldTime, }