Skip to content

xpay: add maxparts option #9692

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.

Sign up for GitHub

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

Open
Lagrang3 wants to merge 1 commit into
base: master
Choose a base branch
from
+519 −234
Open

xpay: add maxparts option #9692

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .msggen.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4690,6 +4690,7 @@
"Xpay.localinvreqid": 10,
"Xpay.maxdelay": 7,
"Xpay.maxfee": 3,
"Xpay.maxparts": 12,
"Xpay.partial_msat": 6,
"Xpay.payer_note": 8,
"Xpay.retry_for": 5
Expand Down Expand Up @@ -15423,6 +15424,10 @@
"added": "v24.11",
"deprecated": null
},
"Xpay.maxparts": {
"added": "v26.12",
"deprecated": null
},
"Xpay.partial_msat": {
"added": "v24.11",
"deprecated": null
Expand Down
1 change: 1 addition & 0 deletions cln-grpc/proto/node.proto
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions cln-grpc/src/convert.rs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions cln-rpc/src/model.rs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions contrib/msggen/msggen/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41437,6 +41437,14 @@
"`localinvreqid` is used by offers to link a payment attempt to a local `invoice_request` offer created by lightning-invoicerequest(7). This ensures that we only make a single payment for an offer, and that the offer is marked `used` once paid."
]
},
"maxparts": {
"type": "u32",
"added": "v26.12",
"description": [
"Maximum number of concurrent payment parts. Zero value is interpreted as unconstrained."
],
"default": "0"
},
"dev_use_shadow": {
"hidden": true,
"type": "boolean",
Expand Down
3 changes: 2 additions & 1 deletion contrib/pyln-client/pyln/client/lightning.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ def pay(self, bolt11, amount_msat=None, label=None, riskfactor=None,

def xpay(self, invstring, amount_msat=None, maxfee=None, retry_for=None,
partial_msat=None, maxdelay=None, payer_note=None, label=None, localinvreqid=None,
dev_use_shadow=None):
dev_use_shadow=None, maxparts=None):
"""
Send payment specified by {invstring}.
"""
Expand All @@ -1200,6 +1200,7 @@ def xpay(self, invstring, amount_msat=None, maxfee=None, retry_for=None,
"retry_for": retry_for,
"partial_msat": partial_msat,
"maxdelay": maxdelay,
"maxparts": maxparts,
"label": label,
"localinvreqid": localinvreqid,
"dev_use_shadow": dev_use_shadow,
Expand Down
428 changes: 214 additions & 214 deletions contrib/pyln-grpc-proto/pyln/grpc/node_pb2.py
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions doc/schemas/xpay.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@
"`localinvreqid` is used by offers to link a payment attempt to a local `invoice_request` offer created by lightning-invoicerequest(7). This ensures that we only make a single payment for an offer, and that the offer is marked `used` once paid."
]
},
"maxparts": {
"type": "u32",
"added": "v26.12",
"description": [
"Maximum number of concurrent payment parts. Zero value is interpreted as unconstrained."
],
"default": "0"
},
"dev_use_shadow": {
"hidden": true,
"type": "boolean",
Expand Down
66 changes: 48 additions & 18 deletions plugins/xpay/xpay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@ struct payment {
u32 maxdelay;
/* If non-zero: maximum number of payment routes that can be pending. */
u32 maxparts;
/* This is the maxparts set by the user, the internal one we send to
* getroutes might be different. */
u32 user_maxparts;
/* BOLT-11 payment secret (NULL for BOLT-12, it uses blinded paths) */
const struct secret *payment_secret;
/* BOLT-11 payment metadata (NULL for BOLT-12, it uses blinded paths) */
Expand Down Expand Up @@ -220,6 +223,7 @@ static struct command_result *xpay_core(struct command *cmd,
u32 retryfor,
const struct amount_msat *partial,
u32 maxdelay,
u32 maxparts,
const struct json_escape *label,
const struct sha256 *local_invreq_id,
bool use_shadow,
Expand Down Expand Up @@ -1901,13 +1905,22 @@ static struct command_result *getroutes_done_err(struct command *aux_cmd,
msg = json_strdup(tmpctx, buf, json_get_member(buf, error, "message"));
json_to_int(buf, json_get_member(buf, error, "code"), &code);

/* If we were restricting the number of parts, we remove that
* restriction and try again. */
if (payment->maxparts) {
payment_log(payment, LOG_INFORM,
"getroute failed with maxparts=%u, so retrying without that restriction",
payment->maxparts);
payment->maxparts = 0;
/* If we were restricting the number of parts, we relax it up to the
* user's provided maxparts (if any) and try again. */
if (payment->maxparts && (payment->user_maxparts == 0 ||

@cdecker cdecker Nov 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not go with user_maxparts right away? I think setting payment->maxparts = payment->user_maxpart and then relaxing if it does not work out is likely simpler, and more in line with what users want. It doesn't make much sense to me to try with a smaller maxparts first adn then try with user_maxparts and then unconstrained when we could implement the user's desired limit right away.

@Lagrang3 Lagrang3 Nov 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We had issues with Phoenix not accepting more than 6 parts.
The user sets maxparts = 10 could lead to failed payment, while maxparts = 5 a success.
But this is counter intuitive because the first case is less restrictive.
maxparts should be interpreted as: "whatever you do, I don't care as long as the number of parts does not exceed my user set maxparts".

Therefore, if the user sets maxparts = 10, the solver will try do its best to find a solution that satisfies that constraint.
We know that Phoenix wallet will not like more than 6, so we first try with 6, when 6 is less than 10, we are honoring the user's request.

payment->user_maxparts > payment->maxparts)) {
Comment on lines +1910 to +1911

@nGoline nGoline Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

To make user's maxparts final this could be:

Suggested change
if (payment->maxparts && (payment->user_maxparts == 0 ||
payment->user_maxparts > payment->maxparts)) {
if (payment->maxparts && !payment->user_maxparts) {


if (payment->user_maxparts)
payment_log(payment, LOG_INFORM,
"getroute failed with maxparts=%u, so "
"retrying with maxparts=%" PRIu32,
payment->maxparts, payment->user_maxparts);
else
payment_log(payment, LOG_INFORM,
"getroute failed with maxparts=%u, so "
"retrying without that restriction",
payment->maxparts);
payment->maxparts = payment->user_maxparts;
return getroutes_for(aux_cmd, payment, payment->amount_being_routed);
}

Expand Down Expand Up @@ -2413,6 +2426,7 @@ struct xpay_params {
const char **layers;
unsigned int retryfor;
u32 maxdelay;
u32 maxparts;
const char *bip353;
const char *payer_note;
struct json_escape *label;
Expand Down Expand Up @@ -2472,6 +2486,7 @@ invoice_fetched(struct command *cmd,
return xpay_core(cmd, inv,
NULL, params->maxfee, params->layers,
params->retryfor, params->partial, params->maxdelay,
params->maxparts,
params->label, NULL, false, false,
params->includefees_msat);
}
Expand Down Expand Up @@ -2548,7 +2563,7 @@ static struct command_result *json_xpay_params(struct command *cmd,
struct amount_msat *msat, *maxfee, *partial;
const char *invstring;
const char **layers;
u32 *maxdelay;
u32 *maxdelay, *maxparts;
const char *payer_note;
unsigned int *retryfor;
struct sha256 *localinvreqid;
Expand All @@ -2568,6 +2583,7 @@ static struct command_result *json_xpay_params(struct command *cmd,
p_opt("payer_note", param_string, &payer_note),
p_opt("label", param_label, &label),
p_opt("localinvreqid", param_sha256, &localinvreqid),
p_opt_def("maxparts", param_u32, &maxparts, 0),
p_opt_dev("dev_use_shadow", param_bool, &dev_use_shadow, true),
NULL))
return command_param_failed();
Expand Down Expand Up @@ -2599,6 +2615,7 @@ static struct command_result *json_xpay_params(struct command *cmd,
xparams->layers = layers;
xparams->retryfor = *retryfor;
xparams->maxdelay = *maxdelay;
xparams->maxparts = *maxparts;
xparams->bip353 = NULL;
xparams->payer_note = payer_note;
xparams->label = label;
Expand Down Expand Up @@ -2629,6 +2646,7 @@ static struct command_result *json_xpay_params(struct command *cmd,
xparams->layers = layers;
xparams->retryfor = *retryfor;
xparams->maxdelay = *maxdelay;
xparams->maxparts = *maxparts;
xparams->bip353 = invstring;
xparams->payer_note = payer_note;
xparams->label = label;
Expand All @@ -2643,9 +2661,9 @@ static struct command_result *json_xpay_params(struct command *cmd,
return send_outreq(req);
}

return xpay_core(cmd, invstring,
msat, maxfee, layers, *retryfor, partial, *maxdelay,
label, localinvreqid, *dev_use_shadow, as_pay, NULL);
return xpay_core(cmd, invstring, msat, maxfee, layers, *retryfor,
partial, *maxdelay, *maxparts, label, localinvreqid,
*dev_use_shadow, as_pay, NULL);
}

static void destroy_payment(struct payment *payment)
Expand All @@ -2665,6 +2683,7 @@ static struct payment *new_payment(const tal_t *ctx,
struct command *cmd,
u32 retryfor,
u32 maxdelay,
u32 maxparts,
const char **layers,
const char *invstring TAKES,
const struct pubkey *destination,
Expand Down Expand Up @@ -2734,6 +2753,7 @@ static struct payment *new_payment(const tal_t *ctx,
AMOUNT_MSAT(5000));
}
payment->maxdelay = maxdelay;
payment->user_maxparts = maxparts;
payment->payment_secret = tal_dup_or_null(payment, struct secret, payment_secret);
payment->payment_metadata = tal_dup_talarr(payment, u8, payment_metadata);
payment->final_cltv = final_cltv;
Expand Down Expand Up @@ -2788,6 +2808,7 @@ static struct command_result *xpay_core(struct command *cmd,
u32 retryfor,
const struct amount_msat *partial,
u32 maxdelay,
u32 maxparts,
const struct json_escape *label,
const struct sha256 *localinvreqid,
bool use_shadow,
Expand Down Expand Up @@ -2849,6 +2870,7 @@ static struct command_result *xpay_core(struct command *cmd,
payment = new_payment(cmd, cmd,
retryfor,
maxdelay,
maxparts,
layers,
take(invstring),
b12inv->invoice_node_id,
Expand Down Expand Up @@ -2956,6 +2978,7 @@ static struct command_result *xpay_core(struct command *cmd,
payment = new_payment(cmd, cmd,
retryfor,
maxdelay,
maxparts,
layers,
take(invstring),
&dst,
Expand Down Expand Up @@ -3031,11 +3054,15 @@ static struct command_result *xpay_core(struct command *cmd,
/* If we are using an unannounced channel, we assume we can
* only do 6 HTLCs at a time. This is currently true for
* Phoenix, which is a large and significant node. */
payment->maxparts = payment->user_maxparts;
node_id_from_pubkey(&dstid, &payment->destination);
if (!gossmap_find_node(gossmap, &dstid))
payment->maxparts = 6;
else
payment->maxparts = 0;
if (!gossmap_find_node(gossmap, &dstid)) {
if (payment->maxparts == 0 || payment->maxparts > 6)
payment->maxparts = 6;
payment_log(

@Andezion Andezion Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe log the real value ("... limiting maxparts to %u"), or log only when the value is actually lowered to 6, because the test hint_small waits for this exact line while asserting num_flows() <= 3

payment, LOG_INFORM,
"The destination is unannounced, limiting maxparts to 6");
}
Comment on lines +3057 to +3065

@nGoline nGoline Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When the user gives maxparts, it should be the limit, full stop: skip the unannounced-destination 6-part heuristic and do not relax on failure. Apply the heuristic (and the relax-to-unconstrained fallback) only when user_maxparts == 0. That also fixes the misleading log line (Andezion), and makes the test case "a larger maxparts does not override that" (tests/test_xpay.py:1977-1983) go away: with maxparts=10 the user asked for 10.

Suggested change
payment->maxparts = payment->user_maxparts;
node_id_from_pubkey(&dstid, &payment->destination);
if (!gossmap_find_node(gossmap, &dstid))
payment->maxparts = 6;
else
payment->maxparts = 0;
if (!gossmap_find_node(gossmap, &dstid)) {
if (payment->maxparts == 0 || payment->maxparts > 6)
payment->maxparts = 6;
payment_log(
payment, LOG_INFORM,
"The destination is unannounced, limiting maxparts to 6");
}
payment->maxparts = payment->user_maxparts;
node_id_from_pubkey(&dstid, &payment->destination);
if (!payment->maxparts && !gossmap_find_node(gossmap, &dstid)) {
payment_log(payment, LOG_INFORM,
"The destination is unannounced, limiting maxparts to 6");
payment->maxparts = 6;
}


if (disable_mpp) {
payment->maxparts = 1;

@Andezion Andezion Nov 5, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

maybe payment->maxparts = payment->user_maxparts = 1; in that branch. That also covers sendamount, because includefees_msat already sets disable_mpp = true

Expand Down Expand Up @@ -3132,6 +3159,7 @@ static struct command_result *json_sendamount(struct command *cmd,
xparams->layers = layers;
xparams->retryfor = *retryfor;
xparams->maxdelay = *maxdelay;
xparams->maxparts = 1;

@nGoline nGoline Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sendamount passing maxparts = 1 as the user limit also incidentally stops the relax-to-0 that could hit assert(attempt->payment->maxparts == 1) in attempt_mpp_amount() (xpay.c:418) for includefees payments, which is a good thing; worth a sentence in the commit message.

xparams->bip353 = NULL;
xparams->payer_note = payer_note;
xparams->label = label;
Expand All @@ -3152,6 +3180,7 @@ static struct command_result *json_sendamount(struct command *cmd,
xparams->layers = layers;
xparams->retryfor = *retryfor;
xparams->maxdelay = *maxdelay;
xparams->maxparts = 1;

@nGoline nGoline Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here: sendamount passing maxparts = 1 as the user limit also incidentally stops the relax-to-0 that could hit assert(attempt->payment->maxparts == 1) in attempt_mpp_amount() (xpay.c:418) for includefees payments, which is a good thing; worth a sentence in the commit message.

xparams->bip353 = invstring;
xparams->payer_note = payer_note;
xparams->label = label;
Expand All @@ -3165,9 +3194,9 @@ static struct command_result *json_sendamount(struct command *cmd,
}

/* Probably a bolt11, an invoice already. */
return xpay_core(cmd, invstring,
&invoice_msat, maxfee, layers, *retryfor, NULL, *maxdelay,
label, NULL, false, false, send_msat);
return xpay_core(cmd, invstring, &invoice_msat, maxfee, layers,
*retryfor, NULL, *maxdelay, /* maxparts = */ 1, label,

@nGoline nGoline Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here: sendamount passing maxparts = 1 as the user limit also incidentally stops the relax-to-0 that could hit assert(attempt->payment->maxparts == 1) in attempt_mpp_amount() (xpay.c:418) for includefees payments, which is a good thing; worth a sentence in the commit message.

NULL, false, false, send_msat);
}

static struct command_result *getchaininfo_done(struct command *aux_cmd,
Expand Down Expand Up @@ -3413,6 +3442,7 @@ static struct command_result *json_xkeysend(struct command *cmd,
payment = new_payment(cmd, cmd,
*retryfor,
*maxdelay,
/* maxparts = */ 1,

@nGoline nGoline Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

xkeysend now passes maxparts = 1 as the user limit. On master keysend starts at maxparts = 1 (line 3472) but getroutes_done_err relaxes it to 0 if no single path works. With user_maxparts = 1 the relax condition at 1910 is false, so any keysend that needs more than one path now fails outright.
This is an undocumented behaviour change for an existing command. Pass 0 here (keeping the internal payment->maxparts = 1 start), or make it a deliberate, changelogged change.

layers,
NULL, /* NULL invstring is the marker of a keysend vs pay */
dst,
Expand Down
Loading
Loading