From b0f0715813977ec4a7bc63e0e3bc2d01bf43bb27 Mon Sep 17 00:00:00 2001 From: bitromortac Date: Fri, 26 Jul 2024 11:17:00 +0200 Subject: [PATCH] routing: fix fee limit condition When iterating edges, pathfinding checks early whether using an edge would violate the requested total fee limit for a route. This check is done on the net amount (an amount the inbound fee is calculated with). However, a possible next hop's fee discount leads to a reduction in fees and as such using the net amount leads to assuming a higher cumulative fee than the route really has, excluding the path erroneously. We perform the fee limit check on the amount to send, which includes both inbound and outbound fees. This should be possible as the first hop's outbound fee is zero and therefore doesn't have to be checked in the end. --- docs/release-notes/release-notes-0.18.3.md | 5 +++ itest/lnd_routing_test.go | 8 ++--- routing/pathfind.go | 36 +++++++++++----------- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/docs/release-notes/release-notes-0.18.3.md b/docs/release-notes/release-notes-0.18.3.md index 4cb59b76d9..6672a6e584 100644 --- a/docs/release-notes/release-notes-0.18.3.md +++ b/docs/release-notes/release-notes-0.18.3.md @@ -36,6 +36,10 @@ * [Fixed a bug](https://github.com/lightningnetwork/lnd/pull/8896) that caused LND to use a default fee rate for the batch channel opening flow. +* The fee limit for payments [was made + compatible](https://github.com/lightningnetwork/lnd/pull/8941) with inbound + fees. + # New Features ## Functional Enhancements ## RPC Additions @@ -150,6 +154,7 @@ # Contributors (Alphabetical Order) * Andras Banki-Horvath +* bitromortac * Bufo * Elle Mouton * Matheus Degiovani diff --git a/itest/lnd_routing_test.go b/itest/lnd_routing_test.go index 93dc23cab2..0b1cfebe84 100644 --- a/itest/lnd_routing_test.go +++ b/itest/lnd_routing_test.go @@ -1397,12 +1397,8 @@ func testFeeLimitAfterQueryRoutes(ht *lntest.HarnessTest) { FeeLimitMsat: 0, } - // We assert that the payment fails because the fee limit doesn't work - // correctly. This is fixed in the next commit. - ht.SendPaymentAssertFail( - alice, sendReq, - lnrpc.PaymentFailureReason_FAILURE_REASON_NO_ROUTE, - ) + // We assert that a route compatible with the fee limit is available. + ht.SendPaymentAssertSettled(alice, sendReq) // Once we're done, close the channels. ht.CloseChannel(alice, chanPointAliceBob) diff --git a/routing/pathfind.go b/routing/pathfind.go index 285d842e3a..652491763d 100644 --- a/routing/pathfind.go +++ b/routing/pathfind.go @@ -723,6 +723,24 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, amountToSend := toNodeDist.netAmountReceived + lnwire.MilliSatoshi(inboundFee) + // Check if accumulated fees would exceed fee limit when this + // node would be added to the path. + totalFee := int64(amountToSend) - int64(amt) + + log.Trace(lnutils.NewLogClosure(func() string { + return fmt.Sprintf( + "Checking fromVertex (%v) with "+ + "minInboundFee=%v, inboundFee=%v, "+ + "amountToSend=%v, amt=%v, totalFee=%v", + fromVertex, minInboundFee, inboundFee, + amountToSend, amt, totalFee, + ) + })) + + if totalFee > 0 && lnwire.MilliSatoshi(totalFee) > r.FeeLimit { + return + } + // Request the success probability for this edge. edgeProbability := r.ProbabilitySource( fromVertex, toNodeDist.node, amountToSend, @@ -779,24 +797,6 @@ func findPath(g *graphParams, r *RestrictParams, cfg *PathFindingConfig, netAmountToReceive := amountToSend + lnwire.MilliSatoshi(outboundFee) - // Check if accumulated fees would exceed fee limit when this - // node would be added to the path. - totalFee := int64(netAmountToReceive) - int64(amt) - - log.Trace(lnutils.NewLogClosure(func() string { - return fmt.Sprintf( - "Checking fromVertex (%v) with "+ - "minInboundFee=%v, inboundFee=%v, "+ - "amountToSend=%v, amt=%v, totalFee=%v", - fromVertex, minInboundFee, inboundFee, - amountToSend, amt, totalFee, - ) - })) - - if totalFee > 0 && lnwire.MilliSatoshi(totalFee) > r.FeeLimit { - return - } - // Calculate total probability of successfully reaching target // by multiplying the probabilities. Both this edge and the rest // of the route must succeed.