diff --git a/channeld/full_channel.c b/channeld/full_channel.c index eca0ee5228a8..c3df6ee66900 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -416,7 +416,7 @@ static bool get_room_above_reserve(const struct channel *channel, return false; } - if (!amount_msat_sub_sat(remainder, *remainder, reserve)) { + if (!amount_msat_deduct_sat(remainder, reserve)) { status_debug("%s cannot afford htlc: would make balance %s" " below reserve %s", side_to_str(side), @@ -504,7 +504,7 @@ static bool htlc_dust(const struct channel *channel, side, &trim_rmvd)) return false; - return amount_msat_sub(trim_total, *trim_total, trim_rmvd); + return amount_msat_deduct(trim_total, trim_rmvd); } /* @@ -773,7 +773,7 @@ static enum channel_add_err add_htlc(struct channel *channel, */ if ((option_anchor_outputs || option_anchors_zero_fee_htlc_tx) && channel->opener == sender - && !amount_msat_sub_sat(&remainder, remainder, AMOUNT_SAT(660))) + && !amount_msat_deduct_sat(&remainder, AMOUNT_SAT(660))) return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; if (channel->opener== sender) { @@ -805,7 +805,7 @@ static enum channel_add_err add_htlc(struct channel *channel, if ((option_anchor_outputs || option_anchors_zero_fee_htlc_tx) && channel->opener != sender - && !amount_msat_sub_sat(&remainder, remainder, AMOUNT_SAT(660))) + && !amount_msat_deduct_sat(&remainder, AMOUNT_SAT(660))) return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; /* Should be able to afford both their own commit tx @@ -1257,12 +1257,12 @@ u32 approx_max_feerate(const struct channel *channel) * `to_remote`). */ if ((option_anchor_outputs || option_anchors_zero_fee_htlc_tx) - && !amount_msat_sub_sat(&avail, avail, AMOUNT_SAT(660))) { + && !amount_msat_deduct_sat(&avail, AMOUNT_SAT(660))) { avail = AMOUNT_MSAT(0); } else { /* We should never go below reserve. */ - if (!amount_msat_sub_sat(&avail, avail, - channel->config[!channel->opener].channel_reserve)) + if (!amount_msat_deduct_sat(&avail, + channel->config[!channel->opener].channel_reserve)) avail = AMOUNT_MSAT(0); } diff --git a/common/amount.c b/common/amount.c index 0f1867369acd..cf872d0cb598 100644 --- a/common/amount.c +++ b/common/amount.c @@ -263,6 +263,18 @@ WARN_UNUSED_RESULT bool amount_msat_accumulate(struct amount_msat *a, return amount_msat_add(a, *a, b); } +WARN_UNUSED_RESULT bool amount_msat_deduct(struct amount_msat *a, + struct amount_msat b) +{ + return amount_msat_sub(a, *a, b); +} + +WARN_UNUSED_RESULT bool amount_msat_deduct_sat(struct amount_msat *a, + struct amount_sat b) +{ + return amount_msat_sub_sat(a, *a, b); +} + WARN_UNUSED_RESULT bool amount_msat_sub(struct amount_msat *val, struct amount_msat a, struct amount_msat b) diff --git a/common/amount.h b/common/amount.h index 11551b913a94..04965889b758 100644 --- a/common/amount.h +++ b/common/amount.h @@ -105,6 +105,14 @@ WARN_UNUSED_RESULT bool amount_sat_add_sat_s64(struct amount_sat *val, WARN_UNUSED_RESULT bool amount_msat_accumulate(struct amount_msat *a, struct amount_msat b); +/* a -= b */ +WARN_UNUSED_RESULT bool amount_msat_deduct(struct amount_msat *a, + struct amount_msat b); + +/* a -= b */ +WARN_UNUSED_RESULT bool amount_msat_deduct_sat(struct amount_msat *a, + struct amount_sat b); + /* returns floor(msat/div) */ struct amount_msat amount_msat_div(struct amount_msat msat, u64 div); diff --git a/common/initial_commit_tx.c b/common/initial_commit_tx.c index 07e3c8909bea..fb585513bc5a 100644 --- a/common/initial_commit_tx.c +++ b/common/initial_commit_tx.c @@ -40,7 +40,7 @@ bool try_subtract_fee(enum side opener, enum side side, else opener_amount = other; - if (amount_msat_sub_sat(opener_amount, *opener_amount, base_fee)) + if (amount_msat_deduct_sat(opener_amount, base_fee)) return true; *opener_amount = AMOUNT_MSAT(0); diff --git a/devtools/mkclose.c b/devtools/mkclose.c index de51c7f64f25..d32aa2a6d9eb 100644 --- a/devtools/mkclose.c +++ b/devtools/mkclose.c @@ -121,7 +121,7 @@ int main(int argc, char *argv[]) if (option_anchor_outputs && !amount_sat_add(&fee, fee, AMOUNT_SAT(660))) errx(1, "Can't afford anchors"); - if (!amount_msat_sub_sat(&local_msat, local_msat, fee)) + if (!amount_msat_deduct_sat(&local_msat, fee)) errx(1, "Can't afford fee %s", fmt_amount_sat(NULL, fee)); if (!amount_sat_sub_msat(&remote_msat, funding_amount, local_msat)) diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 49933a932cc3..1fc0db374c04 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -570,7 +570,7 @@ static struct route_info **select_inchan(const tal_t *ctx, candidates[i].c->our_config.channel_reserve, candidates[i].c->channel_info.their_config.channel_reserve) || !amount_sat_to_msat(&capacity, candidates[i].c->funding_sats) - || !amount_msat_sub_sat(&capacity, capacity, cumulative_reserve)) { + || !amount_msat_deduct_sat(&capacity, cumulative_reserve)) { log_broken(ld->log, "Channel %s capacity overflow!", fmt_short_channel_id(tmpctx, *candidates[i].c->scid)); continue; diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index f5b064ca81ce..de2c79714f65 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -721,7 +721,7 @@ static void subtract_offered_htlcs(const struct channel *channel, hout = htlc_out_map_next(ld->htlcs_out, &outi)) { if (hout->key.channel != channel) continue; - if (!amount_msat_sub(amount, *amount, hout->msat)) + if (!amount_msat_deduct(amount, hout->msat)) *amount = AMOUNT_MSAT(0); } } @@ -738,7 +738,7 @@ static void subtract_received_htlcs(const struct channel *channel, hin = htlc_in_map_next(ld->htlcs_in, &ini)) { if (hin->key.channel != channel) continue; - if (!amount_msat_sub(amount, *amount, hin->msat)) + if (!amount_msat_deduct(amount, hin->msat)) *amount = AMOUNT_MSAT(0); } } @@ -759,9 +759,9 @@ struct amount_msat channel_amount_spendable(const struct channel *channel) /* If we're opener, subtract txfees we'll need to spend this */ if (channel->opener == LOCAL) { - if (!amount_msat_sub_sat(&spendable, spendable, - commit_txfee(channel, spendable, - LOCAL))) + if (!amount_msat_deduct_sat(&spendable, + commit_txfee(channel, spendable, + LOCAL))) return AMOUNT_MSAT(0); } @@ -803,9 +803,9 @@ struct amount_msat channel_amount_receivable(const struct channel *channel) /* If they're opener, subtract txfees they'll need to spend this */ if (channel->opener == REMOTE) { - if (!amount_msat_sub_sat(&receivable, receivable, - commit_txfee(channel, - receivable, REMOTE))) + if (!amount_msat_deduct_sat(&receivable, + commit_txfee(channel, + receivable, REMOTE))) return AMOUNT_MSAT(0); } diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 7c818782bed0..6e6853fe55b9 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -2006,8 +2006,7 @@ static void remove_htlc_out(struct channel *channel, struct htlc_out *hout) const struct channel_coin_mvt *mvt; struct amount_msat oldamt = channel->our_msat; /* We paid for this HTLC, so deduct balance. */ - if (!amount_msat_sub(&channel->our_msat, channel->our_msat, - hout->msat)) { + if (!amount_msat_deduct(&channel->our_msat, hout->msat)) { channel_internal_error(channel, "Underflow our_msat %s - HTLC %s", fmt_amount_msat(tmpctx, diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index d76afa68d153..f04f3e11f29f 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -639,6 +639,12 @@ static struct command_result *do_getroutes(struct command *cmd, "Layer no_mpp_support is active we switch to a " "single path algorithm."); } + if (rq->maxparts == 1 && + info->dev_algo != ALGO_SINGLE_PATH) { + info->dev_algo = ALGO_SINGLE_PATH; + rq_log(tmpctx, rq, LOG_DBG, + "maxparts == 1: switching to a single path algorithm."); + } /* Compute the routes. At this point we might select between multiple * algorithms. Right now there is only one algorithm available. */ @@ -830,6 +836,11 @@ static struct command_result *json_getroutes(struct command *cmd, "amount must be non-zero"); } + if (maxparts == 0) { + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "maxparts must be non-zero"); + } + if (*maxdelay > maxdelay_allowed) { return command_fail(cmd, PAY_USER_ERROR, "maximum delay allowed is %d", @@ -1057,7 +1068,7 @@ static struct command_result *json_askrene_inform_channel(struct command *cmd, if (!reserve_accumulate(askrene->reserved, scidd, amount)) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Amount overflow with reserves"); - if (!amount_msat_sub(amount, *amount, AMOUNT_MSAT(1))) + if (!amount_msat_deduct(amount, AMOUNT_MSAT(1))) *amount = AMOUNT_MSAT(0); if (command_check_only(cmd)) return command_check_done(cmd); diff --git a/plugins/askrene/mcf.c b/plugins/askrene/mcf.c index e41bfb05730e..a59c6feb6b05 100644 --- a/plugins/askrene/mcf.c +++ b/plugins/askrene/mcf.c @@ -345,7 +345,7 @@ static double pickhardt_richter_probability(struct amount_msat low, struct amount_msat all_states, good_states; if (amount_msat_greater_eq(amount, high)) return 0.0; - if (!amount_msat_sub(&amount, amount, low)) + if (!amount_msat_deduct(&amount, low)) return 1.0; if (!amount_msat_sub(&all_states, high, low)) abort(); // we expect high > low @@ -1377,9 +1377,6 @@ linear_routes(const tal_t *ctx, struct route_query *rq, struct reserve_hop *reservations = new_reservations(working_ctx, rq); while (!amount_msat_is_zero(amount_to_deliver)) { - size_t num_parts, parts_slots, excess_parts; - u32 bottleneck_idx; - if (timemono_after(time_mono(), deadline)) { error_message = rq_log(ctx, rq, LOG_BROKEN, "%s: timed out after deadline", @@ -1387,24 +1384,7 @@ linear_routes(const tal_t *ctx, struct route_query *rq, goto fail; } - /* FIXME: This algorithm to limit the number of parts is dumb - * for two reasons: - * 1. it does not take into account that several loop - * iterations here may produce two flows along the same - * path that after "squash_flows" become a single flow. - * 2. limiting the number of "slots" to 1 makes us fail to - * see some solutions that use more than one of those - * existing paths. - * - * A better approach could be to run MCF, remove the excess - * paths and then recompute a MCF on a network where each arc is - * one of the previously remaining paths, ie. redistributing the - * payment amount among the selected paths in a cost-efficient - * way. */ new_flows = tal_free(new_flows); - num_parts = tal_count(*flows); - assert(num_parts < rq->maxparts); - parts_slots = rq->maxparts - num_parts; /* If the amount_to_deliver is very small we better use a single * path computation because: @@ -1415,8 +1395,7 @@ linear_routes(const tal_t *ctx, struct route_query *rq, * do not deliver the entire payment amount by just a small * amount. */ if (amount_msat_less_eq(amount_to_deliver, - SINGLE_PATH_THRESHOLD) || - parts_slots == 1) { + SINGLE_PATH_THRESHOLD)) { new_flows = single_path_flow(working_ctx, rq, srcnode, dstnode, amount_to_deliver, mu, delay_feefactor); @@ -1433,7 +1412,7 @@ linear_routes(const tal_t *ctx, struct route_query *rq, } error_message = - refine_flows(ctx, rq, amount_to_deliver, &new_flows, &bottleneck_idx); + refine_flows(ctx, rq, amount_to_deliver, &new_flows); if (error_message) goto fail; @@ -1457,32 +1436,6 @@ linear_routes(const tal_t *ctx, struct route_query *rq, assert(!amount_msat_is_zero(new_flows[i]->delivers)); } - if (tal_count(new_flows) > parts_slots) { - /* Remove the excees of parts and leave one slot for the - * next round of computations. */ - excess_parts = 1 + tal_count(new_flows) - parts_slots; - } else if (tal_count(new_flows) == parts_slots && - amount_msat_less(all_deliver, amount_to_deliver)) { - /* Leave exactly 1 slot for the next round of - * computations. */ - excess_parts = 1; - } else - excess_parts = 0; - if (excess_parts > 0) { - /* If we removed all the flows we found, avoid selecting them again, - * by disabling one. */ - if (excess_parts == tal_count(new_flows)) - bitmap_set_bit(rq->disabled_chans, bottleneck_idx); - if (!remove_flows(&new_flows, excess_parts)) { - error_message = rq_log(ctx, rq, LOG_BROKEN, - "%s: failed to remove %zu" - " flows from a set of %zu", - __func__, excess_parts, - tal_count(new_flows)); - goto fail; - } - } - /* 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 @@ -1587,9 +1540,8 @@ linear_routes(const tal_t *ctx, struct route_query *rq, } } - if (!amount_msat_sub(&feebudget, feebudget, all_fees) || - !amount_msat_sub(&amount_to_deliver, amount_to_deliver, - all_deliver)) { + if (!amount_msat_deduct(&feebudget, all_fees) || + !amount_msat_deduct(&amount_to_deliver, all_deliver)) { error_message = rq_log(ctx, rq, LOG_BROKEN, "%s: unexpected arithmetic operation " @@ -1607,6 +1559,34 @@ linear_routes(const tal_t *ctx, struct route_query *rq, /* all set! Now squash flows that use the same path */ squash_flows(ctx, rq, flows); + /* If we're over the number of parts, try to cram excess into the + * largest-capacity parts */ + if (tal_count(*flows) > rq->maxparts) { + struct amount_msat fee; + + error_message = reduce_num_flows(rq, rq, flows, amount, rq->maxparts); + if (error_message) { + *flows = tal_free(*flows); + return error_message; + } + + /* Check fee budget! */ + fee = flowset_fee(rq->plugin, *flows); + if (amount_msat_greater(fee, maxfee)) { + error_message = rq_log(rq, rq, LOG_INFORM, + "After reducing the flows to %zu (i.e. maxparts)," + " we had a fee of %s, greater than " + "max of %s.", + tal_count(*flows), + fmt_amount_msat(tmpctx, fee), + fmt_amount_msat(tmpctx, maxfee)); + if (error_message) { + *flows = tal_free(*flows); + return error_message; + } + } + } + /* 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. */ @@ -1619,24 +1599,21 @@ linear_routes(const tal_t *ctx, struct route_query *rq, rq_log(rq, rq, LOG_BROKEN, "%s: check_htlc_min_limits failed", __func__); *flows = tal_free(*flows); - goto fail; + return error_message; } 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 rq_log(rq, rq, LOG_BROKEN, + "%s: check_htlc_max_limits failed", __func__); } if (tal_count(*flows) > rq->maxparts) { - error_message = rq_log( - rq, rq, LOG_BROKEN, - "%s: the number of flows (%zu) exceeds the limit set " - "on payment parts (%" PRIu32 - "), please submit a bug report", - __func__, tal_count(*flows), rq->maxparts); + size_t num_flows = tal_count(*flows); *flows = tal_free(*flows); - goto fail; + return rq_log(rq, rq, LOG_BROKEN, + "%s: the number of flows (%zu) exceeds the limit set " + "on payment parts (%" PRIu32 + "), please submit a bug report", + __func__, num_flows, rq->maxparts); } return NULL; diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index 00ee4f75ced9..5788d069365e 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -1,5 +1,6 @@ #include "config.h" #include +#include #include #include #include @@ -8,14 +9,6 @@ #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. */ @@ -146,12 +139,12 @@ bool create_flow_reservations_verify(const struct route_query *rq, /* 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) + int dir) { struct amount_msat htlc_max; gossmap_chan_get_update_details(rq->gossmap, - c, scidd->dir, + c, dir, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &htlc_max); return htlc_max; @@ -159,12 +152,12 @@ static struct amount_msat get_chan_htlc_max(const struct route_query *rq, 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) + int dir) { struct amount_msat htlc_min; gossmap_chan_get_update_details(rq->gossmap, - c, scidd->dir, + c, dir, NULL, NULL, NULL, NULL, NULL, NULL, &htlc_min, NULL); return htlc_min; @@ -175,85 +168,40 @@ enum why_capped { CAPPED_CAPACITY, }; -/* 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) +static int revcmp_flows(struct flow *const *a, struct flow *const *b, void *unused) { - if (amount_msat_eq(flows[*a]->delivers, flows[*b]->delivers)) + if (amount_msat_eq((*a)->delivers, (*b)->delivers)) return 0; - if (amount_msat_greater(flows[*a]->delivers, flows[*b]->delivers)) + if (amount_msat_greater((*a)->delivers, (*b)->delivers)) return -1; return 1; } // TODO: unit test: // -> make a path -// -> compute x = path_max_deliverable +// -> compute x = flow_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. Sets *bottleneck_idx to one of the contraining channels' idx, if non-NULL */ -static struct amount_msat path_max_deliverable(struct channel_data *path, - u32 *bottleneck_idx) + * delivered. */ +static struct amount_msat flow_max_deliverable(const struct route_query *rq, + const struct flow *flow) { 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); - if (amount_msat_greater(deliver, path[i].htlc_max)) { - if (bottleneck_idx) - *bottleneck_idx = path[i].idx; - deliver = path[i].htlc_max; - } - if (amount_msat_greater(deliver, path[i].liquidity_max)) { - if (bottleneck_idx) - *bottleneck_idx = path[i].idx; - deliver = path[i].liquidity_max; - } + for (size_t i = 0; i < tal_count(flow->path); i++) { + const struct half_chan *hc = &flow->path[i]->half[flow->dirs[i]]; + struct amount_msat unused, known_max, htlc_max; + deliver = amount_msat_sub_fee(deliver, hc->base_fee, + hc->proportional_fee); + htlc_max = get_chan_htlc_max(rq, flow->path[i], flow->dirs[i]); + if (amount_msat_greater(deliver, htlc_max)) + deliver = htlc_max; + + get_constraints(rq, flow->path[i], flow->dirs[i], + &unused, &known_max); + if (amount_msat_greater(deliver, known_max)) + deliver = known_max; } return deliver; } @@ -265,28 +213,36 @@ static struct amount_msat path_max_deliverable(struct channel_data *path, // -> 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) +static struct amount_msat flow_min_deliverable(const struct route_query *rq, + const struct flow *flow) { struct amount_msat least_send = AMOUNT_MSAT(1); - const size_t pathlen = tal_count(path); + const size_t pathlen = tal_count(flow->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)) + const struct half_chan *hc = &flow->path[i]->half[flow->dirs[i]]; + struct amount_msat htlc_min = get_chan_htlc_min(rq, flow->path[i], flow->dirs[i]); + + least_send = amount_msat_max(least_send, htlc_min); + if (!amount_msat_add_fee(&least_send, hc->base_fee, + hc->proportional_fee)) 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++) { + const struct half_chan *hc = &flow->path[i]->half[flow->dirs[i]]; + struct amount_msat htlc_min = get_chan_htlc_min(rq, flow->path[i], flow->dirs[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)); + amount_msat_sub_fee(in_value, hc->base_fee, + hc->proportional_fee); + assert(amount_msat_greater_eq(out_value, htlc_min)); struct amount_msat x = out_value; - if (!amount_msat_add_fee(&x, path[i].fee_base_msat, - path[i].fee_proportional_millionths)) + if (!amount_msat_add_fee(&x, hc->base_fee, + hc->proportional_fee)) abort(); /* if the in_value computed from the out_value is smaller than * it should, then we add 1msat */ @@ -294,11 +250,11 @@ static struct amount_msat path_min_deliverable(struct channel_data *path) !amount_msat_accumulate(&out_value, AMOUNT_MSAT(1))) abort(); /* check conditions */ - assert(amount_msat_greater_eq(out_value, path[i].htlc_min)); + assert(amount_msat_greater_eq(out_value, htlc_min)); x = out_value; assert( - amount_msat_add_fee(&x, path[i].fee_base_msat, - path[i].fee_proportional_millionths) && + amount_msat_add_fee(&x, hc->base_fee, + hc->proportional_fee) && amount_msat_greater_eq(x, in_value)); least_destination = out_value; } @@ -307,27 +263,34 @@ static struct amount_msat path_min_deliverable(struct channel_data *path) 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 struct flow *flow) { 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)) { + struct amount_msat htlc_min = get_chan_htlc_min(rq, flow->path[i], flow->dirs[i]); + const struct half_chan *hc = &flow->path[i]->half[flow->dirs[i]]; + if (amount_msat_less(msat, htlc_min)) { + struct short_channel_id_dir scidd; + /* FIXME: hoist this! */ + size_t idx = flow->dirs[i] + + 2 * gossmap_chan_idx(rq->gossmap, flow->path[i]); + + get_scidd(rq->gossmap, flow, i, &scidd); 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); + fmt_short_channel_id_dir(ctx, &scidd), + fmt_amount_msat(ctx, htlc_min)); + bitmap_set_bit(rq->disabled_chans, idx); break; } if (!amount_msat_add_fee( - &msat, channels[i].fee_base_msat, - channels[i].fee_proportional_millionths)) { + &msat, hc->base_fee, + hc->proportional_fee)) { error_message = rq_log(ctx, rq, LOG_BROKEN, "%s: Adding fee to amount", __func__); @@ -337,13 +300,13 @@ remove_htlc_min_violations(const tal_t *ctx, struct route_query *rq, return error_message; } -static struct amount_msat sum_all_deliver(struct flow **flows, - size_t *flows_index) + +static struct amount_msat sum_all_deliver(struct flow **flows) { struct amount_msat all_deliver = AMOUNT_MSAT(0); - for (size_t i = 0; i < tal_count(flows_index); i++) { + for (size_t i = 0; i < tal_count(flows); i++) { if (!amount_msat_accumulate(&all_deliver, - flows[flows_index[i]]->delivers)) + flows[i]->delivers)) abort(); } return all_deliver; @@ -352,33 +315,32 @@ static struct amount_msat sum_all_deliver(struct flow **flows, /* 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, +static struct amount_msat remove_excess(struct flow ***flows, struct amount_msat max_deliver) { - if (tal_count(flows) == 0) + if (tal_count(*flows) == 0) return AMOUNT_MSAT(0); struct amount_msat all_deliver, excess; - all_deliver = sum_all_deliver(flows, *flows_index); + all_deliver = sum_all_deliver(*flows); /* 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); + asort(*flows, tal_count(*flows), revcmp_flows, NULL); /* 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)) + for (int i = tal_count(*flows) - 1; i >= 0; i--) { + if (!amount_msat_deduct(&excess, + (*flows)[i]->delivers)) break; - if (!amount_msat_sub(&all_deliver, all_deliver, - flows[(*flows_index)[i]]->delivers)) + if (!amount_msat_deduct(&all_deliver, + (*flows)[i]->delivers)) abort(); - tal_arr_remove(flows_index, i); + tal_arr_remove(flows, i); } /* If we still have some excess, remove it from the @@ -386,9 +348,9 @@ static struct amount_msat remove_excess(struct flow **flows, * 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++) { + for (size_t i = 0; i < tal_count(*flows); i++) { double fraction = amount_msat_ratio( - flows[(*flows_index)[i]]->delivers, old_deliver); + (*flows)[i]->delivers, old_deliver); struct amount_msat remove; if (!amount_msat_scale(&remove, old_excess, fraction)) @@ -397,166 +359,169 @@ static struct amount_msat remove_excess(struct flow **flows, /* rounding errors: don't remove more than excess */ remove = amount_msat_min(remove, excess); - if (!amount_msat_sub(&excess, excess, remove)) + if (!amount_msat_deduct(&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)) + if (!amount_msat_deduct(&all_deliver, remove) || + !amount_msat_deduct(&(*flows)[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)) + assert(tal_count(*flows) > 0); + if (!amount_msat_deduct(&all_deliver, excess) || + !amount_msat_deduct(&(*flows)[0]->delivers, excess)) abort(); return all_deliver; } +/* Return true (and set shortage) if flow doesn't deliver this much */ +static bool flows_short(struct flow **flows, + struct amount_msat deliver, + struct amount_msat *shortage) +{ + return amount_msat_sub(shortage, deliver, sum_all_deliver(flows)) + && !amount_msat_is_zero(*shortage); +} + /* 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) + * flow beyond the tolerance fraction (unless negative). + * Returns true if it managed to increase total amount to "deliver". */ +static bool increase_flows(const struct route_query *rq, + struct flow **flows, + struct amount_msat deliver, + double tolerance) { - if (tal_count(flows) == 0) - return AMOUNT_MSAT(0); + const tal_t *working_ctx = tal(NULL, tal_t); + struct amount_msat shortage, *ceiling; + + /* Record max we can deliver for each flow, so we don't exceed it */ + ceiling = tal_arr(working_ctx, struct amount_msat, tal_count(flows)); + for (size_t i = 0; i < tal_count(flows); i++) { + if (tolerance < 0) + ceiling[i] = deliver; + else if (!amount_msat_scale(&ceiling[i], flows[i]->delivers, 1.0 + tolerance)) + abort(); + } - struct amount_msat all_deliver, defect; - all_deliver = sum_all_deliver(flows, *flows_index); + /* This is naive, but since flows can overlap, increasing one + * can alter the remaining capacity of the others! */ + while (flows_short(flows, deliver, &shortage)) { + size_t best_flownum = 0; + struct amount_msat best_remaining = AMOUNT_MSAT(0); + struct reserve_hop **reservations; + struct amount_msat addition; + + /* Because flows can interact, we reserve them all, removing one at a time. */ + reservations = tal_arr(NULL, struct reserve_hop *, tal_count(flows)); + for (size_t i = 0; i < tal_count(flows); i++) { + reservations[i] = new_reservations(reservations, rq); + create_flow_reservations(rq, &reservations[i], flows[i]); + } - /* early exit: target is already met */ - if (!amount_msat_sub(&defect, deliver, all_deliver) || - amount_msat_is_zero(defect)) - return all_deliver; + /* Find flow with most excess capacity. */ + for (size_t i = 0; i < tal_count(flows); i++) { + struct amount_msat capacity, remaining; - asort(*flows_index, tal_count(*flows_index), revcmp_flows, flows); + /* flow_max_deliverable considers reservations *and* + * htlc_max. So remove this reservation, to get the + * real maximum for one flow, then replace it. */ + tal_free(reservations[i]); + capacity = flow_max_deliverable(rq, flows[i]); + reservations[i] = new_reservations(reservations, rq); + create_flow_reservations(rq, &reservations[i], flows[i]); - 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; + /* Don't go above our tolerance */ + if (amount_msat_greater(capacity, ceiling[i])) + capacity = ceiling[i]; - /* no more than tolerance */ - if (!amount_msat_scale(&amt, flow->delivers, tolerance)) - continue; - else - can_add = amount_msat_min(can_add, amt); + if (!amount_msat_sub(&remaining, capacity, flows[i]->delivers)) + abort(); + if (amount_msat_greater(remaining, best_remaining)) { + best_flownum = i; + best_remaining = remaining; + } + } + tal_free(reservations); - /* no more than max_deliverable */ - if (!amount_msat_sub(&amt, max_deliverable[index], - flow->delivers)) - continue; + /* Add 1/n of the remainder, or all we can if that's less than 10 sats. */ + if (amount_msat_less_sat(shortage, AMOUNT_SAT(10))) + addition = shortage; else - can_add = amount_msat_min(can_add, amt); + addition = amount_msat_div_ceil(shortage, tal_count(flows)); - 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; -} + /* Can't add it? */ + if (amount_msat_less(best_remaining, addition)) { + tal_free(working_ctx); + return false; + } -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]); + if (!amount_msat_accumulate(&flows[best_flownum]->delivers, addition)) + abort(); + if (!amount_msat_deduct(&shortage, addition)) + abort(); } - tal_free(tmp_flows); + tal_free(working_ctx); + return true; } const char *refine_flows(const tal_t *ctx, struct route_query *rq, - struct amount_msat deliver, struct flow ***flows, - u32 *bottleneck_idx) + 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)); + /* do not deliver more than HTLC_MAX allow us */ + for (size_t i = 0; i < tal_count(*flows); i++) { + (*flows)[i]->delivers = + amount_msat_min((*flows)[i]->delivers, + flow_max_deliverable(rq, (*flows)[i])); + } + + /* remove excess from MCF granularity if any */ + remove_excess(flows, deliver); + min_deliverable = tal_arrz(working_ctx, struct amount_msat, - tal_count(channel_mpp_cache)); + tal_count(*flows)); 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 + for (size_t i = 0; i < tal_count(*flows); i++) { + // FIXME: does flow_max_deliverable work for a single // channel with 0 fees? - max_deliverable[i] = path_max_deliverable(channel_mpp_cache[i], bottleneck_idx); - min_deliverable[i] = path_min_deliverable(channel_mpp_cache[i]); + min_deliverable[i] = flow_min_deliverable(rq, (*flows)[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); - /* increase flows if necessary to meet the target */ - increase_flows(*flows, &flows_index, deliver, /* tolerance = */ 0.02, - max_deliverable); + increase_flows(rq, *flows, deliver, /* tolerance = */ 0.02); /* 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])) { + for (size_t i = 0; i < tal_count(*flows);) { + if (amount_msat_greater_eq((*flows)[i]->delivers, + flow_min_deliverable(rq, (*flows)[i]))) { i++; continue; } - /* 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]); + ctx, rq, (*flows)[i]); if (error_message) goto fail; + /* htlc_min is not met for this flow */ + tal_arr_remove(flows, 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--) { - if (!amount_msat_is_zero((*flows)[flows_index[i]]->delivers)) + asort(*flows, tal_count(*flows), revcmp_flows, NULL); + for (int i = tal_count(*flows) - 1; i >= 0; i--) { + if (!amount_msat_is_zero((*flows)[i]->delivers)) break; - tal_arr_remove(&flows_index, i); + tal_arr_remove(flows, i); } - /* finally write the remaining flows */ - write_selected_flows(working_ctx, flows_index, flows); - tal_free(working_ctx); return NULL; @@ -565,66 +530,47 @@ const char *refine_flows(const tal_t *ctx, struct route_query *rq, 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) +/* Order of flows in lexicographic order */ +static int cmppath_flows(struct flow *const *a, struct flow *const *b, void *unused) { - return strcmp(paths_str[*a], paths_str[*b]); + const struct flow *fa = *a, *fb = *b; + for (size_t i = 0; i < tal_count(fa->path); i++) { + /* Shorter comes first */ + if (i >= tal_count(fb->path)) + return 1; + if (fa->path[i] < fb->path[i]) + return -1; + if (fa->path[i] > fb->path[i]) + return 1; + } + /* fa equal to fb, but is fb longer? */ + if (tal_count(fb->path) > tal_count(fa->path)) + return -1; + /* equal */ + return 0; } 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++) { + asort(*flows, tal_count(*flows), cmppath_flows, NULL); + for (size_t i = 0; i < tal_count(*flows); 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], NULL); - - 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)) + while (i + 1 < tal_count(*flows) && + cmppath_flows(&flow, &(*flows)[i+1], NULL) == 0) { + struct amount_msat combined, max = flow_max_deliverable(rq, flow); + + if (!amount_msat_add(&combined, flow->delivers, (*flows)[i+1]->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); + flow->delivers = combined; + tal_arr_remove(flows, i+1); } } - - write_selected_flows(working_ctx, flows_index, flows); - - tal_free(working_ctx); } double flows_probability(const tal_t *ctx, struct route_query *rq, @@ -642,29 +588,42 @@ double flows_probability(const tal_t *ctx, struct route_query *rq, return probability; } -/* Compare flows by deliver amount */ -static int reverse_cmp_flows(struct flow *const *fa, struct flow *const *fb, - void *unused UNUSED) +/* Compare flows by remaining capacity (note: they are not reserved, so this + * capacity includes current capacity. */ +static int reverse_cmp_capacity(struct flow *const *fa, struct flow *const *fb, + struct route_query *rq) { - if (amount_msat_eq((*fa)->delivers, (*fb)->delivers)) + struct amount_msat amount_a, amount_b; + + amount_a = flow_max_deliverable(rq, *fa); + amount_b = flow_max_deliverable(rq, *fb); + if (!amount_msat_deduct(&amount_a, (*fa)->delivers)) + abort(); + if (!amount_msat_deduct(&amount_b, (*fb)->delivers)) + abort(); + + if (amount_msat_eq(amount_a, amount_b)) return 0; - if (amount_msat_greater((*fa)->delivers, (*fb)->delivers)) + if (amount_msat_greater(amount_a, amount_b)) return -1; return 1; } -bool remove_flows(struct flow ***flows, u32 n) +const char *reduce_num_flows(const tal_t *ctx, + const struct route_query *rq, + struct flow ***flows, + struct amount_msat deliver, + size_t num_parts) { - if (n == 0) - goto fail; - if (n > tal_count(*flows)) - goto fail; - asort(*flows, tal_count(*flows), reverse_cmp_flows, NULL); - for (size_t count = tal_count(*flows); n > 0; n--, count--) { - assert(count > 0); - tal_arr_remove(flows, count - 1); - } - return true; -fail: - return false; + /* Keep the largest-capacity flows */ + size_t orig_num_flows = tal_count(*flows); + asort(*flows, orig_num_flows, reverse_cmp_capacity, cast_const(struct route_query *, rq)); + tal_resize(flows, num_parts); + + if (!increase_flows(rq, *flows, deliver, -1.0)) + return rq_log(ctx, rq, LOG_INFORM, + "Failed to reduce %zu flows down to maxparts (%zu)", + orig_num_flows, num_parts); + + return NULL; } diff --git a/plugins/askrene/refine.h b/plugins/askrene/refine.h index fcd64be21be6..7c1f1ecbd3d5 100644 --- a/plugins/askrene/refine.h +++ b/plugins/askrene/refine.h @@ -23,12 +23,9 @@ bool create_flow_reservations_verify(const struct route_query *rq, /* Modify flows to meet HTLC min/max requirements. * It takes into account the exact value of the fees expected at each hop. - * If we reduce flows because it's too large for one channel, *bottleneck_idx - * is set to the idx of a channel which caused a reduction (if non-NULL). */ const char *refine_flows(const tal_t *ctx, struct route_query *rq, - struct amount_msat deliver, struct flow ***flows, - u32 *bottleneck_idx); + struct amount_msat deliver, struct flow ***flows); /* Duplicated flows are merged into one. This saves in base fee and HTLC fees. */ @@ -38,7 +35,10 @@ void squash_flows(const tal_t *ctx, struct route_query *rq, double flows_probability(const tal_t *ctx, struct route_query *rq, struct flow ***flows); -/* Helper function: removes n flows from the set. It will remove those flows - * with the lowest amount values. */ -bool remove_flows(struct flow ***flows, u32 n); +/* Modify flows so only N remain, if we can. Returns an error if we cannot. */ +const char *reduce_num_flows(const tal_t *ctx, + const struct route_query *rq, + struct flow ***flows, + struct amount_msat deliver, + size_t num_parts); #endif /* LIGHTNING_PLUGINS_ASKRENE_REFINE_H */ diff --git a/plugins/askrene/reserve.c b/plugins/askrene/reserve.c index 738577fc2bc4..de88091c2c0c 100644 --- a/plugins/askrene/reserve.c +++ b/plugins/askrene/reserve.c @@ -103,7 +103,7 @@ void reserve_sub(const struct reserve_htable *reserved, for (r = reserve_htable_getfirst(reserved, scidd, &rit); r; r = reserve_htable_getnext(reserved, scidd, &rit)) { - if (!amount_msat_sub(amount, *amount, r->rhop.amount)) + if (!amount_msat_deduct(amount, r->rhop.amount)) *amount = AMOUNT_MSAT(0); } } diff --git a/plugins/bkpr/channelsapy.c b/plugins/bkpr/channelsapy.c index f4fa55a0437c..898b3175c734 100644 --- a/plugins/bkpr/channelsapy.c +++ b/plugins/bkpr/channelsapy.c @@ -126,11 +126,9 @@ static void fillin_apy_acct_details(const struct bkpr *bkpr, /* If there is any push_out or lease_fees_out, we subtract * from starting balance */ - ok = amount_msat_sub(&apy->our_start_bal, apy->our_start_bal, - apy->push_out); + ok = amount_msat_deduct(&apy->our_start_bal, apy->push_out); assert(ok); - ok = amount_msat_sub(&apy->our_start_bal, apy->our_start_bal, - apy->lease_out); + ok = amount_msat_deduct(&apy->our_start_bal, apy->lease_out); assert(ok); /* we add values in to starting balance */ diff --git a/plugins/bkpr/onchain_fee.c b/plugins/bkpr/onchain_fee.c index 6cdce78ab3ca..8cfc0c136360 100644 --- a/plugins/bkpr/onchain_fee.c +++ b/plugins/bkpr/onchain_fee.c @@ -288,7 +288,7 @@ static void insert_chain_fees_diff(struct command *cmd, if (!amount_msat_accumulate(¤t_amt, ofs[i]->credit)) plugin_err(cmd->plugin, "Overflow when adding onchain fees"); - if (!amount_msat_sub(¤t_amt, current_amt, ofs[i]->debit)) + if (!amount_msat_deduct(¤t_amt, ofs[i]->debit)) plugin_err(cmd->plugin, "Underflow when subtracting onchain fees"); if (ofs[i]->update_count > max_update_count) max_update_count = ofs[i]->update_count; diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 919ca7de3527..6c78de12cec5 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -482,7 +482,7 @@ payment_constraints_update(struct payment_constraints *cons, return false; /* amount_msat_sub performs a check before actually subtracting. */ - if (!amount_msat_sub(&cons->fee_budget, cons->fee_budget, delta_fee)) + if (!amount_msat_deduct(&cons->fee_budget, delta_fee)) return false; cons->cltv_budget -= delta_cltv; @@ -571,9 +571,8 @@ static bool payment_chanhints_apply_route(struct payment *p) curhint->local->htlc_budget--; } - if (!amount_msat_sub(&curhint->estimated_capacity, - curhint->estimated_capacity, - curhop->amount)) { + if (!amount_msat_deduct(&curhint->estimated_capacity, + curhop->amount)) { /* Given our preemptive test * above, this should never * happen either. */ @@ -1447,7 +1446,7 @@ handle_intermediate_failure(struct command *cmd, * work. This allows us to then allow on equality, this is for * example necessary for local channels where exact matches * should be allowed. */ - if (!amount_msat_sub(&estimated, estimated, AMOUNT_MSAT(1))) + if (!amount_msat_deduct(&estimated, AMOUNT_MSAT(1))) abort(); /* These are an indication that the capacity was insufficient, @@ -3407,10 +3406,8 @@ static struct command_result *shadow_route_listchannels(struct command *cmd, * destination could otherwise cause the route to be too * expensive, while really being ok. If any of these fail then * the above checks are insufficient. */ - if (!amount_msat_sub(&d->constraints.fee_budget, - d->constraints.fee_budget, best_fee) || - !amount_msat_sub(&p->constraints.fee_budget, - p->constraints.fee_budget, best_fee)) + if (!amount_msat_deduct(&d->constraints.fee_budget, best_fee) || + !amount_msat_deduct(&p->constraints.fee_budget, best_fee)) paymod_err(p, "Could not update fee constraints " "for shadow route extension. " diff --git a/plugins/renepay/chan_extra.c b/plugins/renepay/chan_extra.c index 931463ca53d2..d4ca301975d3 100644 --- a/plugins/renepay/chan_extra.c +++ b/plugins/renepay/chan_extra.c @@ -128,7 +128,7 @@ enum renepay_errorcode channel_liquidity(struct amount_msat *liquidity, if (!h) return RENEPAY_CHANNEL_NOT_FOUND; struct amount_msat value_liquidity = h->known_max; - if (!amount_msat_sub(&value_liquidity, value_liquidity, h->htlc_total)) + if (!amount_msat_deduct(&value_liquidity, h->htlc_total)) return RENEPAY_AMOUNT_OVERFLOW; *liquidity = value_liquidity; return RENEPAY_NOERROR; @@ -616,12 +616,12 @@ double edge_probability(struct amount_msat min, struct amount_msat max, goto function_fail; // in_flight cannot be greater than max - if (!amount_msat_sub(&B, B, in_flight)) + if (!amount_msat_deduct(&B, in_flight)) goto function_fail; struct amount_msat A = min; // = MAX(0,min-in_flight); - if (!amount_msat_sub(&A, A, in_flight)) + if (!amount_msat_deduct(&A, in_flight)) A = AMOUNT_MSAT(0); struct amount_msat denominator; // = B-A @@ -655,7 +655,7 @@ chan_extra_remove_htlc(struct chan_extra_map *chan_extra_map, if (h->num_htlcs <= 0) return RENEPAY_PRECONDITION_ERROR; - if (!amount_msat_sub(&h->htlc_total, h->htlc_total, amount)) + if (!amount_msat_deduct(&h->htlc_total, amount)) return RENEPAY_AMOUNT_OVERFLOW; h->num_htlcs--; return RENEPAY_NOERROR; diff --git a/plugins/renepay/mcf.c b/plugins/renepay/mcf.c index 7d2f1490300a..8fdd067b4e24 100644 --- a/plugins/renepay/mcf.c +++ b/plugins/renepay/mcf.c @@ -1440,7 +1440,7 @@ get_flow_paths(const tal_t *ctx, const struct gossmap *gossmap, // substract the excess of msats for not having msat // accuracy struct amount_msat delivered = amount_msat(delta*1000); - if (!amount_msat_sub(&delivered, delivered, excess)) { + if (!amount_msat_deduct(&delivered, excess)) { if (fail) *fail = tal_fmt( ctx, "unable to substract excess"); diff --git a/plugins/renepay/mods.c b/plugins/renepay/mods.c index 34e505816538..0fdd45a95059 100644 --- a/plugins/renepay/mods.c +++ b/plugins/renepay/mods.c @@ -679,7 +679,7 @@ static struct command_result *compute_routes_cb(struct payment *payment) __func__); /* Remaining fee budget. */ - if (!amount_msat_sub(&feebudget, feebudget, fees_spent)) + if (!amount_msat_deduct(&feebudget, fees_spent)) feebudget = AMOUNT_MSAT(0); /* How much are we still trying to send? */ diff --git a/plugins/renepay/routebuilder.c b/plugins/renepay/routebuilder.c index 4e9d15142326..d4c940f77187 100644 --- a/plugins/renepay/routebuilder.c +++ b/plugins/renepay/routebuilder.c @@ -364,7 +364,7 @@ struct route **get_routes(const tal_t *ctx, } // update the fee budget - if (!amount_msat_sub(&feebudget, feebudget, fee)) { + if (!amount_msat_deduct(&feebudget, fee)) { // should never happen tal_report_error( ctx, ecode, fail, PLUGIN_ERROR, @@ -377,8 +377,7 @@ struct route **get_routes(const tal_t *ctx, } // update the amount that we deliver - if (!amount_msat_sub(&amount_to_deliver, - amount_to_deliver, delivering)) { + if (!amount_msat_deduct(&amount_to_deliver, delivering)) { // should never happen tal_report_error( ctx, ecode, fail, PLUGIN_ERROR, diff --git a/plugins/renepay/routetracker.c b/plugins/renepay/routetracker.c index 2e8b9ba2b08d..b30a4a54423f 100644 --- a/plugins/renepay/routetracker.c +++ b/plugins/renepay/routetracker.c @@ -330,11 +330,10 @@ void payment_collect_results(struct payment *payment, *final_msg = tal_strdup(tmpctx, r->final_msg); } - if (!amount_msat_sub(&payment->total_delivering, - payment->total_delivering, - route_delivers(r)) || - !amount_msat_sub(&payment->total_sent, payment->total_sent, - route_sends(r))) { + if (!amount_msat_deduct(&payment->total_delivering, + route_delivers(r)) || + !amount_msat_deduct(&payment->total_sent, + route_sends(r))) { plugin_err(pay_plugin->plugin, "%s: routes do not add up to " "payment total amount.", diff --git a/tests/plugins/channeld_fakenet.c b/tests/plugins/channeld_fakenet.c index d56c5824fb01..c8281671b85d 100644 --- a/tests/plugins/channeld_fakenet.c +++ b/tests/plugins/channeld_fakenet.c @@ -657,9 +657,8 @@ static struct amount_msat calc_capacity(struct info *info, if (!short_channel_id_dir_eq(&info->reservations[i]->scidd, scidd)) continue; /* We should never use more that we have! */ - if (!amount_msat_sub(&dynamic_capacity, - dynamic_capacity, - info->reservations[i]->amount)) + if (!amount_msat_deduct(&dynamic_capacity, + info->reservations[i]->amount)) abort(); status_debug("... minus reservation %s", fmt_amount_msat(tmpctx, info->reservations[i]->amount)); diff --git a/tests/test_askrene.py b/tests/test_askrene.py index 46229650545f..08b8705473fc 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -1563,10 +1563,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: 10, 16: 15, 32: 20, 64: 25, 100: 25}, 0) + expected = ({1: 6, 2: 6, 4: 7, 8: 12, 16: 14, 32: 19, 64: 25, 100: 25}, 0) else: limit = 100 - expected = ({1: 19, 2: 27, 4: 36, 8: 48, 16: 71, 32: 83, 64: 95, 100: 96}, 0) + expected = ({1: 22, 2: 25, 4: 36, 8: 53, 16: 69, 32: 80, 64: 96, 100: 96}, 0) l1.rpc.askrene_create_layer('biases') num_changed = {} @@ -1782,8 +1782,7 @@ def test_simple_dummy_channel(node_factory): def test_maxparts_infloop(node_factory, bitcoind): # Three paths from l1 -> l5. - # FIXME: enhance explain_failure! - l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=[{'broken_log': 'plugin-cln-askrene.*the obvious route'}] + [{}] * 4) + l1, l2, l3, l4, l5 = node_factory.get_nodes(5) for intermediate in (l2, l3, l4): node_factory.join_nodes([l1, intermediate, l5]) @@ -1805,16 +1804,14 @@ def test_maxparts_infloop(node_factory, bitcoind): final_cltv=5) assert len(route['routes']) == 3 - # Now with maxparts == 2. Usually askrene can't figure out why it failed, - # but sometimes it gets a theory. - with pytest.raises(RpcError): - l1.rpc.getroutes(source=l1.info['id'], - destination=l5.info['id'], - amount_msat=amount, - layers=[], - maxfee_msat=amount, - final_cltv=5, - maxparts=2) + route = l1.rpc.getroutes(source=l1.info['id'], + destination=l5.info['id'], + amount_msat=amount, + layers=[], + maxfee_msat=amount, + final_cltv=5, + maxparts=2) + assert len(route['routes']) == 2 def test_askrene_timeout(node_factory, bitcoind):