Add LSPS5 webhook notification support #729
Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Thanks for taking this over!
Need to look into this PR closer, here are some early comments that I noted when briefly scrolling through.
Cargo.toml
Outdated
| bdk_wallet = { version = "2.2.0", default-features = false, features = ["std", "keys-bip39"]} | ||
|
|
||
| reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls"] } | ||
| reqwest = { version = "0.12", default-features = false, features = ["json", "rustls-tls", "blocking"] } |
There was a problem hiding this comment.
We shouldn't ever add the blocking feature for reqwest as it's fundamentally incompatible with the async variant.
src/liquidity.rs
Outdated
| impl std::error::Error for HttpClientError {} | ||
|
|
||
| /// Trait for making HTTP requests. | ||
| pub trait HttpClient: Send + Sync + std::fmt::Debug { |
There was a problem hiding this comment.
All of this code seems like an outdated copy of what was there on the upstream LSPS5 PR at one point in time. Please remove anything related to HttpClient etc. Rather, we should just ... make the call via spawned async task using reqwest when we handle the SendWebhookNotification event.
There was a problem hiding this comment.
Thanks for the feedback! I've updated the PR to use reqwest and removed the HttpClient dependencies. This is still a work in progress, but that should address your concern about the outdated approach.
src/liquidity.rs
Outdated
| /// | ||
| /// [bLIP-55 / LSPS5]: https://github.com/lightning/blips/blob/master/blip-0055.md | ||
| #[derive(Debug, Clone)] | ||
| pub struct LSPS5ServiceConfig { |
There was a problem hiding this comment.
Not sure why we need to replicate this struct?
Camillarhi
force-pushed
the
lsps5
branch
5 times, most recently
from
4a3ef12 to
3fa9f5e
Compare
Integrates LSPS5 (BLIP-0055) from lightning-liquidity to enable webhook-based push notifications for clients. - Add event system integration for LSPS5 events - Expose public API for webhook management This allows client developers to register webhook endpoints with their LSP to receive notifications when their app is offline.
3 participants
Integrates LSPS5 (BLIP-0055) from lightning-liquidity to enable webhook-based push notifications for clients.
This allows client developers to register webhook endpoints with their LSP to receive notifications when their app is offline.