-
Notifications
You must be signed in to change notification settings - Fork 961
xpay phoenix detect #8537
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
rustyrussell
wants to merge
16
commits into
ElementsProject:master
Choose a base branch
from
rustyrussell:guilt/xpay-phoenix-detect
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
xpay phoenix detect #8537
rustyrussell
wants to merge
16
commits into
ElementsProject:master
from
rustyrussell:guilt/xpay-phoenix-detect
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
a353419 to
cccac43
Compare
cccac43 to
faa6643
Compare
I added amount_msat_accumulate for the "a+=b" case, but I was struggling with a name for the subtractive equivalent. After some prompting, ChatGPT suggested deduct. Signed-off-by: Rusty Russell <[email protected]>
This is not worth optimizing that I can see. Using a non-debug build I get the following times for tests/test_askrene.py::test_real_data Before: 143 seconds After: 141 seconds. Signed-off-by: Rusty Russell <[email protected]>
We've already freed the working_ctx, and the fail path does that again. Signed-off-by: Rusty Russell <[email protected]>
It can only fail on overflow, but if it did, the fail path frees working_ctx and returns "error_message". Signed-off-by: Rusty Russell <[email protected]>
Make it calculate on demand. This will be useful when we call it from elsewhere. Signed-off-by: Rusty Russell <[email protected]>
We don't need the indexes array, we can use this directly. We still set up the indexes array (for now) after we call this. Signed-off-by: Rusty Russell <[email protected]>
This removes the index array from code after increase_flows()m, so we use the flows array directly. The next step will be to make increase_flows() use the flows array, and remove the index array indirection entirely. Signed-off-by: Rusty Russell <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
We don't need to convert to strings, we can compare directly. This removes the final use of the index arrays. This of course changes the order of returned routes, which alters test_real_biases, since that biases against the final channel in the *first* route. Took me far too long to diagnose that! Signed-off-by: Rusty Russell <[email protected]>
We added _noidx versions of the sort functions, but now they're the only ones, we can rename them to the old names. Signed-off-by: Rusty Russell <[email protected]>
Rewrite it, so it properly takes into account interactions between flows by using reservations. Signed-off-by: Rusty Russell <[email protected]>
For 1, we use single-path. For 0, reject. Signed-off-by: Rusty Russell <[email protected]>
Now we simply call it at the end. We need to check it hasn't violated fee maxima, but otherwise it's simple. Signed-off-by: Rusty Russell <[email protected]> Changelog-Fixed: Plugins: `askrene` now handles limits on number of htlcs much more gracefully.
Signed-off-by: Rusty Russell <[email protected]>
…an unknown channel. Signed-off-by: Rusty Russell <[email protected]>
…e can't route. This attempts to solve a problem we have with Phoenix clients: This payment has been split in two many parts by the sender: 31 parts vs max 6 parts allowed for on-the-fly funding. The problem is that we don't have any way in bolt11 or bolt12 to specify the maximum number of HTLCs. As a workaround, we start by restricting askrene to 6 parts if the node is not openly reachable, and if it struggles, we remove the restriction. This would work much better if askrene handled maxparts more completely! See-Also: ElementsProject#8331 Changelog-Fixed: `xpay` will not try to send too many HTLCs through unknown channels (6, as that is Phoenix's limit) unless it has no choice Signed-off-by: Rusty Russell <[email protected]>
faa6643 to
67d7d4b
Compare
Lagrang3
approved these changes
Nov 14, 2025
Collaborator
Lagrang3
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is straightforward.
I only have some minor requests on the previous PR #8688.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Based on #8688: this is just the last two commits.
This attempts to solve a problem we have with Phoenix clients:
The problem is that we don't have any way in bolt11 or bolt12 to
specify the maximum number of HTLCs.
As a workaround, we start by restricting askrene to 6 parts if the
node is not openly reachable, and if it struggles, we remove the
restriction. This would work much better if askrene handled maxparts
more completely!
See-Also: #8331