Don't prune `pending_requests` in `persist`
What the developer wrote
Authored by Elias Rohrer
Don't prune `pending_requests` in `persist`
Previously, we would also prune any pending `GetInfo` or expired `Buy`
requests before we `persist`. This could have lead to races where we
drop a pending request and even remove the peer when calling `persist`.
Here, we simply split the pruning logic for the `pending_requests` and
expired JIT channel state, and only prune the latter before persisting.
This generally makes sense, as `pending_requests` isn't currently
persisted, so there is no need to prune before repersisting.
Both are however still pruned on peer disconnection.
What changed, and why it matters
This change fixes a race condition in a Lightning service's save-to-disk routine. Previously, the code would clean up certain in-flight peer requests right before saving state, which could accidentally drop a valid request and even disconnect a peer. Now, cleanup of those in-flight requests is done only when a peer disconnects, while saving only cleans up truly expired channel state. The fix reduces the chance of legitimate requests being lost or peers being wrongly removed.
Review the persist and peer-disconnection paths to confirm no other combined cleanup remains. Add regression tests that simulate an in-flight GetInfo/Buy request during persist to ensure the request and peer are not dropped. Monitor for any follow-up fixes in this area.
Security signals we found
Race condition between request pruning and state persistence
Potential accidental peer removal during persist
In-flight LSPS2 GetInfo/Buy request loss
Separation of volatile request cleanup from persisted state cleanup
Evidence from the diff
In lightning-liquidity/src/lsps2/service.rs, the single method prune_expired_request_state was doing two things: pruning pending_requests (GetInfo and expired Buy) and pruning expired outbound JIT channel state. Because persist called this combined method, pending_requests could be dropped during persistence, potentially causing races that removed a peer. The patch splits the logic: prune_pending_requests handles pending_requests and is only invoked on peer disconnection; prune_expired_request_state now only prunes outbound_channels_by_intercept_scid and remains called from persist. pending_requests is not persisted, so there was no need to prune it before persisting.
Changed components
lightning-liquidity/src/lsps2/service.rsPeerState::prune_expired_request_statePeerState::prune_pending_requestsPeer disconnection cleanup pathpersist pathInspect captured patch +4 / −1
diff --git a/lightning-liquidity/src/lsps2/service.rs b/lightning-liquidity/src/lsps2/service.rs
index 53210d6..a6736e6 100644
--- a/lightning-liquidity/src/lsps2/service.rs
+++ b/lightning-liquidity/src/lsps2/service.rs
@@ -619,7 +619,7 @@ impl PeerState {
self.needs_persist |= true;
}
- fn prune_expired_request_state(&mut self) {
+ fn prune_pending_requests(&mut self) {
self.pending_requests.retain(|_, entry| {
match entry {
LSPS2Request::GetInfo(_) => false,
@@ -629,7 +629,9 @@ impl PeerState {
},
}
});
+ }
+ fn prune_expired_request_state(&mut self) {
self.outbound_channels_by_intercept_scid.retain(|intercept_scid, entry| {
if entry.is_prunable() {
// We abort the flow, and prune any data kept.
@@ -1875,6 +1877,7 @@ where
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
// `persist` which removes it from the store.
+ peer_state_lock.prune_pending_requests();
peer_state_lock.prune_expired_request_state();
}
}
Why this scored 44/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.