Skip to content

Commit 3adc266

Browse files
introduce SLOW_DOWN error so we don't silently fail to send an LSPS5/notification
1 parent 0a50f8b commit 3adc266

File tree

4 files changed

+64
-25
lines changed

4 files changed

+64
-25
lines changed

lightning-liquidity/src/lsps5/event.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,15 +31,13 @@ pub enum LSPS5ServiceEvent {
3131
/// The LSP should send an HTTP POST to the [`url`], using the
3232
/// JSON-serialized [`notification`] as the body and including the `headers`.
3333
/// If the HTTP request fails, the LSP may implement a retry policy according to its
34-
/// implementation preferences, but must respect rate-limiting as defined in
35-
/// [`notification_cooldown_hours`].
34+
/// implementation preferences.
3635
///
3736
/// The notification is signed using the LSP's node ID to ensure authenticity
3837
/// when received by the client. The client verifies this signature using
3938
/// [`validate`], which guards against replay attacks and tampering.
4039
///
4140
/// [`validate`]: super::validator::LSPS5Validator::validate
42-
/// [`notification_cooldown_hours`]: super::service::LSPS5ServiceConfig::notification_cooldown_hours
4341
/// [`url`]: super::msgs::LSPS5WebhookUrl
4442
/// [`notification`]: super::msgs::WebhookNotification
4543
SendWebhookNotification {

lightning-liquidity/src/lsps5/msgs.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ pub const LSPS5_APP_NAME_NOT_FOUND_ERROR_CODE: i32 = 1010;
5151
pub const LSPS5_UNKNOWN_ERROR_CODE: i32 = 1000;
5252
/// An error occurred during serialization of LSPS5 webhook notification.
5353
pub const LSPS5_SERIALIZATION_ERROR_CODE: i32 = 1001;
54+
/// A notification was sent too frequently.
55+
pub const LSPS5_SLOW_DOWN_ERROR_CODE: i32 = 1002;
5456

5557
pub(crate) const LSPS5_SET_WEBHOOK_METHOD_NAME: &str = "lsps5.set_webhook";
5658
pub(crate) const LSPS5_LIST_WEBHOOKS_METHOD_NAME: &str = "lsps5.list_webhooks";
@@ -103,10 +105,18 @@ pub enum LSPS5ProtocolError {
103105

104106
/// Error during serialization of LSPS5 webhook notification.
105107
SerializationError,
108+
109+
/// A notification was sent too frequently.
110+
///
111+
/// This error indicates that the LSP is sending notifications
112+
/// too quickly, violating the notification cooldown [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]
113+
///
114+
/// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS
115+
SlowDownError,
106116
}
107117

108118
impl LSPS5ProtocolError {
109-
/// private code range so we never collide with the spec's codes
119+
/// The error code for the LSPS5 protocol error.
110120
pub fn code(&self) -> i32 {
111121
match self {
112122
LSPS5ProtocolError::AppNameTooLong | LSPS5ProtocolError::WebhookUrlTooLong => {
@@ -118,6 +128,7 @@ impl LSPS5ProtocolError {
118128
LSPS5ProtocolError::AppNameNotFound => LSPS5_APP_NAME_NOT_FOUND_ERROR_CODE,
119129
LSPS5ProtocolError::UnknownError => LSPS5_UNKNOWN_ERROR_CODE,
120130
LSPS5ProtocolError::SerializationError => LSPS5_SERIALIZATION_ERROR_CODE,
131+
LSPS5ProtocolError::SlowDownError => LSPS5_SLOW_DOWN_ERROR_CODE,
121132
}
122133
}
123134
/// The error message for the LSPS5 protocol error.
@@ -133,6 +144,7 @@ impl LSPS5ProtocolError {
133144
LSPS5ProtocolError::SerializationError => {
134145
"Error serializing LSPS5 webhook notification"
135146
},
147+
LSPS5ProtocolError::SlowDownError => "Notification sent too frequently",
136148
}
137149
}
138150
}

lightning-liquidity/src/lsps5/service.rs

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ impl Default for LSPS5ServiceConfig {
9292
/// - `lsps5.remove_webhook` -> delete a named webhook or return [`app_name_not_found`] error.
9393
/// - Prune stale webhooks after a client has no open channels and no activity for at least
9494
/// [`MIN_WEBHOOK_RETENTION_DAYS`].
95-
/// - Rate-limit repeat notifications of the same method to a client by
96-
/// [`notification_cooldown_hours`].
9795
/// - Sign and enqueue outgoing webhook notifications:
9896
/// - Construct JSON-RPC 2.0 Notification objects [`WebhookNotification`],
9997
/// - Timestamp and LN-style zbase32-sign each payload,
@@ -108,7 +106,6 @@ impl Default for LSPS5ServiceConfig {
108106
/// [`bLIP-55 / LSPS5`]: https://github.com/lightning/blips/pull/55/files
109107
/// [`max_webhooks_per_client`]: super::service::LSPS5ServiceConfig::max_webhooks_per_client
110108
/// [`app_name_not_found`]: super::msgs::LSPS5ProtocolError::AppNameNotFound
111-
/// [`notification_cooldown_hours`]: super::service::LSPS5ServiceConfig::notification_cooldown_hours
112109
/// [`WebhookNotification`]: super::msgs::WebhookNotification
113110
/// [`LSPS5ServiceEvent::SendWebhookNotification`]: super::event::LSPS5ServiceEvent::SendWebhookNotification
114111
/// [`app_name`]: super::msgs::LSPS5AppName
@@ -332,10 +329,14 @@ where
332329
/// This builds a [`WebhookNotificationMethod::LSPS5PaymentIncoming`] webhook notification, signs it with your
333330
/// node key, and enqueues HTTP POSTs to all registered webhook URLs for that client.
334331
///
332+
/// This may fail if a similar notification was sent too recently,
333+
/// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`].
334+
///
335335
/// # Parameters
336336
/// - `client_id`: the client's node-ID whose webhooks should be invoked.
337337
///
338338
/// [`WebhookNotificationMethod::LSPS5PaymentIncoming`]: super::msgs::WebhookNotificationMethod::LSPS5PaymentIncoming
339+
/// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS
339340
pub fn notify_payment_incoming(&self, client_id: PublicKey) -> Result<(), LSPS5ProtocolError> {
340341
let notification = WebhookNotification::payment_incoming();
341342
self.send_notifications_to_client_webhooks(client_id, notification)
@@ -349,11 +350,15 @@ where
349350
/// the `timeout` block height, signs it, and enqueues HTTP POSTs to the client's
350351
/// registered webhooks.
351352
///
353+
/// This may fail if a similar notification was sent too recently,
354+
/// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`].
355+
///
352356
/// # Parameters
353357
/// - `client_id`: the client's node-ID whose webhooks should be invoked.
354358
/// - `timeout`: the block height at which the channel contract will expire.
355359
///
356360
/// [`WebhookNotificationMethod::LSPS5ExpirySoon`]: super::msgs::WebhookNotificationMethod::LSPS5ExpirySoon
361+
/// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS
357362
pub fn notify_expiry_soon(
358363
&self, client_id: PublicKey, timeout: u32,
359364
) -> Result<(), LSPS5ProtocolError> {
@@ -367,10 +372,14 @@ where
367372
/// liquidity for `client_id`. Builds a [`WebhookNotificationMethod::LSPS5LiquidityManagementRequest`] notification,
368373
/// signs it, and sends it to all of the client's registered webhook URLs.
369374
///
375+
/// This may fail if a similar notification was sent too recently,
376+
/// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`].
377+
///
370378
/// # Parameters
371379
/// - `client_id`: the client's node-ID whose webhooks should be invoked.
372380
///
373381
/// [`WebhookNotificationMethod::LSPS5LiquidityManagementRequest`]: super::msgs::WebhookNotificationMethod::LSPS5LiquidityManagementRequest
382+
/// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS
374383
pub fn notify_liquidity_management_request(
375384
&self, client_id: PublicKey,
376385
) -> Result<(), LSPS5ProtocolError> {
@@ -384,10 +393,14 @@ where
384393
/// for `client_id` while the client is offline. Builds a [`WebhookNotificationMethod::LSPS5OnionMessageIncoming`]
385394
/// notification, signs it, and enqueues HTTP POSTs to each registered webhook.
386395
///
396+
/// This may fail if a similar notification was sent too recently,
397+
/// violating the notification cooldown period defined in [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`].
398+
///
387399
/// # Parameters
388400
/// - `client_id`: the client's node-ID whose webhooks should be invoked.
389401
///
390402
/// [`WebhookNotificationMethod::LSPS5OnionMessageIncoming`]: super::msgs::WebhookNotificationMethod::LSPS5OnionMessageIncoming
403+
/// [`DEFAULT_NOTIFICATION_COOLDOWN_HOURS`]: super::service::DEFAULT_NOTIFICATION_COOLDOWN_HOURS
391404
pub fn notify_onion_message_incoming(
392405
&self, client_id: PublicKey,
393406
) -> Result<(), LSPS5ProtocolError> {
@@ -408,24 +421,34 @@ where
408421
let now =
409422
LSPSDateTime::new_from_duration_since_epoch(self.time_provider.duration_since_epoch());
410423

411-
for (app_name, webhook) in client_webhooks.iter_mut() {
412-
if webhook
413-
.last_notification_sent
414-
.get(&notification.method)
415-
.map(|last_sent| now.clone().abs_diff(&last_sent))
416-
.map_or(true, |last_sent| {
417-
last_sent >= self.config.notification_cooldown_hours.as_secs()
418-
}) {
419-
webhook.last_notification_sent.insert(notification.method.clone(), now.clone());
420-
webhook.last_used = now.clone();
421-
self.send_notification(
422-
client_id,
423-
app_name.clone(),
424-
webhook.url.clone(),
425-
notification.clone(),
426-
)?;
424+
// We must avoid sending multiple notifications of the same method
425+
// (other than lsps5.webhook_registered) close in time.
426+
if notification.method != WebhookNotificationMethod::LSPS5WebhookRegistered {
427+
let rate_limit_applies = client_webhooks.iter().any(|(_, webhook)| {
428+
webhook
429+
.last_notification_sent
430+
.get(&notification.method)
431+
.map(|last_sent| now.abs_diff(&last_sent))
432+
.map_or(false, |duration| {
433+
duration < DEFAULT_NOTIFICATION_COOLDOWN_HOURS.as_secs()
434+
})
435+
});
436+
437+
if rate_limit_applies {
438+
return Err(LSPS5ProtocolError::SlowDownError);
427439
}
428440
}
441+
442+
for (app_name, webhook) in client_webhooks.iter_mut() {
443+
webhook.last_notification_sent.insert(notification.method.clone(), now.clone());
444+
webhook.last_used = now.clone();
445+
self.send_notification(
446+
client_id,
447+
app_name.clone(),
448+
webhook.url.clone(),
449+
notification.clone(),
450+
)?;
451+
}
429452
Ok(())
430453
}
431454

lightning-liquidity/tests/lsps5_integration_tests.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1093,7 +1093,9 @@ fn test_send_notifications_and_peer_connected_resets_cooldown() {
10931093
}
10941094

10951095
// 2. Second notification before cooldown should NOT be sent
1096-
let _ = service_handler.notify_payment_incoming(client_node_id);
1096+
let result = service_handler.notify_payment_incoming(client_node_id);
1097+
let error = result.unwrap_err();
1098+
assert_eq!(error, LSPS5ProtocolError::SlowDownError);
10971099
assert!(
10981100
service_node.liquidity_manager.next_event().is_none(),
10991101
"Should not emit event due to cooldown"
@@ -1132,7 +1134,11 @@ fn test_send_notifications_and_peer_connected_resets_cooldown() {
11321134
}
11331135

11341136
// 5. Can't send payment_incoming notification again immediately after cooldown
1135-
let _ = service_handler.notify_payment_incoming(client_node_id);
1137+
let result = service_handler.notify_payment_incoming(client_node_id);
1138+
1139+
let error = result.unwrap_err();
1140+
assert_eq!(error, LSPS5ProtocolError::SlowDownError);
1141+
11361142
assert!(
11371143
service_node.liquidity_manager.next_event().is_none(),
11381144
"Should not emit event due to cooldown"

0 commit comments

Comments
 (0)