Reservation leak test and mitigation

.msggen.json

@@ -768,6 +768,7 @@
             "AskRene-Unreserve.path[].short_channel_id_dir": 4
         },
         "Askrene-unreserveRequest": {
+            "AskRene-Unreserve.dev_remove_all": 2,
             "AskRene-Unreserve.path[]": 1
         },
         "Askrene-update-channelRequest": {
@@ -4457,6 +4458,10 @@
             "added": "v24.08",
             "deprecated": null
         },
+        "AskRene-Unreserve.dev_remove_all": {
+            "added": "v25.12",
+            "deprecated": null
+        },
         "AskRene-Unreserve.path[]": {
             "added": "v24.08",
             "deprecated": null

contrib/msggen/msggen/schema.json

@@ -1657,6 +1657,11 @@
                 }
               }
             }
+          },
+          "dev_remove_all": {
+            "hidden": true,
+            "type": "boolean",
+            "added": "v25.12"
           }
         }
       },

doc/schemas/askrene-unreserve.json

@@ -39,6 +39,11 @@
             }
           }
         }
+      },
+      "dev_remove_all": {
+        "hidden": true,
+        "type": "boolean",
+        "added": "v25.12"
       }
     }
   },

plugins/askrene/askrene.c

@@ -865,22 +865,29 @@ 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",
-					    fmt_short_channel_id_dir(tmpctx,
-								     &path[i].scidd));
+	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",
+				    fmt_short_channel_id_dir(tmpctx,
+							     &path[i].scidd));
+			}
 		}
- 	}
+	}
 
 	response = jsonrpc_stream_success(cmd);
 	return command_finished(cmd, response);

plugins/askrene/refine.c

@@ -32,8 +32,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));
+		}
+	}
 }
 
 struct reserve_hop *new_reservations(const tal_t *ctx,

plugins/askrene/reserve.c

@@ -73,6 +73,20 @@ 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;
+
+	/* Note!  This may remove the "wrong" one, but since they're only
+	 * differentiated for debugging, that's OK */
+	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)

plugins/askrene/reserve.h

@@ -25,6 +25,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,

tests/test_askrene.py

@@ -1608,3 +1608,90 @@ def test_askrene_timeout(node_factory, bitcoind):
                      layers=['auto.localchans'],
                      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
+
+
+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": []}