Skip to content

askrene: deprecate auto.no_mpp_support layer, use maxparts=1 instead #8998

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.

Sign up for GitHub

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

Open
ThomsenDrake wants to merge 1 commit into
base: master
Choose a base branch
from
+24 −66
Open

askrene: deprecate auto.no_mpp_support layer, use maxparts=1 instead #8998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/schemas/getroutes.json
View file
Open in desktop
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 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"
Expand Down
51 changes: 1 addition & 50 deletions plugins/askrene/askrene.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions plugins/xpay/xpay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
32 changes: 19 additions & 13 deletions tests/test_askrene.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
)
Expand All @@ -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
Expand All @@ -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,
)
Expand All @@ -900,7 +904,7 @@ def test_getroutes_single_path(node_factory):
},
]
],
layers=["auto.no_mpp_support"],
maxparts=1,
)


Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)