From a765ba1f3d322b31c64e1f0fce9695f534e60fe0 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Wed, 26 May 2021 18:10:39 -0700 Subject: [PATCH 1/3] lntest/mpp_test: ensure ctx.alice and ctx.bob are shutdown Not doing so prevents consecutive subtests from starting properly. --- lntest/itest/lnd_mpp_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lntest/itest/lnd_mpp_test.go b/lntest/itest/lnd_mpp_test.go index 6e64400f3..5290fcae8 100644 --- a/lntest/itest/lnd_mpp_test.go +++ b/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) From d6f88cbe6f8a52412d2098054a1ffc741d31dc96 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Wed, 26 May 2021 18:10:47 -0700 Subject: [PATCH 2/3] lnrpc: allow payment addr override for AMP invoices This permits an AMP invoice to be "pseudo-reusable", where the invoice paramters can be used multiple times so long as a new payment address is supplied. This prevents additional round trips between payer and payee to obtain a new invoice, even though the payments/invoices won't be logically associated via the RPC interface like they would when the full reusable invoices are deployed. --- lnrpc/routerrpc/router_backend.go | 14 +++++++++- lntest/itest/lnd_amp_test.go | 43 +++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/lnrpc/routerrpc/router_backend.go b/lnrpc/routerrpc/router_backend.go index 78dec019b..1814c6358 100644 --- a/lnrpc/routerrpc/router_backend.go +++ b/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 diff --git a/lntest/itest/lnd_amp_test.go b/lntest/itest/lnd_amp_test.go index 1dc147aa4..6cf17920d 100644 --- a/lntest/itest/lnd_amp_test.go +++ b/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 From 4bb6cc9cee275e7f583039ead826d0fc6f6c4c33 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Wed, 26 May 2021 18:10:58 -0700 Subject: [PATCH 3/3] lnrpc/addinvoice: bump default LND invoice timeouts Increases the default MPP expiry from 1 hour to 1 day. For the new AMP invoices, we increase the interval to 1 month. The longer time frames for AMP invoices is used so that the invoice can be pseudo reused as implemented in the prior commit. The BOLT 11 default of 1 hour is still preserved whenever the field is missing in the payment request itself, but as of this commit the field will always be set by lnd. --- lnrpc/invoicesrpc/addinvoice.go | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/lnrpc/invoicesrpc/addinvoice.go b/lnrpc/invoicesrpc/addinvoice.go index 8eff43a14..2d782a67f 100644 --- a/lnrpc/invoicesrpc/addinvoice.go +++ b/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.