Skip to content

Commit a55255e

Browse files
committed
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 <[email protected]>
1 parent b51120b commit a55255e

File tree

6 files changed

+100
-8
lines changed

6 files changed

+100
-8
lines changed

contrib/msggen/msggen/schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16046,7 +16046,7 @@
1604616046
"",
1604716047
"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).",
1604816048
"",
16049-
"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."
16049+
"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."
1605016050
],
1605116051
"categories": [
1605216052
"readonly"

doc/schemas/lightning-getroutes.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"",
1212
"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).",
1313
"",
14-
"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."
14+
"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."
1515
],
1616
"categories": [
1717
"readonly"

plugins/askrene/askrene.c

Lines changed: 47 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ static struct command_result *param_layer_names(struct command *cmd,
8282

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

213+
/* We're going to abuse MCF, and take the largest flow it gives and ram everything
214+
* through it. This is more effective if there's at least a *chance* that can handle
215+
* the full amount.
216+
*
217+
* It's far from perfect, but I have very little sympathy: if you want
218+
* to receive amounts reliably, enable MPP.
219+
*/
220+
static struct layer *remove_small_channel_layer(const tal_t *ctx,
221+
struct askrene *askrene,
222+
struct amount_msat min_amount,
223+
struct gossmap_localmods *localmods)
224+
{
225+
struct layer *layer = new_temp_layer(ctx, askrene, "auto.no_mpp_support");
226+
struct gossmap *gossmap = askrene->gossmap;
227+
struct gossmap_chan *c;
228+
229+
/* We apply this so we see any created channels */
230+
gossmap_apply_localmods(gossmap, localmods);
231+
232+
for (c = gossmap_first_chan(gossmap); c; c = gossmap_next_chan(gossmap, c)) {
233+
struct short_channel_id_dir scidd;
234+
if (amount_msat_greater_eq(gossmap_chan_get_capacity(gossmap, c),
235+
min_amount))
236+
continue;
237+
238+
scidd.scid = gossmap_chan_scid(gossmap, c);
239+
/* Layer will disable this in both directions */
240+
for (scidd.dir = 0; scidd.dir < 2; scidd.dir++) {
241+
const bool enabled = false;
242+
layer_add_update_channel(layer, &scidd, &enabled,
243+
NULL, NULL, NULL, NULL, NULL);
244+
}
245+
}
246+
gossmap_remove_localmods(gossmap, localmods);
247+
248+
return layer;
249+
}
250+
212251
struct amount_msat get_additional_per_htlc_cost(const struct route_query *rq,
213252
const struct short_channel_id_dir *scidd)
214253
{
@@ -321,6 +360,7 @@ static const char *get_routes(const tal_t *ctx,
321360
const char **layers,
322361
struct gossmap_localmods *localmods,
323362
const struct layer *local_layer,
363+
bool single_path,
324364
struct route ***routes,
325365
struct amount_msat **amounts,
326366
const struct additional_cost_htable *additional_costs,
@@ -355,8 +395,10 @@ static const char *get_routes(const tal_t *ctx,
355395
if (streq(layers[i], "auto.localchans")) {
356396
plugin_log(rq->plugin, LOG_DBG, "Adding auto.localchans");
357397
l = local_layer;
398+
} else if (streq(layers[i], "auto.no_mpp_support")) {
399+
plugin_log(rq->plugin, LOG_DBG, "Adding auto.no_mpp_support, sorry");
400+
l = remove_small_channel_layer(layers, askrene, amount, localmods);
358401
} else {
359-
/* Handled below, after other layers */
360402
assert(streq(layers[i], "auto.sourcefree"));
361403
plugin_log(rq->plugin, LOG_DBG, "Adding auto.sourcefree");
362404
l = source_free_layer(layers, askrene, source, localmods);
@@ -406,7 +448,7 @@ static const char *get_routes(const tal_t *ctx,
406448
/* First up, don't care about fees (well, just enough to tiebreak!) */
407449
mu = 1;
408450
flows = minflow(rq, rq, srcnode, dstnode, amount,
409-
mu, delay_feefactor);
451+
mu, delay_feefactor, single_path);
410452
if (!flows) {
411453
ret = explain_failure(ctx, rq, srcnode, dstnode, amount);
412454
goto fail;
@@ -419,7 +461,7 @@ static const char *get_routes(const tal_t *ctx,
419461
"The worst flow delay is %"PRIu64" (> %i), retrying with delay_feefactor %f...",
420462
flows_worst_delay(flows), maxdelay - finalcltv, delay_feefactor);
421463
flows = minflow(rq, rq, srcnode, dstnode, amount,
422-
mu, delay_feefactor);
464+
mu, delay_feefactor, single_path);
423465
if (!flows || delay_feefactor > 10) {
424466
ret = rq_log(ctx, rq, LOG_UNUSUAL,
425467
"Could not find route without excessive delays");
@@ -442,7 +484,7 @@ static const char *get_routes(const tal_t *ctx,
442484
fmt_amount_msat(tmpctx, maxfee),
443485
mu);
444486
new_flows = minflow(rq, rq, srcnode, dstnode, amount,
445-
mu > 100 ? 100 : mu, delay_feefactor);
487+
mu > 100 ? 100 : mu, delay_feefactor, single_path);
446488
if (!flows || mu >= 100) {
447489
ret = rq_log(ctx, rq, LOG_UNUSUAL,
448490
"Could not find route without excessive cost");
@@ -616,6 +658,7 @@ static struct command_result *do_getroutes(struct command *cmd,
616658
info->source, info->dest,
617659
*info->amount, *info->maxfee, *info->finalcltv,
618660
*info->maxdelay, info->layers, localmods, info->local_layer,
661+
have_layer(info->layers, "auto.no_mpp_support"),
619662
&routes, &amounts, info->additional_costs, &probability);
620663
if (err)
621664
return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "%s", err);

plugins/askrene/mcf.c

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -961,7 +961,8 @@ struct flow **minflow(const tal_t *ctx,
961961
const struct gossmap_node *target,
962962
struct amount_msat amount,
963963
u32 mu,
964-
double delay_feefactor)
964+
double delay_feefactor,
965+
bool single_part)
965966
{
966967
struct flow **flow_paths;
967968
/* We allocate everything off this, and free it at the end,
@@ -1044,6 +1045,31 @@ struct flow **minflow(const tal_t *ctx,
10441045
goto fail;
10451046
}
10461047
tal_free(working_ctx);
1048+
1049+
/* This is dumb, but if you don't support MPP you don't deserve any
1050+
* better. Pile it into the largest part if not already. */
1051+
if (single_part) {
1052+
struct flow *best = flow_paths[0];
1053+
for (size_t i = 1; i < tal_count(flow_paths); i++) {
1054+
if (amount_msat_greater(flow_paths[i]->delivers, best->delivers))
1055+
best = flow_paths[i];
1056+
}
1057+
for (size_t i = 0; i < tal_count(flow_paths); i++) {
1058+
if (flow_paths[i] == best)
1059+
continue;
1060+
if (!amount_msat_accumulate(&best->delivers,
1061+
flow_paths[i]->delivers)) {
1062+
rq_log(tmpctx, rq, LOG_BROKEN,
1063+
"%s: failed to extract accumulate flow paths %s+%s",
1064+
__func__,
1065+
fmt_amount_msat(tmpctx, best->delivers),
1066+
fmt_amount_msat(tmpctx, flow_paths[i]->delivers));
1067+
goto fail;
1068+
}
1069+
}
1070+
flow_paths[0] = best;
1071+
tal_resize(&flow_paths, 1);
1072+
}
10471073
return flow_paths;
10481074

10491075
fail:

plugins/askrene/mcf.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ struct route_query;
1616
* @target: the target to pay
1717
* @amount: the amount we want to reach @target
1818
* @mu: 0 = corresponds to only probabilities, 100 corresponds to only fee.
19+
* @delay_feefactor: convert 1 block delay into msat.
20+
* @single_part: don't do MCF at all, just create a single flow.
1921
*
2022
* @delay_feefactor converts 1 block delay into msat, as if it were an additional
2123
* 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,
2931
const struct gossmap_node *target,
3032
struct amount_msat amount,
3133
u32 mu,
32-
double delay_feefactor);
34+
double delay_feefactor,
35+
bool single_part);
3336

3437
/* To sanity check: this is the approximation mcf uses for the cost
3538
* of each channel. */

tests/test_askrene.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,26 @@ def test_getroutes(node_factory):
551551
'amount_msat': 5500005,
552552
'delay': 99 + 6}]])
553553

554+
# We realize that this is impossible in a single path:
555+
with pytest.raises(RpcError, match="The shortest path is 0x2x1, but 0x2x1/1 marked disabled by layer auto.no_mpp_support."):
556+
l1.rpc.getroutes(source=nodemap[0],
557+
destination=nodemap[2],
558+
amount_msat=10000000,
559+
layers=['auto.no_mpp_support'],
560+
maxfee_msat=1000,
561+
final_cltv=99)
562+
563+
# But this will work.
564+
check_getroute_paths(l1,
565+
nodemap[0],
566+
nodemap[2],
567+
9000000,
568+
[[{'short_channel_id_dir': '0x2x3/1',
569+
'next_node_id': nodemap[2],
570+
'amount_msat': 9000009,
571+
'delay': 99 + 6}]],
572+
layers=['auto.no_mpp_support'])
573+
554574

555575
def test_getroutes_fee_fallback(node_factory):
556576
"""Test getroutes call takes into account fees, if excessive"""

0 commit comments

Comments
 (0)