From 10dc40a895b840686f259238aabd8adaa890a7fe Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 4 Oct 2024 08:55:53 +0930 Subject: [PATCH] askrene: clean up reserve array handling. I got confused, as we had a struct containing two arrays. Simply expose the reserve_hop struct and use arrays directly. Signed-off-by: Rusty Russell --- plugins/askrene/askrene.c | 56 +++++++++++++++------------------------ plugins/askrene/refine.c | 48 ++++++++++++++------------------- plugins/askrene/reserve.c | 20 +++++++------- plugins/askrene/reserve.h | 12 ++++++--- 4 files changed, 59 insertions(+), 77 deletions(-) diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index 4115675c3..9b319a88c 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -113,23 +113,17 @@ static struct command_result *param_known_layer(struct command *cmd, return NULL; } -struct reserve_path { - struct short_channel_id_dir *scidds; - struct amount_msat *amounts; -}; - -static struct command_result *parse_reserve_path(struct command *cmd, - const char *name, - const char *buffer, - const jsmntok_t *tok, - struct short_channel_id_dir *scidd, - struct amount_msat *amount) +static struct command_result *parse_reserve_hop(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct reserve_hop *rhop) { const char *err; err = json_scan(tmpctx, buffer, tok, "{short_channel_id_dir:%,amount_msat:%}", - JSON_SCAN(json_to_short_channel_id_dir, scidd), - JSON_SCAN(json_to_msat, amount)); + JSON_SCAN(json_to_short_channel_id_dir, &rhop->scidd), + JSON_SCAN(json_to_msat, &rhop->amount)); if (err) return command_fail_badparam(cmd, name, buffer, tok, err); return NULL; @@ -139,7 +133,7 @@ static struct command_result *param_reserve_path(struct command *cmd, const char *name, const char *buffer, const jsmntok_t *tok, - struct reserve_path **path) + struct reserve_hop **path) { size_t i; const jsmntok_t *t; @@ -147,15 +141,11 @@ static struct command_result *param_reserve_path(struct command *cmd, if (tok->type != JSMN_ARRAY) return command_fail_badparam(cmd, name, buffer, tok, "should be an array"); - *path = tal(cmd, struct reserve_path); - (*path)->scidds = tal_arr(cmd, struct short_channel_id_dir, tok->size); - (*path)->amounts = tal_arr(cmd, struct amount_msat, tok->size); + *path = tal_arr(cmd, struct reserve_hop, tok->size); json_for_each_arr(i, t, tok) { struct command_result *ret; - ret = parse_reserve_path(cmd, name, buffer, t, - &(*path)->scidds[i], - &(*path)->amounts[i]); + ret = parse_reserve_hop(cmd, name, buffer, t, &(*path)[i]); if (ret) return ret; } @@ -695,7 +685,7 @@ static struct command_result *json_askrene_reserve(struct command *cmd, const char *buffer, const jsmntok_t *params) { - struct reserve_path *path; + struct reserve_hop *path; struct json_stream *response; size_t num; struct askrene *askrene = get_askrene(cmd->plugin); @@ -705,15 +695,14 @@ static struct command_result *json_askrene_reserve(struct command *cmd, NULL)) return command_param_failed(); - num = reserves_add(askrene->reserved, path->scidds, path->amounts, - tal_count(path->scidds)); - if (num != tal_count(path->scidds)) { - const struct reserve *r = find_reserve(askrene->reserved, &path->scidds[num]); + num = reserves_add(askrene->reserved, path, tal_count(path)); + if (num != tal_count(path)) { + const struct reserve *r = find_reserve(askrene->reserved, &path[num].scidd); return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Overflow reserving %zu: %s amount %s (%s reserved already)", num, - fmt_short_channel_id_dir(tmpctx, &path->scidds[num]), - fmt_amount_msat(tmpctx, path->amounts[num]), + fmt_short_channel_id_dir(tmpctx, &path[num].scidd), + fmt_amount_msat(tmpctx, path[num].amount), r ? fmt_amount_msat(tmpctx, r->amount) : "none"); } @@ -725,7 +714,7 @@ static struct command_result *json_askrene_unreserve(struct command *cmd, const char *buffer, const jsmntok_t *params) { - struct reserve_path *path; + struct reserve_hop *path; struct json_stream *response; size_t num; struct askrene *askrene = get_askrene(cmd->plugin); @@ -735,15 +724,14 @@ static struct command_result *json_askrene_unreserve(struct command *cmd, NULL)) return command_param_failed(); - num = reserves_remove(askrene->reserved, path->scidds, path->amounts, - tal_count(path->scidds)); - if (num != tal_count(path->scidds)) { - const struct reserve *r = find_reserve(askrene->reserved, &path->scidds[num]); + num = reserves_remove(askrene->reserved, path, tal_count(path)); + if (num != tal_count(path)) { + const struct reserve *r = find_reserve(askrene->reserved, &path[num].scidd); return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Underflow unreserving %zu: %s amount %s (%zu reserved, amount %s)", num, - fmt_short_channel_id_dir(tmpctx, &path->scidds[num]), - fmt_amount_msat(tmpctx, path->amounts[num]), + fmt_short_channel_id_dir(tmpctx, &path[num].scidd), + fmt_amount_msat(tmpctx, path[num].amount), r ? r->num_htlcs : 0, r ? fmt_amount_msat(tmpctx, r->amount) : "none"); } diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index e78eebf7e..317832dcf 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -9,10 +9,6 @@ /* 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. */ -struct reservations { - struct short_channel_id_dir *scidds; - struct amount_msat *amounts; -}; /* Get the scidd for the i'th hop in flow */ static void get_scidd(const struct gossmap *gossmap, @@ -24,48 +20,45 @@ static void get_scidd(const struct gossmap *gossmap, scidd->dir = flow->dirs[i]; } -static void destroy_reservations(struct reservations *r, struct askrene *askrene) +static void destroy_reservations(struct reserve_hop *rhops, struct askrene *askrene) { - assert(tal_count(r->scidds) == tal_count(r->amounts)); - if (reserves_remove(askrene->reserved, - r->scidds, r->amounts, - tal_count(r->scidds)) != tal_count(r->scidds)) { + if (reserves_remove(askrene->reserved, rhops, tal_count(rhops)) + != tal_count(rhops)) { plugin_err(askrene->plugin, "Failed to remove reservations?"); } } -static struct reservations *new_reservations(const tal_t *ctx, - struct route_query *rq) +static struct reserve_hop *new_reservations(const tal_t *ctx, + const struct route_query *rq) { - struct reservations *r = tal(ctx, struct reservations); - r->scidds = tal_arr(r, struct short_channel_id_dir, 0); - r->amounts = tal_arr(r, struct amount_msat, 0); + struct reserve_hop *rhops = tal_arr(ctx, struct reserve_hop, 0); /* Unreserve on free */ - tal_add_destructor2(r, destroy_reservations, get_askrene(rq->plugin)); - return r; + tal_add_destructor2(rhops, destroy_reservations, get_askrene(rq->plugin)); + return rhops; } /* Add reservation: we (ab)use this to temporarily avoid over-usage as * we refine. */ -static void add_reservation(struct reservations *r, +static void add_reservation(struct reserve_hop **reservations, struct route_query *rq, const struct flow *flow, size_t i, struct amount_msat amt) { - struct short_channel_id_dir scidd; + struct reserve_hop rhop; struct askrene *askrene = get_askrene(rq->plugin); size_t idx; - get_scidd(rq->gossmap, flow, i, &scidd); + get_scidd(rq->gossmap, flow, i, &rhop.scidd); + rhop.amount = amt; /* This should not happen, but simply don't reserve if it does */ - if (!reserves_add(askrene->reserved, &scidd, &amt, 1)) { + if (!reserves_add(askrene->reserved, &rhop, 1)) { plugin_log(rq->plugin, LOG_BROKEN, "Failed to reserve %s in %s", fmt_amount_msat(tmpctx, amt), - fmt_short_channel_id_dir(tmpctx, &scidd)); + fmt_short_channel_id_dir(tmpctx, &rhop.scidd)); return; } @@ -75,8 +68,7 @@ static void add_reservation(struct reservations *r, rq->capacities[idx] = 0; /* Record so destructor will unreserve */ - tal_arr_expand(&r->scidds, scidd); - tal_arr_expand(&r->amounts, amt); + tal_arr_expand(reservations, rhop); } /* We have a basic set of flows, but we need to add fees, and take into @@ -93,7 +85,7 @@ static void add_reservation(struct reservations *r, static const char *constrain_flow(const tal_t *ctx, struct route_query *rq, struct flow *flow, - struct reservations *reservations) + struct reserve_hop **reservations) { struct amount_msat msat; int decreased = -1; @@ -187,7 +179,7 @@ static const char *constrain_flow(const tal_t *ctx, /* Flow is now delivering `extra` additional msat, so modify reservations */ static void add_to_flow(struct flow *flow, struct route_query *rq, - struct reservations *reservations, + struct reserve_hop **reservations, struct amount_msat extra) { struct amount_msat orig, updated; @@ -299,7 +291,7 @@ refine_with_fees_and_limits(const tal_t *ctx, struct amount_msat deliver, struct flow ***flows) { - struct reservations *reservations = new_reservations(NULL, rq); + struct reserve_hop *reservations = new_reservations(NULL, rq); struct amount_msat more_to_deliver; const char *flow_constraint_error = NULL; const char *ret; @@ -317,7 +309,7 @@ refine_with_fees_and_limits(const tal_t *ctx, fmt_amount_msat(tmpctx, max)); } - flow_constraint_error = constrain_flow(tmpctx, rq, flow, reservations); + flow_constraint_error = constrain_flow(tmpctx, rq, flow, &reservations); if (!flow_constraint_error) { i++; continue; @@ -398,7 +390,7 @@ refine_with_fees_and_limits(const tal_t *ctx, } /* Make this flow deliver +extra, and modify reservations */ - add_to_flow(f, rq, reservations, extra); + add_to_flow(f, rq, &reservations, extra); /* Should not happen, since extra comes from div... */ if (!amount_msat_sub(&more_to_deliver, more_to_deliver, extra)) diff --git a/plugins/askrene/reserve.c b/plugins/askrene/reserve.c index fb822143c..1ae96b5fe 100644 --- a/plugins/askrene/reserve.c +++ b/plugins/askrene/reserve.c @@ -86,16 +86,15 @@ static bool remove(struct reserve *r, struct amount_msat amount) /* Atomically add to reserves, or fail. * Returns offset of failure, or num on success */ size_t reserves_add(struct reserve_htable *reserved, - const struct short_channel_id_dir *scidds, - const struct amount_msat *amounts, + const struct reserve_hop *hops, size_t num) { for (size_t i = 0; i < num; i++) { - struct reserve *r = reserve_htable_get(reserved, &scidds[i]); + struct reserve *r = reserve_htable_get(reserved, &hops[i].scidd); if (!r) - r = new_reserve(reserved, &scidds[i]); - if (!add(r, amounts[i])) { - reserves_remove(reserved, scidds, amounts, i); + r = new_reserve(reserved, &hops[i].scidd); + if (!add(r, hops[i].amount)) { + reserves_remove(reserved, hops, i); return i; } } @@ -105,14 +104,13 @@ size_t reserves_add(struct reserve_htable *reserved, /* Atomically remove from reserves, to fail. * Returns offset of failure or tal_count(scidds) */ size_t reserves_remove(struct reserve_htable *reserved, - const struct short_channel_id_dir *scidds, - const struct amount_msat *amounts, + const struct reserve_hop *hops, size_t num) { for (size_t i = 0; i < num; i++) { - struct reserve *r = reserve_htable_get(reserved, &scidds[i]); - if (!r || !remove(r, amounts[i])) { - reserves_add(reserved, scidds, amounts, i); + struct reserve *r = reserve_htable_get(reserved, &hops[i].scidd); + if (!r || !remove(r, hops[i].amount)) { + reserves_add(reserved, hops, i); return i; } if (r->num_htlcs == 0) diff --git a/plugins/askrene/reserve.h b/plugins/askrene/reserve.h index 7da617229..67fc805c8 100644 --- a/plugins/askrene/reserve.h +++ b/plugins/askrene/reserve.h @@ -22,18 +22,22 @@ struct reserve_htable *new_reserve_htable(const tal_t *ctx); const struct reserve *find_reserve(const struct reserve_htable *reserved, const struct short_channel_id_dir *scidd); + +struct reserve_hop { + struct short_channel_id_dir scidd; + struct amount_msat amount; +}; + /* Atomically add to reserves, or fail. * Returns offset of failure, or num on success */ size_t reserves_add(struct reserve_htable *reserved, - const struct short_channel_id_dir *scidds, - const struct amount_msat *amounts, + const struct reserve_hop *hops, size_t num); /* Atomically remove from reserves, to fail. * Returns offset of failure or tal_count(scidds) */ size_t reserves_remove(struct reserve_htable *reserved, - const struct short_channel_id_dir *scidds, - const struct amount_msat *amounts, + const struct reserve_hop *hops, size_t num); /* Clear capacities array where we have reserves */