From 795bd9da30d77906cb530f080439387d55cfaf02 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Mon, 23 Feb 2026 14:09:22 +1030 Subject: [PATCH 1/4] askrene: log reservation failures during getroutes Log when a reservation removal failures during getroutes computation. Failed reservation removals can lead to reservation leaks. Changelog-None Signed-off-by: Lagrang3 --- plugins/askrene/child/refine.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/plugins/askrene/child/refine.c b/plugins/askrene/child/refine.c index 05adc78e4841..2556e4876c1a 100644 --- a/plugins/askrene/child/refine.c +++ b/plugins/askrene/child/refine.c @@ -27,8 +27,15 @@ static void get_scidd(const struct gossmap *gossmap, static void destroy_reservations(struct reserve_hop *rhops, struct reserve_htable *reserved) { - for (size_t i = 0; i < tal_count(rhops); i++) - reserve_remove(reserved, &rhops[i]); + for (size_t i = 0; i < tal_count(rhops); i++) { + if (!reserve_remove(reserved, &rhops[i])) { + child_log(tmpctx, + LOG_BROKEN, + "reserve_remove failed: %s on %s", + fmt_amount_msat(tmpctx, rhops[i].amount), + fmt_short_channel_id_dir(tmpctx, &rhops[i].scidd)); + } + } } struct reserve_hop *new_reservations(const tal_t *ctx, From ffbf7b853db645adc1c1efcd3cd5f481bfaee26e Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Mon, 23 Feb 2026 14:09:24 +1030 Subject: [PATCH 2/4] askrene: add test for reservation leaks Changelog-None. Signed-off-by: Lagrang3 --- tests/test_askrene.py | 60 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/test_askrene.py b/tests/test_askrene.py index 09a669e704f2..df6db06f0af9 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -1847,6 +1847,66 @@ def test_askrene_timeout(node_factory, bitcoind): maxfee_msat=1, final_cltv=5) + +def test_reservations_leak(node_factory, executor): + l1, l2, l3, l4, l5, l6 = node_factory.get_nodes( + 6, + opts=[ + {"fee-base": 0, "fee-per-satoshi": 0}, + {"fee-base": 0, "fee-per-satoshi": 0}, + { + "fee-base": 0, + "fee-per-satoshi": 0, + "plugin": os.path.join(os.getcwd(), "tests/plugins/hold_htlcs.py"), + }, + {"fee-base": 0, "fee-per-satoshi": 0}, + {"fee-base": 0, "fee-per-satoshi": 0}, + {"fee-base": 1000, "fee-per-satoshi": 0}, + ], + ) + + # There must be a common non-local channel in both payment paths. + # With a local channel we cannot trigger the reservation leak because we + # reserve slightly different amounts locally due to HTLC onchain costs. + node_factory.join_nodes([l1, l2, l4, l6, l3], wait_for_announce=True) + node_factory.join_nodes([l1, l2, l4, l5], wait_for_announce=True) + + # Use offers instead of bolt11 because we are going to pay through a blinded + # path and trigger a fake channel collision between both payments. + offer1 = l3.rpc.offer("any")["bolt12"] + offer2 = l5.rpc.offer("any")["bolt12"] + + inv1 = l1.rpc.fetchinvoice(offer1, "100sat")["invoice"] + inv2 = l1.rpc.fetchinvoice(offer2, "101sat")["invoice"] + + # Initiate the first payment that has a delay. + fut = executor.submit(l1.rpc.xpay, (inv1)) + + # Wait for the first payment to reserve the path. + l1.daemon.wait_for_log(r"json_askrene_reserve called") + + # A second payment starts. + l1.rpc.xpay(inv2) + l1.daemon.wait_for_log(r"json_askrene_unreserve called") + + l3.daemon.wait_for_log(r"Holding onto an incoming htlc for 10 seconds") + + # There is a payment pending therefore we expect reservations. + reservations = l1.rpc.askrene_listreservations() + assert reservations != {"reservations": []} + + l3.daemon.wait_for_log(r"htlc_accepted hook called") + fut.result() + l1.daemon.wait_for_log(r"json_askrene_unreserve called") + + # The first payment has finished we expect no reservations. + reservations = l1.rpc.askrene_listreservations() + assert reservations == {"reservations": []} + + # We shouldn't fail askrene-unreserve. If it does it means something went + # wrong. + assert l1.daemon.is_in_log("askrene-unreserve failed") is None + # It will exit instantly. l1.rpc.setconfig('askrene-timeout', 0) From 38eeb79e9f461089816af1c617177a0e7eac5c3b Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Mon, 23 Feb 2026 14:09:24 +1030 Subject: [PATCH 3/4] askrene-unreserve: add dev_remove_all option Signed-off-by: Lagrang3 --- doc/schemas/askrene-unreserve.json | 5 +++++ plugins/askrene/askrene.c | 24 +++++++++++++++--------- plugins/askrene/reserve.c | 12 ++++++++++++ plugins/askrene/reserve.h | 3 +++ tests/test_askrene.py | 27 +++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/doc/schemas/askrene-unreserve.json b/doc/schemas/askrene-unreserve.json index ab1f88ce7815..47c9fbaffd2b 100644 --- a/doc/schemas/askrene-unreserve.json +++ b/doc/schemas/askrene-unreserve.json @@ -46,6 +46,11 @@ } } } + }, + "dev_remove_all": { + "hidden": true, + "type": "boolean", + "added": "v25.12" } } }, diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index c16aa6501654..54219e5ea39d 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -946,24 +946,30 @@ static struct command_result *json_askrene_unreserve(struct command *cmd, struct reserve_hop *path; struct json_stream *response; struct askrene *askrene = get_askrene(cmd->plugin); + bool *remove_all; if (!param(cmd, buffer, params, p_req("path", param_reserve_path, &path), + p_opt_dev("dev_remove_all", param_bool, &remove_all, false), NULL)) return command_param_failed(); plugin_log(cmd->plugin, LOG_TRACE, "%s called: %.*s", __func__, json_tok_full_len(params), json_tok_full(buffer, params)); - for (size_t i = 0; i < tal_count(path); i++) { - if (!reserve_remove(askrene->reserved, &path[i])) { - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "Unknown reservation for %s%s%s", - fmt_short_channel_id_dir(tmpctx, - &path[i].scidd), - path[i].layer ? " on layer " : "", - path[i].layer ? layer_name(path[i].layer) : ""); + if (*remove_all) { + reserve_remove_all(askrene->reserved); + } else { + for (size_t i = 0; i < tal_count(path); i++) { + if (!reserve_remove(askrene->reserved, &path[i])) { + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Unknown reservation for %s%s%s", + fmt_short_channel_id_dir(tmpctx, + &path[i].scidd), + path[i].layer ? " on layer " : "", + path[i].layer ? layer_name(path[i].layer) : ""); + } } - } + } response = jsonrpc_stream_success(cmd); return command_finished(cmd, response); diff --git a/plugins/askrene/reserve.c b/plugins/askrene/reserve.c index 95ca5d8c1c0d..4d8441524b92 100644 --- a/plugins/askrene/reserve.c +++ b/plugins/askrene/reserve.c @@ -88,6 +88,18 @@ bool reserve_remove(struct reserve_htable *reserved, return false; } +void reserve_remove_all(struct reserve_htable *reserved) +{ + struct reserve *r; + struct reserve_htable_iter rit; + + for (r = reserve_htable_first(reserved, &rit); r; + r = reserve_htable_next(reserved, &rit)) { + tal_free(r); + } + reserve_htable_clear(reserved); +} + void reserves_clear_capacities(struct reserve_htable *reserved, const struct gossmap *gossmap, fp16_t *capacities) diff --git a/plugins/askrene/reserve.h b/plugins/askrene/reserve.h index c58ff72ca1ac..af616dcb67c0 100644 --- a/plugins/askrene/reserve.h +++ b/plugins/askrene/reserve.h @@ -29,6 +29,9 @@ void reserve_add(struct reserve_htable *reserved, bool reserve_remove(struct reserve_htable *reserved, const struct reserve_hop *rhop); +/* Remove all reservations. */ +void reserve_remove_all(struct reserve_htable *reserved); + /* Clear capacities array where we have reserves */ void reserves_clear_capacities(struct reserve_htable *reserved, const struct gossmap *gossmap, diff --git a/tests/test_askrene.py b/tests/test_askrene.py index df6db06f0af9..ac6cdae120b3 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -1907,6 +1907,33 @@ def test_reservations_leak(node_factory, executor): # wrong. assert l1.daemon.is_in_log("askrene-unreserve failed") is None + +def test_unreserve_all(node_factory): + """Test removing all reservations.""" + l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True) + + # initially no reserves + assert l1.rpc.askrene_listreservations() == {"reservations": []} + + # then we add a couple of reservations + scid12 = first_scid(l1, l2) + scid23 = first_scid(l2, l3) + scid12dir = f"{scid12}/{direction(l1.info['id'], l2.info['id'])}" + scid23dir = f"{scid23}/{direction(l2.info['id'], l3.info['id'])}" + l1.rpc.askrene_reserve( + path=[ + {"short_channel_id_dir": scid12dir, "amount_msat": 1000_000}, + {"short_channel_id_dir": scid23dir, "amount_msat": 1000_001}, + ] + ) + + listres = l1.rpc.askrene_listreservations()["reservations"] + assert len(listres) == 2 + + # remove all reservations + l1.rpc.askrene_unreserve(path=[], dev_remove_all=True) + assert l1.rpc.askrene_listreservations() == {"reservations": []} + # It will exit instantly. l1.rpc.setconfig('askrene-timeout', 0) From db53bc456c78e379b90bb2916ca26af7be287bcb Mon Sep 17 00:00:00 2001 From: ScuttoZ Date: Wed, 11 Mar 2026 17:06:32 +0100 Subject: [PATCH 4/4] askrene: added reservations leak test under load --- tests/test_askrene.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/test_askrene.py b/tests/test_askrene.py index ac6cdae120b3..4a9387999cfe 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -8,10 +8,12 @@ ) import os import pytest +import random import subprocess import time import tempfile import unittest +from concurrent import futures as concurrent_futures def direction(src, dst): @@ -1908,6 +1910,47 @@ def test_reservations_leak(node_factory, executor): assert l1.daemon.is_in_log("askrene-unreserve failed") is None +def test_reservations_leak_under_load(node_factory, executor): + """Stress-test reservation cleanup: concurrent payments over shared channels + must leave zero stale reservations after all payments settle.""" + # Topology: two paths share l4 as a bottleneck relay. + # Path A: l1 -> l2 -> l4 -> l5 + # Path B: l1 -> l3 -> l4 -> l6 + # join_nodes([l1, l2, l4, l5]) creates channels: l1-l2, l2-l4, l4-l5 + # join_nodes([l1, l3, l4, l6]) creates channels: l1-l3, l3-l4, l4-l6 + zero_fee = {"fee-base": 0, "fee-per-satoshi": 0} + l1, l2, l3, l4, l5, l6 = node_factory.get_nodes( + 6, + opts=[zero_fee] * 6, + ) + node_factory.join_nodes([l1, l2, l4, l5], wait_for_announce=True) # creates channels: l1-l2, l2-l4, l4-l5 + node_factory.join_nodes([l1, l3, l4, l6], wait_for_announce=True) # creates channels: l1-l3, l3-l4, l4-l6 + + NUM = 300 + invoices = [l5.rpc.invoice(1000, f"inv-a-{i}", "x")["bolt11"] for i in range(NUM // 2)] + invoices += [l6.rpc.invoice(1000, f"inv-b-{i}", "x")["bolt11"] for i in range(NUM // 2)] + random.shuffle(invoices) + + futs = [executor.submit(l1.rpc.xpay, inv) for inv in invoices] + + # While payments are in flight, reservations must be non-empty: this + # checks the test isn't trivially passing on an empty table. + wait_for(lambda: l1.rpc.askrene_listreservations()["reservations"] != []) + + # Make sure that we have channel contention by looking for repeating scids + def has_channel_contention(): + res = l1.rpc.askrene_listreservations()["reservations"] + scids = [r["short_channel_id_dir"] for r in res] + return len(scids) != len(set(scids)) + wait_for(has_channel_contention) + + for f in concurrent_futures.as_completed(futs, timeout=TIMEOUT): + f.result() # raise on any payment failure + + assert l1.rpc.askrene_listreservations() == {"reservations": []} + assert l1.daemon.is_in_log("reserve_remove failed") is None + + def test_unreserve_all(node_factory): """Test removing all reservations.""" l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True)