From 6640bc6f65cf9e65cb6ac1120759e7efdc7ec63d Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Tue, 7 Jan 2025 10:30:44 +0100 Subject: [PATCH 1/3] renepay: bugfix: read groupids as u64 Changelog-Fixed: renepay: read groupids as u64 integers. Signed-off-by: Lagrang3 --- plugins/renepay/mods.c | 30 +++++++++++++++--------------- plugins/renepay/route.c | 8 ++++---- plugins/renepay/route.h | 8 ++++---- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/plugins/renepay/mods.c b/plugins/renepay/mods.c index f8650db16724..ee9ecd84af67 100644 --- a/plugins/renepay/mods.c +++ b/plugins/renepay/mods.c @@ -15,7 +15,7 @@ #include #include -#define INVALID_ID UINT32_MAX +#define INVALID_ID UINT64_MAX #define OP_NULL NULL #define OP_CALL (void *)1 @@ -107,7 +107,7 @@ static void add_hintchan(struct payment *payment, const struct node_id *src, */ struct success_data { - u32 parts, created_at, groupid; + u64 parts, created_at, groupid; struct amount_msat deliver_msat, sent_msat; struct preimage preimage; }; @@ -130,7 +130,7 @@ static bool success_data_from_listsendpays(const char *buf, json_for_each_arr(i, t, arr) { - u32 groupid; + u64 groupid; struct amount_msat this_msat, this_sent; const jsmntok_t *status_tok = json_get_member(buf, t, "status"); @@ -158,10 +158,10 @@ static bool success_data_from_listsendpays(const char *buf, ",amount_sent_msat:%" ",created_at:%" ",payment_preimage:%}", - JSON_SCAN(json_to_u32, &groupid), + JSON_SCAN(json_to_u64, &groupid), JSON_SCAN(json_to_msat, &this_msat), JSON_SCAN(json_to_msat, &this_sent), - JSON_SCAN(json_to_u32, &success->created_at), + JSON_SCAN(json_to_u64, &success->created_at), JSON_SCAN(json_to_preimage, &success->preimage)); if (err) @@ -941,12 +941,12 @@ static struct command_result *pendingsendpays_done(struct command *cmd, size_t i; const char *err; const jsmntok_t *t, *arr; - u32 max_group_id = 0; + u64 max_group_id = 0; /* Data for pending payments, this will be the one * who's result gets replayed if we end up suspending. */ - u32 pending_group_id = INVALID_ID; - u32 max_pending_partid = 0; + u64 pending_group_id = INVALID_ID; + u64 max_pending_partid = 0; struct amount_msat pending_sent = AMOUNT_MSAT(0), pending_msat = AMOUNT_MSAT(0); @@ -978,14 +978,14 @@ static struct command_result *pendingsendpays_done(struct command *cmd, // find if there is one pending group json_for_each_arr(i, t, arr) { - u32 groupid; + u64 groupid; const char *status; err = json_scan(tmpctx, buf, t, "{status:%" ",groupid:%}", JSON_SCAN_TAL(tmpctx, json_strdup, &status), - JSON_SCAN(json_to_u32, &groupid)); + JSON_SCAN(json_to_u64, &groupid)); if (err) plugin_err(pay_plugin->plugin, @@ -1003,7 +1003,7 @@ static struct command_result *pendingsendpays_done(struct command *cmd, * pending sendpays. */ json_for_each_arr(i, t, arr) { - u32 partid = 0, groupid; + u64 partid = 0, groupid; struct amount_msat this_msat, this_sent; const char *status; @@ -1017,8 +1017,8 @@ static struct command_result *pendingsendpays_done(struct command *cmd, ",amount_msat:%" ",amount_sent_msat:%}", JSON_SCAN_TAL(tmpctx, json_strdup, &status), - JSON_SCAN(json_to_u32, &partid), - JSON_SCAN(json_to_u32, &groupid), + JSON_SCAN(json_to_u64, &partid), + JSON_SCAN(json_to_u64, &groupid), JSON_SCAN(json_to_msat, &this_msat), JSON_SCAN(json_to_msat, &this_sent)); @@ -1066,9 +1066,9 @@ static struct command_result *pendingsendpays_done(struct command *cmd, plugin_log(pay_plugin->plugin, LOG_DBG, "There are pending sendpays to this invoice. " - "groupid = %" PRIu32 " " + "groupid = %" PRIu64 " " "delivering = %s, " - "last_partid = %" PRIu32, + "last_partid = %" PRIu64, pending_group_id, fmt_amount_msat(tmpctx, payment->total_delivering), max_pending_partid); diff --git a/plugins/renepay/route.c b/plugins/renepay/route.c index 66e07cdb0436..521f956ba690 100644 --- a/plugins/renepay/route.c +++ b/plugins/renepay/route.c @@ -1,8 +1,8 @@ #include "config.h" #include -struct route *new_route(const tal_t *ctx, u32 groupid, - u32 partid, struct sha256 payment_hash, +struct route *new_route(const tal_t *ctx, u64 groupid, + u64 partid, struct sha256 payment_hash, struct amount_msat amount_deliver, struct amount_msat amount_sent) { @@ -32,7 +32,7 @@ struct route *new_route(const tal_t *ctx, u32 groupid, * @gossmap: global gossmap * @flow: the flow to convert to route */ struct route *flow_to_route(const tal_t *ctx, - u32 groupid, u32 partid, struct sha256 payment_hash, + u64 groupid, u64 partid, struct sha256 payment_hash, u32 final_cltv, struct gossmap *gossmap, struct flow *flow, bool blinded_destination) @@ -83,7 +83,7 @@ struct route *flow_to_route(const tal_t *ctx, } struct route **flows_to_routes(const tal_t *ctx, - u32 groupid, u32 partid, + u64 groupid, u64 partid, struct sha256 payment_hash, u32 final_cltv, struct gossmap *gossmap, struct flow **flows) { diff --git a/plugins/renepay/route.h b/plugins/renepay/route.h index 60d167c9a53f..592513d5b188 100644 --- a/plugins/renepay/route.h +++ b/plugins/renepay/route.h @@ -117,19 +117,19 @@ static inline bool routekey_equal(const struct route *route, HTABLE_DEFINE_NODUPS_TYPE(struct route, route_get_key, routekey_hash, routekey_equal, route_map); -struct route *new_route(const tal_t *ctx, u32 groupid, - u32 partid, struct sha256 payment_hash, +struct route *new_route(const tal_t *ctx, u64 groupid, + u64 partid, struct sha256 payment_hash, struct amount_msat amount, struct amount_msat amount_sent); struct route *flow_to_route(const tal_t *ctx, - u32 groupid, u32 partid, struct sha256 payment_hash, + u64 groupid, u64 partid, struct sha256 payment_hash, u32 final_cltv, struct gossmap *gossmap, struct flow *flow, bool blinded_destination); struct route **flows_to_routes(const tal_t *ctx, - u32 groupid, u32 partid, + u64 groupid, u64 partid, struct sha256 payment_hash, u32 final_cltv, struct gossmap *gossmap, struct flow **flows); From 9eecac4ba24ece7a4da9bb6c95519c642086a7c8 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Thu, 9 Jan 2025 09:19:30 +0100 Subject: [PATCH 2/3] renepay: names by convention Change functions json_pay and json_paystatus to json_renepay and json_renepaystatus to match the conventional naming. This is helpful for grep searches. Changelog-None Signed-off-by: Lagrang3 --- plugins/renepay/main.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/renepay/main.c b/plugins/renepay/main.c index 0fb75f3aed61..01b4d13be34c 100644 --- a/plugins/renepay/main.c +++ b/plugins/renepay/main.c @@ -96,9 +96,9 @@ static const char *init(struct command *init_cmd, return NULL; } -static struct command_result *json_paystatus(struct command *cmd, - const char *buf, - const jsmntok_t *params) +static struct command_result *json_renepaystatus(struct command *cmd, + const char *buf, + const jsmntok_t *params) { const char *invstring; struct json_stream *ret; @@ -161,8 +161,8 @@ static struct command_result * payment_start(struct payment *p) return payment_continue(p); } -static struct command_result *json_pay(struct command *cmd, const char *buf, - const jsmntok_t *params) +static struct command_result *json_renepay(struct command *cmd, const char *buf, + const jsmntok_t *params) { /* === Parse command line arguments === */ // TODO check if we leak some of these temporary variables @@ -452,11 +452,11 @@ static struct command_result *json_pay(struct command *cmd, const char *buf, static const struct plugin_command commands[] = { { "renepaystatus", - json_paystatus + json_renepaystatus }, { "renepay", - json_pay + json_renepay }, { "renesendpay", From 728edf30e4d1d0a618dda0c177c1db510c914b1e Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Thu, 9 Jan 2025 10:51:31 +0100 Subject: [PATCH 3/3] renepay: change the groupid selection Searches the first unused groupid and uses that number for the current payment attempt. We previously used the highest value of used groupid + 1, which breaks in a few corner cases. Changelog-None Signed-off-by: Lagrang3 --- plugins/renepay/mods.c | 45 ++++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/plugins/renepay/mods.c b/plugins/renepay/mods.c index ee9ecd84af67..c97414de6238 100644 --- a/plugins/renepay/mods.c +++ b/plugins/renepay/mods.c @@ -1,4 +1,5 @@ #include "config.h" +#include #include #include #include @@ -15,8 +16,6 @@ #include #include -#define INVALID_ID UINT64_MAX - #define OP_NULL NULL #define OP_CALL (void *)1 #define OP_IF (void *)2 @@ -932,6 +931,15 @@ REGISTER_PAYMENT_MODIFIER(checktimeout, checktimeout_cb); * we should return payment_success inmediately. */ +static int cmp_u64(const u64 *a, const u64 *b, void *unused) +{ + if (*a < *b) + return -1; + if (*a > *b) + return 1; + return 0; +} + static struct command_result *pendingsendpays_done(struct command *cmd, const char *method UNUSED, const char *buf, @@ -941,11 +949,12 @@ static struct command_result *pendingsendpays_done(struct command *cmd, size_t i; const char *err; const jsmntok_t *t, *arr; - u64 max_group_id = 0; /* Data for pending payments, this will be the one * who's result gets replayed if we end up suspending. */ - u64 pending_group_id = INVALID_ID; + bool has_pending = false; + u64 unused_groupid; + u64 pending_group_id COMPILER_WANTS_INIT("12.3.0-17ubuntu1 -O3"); u64 max_pending_partid = 0; struct amount_msat pending_sent = AMOUNT_MSAT(0), pending_msat = AMOUNT_MSAT(0); @@ -975,6 +984,8 @@ static struct command_result *pendingsendpays_done(struct command *cmd, return payment_success(payment, &success.preimage); } + u64 *groupid_arr = tal_arr(tmpctx, u64, 0); + // find if there is one pending group json_for_each_arr(i, t, arr) { @@ -994,10 +1005,12 @@ static struct command_result *pendingsendpays_done(struct command *cmd, __func__, err); if (streq(status, "pending")) { + has_pending = true; pending_group_id = groupid; - break; } + tal_arr_expand(&groupid_arr, groupid); } + assert(tal_count(groupid_arr) == arr->size); /* We need two loops to get the highest partid for a groupid that has * pending sendpays. */ @@ -1028,12 +1041,8 @@ static struct command_result *pendingsendpays_done(struct command *cmd, "following error: %s", __func__, err); - /* If we decide to create a new group, we base it on - * max_group_id */ - if (groupid > max_group_id) - max_group_id = groupid; - - if (groupid == pending_group_id && partid > max_pending_partid) + if (has_pending && groupid == pending_group_id && + partid > max_pending_partid) max_pending_partid = partid; /* status could be completed, pending or failed */ @@ -1057,7 +1066,17 @@ static struct command_result *pendingsendpays_done(struct command *cmd, assert(!streq(status, "complete")); } - if (pending_group_id != INVALID_ID) { + /* find the first unused groupid */ + unused_groupid = 1; + asort(groupid_arr, tal_count(groupid_arr), cmp_u64, NULL); + for (i = 0; i < tal_count(groupid_arr); i++) { + if (unused_groupid < groupid_arr[i]) + break; + if (unused_groupid == groupid_arr[i]) + unused_groupid++; + } + + if (has_pending) { /* Continue where we left off? */ payment->groupid = pending_group_id; payment->next_partid = max_pending_partid + 1; @@ -1075,7 +1094,7 @@ static struct command_result *pendingsendpays_done(struct command *cmd, } else { /* There are no pending nor completed sendpays, get me the last * sendpay group. */ - payment->groupid = max_group_id + 1; + payment->groupid = unused_groupid; payment->next_partid = 1; payment->total_sent = AMOUNT_MSAT(0); payment->total_delivering = AMOUNT_MSAT(0);