From 09f61fa03cd4691a61a4071b427cf89cf297c6f5 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Wed, 9 Jul 2025 15:48:44 +0100 Subject: [PATCH 1/8] askrene: add new functions to refine flows Try a new way to refine flows, ie. reduce excess due to MCF accuracy and HTLC max constraints after hop amounts are computed with fees included. Changelog-None. Signed-off-by: Lagrang3 --- plugins/askrene/refine.c | 223 +++++++++++++++++++++++++++++++++++++++ plugins/askrene/refine.h | 5 + 2 files changed, 228 insertions(+) diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index 51f030ade8e8..a44a9cd0931c 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -1,4 +1,5 @@ #include "config.h" +#include #include #include #include @@ -6,6 +7,14 @@ #include #include +/* Channel data for fast retrieval. */ +struct channel_data { + struct amount_msat htlc_min, htlc_max, liquidity_max; + u32 fee_base_msat, fee_proportional_millionths; + struct short_channel_id_dir scidd; + u32 idx; +}; + /* We (ab)use the reservation system to place temporary reservations * on channels while we are refining each flow. This has the effect * of making flows aware of each other. */ @@ -604,3 +613,217 @@ refine_with_fees_and_limits(const tal_t *ctx, tal_free(reservations); return ret; } + +/* Cache channel data along the path used by this flow. */ +static struct channel_data *new_channel_path_cache(const tal_t *ctx, + struct route_query *rq, + struct flow *flow) +{ + const size_t pathlen = tal_count(flow->path); + struct channel_data *path = tal_arr(ctx, struct channel_data, pathlen); + + for (size_t i = 0; i < pathlen; i++) { + /* knowledge on liquidity bounds */ + struct amount_msat known_min, known_max; + const struct half_chan *h = flow_edge(flow, i); + struct short_channel_id_dir scidd; + + get_scidd(rq->gossmap, flow, i, &scidd); + get_constraints(rq, flow->path[i], flow->dirs[i], &known_min, + &known_max); + + path[i].htlc_min = get_chan_htlc_min(rq, flow->path[i], &scidd); + path[i].htlc_max = get_chan_htlc_max(rq, flow->path[i], &scidd); + path[i].fee_base_msat = h->base_fee; + path[i].fee_proportional_millionths = h->proportional_fee; + path[i].liquidity_max = known_max; + path[i].scidd = scidd; + path[i].idx = scidd.dir + + 2 * gossmap_chan_idx(rq->gossmap, flow->path[i]); + } + return path; +} + +/* Cache channel data along multiple paths. */ +static struct channel_data **new_channel_mpp_cache(const tal_t *ctx, + struct route_query *rq, + struct flow **flows) +{ + const size_t npaths = tal_count(flows); + struct channel_data **paths = + tal_arr(ctx, struct channel_data *, npaths); + for (size_t i = 0; i < npaths; i++) { + paths[i] = new_channel_path_cache(paths, rq, flows[i]); + } + return paths; +} + +/* Reverse order: bigger first */ +static int revcmp_flows(const size_t *a, const size_t *b, struct flow **flows) +{ + if (amount_msat_eq(flows[*a]->delivers, flows[*b]->delivers)) + return 0; + if (amount_msat_greater(flows[*a]->delivers, flows[*b]->delivers)) + return -1; + return 1; +} + +// TODO: unit test: +// -> make a path +// -> compute x = path_max_deliverable +// -> check that htlc_max are all satisfied +// -> check that (x+1) at least one htlc_max is violated +/* Given the channel constraints, return the maximum amount that can be + * delivered. */ +static struct amount_msat path_max_deliverable(struct channel_data *path) +{ + struct amount_msat deliver = AMOUNT_MSAT(-1); + for (size_t i = 0; i < tal_count(path); i++) { + deliver = + amount_msat_sub_fee(deliver, path[i].fee_base_msat, + path[i].fee_proportional_millionths); + deliver = amount_msat_min(deliver, path[i].htlc_max); + deliver = amount_msat_min(deliver, path[i].liquidity_max); + } + return deliver; +} + +static struct amount_msat sum_all_deliver(struct flow **flows, + size_t *flows_index) +{ + struct amount_msat all_deliver = AMOUNT_MSAT(0); + for (size_t i = 0; i < tal_count(flows_index); i++) { + if (!amount_msat_accumulate(&all_deliver, + flows[flows_index[i]]->delivers)) + abort(); + } + return all_deliver; +} + +/* It reduces the amount of the flows and/or removes some flows in order to + * deliver no more than max_deliver. It will leave at least one flow. + * Returns the total delivery amount. */ +static struct amount_msat remove_excess(struct flow **flows, + size_t **flows_index, + struct amount_msat max_deliver) +{ + if (tal_count(flows) == 0) + return AMOUNT_MSAT(0); + + struct amount_msat all_deliver, excess; + all_deliver = sum_all_deliver(flows, *flows_index); + + /* early exit: there is no excess */ + if (!amount_msat_sub(&excess, all_deliver, max_deliver) || + amount_msat_is_zero(excess)) + return all_deliver; + + asort(*flows_index, tal_count(*flows_index), revcmp_flows, flows); + + /* Remove the smaller parts if they deliver less than the + * excess. */ + for (int i = tal_count(*flows_index) - 1; i >= 0; i--) { + if (!amount_msat_sub(&excess, excess, + flows[(*flows_index)[i]]->delivers)) + break; + if (!amount_msat_sub(&all_deliver, all_deliver, + flows[(*flows_index)[i]]->delivers)) + abort(); + tal_arr_remove(flows_index, i); + } + + /* If we still have some excess, remove it from the + * current flows in the same proportion every flow contributes to the + * total. */ + struct amount_msat old_excess = excess; + struct amount_msat old_deliver = all_deliver; + for (size_t i = 0; i < tal_count(*flows_index); i++) { + double fraction = amount_msat_ratio( + flows[(*flows_index)[i]]->delivers, old_deliver); + struct amount_msat remove; + + if (!amount_msat_scale(&remove, old_excess, fraction)) + abort(); + + /* rounding errors: don't remove more than excess */ + remove = amount_msat_min(remove, excess); + + if (!amount_msat_sub(&excess, excess, remove)) + abort(); + + if (!amount_msat_sub(&all_deliver, all_deliver, remove) || + !amount_msat_sub(&flows[(*flows_index)[i]]->delivers, + flows[(*flows_index)[i]]->delivers, + remove)) + abort(); + } + + /* any rounding error left, take it from the first */ + assert(tal_count(*flows_index) > 0); + if (!amount_msat_sub(&all_deliver, all_deliver, excess) || + !amount_msat_sub(&flows[(*flows_index)[0]]->delivers, + flows[(*flows_index)[0]]->delivers, excess)) + abort(); + return all_deliver; +} + +/* FIXME: on failure return error message */ +const char *refine_flows(const tal_t *ctx, struct route_query *rq, + struct amount_msat deliver, struct flow ***flows) +{ + const tal_t *working_ctx = tal(ctx, tal_t); + struct amount_msat *max_deliverable; + struct channel_data **channel_mpp_cache; + size_t *flows_index; + + /* we might need to access this data multiple times, so we cache + * it */ + channel_mpp_cache = new_channel_mpp_cache(working_ctx, rq, *flows); + max_deliverable = tal_arrz(working_ctx, struct amount_msat, + tal_count(channel_mpp_cache)); + flows_index = tal_arrz(working_ctx, size_t, tal_count(*flows)); + for (size_t i = 0; i < tal_count(channel_mpp_cache); i++) { + // FIXME: does path_max_deliverable work for a single + // channel with 0 fees? + max_deliverable[i] = path_max_deliverable(channel_mpp_cache[i]); + /* We use an array of indexes to keep track of the order + * of the flows. Likewise flows can be removed by simply + * shrinking the flows_index array. */ + flows_index[i] = i; + } + + /* do not deliver more than HTLC_MAX allow us */ + for (size_t i = 0; i < tal_count(flows_index); i++) { + (*flows)[flows_index[i]]->delivers = + amount_msat_min((*flows)[flows_index[i]]->delivers, + max_deliverable[flows_index[i]]); + } + + /* remove excess from MCF granularity if any */ + remove_excess(*flows, &flows_index, deliver); + + /* remove 0 amount flows if any */ + asort(flows_index, tal_count(flows_index), revcmp_flows, *flows); + for (int i = tal_count(flows_index) - 1; i >= 0; i--) { + if (!amount_msat_is_zero((*flows)[flows_index[i]]->delivers)) + break; + tal_arr_remove(&flows_index, i); + } + + /* finally write the remaining flows */ + struct flow **tmp_flows = tal_arr(working_ctx, struct flow *, 0); + for (size_t i = 0; i < tal_count(flows_index); i++) { + tal_arr_expand(&tmp_flows, (*flows)[flows_index[i]]); + (*flows)[flows_index[i]] = NULL; + } + for (size_t i = 0; i < tal_count(*flows); i++) { + (*flows)[i] = tal_free((*flows)[i]); + } + tal_resize(flows, 0); + for (size_t i = 0; i < tal_count(tmp_flows); i++) { + tal_arr_expand(flows, tmp_flows[i]); + } + + tal_free(working_ctx); + return NULL; +} diff --git a/plugins/askrene/refine.h b/plugins/askrene/refine.h index 66befec9b1e1..8757f73618c5 100644 --- a/plugins/askrene/refine.h +++ b/plugins/askrene/refine.h @@ -23,4 +23,9 @@ refine_with_fees_and_limits(const tal_t *ctx, struct amount_msat deliver, struct flow ***flows, double *flowset_probability); + +/* Modify flows to meet HTLC min/max requirements. + * It takes into account the exact value of the fees expected at each hop. */ +const char *refine_flows(const tal_t *ctx, struct route_query *rq, + struct amount_msat deliver, struct flow ***flows); #endif /* LIGHTNING_PLUGINS_ASKRENE_REFINE_H */ From bcffc076c5067f144e103f88a699f598b209d21f Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Wed, 9 Jul 2025 16:02:13 +0100 Subject: [PATCH 2/8] askrene: refine: remove flows with HTLC min faults Changelog-None Signed-off-by: Lagrang3 --- plugins/askrene/refine.c | 63 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index a44a9cd0931c..e93d095363e3 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -688,6 +688,53 @@ static struct amount_msat path_max_deliverable(struct channel_data *path) return deliver; } +// TODO: unit test: +// -> make a path +// -> compute x = path_min_deliverable +// -> check that htlc_min are all satisfied +// -> check that (x-1) at least one htlc_min is violated +/* The least amount that we can deliver at the destination such that when one + * computes the hop amounts backwards the htlc_min are always met. */ +static struct amount_msat path_min_deliverable(struct channel_data *path) +{ + struct amount_msat least_send = AMOUNT_MSAT(1); + const size_t pathlen = tal_count(path); + for (size_t i = pathlen - 1; i < pathlen; i--) { + least_send = amount_msat_max(least_send, path[i].htlc_min); + if (!amount_msat_add_fee(&least_send, path[i].fee_base_msat, + path[i].fee_proportional_millionths)) + abort(); + } + /* least_send: is the least amount we can send in order to deliver at + * least 1 msat at the destination. */ + struct amount_msat least_destination = least_send; + for (size_t i = 0; i < pathlen; i++) { + struct amount_msat in_value = least_destination; + struct amount_msat out_value = + amount_msat_sub_fee(in_value, path[i].fee_base_msat, + path[i].fee_proportional_millionths); + assert(amount_msat_greater_eq(out_value, path[i].htlc_min)); + struct amount_msat x = out_value; + if (!amount_msat_add_fee(&x, path[i].fee_base_msat, + path[i].fee_proportional_millionths)) + abort(); + /* if the in_value computed from the out_value is smaller than + * it should, then we add 1msat */ + if (amount_msat_less(x, in_value) && + !amount_msat_accumulate(&out_value, AMOUNT_MSAT(1))) + abort(); + /* check conditions */ + assert(amount_msat_greater_eq(out_value, path[i].htlc_min)); + x = out_value; + assert( + amount_msat_add_fee(&x, path[i].fee_base_msat, + path[i].fee_proportional_millionths) && + amount_msat_greater_eq(x, in_value)); + least_destination = out_value; + } + return least_destination; +} + static struct amount_msat sum_all_deliver(struct flow **flows, size_t *flows_index) { @@ -773,6 +820,7 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, { const tal_t *working_ctx = tal(ctx, tal_t); struct amount_msat *max_deliverable; + struct amount_msat *min_deliverable; struct channel_data **channel_mpp_cache; size_t *flows_index; @@ -781,11 +829,14 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, channel_mpp_cache = new_channel_mpp_cache(working_ctx, rq, *flows); max_deliverable = tal_arrz(working_ctx, struct amount_msat, tal_count(channel_mpp_cache)); + min_deliverable = tal_arrz(working_ctx, struct amount_msat, + tal_count(channel_mpp_cache)); flows_index = tal_arrz(working_ctx, size_t, tal_count(*flows)); for (size_t i = 0; i < tal_count(channel_mpp_cache); i++) { // FIXME: does path_max_deliverable work for a single // channel with 0 fees? max_deliverable[i] = path_max_deliverable(channel_mpp_cache[i]); + min_deliverable[i] = path_min_deliverable(channel_mpp_cache[i]); /* We use an array of indexes to keep track of the order * of the flows. Likewise flows can be removed by simply * shrinking the flows_index array. */ @@ -802,6 +853,18 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, /* remove excess from MCF granularity if any */ remove_excess(*flows, &flows_index, deliver); + /* detect htlc_min violations */ + for (size_t i = 0; i < tal_count(flows_index);) { + size_t k = flows_index[i]; + if (amount_msat_greater_eq((*flows)[k]->delivers, + min_deliverable[k])) { + i++; + continue; + } + /* htlc_min is not met for this flow */ + tal_arr_remove(&flows_index, i); + } + /* remove 0 amount flows if any */ asort(flows_index, tal_count(flows_index), revcmp_flows, *flows); for (int i = tal_count(flows_index) - 1; i >= 0; i--) { From 7e7173ce659d509ac9d9d4e1cbbaff1c8371137e Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Wed, 9 Jul 2025 16:05:11 +0100 Subject: [PATCH 3/8] askrene: refine: disable HTLC min violations Disable channels with HTLC min violations so that we don't hit them twice when computing routes. Changelog-None Signed-off-by: Lagrang3 --- plugins/askrene/askrene.c | 7 +++++++ plugins/askrene/askrene.h | 4 ++++ plugins/askrene/mcf.c | 21 ++++++++++++++------ plugins/askrene/refine.c | 41 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 6 deletions(-) diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index 31126f82c350..ad6bd088a7e7 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -564,6 +564,13 @@ static struct command_result *do_getroutes(struct command *cmd, /* we temporarily apply localmods */ gossmap_apply_localmods(askrene->gossmap, localmods); + /* I want to be able to disable channels while working on this query. + * Layers are for user interaction and cannot be used for this purpose. + */ + rq->disabled_chans = + tal_arrz(rq, bitmap, + 2 * BITMAP_NWORDS(gossmap_max_chan_idx(askrene->gossmap))); + /* localmods can add channels, so we need to allocate biases array * *afterwards* */ rq->biases = diff --git a/plugins/askrene/askrene.h b/plugins/askrene/askrene.h index 8688835e975e..7f1352f6f5f7 100644 --- a/plugins/askrene/askrene.h +++ b/plugins/askrene/askrene.h @@ -2,6 +2,7 @@ #define LIGHTNING_PLUGINS_ASKRENE_ASKRENE_H #include "config.h" #include +#include #include #include #include @@ -60,6 +61,9 @@ struct route_query { /* Additional per-htlc cost for local channels */ const struct additional_cost_htable *additional_costs; + + /* channels we disable during computation to meet constraints */ + bitmap *disabled_chans; }; /* Given a gossmap channel, get the current known min/max */ diff --git a/plugins/askrene/mcf.c b/plugins/askrene/mcf.c index 696b810c7574..6d50cb8bdfe7 100644 --- a/plugins/askrene/mcf.c +++ b/plugins/askrene/mcf.c @@ -319,6 +319,15 @@ static void set_capacity(s64 *capacity, u64 value, u64 *cap_on_capacity) *cap_on_capacity -= *capacity; } +/* Helper to check whether a channel is available */ +static bool channel_is_available(const struct route_query *rq, + const struct gossmap_chan *chan, const int dir) +{ + const u32 c_idx = gossmap_chan_idx(rq->gossmap, chan); + return gossmap_chan_set(chan, dir) && chan->half[dir].enabled && + !bitmap_test_bit(rq->disabled_chans, c_idx * 2 + dir); +} + /* FIXME: unit test this */ /* The probability of forwarding a payment amount given a high and low liquidity * bounds. @@ -568,7 +577,7 @@ static void init_linear_network(const tal_t *ctx, const struct gossmap_chan *c = gossmap_nth_chan(gossmap, node, j, &half); - if (!gossmap_chan_set(c, half) || !c->half[half].enabled) + if (!channel_is_available(params->rq, c, half)) continue; /* If a channel insists on more than our total, remove it */ @@ -644,7 +653,7 @@ struct chan_flow * */ static struct node find_path_or_cycle( const tal_t *working_ctx, - const struct gossmap *gossmap, + const struct route_query *rq, const struct chan_flow *chan_flow, const struct node source, const s64 *balance, @@ -653,6 +662,7 @@ static struct node find_path_or_cycle( int *prev_dir, u32 *prev_idx) { + const struct gossmap *gossmap = rq->gossmap; const size_t max_num_nodes = gossmap_max_node_idx(gossmap); bitmap *visited = tal_arrz(working_ctx, bitmap, BITMAP_NWORDS(max_num_nodes)); @@ -671,7 +681,7 @@ static struct node find_path_or_cycle( const struct gossmap_chan *c = gossmap_nth_chan(gossmap, cur, i, &dir); - if (!gossmap_chan_set(c, dir) || !c->half[dir].enabled) + if (!channel_is_available(rq, c, dir)) continue; const u32 c_idx = gossmap_chan_idx(gossmap, c); @@ -877,7 +887,7 @@ get_flow_paths(const tal_t *ctx, while (balance[source.idx] < 0) { prev_chan[source.idx] = NULL; struct node sink = find_path_or_cycle( - working_ctx, params->rq->gossmap, chan_flow, source, + working_ctx, params->rq, chan_flow, source, balance, prev_chan, prev_dir, prev_idx); if (balance[sink.idx] > 0) @@ -1107,8 +1117,7 @@ static void init_linear_network_single_path( gossmap_nth_chan(gossmap, node, j, &half); struct amount_msat mincap, maxcap; - if (!gossmap_chan_set(c, half) || - !c->half[half].enabled) + if (!channel_is_available(params->rq, c, half)) continue; /* If a channel cannot forward the total amount we don't diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index e93d095363e3..97472443dfab 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -735,6 +735,38 @@ static struct amount_msat path_min_deliverable(struct channel_data *path) return least_destination; } +static const char * +remove_htlc_min_violations(const tal_t *ctx, struct route_query *rq, + const struct flow *flow, + const struct channel_data *channels) +{ + const char *error_message = NULL; + struct amount_msat msat = flow->delivers; + for (size_t i = tal_count(flow->path) - 1; i < tal_count(flow->path); + i--) { + if (amount_msat_less(msat, channels[i].htlc_min)) { + rq_log( + ctx, rq, LOG_INFORM, + "Sending %s across %s would violate htlc_min " + "(~%s), disabling this channel", + fmt_amount_msat(ctx, msat), + fmt_short_channel_id_dir(ctx, &channels[i].scidd), + fmt_amount_msat(ctx, channels[i].htlc_min)); + bitmap_set_bit(rq->disabled_chans, channels[i].idx); + break; + } + if (!amount_msat_add_fee( + &msat, channels[i].fee_base_msat, + channels[i].fee_proportional_millionths)) { + error_message = + rq_log(ctx, rq, LOG_BROKEN, + "%s: Adding fee to amount", __func__); + break; + } + } + return error_message; +} + static struct amount_msat sum_all_deliver(struct flow **flows, size_t *flows_index) { @@ -819,6 +851,7 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, struct amount_msat deliver, struct flow ***flows) { const tal_t *working_ctx = tal(ctx, tal_t); + const char *error_message = NULL; struct amount_msat *max_deliverable; struct amount_msat *min_deliverable; struct channel_data **channel_mpp_cache; @@ -863,6 +896,10 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, } /* htlc_min is not met for this flow */ tal_arr_remove(&flows_index, i); + error_message = remove_htlc_min_violations( + working_ctx, rq, (*flows)[k], channel_mpp_cache[k]); + if (error_message) + goto fail; } /* remove 0 amount flows if any */ @@ -889,4 +926,8 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, tal_free(working_ctx); return NULL; + +fail: + tal_free(working_ctx); + return error_message; } From dacf1c273a8dfe19be8d0b5b92874d5220622ba9 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Wed, 9 Jul 2025 19:19:52 +0100 Subject: [PATCH 4/8] askrene: refine: expose some of the refine API that can be useful for us in the mcf.c main loop. Changelog-None Signed-off-by: Lagrang3 --- plugins/askrene/refine.c | 145 ++++++++++++++++++++++++++++++++++----- plugins/askrene/refine.h | 22 ++++++ 2 files changed, 150 insertions(+), 17 deletions(-) diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index 97472443dfab..0d1a2c2fa569 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -6,6 +6,7 @@ #include #include #include +#include /* Channel data for fast retrieval. */ struct channel_data { @@ -35,8 +36,8 @@ static void destroy_reservations(struct reserve_hop *rhops, struct askrene *askr reserve_remove(askrene->reserved, &rhops[i]); } -static struct reserve_hop *new_reservations(const tal_t *ctx, - const struct route_query *rq) +struct reserve_hop *new_reservations(const tal_t *ctx, + const struct route_query *rq) { struct reserve_hop *rhops = tal_arr(ctx, struct reserve_hop, 0); @@ -108,9 +109,9 @@ static void subtract_reservation(struct reserve_hop **reservations, reserve_add(askrene->reserved, prev, rq->cmd->id); } -static void create_flow_reservations(const struct route_query *rq, - struct reserve_hop **reservations, - const struct flow *flow) +void create_flow_reservations(const struct route_query *rq, + struct reserve_hop **reservations, + const struct flow *flow) { struct amount_msat msat; @@ -170,6 +171,32 @@ static void change_flow_delivers(const struct route_query *rq, create_flow_reservations(rq, reservations, flow); } +bool create_flow_reservations_verify(const struct route_query *rq, + struct reserve_hop **reservations, + const struct flow *flow) +{ + struct amount_msat msat; + msat = flow->delivers; + for (int i = tal_count(flow->path) - 1; i >= 0; i--) { + struct amount_msat known_min, known_max; + const struct half_chan *h = flow_edge(flow, i); + struct amount_msat amount_to_reserve = msat; + struct short_channel_id_dir scidd; + + get_scidd(rq->gossmap, flow, i, &scidd); + get_constraints(rq, flow->path[i], flow->dirs[i], &known_min, + &known_max); + if (amount_msat_greater(amount_to_reserve, known_max)) + return false; + + if (!amount_msat_add_fee(&msat, h->base_fee, + h->proportional_fee)) + abort(); + } + create_flow_reservations(rq, reservations, flow); + return true; +} + /* We use an fp16_t approximatin for htlc_max/min: this gets the exact value. */ static struct amount_msat get_chan_htlc_max(const struct route_query *rq, const struct gossmap_chan *c, @@ -846,6 +873,24 @@ static struct amount_msat remove_excess(struct flow **flows, return all_deliver; } +static void write_selected_flows(const tal_t *ctx, size_t *flows_index, + struct flow ***flows) +{ + struct flow **tmp_flows = tal_arr(ctx, struct flow *, 0); + for (size_t i = 0; i < tal_count(flows_index); i++) { + tal_arr_expand(&tmp_flows, (*flows)[flows_index[i]]); + (*flows)[flows_index[i]] = NULL; + } + for (size_t i = 0; i < tal_count(*flows); i++) { + (*flows)[i] = tal_free((*flows)[i]); + } + tal_resize(flows, 0); + for (size_t i = 0; i < tal_count(tmp_flows); i++) { + tal_arr_expand(flows, tmp_flows[i]); + } + tal_free(tmp_flows); +} + /* FIXME: on failure return error message */ const char *refine_flows(const tal_t *ctx, struct route_query *rq, struct amount_msat deliver, struct flow ***flows) @@ -911,18 +956,7 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, } /* finally write the remaining flows */ - struct flow **tmp_flows = tal_arr(working_ctx, struct flow *, 0); - for (size_t i = 0; i < tal_count(flows_index); i++) { - tal_arr_expand(&tmp_flows, (*flows)[flows_index[i]]); - (*flows)[flows_index[i]] = NULL; - } - for (size_t i = 0; i < tal_count(*flows); i++) { - (*flows)[i] = tal_free((*flows)[i]); - } - tal_resize(flows, 0); - for (size_t i = 0; i < tal_count(tmp_flows); i++) { - tal_arr_expand(flows, tmp_flows[i]); - } + write_selected_flows(working_ctx, flows_index, flows); tal_free(working_ctx); return NULL; @@ -931,3 +965,80 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, tal_free(working_ctx); return error_message; } + +/* Order of flows according to path string */ +static int cmppath_flows(const size_t *a, const size_t *b, char **paths_str) +{ + return strcmp(paths_str[*a], paths_str[*b]); +} + +void squash_flows(const tal_t *ctx, struct route_query *rq, + struct flow ***flows) +{ + const tal_t *working_ctx = tal(ctx, tal_t); + size_t *flows_index = tal_arrz(working_ctx, size_t, tal_count(*flows)); + char **paths_str = tal_arrz(working_ctx, char *, tal_count(*flows)); + struct channel_data **channel_mpp_cache = + new_channel_mpp_cache(working_ctx, rq, *flows); + struct amount_msat *max_deliverable = tal_arrz( + working_ctx, struct amount_msat, tal_count(channel_mpp_cache)); + + for (size_t i = 0; i < tal_count(flows_index); i++) { + struct flow *flow = (*flows)[i]; + struct short_channel_id_dir scidd; + flows_index[i] = i; + paths_str[i] = tal_strdup(working_ctx, ""); + max_deliverable[i] = path_max_deliverable(channel_mpp_cache[i]); + + for (size_t j = 0; j < tal_count(flow->path); j++) { + scidd.scid = + gossmap_chan_scid(rq->gossmap, flow->path[j]); + scidd.dir = flow->dirs[j]; + tal_append_fmt( + &paths_str[i], "%s%s", j > 0 ? "->" : "", + fmt_short_channel_id_dir(working_ctx, &scidd)); + } + } + + asort(flows_index, tal_count(flows_index), cmppath_flows, paths_str); + for (size_t i = 0; i < tal_count(flows_index); i++) { + const size_t j = i + 1; + struct amount_msat combined; + struct amount_msat max = max_deliverable[flows_index[i]]; + + /* same path? We merge */ + while (j < tal_count(flows_index) && + cmppath_flows(&flows_index[i], + &flows_index[j], + paths_str) == 0) { + if (!amount_msat_add( + &combined, (*flows)[flows_index[i]]->delivers, + (*flows)[flows_index[j]]->delivers)) + abort(); + /* do we break any HTLC max limits */ + if (amount_msat_greater(combined, max)) + break; + (*flows)[flows_index[i]]->delivers = combined; + tal_arr_remove(&flows_index, j); + } + } + + write_selected_flows(working_ctx, flows_index, flows); + + tal_free(working_ctx); +} + +double flows_probability(const tal_t *ctx, struct route_query *rq, + struct flow ***flows) +{ + const tal_t *working_ctx = tal(ctx, tal_t); + struct reserve_hop *reservations = new_reservations(working_ctx, rq); + double probability = 1.0; + + for (size_t i = 0; i < tal_count(*flows); i++) { + probability *= flow_probability((*flows)[i], rq); + create_flow_reservations(rq, &reservations, (*flows)[i]); + } + tal_free(working_ctx); + return probability; +} diff --git a/plugins/askrene/refine.h b/plugins/askrene/refine.h index 8757f73618c5..b608e9a50ef3 100644 --- a/plugins/askrene/refine.h +++ b/plugins/askrene/refine.h @@ -7,6 +7,13 @@ struct route_query; struct amount_msat; struct flow; +struct reserve_hop *new_reservations(const tal_t *ctx, + const struct route_query *rq); + +void create_flow_reservations(const struct route_query *rq, + struct reserve_hop **reservations, + const struct flow *flow); + /* We got an answer from min-cost-flow, but we now need to: * 1. Add fixup exact delivery amounts since MCF deals in larger granularity than msat. * 2. Add fees which accumulate through the route. @@ -24,8 +31,23 @@ refine_with_fees_and_limits(const tal_t *ctx, struct flow ***flows, double *flowset_probability); +/* create flow reservations, but first verify that the flow indeeds fits in the + * liquidity constraints. Takes into account reservations that include per HTLC + * extra amounts to pay for onchain fees. */ +bool create_flow_reservations_verify(const struct route_query *rq, + struct reserve_hop **reservations, + const struct flow *flow); + /* Modify flows to meet HTLC min/max requirements. * It takes into account the exact value of the fees expected at each hop. */ const char *refine_flows(const tal_t *ctx, struct route_query *rq, struct amount_msat deliver, struct flow ***flows); + +/* Duplicated flows are merged into one. This saves in base fee and HTLC fees. + */ +void squash_flows(const tal_t *ctx, struct route_query *rq, + struct flow ***flows); + +double flows_probability(const tal_t *ctx, struct route_query *rq, + struct flow ***flows); #endif /* LIGHTNING_PLUGINS_ASKRENE_REFINE_H */ From 7fea9928d330c623ce4eb609e4733cee839851b1 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Fri, 1 Aug 2025 04:55:27 +0100 Subject: [PATCH 5/8] askrene: refine: add a step to increase flows ... by a small amount if the deliver amount is less than the requested amount by X. This step saves runtime by avoiding calling an extra MCF and it helps us solve a small percentage of cases where the only available routes have HTLCmin that is bigger than X. Changelog-None Signed-off-by: Lagrang3 --- plugins/askrene/refine.c | 56 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index 0d1a2c2fa569..d6e785e5d9bc 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -873,6 +873,58 @@ static struct amount_msat remove_excess(struct flow **flows, return all_deliver; } +/* It increases the flows to meet the deliver target. It does not increase any + * flow beyond the tolerance fraction. It doesn't increase any flow above its + * max_deliverable value. + * Returns the total delivery amount. */ +static struct amount_msat increase_flows(struct flow **flows, + size_t **flows_index, + struct amount_msat deliver, + double tolerance, + struct amount_msat *max_deliverable) +{ + if (tal_count(flows) == 0) + return AMOUNT_MSAT(0); + + struct amount_msat all_deliver, defect; + all_deliver = sum_all_deliver(flows, *flows_index); + + /* early exit: target is already met */ + if (!amount_msat_sub(&defect, deliver, all_deliver) || + amount_msat_is_zero(defect)) + return all_deliver; + + asort(*flows_index, tal_count(*flows_index), revcmp_flows, flows); + + all_deliver = AMOUNT_MSAT(0); + for (size_t i = 0; + i < tal_count(*flows_index) && !amount_msat_is_zero(defect); i++) { + const size_t index = (*flows_index)[i]; + struct flow *flow = flows[index]; + struct amount_msat can_add = defect, amt; + + /* no more than tolerance */ + if (!amount_msat_scale(&amt, flow->delivers, tolerance)) + continue; + else + can_add = amount_msat_min(can_add, amt); + + /* no more than max_deliverable */ + if (!amount_msat_sub(&amt, max_deliverable[index], + flow->delivers)) + continue; + else + can_add = amount_msat_min(can_add, amt); + + if (!amount_msat_add(&flow->delivers, flow->delivers, + can_add) || + !amount_msat_sub(&defect, defect, can_add) || + !amount_msat_accumulate(&all_deliver, flow->delivers)) + abort(); + } + return all_deliver; +} + static void write_selected_flows(const tal_t *ctx, size_t *flows_index, struct flow ***flows) { @@ -931,6 +983,10 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, /* remove excess from MCF granularity if any */ remove_excess(*flows, &flows_index, deliver); + /* increase flows if necessary to meet the target */ + increase_flows(*flows, &flows_index, deliver, /* tolerance = */ 0.02, + max_deliverable); + /* detect htlc_min violations */ for (size_t i = 0; i < tal_count(flows_index);) { size_t k = flows_index[i]; From c39c02312de9f46be2e613afd5b966d034dbbeb1 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Wed, 9 Jul 2025 20:27:55 +0100 Subject: [PATCH 6/8] askrene: rework the caller of the MCF solver We use a wrapper around the MCF solver that takes care of finding the best linearization parameters and fixing the flow values to meet the htlc_min and htlc_max constraints. We have reworked the current implementation and made it a bit more similar to renepay's version. Out of 50000 simulated payment situations distributed accross payment amounts of 1e2, 1e3, 1e4, 1e5 and 1e6 sats, we find that 133 failed cases in the master branch turn to success with the current changes, while only 3 success cases in the master are not solved by the changes. master +-------+------+ | S | F | +---+-------+------+ | S | 46329 | 133 | changes +---+-------+------+ | F | 3 | 3535 | +---+-------+------+ Out of the 133 cases that flipped from failure to success the failed reasons were: 122 -> "Could not find route without excessive cost" 5 -> "We couldn't quite afford it" 5 -> "Amount *msat below minimum" 1 -> tripped an HTLC min check Changelog-None. Signed-off-by: Lagrang3 --- plugins/askrene/flow.c | 30 ++-- plugins/askrene/mcf.c | 326 +++++++++++++++++++++++++---------------- tests/test_askrene.py | 12 +- 3 files changed, 225 insertions(+), 143 deletions(-) diff --git a/plugins/askrene/flow.c b/plugins/askrene/flow.c index f6a7ac1f7e34..9431f7d5e001 100644 --- a/plugins/askrene/flow.c +++ b/plugins/askrene/flow.c @@ -32,17 +32,24 @@ struct amount_msat flowset_delivers(struct plugin *plugin, return final; } -static double edge_probability(struct amount_msat sent, - struct amount_msat mincap, - struct amount_msat maxcap, - struct amount_msat used) +/* Stolen whole-cloth from @Lagrang3 in renepay's flow.c. Wrong + * because of htlc overhead in reservations! */ +static double edge_probability(const struct route_query *rq, + const struct short_channel_id_dir *scidd, + struct amount_msat sent) { struct amount_msat numerator, denominator; + struct amount_msat mincap, maxcap, additional; + const struct gossmap_chan *c = gossmap_find_chan(rq->gossmap, &scidd->scid); - if (!amount_msat_sub(&mincap, mincap, used)) - mincap = AMOUNT_MSAT(0); - if (!amount_msat_sub(&maxcap, maxcap, used)) - maxcap = AMOUNT_MSAT(0); + get_constraints(rq, c, scidd->dir, &mincap, &maxcap); + + /* We add an extra per-htlc reservation for the *next* HTLC, so we "over-reserve" + * on local channels. Undo that! */ + additional = get_additional_per_htlc_cost(rq, scidd); + if (!amount_msat_accumulate(&mincap, additional) + || !amount_msat_accumulate(&maxcap, additional)) + abort(); if (amount_msat_less_eq(sent, mincap)) return 1.0; @@ -129,10 +136,11 @@ double flow_probability(const struct flow *flow, for (int i = (int)pathlen - 1; i >= 0; i--) { const struct half_chan *h = flow_edge(flow, i); - struct amount_msat mincap, maxcap; + struct short_channel_id_dir scidd; + scidd.scid = gossmap_chan_scid(rq->gossmap, flow->path[i]); + scidd.dir = flow->dirs[i]; - get_constraints(rq, flow->path[i], flow->dirs[i], &mincap, &maxcap); - prob *= edge_probability(spend, mincap, maxcap, AMOUNT_MSAT(0)); + prob *= edge_probability(rq, &scidd, spend); if (!amount_msat_add_fee(&spend, h->base_fee, h->proportional_fee)) { diff --git a/plugins/askrene/mcf.c b/plugins/askrene/mcf.c index 6d50cb8bdfe7..0e87e1c726e0 100644 --- a/plugins/askrene/mcf.c +++ b/plugins/askrene/mcf.c @@ -169,6 +169,9 @@ static const double CHANNEL_PIVOTS[]={0,0.5,0.8,0.95}; static const s64 INFINITE = INT64_MAX; static const s64 MU_MAX = 100; +/* every payment under 1000sat will be routed through a single path */ +static const struct amount_msat SINGLE_PATH_THRESHOLD = AMOUNT_MSAT(1000000); + /* Let's try this encoding of arcs: * Each channel `c` has two possible directions identified by a bit * `half` or `!half`, and each one of them has to be @@ -1070,22 +1073,6 @@ struct flow **minflow(const tal_t *ctx, return NULL; } -static struct amount_msat linear_flows_cost(struct flow **flows, - struct amount_msat total_amount, - double delay_feefactor) -{ - struct amount_msat total = AMOUNT_MSAT(0); - - for (size_t i = 0; i < tal_count(flows); i++) { - if (!amount_msat_accumulate(&total, - linear_flow_cost(flows[i], - total_amount, - delay_feefactor))) - abort(); - } - return total; -} - /* Initialize the data vectors for the single-path solver. */ static void init_linear_network_single_path( const tal_t *ctx, const struct pay_parameters *params, struct graph **graph, @@ -1266,6 +1253,13 @@ struct flow **single_path_flow(const tal_t *ctx, const struct route_query *rq, return NULL; } +/* FIXME: add extra constraint maximum route length, use an activation + * probability cost for each channel. Recall that every activation cost, eg. + * base fee and activation probability can only be properly added modifying the + * graph topology by creating an activation node for every half channel. */ +/* FIXME: add extra constraint maximum number of routes, fixes issue 8331. */ +/* FIXME: add a boolean option to make recipient pay for fees, fixes issue 8353. + */ static const char * linear_routes(const tal_t *ctx, struct route_query *rq, const struct gossmap_node *srcnode, @@ -1277,133 +1271,213 @@ linear_routes(const tal_t *ctx, struct route_query *rq, const struct gossmap_node *, struct amount_msat, u32, double)) { - *flows = NULL; - const char *ret; - double delay_feefactor = 1.0 / 1000000; - - /* First up, don't care about fees (well, just enough to tiebreak!) */ + const tal_t *working_ctx = tal(ctx, tal_t); + const char *error_message; + struct amount_msat amount_to_deliver = amount; + struct amount_msat feebudget = maxfee; + + /* FIXME: mu is an integer from 0 to MU_MAX that we use to combine fees + * and probability costs, but I think we can make it a real number from + * 0 to 1. */ u32 mu = 1; - tal_free(*flows); - *flows = solver(ctx, rq, srcnode, dstnode, amount, mu, delay_feefactor); - if (!*flows) { - ret = explain_failure(ctx, rq, srcnode, dstnode, amount); - goto fail; - } + /* we start at 1e-6 and increase it exponentially (x2) up to 10. */ + double delay_feefactor = 1e-6; + + struct flow **new_flows = NULL; + struct amount_msat all_deliver; + + *flows = tal_arr(working_ctx, struct flow *, 0); + + /* Re-use the reservation system to make flows aware of each other. */ + struct reserve_hop *reservations = new_reservations(working_ctx, rq); + + while (!amount_msat_is_zero(amount_to_deliver)) { + new_flows = tal_free(new_flows); + + /* If the amount_to_deliver is very small we better use a single + * path computation because: + * 1. we save cpu cycles + * 2. we have better control over htlc_min violations. + * We need to make the distinction here because after + * refine_with_fees_and_limits we might have a set of flows that + * do not deliver the entire payment amount by just a small + * amount. */ + if(amount_msat_less_eq(amount_to_deliver, SINGLE_PATH_THRESHOLD)){ + new_flows = single_path_flow(working_ctx, rq, srcnode, dstnode, + amount_to_deliver, mu, delay_feefactor); + } else { + new_flows = + solver(working_ctx, rq, srcnode, dstnode, + amount_to_deliver, mu, delay_feefactor); + } - /* Too much delay? */ - while (finalcltv + flows_worst_delay(*flows) > maxdelay) { - delay_feefactor *= 2; - rq_log(tmpctx, rq, LOG_UNUSUAL, - "The worst flow delay is %" PRIu64 - " (> %i), retrying with delay_feefactor %f...", - flows_worst_delay(*flows), maxdelay - finalcltv, - delay_feefactor); - tal_free(*flows); - *flows = solver(ctx, rq, srcnode, dstnode, amount, mu, - delay_feefactor); - if (!*flows || delay_feefactor > 10) { - ret = rq_log( - ctx, rq, LOG_UNUSUAL, - "Could not find route without excessive delays"); + if (!new_flows) { + error_message = explain_failure( + ctx, rq, srcnode, dstnode, amount_to_deliver); goto fail; } - } - /* Too expensive? */ -too_expensive: - while (amount_msat_greater(flowset_fee(rq->plugin, *flows), maxfee)) { - struct flow **new_flows; - - if (mu == 1) - mu = 10; - else - mu += 10; - rq_log(tmpctx, rq, LOG_UNUSUAL, - "The flows had a fee of %s, greater than max of %s, " - "retrying with mu of %u%%...", - fmt_amount_msat(tmpctx, flowset_fee(rq->plugin, *flows)), - fmt_amount_msat(tmpctx, maxfee), mu); - new_flows = solver(ctx, rq, srcnode, dstnode, amount, - mu > 100 ? 100 : mu, delay_feefactor); - if (!*flows || mu >= 100) { - ret = rq_log( - ctx, rq, LOG_UNUSUAL, - "Could not find route without excessive cost"); + error_message = + refine_flows(ctx, rq, amount_to_deliver, &new_flows); + if (error_message) + goto fail; + + /* we finished removing flows and excess */ + all_deliver = flowset_delivers(rq->plugin, new_flows); + if (amount_msat_is_zero(all_deliver)) { + /* We removed all flows and we have not modified the + * MCF parameters. We will not have an infinite loop + * here because at least we have disabled some channels. + */ + continue; + } + + /* We might want to overpay sometimes, eg. shadow routing, but + * right now if all_deliver > amount_to_deliver means a bug. */ + assert(amount_msat_greater_eq(amount_to_deliver, all_deliver)); + + /* no flows should send 0 amount */ + for (size_t i = 0; i < tal_count(new_flows); i++) { + // FIXME: replace all assertions with LOG_BROKEN + assert(!amount_msat_is_zero(new_flows[i]->delivers)); + } + + /* Is this set of flows too expensive? + * We can check if the new flows are within the fee budget, + * however in some cases we have discarded some flows at this + * point and the new flows do not deliver all the value we need + * so that a further solver iteration is needed. Hence we + * check if the fees paid by these new flows are below the + * feebudget proportionally adjusted by the amount this set of + * flows deliver with respect to the total remaining amount, + * ie. we avoid "consuming" all the feebudget if we still need + * to run MCF again for some remaining amount. */ + struct amount_msat all_fees = + flowset_fee(rq->plugin, new_flows); + const double deliver_fraction = + amount_msat_ratio(all_deliver, amount_to_deliver); + struct amount_msat partial_feebudget; + if (!amount_msat_scale(&partial_feebudget, feebudget, + deliver_fraction)) { + error_message = + rq_log(ctx, rq, LOG_BROKEN, + "%s: failed to scale the fee budget (%s) by " + "fraction (%lf)", + __func__, fmt_amount_msat(tmpctx, feebudget), + deliver_fraction); goto fail; } + if (amount_msat_greater(all_fees, partial_feebudget)) { + if (mu < MU_MAX) { + /* all_fees exceed the strong budget limit, try + * to fix it increasing mu. */ + if (mu == 1) + mu = 10; + else + mu += 10; + mu = MIN(mu, MU_MAX); + rq_log( + tmpctx, rq, LOG_INFORM, + "The flows had a fee of %s, greater than " + "max of %s, retrying with mu of %u%%...", + fmt_amount_msat(tmpctx, all_fees), + fmt_amount_msat(tmpctx, partial_feebudget), + mu); + continue; + } else if (amount_msat_greater(all_fees, feebudget)) { + /* we cannot increase mu anymore and all_fees + * already exceeds feebudget we fail. */ + error_message = + rq_log(ctx, rq, LOG_UNUSUAL, + "Could not find route without " + "excessive cost"); + goto fail; + } else { + /* mu cannot be increased but at least all_fees + * does not exceed feebudget, we give it a shot. + */ + rq_log( + tmpctx, rq, LOG_UNUSUAL, + "The flows had a fee of %s, greater than " + "max of %s, but still within the fee " + "budget %s, we accept those flows.", + fmt_amount_msat(tmpctx, all_fees), + fmt_amount_msat(tmpctx, partial_feebudget), + fmt_amount_msat(tmpctx, feebudget)); + } + } - /* This is possible, because MCF's linear fees are not the same. - */ - if (amount_msat_greater(flowset_fee(rq->plugin, new_flows), - flowset_fee(rq->plugin, *flows))) { - struct amount_msat old_cost = - linear_flows_cost(*flows, amount, delay_feefactor); - struct amount_msat new_cost = linear_flows_cost( - new_flows, amount, delay_feefactor); - if (amount_msat_greater_eq(new_cost, old_cost)) { - rq_log(tmpctx, rq, LOG_BROKEN, - "Old flows cost %s:", - fmt_amount_msat(tmpctx, old_cost)); - for (size_t i = 0; i < tal_count(*flows); i++) { - rq_log( - tmpctx, rq, LOG_BROKEN, - "Flow %zu/%zu: %s (linear cost %s)", - i, tal_count(*flows), - fmt_flow_full(tmpctx, rq, (*flows)[i]), - fmt_amount_msat( - tmpctx, linear_flow_cost( - (*flows)[i], amount, - delay_feefactor))); - } - rq_log(tmpctx, rq, LOG_BROKEN, - "Old flows cost %s:", - fmt_amount_msat(tmpctx, new_cost)); - for (size_t i = 0; i < tal_count(new_flows); - i++) { - rq_log( - tmpctx, rq, LOG_BROKEN, - "Flow %zu/%zu: %s (linear cost %s)", - i, tal_count(new_flows), - fmt_flow_full(tmpctx, rq, - new_flows[i]), - fmt_amount_msat( - tmpctx, - linear_flow_cost( - new_flows[i], amount, - delay_feefactor))); - } + /* Too much delay? */ + if (finalcltv + flows_worst_delay(new_flows) > maxdelay) { + if (delay_feefactor > 10) { + error_message = + rq_log(ctx, rq, LOG_UNUSUAL, + "Could not find route without " + "excessive delays"); + goto fail; } + + delay_feefactor *= 2; + rq_log(tmpctx, rq, LOG_INFORM, + "The worst flow delay is %" PRIu64 + " (> %i), retrying with delay_feefactor %f...", + flows_worst_delay(new_flows), maxdelay - finalcltv, + delay_feefactor); + continue; } - tal_free(*flows); - *flows = new_flows; - } - if (finalcltv + flows_worst_delay(*flows) > maxdelay) { - ret = rq_log( - ctx, rq, LOG_UNUSUAL, - "Could not find route without excessive cost or delays"); - goto fail; + all_fees = AMOUNT_MSAT(0); + all_deliver = AMOUNT_MSAT(0); + /* add the new flows to the final solution */ + for (size_t i = 0; i < tal_count(new_flows); i++) { + /* last check: every time we add a new reservation to a + * local channel we remove some amount to pay for fees + * on the additional HTLC. */ + if (create_flow_reservations_verify(rq, &reservations, + new_flows[i])) { + tal_arr_expand(flows, new_flows[i]); + tal_steal(*flows, new_flows[i]); + if (!amount_msat_accumulate( + &all_deliver, new_flows[i]->delivers) || + !amount_msat_accumulate( + &all_fees, + flow_fee(rq->plugin, new_flows[i]))) + abort(); + } + } + + if (!amount_msat_sub(&feebudget, feebudget, all_fees) || + !amount_msat_sub(&amount_to_deliver, amount_to_deliver, + all_deliver)) { + error_message = + rq_log(ctx, rq, LOG_BROKEN, + "%s: unexpected arithmetic operation " + "failure on amount_msat", + __func__); + goto fail; + } } + /* transfer ownership */ + *flows = tal_steal(ctx, *flows); - /* The above did not take into account the extra funds to pay - * fees, so we try to adjust now. We could re-run MCF if this - * fails, but failure basically never happens where payment is - * still possible */ - ret = refine_with_fees_and_limits(ctx, rq, amount, flows, probability); - if (ret) - goto fail; + /* cleanup */ + tal_free(working_ctx); - /* Again, a tiny corner case: refine step can make us exceed maxfee */ - if (amount_msat_greater(flowset_fee(rq->plugin, *flows), maxfee)) { - rq_log(tmpctx, rq, LOG_UNUSUAL, - "After final refinement, fee was excessive: retrying"); - goto too_expensive; - } + /* all set! Now squash flows that use the same path */ + squash_flows(ctx, rq, flows); + + /* flows_probability re-does a temporary reservation so we need to call + * it after we have cleaned the reservations we used to build the flows + * hence after we freed working_ctx. */ + *probability = flows_probability(ctx, rq, flows); return NULL; fail: - assert(ret != NULL); - return ret; + /* cleanup */ + tal_free(working_ctx); + + assert(error_message != NULL); + return error_message; } const char *default_routes(const tal_t *ctx, struct route_query *rq, diff --git a/tests/test_askrene.py b/tests/test_askrene.py index 01a721e0df69..b54b3776a63e 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -781,7 +781,7 @@ def test_getroutes_auto_sourcefree(node_factory): def test_getroutes_maxdelay(node_factory): gsfile, nodemap = generate_gossip_store([GenChannel(0, 1, forward=GenChannel.Half(propfee=10000, delay=80)), - GenChannel(0, 1, forward=GenChannel.Half(propfee=10001, delay=40))]) + GenChannel(0, 1, forward=GenChannel.Half(propfee=20000, delay=40))]) # Set up l1 with this as the gossip_store l1 = node_factory.get_node(gossip_store_file=gsfile.name) @@ -814,7 +814,7 @@ def test_getroutes_maxdelay(node_factory): 'amount_msat': 1000, 'path': [{'short_channel_id_dir': '0x1x1/1', 'next_node_id': nodemap[1], - 'amount_msat': 1010, + 'amount_msat': 1020, 'delay': 139}]}]} # Excessive maxdelay parameter @@ -1204,10 +1204,10 @@ def test_real_data(node_factory, bitcoind): # CI, it's slow. if SLOW_MACHINE: limit = 25 - expected = (6, 25, 1568821, 143649, 91) + expected = (6, 25, 1568821, 144649, 91) else: limit = 100 - expected = (9, 96, 6565467, 630668, 91) + expected = (9, 96, 6565466, 668476, 90) fees = {} for n in range(0, limit): @@ -1321,10 +1321,10 @@ def test_real_biases(node_factory, bitcoind): # CI, it's slow. if SLOW_MACHINE: limit = 25 - expected = ({1: 6, 2: 6, 4: 7, 8: 12, 16: 14, 32: 19, 64: 25, 100: 25}, 0) + expected = ({1: 6, 2: 6, 4: 7, 8: 10, 16: 15, 32: 20, 64: 25, 100: 25}, 0) else: limit = 100 - expected = ({1: 22, 2: 25, 4: 36, 8: 53, 16: 69, 32: 80, 64: 96, 100: 96}, 0) + expected = ({1: 19, 2: 27, 4: 36, 8: 48, 16: 71, 32: 83, 64: 95, 100: 96}, 0) l1.rpc.askrene_create_layer('biases') num_changed = {} From 1637c6c2a57fb231c1b8a14b691463ebe3e9eb54 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Wed, 9 Jul 2025 20:34:55 +0100 Subject: [PATCH 7/8] askrene: refine: remove refine_with_fee_and_limits and remove unused helper functions. Changelog-None Signed-off-by: Lagrang3 --- plugins/askrene/refine.c | 466 --------------------------------------- plugins/askrene/refine.h | 17 -- 2 files changed, 483 deletions(-) diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index d6e785e5d9bc..ee57628b5435 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -90,25 +90,6 @@ static void add_reservation(struct reserve_hop **reservations, tal_arr_expand(reservations, rhop); } -static void subtract_reservation(struct reserve_hop **reservations, - const struct route_query *rq, - const struct gossmap_chan *chan, - const struct short_channel_id_dir *scidd, - struct amount_msat amt) -{ - struct reserve_hop *prev; - struct askrene *askrene = get_askrene(rq->plugin); - - prev = find_reservation(*reservations, scidd); - assert(prev); - - reserve_remove(askrene->reserved, prev); - if (!amount_msat_sub(&prev->amount, prev->amount, amt)) - abort(); - /* Adding a zero reserve is weird, but legal and easy! */ - reserve_add(askrene->reserved, prev, rq->cmd->id); -} - void create_flow_reservations(const struct route_query *rq, struct reserve_hop **reservations, const struct flow *flow) @@ -136,41 +117,6 @@ void create_flow_reservations(const struct route_query *rq, } } -static void remove_flow_reservations(const struct route_query *rq, - struct reserve_hop **reservations, - const struct flow *flow) -{ - struct amount_msat msat = flow->delivers; - for (int i = tal_count(flow->path) - 1; i >= 0; i--) { - const struct half_chan *h = flow_edge(flow, i); - struct amount_msat amount_to_reserve; - struct short_channel_id_dir scidd; - - get_scidd(rq->gossmap, flow, i, &scidd); - - /* Reserve more for local channels if it reduces capacity */ - if (!amount_msat_add(&amount_to_reserve, msat, - get_additional_per_htlc_cost(rq, &scidd))) - abort(); - - subtract_reservation(reservations, rq, flow->path[i], &scidd, - amount_to_reserve); - if (!amount_msat_add_fee(&msat, - h->base_fee, h->proportional_fee)) - plugin_err(rq->plugin, "Adding fee to amount"); - } -} - -static void change_flow_delivers(const struct route_query *rq, - struct flow *flow, - struct reserve_hop **reservations, - struct amount_msat new) -{ - remove_flow_reservations(rq, reservations, flow); - flow->delivers = new; - create_flow_reservations(rq, reservations, flow); -} - bool create_flow_reservations_verify(const struct route_query *rq, struct reserve_hop **reservations, const struct flow *flow) @@ -229,418 +175,6 @@ enum why_capped { CAPPED_CAPACITY, }; -/* Get exact maximum we can deliver with this flow. Returns reason - * why this is the limit (max_hltc or capacity), and optionally sets scidd */ -static enum why_capped flow_max_capacity(const struct route_query *rq, - const struct flow *flow, - struct amount_msat *deliverable, - struct short_channel_id_dir *scidd_why, - struct amount_msat *amount_why) -{ - struct amount_msat max_msat = AMOUNT_MSAT(-1ULL); - enum why_capped why_capped = CAPPED_CAPACITY; - - for (int i = tal_count(flow->path) - 1; i >= 0; i--) { - const struct half_chan *h = flow_edge(flow, i); - struct amount_msat min, max, htlc_max; - struct short_channel_id_dir scidd; - - get_scidd(rq->gossmap, flow, i, &scidd); - /* We can pass constraints due to addition of fees! */ - get_constraints(rq, flow->path[i], flow->dirs[i], &min, &max); - - if (amount_msat_greater(max_msat, max)) { - why_capped = CAPPED_CAPACITY; - if (scidd_why) - *scidd_why = scidd; - if (amount_why) - *amount_why = max; - max_msat = max; - } - - htlc_max = get_chan_htlc_max(rq, flow->path[i], &scidd); - if (amount_msat_greater(max_msat, htlc_max)) { - why_capped = CAPPED_HTLC_MAX; - if (scidd_why) - *scidd_why = scidd; - if (amount_why) - *amount_why = htlc_max; - max_msat = htlc_max; - } - if (!amount_msat_add_fee(&max_msat, h->base_fee, h->proportional_fee)) - max_msat = AMOUNT_MSAT(-1ULL); - } - - /* Calculate deliverable max */ - *deliverable = max_msat; - for (size_t i = 0; i < tal_count(flow->path); i++) { - const struct half_chan *h = flow_edge(flow, i); - *deliverable = amount_msat_sub_fee(*deliverable, - h->base_fee, - h->proportional_fee); - } - return why_capped; -} - - -/* We have a basic set of flows, but we need to add fees, and take into - * account that "spendable" estimates are for a single HTLC. This can - * push us again over capacity or htlc_maximum_msat. - * - * We may have to reduce the flow amount in response to these. - * - * We also check for going below htlc_maximum_msat at this point: this - * is unusual and means it's small, so we just remove that flow if - * this happens, as we can make it up by buffing up the other flows - * (or, it's simply impossible). - */ -static const char *constrain_flow(const tal_t *ctx, - struct route_query *rq, - struct flow *flow) -{ - struct amount_msat deliverable, msat, amount_capped; - enum why_capped why_capped; - struct short_channel_id_dir scidd_capped; - - why_capped = flow_max_capacity(rq, flow, &deliverable, - &scidd_capped, &amount_capped); - if (amount_msat_less(deliverable, flow->delivers)) { - rq_log(tmpctx, rq, LOG_INFORM, - "Flow reduced to deliver %s not %s, because %s %s %s", - fmt_amount_msat(tmpctx, deliverable), - fmt_amount_msat(tmpctx, flow->delivers), - fmt_short_channel_id_dir(tmpctx, &scidd_capped), - why_capped == CAPPED_HTLC_MAX - ? "advertizes htlc_maximum_msat" - : "has remaining capacity", - fmt_amount_msat(tmpctx, amount_capped)); - flow->delivers = deliverable; - } - - /* Now, check if any of them violate htlc_min */ - msat = flow->delivers; - for (int i = tal_count(flow->path) - 1; i >= 0; i--) { - const struct half_chan *h = flow_edge(flow, i); - struct amount_msat min; - struct short_channel_id_dir scidd; - - get_scidd(rq->gossmap, flow, i, &scidd); - min = get_chan_htlc_min(rq, flow->path[i], &scidd); - - if (amount_msat_less(msat, min)) { - return rq_log(ctx, rq, LOG_UNUSUAL, - "Amount %s below minimum %s across %s", - fmt_amount_msat(tmpctx, msat), - fmt_amount_msat(tmpctx, min), - fmt_short_channel_id_dir(tmpctx, &scidd)); - } - if (!amount_msat_add_fee(&msat, - h->base_fee, h->proportional_fee)) - plugin_err(rq->plugin, "Adding fee to amount"); - } - return NULL; -} - -static struct amount_msat flow_remaining_capacity(const struct route_query *rq, - struct reserve_hop **reservations, - const struct flow *flow) -{ - struct amount_msat max, diff; - - /* Remove ourselves from reservation temporarily, so we don't - * accidentally cap! */ - remove_flow_reservations(rq, reservations, flow); - flow_max_capacity(rq, flow, &max, NULL, NULL); - - /* This seems to happen. Which is strange, since flows should - already be constrained. */ - if (!amount_msat_sub(&diff, max, flow->delivers)) { - plugin_log(rq->plugin, LOG_BROKEN, - "Flow delivers %s but max only %s? flow=%s", - fmt_amount_msat(tmpctx, flow->delivers), - fmt_amount_msat(tmpctx, max), - fmt_flow_full(rq, rq, flow)); - for (size_t i = 0; i < tal_count(*reservations); i++) { - plugin_log(rq->plugin, LOG_BROKEN, - "Reservation #%zi: %s on %s", - i, - fmt_amount_msat(tmpctx, (*reservations)[i].amount), - fmt_short_channel_id_dir(tmpctx, &(*reservations)[i].scidd)); - } - diff = AMOUNT_MSAT(0); - } - create_flow_reservations(rq, reservations, flow); - return diff; -} - -/* What's the "best" flow to add to? */ -static struct flow *pick_most_likely_flow(const struct route_query *rq, - struct flow **flows, - struct reserve_hop **reservations, - struct amount_msat additional) -{ - double best_prob = 0; - struct flow *best_flow = NULL; - - for (size_t i = 0; i < tal_count(flows); i++) { - struct amount_msat cap; - double prob = flow_probability(flows[i], rq); - if (prob < best_prob) - continue; - cap = flow_remaining_capacity(rq, reservations, flows[i]); - if (amount_msat_less(cap, additional)) - continue; - best_prob = prob; - best_flow = flows[i]; - } - - return best_flow; -} - -/* A secondary check for htlc_min violations, after excess trimming. */ -static const char *flow_violates_min(const tal_t *ctx, - struct route_query *rq, - const struct flow *flow) -{ - struct amount_msat msat = flow->delivers; - for (int i = tal_count(flow->path) - 1; i >= 0; i--) { - const struct half_chan *h = flow_edge(flow, i); - struct amount_msat min; - struct short_channel_id_dir scidd; - get_scidd(rq->gossmap, flow, i, &scidd); - - min = get_chan_htlc_min(rq, flow->path[i], &scidd); - if (amount_msat_less(msat, min)) { - return rq_log(ctx, rq, LOG_UNUSUAL, - "Sending %s across %s would violate htlc_min (~%s)", - fmt_amount_msat(tmpctx, msat), - fmt_short_channel_id_dir(tmpctx, &scidd), - fmt_amount_msat(tmpctx, min)); - } - if (!amount_msat_add_fee(&msat, h->base_fee, h->proportional_fee)) - plugin_err(rq->plugin, "Adding fee to amount"); - } - return NULL; -} - -/* If one flow is constrained by htlc_max, we might be able to simply - * duplicate it. This is naive: it could still fail due to total - * capacity, but it is a corner case anyway. */ -static bool duplicate_one_flow(const struct route_query *rq, - struct reserve_hop **reservations, - struct flow ***flows) -{ - for (size_t i = 0; i < tal_count(*flows); i++) { - struct flow *flow = (*flows)[i], *new_flow; - struct amount_msat max, new_amount; - /* Don't create 0 flow (shouldn't happen, but be sure) */ - if (amount_msat_less(flow->delivers, AMOUNT_MSAT(2))) - continue; - - if (flow_max_capacity(rq, flow, &max, NULL, NULL) - != CAPPED_HTLC_MAX) - continue; - - new_flow = tal(*flows, struct flow); - new_flow->path = tal_dup_talarr(new_flow, - const struct gossmap_chan *, - flow->path); - new_flow->dirs = tal_dup_talarr(new_flow, int, - flow->dirs); - new_flow->delivers = amount_msat_div(flow->delivers, 2); - create_flow_reservations(rq, reservations, new_flow); - - if (!amount_msat_sub(&new_amount, - flow->delivers, new_flow->delivers)) - abort(); - change_flow_delivers(rq, flow, reservations, new_amount); - tal_arr_expand(flows, new_flow); - return true; - } - return false; -} - -/* Stolen whole-cloth from @Lagrang3 in renepay's flow.c. Wrong - * because of htlc overhead in reservations! */ -static double edge_probability(const struct route_query *rq, - const struct short_channel_id_dir *scidd, - struct amount_msat sent) -{ - struct amount_msat numerator, denominator; - struct amount_msat mincap, maxcap, additional; - const struct gossmap_chan *c = gossmap_find_chan(rq->gossmap, &scidd->scid); - - get_constraints(rq, c, scidd->dir, &mincap, &maxcap); - - /* We add an extra per-htlc reservation for the *next* HTLC, so we "over-reserve" - * on local channels. Undo that! */ - additional = get_additional_per_htlc_cost(rq, scidd); - if (!amount_msat_accumulate(&mincap, additional) - || !amount_msat_accumulate(&maxcap, additional)) - abort(); - - if (amount_msat_less_eq(sent, mincap)) - return 1.0; - else if (amount_msat_greater(sent, maxcap)) - return 0.0; - - /* Linear probability: 1 - (spend - min) / (max - min) */ - - /* spend > mincap, from above. */ - if (!amount_msat_sub(&numerator, sent, mincap)) - abort(); - /* This can only fail is maxcap was < mincap, - * so we would be captured above */ - if (!amount_msat_sub(&denominator, maxcap, mincap)) - abort(); - return 1.0 - amount_msat_ratio(numerator, denominator); -} - -const char * -refine_with_fees_and_limits(const tal_t *ctx, - struct route_query *rq, - struct amount_msat deliver, - struct flow ***flows, - double *flowset_probability) -{ - struct reserve_hop *reservations = new_reservations(NULL, rq); - struct amount_msat more_to_deliver; - const char *flow_constraint_error = NULL; - const char *ret; - bool tried_split_flow = false; - - for (size_t i = 0; i < tal_count(*flows);) { - struct flow *flow = (*flows)[i]; - - flow_constraint_error = constrain_flow(tmpctx, rq, flow); - if (!flow_constraint_error) { - create_flow_reservations(rq, &reservations, flow); - i++; - continue; - } - - rq_log(tmpctx, rq, LOG_UNUSUAL, "Flow %zu/%zu was too constrained (%s) so removing it", - i, tal_count(*flows), flow_constraint_error); - /* This flow was reduced to 0 / impossible, remove */ - tal_arr_remove(flows, i); - } - - /* Due to granularity of MCF, we can deliver slightly more than expected: - * trim one in that case. */ - if (!amount_msat_sub(&more_to_deliver, deliver, - flowset_delivers(rq->plugin, *flows))) { - struct amount_msat excess; - if (!amount_msat_sub(&excess, - flowset_delivers(rq->plugin, *flows), - deliver)) - abort(); - for (size_t i = 0; i < tal_count(*flows); i++) { - const char *err; - struct amount_msat trimmed; - if (!amount_msat_sub(&trimmed, (*flows)[i]->delivers, excess)) - continue; - - rq_log(tmpctx, rq, LOG_DBG, - "%s extra, trimming flow %zu/%zu", - fmt_amount_msat(tmpctx, excess), i, tal_count(*flows)); - change_flow_delivers(rq, (*flows)[i], &reservations, trimmed); - /* In theory, this can violate min_htlc! Thanks @Lagrang3! */ - err = flow_violates_min(tmpctx, rq, (*flows)[i]); - if (err) { - /* This flow was reduced to 0 / impossible, remove */ - remove_flow_reservations(rq, &reservations, (*flows)[i]); - tal_arr_remove(flows, i); - i--; - /* If this causes failure, indicate why! */ - flow_constraint_error = err; - continue; - } - break; - } - - /* Usually this should shed excess, *BUT* maybe one - * was deleted instead for being below minimum */ - if (!amount_msat_sub(&more_to_deliver, deliver, - flowset_delivers(rq->plugin, *flows))) { - ret = rq_log(ctx, rq, LOG_UNUSUAL, - "Flowset delivers %s instead of %s, can't shed excess?", - fmt_amount_msat(tmpctx, flowset_delivers(rq->plugin, *flows)), - fmt_amount_msat(tmpctx, deliver)); - goto out; - } - - rq_log(tmpctx, rq, LOG_DBG, - "After dealing with excess, more_to_deliver=%s", - fmt_amount_msat(tmpctx, more_to_deliver)); - } - - /* The residual is minimal. In theory we could add one msat at a time - * to the most probably flow which has capacity. For speed, we break it - * into the number of flows, then assign each one. */ - while (!amount_msat_is_zero(more_to_deliver) && tal_count(*flows)) { - struct flow *f; - struct amount_msat extra, new_delivers; - - /* How much more do we deliver? Round up if we can */ - extra = amount_msat_div(more_to_deliver, tal_count(*flows)); - if (amount_msat_less(extra, more_to_deliver)) { - if (!amount_msat_accumulate(&extra, AMOUNT_MSAT(1))) - abort(); - } - - /* This happens when we have a single flow, and hit - * htlc_max. For this corner case, we split into an - * additional flow, but only once! */ - f = pick_most_likely_flow(rq, *flows, &reservations, extra); - if (!f) { - if (tried_split_flow || !duplicate_one_flow(rq, &reservations, flows)) { - ret = rq_log(ctx, rq, LOG_BROKEN, - "We couldn't quite afford it, we need to send %s more for fees: please submit a bug report!", - fmt_amount_msat(tmpctx, more_to_deliver)); - goto out; - } - tried_split_flow = true; - continue; - } - - if (!amount_msat_add(&new_delivers, f->delivers, extra)) - abort(); - change_flow_delivers(rq, f, &reservations, new_delivers); - - /* Should not happen, since extra comes from div... */ - if (!amount_msat_sub(&more_to_deliver, more_to_deliver, extra)) - abort(); - } - - if (!amount_msat_eq(deliver, flowset_delivers(rq->plugin, *flows))) { - /* This should only happen if there were no flows */ - if (tal_count(*flows) == 0) { - ret = tal_steal(ctx, flow_constraint_error); - goto out; - } - plugin_err(rq->plugin, "Flows delivered only %s of %s?", - fmt_amount_msat(tmpctx, flowset_delivers(rq->plugin, *flows)), - fmt_amount_msat(tmpctx, deliver)); - } - - ret = NULL; - - /* Total flowset probability is now easily calculated given reservations - * contains the total amounts through each channel (once we remove them) */ - destroy_reservations(reservations, get_askrene(rq->plugin)); - tal_add_destructor2(reservations, destroy_reservations, get_askrene(rq->plugin)); - - *flowset_probability = 1.0; - for (size_t i = 0; i < tal_count(reservations); i++) { - const struct reserve_hop *r = &reservations[i]; - *flowset_probability *= edge_probability(rq, &r->scidd, r->amount); - } - -out: - tal_free(reservations); - return ret; -} - /* Cache channel data along the path used by this flow. */ static struct channel_data *new_channel_path_cache(const tal_t *ctx, struct route_query *rq, diff --git a/plugins/askrene/refine.h b/plugins/askrene/refine.h index b608e9a50ef3..065cd1b2a332 100644 --- a/plugins/askrene/refine.h +++ b/plugins/askrene/refine.h @@ -14,23 +14,6 @@ void create_flow_reservations(const struct route_query *rq, struct reserve_hop **reservations, const struct flow *flow); -/* We got an answer from min-cost-flow, but we now need to: - * 1. Add fixup exact delivery amounts since MCF deals in larger granularity than msat. - * 2. Add fees which accumulate through the route. - * 3. Check for htlc_minimum_msat violations (we simply remove those flows). - * 4. Trim any flows which (after fees) now violate maximum htlc_minimum_msat/capacity bounds. - * - * We try to reassign missing sats to the remaining flows, which is usually easy. - * - * Returns NULL on success, or an error message for the caller. - */ -const char * -refine_with_fees_and_limits(const tal_t *ctx, - struct route_query *rq, - struct amount_msat deliver, - struct flow ***flows, - double *flowset_probability); - /* create flow reservations, but first verify that the flow indeeds fits in the * liquidity constraints. Takes into account reservations that include per HTLC * extra amounts to pay for onchain fees. */ From f31c0933316ffa1d6d2e346b5da200182dbdde62 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Wed, 9 Jul 2025 21:09:23 +0100 Subject: [PATCH 8/8] askrene: paranoid checks ... that new flows respect the HTLC min/max constraints. Changelog-None Signed-off-by: Lagrang3 --- plugins/askrene/mcf.c | 100 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/plugins/askrene/mcf.c b/plugins/askrene/mcf.c index 0e87e1c726e0..1c43efc772d2 100644 --- a/plugins/askrene/mcf.c +++ b/plugins/askrene/mcf.c @@ -1253,6 +1253,89 @@ struct flow **single_path_flow(const tal_t *ctx, const struct route_query *rq, return NULL; } +/* Get the scidd for the i'th hop in flow */ +static void get_scidd(const struct gossmap *gossmap, const struct flow *flow, + size_t i, struct short_channel_id_dir *scidd) +{ + scidd->scid = gossmap_chan_scid(gossmap, flow->path[i]); + scidd->dir = flow->dirs[i]; +} + +/* We use an fp16_t approximatin for htlc_max/min: this gets the exact value. */ +static struct amount_msat +get_chan_htlc_max(const struct route_query *rq, const struct gossmap_chan *c, + const struct short_channel_id_dir *scidd) +{ + struct amount_msat htlc_max; + + gossmap_chan_get_update_details(rq->gossmap, c, scidd->dir, NULL, NULL, + NULL, NULL, NULL, NULL, NULL, + &htlc_max); + return htlc_max; +} + +static struct amount_msat +get_chan_htlc_min(const struct route_query *rq, const struct gossmap_chan *c, + const struct short_channel_id_dir *scidd) +{ + struct amount_msat htlc_min; + + gossmap_chan_get_update_details(rq->gossmap, c, scidd->dir, NULL, NULL, + NULL, NULL, NULL, NULL, &htlc_min, + NULL); + return htlc_min; +} + +static bool check_htlc_min_limits(struct route_query *rq, struct flow **flows) +{ + + for (size_t k = 0; k < tal_count(flows); k++) { + struct flow *flow = flows[k]; + size_t pathlen = tal_count(flow->path); + struct amount_msat hop_amt = flow->delivers; + for (size_t i = pathlen - 1; i < pathlen; i--) { + const struct half_chan *h = flow_edge(flow, i); + struct short_channel_id_dir scidd; + + get_scidd(rq->gossmap, flow, i, &scidd); + struct amount_msat htlc_min = + get_chan_htlc_min(rq, flow->path[i], &scidd); + if (amount_msat_less(hop_amt, htlc_min)) + return false; + + if (!amount_msat_add_fee(&hop_amt, h->base_fee, + h->proportional_fee)) + abort(); + } + } + return true; +} + +static bool check_htlc_max_limits(struct route_query *rq, struct flow **flows) +{ + + for (size_t k = 0; k < tal_count(flows); k++) { + struct flow *flow = flows[k]; + size_t pathlen = tal_count(flow->path); + struct amount_msat hop_amt = flow->delivers; + for (size_t i = pathlen - 1; i < pathlen; i--) { + const struct half_chan *h = flow_edge(flow, i); + struct short_channel_id_dir scidd; + + get_scidd(rq->gossmap, flow, i, &scidd); + struct amount_msat htlc_max = + get_chan_htlc_max(rq, flow->path[i], &scidd); + if (amount_msat_greater(hop_amt, htlc_max)) + return false; + + if (!amount_msat_add_fee(&hop_amt, h->base_fee, + h->proportional_fee)) + abort(); + } + } + return true; +} + /* FIXME: add extra constraint maximum route length, use an activation * probability cost for each channel. Recall that every activation cost, eg. * base fee and activation probability can only be properly added modifying the @@ -1471,6 +1554,23 @@ linear_routes(const tal_t *ctx, struct route_query *rq, * hence after we freed working_ctx. */ *probability = flows_probability(ctx, rq, flows); + /* we should have fixed all htlc violations, "don't trust, + * verify" */ + if (!check_htlc_min_limits(rq, *flows)) { + error_message = + rq_log(rq, rq, LOG_BROKEN, + "%s: check_htlc_min_limits failed", __func__); + *flows = tal_free(*flows); + goto fail; + } + if (!check_htlc_max_limits(rq, *flows)) { + error_message = + rq_log(rq, rq, LOG_BROKEN, + "%s: check_htlc_max_limits failed", __func__); + *flows = tal_free(*flows); + goto fail; + } + return NULL; fail: /* cleanup */