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

@@ -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"`

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
 }
 

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, gc bool) {
-func TestCancelInvoice(t *testing.T) {
 	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",
+	// If the invoice has been deleted (or not present) then we expect the
-			failResolution.Outcome)
+	// 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) {

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,
 	}