From 4732c09a2651c38e9b846ba3851db7fe9824438e Mon Sep 17 00:00:00 2001 From: ziggie Date: Sat, 3 Feb 2024 12:09:11 +0000 Subject: [PATCH] multi: Fix final hop payload size for AMP payments. --- itest/lnd_amp_test.go | 4 ++++ lnrpc/routerrpc/router_backend.go | 3 +++ record/amp.go | 10 ++++++++++ routing/pathfind.go | 14 ++++++++++++++ routing/pathfind_test.go | 11 +++++++++++ routing/payment_lifecycle.go | 2 +- routing/payment_session.go | 1 + 7 files changed, 44 insertions(+), 1 deletion(-) diff --git a/itest/lnd_amp_test.go b/itest/lnd_amp_test.go index ba17a30b6..61b6d194a 100644 --- a/itest/lnd_amp_test.go +++ b/itest/lnd_amp_test.go @@ -104,6 +104,10 @@ func testSendPaymentAMPInvoiceCase(ht *lntest.HarnessTest, // 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. + // + // NOTE: This will only work when the peer has spontaneous AMP payments + // enabled otherwise no invoice under a different payment_addr will be + // found. var ( expNumInvoices = 1 externalPayAddr []byte diff --git a/lnrpc/routerrpc/router_backend.go b/lnrpc/routerrpc/router_backend.go index 0422f2a9f..4d82c72e4 100644 --- a/lnrpc/routerrpc/router_backend.go +++ b/lnrpc/routerrpc/router_backend.go @@ -967,6 +967,9 @@ func (r *RouterBackend) extractIntentFromSendRequest( // pseudo-reusable, e.g. the invoice parameters are // reused (amt, cltv, hop hints, etc) even though the // payments will share different payment hashes. + // + // NOTE: This will only work when the peer has + // spontaneous AMP payments enabled. if len(rpcPayReq.PaymentAddr) > 0 { var addr [32]byte copy(addr[:], rpcPayReq.PaymentAddr) diff --git a/record/amp.go b/record/amp.go index eb431fec3..f63c7a141 100644 --- a/record/amp.go +++ b/record/amp.go @@ -19,6 +19,16 @@ type AMP struct { childIndex uint32 } +// MaxAmpPayLoadSize is an AMP Record which when serialized to a tlv record uses +// the maximum payload size. The `childIndex` is created randomly and is a +// 4 byte `varint` type so we make sure we use an index which will be encoded in +// 4 bytes. +var MaxAmpPayLoadSize = AMP{ + rootShare: [32]byte{}, + setID: [32]byte{}, + childIndex: 0x80000000, +} + // NewAMP generate a new AMP record with the given root_share, set_id, and // child_index. func NewAMP(rootShare, setID [32]byte, childIndex uint32) *AMP { diff --git a/routing/pathfind.go b/routing/pathfind.go index 9380c7c7d..53eb0dfb4 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -408,6 +408,11 @@ type RestrictParams struct { // invoices. PaymentAddr *[32]byte + // Amp signals to the pathfinder that this payment is an AMP payment + // and therefore it needs to account for additional AMP data in the + // final hop payload size calculation. + Amp *AMPOptions + // Metadata is additional data that is sent along with the payment to // the payee. Metadata []byte @@ -1134,12 +1139,21 @@ func lastHopPayloadSize(r *RestrictParams, finalHtlcExpiry int32, mpp = record.NewMPP(amount, *r.PaymentAddr) } + var amp *record.AMP + if r.Amp != nil { + // The AMP payload is not easy accessible at this point but we + // are only interested in the size of the payload so we just use + // the AMP record dummy. + amp = &record.MaxAmpPayLoadSize + } + finalHop := route.Hop{ AmtToForward: amount, OutgoingTimeLock: uint32(finalHtlcExpiry), CustomRecords: r.DestCustomRecords, LegacyPayload: legacy, MPP: mpp, + AMP: amp, Metadata: r.Metadata, } diff --git a/routing/pathfind_test.go b/routing/pathfind_test.go index 63ec579e3..6bf754c82 100644 --- a/routing/pathfind_test.go +++ b/routing/pathfind_test.go @@ -3469,6 +3469,7 @@ func TestLastHopPayloadSize(t *testing.T) { ) _, blindedPoint = btcec.PrivKeyFromBytes([]byte{5}) paymentAddr = &[32]byte{1} + ampOptions = &Options{} amtToForward = lnwire.MilliSatoshi(10000) finalHopExpiry int32 = 144 @@ -3510,6 +3511,7 @@ func TestLastHopPayloadSize(t *testing.T) { PaymentAddr: paymentAddr, DestCustomRecords: customRecords, Metadata: metadata, + Amp: ampOptions, }, amount: amtToForward, finalHopExpiry: finalHopExpiry, @@ -3524,6 +3526,7 @@ func TestLastHopPayloadSize(t *testing.T) { PaymentAddr: paymentAddr, DestCustomRecords: customRecords, Metadata: metadata, + Amp: ampOptions, }, amount: amtToForward, finalHopExpiry: finalHopExpiry, @@ -3560,6 +3563,13 @@ func TestLastHopPayloadSize(t *testing.T) { ) } + // In case it's an AMP payment we use the max AMP record + // size to estimate the final hop size. + var amp *record.AMP + if tc.restrictions.Amp != nil { + amp = &record.MaxAmpPayLoadSize + } + var finalHop route.Hop if tc.restrictions.BlindedPayment != nil { blindedPath := tc.restrictions.BlindedPayment. @@ -3586,6 +3596,7 @@ func TestLastHopPayloadSize(t *testing.T) { OutgoingTimeLock: uint32(tc.finalHopExpiry), Metadata: tc.restrictions.Metadata, MPP: mpp, + AMP: amp, CustomRecords: tc.restrictions.DestCustomRecords, } } diff --git a/routing/payment_lifecycle.go b/routing/payment_lifecycle.go index 9853f8fd9..e3bf77170 100644 --- a/routing/payment_lifecycle.go +++ b/routing/payment_lifecycle.go @@ -628,7 +628,7 @@ func (p *paymentLifecycle) createNewPaymentAttempt(rt *route.Route, return nil, err } - // It this shard carries MPP or AMP options, add them to the last hop + // If this shard carries MPP or AMP options, add them to the last hop // on the route. hop := rt.Hops[len(rt.Hops)-1] if shard.MPP() != nil { diff --git a/routing/payment_session.go b/routing/payment_session.go index 61496e915..2d174244c 100644 --- a/routing/payment_session.go +++ b/routing/payment_session.go @@ -258,6 +258,7 @@ func (p *paymentSession) RequestRoute(maxAmt, feeLimit lnwire.MilliSatoshi, DestCustomRecords: p.payment.DestCustomRecords, DestFeatures: p.payment.DestFeatures, PaymentAddr: p.payment.PaymentAddr, + Amp: p.payment.amp, Metadata: p.payment.Metadata, }