Repository navigation
Conversation
`lightning-liquidity` requires `Event::ChannelClosed` to be forwarded to the LSPS2 service handler, which we never did. The service hence kept the state of every JIT channel that ever got paid for, and with it the persisted state of the respective client, forever. That state also counts towards the service's global peer limit. We now hand closed channels to the LSPS2 service, allowing it to prune the state of JIT channels that are gone. Co-Authored-By: HAL 9000
When we couldn't open a JIT channel as an LSPS2 service, or when the channel closed before becoming ready, we never told `lightning-liquidity` about it. The intercepted HTLCs were therefore held until they timed out, and as the JIT channel state stayed stuck waiting for the channel, no later payment to the same invoice could trigger another attempt. We now report failed channel opens to the LSPS2 service, which fails the held HTLCs back right away and lets the payer retry. Co-Authored-By: HAL 9000
|
👋 Thanks for assigning @jkczyz as a reviewer! |
049f1a4 to
3a3dafb
Compare
| // A JIT channel closing before it became ready is a failed channel open. This errors | ||
| // for any channel that isn't such a JIT channel, which is the common case. | ||
| if let Err(e) = lsps2_service_handler | ||
| .channel_open_failed(counterparty_node_id, user_channel_id) |
There was a problem hiding this comment.
🤖 If add_event fails after this, the ChannelClosed is replayed and we call channel_open_failed again. If the payer re-sent in the meantime and a second channel is pending, the replay fails its HTLCs and resets the state. That channel's channel_ready then errors, so we fund a channel with nothing to forward.
The window is narrow, but a cheap guard would be to skip this when list_channels_with_counterparty still has another channel with the same user_channel_id.
| ); | ||
| } | ||
|
|
||
| if let Err(e) = lsps2_service_handler.channel_closed(channel_id).await { |
There was a problem hiding this comment.
🤖 lightning-liquidity's channel_closed only prunes JIT channels in PaymentForwarded. A JIT channel that closes after channel_ready but before the payment is forwarded (PendingPaymentForward / PendingPayment) is still never pruned, and channel_open_failed rejects it too. Is that intended, or a follow-up upstream?
| let Some(peer) = peer_manager.peer_by_node_id(&their_network_key) else { | ||
| log_error!( | ||
| self.logger, | ||
| "Failed to open LSPS2 channel to {} due to peer not being not connected.", |
There was a problem hiding this comment.
Pre-existing but need to drop one "not" here.
| // TODO: We fail the payment back here. Eventually we will need to remember | ||
| // the pending requests and regularly retry opening the channel until we | ||
| // succeed. | ||
| self.fail_channel_open(&their_network_key, user_channel_id).await; |
There was a problem hiding this comment.
🤖 The insufficient-funds path is tested, but this one isn't, and it's easy to trigger. For example: stop the client, pay, expect PaymentFailed, then restart the client and check that a retry succeeds.
lightning-liquidityexpects LSPS2 services to forwardEvent::ChannelClosedand to report failed JIT channel opens. We did neither:
the state of every client that ever paid for a channel. That state also
counts towards the service's global peer limit.
timed out, and the JIT state stayed stuck waiting for the channel, so
retrying the same invoice could never trigger another open.
This PR fixes both, one commit each:
ChannelClosedtoLSPS2ServiceHandler::channel_closed.LSPS2ServiceHandler::channel_open_failedwhen we can't open thechannel (peer not connected, insufficient on-chain funds,
create_channelfailing) and when the channel closes before becoming ready. The held HTLCs
are failed back right away and the payer can retry.