Skip to content

Commit

Permalink
routing: fix fee limit condition
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bitromortac committed Jul 30, 2024
1 parent 557b337 commit b0f0715
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 24 deletions.
5 changes: 5 additions & 0 deletions docs/release-notes/release-notes-0.18.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -150,6 +154,7 @@
# Contributors (Alphabetical Order)

* Andras Banki-Horvath
* bitromortac
* Bufo
* Elle Mouton
* Matheus Degiovani
Expand Down
8 changes: 2 additions & 6 deletions itest/lnd_routing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 18 additions & 18 deletions routing/pathfind.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit b0f0715

Please sign in to comment.