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/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 696b810c7574..1c43efc772d2 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 @@ -319,6 +322,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 +580,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 +656,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 +665,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 +684,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 +890,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) @@ -1060,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, @@ -1107,8 +1104,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 @@ -1257,6 +1253,96 @@ 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 + * 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, @@ -1268,133 +1354,230 @@ 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)); + } + } + + /* 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; + } - /* 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))); - } + 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(); } } - tal_free(*flows); - *flows = new_flows; + + 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); + + /* cleanup */ + tal_free(working_ctx); - if (finalcltv + flows_worst_delay(*flows) > maxdelay) { - ret = rq_log( - ctx, rq, LOG_UNUSUAL, - "Could not find route without excessive cost or delays"); + /* 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); + + /* 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; } - - /* 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) + 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; - - /* 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; } 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/plugins/askrene/refine.c b/plugins/askrene/refine.c index 51f030ade8e8..ee57628b5435 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -1,10 +1,20 @@ #include "config.h" +#include #include #include #include #include #include #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 @@ -26,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); @@ -80,28 +90,9 @@ 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); -} - -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; @@ -126,39 +117,30 @@ static void create_flow_reservations(const struct route_query *rq, } } -static void remove_flow_reservations(const struct route_query *rq, +bool create_flow_reservations_verify(const struct route_query *rq, struct reserve_hop **reservations, const struct flow *flow) { - struct amount_msat msat = flow->delivers; + 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; + 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; - /* Reserve more for local channels if it reduces capacity */ - if (!amount_msat_add(&amount_to_reserve, msat, - get_additional_per_htlc_cost(rq, &scidd))) + if (!amount_msat_add_fee(&msat, h->base_fee, + h->proportional_fee)) 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); + return true; } /* We use an fp16_t approximatin for htlc_max/min: this gets the exact value. */ @@ -193,414 +175,460 @@ 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) +/* 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) { - struct amount_msat max_msat = AMOUNT_MSAT(-1ULL); - enum why_capped why_capped = CAPPED_CAPACITY; + const size_t pathlen = tal_count(flow->path); + struct channel_data *path = tal_arr(ctx, struct channel_data, pathlen); - for (int i = tal_count(flow->path) - 1; i >= 0; i--) { + 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 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); + 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; +} - /* 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); +/* 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 why_capped; + 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; +} -/* 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) +// 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 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; + 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; +} - /* 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"); +// 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(); } - return NULL; + /* 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 flow_remaining_capacity(const struct route_query *rq, - struct reserve_hop **reservations, - const struct flow *flow) +static const char * +remove_htlc_min_violations(const tal_t *ctx, struct route_query *rq, + const struct flow *flow, + const struct channel_data *channels) { - 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)); + 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; } - diff = AMOUNT_MSAT(0); } - create_flow_reservations(rq, reservations, flow); - return diff; + return error_message; } -/* 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) +static struct amount_msat sum_all_deliver(struct flow **flows, + size_t *flows_index) { - 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]; + 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 best_flow; + return all_deliver; } -/* 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) +/* 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) { - 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); + if (tal_count(flows) == 0) + return AMOUNT_MSAT(0); - 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"); + 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); } - 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 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(); - if (flow_max_capacity(rq, flow, &max, NULL, NULL) - != CAPPED_HTLC_MAX) - continue; + /* rounding errors: don't remove more than excess */ + remove = amount_msat_min(remove, excess); + + if (!amount_msat_sub(&excess, excess, remove)) + abort(); - 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)) + if (!amount_msat_sub(&all_deliver, all_deliver, remove) || + !amount_msat_sub(&flows[(*flows_index)[i]]->delivers, + flows[(*flows_index)[i]]->delivers, + remove)) abort(); - change_flow_delivers(rq, flow, reservations, new_amount); - tal_arr_expand(flows, new_flow); - return true; } - return false; + + /* 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; } -/* 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) +/* 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) { - struct amount_msat numerator, denominator; - struct amount_msat mincap, maxcap, additional; - const struct gossmap_chan *c = gossmap_find_chan(rq->gossmap, &scidd->scid); + if (tal_count(flows) == 0) + return AMOUNT_MSAT(0); - get_constraints(rq, c, scidd->dir, &mincap, &maxcap); + struct amount_msat all_deliver, defect; + all_deliver = sum_all_deliver(flows, *flows_index); - /* 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(); + /* early exit: target is already met */ + if (!amount_msat_sub(&defect, deliver, all_deliver) || + amount_msat_is_zero(defect)) + return all_deliver; - if (amount_msat_less_eq(sent, mincap)) - return 1.0; - else if (amount_msat_greater(sent, maxcap)) - return 0.0; + asort(*flows_index, tal_count(*flows_index), revcmp_flows, flows); - /* Linear probability: 1 - (spend - min) / (max - min) */ + 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; - /* 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); + /* 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; } -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) +static void write_selected_flows(const tal_t *ctx, size_t *flows_index, + struct flow ***flows) { - 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; + 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); +} - for (size_t i = 0; i < tal_count(*flows);) { - struct flow *flow = (*flows)[i]; +/* 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); + const char *error_message = NULL; + struct amount_msat *max_deliverable; + struct amount_msat *min_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)); + 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. */ + 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); - flow_constraint_error = constrain_flow(tmpctx, rq, flow); - if (!flow_constraint_error) { - create_flow_reservations(rq, &reservations, flow); + /* 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]; + if (amount_msat_greater_eq((*flows)[k]->delivers, + min_deliverable[k])) { 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); + /* 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; } - /* 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; - } + /* 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; - } - - /* 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)); + tal_arr_remove(&flows_index, i); } - /* 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(); - } + /* finally write the remaining flows */ + write_selected_flows(working_ctx, flows_index, flows); - /* 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; - } + tal_free(working_ctx); + return NULL; - if (!amount_msat_add(&new_delivers, f->delivers, extra)) - abort(); - change_flow_delivers(rq, f, &reservations, new_delivers); +fail: + tal_free(working_ctx); + return error_message; +} - /* Should not happen, since extra comes from div... */ - if (!amount_msat_sub(&more_to_deliver, more_to_deliver, extra)) - abort(); +/* 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)); + } } - 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; + 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); } - 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; + write_selected_flows(working_ctx, flows_index, flows); + + tal_free(working_ctx); +} - /* 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)); +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; - *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); + for (size_t i = 0; i < tal_count(*flows); i++) { + probability *= flow_probability((*flows)[i], rq); + create_flow_reservations(rq, &reservations, (*flows)[i]); } - -out: - tal_free(reservations); - return ret; + tal_free(working_ctx); + return probability; } diff --git a/plugins/askrene/refine.h b/plugins/askrene/refine.h index 66befec9b1e1..065cd1b2a332 100644 --- a/plugins/askrene/refine.h +++ b/plugins/askrene/refine.h @@ -7,20 +7,30 @@ struct route_query; struct amount_msat; struct 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. +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); + +/* 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. */ -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); +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 */ 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 = {}