Merge pull request #5336 from cfromknecht/reuse-amp-invoice

lnrpc: allow AMP pay-addr override + bump invoice timeouts

lnrpc/invoicesrpc/addinvoice.go

@@ -23,6 +23,16 @@ import (
 	"github.com/lightningnetwork/lnd/zpay32"
 )
 
+const (
+	// DefaultInvoiceExpiry is the default invoice expiry for new MPP
+	// invoices.
+	DefaultInvoiceExpiry = 24 * time.Hour
+
+	// DefaultAMPInvoiceExpiry is the default invoice expiry for new AMP
+	// invoices.
+	DefaultAMPInvoiceExpiry = 30 * 24 * time.Hour
+)
+
 // AddInvoiceConfig contains dependencies for invoice creation.
 type AddInvoiceConfig struct {
 	// AddInvoice is called to add the invoice to the registry.
@@ -270,10 +280,12 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig,
 		options = append(options, zpay32.FallbackAddr(addr))
 	}
 
+	switch {
+
 	// If expiry is set, specify it. If it is not provided, no expiry time
 	// will be explicitly added to this payment request, which will imply
 	// the default 3600 seconds.
-	if invoice.Expiry > 0 {
+	case invoice.Expiry > 0:
 
 		// We'll ensure that the specified expiry is restricted to sane
 		// number of seconds. As a result, we'll reject an invoice with
@@ -289,6 +301,15 @@ func AddInvoice(ctx context.Context, cfg *AddInvoiceConfig,
 
 		expiry := time.Duration(invoice.Expiry) * time.Second
 		options = append(options, zpay32.Expiry(expiry))
+
+	// If no custom expiry is provided, use the default MPP expiry.
+	case !invoice.Amp:
+		options = append(options, zpay32.Expiry(DefaultInvoiceExpiry))
+
+	// Otherwise, use the default AMP expiry.
+	default:
+		options = append(options, zpay32.Expiry(DefaultAMPInvoiceExpiry))
+
 	}
 
 	// If the description hash is set, then we add it do the list of options.

lnrpc/routerrpc/router_backend.go

@@ -710,6 +710,7 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 			payIntent.MaxParts = 1
 		}
 
+		payAddr := payReq.PaymentAddr
 		if payReq.Features.HasFeature(lnwire.AMPOptional) {
 			// Generate random SetID and root share.
 			var setID [32]byte
@@ -730,6 +731,17 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 			if err != nil {
 				return nil, err
 			}
+
+			// For AMP invoices, we'll allow users to override the
+			// included payment addr to allow the invoice to be
+			// pseudo-reusable, e.g. the invoice parameters are
+			// reused (amt, cltv, hop hints, etc) even though the
+			// payments will share different payment hashes.
+			if len(rpcPayReq.PaymentAddr) > 0 {
+				var addr [32]byte
+				copy(addr[:], rpcPayReq.PaymentAddr)
+				payAddr = &addr
+			}
 		} else {
 			err = payIntent.SetPaymentHash(*payReq.PaymentHash)
 			if err != nil {
@@ -745,7 +757,7 @@ func (r *RouterBackend) extractIntentFromSendRequest(
 			payIntent.RouteHints, payReq.RouteHints...,
 		)
 		payIntent.DestFeatures = payReq.Features
-		payIntent.PaymentAddr = payReq.PaymentAddr
+		payIntent.PaymentAddr = payAddr
 		payIntent.PaymentRequest = []byte(rpcPayReq.PaymentRequest)
 	} else {
 		// Otherwise, If the payment request field was not specified

lntest/itest/lnd_amp_test.go

@@ -20,6 +20,19 @@ import (
 // testSendPaymentAMPInvoice tests that we can send an AMP payment to a
 // specified AMP invoice using SendPaymentV2.
 func testSendPaymentAMPInvoice(net *lntest.NetworkHarness, t *harnessTest) {
+	t.t.Run("native payaddr", func(t *testing.T) {
+		tt := newHarnessTest(t, net)
+		testSendPaymentAMPInvoiceCase(net, tt, false)
+	})
+	t.t.Run("external payaddr", func(t *testing.T) {
+		tt := newHarnessTest(t, net)
+		testSendPaymentAMPInvoiceCase(net, tt, true)
+	})
+}
+
+func testSendPaymentAMPInvoiceCase(net *lntest.NetworkHarness, t *harnessTest,
+	useExternalPayAddr bool) {
+
 	ctxb := context.Background()
 
 	ctx := newMppTestContext(t, net)
@@ -88,11 +101,28 @@ func testSendPaymentAMPInvoice(net *lntest.NetworkHarness, t *harnessTest) {
 		t.Fatalf("dave policy update: %v", err)
 	}
 
+	// Generate an external payment address when attempting to pseudo-reuse
+	// an AMP invoice. When using an external payment address, we'll also
+	// expect an extra invoice to appear in the ListInvoices response, since
+	// a new invoice will be JIT inserted under a different payment address
+	// than the one in the invoice.
+	var (
+		expNumInvoices  = 1
+		externalPayAddr []byte
+	)
+	if useExternalPayAddr {
+		expNumInvoices = 2
+		externalPayAddr = make([]byte, 32)
+		_, err = rand.Read(externalPayAddr)
+		require.NoError(t.t, err)
+	}
+
 	ctxt, _ := context.WithTimeout(context.Background(), 4*defaultTimeout)
 	payment := sendAndAssertSuccess(
 		ctxt, t, ctx.alice,
 		&routerrpc.SendPaymentRequest{
 			PaymentRequest: addInvoiceResp.PaymentRequest,
+			PaymentAddr:    externalPayAddr,
 			TimeoutSeconds: 60,
 			FeeLimitMsat:   noFeeLimitMsat,
 		},
@@ -118,6 +148,14 @@ func testSendPaymentAMPInvoice(net *lntest.NetworkHarness, t *harnessTest) {
 			minExpectedShards, succeeded)
 	}
 
+	// When an external payment address is supplied, we'll get an extra
+	// notification for the JIT inserted invoice, since it differs from the
+	// original.
+	if useExternalPayAddr {
+		_, err = bobInvoiceSubscription.Recv()
+		require.NoError(t.t, err)
+	}
+
 	// There should now be a settle event for the invoice.
 	rpcInvoice, err = bobInvoiceSubscription.Recv()
 	require.NoError(t.t, err)
@@ -128,8 +166,8 @@ func testSendPaymentAMPInvoice(net *lntest.NetworkHarness, t *harnessTest) {
 		ctxb, &lnrpc.ListInvoiceRequest{},
 	)
 	require.NoError(t.t, err)
-	require.Equal(t.t, 1, len(invoiceResp.Invoices))
-	assertInvoiceEqual(t.t, rpcInvoice, invoiceResp.Invoices[0])
+	require.Equal(t.t, expNumInvoices, len(invoiceResp.Invoices))
+	assertInvoiceEqual(t.t, rpcInvoice, invoiceResp.Invoices[expNumInvoices-1])
 
 	// Assert that the invoice is settled for the total payment amount and
 	// has the correct payment address.
@@ -161,6 +199,7 @@ func testSendPaymentAMPInvoice(net *lntest.NetworkHarness, t *harnessTest) {
 		validPreimage := childPreimage.Matches(childHash)
 		require.True(t.t, validPreimage)
 	}
+
 }
 
 // testSendPaymentAMP tests that we can send an AMP payment to a specified

lntest/itest/lnd_mpp_test.go

@@ -344,6 +344,8 @@ func (c *mppTestContext) closeChannels() {
 }
 
 func (c *mppTestContext) shutdownNodes() {
+	shutdownAndAssert(c.net, c.t, c.alice)
+	shutdownAndAssert(c.net, c.t, c.bob)
 	shutdownAndAssert(c.net, c.t, c.carol)
 	shutdownAndAssert(c.net, c.t, c.dave)
 	shutdownAndAssert(c.net, c.t, c.eve)