Skip `LSPS2ServiceHandler` persistence if unnecessary
What the developer wrote
Authored by Elias Rohrer
Skip `LSPS2ServiceHandler` persistence if unnecessary
.. we only persist the service handler if necessary.
What changed, and why it matters
This commit is a performance and reliability optimization for the LSPS2 (Lightning Service Provider Specification 2) service handler in rust-lightning. It adds a flag so the code only writes peer state to disk when something has actually changed, instead of writing it every time. If a write fails, it marks the state as needing to be saved again later. There is no direct security vulnerability being fixed here; it is mainly about avoiding unnecessary disk writes and making sure failed writes are retried.
No immediate security action required. Review the new TODO about removing pruned peer state entries from the KVStore to ensure stale data does not accumulate. Monitor that the silent Ok on missing peer state does not mask unexpected consistency issues in production.
Security signals we found
Avoidance of unnecessary persistence operations reduces wear and write-amplification surface
Retry-on-failure logic (reset needs_persist after error) improves durability consistency
Behavioral change: missing peer state during persist now silently succeeds instead of returning an error
TODO comment indicates incomplete cleanup of pruned peer state from KVStore
Evidence from the diff
The change introduces a needs_persist: bool field to PeerState in lightning-liquidity/src/lsps2/service.rs. The flag is set to true whenever outbound JIT channels or intercept SCID mappings are inserted or removed. Serialization always writes false for the flag (static_value), so on-disk state does not carry it. The persist and persist_peer methods now skip encoding/writing when needs_persist is false, and on a failed write the flag is reset to true so a retry will occur. A missing-peer case now returns Ok(()) instead of an error. A TODO comment notes that peer state entries should be removed from the KV store when pruned.
Changed components
lightning-liquidity/src/lsps2/service.rsLSPS2ServiceHandler persistence logicPeerState struct and serializationInspect captured patch +42 / −8
diff --git a/lightning-liquidity/src/lsps2/service.rs b/lightning-liquidity/src/lsps2/service.rs
index aa3f0e7..f7698b2 100644
--- a/lightning-liquidity/src/lsps2/service.rs
+++ b/lightning-liquidity/src/lsps2/service.rs
@@ -470,6 +470,7 @@ pub(crate) struct PeerState {
intercept_scid_by_user_channel_id: HashMap<u128, u64>,
intercept_scid_by_channel_id: HashMap<ChannelId, u64>,
pending_requests: HashMap<LSPSRequestId, LSPS2Request>,
+ needs_persist: bool,
}
impl PeerState {
@@ -478,16 +479,19 @@ impl PeerState {
let pending_requests = new_hash_map();
let intercept_scid_by_user_channel_id = new_hash_map();
let intercept_scid_by_channel_id = new_hash_map();
+ let needs_persist = false;
Self {
outbound_channels_by_intercept_scid,
pending_requests,
intercept_scid_by_user_channel_id,
intercept_scid_by_channel_id,
+ needs_persist,
}
}
fn insert_outbound_channel(&mut self, intercept_scid: u64, channel: OutboundJITChannel) {
self.outbound_channels_by_intercept_scid.insert(intercept_scid, channel);
+ self.needs_persist |= true;
}
fn prune_expired_request_state(&mut self) {
@@ -506,6 +510,7 @@ impl PeerState {
// We abort the flow, and prune any data kept.
self.intercept_scid_by_channel_id.retain(|_, iscid| intercept_scid != iscid);
self.intercept_scid_by_user_channel_id.retain(|_, iscid| intercept_scid != iscid);
+ // TODO: Remove peer state entry from the KVStore
return false;
}
true
@@ -533,6 +538,7 @@ impl_writeable_tlv_based!(PeerState, {
(2, intercept_scid_by_user_channel_id, required),
(4, intercept_scid_by_channel_id, required),
(_unused, pending_requests, (static_value, new_hash_map())),
+ (_unused, needs_persist, (static_value, false)),
});
macro_rules! get_or_insert_peer_state_entry {
@@ -831,6 +837,9 @@ where
match outer_state_lock.get(counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
+ peer_state.needs_persist |= peer_state
+ .outbound_channels_by_intercept_scid
+ .contains_key(&intercept_scid);
if let Some(jit_channel) =
peer_state.outbound_channels_by_intercept_scid.get_mut(&intercept_scid)
{
@@ -918,6 +927,8 @@ where
match outer_state_lock.get(counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
+ peer_state.needs_persist |=
+ peer_state.intercept_scid_by_channel_id.contains_key(&channel_id);
if let Some(intercept_scid) =
peer_state.intercept_scid_by_channel_id.get(&channel_id).copied()
{
@@ -986,6 +997,8 @@ where
match outer_state_lock.get(counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
+ peer_state.needs_persist |=
+ peer_state.intercept_scid_by_channel_id.contains_key(&next_channel_id);
if let Some(intercept_scid) =
peer_state.intercept_scid_by_channel_id.get(&next_channel_id).copied()
{
@@ -1090,6 +1103,7 @@ where
peer_state.intercept_scid_by_user_channel_id.remove(&user_channel_id);
peer_state.outbound_channels_by_intercept_scid.remove(&intercept_scid);
peer_state.intercept_scid_by_channel_id.retain(|_, &mut scid| scid != intercept_scid);
+ peer_state.needs_persist |= true;
Ok(())
}
@@ -1121,6 +1135,8 @@ where
err: format!("Could not find a channel with user_channel_id {}", user_channel_id),
})?;
+ peer_state.needs_persist |=
+ peer_state.outbound_channels_by_intercept_scid.contains_key(&intercept_scid);
let jit_channel = peer_state
.outbound_channels_by_intercept_scid
.get_mut(&intercept_scid)
@@ -1170,6 +1186,8 @@ where
match outer_state_lock.get(counterparty_node_id) {
Some(inner_state_lock) => {
let mut peer_state = inner_state_lock.lock().unwrap();
+ peer_state.needs_persist |=
+ peer_state.intercept_scid_by_user_channel_id.contains_key(&user_channel_id);
if let Some(intercept_scid) =
peer_state.intercept_scid_by_user_channel_id.get(&user_channel_id).copied()
{
@@ -1486,13 +1504,19 @@ where
let outer_state_lock = self.per_peer_state.read().unwrap();
let encoded = match outer_state_lock.get(&counterparty_node_id) {
None => {
- let err = lightning::io::Error::new(
- lightning::io::ErrorKind::Other,
- "Failed to get peer entry",
- );
- return Err(err);
+ // We dropped the peer state by now.
+ return Ok(());
+ },
+ Some(entry) => {
+ let mut peer_state_lock = entry.lock().unwrap();
+ if !peer_state_lock.needs_persist {
+ // We already have persisted otherwise by now.
+ return Ok(());
+ } else {
+ peer_state_lock.needs_persist = false;
+ peer_state_lock.encode()
+ }
},
- Some(entry) => entry.lock().unwrap().encode(),
};
let key = counterparty_node_id.to_string();
@@ -1504,7 +1528,14 @@ where
)
};
- fut.await
+ fut.await.map_err(|e| {
+ self.per_peer_state
+ .read()
+ .unwrap()
+ .get(&counterparty_node_id)
+ .map(|p| p.lock().unwrap().needs_persist = true);
+ e
+ })
}
pub(crate) async fn persist(&self) -> Result<(), lightning::io::Error> {
@@ -1513,7 +1544,10 @@ where
// time.
let need_persist: Vec<PublicKey> = {
let outer_state_lock = self.per_peer_state.read().unwrap();
- outer_state_lock.iter().filter_map(|(k, v)| Some(*k)).collect()
+ outer_state_lock
+ .iter()
+ .filter_map(|(k, v)| if v.lock().unwrap().needs_persist { Some(*k) } else { None })
+ .collect()
};
for counterparty_node_id in need_persist.into_iter() {
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.