Conversation
hieblmi
left a comment
There was a problem hiding this comment.
To catch all future cases where we need to cancel we clould also...
initiationSucceeded := false
defer func() {
if !initiationSucceeded {
cancelProbeInvoice(
globalCtx, cfg.lnd.Invoices, probeHash,
)
}
}()
and in the end
initiationSucceeded = true
return &loopInInitResult{
swap: swap,
serverMessage: swapResp.serverMessage,
}, nil
66089da to
e23925d
Compare
Great improvement! Applied. Note that it could cancel the invoice in some other failing paths where it is not needed, before the server has seen the invoice or after the watcher has already canceled it. Another attempt can return "no such invoice" if the server runs with So I decided to apply the following rule: memorize (in RAM) if the invoice was successfully cancelled and skip the second attempt in that case. I made a shared function for that. This complicated the code a bit, but I think it is still manageable. @hieblmi Please take another look! |
e23925d to
1b60544
Compare
Before a Loop In, the server probes the route to the client by paying a probe hold invoice whose preimage nobody knows. The client's watcher cancels the invoice once the probe HTLC is accepted, failing the probe back to the server. When the server call returns, the client stops waiting for the probe. If initiation fails, the server may still be routing a probe, and an HTLC that arrives afterwards can remain held until shortly before its CLTV expiry. The same applies when the server answers successfully before the probe reaches the client, which fails initiation with a probe error. Defer probe cancellation after successful invoice creation and skip this cleanup only when initiation succeeds. This covers all subsequent failure paths, including future ones. Cleanup uses a context detached from the initiation context, with a ten-second budget covering both waiting for another cancellation and the RPC itself. Give the watcher and deferred cleanup one shared cancellation function per probe. It serializes attempts with a context-aware gate and remembers only cancellations that returned success. Later calls then skip the RPC, including when lnd has already garbage-collected the canceled invoice. The watcher retains its existing cancellation context and RPC timeout. Failed attempts are logged and leave the other caller free to try within its remaining budget; there is no automatic retry loop. Invoice-notfound errors are not suppressed. They can still occur if lnd canceled and deleted the invoice but its response was lost, or if another caller canceled it. The remembered success is local to this probe's lifetime. Add regression coverage for successful cancellation reuse, failure after the watcher has canceled, retrying failed attempts without suppressing invoice-not-found, concurrent callers, and deadlines while waiting.
1b60544 to
a309794
Compare
Before a Loop In, the server probes the route to the client by paying a probe invoice whose preimage nobody knows. The client's probe watcher cancels the invoice once the probe HTLC is accepted, which fails the probe back. The server probes within the
NewLoopInSwapcall, and the client only accepts a successful call if the probe reached it, so a swap never starts before its probe is done.This PR is about initiations that fail. The watcher stops when the call returns, but a failed initiation does not mean that the probe is finished:
A probe HTLC that arrived after that was accepted by the still open hold invoice and held. lnd does not expire an accepted hold invoice by time. It only cancels the HTLC shortly before its CLTV expiry: about 60 blocks later with lnd's defaults, an 80-block invoice CLTV delta and an 18-block hold expiry delta. The server's payment stayed pending along the route for those roughly ten hours.
The client now cancels the probe invoice whenever initiation fails after the invoice was created, including on a probe error. A probe HTLC that arrives later is failed right away, and one that already arrived is failed back.
This matters only when a probe outlives the initiation call. It protects the liquidity of the server and the routing nodes, not the user's funds.
Tests
TestLoopInCancelsProbeInvoiceOnInitiationFailure: the probe invoice is canceled when the server call fails, and when the call succeeds without the probe having reached the client.TestAwaitProbeCancelInvoiceUsesLiveContextNot covered
The cancellation is attempted once. If it fails, it is logged. The open invoice then expires after its hour, and a probe HTLC that arrives before that is held as described above. Retrying is a separate follow-up.
Pull Request Checklist
docs/release-notes/release-notes-next.md, or apply theno-changeloglabel (required by CI)