Renepay refactors and cleanups #6538
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
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Time is what users care about, so remove this. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
You cannot refresh the gossmap with localmods applied, nor apply localmods when others have applied localmods in the same process. There are optimizations we could do, but for now always apply/unapply before querying gossmap. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There was a problem hiding this comment.
CI Is broken
Submodule path 'external/lowdown': checked out 'edca6ce6d5336efb147321a43c47a698de41bb7c'
tests/test_renepay.py:315:58: F405 'TEST_NETWORK' may be undefined, or defined from star imports: fixtures
make: *** [Makefile:544: check-python-flake8] Error 1
rustyrussell
force-pushed
the
guilt/renepay-refactor
branch
from
67c1d1e
to
b834d5a
Compare
The general pattern for xxx_new is that it should populate all fields, for encapsulation and so you never can have a half-formed object. This means a fair bit of work for now, but it pays off in the next patch. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Results in payments having a pointer to the start of the object, which helps our memleak code. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Simply tell it to scan the entire object. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There are a few fields in `struct renepay` which are genuinely transient, but it makes the code much harder to follow than simply having a single structure. More cleanups will follow, but this is the minimal set. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Simplifies the logic since we bail out if there are two different group ids in progress. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can use json_scan(), and share a routine to map the notification to the pay_flow. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Use json_scan(), and use the new pay_flow_from_notification() routine. Also, the tal_dup_or_null can be tal_dup, since &preimage is never NULL. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
As recommended by your TODO, a bit simpler: we also make the hash function return a ptr rather than the (now rather large) struct. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Treat it just like "PAY_TRY_OTHER_ROUTE", except it is from the final node: this means we correctly process that it "succeeded". Add a test: this crashes sometimes, but it's cleaned up soon... Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
guilt/renepay-refactor
branch
2 times, most recently
from
b60c581
to
7e7acc2
Compare
Avoids a gratuitous "ctx" field, and the simplified declaration is now understood by `make update-mocks`. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It's not required, but it should be there so we might as well use it (though we sometimes don't put one in, esp if it's a private channel). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The main function here is payment_reconsider: * Each payment has a list of pay_flow. * This is populated in try_paying(), calling add_payflows & sendpay_new_flows. * When we get a notification, we resolve a pay_flow using one of the pay_flow_failedxxx or pay_flow_succeeded functions. * They call payment_reconsider() which cleans up finished flows decides what to do: often calling try_paying again. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to make sure that on every path, we terminate the flow. The simplest way to do this is encourage the pattern "return pay_flow_xxx(flow)". Indeed, this caught a few places I missed! Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Unifies the pay_flow resolve functions, and moves remove_htlc_payflow and commit_htlc_payflow to the top. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Consistency FTW. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
These show that we should clean up our notes. Here's the result from test_hardmpp: # we have computed a set of 1 flows with probability 0.328, fees 0msat and delay 23 # No MPP, so added 0msat shadow fee # Shadow route on flow 0/1 added 0 block delay. now 5 # sendpay flow groupid=1, partid=1, delivering=1800000000msat, probability=0.328 # Update chan knowledge scid=103x2x0, dir=0: [0msat,1799999999msat] # onion error WIRE_TEMPORARY_CHANNEL_FAILURE from node #1 103x2x0: failed: WIRE_TEMPORARY_CHANNEL_FAILURE (reply from remote) # we have computed a set of 2 flows with probability 0.115, fees 0msat and delay 23 # Shadow route on flow 0/2 added 0 block delay. now 5 # Shadow route on flow 1/2 added 0 block delay. now 5 # sendpay flow groupid=1, partid=3, delivering=500000000msat, probability=0.475 # sendpay flow groupid=1, partid=2, delivering=1300000000msat, probability=0.242 # Update chan knowledge scid=103x2x0, dir=0: [0msat,1299999999msat] # onion error WIRE_TEMPORARY_CHANNEL_FAILURE from node #1 103x2x0: failed: WIRE_TEMPORARY_CHANNEL_FAILURE (reply from remote) # we have computed a set of 2 flows with probability 0.084, fees 0msat and delay 23 # Shadow route on flow 0/2 added 0 block delay. now 5 # Shadow route on flow 1/2 added 0 block delay. now 5 # sendpay flow groupid=1, partid=5, delivering=260000000msat, probability=0.467 # sendpay flow groupid=1, partid=4, delivering=1040000000msat, probability=0.179 # Update chan knowledge scid=103x2x0, dir=0: [0msat,1039999999msat] # onion error WIRE_TEMPORARY_CHANNEL_FAILURE from node #1 103x2x0: failed: WIRE_TEMPORARY_CHANNEL_FAILURE (reply from remote) # we have computed a set of 2 flows with probability 0.052, fees 0msat and delay 23 # Shadow route on flow 0/2 added 0 block delay. now 5 # Shadow route on flow 1/2 added 0 block delay. now 5 # sendpay flow groupid=1, partid=7, delivering=120000000msat, probability=0.494 # sendpay flow groupid=1, partid=6, delivering=920000000msat, probability=0.105 Ideally it would look something like: # Computed 1 flows, probability=0.328: # Flow 1: 103x2x0 1800000000msat fee=0msat probability=0.328 shadow=+0msat/0blocks # Flow 1: FAIL: TEMPORARY_CHANNEL_FAILURE for 103x2x0. # Computed 2 flows, probability=0.115: # Flow 2: XXX->XXX 1300000000msat fee=XXX, probability=0.475 shadow=+0msat/0blocks # Flow 3: XXX->XXX 500000000msat fee=XXX, probability=0.475 shadow=+0msat/0blocks # Flow 2: FAIL: TEMPORARY_CHANNEL_FAILURE from node #1 103x2x0 # Computed 2 flows (3 total), probability=0.084, fee=0msat, delay=23 ... # Flow 4: SUCCESS, 2 in progress should succeed soon. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
guilt/renepay-refactor
branch
from
7e7acc2
to
4f25609
Compare
Lagrang3
reviewed
Aug 11, 2023
Lagrang3
reviewed
Aug 11, 2023
Lagrang3
reviewed
Aug 11, 2023
This was referenced Aug 12, 2023
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.
Please each commit carefully, as they do many different things.
The notifications at the end are glorious, but I also suggest how I'd like them to look in an Ideal World!