askrene: make increase_flows function more generic.
What changed, and why it matters
This commit rewrites a routing helper inside Core Lightning's experimental 'askrene' plugin so it correctly accounts for overlapping payment routes when increasing payment amounts. The old code sorted routes and increased each independently, which could ignore shared channel capacity limits. The new code reserves capacity across all routes, picks the route with the most spare capacity, and stops if it cannot meet the target. There is no direct evidence this is a security fix, but incorrect capacity accounting in payment routing can lead to failed routes, stuck payments, or unintended fee/HTLC behavior.
Treat as a normal code-quality/robustness improvement. Reviewers should verify that the new reservation loop correctly restores reservations after each temporary removal, that tal_free(reservations) does not leave dangling pointers, and that the bool return value is handled appropriately by callers. No immediate security response is indicated by the available evidence.
Security signals we found
Algorithmic correctness fix in payment routing capacity reservation
Change from independent per-flow increases to global reservation-aware allocation
Potential for prior code to over-allocate across overlapping routes, causing route/payment failures or inconsistent HTLC reservations
No explicit security framing, CVE, or advisory in commit or supplied references
Evidence from the diff
The patch refactors increase_flows() in plugins/askrene/refine.c. Previously it sorted flows by delivered amount and greedily added up to tolerance*delivers or max_deliverable per flow, without considering that increasing one flow reduces capacity available to overlapping flows. The rewrite: (1) computes a per-flow ceiling (tolerance-based or deliver target if tolerance negative), (2) repeatedly checks aggregate shortage via flows_short(), (3) creates reservations for all flows, removes each reservation in turn to measure real remaining capacity with flow_max_deliverable(), (4) selects the flow with the greatest remaining capacity, (5) adds either 1/n of the shortage or the whole shortage if under 10 satoshis, and (6) returns false if any step cannot satisfy the shortage. The function signature changes from returning struct amount_msat to returning bool. The commit message frames this as making the function ‘more generic’ and ‘properly takes into account interactions between flows by using reservations.’
Changed components
plugins/askrene/refine.cincrease_flows() functionflow reservation logic (reserve_hop, new_reservations, create_flow_reservations, flow_max_deliverable)askrene payment refinement/routing pathInspect captured patch +76 / −36
diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c
index 2b9f1b1a..052d125b 100644
--- a/plugins/askrene/refine.c
+++ b/plugins/askrene/refine.c
@@ -383,54 +383,94 @@ static struct amount_msat remove_excess(struct flow ***flows,
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(const struct route_query *rq,
- struct flow **flows,
- struct amount_msat deliver,
- double tolerance)
+ * 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;
- struct amount_msat all_deliver, defect;
- all_deliver = sum_all_deliver(flows);
+ /* 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();
+ }
- /* early exit: target is already met */
- if (!amount_msat_sub(&defect, deliver, all_deliver) ||
- amount_msat_is_zero(defect))
- return all_deliver;
+ /* 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]);
+ }
- asort(flows, tal_count(flows), revcmp_flows, NULL);
+ /* Find flow with most excess capacity. */
+ for (size_t i = 0; i < tal_count(flows); i++) {
+ struct amount_msat capacity, remaining;
- all_deliver = AMOUNT_MSAT(0);
- for (size_t i = 0;
- i < tal_count(flows) && !amount_msat_is_zero(defect); i++) {
- struct flow *flow = flows[i];
- struct amount_msat can_add = defect, amt;
+ /* 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], NULL);
+ reservations[i] = new_reservations(reservations, rq);
+ create_flow_reservations(rq, &reservations[i], flows[i]);
- /* no more than tolerance */
- if (!amount_msat_scale(&amt, flow->delivers, tolerance))
- continue;
- else
- can_add = amount_msat_min(can_add, amt);
+ /* Don't go above our tolerance */
+ if (amount_msat_greater(capacity, ceiling[i]))
+ capacity = ceiling[i];
- /* no more than max_deliverable */
- if (!amount_msat_sub(&amt, flow_max_deliverable(rq, flow, NULL),
- flow->delivers))
- continue;
+ 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);
+
+ /* 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))
+ /* Can't add it? */
+ if (amount_msat_less(best_remaining, addition)) {
+ tal_free(working_ctx);
+ return false;
+ }
+
+ if (!amount_msat_accumulate(&flows[best_flownum]->delivers, addition))
+ abort();
+ if (!amount_msat_deduct(&shortage, addition))
abort();
}
- return all_deliver;
+ tal_free(working_ctx);
+ return true;
}
const char *refine_flows(const tal_t *ctx, struct route_query *rq,
Why this scored 23/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.