Only prune on `peer_{dis}connected`
What the developer wrote
Authored by Elias Rohrer
Only prune on `peer_{dis}connected`
Previously, we'd constantly check whether or not we can prune stale
webhooks. While not wrong, it lead to a bunch of ~unnecessary
operations, especially given that we only prune once a day currently.
Here we move pruning to `peer_connected`/`peer_disconnected`, which is
similar to what we do for LSPS2, and should still be more than enough.
What changed, and why it matters
This commit is a routine performance cleanup, not a security fix. It moves a once-a-day cleanup task (pruning old webhook records) so it only runs when peers connect or disconnect, instead of running during every message-handling operation. The change reduces unnecessary work and avoids briefly holding a write lock during common read-only operations. There is no indication this fixes a vulnerability.
No security action required. Treat as normal code-quality/performance maintenance.
Security signals we found
No security framing in commit title or message
No CVE, advisory, or security reference present
Change reduces lock contention and write-lock usage, which marginally improves availability
No memory safety, cryptographic, or authorization changes
No bug or crash pattern described
Evidence from the diff
The patch removes calls to check_prune_stale_webhooks from handle_custom_message, list_webhooks, remove_webhook, and notify_webhook, and adds them only to peer_connected and peer_disconnected. It also changes list_webhooks from taking a write lock to a read lock. The test is updated to trigger pruning via a disconnect/reconnect cycle before asserting stale webhooks are gone. The commit message frames this as an efficiency improvement aligned with LSPS2 behavior.
Changed components
lightning-liquidity/src/lsps5/service.rslightning-liquidity/tests/lsps5_integration_tests.rsInspect captured patch +13 / −9
diff --git a/lightning-liquidity/src/lsps5/service.rs b/lightning-liquidity/src/lsps5/service.rs
index 189953a..72c3d83 100644
--- a/lightning-liquidity/src/lsps5/service.rs
+++ b/lightning-liquidity/src/lsps5/service.rs
@@ -179,7 +179,6 @@ where
let mut message_queue_notifier = self.pending_messages.notifier();
let mut outer_state_lock = self.per_peer_state.write().unwrap();
- self.check_prune_stale_webhooks(&mut outer_state_lock);
let peer_state =
outer_state_lock.entry(counterparty_node_id).or_insert_with(PeerState::default);
@@ -263,9 +262,7 @@ where
) -> Result<(), LightningError> {
let mut message_queue_notifier = self.pending_messages.notifier();
- let mut outer_state_lock = self.per_peer_state.write().unwrap();
- self.check_prune_stale_webhooks(&mut outer_state_lock);
-
+ let outer_state_lock = self.per_peer_state.read().unwrap();
let app_names =
outer_state_lock.get(&counterparty_node_id).map(|p| p.app_names()).unwrap_or_default();
@@ -285,7 +282,6 @@ where
let mut message_queue_notifier = self.pending_messages.notifier();
let mut outer_state_lock = self.per_peer_state.write().unwrap();
- self.check_prune_stale_webhooks(&mut outer_state_lock);
if let Some(peer_state) = outer_state_lock.get_mut(&counterparty_node_id) {
if peer_state.remove_webhook(¶ms.app_name) {
@@ -410,8 +406,6 @@ where
&self, client_id: PublicKey, notification: WebhookNotification,
) -> Result<(), LSPS5ProtocolError> {
let mut outer_state_lock = self.per_peer_state.write().unwrap();
- self.check_prune_stale_webhooks(&mut outer_state_lock);
-
let peer_state = if let Some(peer_state) = outer_state_lock.get_mut(&client_id) {
peer_state
} else {
@@ -506,6 +500,7 @@ where
if let Some(peer_state) = outer_state_lock.get_mut(counterparty_node_id) {
peer_state.reset_notification_cooldown();
}
+ self.check_prune_stale_webhooks(&mut outer_state_lock);
}
pub(crate) fn peer_disconnected(&self, counterparty_node_id: &PublicKey) {
@@ -513,6 +508,7 @@ where
if let Some(peer_state) = outer_state_lock.get_mut(counterparty_node_id) {
peer_state.reset_notification_cooldown();
}
+ self.check_prune_stale_webhooks(&mut outer_state_lock);
}
}
diff --git a/lightning-liquidity/tests/lsps5_integration_tests.rs b/lightning-liquidity/tests/lsps5_integration_tests.rs
index 9f6e520..e526d3e 100644
--- a/lightning-liquidity/tests/lsps5_integration_tests.rs
+++ b/lightning-liquidity/tests/lsps5_integration_tests.rs
@@ -861,7 +861,15 @@ fn stale_webhooks() {
MIN_WEBHOOK_RETENTION_DAYS.as_secs() + PRUNE_STALE_WEBHOOKS_INTERVAL_DAYS.as_secs(),
);
- // LIST calls prune before executing -> should be empty after advancing time
+ // LIST should be empty after advancing time and reconnection
+ service_node.liquidity_manager.peer_disconnected(client_node_id);
+ 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 _ = client_handler.list_webhooks(service_node_id);
let list_req2 = get_lsps_message!(client_node, service_node_id);
service_node.liquidity_manager.handle_custom_message(list_req2, client_node_id).unwrap();
@@ -1068,7 +1076,7 @@ fn test_notify_without_webhooks_does_nothing() {
}
#[test]
-fn test_send_notifications_and_peer_connected_resets_cooldown() {
+fn test_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);
Why this scored 18/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.