bookkeeper: restore limit on asking for all channelmoves at once.
What the developer wrote
Authored by Rusty Russell
bookkeeper: restore limit on asking for all channelmoves at once.
Now we've found all the issues, the latency spike (4 seconds on my laptop)
for querying 2M elements remains.
Restore the limited sampling which we reverted, but make it 10,000 now.
This doesn't help our worst-case latency, because sql still asks for all 2M entries on
first access. We address that next.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This is a performance fix for the bookkeeper plugin. It changes how the plugin asks Core Lightning for lists of channel movements, requesting them in batches of 10,000 instead of all at once. The commit message says this avoids a multi-second delay when there are millions of entries. There is no direct security vulnerability here, but very large unbounded queries can contribute to denial-of-service-like latency or memory pressure, so the patch is defensively relevant.
Treat as a routine performance and availability hardening patch. No urgent security action required. Users running nodes with very large channel move histories may notice reduced startup/refresh latency. Continue monitoring for the follow-up SQL-level optimization mentioned in the commit message.
Security signals we found
Unbounded RPC query replaced with paginated/batched query
Latency spike (~4 seconds) with 2M records reduced by batching
Potential denial-of-service vector via large datastore/channelmove sets
No input validation, authentication, or cryptographic changes
Evidence from the diff
The patch reintroduces batched listchannelmoves RPC calls with a LISTCHANNELMOVES_LIMIT of 10,000, using index=created, start=channelmoves_index+1, and limit. Previously the plugin requested all channel moves at once, causing a ~4 second latency spike with 2M rows. The new helper limited_listchannelmoves() recurses through listchannelmoves_done() when the returned array size equals the limit. This is a resource-consumption/availability improvement, not a memory-safety or cryptographic bug fix.
Changed components
plugins/bkpr/bookkeeper.cbookkeeper pluginlistchannelmoves RPC handlingrefresh_info lifecycleInspect captured patch +27 / −8
diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c
index 57948311..87caf878 100644
--- a/plugins/bkpr/bookkeeper.c
+++ b/plugins/bkpr/bookkeeper.c
@@ -55,6 +55,10 @@ static struct refresh_info *use_rinfo(struct refresh_info *rinfo)
return rinfo;
}
+/* Recursion */
+static struct command_result *limited_listchannelmoves(struct command *cmd,
+ struct refresh_info *rinfo);
+
static struct command_result *rinfo_one_done(struct command *cmd,
struct refresh_info *rinfo)
{
@@ -105,6 +109,7 @@ static struct fee_sum *find_sum_for_txid(struct fee_sum **sums,
return NULL;
}
+#define LISTCHANNELMOVES_LIMIT 10000
static struct command_result *listchannelmoves_done(struct command *cmd,
const char *method,
const char *buf,
@@ -126,16 +131,36 @@ static struct command_result *listchannelmoves_done(struct command *cmd,
"create-or-replace",
datastore_done, NULL, use_rinfo(rinfo));
+ /* If there might be more, try asking for more */
+ if (moves->size == LISTCHANNELMOVES_LIMIT)
+ limited_listchannelmoves(cmd, rinfo);
+
return rinfo_one_done(cmd, rinfo);
}
+/* We do 1000 at a time to avoid overwhelming lightningd */
+static struct command_result *limited_listchannelmoves(struct command *cmd,
+ struct refresh_info *rinfo)
+{
+ struct bkpr *bkpr = bkpr_of(cmd->plugin);
+ struct out_req *req;
+
+ req = jsonrpc_request_start(cmd, "listchannelmoves",
+ listchannelmoves_done,
+ plugin_broken_cb,
+ use_rinfo(rinfo));
+ json_add_string(req->js, "index", "created");
+ json_add_u64(req->js, "start", bkpr->channelmoves_index + 1);
+ json_add_u64(req->js, "limit", LISTCHANNELMOVES_LIMIT);
+ return send_outreq(req);
+}
+
static struct command_result *listchainmoves_done(struct command *cmd,
const char *method,
const char *buf,
const jsmntok_t *result,
struct refresh_info *rinfo)
{
- struct out_req *req;
const jsmntok_t *moves, *t;
size_t i;
struct bkpr *bkpr = bkpr_of(cmd->plugin);
@@ -151,13 +176,7 @@ static struct command_result *listchainmoves_done(struct command *cmd,
"create-or-replace",
datastore_done, NULL, use_rinfo(rinfo));
- req = jsonrpc_request_start(cmd, "listchannelmoves",
- listchannelmoves_done,
- plugin_broken_cb,
- use_rinfo(rinfo));
- json_add_string(req->js, "index", "created");
- json_add_u64(req->js, "start", bkpr->channelmoves_index + 1);
- send_outreq(req);
+ limited_listchannelmoves(cmd, rinfo);
return rinfo_one_done(cmd, rinfo);
}
Why this scored 18/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.