From f8b568f097904832ac3e08451f4d18d788a7c1b9 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 15:19:48 +1030 Subject: [PATCH 01/10] devtools: allow encode with `9` flag (for features). Signed-off-by: Rusty Russell --- devtools/bolt11-cli.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/devtools/bolt11-cli.c b/devtools/bolt11-cli.c index e28762bea779..5553822c6738 100644 --- a/devtools/bolt11-cli.c +++ b/devtools/bolt11-cli.c @@ -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)); From 383239c879b02fae48fb29866ddca8f1f8e1c53a Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 15:20:48 +1030 Subject: [PATCH 02/10] pytest: test that xpay doesn't use MPP if invoice doesn't permit it. Signed-off-by: Rusty Russell --- tests/test_xpay.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/test_xpay.py b/tests/test_xpay.py index d20bc1419311..a9220e3e7023 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -254,6 +254,7 @@ def test_xpay_fake_channeld(node_factory, bitcoind, chainparams): f"currency={chainparams['bip173_prefix']}", f"p={hash_hex}", f"s={'00' * 32}", + "9=020000", # option_basic_mpp f"d=Paying node {n}", f"amount={AMOUNT}msat"]).decode('utf-8').strip() assert l1.rpc.decode(inv)['payee'] == nodeids[n] @@ -286,6 +287,7 @@ def test_xpay_fake_channeld(node_factory, bitcoind, chainparams): "encode", n.to_bytes(length=8, byteorder=sys.byteorder).hex() + '01' * 24, f"p={hash_hex}", + "9=020000", # option_basic_mpp f"s={'00' * 32}", f"d=Paying node {n}", f"amount={AMOUNT}msat"]).decode('utf-8').strip() @@ -556,6 +558,7 @@ def test_xpay_maxfee(node_factory, bitcoind, chainparams): n.to_bytes(length=8, byteorder=sys.byteorder).hex() + '01' * 24, f"currency={chainparams['bip173_prefix']}", f"p={hash_hex}", + "9=020000", # option_basic_mpp f"s={'00' * 32}", f"d=Paying node {n} with maxfee", f"amount={AMOUNT}msat"]).decode('utf-8').strip() @@ -616,3 +619,34 @@ def test_xpay_zeroconf(node_factory): offer = l2.rpc.offer('any')['bolt12'] b12 = l1.rpc.fetchinvoice(offer, '100000msat')['invoice'] l1.rpc.xpay(b12) + + +@pytest.mark.xfail(strict=True) +def test_xpay_no_mpp(node_factory, chainparams): + """Suppress mpp, resulting in a single payment part""" + l1, l2, l3, l4 = node_factory.get_nodes(4) + node_factory.join_nodes([l1, l2, l3], wait_for_announce=True) + node_factory.join_nodes([l1, l4, l3], wait_for_announce=True) + + # Amount needs to be enought that it bothers splitting, but not + # so much that it can't pay without mpp. + AMOUNT = 500000000 + + # We create a version of this which doesn't support MPP + no_mpp = l3.rpc.invoice(AMOUNT, 'test_xpay_no_mpp2', 'test_xpay_no_mpp without mpp') + b11_no_mpp = subprocess.check_output(["devtools/bolt11-cli", + "encode", + # secret for l3 + "dae24b3853e1443a176daba5544ee04f7db33ebe38e70bdfdb1da34e89512c10", + f"currency={chainparams['bip173_prefix']}", + f"p={no_mpp['payment_hash']}", + f"s={no_mpp['payment_secret']}", + f"d=Paying l3 without mpp", + f"amount={AMOUNT}"]).decode('utf-8').strip() + + # This should not mpp! + ret = l1.rpc.xpay(b11_no_mpp) + assert ret['failed_parts'] == 0 + assert ret['successful_parts'] == 1 + assert ret['amount_msat'] == AMOUNT + assert ret['amount_sent_msat'] == AMOUNT + AMOUNT // 100000 + 1 From 1ca31f45b97ca0789e9a5c43e3d8454f3f0dcfb0 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 15:21:48 +1030 Subject: [PATCH 03/10] askrene: remove all small channels if there's no MPP support. 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 --- contrib/msggen/msggen/schema.json | 2 +- doc/schemas/lightning-getroutes.json | 2 +- plugins/askrene/askrene.c | 51 +++++++++++++++++++++++++--- plugins/askrene/mcf.c | 28 ++++++++++++++- plugins/askrene/mcf.h | 5 ++- tests/test_askrene.py | 20 +++++++++++ 6 files changed, 100 insertions(+), 8 deletions(-) diff --git a/contrib/msggen/msggen/schema.json b/contrib/msggen/msggen/schema.json index b711937e7685..aedfac9a30bf 100644 --- a/contrib/msggen/msggen/schema.json +++ b/contrib/msggen/msggen/schema.json @@ -16117,7 +16117,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 three 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" diff --git a/doc/schemas/lightning-getroutes.json b/doc/schemas/lightning-getroutes.json index ee3f913ab4c6..cfa2ac3a3b90 100644 --- a/doc/schemas/lightning-getroutes.json +++ b/doc/schemas/lightning-getroutes.json @@ -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 three 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" diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index 57631bb5a897..958859fa4c94 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -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])) { @@ -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) { @@ -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, @@ -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); @@ -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; @@ -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"); @@ -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"); @@ -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); diff --git a/plugins/askrene/mcf.c b/plugins/askrene/mcf.c index bd29d1830eca..6339510a5ce9 100644 --- a/plugins/askrene/mcf.c +++ b/plugins/askrene/mcf.c @@ -965,7 +965,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, @@ -1048,6 +1049,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: diff --git a/plugins/askrene/mcf.h b/plugins/askrene/mcf.h index 2a1b8fcf0cd2..f8100e766dd6 100644 --- a/plugins/askrene/mcf.h +++ b/plugins/askrene/mcf.h @@ -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 @@ -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. */ diff --git a/tests/test_askrene.py b/tests/test_askrene.py index 4a3685a788ae..7404d4664cc3 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -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""" From c520fad006505e9d2d037ee1aad548aa354fe0c4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 15:22:48 +1030 Subject: [PATCH 04/10] xpay: avoid MPP if invoice does not allow it. 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 Changelog-Fixed: Plugins: xpay: suppress multi-part payment if invoice doesn't allow it (please, fix your nodes!) --- plugins/xpay/xpay.c | 11 +++++++++++ tests/test_xpay.py | 1 - 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c index b74ecbef6049..fca3f68343f7 100644 --- a/plugins/xpay/xpay.c +++ b/plugins/xpay/xpay.c @@ -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) */ @@ -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); @@ -1457,6 +1461,11 @@ preapproveinvoice_succeed(struct command *cmd, payment->unique_id = xpay->counter++; payment->private_layer = tal_fmt(payment, "xpay-%"PRIu64, payment->unique_id); + + /* Now unique_id is set, we can log this message */ + if (payment->disable_mpp) + payment_log(payment, LOG_INFORM, "No MPP support: this is going to be hard to pay"); + return populate_private_layer(cmd, payment); } @@ -1551,6 +1560,7 @@ 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; } + payment->disable_mpp = false; } else { struct bolt11 *b11 = bolt11_decode(tmpctx, payment->invstring, @@ -1586,6 +1596,7 @@ 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"); diff --git a/tests/test_xpay.py b/tests/test_xpay.py index a9220e3e7023..24285d86230c 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -621,7 +621,6 @@ def test_xpay_zeroconf(node_factory): l1.rpc.xpay(b12) -@pytest.mark.xfail(strict=True) def test_xpay_no_mpp(node_factory, chainparams): """Suppress mpp, resulting in a single payment part""" l1, l2, l3, l4 = node_factory.get_nodes(4) From e271f35a44cb2741f4a0c861959307295adcf97d Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 20:59:42 +1030 Subject: [PATCH 05/10] offers: don't call create_invoicereq twice. We ignored the second one, but still it's unnecessary. Signed-off-by: Rusty Russell --- plugins/offers_invreq_hook.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/offers_invreq_hook.c b/plugins/offers_invreq_hook.c index 7669ad1e770d..49e3bd70a1ff 100644 --- a/plugins/offers_invreq_hook.c +++ b/plugins/offers_invreq_hook.c @@ -353,7 +353,7 @@ static struct command_result *add_blindedpaths(struct command *cmd, struct invreq *ir) { if (!we_want_blinded_path(cmd->plugin)) - create_invoicereq(cmd, ir); + return create_invoicereq(cmd, ir); return find_best_peer(cmd, OPT_ROUTE_BLINDING, found_best_peer, ir); From f45941759af2355c5b1b813a3842b51277e089c4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 21:00:42 +1030 Subject: [PATCH 06/10] common: if something isn't deprecated yet, it's always ok. Signed-off-by: Rusty Russell --- common/deprecation.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/deprecation.c b/common/deprecation.c index 49012b1b0b12..6adabb7fa5ef 100644 --- a/common/deprecation.c +++ b/common/deprecation.c @@ -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; From 91251082d898394052c204129128440a1acb0e78 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 21:00:43 +1030 Subject: [PATCH 07/10] xpay: in future, don't MPP to pay bolt12 invoices unless invoice explicitly says so. We don't want to enable this yet, since we only just fixed CLN this release! Signed-off-by: Rusty Russell --- doc/developers-guide/deprecations.md | 1 + plugins/xpay/xpay.c | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/developers-guide/deprecations.md b/doc/developers-guide/deprecations.md index ebe6ae1b0e43..e82670369ead 100644 --- a/doc/developers-guide/deprecations.md +++ b/doc/developers-guide/deprecations.md @@ -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. diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c index fca3f68343f7..bf1e290b4d07 100644 --- a/plugins/xpay/xpay.c +++ b/plugins/xpay/xpay.c @@ -1560,7 +1560,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; } - payment->disable_mpp = false; + /* 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, From 068d5b1d89d3dec91e60ed47ec12ded3b08720f7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 21:00:43 +1030 Subject: [PATCH 08/10] offers: correctly advertize MPP in invoice features. 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 --- common/features.c | 5 ++++- plugins/libplugin.c | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/common/features.c b/common/features.c index 41daa405527f..88207aa66a19 100644 --- a/common/features.c +++ b/common/features.c @@ -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[] = { diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 51936be1c1e0..c11be93ca301 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -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); From 1e53aa651b1a5a5febb1de9912c04b6a3b5ff59f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 21:00:43 +1030 Subject: [PATCH 09/10] offers: fix incorrect name for field. 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 --- plugins/offers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/offers.c b/plugins/offers.c index 781367ea0939..254bcea3d3ec 100644 --- a/plugins/offers.c +++ b/plugins/offers.c @@ -1211,7 +1211,7 @@ static void json_add_b12_invoice(struct command *cmd, invoice->invoice_fallbacks); if (invoice->invoice_features) - json_add_hex_talarr(js, "features", invoice->invoice_features); + json_add_hex_talarr(js, "invoice_features", invoice->invoice_features); if (invoice->invoice_node_id) json_add_pubkey(js, "invoice_node_id", invoice->invoice_node_id); From 2710431d478fc52caf6427e313a471028519d0a3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 21:00:43 +1030 Subject: [PATCH 10/10] pytest: test that we indeed do MPP even if not advertized. Signed-off-by: Rusty Russell --- tests/test_xpay.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_xpay.py b/tests/test_xpay.py index 24285d86230c..6b6cdefbf75c 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -649,3 +649,30 @@ def test_xpay_no_mpp(node_factory, chainparams): assert ret['successful_parts'] == 1 assert ret['amount_msat'] == AMOUNT assert ret['amount_sent_msat'] == AMOUNT + AMOUNT // 100000 + 1 + + +def test_xpay_bolt12_no_mpp(node_factory, chainparams): + """We should not (yet!) avoid mpp if BOLT12 invoice doesn't say we should""" + l1, l2, l3, l4 = node_factory.get_nodes(4, opts=[{}, {}, {'dev-force-features': -17}, {}]) + node_factory.join_nodes([l1, l2, l3], wait_for_announce=True) + node_factory.join_nodes([l1, l4, l3], wait_for_announce=True) + + # Amount needs to be enought that it bothers splitting, but not + # so much that it can't pay without mpp. + AMOUNT = 500000000 + + # l2 will advertize mpp, l3 won't. + l2offer = l2.rpc.offer(AMOUNT, 'test_xpay_bolt12_no_mpp') + invl2 = l1.rpc.fetchinvoice(l2offer['bolt12']) + l3offer = l3.rpc.offer(AMOUNT, 'test_xpay_bolt12_no_mpp') + invl3 = l1.rpc.fetchinvoice(l3offer['bolt12']) + + assert l1.rpc.decode(invl2['invoice'])['invoice_features'] == "020000" + assert l1.rpc.decode(invl3['invoice'])['invoice_features'] == "" + + # This will MPP ANYWAY, even though MPP is not specified! + ret = l1.rpc.xpay(invl3['invoice']) + assert ret['failed_parts'] == 0 + assert ret['successful_parts'] == 2 + assert ret['amount_msat'] == AMOUNT + assert ret['amount_sent_msat'] == AMOUNT + AMOUNT // 100000 + 1