Merge pull request #4167 from joostjager/hold-keysend

multi: hold keysend payments

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

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 {
 

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
 	}

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)

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.

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{