askrene bug: The shortest path is reserve but it should not

[vincenzopalazzo]

Updating from 24.05 to 25.05 I noted a strange bug in the xpay (or renepay path finding algo) that have some sort fo memory leak that do not free the previous route reservetion that cause the failure of the pay command with the following error. In particular what bring me to assume that there is a memory leak is Raw error: cln err code: 205- messageFailed: We could not find a usable set of paths. The shortest path is .... but 0x0x0/1 already reserved 1813499msat by command "cln4go/6528/cln:xpay-as-pay#1558717/cln-xpay:askrene-reserve#134186/cln:askrene-reserve#1558904" (723567 seconds ago)`

Raw error: cln err code: `205` - message `Failed: We could not find a usable set of paths. The shortest path is 918993x2243x0->916866x678x6->903128x1195x0->0x0x0, but 0x0x0/1 already reserved 1813499msat by command \"cln4go/6528/cln:xpay-as-pay#1558717/cln-xpay:askrene-reserve#134186/cln:askrene-reserve#1558904\" (723567 seconds ago), 1813499msat by command \"cln4go/6528/cln:xpay-as-pay#1558717/cln-xpay:askrene-reserve#134221/cln:askrene-reserve#1559005\" (723560 seconds ago), 7936000msat by command \"cln4go/5539/cln:xpay-as-pay#1644637/cln-xpay:askrene-reserve#150200/cln:askrene-reserve#1644652\" (619295 seconds ago), 3047000msat by command \"cln4go/4082/cln:xpay-as-pay#1770630/cln-xpay:askrene-reserve#166889/cln:askrene-reserve#1770644\" (346055 seconds ago), 2494012msat by command \"cln4go/7669/cln:xpay-as-pay#2054222/cln-xpay:getroutes#192393/cln:getroutes#2054255\" (24982 seconds ago)`

This error got resolved by restarting the node, but due that the payout and the node it is fully automated this "restart" and retrying is causing a few problems!

---

This bug is found in the ocean node that process around 700 bolt12 payment a day and I did not spend time in trying to reproducing it, but the setup of ocean is kind of unique and look like the following one: Ocean Node -> exit node (a single one) -> Lightning network -> miner, so the exit note and the last of to reach the miner is most luckily the same channel across several weeks

[Lagrang3]

xpay adds and removes reservations in packets using askrene-reserve and askrene-unreserve.
While I've seen some code within askrene that adds values to existing reservations.
I think this can cause troubles in concurrent payments.
For example:
Some payment starts with xpay, we make a reservation of 10 sats on channel C.
While this payment is pending, another request arrives, and we call getroutes and then refine adds 2 sats to the same channel C.
Now channel C has 12 sats reserved.
But the first payment completes and tries to unreserve 10 sats.
askrene-unreserve is called but it finds that channel C has a reservation for 12 sats, not 10.
An UNUSUAL log is produced "askrene-unreserve failed ...", and those 10 sats don't get removed.

I need to check whether this can actually happen.
But if it does, we can avoid it by always adding reservations in packets or always adding reservations on top of existing reservations.
The mix of both can cause these leaks.

Another problem that I have noticed is the collision of fake channels across payments.
Channel 0x0x0 is clearly a fake channel. We use that trick to handle blinded paths, ie. one fake channel for each blinded path.
The problem is that if we have two concurrent payments going on we would like to make a distinction between
channel 0x0x0 for payment 1 and channel 0x0x0 for payment 2.
We could have that separation if reservations "lived" inside a layer, but unfortunately they are global.

[Lagrang3]

In version 25.05 I noticed this line:

lightning/plugins/askrene/refine.c

Line 595 in 2b8b709

tal_add_destructor2(reservations, destroy_reservations, get_askrene(rq->plugin));

which I think it should be:

tal_del_destructor2(reservations, destroy_reservations, get_askrene(rq->plugin));

so that we don't remove the same reservations twice.
If we try to remove twice an existing reservation, we might remove a reservation created by another payment and a
future call to askrene-unreserve may truncate

lightning/plugins/askrene/askrene.c

Line 888 in 2b8b709

if (!reserve_remove(askrene->reserved, &path[i])) {

leading to some reservations at the end of the path to leak.

An indication that this is what happened is to see something like askrene-unreserve failed: followed by a string containing Unknown reservation for.

We don't have that line in the newest version after the refactoring of askrene (#8358 ). But to make sure I will try to build a test that leaks reservation in 25.05 and add it to the current regression suite test_askrene.py.