Skip to content

Askrene: fix constraints #8358

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

Merged

Conversation

Lagrang3
Copy link
Collaborator

@Lagrang3 Lagrang3 commented Jun 17, 2025

Description

In this PR I am refactoring the wrapper of the MCF solver and the refine procedure in askrene
to fix some issues we have seen when building routes in some common corner cases.
It addresses issues: #8339 and #7960.

Results

We have tested (with this tool https://github.com/Lagrang3/askrene-benchmarks/blob/master/test_askrene_bench.py)
the performance of askrene before and after the changes introduced in this PR.

We have found that in a test set of 50000 payment attempts
(random pairs of nodes and amounts from the set {1e2, 1e3, 1e4, 1e5, 1e6}) that 133 failed cases using the master branch
flip to success in with the changes in this PR, while only 3 cases that were successes in the master become failures
with this PR.

master Success master Failure
PR Success 46329 133
PR Failure 3 3535

Out of the 133 cases that flipped from failure to success the failed message was attributed to the following:

number of cases message
122 Could not find route without excessive cost
5 We couldn't quite afford it
5 Amount Xmsat below minimum Xmsat across X
1 Returned a flow that failed HTLC min constraints

For completeness we also show the failure cases that remained failure after the changes in this PR:

number of cases message
1383 The destination has disabled
1053 Could not find route without excessive cost
629 The source has disabled
260 The shortest path is X, but X marked disabled by gossip message
62 The shortest path is X, but X exceeds htlc_maximum_msat
55 Missing gossip for destination
55 The shortest path is X, but X exceeds htlc_maximum_msat
29 The shortest path is X, but X is constrained
6 The shortest path is X, but isn't big enough to carry
2 The shortest path is X, but has no gossip
1 We couldn't quite afford it
0 Amount Xmsat below minimum Xmsat across X
0 Returned a flow that failed HTLC min constraints

That means that we are able to fix 5 out of 6 cases of "We couldn't quite afford it".

There is also a significant decrease in runtime for payments around 1000sats (~3x) and significant
less fees for payments around 100sats (~3x):
meantime-big
fees-big
runtime-big

Tasks

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

Follow up

In a following PR I will address other issues related to refine and mcf:

This is part of a sequence of refactorings and improvements, last one is #8332.

@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch from 425e9cb to a471749 Compare June 23, 2025 11:56
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch from 57b4471 to be9cde6 Compare July 1, 2025 07:46
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch 3 times, most recently from 1d475e5 to 73fd757 Compare July 16, 2025 08:06
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch from 73fd757 to d570cff Compare July 17, 2025 13:22
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch from d570cff to 3c2d662 Compare July 18, 2025 06:04
Try a new way to refine flows,
ie. reduce excess due to MCF accuracy and HTLC max constraints
after hop amounts are computed with fees included.

Changelog-None.

Signed-off-by: Lagrang3 <[email protected]>
Disable channels with HTLC min violations so that we don't hit them
twice when computing routes.

Changelog-None

Signed-off-by: Lagrang3 <[email protected]>
that can be useful for us in the mcf.c main loop.

Changelog-None

Signed-off-by: Lagrang3 <[email protected]>
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch from 1288efe to 9fea795 Compare August 5, 2025 05:06
@Lagrang3 Lagrang3 marked this pull request as ready for review August 5, 2025 05:06
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch 2 times, most recently from 010a89e to 1d4e372 Compare August 5, 2025 07:15
by a small amount if the deliver amount is less than the requested
amount by X.
This step saves runtime by avoiding calling an extra MCF
and it helps us solve a small percentage of cases where the only
available routes have HTLCmin that is bigger than X.

Changelog-None

Signed-off-by: Lagrang3 <[email protected]>
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch 2 times, most recently from 16a7cb1 to 42aa01e Compare August 5, 2025 11:18
We use a wrapper around the MCF solver that takes care of finding the
best linearization parameters and fixing the flow values to meet the
htlc_min and htlc_max constraints.
We have reworked the current implementation and made it a bit more
similar to renepay's version.

Out of 50000 simulated payment situations distributed accross payment
amounts of 1e2, 1e3, 1e4, 1e5 and 1e6 sats, we find that 133 failed
cases in the master branch turn to success with the current changes,
while only 3 success cases in the master are not solved by the changes.

                master
            +-------+------+
            | S     | F    |
        +---+-------+------+
        | S | 46329 | 133  |
changes +---+-------+------+
        | F | 3     | 3535 |
        +---+-------+------+

Out of the 133 cases that flipped from failure to success the failed
reasons were:

122 -> "Could not find route without excessive cost"
5   -> "We couldn't quite afford it"
5   -> "Amount *msat below minimum"
1   -> tripped an HTLC min check

Changelog-None.

Signed-off-by: Lagrang3 <[email protected]>
and remove unused helper functions.

Changelog-None

Signed-off-by: Lagrang3 <[email protected]>
that new flows respect the HTLC min/max constraints.

Changelog-None

Signed-off-by: Lagrang3 <[email protected]>
@Lagrang3 Lagrang3 force-pushed the askrene-fix-constraints branch from 42aa01e to f31c093 Compare August 6, 2025 07:22
@Lagrang3 Lagrang3 added this to the v25.09 milestone Aug 6, 2025
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack f31c093

Great work!

@rustyrussell rustyrussell merged commit 5b8102e into ElementsProject:master Aug 8, 2025
40 checks passed
@Lagrang3 Lagrang3 deleted the askrene-fix-constraints branch August 8, 2025 08:06
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