LSPS5: Correct notification cooldown & reset logic

per method cooldown enforced correctly, and reset
after peer_connected event

Author: martinsaposnic

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 {

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<WebhookNotificationMethod, LSPSDateTime>,
 }
 
@@ -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
@@ -399,9 +410,8 @@ where
 				.last_notification_sent
 				.get(&notification.method)
 				.map(|last_sent| now.clone().abs_diff(&last_sent))
-				.map_or(true, |duration| {
-					duration >= self.config.notification_cooldown_hours.as_secs()
-				}) {
+				.map_or(true, |duration| duration >= DEFAULT_NOTIFICATION_COOLDOWN_HOURS.as_secs())
+			{
 				webhook.last_notification_sent.insert(notification.method.clone(), now.clone());
 				webhook.last_used = now.clone();
 				self.send_notification(
@@ -487,6 +497,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<CM: Deref, NS: Deref, TP: Deref> LSPSProtocolMessageHandler for LSPS5ServiceHandler<CM, NS, TP>

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(())
 	}
 }

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),

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<Node<'a, 'b, 'c>>, time_provider: Arc<dyn TimeProvider + Send + Sync>,
 ) -> (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,112 @@ 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::<MockTimeProvider>::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 _ = service_handler.notify_payment_incoming(client_node_id);
+	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 _ = service_handler.notify_payment_incoming(client_node_id);
+	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"),
+	}
+}