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 <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2024-10-04 08:55:53 +09:30
parent 2df53c32c2
commit 10dc40a895
4 changed files with 59 additions and 77 deletions

View file

@ -113,23 +113,17 @@ static struct command_result *param_known_layer(struct command *cmd,
return NULL; return NULL;
} }
struct reserve_path { static struct command_result *parse_reserve_hop(struct command *cmd,
struct short_channel_id_dir *scidds; const char *name,
struct amount_msat *amounts; const char *buffer,
}; const jsmntok_t *tok,
struct reserve_hop *rhop)
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)
{ {
const char *err; const char *err;
err = json_scan(tmpctx, buffer, tok, "{short_channel_id_dir:%,amount_msat:%}", 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_short_channel_id_dir, &rhop->scidd),
JSON_SCAN(json_to_msat, amount)); JSON_SCAN(json_to_msat, &rhop->amount));
if (err) if (err)
return command_fail_badparam(cmd, name, buffer, tok, err); return command_fail_badparam(cmd, name, buffer, tok, err);
return NULL; return NULL;
@ -139,7 +133,7 @@ static struct command_result *param_reserve_path(struct command *cmd,
const char *name, const char *name,
const char *buffer, const char *buffer,
const jsmntok_t *tok, const jsmntok_t *tok,
struct reserve_path **path) struct reserve_hop **path)
{ {
size_t i; size_t i;
const jsmntok_t *t; const jsmntok_t *t;
@ -147,15 +141,11 @@ static struct command_result *param_reserve_path(struct command *cmd,
if (tok->type != JSMN_ARRAY) if (tok->type != JSMN_ARRAY)
return command_fail_badparam(cmd, name, buffer, tok, "should be an array"); return command_fail_badparam(cmd, name, buffer, tok, "should be an array");
*path = tal(cmd, struct reserve_path); *path = tal_arr(cmd, struct reserve_hop, tok->size);
(*path)->scidds = tal_arr(cmd, struct short_channel_id_dir, tok->size);
(*path)->amounts = tal_arr(cmd, struct amount_msat, tok->size);
json_for_each_arr(i, t, tok) { json_for_each_arr(i, t, tok) {
struct command_result *ret; struct command_result *ret;
ret = parse_reserve_path(cmd, name, buffer, t, ret = parse_reserve_hop(cmd, name, buffer, t, &(*path)[i]);
&(*path)->scidds[i],
&(*path)->amounts[i]);
if (ret) if (ret)
return ret; return ret;
} }
@ -695,7 +685,7 @@ static struct command_result *json_askrene_reserve(struct command *cmd,
const char *buffer, const char *buffer,
const jsmntok_t *params) const jsmntok_t *params)
{ {
struct reserve_path *path; struct reserve_hop *path;
struct json_stream *response; struct json_stream *response;
size_t num; size_t num;
struct askrene *askrene = get_askrene(cmd->plugin); struct askrene *askrene = get_askrene(cmd->plugin);
@ -705,15 +695,14 @@ static struct command_result *json_askrene_reserve(struct command *cmd,
NULL)) NULL))
return command_param_failed(); return command_param_failed();
num = reserves_add(askrene->reserved, path->scidds, path->amounts, num = reserves_add(askrene->reserved, path, tal_count(path));
tal_count(path->scidds)); if (num != tal_count(path)) {
if (num != tal_count(path->scidds)) { const struct reserve *r = find_reserve(askrene->reserved, &path[num].scidd);
const struct reserve *r = find_reserve(askrene->reserved, &path->scidds[num]);
return command_fail(cmd, JSONRPC2_INVALID_PARAMS, return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Overflow reserving %zu: %s amount %s (%s reserved already)", "Overflow reserving %zu: %s amount %s (%s reserved already)",
num, num,
fmt_short_channel_id_dir(tmpctx, &path->scidds[num]), fmt_short_channel_id_dir(tmpctx, &path[num].scidd),
fmt_amount_msat(tmpctx, path->amounts[num]), fmt_amount_msat(tmpctx, path[num].amount),
r ? fmt_amount_msat(tmpctx, r->amount) : "none"); 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 char *buffer,
const jsmntok_t *params) const jsmntok_t *params)
{ {
struct reserve_path *path; struct reserve_hop *path;
struct json_stream *response; struct json_stream *response;
size_t num; size_t num;
struct askrene *askrene = get_askrene(cmd->plugin); struct askrene *askrene = get_askrene(cmd->plugin);
@ -735,15 +724,14 @@ static struct command_result *json_askrene_unreserve(struct command *cmd,
NULL)) NULL))
return command_param_failed(); return command_param_failed();
num = reserves_remove(askrene->reserved, path->scidds, path->amounts, num = reserves_remove(askrene->reserved, path, tal_count(path));
tal_count(path->scidds)); if (num != tal_count(path)) {
if (num != tal_count(path->scidds)) { const struct reserve *r = find_reserve(askrene->reserved, &path[num].scidd);
const struct reserve *r = find_reserve(askrene->reserved, &path->scidds[num]);
return command_fail(cmd, JSONRPC2_INVALID_PARAMS, return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Underflow unreserving %zu: %s amount %s (%zu reserved, amount %s)", "Underflow unreserving %zu: %s amount %s (%zu reserved, amount %s)",
num, num,
fmt_short_channel_id_dir(tmpctx, &path->scidds[num]), fmt_short_channel_id_dir(tmpctx, &path[num].scidd),
fmt_amount_msat(tmpctx, path->amounts[num]), fmt_amount_msat(tmpctx, path[num].amount),
r ? r->num_htlcs : 0, r ? r->num_htlcs : 0,
r ? fmt_amount_msat(tmpctx, r->amount) : "none"); r ? fmt_amount_msat(tmpctx, r->amount) : "none");
} }

View file

@ -9,10 +9,6 @@
/* We (ab)use the reservation system to place temporary reservations /* We (ab)use the reservation system to place temporary reservations
* on channels while we are refining each flow. This has the effect * on channels while we are refining each flow. This has the effect
* of making flows aware of each other. */ * 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 */ /* Get the scidd for the i'th hop in flow */
static void get_scidd(const struct gossmap *gossmap, 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]; 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, rhops, tal_count(rhops))
if (reserves_remove(askrene->reserved, != tal_count(rhops)) {
r->scidds, r->amounts,
tal_count(r->scidds)) != tal_count(r->scidds)) {
plugin_err(askrene->plugin, "Failed to remove reservations?"); plugin_err(askrene->plugin, "Failed to remove reservations?");
} }
} }
static struct reservations *new_reservations(const tal_t *ctx, static struct reserve_hop *new_reservations(const tal_t *ctx,
struct route_query *rq) const struct route_query *rq)
{ {
struct reservations *r = tal(ctx, struct reservations); struct reserve_hop *rhops = tal_arr(ctx, struct reserve_hop, 0);
r->scidds = tal_arr(r, struct short_channel_id_dir, 0);
r->amounts = tal_arr(r, struct amount_msat, 0);
/* Unreserve on free */ /* Unreserve on free */
tal_add_destructor2(r, destroy_reservations, get_askrene(rq->plugin)); tal_add_destructor2(rhops, destroy_reservations, get_askrene(rq->plugin));
return r; return rhops;
} }
/* Add reservation: we (ab)use this to temporarily avoid over-usage as /* Add reservation: we (ab)use this to temporarily avoid over-usage as
* we refine. */ * we refine. */
static void add_reservation(struct reservations *r, static void add_reservation(struct reserve_hop **reservations,
struct route_query *rq, struct route_query *rq,
const struct flow *flow, const struct flow *flow,
size_t i, size_t i,
struct amount_msat amt) struct amount_msat amt)
{ {
struct short_channel_id_dir scidd; struct reserve_hop rhop;
struct askrene *askrene = get_askrene(rq->plugin); struct askrene *askrene = get_askrene(rq->plugin);
size_t idx; 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 */ /* 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, plugin_log(rq->plugin, LOG_BROKEN,
"Failed to reserve %s in %s", "Failed to reserve %s in %s",
fmt_amount_msat(tmpctx, amt), fmt_amount_msat(tmpctx, amt),
fmt_short_channel_id_dir(tmpctx, &scidd)); fmt_short_channel_id_dir(tmpctx, &rhop.scidd));
return; return;
} }
@ -75,8 +68,7 @@ static void add_reservation(struct reservations *r,
rq->capacities[idx] = 0; rq->capacities[idx] = 0;
/* Record so destructor will unreserve */ /* Record so destructor will unreserve */
tal_arr_expand(&r->scidds, scidd); tal_arr_expand(reservations, rhop);
tal_arr_expand(&r->amounts, amt);
} }
/* We have a basic set of flows, but we need to add fees, and take into /* 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, static const char *constrain_flow(const tal_t *ctx,
struct route_query *rq, struct route_query *rq,
struct flow *flow, struct flow *flow,
struct reservations *reservations) struct reserve_hop **reservations)
{ {
struct amount_msat msat; struct amount_msat msat;
int decreased = -1; 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 */ /* Flow is now delivering `extra` additional msat, so modify reservations */
static void add_to_flow(struct flow *flow, static void add_to_flow(struct flow *flow,
struct route_query *rq, struct route_query *rq,
struct reservations *reservations, struct reserve_hop **reservations,
struct amount_msat extra) struct amount_msat extra)
{ {
struct amount_msat orig, updated; struct amount_msat orig, updated;
@ -299,7 +291,7 @@ refine_with_fees_and_limits(const tal_t *ctx,
struct amount_msat deliver, struct amount_msat deliver,
struct flow ***flows) struct flow ***flows)
{ {
struct reservations *reservations = new_reservations(NULL, rq); struct reserve_hop *reservations = new_reservations(NULL, rq);
struct amount_msat more_to_deliver; struct amount_msat more_to_deliver;
const char *flow_constraint_error = NULL; const char *flow_constraint_error = NULL;
const char *ret; const char *ret;
@ -317,7 +309,7 @@ refine_with_fees_and_limits(const tal_t *ctx,
fmt_amount_msat(tmpctx, max)); 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) { if (!flow_constraint_error) {
i++; i++;
continue; continue;
@ -398,7 +390,7 @@ refine_with_fees_and_limits(const tal_t *ctx,
} }
/* Make this flow deliver +extra, and modify reservations */ /* 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... */ /* Should not happen, since extra comes from div... */
if (!amount_msat_sub(&more_to_deliver, more_to_deliver, extra)) if (!amount_msat_sub(&more_to_deliver, more_to_deliver, extra))

View file

@ -86,16 +86,15 @@ static bool remove(struct reserve *r, struct amount_msat amount)
/* Atomically add to reserves, or fail. /* Atomically add to reserves, or fail.
* Returns offset of failure, or num on success */ * Returns offset of failure, or num on success */
size_t reserves_add(struct reserve_htable *reserved, size_t reserves_add(struct reserve_htable *reserved,
const struct short_channel_id_dir *scidds, const struct reserve_hop *hops,
const struct amount_msat *amounts,
size_t num) size_t num)
{ {
for (size_t i = 0; i < num; i++) { 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) if (!r)
r = new_reserve(reserved, &scidds[i]); r = new_reserve(reserved, &hops[i].scidd);
if (!add(r, amounts[i])) { if (!add(r, hops[i].amount)) {
reserves_remove(reserved, scidds, amounts, i); reserves_remove(reserved, hops, i);
return i; return i;
} }
} }
@ -105,14 +104,13 @@ size_t reserves_add(struct reserve_htable *reserved,
/* Atomically remove from reserves, to fail. /* Atomically remove from reserves, to fail.
* Returns offset of failure or tal_count(scidds) */ * Returns offset of failure or tal_count(scidds) */
size_t reserves_remove(struct reserve_htable *reserved, size_t reserves_remove(struct reserve_htable *reserved,
const struct short_channel_id_dir *scidds, const struct reserve_hop *hops,
const struct amount_msat *amounts,
size_t num) size_t num)
{ {
for (size_t i = 0; i < num; i++) { 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 || !remove(r, amounts[i])) { if (!r || !remove(r, hops[i].amount)) {
reserves_add(reserved, scidds, amounts, i); reserves_add(reserved, hops, i);
return i; return i;
} }
if (r->num_htlcs == 0) if (r->num_htlcs == 0)

View file

@ -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 reserve *find_reserve(const struct reserve_htable *reserved,
const struct short_channel_id_dir *scidd); 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. /* Atomically add to reserves, or fail.
* Returns offset of failure, or num on success */ * Returns offset of failure, or num on success */
size_t reserves_add(struct reserve_htable *reserved, size_t reserves_add(struct reserve_htable *reserved,
const struct short_channel_id_dir *scidds, const struct reserve_hop *hops,
const struct amount_msat *amounts,
size_t num); size_t num);
/* Atomically remove from reserves, to fail. /* Atomically remove from reserves, to fail.
* Returns offset of failure or tal_count(scidds) */ * Returns offset of failure or tal_count(scidds) */
size_t reserves_remove(struct reserve_htable *reserved, size_t reserves_remove(struct reserve_htable *reserved,
const struct short_channel_id_dir *scidds, const struct reserve_hop *hops,
const struct amount_msat *amounts,
size_t num); size_t num);
/* Clear capacities array where we have reserves */ /* Clear capacities array where we have reserves */