Prefactor: Simplify `last_notification_sent` tracking
What changed, and why it matters
This commit is a code cleanup (prefactor) that changes how a webhook notification cooldown is tracked. Previously, the system remembered the last time each type of notification was sent separately. Now it only remembers the last time any notification was sent. This means a user could hit the cooldown for one kind of alert and then not receive a different kind of alert for up to a minute, even though the old behavior would have allowed it. The change is intentional and documented in the commit message, and the tests were updated to match. It is not a hidden vulnerability, but it does slightly broaden when notifications can be suppressed.
Treat as a behavior-affecting refactor rather than a security patch. Review whether a global one-minute cooldown for all LSPS5 webhook notifications is acceptable for downstream users, and ensure documentation or release notes mention the changed semantics. No urgent security action is required.
Security signals we found
Rate-limiting logic changed from per-method to global cooldown
Behavioral test expectations changed to match new cooldown semantics
Commit explicitly describes the change as intentional simplification, not a fix
No input validation, memory safety, or cryptographic changes present
Evidence from the diff
The patch refactors Webhook::last_notification_sent from HashMap<WebhookNotificationMethod, LSPSDateTime> to Option<LSPSDateTime>. As a result, the per-method cooldown described in bLIP-55 becomes a global cooldown across all notification methods. The rate-limit check now uses a single timestamp for any prior notification, and peer_connected resets that single timestamp. Tests were adjusted to advance the mock clock between different notification methods and to remove a test case that previously asserted a different method could bypass the cooldown. The commit message explicitly frames this as a deliberate simplification after the cooldown was reduced to one minute elsewhere.
Changed components
lightning-liquidity/src/lsps5/service.rslightning-liquidity/tests/lsps5_integration_tests.rsInspect captured patch +36 / −40
diff --git a/lightning-liquidity/src/lsps5/service.rs b/lightning-liquidity/src/lsps5/service.rs
index bb02509..e956ebe 100644
--- a/lightning-liquidity/src/lsps5/service.rs
+++ b/lightning-liquidity/src/lsps5/service.rs
@@ -54,9 +54,9 @@ struct Webhook {
// 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>,
+ // Timestamp when we last sent a notification to the client. This is used to enforce
+ // notification cooldowns.
+ last_notification_sent: Option<LSPSDateTime>,
}
/// Server-side configuration options for LSPS5 Webhook Registration.
@@ -184,11 +184,8 @@ where
match client_webhooks.entry(params.app_name.clone()) {
Entry::Occupied(mut entry) => {
no_change = entry.get().url == params.webhook;
- let (last_used, last_notification_sent) = if no_change {
- (entry.get().last_used, entry.get().last_notification_sent.clone())
- } else {
- (now, new_hash_map())
- };
+ let last_used = if no_change { entry.get().last_used } else { now };
+ let last_notification_sent = entry.get().last_notification_sent;
entry.insert(Webhook {
_app_name: params.app_name.clone(),
url: params.webhook.clone(),
@@ -217,7 +214,7 @@ where
url: params.webhook.clone(),
_counterparty_node_id: counterparty_node_id,
last_used: now,
- last_notification_sent: new_hash_map(),
+ last_notification_sent: None,
});
},
}
@@ -425,11 +422,9 @@ where
// (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.duration_since(&last_sent))
- .map_or(false, |duration| duration < NOTIFICATION_COOLDOWN_TIME)
+ webhook.last_notification_sent.as_ref().map_or(false, |last_sent| {
+ now.duration_since(&last_sent) < NOTIFICATION_COOLDOWN_TIME
+ })
});
if rate_limit_applies {
@@ -438,14 +433,14 @@ where
}
for (app_name, webhook) in client_webhooks.iter_mut() {
- webhook.last_notification_sent.insert(notification.method.clone(), now);
- webhook.last_used = now;
self.send_notification(
client_id,
app_name.clone(),
webhook.url.clone(),
notification.clone(),
)?;
+ webhook.last_used = now;
+ webhook.last_notification_sent = Some(now);
}
Ok(())
}
@@ -527,7 +522,7 @@ where
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();
+ webhook.last_notification_sent = None;
}
}
}
diff --git a/lightning-liquidity/tests/lsps5_integration_tests.rs b/lightning-liquidity/tests/lsps5_integration_tests.rs
index 9035755..7b24826 100644
--- a/lightning-liquidity/tests/lsps5_integration_tests.rs
+++ b/lightning-liquidity/tests/lsps5_integration_tests.rs
@@ -411,11 +411,13 @@ fn webhook_error_handling_test() {
#[test]
fn webhook_notification_delivery_test() {
+ 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, validator) = lsps5_test_setup(nodes, Arc::new(DefaultTimeProvider));
+ let (lsps_nodes, validator) = 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();
@@ -499,6 +501,8 @@ fn webhook_notification_delivery_test() {
"No event should be emitted due to cooldown"
);
+ mock_time_provider.advance_time(NOTIFICATION_COOLDOWN_TIME.as_secs() + 1);
+
let timeout_block = 700000; // Some future block height
let _ = service_handler.notify_expiry_soon(client_node_id, timeout_block);
@@ -719,11 +723,13 @@ fn idempotency_set_webhook_test() {
#[test]
fn replay_prevention_test() {
+ 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, validator) = lsps5_test_setup(nodes, Arc::new(DefaultTimeProvider));
+ let (lsps_nodes, validator) = 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();
@@ -774,6 +780,9 @@ fn replay_prevention_test() {
// Fill up the validator's signature cache to push out the original signature.
for i in 0..MAX_RECENT_SIGNATURES {
+ // Advance time, allowing for another notification
+ mock_time_provider.advance_time(NOTIFICATION_COOLDOWN_TIME.as_secs() + 1);
+
let timeout_block = 700000 + i as u32;
let _ = service_handler.notify_expiry_soon(client_node_id, timeout_block);
let event = service_node.liquidity_manager.next_event().unwrap();
@@ -871,11 +880,13 @@ fn stale_webhooks() {
#[test]
fn test_all_notifications() {
+ 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, validator) = lsps5_test_setup(nodes, Arc::new(DefaultTimeProvider));
+ let (lsps_nodes, validator) = 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();
@@ -894,9 +905,16 @@ fn test_all_notifications() {
// consume initial SendWebhookNotification
let _ = service_node.liquidity_manager.next_event().unwrap();
+ mock_time_provider.advance_time(NOTIFICATION_COOLDOWN_TIME.as_secs() + 1);
let _ = service_handler.notify_onion_message_incoming(client_node_id);
+
+ mock_time_provider.advance_time(NOTIFICATION_COOLDOWN_TIME.as_secs() + 1);
let _ = service_handler.notify_payment_incoming(client_node_id);
+
+ mock_time_provider.advance_time(NOTIFICATION_COOLDOWN_TIME.as_secs() + 1);
let _ = service_handler.notify_expiry_soon(client_node_id, 1000);
+
+ mock_time_provider.advance_time(NOTIFICATION_COOLDOWN_TIME.as_secs() + 1);
let _ = service_handler.notify_liquidity_management_request(client_node_id);
let expected_notifications = vec![
@@ -1101,24 +1119,7 @@ fn test_send_notifications_and_peer_connected_resets_cooldown() {
"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
+ // 3. Advance time past cooldown and ensure payment_incoming can be sent again
mock_time_provider.advance_time(NOTIFICATION_COOLDOWN_TIME.as_secs() + 1);
let _ = service_handler.notify_payment_incoming(client_node_id);
@@ -1133,7 +1134,7 @@ fn test_send_notifications_and_peer_connected_resets_cooldown() {
_ => panic!("Expected SendWebhookNotification event after cooldown"),
}
- // 5. Can't send payment_incoming notification again immediately after cooldown
+ // 4. 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();
@@ -1144,7 +1145,7 @@ fn test_send_notifications_and_peer_connected_resets_cooldown() {
"Should not emit event due to cooldown"
);
- // 6. After peer_connected, notification should be sent again immediately
+ // 5. After peer_connected, notification should be sent again immediately
let init_msg = Init {
features: lightning_types::features::InitFeatures::empty(),
remote_network_address: None,
Why this scored 20/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.