Add LSPS5 webhook notification support - #993
Camillarhi wants to merge 1 commit into
Conversation
|
🎉 This PR is now ready for review! |
1d750fb to
9d4f022
Compare
2601d0f to
9eb148f
Compare
| }, | ||
| }; | ||
|
|
||
| pending_set_webhook_requests_lock.insert(request_id, sender); |
There was a problem hiding this comment.
LSPS1/LSPS2 wrap the same kind of map in PendingRequest/PendingRequestGuard (see client/lsps2.rs), held across the .await and removing its own entry on drop.
Here nothing removes the entry when tokio::time::timeout(...) in lsps5_set_webhook/lsps5_list_webhooks/lsps5_remove_webhook gives up — the oneshot::Sender and its HashMap entry stay behind for the life of the node. On a node with a slow or flaky LSPS5 LSP this grows unbounded. Same pattern at L124 (pending_list_webhooks_requests) and L176 (pending_remove_webhook_requests)
Could we reuse PendingRequestGuard here, the way LSPS2 does? None of these three calls need the fan-out (followers) side of PendingRequest — only one caller ever awaits a given set_webhook/list_webhooks/remove_webhook — but the drop-cleanup is exactly what's missing.
There was a problem hiding this comment.
Thanks! This has been updated to use PendingRequestGuard just like LSPS2 does
| let lsps2_service_config = | ||
| self.lsps2_service.as_ref().map(|s| s.ldk_service_config.clone()); | ||
| let lsps5_service_config = self.lsps5_service.clone(); | ||
| let advertise_service = self |
There was a problem hiding this comment.
advertise_service sets the shared LSPS feature bit for any configured service, not just LSPS2 — but this only reads it off lsps2_service. A node with enable_liquidity_provider(None, Some(lsps5_cfg)) always gets advertise_service = false, with no way to turn it on.
Is that intentional, or should LSPS5-only providers be able to advertise too?
There was a problem hiding this comment.
Yeah, it's reachable. Though the flag sets the shared LSPS0, so it was never really an LSPS2 thing. If I add it to the LSPS5 config too, then LSPS1 service lands, and that's three copies of the same flag. One node-level setting is probably where this should end up, so I'll take a look at that instead of duplicating it
| e | ||
| ), | ||
| } | ||
| Error::LiquidityNotifyWebhookFailed |
There was a problem hiding this comment.
SlowDownError (the notification cooldown) and every other failure both map to Error::LiquidityNotifyWebhookFailed. A caller can't tell "you're rate-limited, retry shortly" from "this genuinely failed" without parsing logs.
Worth a distinct Error::LiquidityNotifyRateLimited (or similar) so callers can branch on it?
There was a problem hiding this comment.
Thanks. This will be updated to return a distict error for slow down
Implement the bLIP-55 / LSPS5 webhook registration protocol on top of the
multi-LSP liquidity module (src/liquidity/{client,service}).
Client side, exposed via Node::liquidity().lsps5():
- set_webhook / list_webhooks / remove_webhook to manage webhook
registrations with a given LSP. Each takes the LSP's node ID explicitly:
bLIP-55 has the notification service verify the x-lsps5-signature header
against the signing LSP's node ID, so a registration is meaningful only
for one LSP at a time.
Service side, enabled by passing an LSPS5ServiceConfig to
Builder::enable_liquidity_provider():
- Deliver outgoing webhook notifications over HTTPS in response to
LSPS5ServiceEvent::SendWebhookNotification.
- Send lsps5.payment_incoming when an inbound HTLC forward to a client
fails because the client is offline (wired from LdkEvent::
HTLCHandlingFailed with LocalHTLCFailureReason::PeerOffline).
- Send lsps5.onion_message_incoming when an intercepted onion message
targets a client that is currently offline.
- Send lsps5.expiry_soon from a periodic task that scans channels for outbound
HTLCs approaching their cltv_expiry, so a client that went offline holding an
HTLC has a chance to come online and settle before it expires.
Adds integration tests covering webhook registration and the
payment_incoming trigger, and wires the feature through the UniFFI
bindings.
3 participants
Integrates LSPS5 (bLIP-0055) from lightning-liquidity, enabling webhook-based push notifications so clients can be alerted to events while their app is offline. Built on the refactored multi-LSP liquidity module
(src/liquidity/{client,service}).When
node_idisNone,set_webhookandremove_webhookfan out to every LSPS5-capable LSP, so a webhook can be configured once across all configured LSPs.Lets a node act as an LSPS5 server:
LSPS5ServiceEvent::SendWebhookNotification.onion_message_incomingnotification when an intercepted onion message targets a client that is currently offline (wired fromLdkEvent::OnionMessageIntercepted, gated on peer connectivity via the peer manager).Reopening after an accidental force-push pushed the branch to main's tip and auto-closed #729.
Fixes: #1017