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

Renepay refactors and cleanups #6538

Merged

Commits on Aug 9, 2023

  1. renepay: remove unused all_flows field.

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    rustyrussell committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    2e1d6c3 View commit details
    Browse the repository at this point in the history
  2. renepay: remove attempt limit.

    Time is what users care about, so remove this.
    
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    rustyrussell committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    8c0e4fe View commit details
    Browse the repository at this point in the history
  3. renepay: fix localmods.

    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>
    rustyrussell committed Aug 9, 2023
    Configuration menu
    Copy the full SHA
    9fe203e View commit details
    Browse the repository at this point in the history

Commits on Aug 10, 2023

  1. renepay: use more formal allocator pattern.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    70042fe View commit details
    Browse the repository at this point in the history
  2. renepay: move list_node to first member of struct payment.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    52260c6 View commit details
    Browse the repository at this point in the history
  3. renepay: make memleak simpler.

    Simply tell it to scan the entire object.
    
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    dccf487 View commit details
    Browse the repository at this point in the history
  4. renepay: merge struct renepay and struct payment into one.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    698f415 View commit details
    Browse the repository at this point in the history
  5. renepay: get max group_id in single iteration.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    fa90e58 View commit details
    Browse the repository at this point in the history
  6. renepay: simplify JSON handling.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    2b813ac View commit details
    Browse the repository at this point in the history
  7. renepay: simplify JSON handling in notification_sendpay_success.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    775cebc View commit details
    Browse the repository at this point in the history
  8. renepay: don't re-parse bolt11 to get routehints.

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    0fb46e1 View commit details
    Browse the repository at this point in the history
  9. renepay: put the entire hash in the key struct.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    e1da480 View commit details
    Browse the repository at this point in the history
  10. renepay: remove unused result member.

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    149057f View commit details
    Browse the repository at this point in the history
  11. renepay: remove always-true "first_time" and "unlikely_ok" flags.

    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    99f331e View commit details
    Browse the repository at this point in the history
  12. renepay: fix up handling of errors from final node.

    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 committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    c358b56 View commit details
    Browse the repository at this point in the history
  13. renepay: make pay_plugin a tal object.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    e4b386f View commit details
    Browse the repository at this point in the history
  14. renepay: grab update from WIRE_TEMPORARY_CHANNEL_FAILURE if present.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    8cdef9a View commit details
    Browse the repository at this point in the history
  15. renepay: drive *all* progress from termination of struct pay_flow.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    a578732 View commit details
    Browse the repository at this point in the history
  16. renepay: add dummy pf_resuly type to ensure we deal with the flow.

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    d4ea3da View commit details
    Browse the repository at this point in the history
  17. renepay: do less work in destroy_pay_flow, and reorder pay_flow.c

    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>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    07d78c9 View commit details
    Browse the repository at this point in the history
  18. renepay: trivial cleanup to rename flow to pf everywhere.

    Consistency FTW.
    
    Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
    rustyrussell committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    a64a6ba View commit details
    Browse the repository at this point in the history
  19. renepay: add command notifications

    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 committed Aug 10, 2023
    Configuration menu
    Copy the full SHA
    4f25609 View commit details
    Browse the repository at this point in the history