Don't hold write lock in `LSPS{1,2}ServiceHandler::peer_disconnected`
What the developer wrote
Authored by Elias Rohrer
Don't hold write lock in `LSPS{1,2}ServiceHandler::peer_disconnected`
.. as there's no need to do so.
What changed, and why it matters
This commit changes two peer-disconnection handlers in the lightning-liquidity crate so they take a read lock instead of a write lock on shared peer state. The stated reason is simply that a write lock is unnecessary. The change reduces lock contention and may prevent a potential performance or liveness issue, but the diff alone does not show a concrete security vulnerability being fixed.
Treat as a minor hardening/performance improvement. Review whether any other paths unnecessarily hold write locks, and verify that the inner peer-state mutex still protects all required mutations.
Security signals we found
Lock downgrade from write to read in peer-disconnection path
Potential reduction of lock contention / denial-of-service surface
No unsafe code, no cryptographic changes, no input validation changes
Evidence from the diff
In LSPS1 and LSPS2 service handlers, peer_disconnected previously acquired a write lock on per_peer_state, then only read from it to retrieve the inner per-peer mutex. The patch downgrades this to a read lock. The inner peer state is still mutated through its own mutex, so correctness is preserved while reducing exclusive locking. No memory safety bug, data race, or cryptographic flaw is visible in the diff.
Changed components
lightning-liquidity/src/lsps1/service.rslightning-liquidity/src/lsps2/service.rsInspect captured patch +2 / −2
diff --git a/lightning-liquidity/src/lsps1/service.rs b/lightning-liquidity/src/lsps1/service.rs
index 478fc29..d02d0f3 100644
--- a/lightning-liquidity/src/lsps1/service.rs
+++ b/lightning-liquidity/src/lsps1/service.rs
@@ -128,7 +128,7 @@ where
}
pub(crate) fn peer_disconnected(&self, counterparty_node_id: PublicKey) {
- let outer_state_lock = self.per_peer_state.write().unwrap();
+ let outer_state_lock = self.per_peer_state.read().unwrap();
if let Some(inner_state_lock) = outer_state_lock.get(&counterparty_node_id) {
let mut peer_state_lock = inner_state_lock.lock().unwrap();
// We clean up the peer state, but leave removing the peer entry to the prune logic in
diff --git a/lightning-liquidity/src/lsps2/service.rs b/lightning-liquidity/src/lsps2/service.rs
index 35942dc..665cda1 100644
--- a/lightning-liquidity/src/lsps2/service.rs
+++ b/lightning-liquidity/src/lsps2/service.rs
@@ -1871,7 +1871,7 @@ where
}
pub(crate) fn peer_disconnected(&self, counterparty_node_id: PublicKey) {
- let outer_state_lock = self.per_peer_state.write().unwrap();
+ let outer_state_lock = self.per_peer_state.read().unwrap();
if let Some(inner_state_lock) = outer_state_lock.get(&counterparty_node_id) {
let mut peer_state_lock = inner_state_lock.lock().unwrap();
// We clean up the peer state, but leave removing the peer entry to the prune logic in
Why this scored 29/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.