Skip to content
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

Pay: handle when we're close to htlc limit #4563

Merged

Conversation

rustyrussell
Copy link
Contributor

Didn't fix the problem, broke pay :( Help @cdecker !

@rustyrussell rustyrussell requested a review from cdecker May 28, 2021 04:30
@ElementsProject ElementsProject deleted a comment from jonathancaras May 28, 2021
@rustyrussell rustyrussell changed the title BROKEN Pay: handle when we're close to htlc limit Pay: handle when we're close to htlc limit May 29, 2021
@cdecker cdecker force-pushed the pay-calc-htlc-limit-properly branch 2 times, most recently from f1c23a2 to 14c0877 Compare June 2, 2021 14:49
@cdecker
Copy link
Member

cdecker commented Jun 2, 2021

Added one last fix that makes sure we don't exhaust the HTLC budget outright with the pre-splitter, leaving us some wiggle room to adapt to failures.

@rustyrussell, can you review this despite being the opener? :D

@cdecker cdecker added this to the v0.10.1 milestone Jun 2, 2021
@cdecker cdecker marked this pull request as ready for review June 2, 2021 14:51
rustyrussell and others added 5 commits June 3, 2021 13:15
As you can see, I did a lot of debugging before realizing that the
actual problem is in the pay plugin :(

Signed-off-by: Rusty Russell <[email protected]>
This was triggered by having some part being started after the overall
command already gave up, cleaning up the `cmd` context from which the
routehints were allocated. The early exit of the command, as a result
from a terminal state does not guarantee that no later attempt will
try to find a route, especially if the attempt was started before we
knew that it is doomed.
We were counting the attempts including the root payment, which
resulted in an off-by-one error with the `test_pay_low_max_htlcs`
test. Counting the children of the root payment after the presplitter
had a go is the correct way to do it, since at that time we only have
one level in the tree, no need to recurse and potentially count
ourselves.
Changelog-Fixed: pay: The presplitter mod will no longer exhaust the HTLC budget.
@rustyrussell
Copy link
Contributor Author

OK, neatened up the commits and rebased on master. Thanks @cdecker!

Ack 10e0e96

@rustyrussell rustyrussell merged commit 5e1fadf into ElementsProject:master Jun 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants