From 31a02dfb1dcbc495f0ec93fc9dbf25bf453024fa Mon Sep 17 00:00:00 2001 From: Storm Knight Date: Fri, 27 Mar 2026 10:33:18 -0400 Subject: [PATCH] askrene: deprecate auto.no_mpp_support layer, use maxparts=1 instead The auto.no_mpp_support layer was used to force single-path payments for destinations that don't support MPP. Since maxparts=1 already switches to the single-path algorithm, the layer is redundant. Changes: - askrene: remove remove_small_channel_layer() and auto.no_mpp_support handling from layer validation, creation, and algorithm override - xpay: when disable_mpp is set, send maxparts=1 to getroutes instead of adding the auto.no_mpp_support layer - tests: update test_getroutes_single_path, test_excessive_fee_cost, and test_impossible_payment to use maxparts=1 instead of the layer - docs: update getroutes.json to describe three automatic layers and point to maxparts=1 for single-path payments Fixes #8871 --- doc/schemas/getroutes.json | 2 +- plugins/askrene/askrene.c | 51 +------------------------------------- plugins/xpay/xpay.c | 5 ++-- tests/test_askrene.py | 32 ++++++++++++++---------- 4 files changed, 24 insertions(+), 66 deletions(-) diff --git a/doc/schemas/getroutes.json b/doc/schemas/getroutes.json index 04f502e2890b..87b83f07d33d 100644 --- a/doc/schemas/getroutes.json +++ b/doc/schemas/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 four 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. *auto.no_mpp_support* forces getroutes to return a single path solution which is useful for payments for which MPP is not supported. And *auto.include_fees* that fixes the send amount and deducts fee from there, ie. the receiver pays for fees instead of the sender." + "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.include_fees* that fixes the send amount and deducts fee from there, ie. the receiver pays for fees instead of the sender. For single-path payments (where the destination does not support MPP), use the *maxparts* parameter set to 1 instead of a layer." ], "categories": [ "readonly" diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index 54219e5ea39d..3b7ca62b1366 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -114,7 +114,6 @@ 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") || streq((*arr)[i], "auto.include_fees")) continue; @@ -252,44 +251,6 @@ 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; -} - PRINTF_FMT(4, 5) static const char *cmd_log(const tal_t *ctx, struct command *cmd, @@ -374,9 +335,6 @@ static const struct layer **apply_layers(const tal_t *ctx, if (streq(layernames[i], "auto.localchans")) { cmd_log(tmpctx, cmd, LOG_DBG, "Adding auto.localchans"); l = local_layer; - } else if (streq(layernames[i], "auto.no_mpp_support")) { - cmd_log(tmpctx, cmd, LOG_DBG, "Adding auto.no_mpp_support, sorry"); - l = remove_small_channel_layer(layernames, askrene, amount, localmods); } else if (streq(layernames[i], "auto.include_fees")) { cmd_log(tmpctx, cmd, LOG_DBG, "Adding auto.include_fees"); /* This layer takes effect when converting flows @@ -605,14 +563,7 @@ static struct command_result *do_getroutes(struct command *cmd, goto fail; } - /* auto.no_mpp_support layer overrides any choice of algorithm. */ - if (have_layer(info->layers, "auto.no_mpp_support") && - info->dev_algo != ALGO_SINGLE_PATH) { - info->dev_algo = ALGO_SINGLE_PATH; - cmd_log(tmpctx, cmd, LOG_DBG, - "Layer no_mpp_support is active we switch to a " - "single path algorithm."); - } + /* maxparts=1 means the destination doesn't support MPP. */ if (info->maxparts == 1 && info->dev_algo != ALGO_SINGLE_PATH) { info->dev_algo = ALGO_SINGLE_PATH; diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c index e05e2df3f85e..8899adb175df 100644 --- a/plugins/xpay/xpay.c +++ b/plugins/xpay/xpay.c @@ -1467,8 +1467,6 @@ 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); @@ -1477,6 +1475,9 @@ static struct command_result *getroutes_for(struct command *aux_cmd, size_t count_pending = count_current_attempts(payment); assert(payment->maxparts > count_pending); json_add_u32(req->js, "maxparts", payment->maxparts - count_pending); + } else if (payment->disable_mpp) { + /* Destination doesn't support MPP: force single path. */ + json_add_u32(req->js, "maxparts", 1); } return send_payment_req(aux_cmd, payment, req); diff --git a/tests/test_askrene.py b/tests/test_askrene.py index 513e9ec7197f..27ac4567af95 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -661,14 +661,18 @@ def check_getroute_paths(node, paths, layers=[], maxfee_msat=1000, - final_cltv=99): + final_cltv=99, + maxparts=0): """Check that routes are as expected in result""" - getroutes = node.rpc.getroutes(source=source, - destination=destination, - amount_msat=amount_msat, - layers=layers, - maxfee_msat=maxfee_msat, - final_cltv=final_cltv) + kwargs = dict(source=source, + destination=destination, + amount_msat=amount_msat, + layers=layers, + maxfee_msat=maxfee_msat, + final_cltv=final_cltv) + if maxparts: + kwargs["maxparts"] = maxparts + getroutes = node.rpc.getroutes(**kwargs) assert getroutes['probability_ppm'] <= 1000000 # Total delivered should be amount we told it to send. @@ -841,7 +845,7 @@ def test_getroutes_single_path(node_factory): source=nodemap[1], destination=nodemap[2], amount_msat=10000001, - layers=["auto.no_mpp_support"], + maxparts=1, maxfee_msat=1000, final_cltv=99, ) @@ -862,7 +866,7 @@ def test_getroutes_single_path(node_factory): } ] ], - layers=["auto.no_mpp_support"], + maxparts=1, ) # To be able to route this amount two parts are needed, therefore a single @@ -873,7 +877,7 @@ def test_getroutes_single_path(node_factory): source=nodemap[0], destination=nodemap[2], amount_msat=10000001, - layers=["auto.no_mpp_support"], + maxparts=1, maxfee_msat=1000, final_cltv=99, ) @@ -900,7 +904,7 @@ def test_getroutes_single_path(node_factory): }, ] ], - layers=["auto.no_mpp_support"], + maxparts=1, ) @@ -2097,7 +2101,8 @@ def test_excessive_fee_cost(node_factory): source=l1.info["id"], destination=node1, amount_msat=one_btc // 2, - layers=["mylayer", "auto.no_mpp_support"], + layers=["mylayer"], + maxparts=1, maxfee_msat=1000, final_cltv=5, ) @@ -2581,7 +2586,8 @@ def test_impossible_payment(node_factory): source=node1, destination=node3, amount_msat=pay_amt, - layers=["mylayer", "auto.no_mpp_support"], + layers=["mylayer"], + maxparts=1, maxfee_msat=2 * pay_amt, final_cltv=5, )