Xpay limit parts #8448
Xpay limit parts #8448
Conversation
doc/schemas/xpay.json
Outdated
| "description": [ | ||
| "Maximum number of pending routes at any given time." | ||
| ], | ||
| "default": "6" |
There was a problem hiding this comment.
What could be a good default value for this?
There was a problem hiding this comment.
I think we should only restrict it if:
- We have more than 6, AND
- We get a rejection from the penultimate node.
A bit more complex (we must not ask for getroutes again if our maxparts has dropped), but avoids exposing this to users!
There was a problem hiding this comment.
I see... Let me for the time being remove the option from the schema and flag maxparts as a developer option.
Lagrang3
force-pushed
the
xpay-limit-parts
branch
from
d79b15d to
1d75ffe
Compare
doc/schemas/xpay.json
Outdated
| "description": [ | ||
| "Maximum number of pending routes at any given time." | ||
| ], | ||
| "default": "6" |
There was a problem hiding this comment.
I think we should only restrict it if:
- We have more than 6, AND
- We get a rejection from the penultimate node.
A bit more complex (we must not ask for getroutes again if our maxparts has dropped), but avoids exposing this to users!
Lagrang3
force-pushed
the
xpay-limit-parts
branch
2 times, most recently
from
00ccf25 to
0c93959
Compare
remove_flows is a helper function to remove flows from a set so that we keep the number of flows limited. Signed-off-by: Lagrang3 <[email protected]>
Changelog-Added: askrene: add a new parameter maxparts to getroutes that limits the number of routes in the solution. Signed-off-by: Lagrang3 <[email protected]>
dev_maxparts limits the number of pending routes allowed at any given time. Changelog-None Signed-off-by: Lagrang3 <[email protected]>
Lagrang3
force-pushed
the
xpay-limit-parts
branch
from
0c93959 to
f953656
Compare
There was a problem hiding this comment.
Ack f953656
Since we're close to release, this is OK, but can we open an issue to autodetect this overcrowing problem and then reduce?
| asort(*flows, tal_count(*flows), reverse_cmp_flows, NULL); | ||
| for (size_t count = tal_count(*flows); n > 0; n--, count--) { | ||
| assert(count > 0); | ||
| tal_arr_remove(flows, count - 1); |
There was a problem hiding this comment.
You didn't free the flow when you removed it? I think you'll want this helper later, so:
static void remove_and_free_flow(struct flow ***flows, size_t i)
{
tal_free((*flows)[i]);
tal_arr_remove(flows, i);
}
There was a problem hiding this comment.
But in this case it doesn't leak, so we can leave this neatning until later.
There was a problem hiding this comment.
Thanks! Removal was handled in the first version but I forgot after the changes.
Will do.
This PR attempts to limit the number of payment routes in xpay and askrene.
The solution proposed should be considered only a patch, because the algorithm is far
from optimal.
Solves issue #8331
Important
25.09 FREEZE July 28TH: Non-bugfix PRs not ready by this date will wait for 25.12.
RC1 is scheduled on August 11th
The final release is scheduled for September 1st.
Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked: