Improve SendPaymentV2 performance #10406
Open
+121
−63
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Replace the (bandwidth, bool) return signature with (bandwidth, error) to provide more context about why bandwidth is unavailable. This allows callers to distinguish between: - Channel not found in local channels map (ErrLocalChannelNotFound) - Channel found but unusable (offline, HTLC limits, etc.) The new error-based approach improves error handling throughout the routing package: - pathfind: Use capacity fallback only for channels which are not found in the local graph map, which can happen when channels were opened and activated during the payment process started for example. - unified_edges: Skip unusable local channels instead of using max bandwidth - Tests updated to use checkErrIs/checkErrContains for precise validation This fixes TODO comments about unclear bandwidth hint failures and improves channel selection by avoiding attempts to route through channels that cannot carry payments.
ziggie1984
force-pushed
the
fix-sendpaymentv2-performance
branch
from
ce40b53 to
745bcf5
Compare
Roasbeef
approved these changes
Dec 10, 2025
| bandwidth = lnwire.NewMSatFromSatoshis( | ||
| channel.Capacity, | ||
| ) | ||
| } else { |
Member
There was a problem hiding this comment.
Ah ok, so before we'd assume the capacity and potentially try to route over a channel that may not actually be online. This would be caught later in the switch, but it's better to catch this issue earlier.
| "this channel", shortID, bandwidth) | ||
| if err != nil { | ||
| // If the channel is not in our local channels map, use | ||
| // the channel capacity as a fallback. This can happen |
Member
There was a problem hiding this comment.
Yep this is accurate, as this is a per session snapshot. We could try to make this update dynanmically with newly mined channels, but I don't think that's really worth the extra effort.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I analysed a pg related OOM issue and analysing the logs I saw a LOT of this error messages:
what we see above is the reason because we would not distinguish between a channel which is just not in our map or which is currently unusable. That let to a lot of unnecessary retries and pathfinding calls because we would assume full bandwidth when we get this error.
This problem was already documented in 2 TODOs which are now done and removed.
The reason why it only got fixed now is because we changed some logging in this area which now made this problem obvious.
Log-Entries showed like 22000 similar log lines in 5 days (node does a lot of heavy rebalancing)