From e2a9b17254b092018c209e613b160bb898897921 Mon Sep 17 00:00:00 2001 From: ziggie Date: Sat, 2 Aug 2025 17:34:23 +0200 Subject: [PATCH 1/4] multi: skip range check in pathfinder and switch for custom htlc payments --- htlcswitch/link.go | 115 +++++++++++++++------ routing/bandwidth.go | 29 ++++-- routing/integrated_routing_context_test.go | 5 +- routing/unified_edges.go | 11 +- 4 files changed, 112 insertions(+), 48 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 1abf496a1..c5a445243 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -2547,36 +2547,12 @@ func (l *channelLink) canSendHtlc(policy models.ForwardingPolicy, heightNow uint32, originalScid lnwire.ShortChannelID, customRecords lnwire.CustomRecords) *LinkError { - // As our first sanity check, we'll ensure that the passed HTLC isn't - // too small for the next hop. If so, then we'll cancel the HTLC - // directly. - if amt < policy.MinHTLCOut { - l.log.Warnf("outgoing htlc(%x) is too small: min_htlc=%v, "+ - "htlc_value=%v", payHash[:], policy.MinHTLCOut, - amt) - - // As part of the returned error, we'll send our latest routing - // policy so the sending node obtains the most up to date data. - cb := func(upd *lnwire.ChannelUpdate1) lnwire.FailureMessage { - return lnwire.NewAmountBelowMinimum(amt, *upd) - } - failure := l.createFailureWithUpdate(false, originalScid, cb) - return NewLinkError(failure) - } - - // Next, ensure that the passed HTLC isn't too large. If so, we'll - // cancel the HTLC directly. - if policy.MaxHTLC != 0 && amt > policy.MaxHTLC { - l.log.Warnf("outgoing htlc(%x) is too large: max_htlc=%v, "+ - "htlc_value=%v", payHash[:], policy.MaxHTLC, amt) - - // As part of the returned error, we'll send our latest routing - // policy so the sending node obtains the most up-to-date data. - cb := func(upd *lnwire.ChannelUpdate1) lnwire.FailureMessage { - return lnwire.NewTemporaryChannelFailure(upd) - } - failure := l.createFailureWithUpdate(false, originalScid, cb) - return NewDetailedLinkError(failure, OutgoingFailureHTLCExceedsMax) + // Validate HTLC amount against policy limits. + linkErr := l.validateHtlcAmount( + policy, payHash, amt, originalScid, customRecords, + ) + if linkErr != nil { + return linkErr } // We want to avoid offering an HTLC which will expire in the near @@ -2591,6 +2567,7 @@ func (l *channelLink) canSendHtlc(policy models.ForwardingPolicy, return lnwire.NewExpiryTooSoon(*upd) } failure := l.createFailureWithUpdate(false, originalScid, cb) + return NewLinkError(failure) } @@ -2606,7 +2583,8 @@ func (l *channelLink) canSendHtlc(policy models.ForwardingPolicy, // We now check the available bandwidth to see if this HTLC can be // forwarded. availableBandwidth := l.Bandwidth() - auxBandwidth, err := fn.MapOptionZ( + + auxBandwidth, externalErr := fn.MapOptionZ( l.cfg.AuxTrafficShaper, func(ts AuxTrafficShaper) fn.Result[OptionalBandwidth] { var htlcBlob fn.Option[tlv.Blob] @@ -2624,8 +2602,10 @@ func (l *channelLink) canSendHtlc(policy models.ForwardingPolicy, return l.AuxBandwidth(amt, originalScid, htlcBlob, ts) }, ).Unpack() - if err != nil { - l.log.Errorf("Unable to determine aux bandwidth: %v", err) + if externalErr != nil { + l.log.Errorf("Unable to determine aux bandwidth: %v", + externalErr) + return NewLinkError(&lnwire.FailTemporaryNodeFailure{}) } @@ -2645,6 +2625,7 @@ func (l *channelLink) canSendHtlc(policy models.ForwardingPolicy, return lnwire.NewTemporaryChannelFailure(upd) } failure := l.createFailureWithUpdate(false, originalScid, cb) + return NewDetailedLinkError( failure, OutgoingFailureInsufficientBalance, ) @@ -4716,3 +4697,71 @@ func (l *channelLink) processLocalUpdateFailHTLC(ctx context.Context, // Immediately update the commitment tx to minimize latency. l.updateCommitTxOrFail(ctx) } + +// validateHtlcAmount checks if the HTLC amount is within the policy's +// minimum and maximum limits. Returns a LinkError if validation fails. +func (l *channelLink) validateHtlcAmount(policy models.ForwardingPolicy, + payHash [32]byte, amt lnwire.MilliSatoshi, + originalScid lnwire.ShortChannelID, + customRecords lnwire.CustomRecords) *LinkError { + + // In case we are dealing with a custom HTLC, we don't need to validate + // the HTLC constraints. + // + // NOTE: Custom HTLCs are only locally sourced and will use custom + // channels which are not routable channels and should have their policy + // not restricted in the first place. However to be sure we skip this + // check otherwise we might end up in a loop of sending to the same + // route again and again because link errors are not persisted in + // mission control. + if fn.MapOptionZ( + l.cfg.AuxTrafficShaper, + func(ts AuxTrafficShaper) bool { + return ts.IsCustomHTLC(customRecords) + }, + ) { + + l.log.Debugf("Skipping htlc amount policy validation for " + + "custom htlc") + + return nil + } + + // As our first sanity check, we'll ensure that the passed HTLC isn't + // too small for the next hop. If so, then we'll cancel the HTLC + // directly. + if amt < policy.MinHTLCOut { + l.log.Warnf("outgoing htlc(%x) is too small: min_htlc=%v, "+ + "htlc_value=%v", payHash[:], policy.MinHTLCOut, + amt) + + // As part of the returned error, we'll send our latest routing + // policy so the sending node obtains the most up to date data. + cb := func(upd *lnwire.ChannelUpdate1) lnwire.FailureMessage { + return lnwire.NewAmountBelowMinimum(amt, *upd) + } + failure := l.createFailureWithUpdate(false, originalScid, cb) + + return NewLinkError(failure) + } + + // Next, ensure that the passed HTLC isn't too large. If so, we'll + // cancel the HTLC directly. + if policy.MaxHTLC != 0 && amt > policy.MaxHTLC { + l.log.Warnf("outgoing htlc(%x) is too large: max_htlc=%v, "+ + "htlc_value=%v", payHash[:], policy.MaxHTLC, amt) + + // As part of the returned error, we'll send our latest routing + // policy so the sending node obtains the most up-to-date data. + cb := func(upd *lnwire.ChannelUpdate1) lnwire.FailureMessage { + return lnwire.NewTemporaryChannelFailure(upd) + } + failure := l.createFailureWithUpdate(false, originalScid, cb) + + return NewDetailedLinkError( + failure, OutgoingFailureHTLCExceedsMax, + ) + } + + return nil +} diff --git a/routing/bandwidth.go b/routing/bandwidth.go index afe085c2e..df68cea4f 100644 --- a/routing/bandwidth.go +++ b/routing/bandwidth.go @@ -24,9 +24,9 @@ type bandwidthHints interface { availableChanBandwidth(channelID uint64, amount lnwire.MilliSatoshi) (lnwire.MilliSatoshi, bool) - // firstHopCustomBlob returns the custom blob for the first hop of the - // payment, if available. - firstHopCustomBlob() fn.Option[tlv.Blob] + // isCustomHTLCPayment returns true if this payment is a custom payment. + // For custom payments policy checks might not be needed. + isCustomHTLCPayment() bool } // getLinkQuery is the function signature used to lookup a link. @@ -207,8 +207,23 @@ func (b *bandwidthManager) availableChanBandwidth(channelID uint64, return bandwidth, true } -// firstHopCustomBlob returns the custom blob for the first hop of the payment, -// if available. -func (b *bandwidthManager) firstHopCustomBlob() fn.Option[tlv.Blob] { - return b.firstHopBlob +// isCustomHTLCPayment returns true if this payment is a custom payment. +// For custom payments policy checks might not be needed. +func (b *bandwidthManager) isCustomHTLCPayment() bool { + return fn.MapOptionZ(b.firstHopBlob, func(blob tlv.Blob) bool { + customRecords, err := lnwire.ParseCustomRecords(blob) + if err != nil { + log.Warnf("failed to parse custom records when "+ + "checking if payment is custom: %v", err) + + return false + } + + return fn.MapOptionZ( + b.trafficShaper, + func(s htlcswitch.AuxTrafficShaper) bool { + return s.IsCustomHTLC(customRecords) + }, + ) + }) } diff --git a/routing/integrated_routing_context_test.go b/routing/integrated_routing_context_test.go index e89df8aee..a98fa5602 100644 --- a/routing/integrated_routing_context_test.go +++ b/routing/integrated_routing_context_test.go @@ -11,7 +11,6 @@ import ( "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnwire" "github.com/lightningnetwork/lnd/routing/route" - "github.com/lightningnetwork/lnd/tlv" "github.com/lightningnetwork/lnd/zpay32" "github.com/stretchr/testify/require" ) @@ -36,8 +35,8 @@ func (m *mockBandwidthHints) availableChanBandwidth(channelID uint64, return balance, ok } -func (m *mockBandwidthHints) firstHopCustomBlob() fn.Option[tlv.Blob] { - return fn.None[tlv.Blob]() +func (m *mockBandwidthHints) isCustomHTLCPayment() bool { + return false } // integratedRoutingContext defines the context in which integrated routing diff --git a/routing/unified_edges.go b/routing/unified_edges.go index f80e1cb1a..9b8f6c5c0 100644 --- a/routing/unified_edges.go +++ b/routing/unified_edges.go @@ -265,12 +265,13 @@ func (u *edgeUnifier) getEdgeLocal(netAmtReceived lnwire.MilliSatoshi, // Add inbound fee to get to the amount that is sent over the // local channel. amt := netAmtReceived + lnwire.MilliSatoshi(inboundFee) - // Check valid amount range for the channel. We skip this test - // for payments with custom HTLC data, as the amount sent on - // the BTC layer may differ from the amount that is actually - // forwarded in custom channels. - if bandwidthHints.firstHopCustomBlob().IsNone() && + + // for payments with custom htlc data we skip the amount range + // check because the amt of the payment does not relate to the + // actual amount carried by the HTLC but instead is encoded in + // the blob data. + if !bandwidthHints.isCustomHTLCPayment() && !edge.amtInRange(amt) { log.Debugf("Amount %v not in range for edge %v", From 5c22e5eb25eb6139385e4f801508f7b0d1a48d62 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 5 Aug 2025 16:53:24 +0200 Subject: [PATCH 2/4] itest: add payment test with max htlc restriction --- itest/list_on_test.go | 4 ++ itest/lnd_max_htlc_path_test.go | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 itest/lnd_max_htlc_path_test.go diff --git a/itest/list_on_test.go b/itest/list_on_test.go index 851662f13..7f9eaf574 100644 --- a/itest/list_on_test.go +++ b/itest/list_on_test.go @@ -242,6 +242,10 @@ var allTestCases = []*lntest.TestCase{ Name: "wumbo channels", TestFunc: testWumboChannels, }, + { + Name: "max htlc path payment", + TestFunc: testMaxHtlcPathPayment, + }, { Name: "max htlc pathfind", TestFunc: testMaxHtlcPathfind, diff --git a/itest/lnd_max_htlc_path_test.go b/itest/lnd_max_htlc_path_test.go new file mode 100644 index 000000000..3e8db169b --- /dev/null +++ b/itest/lnd_max_htlc_path_test.go @@ -0,0 +1,74 @@ +package itest + +import ( + "github.com/lightningnetwork/lnd/lnrpc" + "github.com/lightningnetwork/lnd/lnrpc/routerrpc" + "github.com/lightningnetwork/lnd/lntest" +) + +// testMaxHtlcPathPayment tests that when a payment is attempted, the path +// finding logic correctly takes into account the max_htlc value of the first +// channel. +func testMaxHtlcPathPayment(ht *lntest.HarnessTest) { + // Create a channel Alice->Bob. + chanPoints, nodes := ht.CreateSimpleNetwork( + [][]string{nil, nil}, lntest.OpenChannelParams{ + Amt: 1000000, + }, + ) + alice, bob := nodes[0], nodes[1] + chanPoint := chanPoints[0] + + // Alice and Bob should have one channel open with each other now. + ht.AssertNodeNumChannels(alice, 1) + ht.AssertNodeNumChannels(bob, 1) + + // Define a max_htlc value that is lower than the default. + const ( + maxHtlcMsat = 50000000 + minHtlcMsat = 1000 + timeLockDelta = 80 + baseFeeMsat = 0 + feeRate = 0 + ) + + // Update Alice's channel policy to set the new max_htlc value. + req := &lnrpc.PolicyUpdateRequest{ + Scope: &lnrpc.PolicyUpdateRequest_ChanPoint{ + ChanPoint: chanPoint, + }, + MaxHtlcMsat: maxHtlcMsat, + MinHtlcMsat: minHtlcMsat, + BaseFeeMsat: baseFeeMsat, + FeeRate: feeRate, + TimeLockDelta: timeLockDelta, + } + alice.RPC.UpdateChannelPolicy(req) + + expectedPolicy := &lnrpc.RoutingPolicy{ + FeeBaseMsat: baseFeeMsat, + FeeRateMilliMsat: feeRate, + TimeLockDelta: timeLockDelta, + MinHtlc: minHtlcMsat, + MaxHtlcMsat: maxHtlcMsat, + } + + // Wait for the policy update to propagate to Bob. + ht.AssertChannelPolicyUpdate( + bob, alice, expectedPolicy, chanPoint, false, + ) + + // Create an invoice for an amount greater than the max htlc value. + invoiceAmt := int64(maxHtlcMsat + 10_000_000) + invoice := &lnrpc.Invoice{ValueMsat: invoiceAmt} + resp := bob.RPC.AddInvoice(invoice) + + // Attempt to pay the invoice from Alice. The payment should be + // splitted into two parts, one for the max_htlc value and one for the + // remaining amount and succeed. + payReq := &routerrpc.SendPaymentRequest{ + PaymentRequest: resp.PaymentRequest, + FeeLimitMsat: noFeeLimitMsat, + } + ht.SendPaymentAssertSettled(alice, payReq) +} From 28f286a2c11b0d4974fe1be111102db19b1d1a03 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 5 Aug 2025 17:22:26 +0200 Subject: [PATCH 3/4] docs: fix 19.2 release-notes --- docs/release-notes/release-notes-0.19.2.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.2.md b/docs/release-notes/release-notes-0.19.2.md index 219fb3b34..63eeb12c8 100644 --- a/docs/release-notes/release-notes-0.19.2.md +++ b/docs/release-notes/release-notes-0.19.2.md @@ -113,5 +113,10 @@ much more slowly. ## Tooling and Documentation # Contributors (Alphabetical Order) + +* Calvin Zachman +* George Tsagkarelis * hieblmi +* Oliver Gugger * Yong Yu +* Ziggie From a19e7f2e5fcf23620015a82a00269fdd8216bd50 Mon Sep 17 00:00:00 2001 From: ziggie Date: Tue, 5 Aug 2025 17:22:44 +0200 Subject: [PATCH 4/4] docs: add release-notes --- docs/release-notes/release-notes-0.19.3.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/release-notes/release-notes-0.19.3.md b/docs/release-notes/release-notes-0.19.3.md index 60f7c6b91..241b905dd 100644 --- a/docs/release-notes/release-notes-0.19.3.md +++ b/docs/release-notes/release-notes-0.19.3.md @@ -29,6 +29,11 @@ imported in a watch-only (e.g. remote-signing) setup](https://github.com/lightningnetwork/lnd/pull/10119). +- [Fixed](https://github.com/lightningnetwork/lnd/pull/10125) a case in the + payment lifecycle where we would retry the same route over and over again in + situations where the sending amount would violate the channel policy + restriction (min,max HTLC). + # New Features ## Functional Enhancements @@ -77,3 +82,4 @@ * Olaoluwa Osuntokun * Oliver Gugger * Yong Yu +* Ziggie