askrene: consider fees during *explain failure*
What the developer wrote
Authored by Lagrang3
askrene: consider fees during *explain failure*
We would try to explain a failure by simply finding a path between
source and destination and then checking the constraints.
However, we did not add fees.
Changelog-None
Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
What changed, and why it matters
This commit fixes a bug in Core Lightning's 'askrene' plugin, which is used to explain why a Lightning Network payment failed. Previously, when explaining a failure, the plugin checked whether each channel along a route could carry the payment amount, but it ignored the fees that must also be carried by earlier channels. This could cause askrene to incorrectly report that a path should have worked, or to miss the real reason a payment failed. The fix calculates the rolling amount including fees from destination back to source, and uses that larger amount when checking channel capacity and limits.
Apply the patch. Operators relying on askrene failure explanations should upgrade so that reported failure reasons accurately reflect fee requirements. No immediate emergency response is indicated, as this is a diagnostic correctness fix rather than an active exploit.
Security signals we found
Incorrect failure diagnosis could mislead operators about why a payment failed
Fee omission could cause underestimation of required channel capacity
Potential fee overflow now explicitly handled
No direct funds loss or remote code execution path evident
Evidence from the diff
In plugins/askrene/explain_failure.c, explain_failure() previously validated each hop against the original amount without accounting for fees accumulated upstream. The patch introduces a backward pass over hops, computing path_amount[i] as the amount that must traverse hop i including downstream fees (base + proportional). It also adds handling for fee overflow. Subsequent constraint checks (capacity, max, htlc_max, htlc_min) now compare against path_amount[i] instead of amount. This corrects the failure explanation logic but does not change actual payment routing.
Changed components
plugins/askrene/explain_failure.cCore Lightning askrene pluginInspect captured patch +36 / −9
diff --git a/plugins/askrene/explain_failure.c b/plugins/askrene/explain_failure.c
index f9720db0..be1f137c 100644
--- a/plugins/askrene/explain_failure.c
+++ b/plugins/askrene/explain_failure.c
@@ -215,6 +215,11 @@ const char *explain_failure(const tal_t *ctx,
const struct dijkstra *dij;
char *path;
const char *cap_check;
+ const char *explanation;
+ struct short_channel_id_dir scidd;
+ struct gossmap_chan *c;
+ struct amount_msat rolling_amount;
+ struct amount_msat *path_amount;
/* Do we have enough funds? */
cap_check = check_capacity(ctx, rq, srcnode, OUT_OF_NODE,
@@ -247,11 +252,33 @@ const char *explain_failure(const tal_t *ctx,
fmt_short_channel_id(tmpctx, hops[i].scid));
}
+ path_amount = tal_arr(tmpctx, struct amount_msat, tal_count(hops));
+ rolling_amount = amount;
+ for (size_t i = tal_count(hops) - 1; i < tal_count(hops); i--) {
+ scidd.scid = hops[i].scid;
+ scidd.dir = hops[i].direction;
+ c = gossmap_find_chan(rq->gossmap, &scidd.scid);
+
+ path_amount[i] = rolling_amount;
+ if (!amount_msat_add_fee(&rolling_amount,
+ c->half[scidd.dir].base_fee,
+ c->half[scidd.dir].proportional_fee)) {
+ /* Should not happen, but since the branch exists we use
+ * it. */
+ explanation = tal_fmt(
+ tmpctx, "produces a fee overflow for amount %s",
+ fmt_amount_msat(tmpctx, rolling_amount));
+ return rq_log(ctx, rq, LOG_INFORM,
+ NO_USABLE_PATHS_STRING
+ " The shortest path is %s, but %s %s",
+ path,
+ fmt_short_channel_id_dir(tmpctx, &scidd),
+ explanation);
+ }
+ }
+
/* Now walk through this: is it disabled? Insuff capacity? */
for (size_t i = 0; i < tal_count(hops); i++) {
- const char *explanation;
- struct short_channel_id_dir scidd;
- struct gossmap_chan *c;
struct amount_msat cap_msat, min, max, htlc_max, htlc_min;
scidd.scid = hops[i].scid;
@@ -266,16 +293,16 @@ const char *explain_failure(const tal_t *ctx,
explanation = "has no gossip";
else if (!c->half[scidd.dir].enabled)
explanation = describe_disabled(tmpctx, rq, c, &scidd);
- else if (amount_msat_greater(amount, cap_msat))
- explanation = describe_capacity(tmpctx, rq, &scidd, amount);
- else if (amount_msat_greater(amount, max))
+ else if (amount_msat_greater(path_amount[i], cap_msat))
+ explanation = describe_capacity(tmpctx, rq, &scidd, path_amount[i]);
+ else if (amount_msat_greater(path_amount[i], max))
explanation = why_max_constrained(tmpctx, rq,
- &scidd, amount);
- else if (amount_msat_greater(amount, htlc_max))
+ &scidd, path_amount[i]);
+ else if (amount_msat_greater(path_amount[i], htlc_max))
explanation = tal_fmt(tmpctx,
"exceeds htlc_maximum_msat ~%s",
fmt_amount_msat(tmpctx, htlc_max));
- else if (amount_msat_less(amount, htlc_min))
+ else if (amount_msat_less(path_amount[i], htlc_min))
explanation = tal_fmt(tmpctx,
"below htlc_minumum_msat ~%s",
fmt_amount_msat(tmpctx, htlc_min));
Why this scored 37/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.