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

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion common/deprecation.c
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ bool deprecated_ok_(bool deprecated_apis,
level = deprecation(start, end);
switch (level) {
case DEPRECATED_SOON:
return false;
return true;
case DEPRECATED:
/* Complain if we're disallowing becuase it's deprecated */
allow = deprecated_apis;
Expand Down
5 changes: 4 additions & 1 deletion common/features.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ const char *feature_place_names[] = {
NULL,
"node",
"channel",
"invoice"
"invoice",
"bolt12_offer",
"bolt12_invreq",
"bolt12_invoice",
};

static const struct feature_style feature_styles[] = {
Expand Down
2 changes: 1 addition & 1 deletion contrib/msggen/msggen/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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

],
"categories": [
"readonly"
Expand Down
5 changes: 3 additions & 2 deletions devtools/bolt11-cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,10 @@ static void encode(const tal_t *ctx,
b11->timestamp = strtoul(val, &endp, 10);
if (!b11->timestamp || *endp != '\0')
errx(ERROR_USAGE, "Invalid amount %s", val);
/* Allow raw numbered fields */
/* Allow raw numbered fields (except 9, that's below) */
} else if ((fieldnum = strtoul(fname, &endp, 10)) != 0
&& fieldnum < 256) {
&& fieldnum < 256
&& fieldnum != 9) {
struct bolt11_field *extra = tal(b11, struct bolt11_field);
extra->tag = fieldnum;
extra->data = tal_hexdata(extra, val, strlen(val));
Expand Down
1 change: 1 addition & 0 deletions doc/developers-guide/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ hidden: false
| close.tx | Field | v24.11 | v25.11 | Use txs array instead |
| close.txid | Field | v24.11 | v25.11 | Use txids array instead |
| experimental-offers | Config | v24.11 | v25.05 | Now the default |
| xpay.ignore_bolt12_mpp | Field | v25.05 | v25.11 | Try MPP even if the BOLT12 invoice doesn't explicitly allow it (CLN didn't until 25.02) |

Inevitably there are features which need to change: either to be generalized, or removed when they can no longer be supported.

Expand Down
2 changes: 1 addition & 1 deletion doc/schemas/lightning-getroutes.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,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."
],
"categories": [
"readonly"
Expand Down
51 changes: 47 additions & 4 deletions plugins/askrene/askrene.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ static struct command_result *param_layer_names(struct command *cmd,

/* Must be a known layer name */
if (streq((*arr)[i], "auto.localchans")
|| streq((*arr)[i], "auto.no_mpp_support")
|| streq((*arr)[i], "auto.sourcefree"))
continue;
if (!find_layer(get_askrene(cmd->plugin), (*arr)[i])) {
Expand Down Expand Up @@ -209,6 +210,44 @@ static struct layer *source_free_layer(const tal_t *ctx,
return layer;
}

/* We're going to abuse MCF, and take the largest flow it gives and ram everything
* through it. This is more effective if there's at least a *chance* that can handle
* the full amount.
*
* It's far from perfect, but I have very little sympathy: if you want
* to receive amounts reliably, enable MPP.
*/
static struct layer *remove_small_channel_layer(const tal_t *ctx,
struct askrene *askrene,
struct amount_msat min_amount,
struct gossmap_localmods *localmods)
{
struct layer *layer = new_temp_layer(ctx, askrene, "auto.no_mpp_support");
struct gossmap *gossmap = askrene->gossmap;
struct gossmap_chan *c;

/* We apply this so we see any created channels */
gossmap_apply_localmods(gossmap, localmods);

for (c = gossmap_first_chan(gossmap); c; c = gossmap_next_chan(gossmap, c)) {
struct short_channel_id_dir scidd;
if (amount_msat_greater_eq(gossmap_chan_get_capacity(gossmap, c),
min_amount))
continue;

scidd.scid = gossmap_chan_scid(gossmap, c);
/* Layer will disable this in both directions */
for (scidd.dir = 0; scidd.dir < 2; scidd.dir++) {
const bool enabled = false;
layer_add_update_channel(layer, &scidd, &enabled,
NULL, NULL, NULL, NULL, NULL);
}
}
gossmap_remove_localmods(gossmap, localmods);

return layer;
}

struct amount_msat get_additional_per_htlc_cost(const struct route_query *rq,
const struct short_channel_id_dir *scidd)
{
Expand Down Expand Up @@ -321,6 +360,7 @@ static const char *get_routes(const tal_t *ctx,
const char **layers,
struct gossmap_localmods *localmods,
const struct layer *local_layer,
bool single_path,
struct route ***routes,
struct amount_msat **amounts,
const struct additional_cost_htable *additional_costs,
Expand Down Expand Up @@ -355,8 +395,10 @@ static const char *get_routes(const tal_t *ctx,
if (streq(layers[i], "auto.localchans")) {
plugin_log(rq->plugin, LOG_DBG, "Adding auto.localchans");
l = local_layer;
} else if (streq(layers[i], "auto.no_mpp_support")) {
plugin_log(rq->plugin, LOG_DBG, "Adding auto.no_mpp_support, sorry");
l = remove_small_channel_layer(layers, askrene, amount, localmods);
} else {
/* Handled below, after other layers */
assert(streq(layers[i], "auto.sourcefree"));
plugin_log(rq->plugin, LOG_DBG, "Adding auto.sourcefree");
l = source_free_layer(layers, askrene, source, localmods);
Expand Down Expand Up @@ -406,7 +448,7 @@ static const char *get_routes(const tal_t *ctx,
/* First up, don't care about fees (well, just enough to tiebreak!) */
mu = 1;
flows = minflow(rq, rq, srcnode, dstnode, amount,
mu, delay_feefactor);
mu, delay_feefactor, single_path);
if (!flows) {
ret = explain_failure(ctx, rq, srcnode, dstnode, amount);
goto fail;
Expand All @@ -419,7 +461,7 @@ static const char *get_routes(const tal_t *ctx,
"The worst flow delay is %"PRIu64" (> %i), retrying with delay_feefactor %f...",
flows_worst_delay(flows), maxdelay - finalcltv, delay_feefactor);
flows = minflow(rq, rq, srcnode, dstnode, amount,
mu, delay_feefactor);
mu, delay_feefactor, single_path);
if (!flows || delay_feefactor > 10) {
ret = rq_log(ctx, rq, LOG_UNUSUAL,
"Could not find route without excessive delays");
Expand All @@ -442,7 +484,7 @@ static const char *get_routes(const tal_t *ctx,
fmt_amount_msat(tmpctx, maxfee),
mu);
new_flows = minflow(rq, rq, srcnode, dstnode, amount,
mu > 100 ? 100 : mu, delay_feefactor);
mu > 100 ? 100 : mu, delay_feefactor, single_path);
if (!flows || mu >= 100) {
ret = rq_log(ctx, rq, LOG_UNUSUAL,
"Could not find route without excessive cost");
Expand Down Expand Up @@ -616,6 +658,7 @@ static struct command_result *do_getroutes(struct command *cmd,
info->source, info->dest,
*info->amount, *info->maxfee, *info->finalcltv,
*info->maxdelay, info->layers, localmods, info->local_layer,
have_layer(info->layers, "auto.no_mpp_support"),
&routes, &amounts, info->additional_costs, &probability);
if (err)
return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "%s", err);
Expand Down
28 changes: 27 additions & 1 deletion plugins/askrene/mcf.c
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,8 @@ struct flow **minflow(const tal_t *ctx,
const struct gossmap_node *target,
struct amount_msat amount,
u32 mu,
double delay_feefactor)
double delay_feefactor,
bool single_part)
{
struct flow **flow_paths;
/* We allocate everything off this, and free it at the end,
Expand Down Expand Up @@ -1044,6 +1045,31 @@ struct flow **minflow(const tal_t *ctx,
goto fail;
}
tal_free(working_ctx);

/* This is dumb, but if you don't support MPP you don't deserve any
* better. Pile it into the largest part if not already. */
if (single_part) {
struct flow *best = flow_paths[0];
for (size_t i = 1; i < tal_count(flow_paths); i++) {
if (amount_msat_greater(flow_paths[i]->delivers, best->delivers))
best = flow_paths[i];
}
for (size_t i = 0; i < tal_count(flow_paths); i++) {
if (flow_paths[i] == best)
continue;
if (!amount_msat_accumulate(&best->delivers,
flow_paths[i]->delivers)) {
rq_log(tmpctx, rq, LOG_BROKEN,
"%s: failed to extract accumulate flow paths %s+%s",
__func__,
fmt_amount_msat(tmpctx, best->delivers),
fmt_amount_msat(tmpctx, flow_paths[i]->delivers));
goto fail;
}
}
flow_paths[0] = best;
tal_resize(&flow_paths, 1);
}
return flow_paths;

fail:
Expand Down
5 changes: 4 additions & 1 deletion plugins/askrene/mcf.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ struct route_query;
* @target: the target to pay
* @amount: the amount we want to reach @target
* @mu: 0 = corresponds to only probabilities, 100 corresponds to only fee.
* @delay_feefactor: convert 1 block delay into msat.
* @single_part: don't do MCF at all, just create a single flow.
*
* @delay_feefactor converts 1 block delay into msat, as if it were an additional
* fee. So if a CLTV delay on a node is 5 blocks, that's treated as if it
Expand All @@ -29,7 +31,8 @@ struct flow **minflow(const tal_t *ctx,
const struct gossmap_node *target,
struct amount_msat amount,
u32 mu,
double delay_feefactor);
double delay_feefactor,
bool single_part);

/* To sanity check: this is the approximation mcf uses for the cost
* of each channel. */
Expand Down
6 changes: 6 additions & 0 deletions plugins/libplugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -1475,6 +1475,12 @@ static struct feature_set *json_to_feature_set(struct plugin *plugin,
p = CHANNEL_FEATURE;
else if (json_tok_streq(buf, t, "invoice"))
p = BOLT11_FEATURE;
else if (json_tok_streq(buf, t, "bolt12_offer"))
p = BOLT12_OFFER_FEATURE;
else if (json_tok_streq(buf, t, "bolt12_invreq"))
p = BOLT12_INVREQ_FEATURE;
else if (json_tok_streq(buf, t, "bolt12_invoice"))
p = BOLT12_INVOICE_FEATURE;
else
continue;
fset->bits[p] = json_tok_bin_from_hex(fset, buf, t + 1);
Expand Down
2 changes: 1 addition & 1 deletion plugins/offers_invreq_hook.c
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ static struct command_result *add_blindedpaths(struct command *cmd,
/* Don't bother if we're public */
node_id_from_pubkey(&local_nodeid, &id);
if (gossmap_find_node(get_gossmap(cmd->plugin), &local_nodeid))
create_invoicereq(cmd, ir);
return create_invoicereq(cmd, ir);

return find_best_peer(cmd, OPT_ROUTE_BLINDING,
found_best_peer, ir);
Expand Down
12 changes: 12 additions & 0 deletions plugins/xpay/xpay.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ struct payment {
struct amount_msat maxfee;
/* Maximum delay on the route we're ok with */
u32 *maxdelay;
/* Do we have to do it all in a single part? */
bool disable_mpp;
/* BOLT11 payment secret (NULL for BOLT12, it uses blinded paths) */
const struct secret *payment_secret;
/* BOLT11 payment metadata (NULL for BOLT12, it uses blinded paths) */
Expand Down Expand Up @@ -1192,6 +1194,8 @@ static struct command_result *getroutes_for(struct command *aux_cmd,
/* Add user-specified layers */
for (size_t i = 0; i < tal_count(payment->layers); i++)
json_add_string(req->js, NULL, payment->layers[i]);
if (payment->disable_mpp)
json_add_string(req->js, NULL, "auto.no_mpp_support");
json_array_end(req->js);
json_add_amount_msat(req->js, "maxfee_msat", maxfee);
json_add_u32(req->js, "final_cltv", payment->final_cltv);
Expand Down Expand Up @@ -1551,6 +1555,10 @@ static struct command_result *json_xpay_core(struct command *cmd,
if (payment->payinfos[i]->cltv_expiry_delta > payment->final_cltv)
payment->final_cltv = payment->payinfos[i]->cltv_expiry_delta;
}
/* We will start honoring this flag in future */
payment->disable_mpp = !feature_offered(b12inv->invoice_features, OPT_BASIC_MPP);
if (payment->disable_mpp && command_deprecated_in_ok(cmd, "ignore_bolt12_mpp", "v25.05", "v25.11"))
payment->disable_mpp = false;
} else {
struct bolt11 *b11
= bolt11_decode(tmpctx, payment->invstring,
Expand Down Expand Up @@ -1586,12 +1594,16 @@ static struct command_result *json_xpay_core(struct command *cmd,
else
payment->full_amount = *msat;

payment->disable_mpp = !feature_offered(b11->features, OPT_BASIC_MPP);
if (amount_msat_is_zero(payment->full_amount))
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Cannot pay bolt11 invoice with zero amount");
invexpiry = b11->timestamp + b11->expiry;
}

if (payment->disable_mpp)
payment_log(payment, LOG_INFORM, "No MPP support: this is going to be hard to pay");

now = time_now().ts.tv_sec;
if (now > invexpiry)
return command_fail(cmd, PAY_INVOICE_EXPIRED,
Expand Down
20 changes: 20 additions & 0 deletions tests/test_askrene.py
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,26 @@ def test_getroutes(node_factory):
'amount_msat': 5500005,
'delay': 99 + 6}]])

# We realize that this is impossible in a single path:
with pytest.raises(RpcError, match="The shortest path is 0x2x1, but 0x2x1/1 marked disabled by layer auto.no_mpp_support."):
l1.rpc.getroutes(source=nodemap[0],
destination=nodemap[2],
amount_msat=10000000,
layers=['auto.no_mpp_support'],
maxfee_msat=1000,
final_cltv=99)

# But this will work.
check_getroute_paths(l1,
nodemap[0],
nodemap[2],
9000000,
[[{'short_channel_id_dir': '0x2x3/1',
'next_node_id': nodemap[2],
'amount_msat': 9000009,
'delay': 99 + 6}]],
layers=['auto.no_mpp_support'])


def test_getroutes_fee_fallback(node_factory):
"""Test getroutes call takes into account fees, if excessive"""
Expand Down
Loading
Loading