Askrene fixes and enhancements #7640
Merged
Conversation
This file contains 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
rustyrussell
force-pushed
the
guilt/xpay
branch
from
e5a09c2
to
1f2ca46
Compare
|
I love this ac38c56, it is basically |
Lagrang3
reviewed
Sep 5, 2024
Lagrang3
reviewed
Sep 9, 2024
Lagrang3
reviewed
Sep 9, 2024
Lagrang3
reviewed
Sep 9, 2024
This was incorrect once we stopped removing old payments on failure, which was the reason we had to remove the HTLCs. It also removed by partid, which is wrong since it should have done old_payment->partid and old_payment->groupid! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If I put in X, how much can I get out after fees are subtracted? This was inspired by Eduardo's channel_maximum_forward in renepay, which is basically the same thing. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Saves some typing, and is clearer than checking if both args really are the same! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…is_zero/amount_msat_is_zero. I used `amount_msat_eq(x, AMOUNT_MSAT(0))` because I forgot this function existed. I probably missed it because the name is surprising, so add "is" in there to make it clear it's a boolean function. You'll note almost all the places which did use it are Eduardo's and Lisa's code, so maybe it's just me. Fix up a few places which I could use it, too. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…nd check total. There aren't, but make sure. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Not quite the same, as it doesn't have the "auto.local" layer, but it exhibits the same problem if we revert the fix for test_live_spendable. And it's much faster! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes it much easier to use generated gossip_store files. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We had a workaround for channels added by "auto.local", but instead we should make it work properly. I didn't do this before because we can't manipulate the localmods while they're applied, but it's simple to do it in two stages. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Rather than making the callers do this, make the invoice decoder perform the various sanity checks. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Simply calculate it when we need it, which means we don't have to keep it up-to-date as we tweak the flow. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is clearer: it's the final amount, not the amount we send! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is the root cause of the problem worked around in 50949b7 "askrene: hack in some padding so we don't overflow capacities." When adding fees to flows, we didn't recheck the boundary conditions: in renepay this is done by routebuilder. Fortunately, we can use our "reservations" infrastructure to temporarily use capacity as we process flows, so we handle the cases where they are not independent correclty. My assumption is that the resulting errors are small, so we divide them between the remaining flows based on highest-to-least probability. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…tra millisats. We don't actually hit the htlc_max cases, since the flow code already constrains us to that. And handling htlc_min is better done in the caller, where diagnostics are better (basically, we should eliminate them, and if that means no route, give a clear error message). And the refinement step can handle any extra millisats from rounding. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…estrictions properly. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…al HTLCs. "spendable" is for a single HTLC: if we own the channel, this amount decreases with every HTLC, as we have to pay fees. We have access to this since we call listpeerchannels anyway, so we can calculate the additional costs and use it in the refining phase. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
askrene.c was getting quite long, and this is self-contained. The only code change is a convenience accessor for the per-htlc-cost hash table. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Thanks to @Lagrang3 for spotting this! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
guilt/xpay
branch
from
2f71827
to
39af9fc
Compare
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
rustyrussell
force-pushed
the
guilt/xpay
branch
from
39af9fc
to
c7cfb06
Compare
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.
This fixes some corner cases which existed in askrene because I didn't get the nuance of @Lagrang3's MCF code. It makes a clearly-separate post "refinement" pass, which handles the finer details of adding fees, taking into account minimum htlc amounts, and also adds accommodations for the reduced capacity of local channels when we add more than one HTLC.