From e15614cfc89f2b47440d587ed3c72e9f38cfb843 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Fri, 7 Nov 2025 11:29:26 +0100 Subject: [PATCH 1/3] askrene: fix reservation leak in version 25.05 Changelog-Fixed: fix reservation leak in version 25.05 Signed-off-by: Lagrang3 --- plugins/askrene/refine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index 51f030ade8e8..ed88c39bf71a 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -592,7 +592,7 @@ refine_with_fees_and_limits(const tal_t *ctx, /* Total flowset probability is now easily calculated given reservations * contains the total amounts through each channel (once we remove them) */ destroy_reservations(reservations, get_askrene(rq->plugin)); - tal_add_destructor2(reservations, destroy_reservations, get_askrene(rq->plugin)); + tal_del_destructor2(reservations, destroy_reservations, get_askrene(rq->plugin)); *flowset_probability = 1.0; for (size_t i = 0; i < tal_count(reservations); i++) { From 2dc03f4b5932f73416b87a8d8e9c899b6dce5295 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Fri, 7 Nov 2025 11:31:44 +0100 Subject: [PATCH 2/3] 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/refine.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/plugins/askrene/refine.c b/plugins/askrene/refine.c index ed88c39bf71a..a7ef28142589 100644 --- a/plugins/askrene/refine.c +++ b/plugins/askrene/refine.c @@ -22,8 +22,15 @@ static void get_scidd(const struct gossmap *gossmap, static void destroy_reservations(struct reserve_hop *rhops, struct askrene *askrene) { - for (size_t i = 0; i < tal_count(rhops); i++) - reserve_remove(askrene->reserved, &rhops[i]); + for (size_t i = 0; i < tal_count(rhops); i++){ + if (!reserve_remove(askrene->reserved, &rhops[i])) { + plugin_log( + askrene->plugin, LOG_BROKEN, + "reserve_remove failed: %s on %s", + fmt_amount_msat(tmpctx, rhops[i].amount), + fmt_short_channel_id_dir(tmpctx, &rhops[i].scidd)); + } + } } static struct reserve_hop *new_reservations(const tal_t *ctx, From 4199c2a171013febecbca6f366b82ddb4db5f272 Mon Sep 17 00:00:00 2001 From: Lagrang3 Date: Fri, 7 Nov 2025 11:28:38 +0100 Subject: [PATCH 3/3] 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 8944fc262001..d8b282cf22ed 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -1473,3 +1473,63 @@ def test_simple_dummy_channel(node_factory): final_cltv=5, layers=["mylayer"], ) + + +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