From 24e3234dfae59411c6c1f81549aecae28a154dfe Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Thu, 9 Apr 2020 10:42:10 +0200 Subject: [PATCH] invoices: optionally hold and auto-cancel keysend payments Adds a new configuration flag to lnd that will keep keysend payments in the accepted state. An application can then inspect the payment parameters and decide whether to settle or cancel. The on-the-fly inserted keysend invoices get a configurable expiry time. This is a safeguard in case the application that should decide on the keysend payments isn't active. --- config.go | 2 + invoices/invoice_expiry_watcher.go | 14 +++- invoices/invoice_expiry_watcher_test.go | 6 +- invoices/invoiceregistry.go | 15 ++-- invoices/invoiceregistry_test.go | 100 ++++++++++++++++++++++++ server.go | 1 + 6 files changed, 128 insertions(+), 10 deletions(-) diff --git a/config.go b/config.go index b23bb5e4..fff67e47 100644 --- a/config.go +++ b/config.go @@ -233,6 +233,8 @@ type Config struct { AcceptKeySend bool `long:"accept-keysend" description:"If true, spontaneous payments through keysend will be accepted. [experimental]"` + KeysendHoldTime time.Duration `long:"keysend-hold-time" description:"If non-zero, keysend payments are accepted but not immediately settled. If the payment isn't settled manually after the specified time, it is canceled automatically. [experimental]"` + Routing *routing.Conf `group:"routing" namespace:"routing"` Workers *lncfg.Workers `group:"workers" namespace:"workers"` diff --git a/invoices/invoice_expiry_watcher.go b/invoices/invoice_expiry_watcher.go index 08c57f39..f0db08d1 100644 --- a/invoices/invoice_expiry_watcher.go +++ b/invoices/invoice_expiry_watcher.go @@ -17,6 +17,7 @@ import ( type invoiceExpiry struct { PaymentHash lntypes.Hash Expiry time.Time + Keysend bool } // Less implements PriorityQueueItem.Less such that the top item in the @@ -41,7 +42,7 @@ type InvoiceExpiryWatcher struct { clock clock.Clock // cancelInvoice is a template method that cancels an expired invoice. - cancelInvoice func(lntypes.Hash) error + cancelInvoice func(lntypes.Hash, bool) error // expiryQueue holds invoiceExpiry items and is used to find the next // invoice to expire. @@ -71,7 +72,7 @@ func NewInvoiceExpiryWatcher(clock clock.Clock) *InvoiceExpiryWatcher { // expects a cancellation function passed that will be use to cancel expired // invoices by their payment hash. func (ew *InvoiceExpiryWatcher) Start( - cancelInvoice func(lntypes.Hash) error) error { + cancelInvoice func(lntypes.Hash, bool) error) error { ew.Lock() defer ew.Unlock() @@ -121,6 +122,7 @@ func (ew *InvoiceExpiryWatcher) prepareInvoice( return &invoiceExpiry{ PaymentHash: paymentHash, Expiry: expiry, + Keysend: len(invoice.PaymentRequest) == 0, } } @@ -190,7 +192,13 @@ func (ew *InvoiceExpiryWatcher) cancelNextExpiredInvoice() { return } - err := ew.cancelInvoice(top.PaymentHash) + // Don't force-cancel already accepted invoices. An exception to + // this are auto-generated keysend invoices. Because those move + // to the Accepted state directly after being opened, the expiry + // field would never be used. Enabling cancellation for accepted + // keysend invoices creates a safety mechanism that can prevents + // channel force-closes. + err := ew.cancelInvoice(top.PaymentHash, top.Keysend) if err != nil && err != channeldb.ErrInvoiceAlreadySettled && err != channeldb.ErrInvoiceAlreadyCanceled { diff --git a/invoices/invoice_expiry_watcher_test.go b/invoices/invoice_expiry_watcher_test.go index e7e28b50..2aa0f87b 100644 --- a/invoices/invoice_expiry_watcher_test.go +++ b/invoices/invoice_expiry_watcher_test.go @@ -34,7 +34,9 @@ func newInvoiceExpiryWatcherTest(t *testing.T, now time.Time, test.wg.Add(numExpiredInvoices) - err := test.watcher.Start(func(paymentHash lntypes.Hash) error { + err := test.watcher.Start(func(paymentHash lntypes.Hash, + force bool) error { + test.canceledInvoices = append(test.canceledInvoices, paymentHash) test.wg.Done() return nil @@ -81,7 +83,7 @@ func (t *invoiceExpiryWatcherTest) checkExpectations() { // Tests that InvoiceExpiryWatcher can be started and stopped. func TestInvoiceExpiryWatcherStartStop(t *testing.T) { watcher := NewInvoiceExpiryWatcher(clock.NewTestClock(testTime)) - cancel := func(lntypes.Hash) error { + cancel := func(lntypes.Hash, bool) error { t.Fatalf("unexpected call") return nil } diff --git a/invoices/invoiceregistry.go b/invoices/invoiceregistry.go index e2af66a9..84d64617 100644 --- a/invoices/invoiceregistry.go +++ b/invoices/invoiceregistry.go @@ -56,6 +56,10 @@ type RegistryConfig struct { // AcceptKeySend indicates whether we want to accept spontaneous key // send payments. AcceptKeySend bool + + // KeysendHoldTime indicates for how long we want to accept and hold + // spontaneous keysend payments. + KeysendHoldTime time.Duration } // htlcReleaseEvent describes an htlc auto-release event. It is used to release @@ -165,10 +169,7 @@ func (i *InvoiceRegistry) populateExpiryWatcher() error { func (i *InvoiceRegistry) Start() error { // Start InvoiceExpiryWatcher and prepopulate it with existing active // invoices. - err := i.expiryWatcher.Start(func(paymentHash lntypes.Hash) error { - cancelIfAccepted := false - return i.cancelInvoiceImpl(paymentHash, cancelIfAccepted) - }) + err := i.expiryWatcher.Start(i.cancelInvoiceImpl) if err != nil { return err @@ -639,7 +640,6 @@ func (i *InvoiceRegistry) cancelSingleHtlc(invoiceRef channeldb.InvoiceRef, // processKeySend just-in-time inserts an invoice if this htlc is a keysend // htlc. func (i *InvoiceRegistry) processKeySend(ctx invoiceUpdateCtx) error { - // Retrieve keysend record if present. preimageSlice, ok := ctx.customRecords[record.KeySendType] if !ok { @@ -697,6 +697,11 @@ func (i *InvoiceRegistry) processKeySend(ctx invoiceUpdateCtx) error { }, } + if i.cfg.KeysendHoldTime != 0 { + invoice.HodlInvoice = true + invoice.Terms.Expiry = i.cfg.KeysendHoldTime + } + // Insert invoice into database. Ignore duplicates, because this // may be a replay. _, err = i.AddInvoice(invoice, ctx.hash) diff --git a/invoices/invoiceregistry_test.go b/invoices/invoiceregistry_test.go index a9131f8a..c77b38ed 100644 --- a/invoices/invoiceregistry_test.go +++ b/invoices/invoiceregistry_test.go @@ -10,6 +10,7 @@ import ( "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/record" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // TestSettleInvoice tests settling of an invoice and related notifications. @@ -780,6 +781,105 @@ func testKeySend(t *testing.T, keySendEnabled bool) { checkSubscription() } +// TestHoldKeysend tests receiving a spontaneous payment that is held. +func TestHoldKeysend(t *testing.T) { + t.Run("settle", func(t *testing.T) { + testHoldKeysend(t, false) + }) + t.Run("timeout", func(t *testing.T) { + testHoldKeysend(t, true) + }) +} + +// testHoldKeysend is the inner test function that tests hold-keysend. +func testHoldKeysend(t *testing.T, timeoutKeysend bool) { + defer timeout()() + + const holdDuration = time.Minute + + ctx := newTestContext(t) + defer ctx.cleanup() + + ctx.registry.cfg.AcceptKeySend = true + ctx.registry.cfg.KeysendHoldTime = holdDuration + + allSubscriptions, err := ctx.registry.SubscribeNotifications(0, 0) + assert.Nil(t, err) + defer allSubscriptions.Cancel() + + hodlChan := make(chan interface{}, 1) + + amt := lnwire.MilliSatoshi(1000) + expiry := uint32(testCurrentHeight + 20) + + // Create key for keysend. + preimage := lntypes.Preimage{1, 2, 3} + hash := preimage.Hash() + + // Try to settle invoice with a valid keysend htlc. + keysendPayload := &mockPayload{ + customRecords: map[uint64][]byte{ + record.KeySendType: preimage[:], + }, + } + + resolution, err := ctx.registry.NotifyExitHopHtlc( + hash, amt, expiry, + testCurrentHeight, getCircuitKey(10), hodlChan, keysendPayload, + ) + if err != nil { + t.Fatal(err) + } + + // No immediate resolution is expected. + require.Nil(t, resolution, "expected hold resolution") + + // We expect a new invoice notification to be sent out. + newInvoice := <-allSubscriptions.NewInvoices + if newInvoice.State != channeldb.ContractOpen { + t.Fatalf("expected state ContractOpen, but got %v", + newInvoice.State) + } + + // We expect no further invoice notifications yet (on the all invoices + // subscription). + select { + case <-allSubscriptions.NewInvoices: + t.Fatalf("no invoice update expected") + case <-time.After(100 * time.Millisecond): + } + + if timeoutKeysend { + // Advance the clock to just past the hold duration. + ctx.clock.SetTime(ctx.clock.Now().Add( + holdDuration + time.Millisecond), + ) + + // Expect the keysend payment to be failed. + res := <-hodlChan + failResolution, ok := res.(*HtlcFailResolution) + require.Truef( + t, ok, "expected fail resolution, got: %T", + resolution, + ) + require.Equal( + t, ResultCanceled, failResolution.Outcome, + "expected keysend payment to be failed", + ) + + return + } + + // Settle keysend payment manually. + require.Nil(t, ctx.registry.SettleHodlInvoice( + *newInvoice.Terms.PaymentPreimage, + )) + + // We expect a settled notification to be sent out. + settledInvoice := <-allSubscriptions.SettledInvoices + assert.Equal(t, settledInvoice.State, channeldb.ContractSettled) +} + // TestMppPayment tests settling of an invoice with multiple partial payments. // It covers the case where there is a mpp timeout before the whole invoice is // paid and the case where the invoice is settled in time. diff --git a/server.go b/server.go index 633bafe4..3cd357a0 100644 --- a/server.go +++ b/server.go @@ -417,6 +417,7 @@ func newServer(cfg *Config, listenAddrs []net.Addr, chanDB *channeldb.DB, HtlcHoldDuration: invoices.DefaultHtlcHoldDuration, Clock: clock.NewDefaultClock(), AcceptKeySend: cfg.AcceptKeySend, + KeysendHoldTime: cfg.KeysendHoldTime, } s := &server{