Skip to content

xpay: don't MPP if we're told not to #8059

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
merged 10 commits into from
Feb 14, 2025

Conversation

rustyrussell
Copy link
Contributor

Fixes: #8042

Apparently Muun.

We can't (yet) honor this for bolt12, because we didn't set the MPP feature until this PR.

@rustyrussell rustyrussell added this to the v25.02 milestone Feb 7, 2025
@rustyrussell rustyrussell requested a review from cdecker as a code owner February 7, 2025 03:47
@@ -16046,7 +16046,7 @@
"",
"Layers are generally maintained by plugins, either to contain persistent information about capacities which have been discovered, or to contain transient information for this particular payment (such as blinded paths or routehints).",
"",
"There are two automatic layers: *auto.localchans* contains information on local channels from this node (including non-public ones), and their exact current spendable capacities, and *auto.sourcefree* overrides all channels (including those from previous layers) leading out of the *source* to be zero fee and zero delay. These are both useful in the case where the source is the current node."
"There are trehee automatic layers: *auto.localchans* contains information on local channels from this node (including non-public ones), and their exact current spendable capacities. *auto.sourcefree* overrides all channels (including those from previous layers) leading out of the *source* to be zero fee and zero delay. These are both useful in the case where the source is the current node. And *auto.no_mpp_support* forces getroutes to return a single flow, though only basic checks are done that the result is useful."
Copy link
Collaborator

Choose a reason for hiding this comment

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

*three

Copy link
Collaborator

@Lagrang3 Lagrang3 left a comment

Choose a reason for hiding this comment

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

Nice fix.
I thought we could have some algorithm selection in askrene, so the single path payment would correspond to some specific algorithm specifically aimed at single
paths. But it would have taken some more time to write.

@rustyrussell rustyrussell force-pushed the guilt/xpay-no-mpp branch 2 times, most recently from 0d2b5f0 to 3c84972 Compare February 13, 2025 03:54
@endothermicdev
Copy link
Collaborator

payment_log(payment, LOG_INFORM, "No MPP support: this is going to be hard to pay"); is generating uninitialized memory? Not sure what that is about.

@rustyrussell
Copy link
Contributor Author

payment_log(payment, LOG_INFORM, "No MPP support: this is going to be hard to pay"); is generating uninitialized memory? Not sure what that is about.

Ah, we can't use payment_log there, since unique_id is not assigned yet! I moved it to later...

@rustyrussell rustyrussell enabled auto-merge (rebase) February 14, 2025 02:14
This is an inefficient hack.  Can you tell I really didn't want to
implement this?  MPP was finalized in 2018 FFS.

We do this by adding another "auto" layer, which removes all too-small
channels, and then makes our MPP node pile all the funds into the largest
channel it chooses.

Signed-off-by: Rusty Russell <[email protected]>
This is deeply annoying, and we may have to support this properly
(using a separate algorithm entirely) if other implementations don't
fix their crap.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: Plugins: xpay: suppress multi-part payment if invoice doesn't allow it (please, fix your nodes!)
We ignored the second one, but still it's unnecessary.

Signed-off-by: Rusty Russell <[email protected]>
…icitly says so.

We don't want to enable this yet, since we only just fixed CLN this release!

Signed-off-by: Rusty Russell <[email protected]>
Turns out we weren't wiring them through!  And libplugin wasn't reading them anyway.

Changelog-Fixed: lightningd: tell plugins our bolt12 features (so our bolt12 invoices explicitly allow MPP).
Signed-off-by: Rusty Russell <[email protected]>
It wasn't documented, and hopefully nobody was using it.

Changelog-Fixed: `decode` for bolt12 invoices "features" field renamed to "invoice_features" (as documentation said)
Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell merged commit 4bb7b49 into ElementsProject:master Feb 14, 2025
16 of 40 checks passed
@Lagrang3 Lagrang3 mentioned this pull request Mar 11, 2025
2 tasks
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.

xpay: act on mpp feature bits
3 participants