diff --git a/lightning-liquidity/src/lsps5/event.rs b/lightning-liquidity/src/lsps5/event.rs index 7730428e5ce..f401c0e10ac 100644 --- a/lightning-liquidity/src/lsps5/event.rs +++ b/lightning-liquidity/src/lsps5/event.rs @@ -31,15 +31,13 @@ pub enum LSPS5ServiceEvent { /// The LSP should send an HTTP POST to the [`url`], using the /// JSON-serialized [`notification`] as the body and including the `headers`. /// If the HTTP request fails, the LSP may implement a retry policy according to its - /// implementation preferences, but must respect rate-limiting as defined in - /// [`notification_cooldown_hours`]. + /// implementation preferences. /// /// The notification is signed using the LSP's node ID to ensure authenticity /// when received by the client. The client verifies this signature using /// [`validate`], which guards against replay attacks and tampering. /// /// [`validate`]: super::validator::LSPS5Validator::validate - /// [`notification_cooldown_hours`]: super::service::LSPS5ServiceConfig::notification_cooldown_hours /// [`url`]: super::msgs::LSPS5WebhookUrl /// [`notification`]: super::msgs::WebhookNotification SendWebhookNotification { diff --git a/lightning-liquidity/src/lsps5/msgs.rs b/lightning-liquidity/src/lsps5/msgs.rs index ef0aaf4f5f4..c61bc2b982d 100644 --- a/lightning-liquidity/src/lsps5/msgs.rs +++ b/lightning-liquidity/src/lsps5/msgs.rs @@ -51,6 +51,8 @@ pub const LSPS5_APP_NAME_NOT_FOUND_ERROR_CODE: i32 = 1010; pub const LSPS5_UNKNOWN_ERROR_CODE: i32 = 1000; /// An error occurred during serialization of LSPS5 webhook notification. pub const LSPS5_SERIALIZATION_ERROR_CODE: i32 = 1001; +/// A notification was sent too frequently. +pub const LSPS5_SLOW_DOWN_ERROR_CODE: i32 = 1002; pub(crate) const LSPS5_SET_WEBHOOK_METHOD_NAME: &str = "lsps5.set_webhook"; pub(crate) const LSPS5_LIST_WEBHOOKS_METHOD_NAME: &str = "lsps5.list_webhooks"; @@ -103,10 +105,18 @@ pub enum LSPS5ProtocolError { /// Error during serialization of LSPS5 webhook notification. SerializationError, + + /// A notification was sent too frequently. + /// + /// This error indicates that the LSP is sending notifications + /// too quickly, violating the notification cooldown [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`] + /// + /// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS + SlowDownError, } impl LSPS5ProtocolError { - /// private code range so we never collide with the spec's codes + /// The error code for the LSPS5 protocol error. pub fn code(&self) -> i32 { match self { LSPS5ProtocolError::AppNameTooLong | LSPS5ProtocolError::WebhookUrlTooLong => { @@ -118,6 +128,7 @@ impl LSPS5ProtocolError { LSPS5ProtocolError::AppNameNotFound => LSPS5_APP_NAME_NOT_FOUND_ERROR_CODE, LSPS5ProtocolError::UnknownError => LSPS5_UNKNOWN_ERROR_CODE, LSPS5ProtocolError::SerializationError => LSPS5_SERIALIZATION_ERROR_CODE, + LSPS5ProtocolError::SlowDownError => LSPS5_SLOW_DOWN_ERROR_CODE, } } /// The error message for the LSPS5 protocol error. @@ -133,6 +144,7 @@ impl LSPS5ProtocolError { LSPS5ProtocolError::SerializationError => { "Error serializing LSPS5 webhook notification" }, + LSPS5ProtocolError::SlowDownError => "Notification sent too frequently", } } } diff --git a/lightning-liquidity/src/lsps5/service.rs b/lightning-liquidity/src/lsps5/service.rs index 97fde364ba1..52d8310f971 100644 --- a/lightning-liquidity/src/lsps5/service.rs +++ b/lightning-liquidity/src/lsps5/service.rs @@ -51,7 +51,11 @@ struct StoredWebhook { _app_name: LSPS5AppName, url: LSPS5WebhookUrl, _counterparty_node_id: PublicKey, + // Timestamp used for tracking when the webhook was created / updated, or when the last notification was sent. + // This is used to determine if the webhook is stale and should be pruned. last_used: LSPSDateTime, + // Map of last notification sent timestamps for each notification method. + // This is used to enforce notification cooldowns. last_notification_sent: HashMap, } @@ -60,8 +64,18 @@ struct StoredWebhook { pub struct LSPS5ServiceConfig { /// Maximum number of webhooks allowed per client. pub max_webhooks_per_client: u32, - /// Minimum time between sending the same notification type in hours (default: 24) - pub notification_cooldown_hours: Duration, +} + +/// Default maximum number of webhooks allowed per client. +pub const DEFAULT_MAX_WEBHOOKS_PER_CLIENT: u32 = 10; +/// Default notification cooldown time in hours. +pub const DEFAULT_NOTIFICATION_COOLDOWN_HOURS: Duration = Duration::from_secs(60 * 60); // 1 hour + +// Default configuration for LSPS5 service. +impl Default for LSPS5ServiceConfig { + fn default() -> Self { + Self { max_webhooks_per_client: DEFAULT_MAX_WEBHOOKS_PER_CLIENT } + } } /// Service-side handler for the [`bLIP-55 / LSPS5`] webhook registration protocol. @@ -78,8 +92,6 @@ pub struct LSPS5ServiceConfig { /// - `lsps5.remove_webhook` -> delete a named webhook or return [`app_name_not_found`] error. /// - Prune stale webhooks after a client has no open channels and no activity for at least /// [`MIN_WEBHOOK_RETENTION_DAYS`]. -/// - Rate-limit repeat notifications of the same method to a client by -/// [`notification_cooldown_hours`]. /// - Sign and enqueue outgoing webhook notifications: /// - Construct JSON-RPC 2.0 Notification objects [`WebhookNotification`], /// - Timestamp and LN-style zbase32-sign each payload, @@ -94,7 +106,6 @@ pub struct LSPS5ServiceConfig { /// [`bLIP-55 / LSPS5`]: https://github.com/lightning/blips/pull/55/files /// [`max_webhooks_per_client`]: super::service::LSPS5ServiceConfig::max_webhooks_per_client /// [`app_name_not_found`]: super::msgs::LSPS5ProtocolError::AppNameNotFound -/// [`notification_cooldown_hours`]: super::service::LSPS5ServiceConfig::notification_cooldown_hours /// [`WebhookNotification`]: super::msgs::WebhookNotification /// [`LSPS5ServiceEvent::SendWebhookNotification`]: super::event::LSPS5ServiceEvent::SendWebhookNotification /// [`app_name`]: super::msgs::LSPS5AppName @@ -318,10 +329,14 @@ where /// This builds a [`WebhookNotificationMethod::LSPS5PaymentIncoming`] webhook notification, signs it with your /// node key, and enqueues HTTP POSTs to all registered webhook URLs for that client. /// + /// This may fail if a similar notification was sent too recently, + /// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]. + /// /// # Parameters /// - `client_id`: the client's node-ID whose webhooks should be invoked. /// /// [`WebhookNotificationMethod::LSPS5PaymentIncoming`]: super::msgs::WebhookNotificationMethod::LSPS5PaymentIncoming + /// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS pub fn notify_payment_incoming(&self, client_id: PublicKey) -> Result<(), LSPS5ProtocolError> { let notification = WebhookNotification::payment_incoming(); self.send_notifications_to_client_webhooks(client_id, notification) @@ -335,11 +350,15 @@ where /// the `timeout` block height, signs it, and enqueues HTTP POSTs to the client's /// registered webhooks. /// + /// This may fail if a similar notification was sent too recently, + /// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]. + /// /// # Parameters /// - `client_id`: the client's node-ID whose webhooks should be invoked. /// - `timeout`: the block height at which the channel contract will expire. /// /// [`WebhookNotificationMethod::LSPS5ExpirySoon`]: super::msgs::WebhookNotificationMethod::LSPS5ExpirySoon + /// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS pub fn notify_expiry_soon( &self, client_id: PublicKey, timeout: u32, ) -> Result<(), LSPS5ProtocolError> { @@ -353,10 +372,14 @@ where /// liquidity for `client_id`. Builds a [`WebhookNotificationMethod::LSPS5LiquidityManagementRequest`] notification, /// signs it, and sends it to all of the client's registered webhook URLs. /// + /// This may fail if a similar notification was sent too recently, + /// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]. + /// /// # Parameters /// - `client_id`: the client's node-ID whose webhooks should be invoked. /// /// [`WebhookNotificationMethod::LSPS5LiquidityManagementRequest`]: super::msgs::WebhookNotificationMethod::LSPS5LiquidityManagementRequest + /// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS pub fn notify_liquidity_management_request( &self, client_id: PublicKey, ) -> Result<(), LSPS5ProtocolError> { @@ -370,10 +393,14 @@ where /// for `client_id` while the client is offline. Builds a [`WebhookNotificationMethod::LSPS5OnionMessageIncoming`] /// notification, signs it, and enqueues HTTP POSTs to each registered webhook. /// + /// This may fail if a similar notification was sent too recently, + /// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]. + /// /// # Parameters /// - `client_id`: the client's node-ID whose webhooks should be invoked. /// /// [`WebhookNotificationMethod::LSPS5OnionMessageIncoming`]: super::msgs::WebhookNotificationMethod::LSPS5OnionMessageIncoming + /// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS pub fn notify_onion_message_incoming( &self, client_id: PublicKey, ) -> Result<(), LSPS5ProtocolError> { @@ -394,24 +421,34 @@ where let now = LSPSDateTime::new_from_duration_since_epoch(self.time_provider.duration_since_epoch()); - for (app_name, webhook) in client_webhooks.iter_mut() { - if webhook - .last_notification_sent - .get(¬ification.method) - .map(|last_sent| now.clone().abs_diff(&last_sent)) - .map_or(true, |duration| { - duration >= self.config.notification_cooldown_hours.as_secs() - }) { - webhook.last_notification_sent.insert(notification.method.clone(), now.clone()); - webhook.last_used = now.clone(); - self.send_notification( - client_id, - app_name.clone(), - webhook.url.clone(), - notification.clone(), - )?; + // We must avoid sending multiple notifications of the same method + // (other than lsps5.webhook_registered) close in time. + if notification.method != WebhookNotificationMethod::LSPS5WebhookRegistered { + let rate_limit_applies = client_webhooks.iter().any(|(_, webhook)| { + webhook + .last_notification_sent + .get(¬ification.method) + .map(|last_sent| now.abs_diff(&last_sent)) + .map_or(false, |duration| { + duration < DEFAULT_NOTIFICATION_COOLDOWN_HOURS.as_secs() + }) + }); + + if rate_limit_applies { + return Err(LSPS5ProtocolError::SlowDownError); } } + + for (app_name, webhook) in client_webhooks.iter_mut() { + webhook.last_notification_sent.insert(notification.method.clone(), now.clone()); + webhook.last_used = now.clone(); + self.send_notification( + client_id, + app_name.clone(), + webhook.url.clone(), + notification.clone(), + )?; + } Ok(()) } @@ -487,6 +524,15 @@ where .iter() .any(|c| c.is_usable && c.counterparty.node_id == *client_id) } + + pub(crate) fn peer_connected(&self, counterparty_node_id: &PublicKey) { + let mut webhooks = self.webhooks.lock().unwrap(); + if let Some(client_webhooks) = webhooks.get_mut(counterparty_node_id) { + for webhook in client_webhooks.values_mut() { + webhook.last_notification_sent.clear(); + } + } + } } impl LSPSProtocolMessageHandler for LSPS5ServiceHandler diff --git a/lightning-liquidity/src/manager.rs b/lightning-liquidity/src/manager.rs index bba3f6ea0d3..27c67d04799 100644 --- a/lightning-liquidity/src/manager.rs +++ b/lightning-liquidity/src/manager.rs @@ -715,8 +715,13 @@ where } } fn peer_connected( - &self, _: bitcoin::secp256k1::PublicKey, _: &lightning::ln::msgs::Init, _: bool, + &self, counterparty_node_id: bitcoin::secp256k1::PublicKey, _: &lightning::ln::msgs::Init, + _: bool, ) -> Result<(), ()> { + if let Some(lsps5_service_handler) = self.lsps5_service_handler.as_ref() { + lsps5_service_handler.peer_connected(&counterparty_node_id); + } + Ok(()) } } diff --git a/lightning-liquidity/tests/lsps0_integration_tests.rs b/lightning-liquidity/tests/lsps0_integration_tests.rs index 7fd7a7c185f..423d49785f2 100644 --- a/lightning-liquidity/tests/lsps0_integration_tests.rs +++ b/lightning-liquidity/tests/lsps0_integration_tests.rs @@ -23,7 +23,6 @@ use lightning::ln::functional_test_utils::{ use lightning::ln::peer_handler::CustomMessageHandler; use std::sync::Arc; -use std::time::Duration; #[test] fn list_protocols_integration_test() { @@ -36,10 +35,7 @@ fn list_protocols_integration_test() { let lsps2_service_config = LSPS2ServiceConfig { promise_secret }; #[cfg(lsps1_service)] let lsps1_service_config = LSPS1ServiceConfig { supported_options: None, token: None }; - let lsps5_service_config = LSPS5ServiceConfig { - max_webhooks_per_client: 10, - notification_cooldown_hours: Duration::from_secs(3600), - }; + let lsps5_service_config = LSPS5ServiceConfig::default(); let service_config = LiquidityServiceConfig { #[cfg(lsps1_service)] lsps1_service_config: Some(lsps1_service_config), diff --git a/lightning-liquidity/tests/lsps5_integration_tests.rs b/lightning-liquidity/tests/lsps5_integration_tests.rs index aa85cf0a1d8..61e7c12f95b 100644 --- a/lightning-liquidity/tests/lsps5_integration_tests.rs +++ b/lightning-liquidity/tests/lsps5_integration_tests.rs @@ -7,6 +7,7 @@ use common::{create_service_and_client_nodes, get_lsps_message, LSPSNodes}; use lightning::ln::functional_test_utils::{ create_chanmon_cfgs, create_network, create_node_cfgs, create_node_chanmgrs, Node, }; +use lightning::ln::msgs::Init; use lightning::ln::peer_handler::CustomMessageHandler; use lightning::util::hash_tables::{HashMap, HashSet}; use lightning_liquidity::events::LiquidityEvent; @@ -17,7 +18,9 @@ use lightning_liquidity::lsps5::msgs::{ LSPS5AppName, LSPS5ClientError, LSPS5ProtocolError, LSPS5WebhookUrl, WebhookNotification, WebhookNotificationMethod, }; -use lightning_liquidity::lsps5::service::LSPS5ServiceConfig; +use lightning_liquidity::lsps5::service::{ + LSPS5ServiceConfig, DEFAULT_MAX_WEBHOOKS_PER_CLIENT, DEFAULT_NOTIFICATION_COOLDOWN_HOURS, +}; use lightning_liquidity::lsps5::service::{ MIN_WEBHOOK_RETENTION_DAYS, PRUNE_STALE_WEBHOOKS_INTERVAL_DAYS, }; @@ -27,18 +30,10 @@ use lightning_liquidity::{LiquidityClientConfig, LiquidityServiceConfig}; use std::sync::{Arc, RwLock}; use std::time::Duration; -/// Default maximum number of webhooks allowed per client. -pub(crate) const DEFAULT_MAX_WEBHOOKS_PER_CLIENT: u32 = 10; -/// Default notification cooldown time in hours. -pub(crate) const DEFAULT_NOTIFICATION_COOLDOWN_HOURS: Duration = Duration::from_secs(24 * 60 * 60); - pub(crate) fn lsps5_test_setup<'a, 'b, 'c>( nodes: Vec>, time_provider: Arc, ) -> (LSPSNodes<'a, 'b, 'c>, LSPS5Validator) { - let lsps5_service_config = LSPS5ServiceConfig { - max_webhooks_per_client: DEFAULT_MAX_WEBHOOKS_PER_CLIENT, - notification_cooldown_hours: DEFAULT_NOTIFICATION_COOLDOWN_HOURS, - }; + let lsps5_service_config = LSPS5ServiceConfig::default(); let service_config = LiquidityServiceConfig { #[cfg(lsps1_service)] lsps1_service_config: None, @@ -1053,3 +1048,118 @@ fn test_notify_without_webhooks_does_nothing() { let _ = service_handler.notify_onion_message_incoming(client_node_id); assert!(service_node.liquidity_manager.next_event().is_none()); } + +#[test] +fn test_send_notifications_and_peer_connected_resets_cooldown() { + let mock_time_provider = Arc::new(MockTimeProvider::new(1000)); + let time_provider = Arc::::clone(&mock_time_provider); + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + let (lsps_nodes, _) = lsps5_test_setup(nodes, time_provider); + let LSPSNodes { service_node, client_node } = lsps_nodes; + let service_node_id = service_node.inner.node.get_our_node_id(); + let client_node_id = client_node.inner.node.get_our_node_id(); + + let client_handler = client_node.liquidity_manager.lsps5_client_handler().unwrap(); + let service_handler = service_node.liquidity_manager.lsps5_service_handler().unwrap(); + + let app_name = "CooldownTestApp"; + let webhook_url = "https://www.example.org/cooldown"; + let _ = client_handler + .set_webhook(service_node_id, app_name.to_string(), webhook_url.to_string()) + .expect("Register webhook request should succeed"); + let set_req = get_lsps_message!(client_node, service_node_id); + service_node.liquidity_manager.handle_custom_message(set_req, client_node_id).unwrap(); + + let _ = service_node.liquidity_manager.next_event().unwrap(); + + let resp = get_lsps_message!(service_node, client_node_id); + client_node.liquidity_manager.handle_custom_message(resp, service_node_id).unwrap(); + let _ = client_node.liquidity_manager.next_event().unwrap(); + + // 1. First notification should be sent + let _ = service_handler.notify_payment_incoming(client_node_id); + let event = service_node.liquidity_manager.next_event().unwrap(); + match event { + LiquidityEvent::LSPS5Service(LSPS5ServiceEvent::SendWebhookNotification { + notification, + .. + }) => { + assert_eq!(notification.method, WebhookNotificationMethod::LSPS5PaymentIncoming); + }, + _ => panic!("Expected SendWebhookNotification event"), + } + + // 2. Second notification before cooldown should NOT be sent + let result = service_handler.notify_payment_incoming(client_node_id); + let error = result.unwrap_err(); + assert_eq!(error, LSPS5ProtocolError::SlowDownError); + assert!( + service_node.liquidity_manager.next_event().is_none(), + "Should not emit event due to cooldown" + ); + + // 3. Notification of a different method CAN be sent + let timeout_block = 424242; + let _ = service_handler.notify_expiry_soon(client_node_id, timeout_block); + let event = service_node.liquidity_manager.next_event().unwrap(); + match event { + LiquidityEvent::LSPS5Service(LSPS5ServiceEvent::SendWebhookNotification { + notification, + .. + }) => { + assert!(matches!( + notification.method, + WebhookNotificationMethod::LSPS5ExpirySoon { timeout } if timeout == timeout_block + )); + }, + _ => panic!("Expected SendWebhookNotification event for expiry_soon"), + } + + // 4. Advance time past cooldown and ensure payment_incoming can be sent again + mock_time_provider.advance_time(DEFAULT_NOTIFICATION_COOLDOWN_HOURS.as_secs() + 1); + + let _ = service_handler.notify_payment_incoming(client_node_id); + let event = service_node.liquidity_manager.next_event().unwrap(); + match event { + LiquidityEvent::LSPS5Service(LSPS5ServiceEvent::SendWebhookNotification { + notification, + .. + }) => { + assert_eq!(notification.method, WebhookNotificationMethod::LSPS5PaymentIncoming); + }, + _ => panic!("Expected SendWebhookNotification event after cooldown"), + } + + // 5. Can't send payment_incoming notification again immediately after cooldown + let result = service_handler.notify_payment_incoming(client_node_id); + + let error = result.unwrap_err(); + assert_eq!(error, LSPS5ProtocolError::SlowDownError); + + assert!( + service_node.liquidity_manager.next_event().is_none(), + "Should not emit event due to cooldown" + ); + + // 6. After peer_connected, notification should be sent again immediately + let init_msg = Init { + features: lightning_types::features::InitFeatures::empty(), + remote_network_address: None, + networks: None, + }; + service_node.liquidity_manager.peer_connected(client_node_id, &init_msg, false).unwrap(); + let _ = service_handler.notify_payment_incoming(client_node_id); + let event = service_node.liquidity_manager.next_event().unwrap(); + match event { + LiquidityEvent::LSPS5Service(LSPS5ServiceEvent::SendWebhookNotification { + notification, + .. + }) => { + assert_eq!(notification.method, WebhookNotificationMethod::LSPS5PaymentIncoming); + }, + _ => panic!("Expected SendWebhookNotification event after peer_connected"), + } +}