From 461eb2eddb9d04e7f95d8080bd02a79e2bb24823 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 14 Feb 2025 15:21:48 +1030 Subject: [PATCH] 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 b711937e7..aedfac9a3 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 ee3f913ab..cfa2ac3a3 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 57631bb5a..958859fa4 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 @@ too_expensive: 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 bd29d1830..6339510a5 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 2a1b8fcf0..f8100e766 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 4a3685a78..7404d4664 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"""