Add upgrade test for legacy post-close monitor update persistence
What changed, and why it matters
This commit only adds a new regression test. It does not change any production code. The test verifies that modern LDK can still read and write very old (pre-0.1) channel monitor update records that used a special sentinel value (u64::MAX) for updates after a channel was closed. There is no vulnerability being fixed here; it is defensive test coverage for an already-handled legacy data format.
No action required. Review the existing production code that handles u64::MAX update IDs if desired, but the commit itself is purely additive test coverage.
Security signals we found
Adds regression/upgrade test only
Touches channel monitor persistence and update replay logic
References legacy u64::MAX sentinel update_id behavior
Evidence from the diff
The diff adds one test, legacy_closed_channel_update, in lightning/src/util/persist.rs. It constructs a ChannelMonitorUpdate with update_id = u64::MAX (the legacy sentinel for post-close updates), writes it to the KV store the way pre-0.1 LDK would have, reads it back via read_all_channel_monitors_with_updates, and then calls update_persisted_channel to confirm that such a sentinel update triggers a full monitor write and purges stale incremental update files. No production logic is modified.
Changed components
lightning/src/util/persist.rs (test module only)Inspect captured patch +91 / −0
diff --git a/lightning/src/util/persist.rs b/lightning/src/util/persist.rs
index bf8a0cf..ddc2856 100644
--- a/lightning/src/util/persist.rs
+++ b/lightning/src/util/persist.rs
@@ -1855,6 +1855,7 @@ impl From<u64> for UpdateName {
#[cfg(test)]
mod tests {
use super::*;
+ use crate::chain::channelmonitor::ChannelMonitorUpdateStep;
use crate::chain::ChannelMonitorUpdateStatus;
use crate::events::ClosureReason;
use crate::ln::functional_test_utils::*;
@@ -2261,6 +2262,96 @@ mod tests {
.is_err());
}
+ // Confirm we still handle the `u64::MAX` `update_id` that pre-0.1 LDK used for post-close
+ // `ChannelMonitorUpdate`s, both when reading a leftover update from disk and when one is handed
+ // to the persister to write.
+ #[test]
+ fn legacy_closed_channel_update() {
+ let max_pending_updates = 7;
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let kv_store = TestStore::new(false);
+ let persister = MonitorUpdatingPersister::new(
+ &kv_store,
+ &chanmon_cfgs[0].logger,
+ max_pending_updates,
+ &chanmon_cfgs[0].keys_manager,
+ &chanmon_cfgs[0].keys_manager,
+ &chanmon_cfgs[0].tx_broadcaster,
+ &chanmon_cfgs[0].fee_estimator,
+ );
+ let mut node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let chain_mon_0 = test_utils::TestChainMonitor::new(
+ Some(&chanmon_cfgs[0].chain_source),
+ &chanmon_cfgs[0].tx_broadcaster,
+ &chanmon_cfgs[0].logger,
+ &chanmon_cfgs[0].fee_estimator,
+ &persister,
+ &chanmon_cfgs[0].keys_manager,
+ );
+ node_cfgs[0].chain_monitor = chain_mon_0;
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let _ = create_announced_chan_between_nodes(&nodes, 0, 1);
+ send_payment(&nodes[0], &vec![&nodes[1]][..], 8_000_000);
+ send_payment(&nodes[1], &vec![&nodes[0]][..], 4_000_000);
+
+ let persisted_chan_data = persister.read_all_channel_monitors_with_updates().unwrap();
+ assert_eq!(persisted_chan_data.len(), 1);
+ let (_, monitor) = &persisted_chan_data[0];
+ let monitor_name = monitor.persistence_key();
+ let monitor_key = monitor_name.to_string();
+ assert_ne!(monitor.get_latest_update_id(), u64::MAX);
+
+ let legacy_update = ChannelMonitorUpdate {
+ update_id: u64::MAX,
+ updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast: true }],
+ channel_id: Some(monitor.channel_id()),
+ };
+
+ // Store the update as a standalone file, as a pre-0.1 persister would have, and check that
+ // reading the monitor back replays it.
+ KVStoreSync::write(
+ &kv_store,
+ CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
+ &monitor_key,
+ UpdateName::from(u64::MAX).as_str(),
+ legacy_update.encode(),
+ )
+ .unwrap();
+
+ let persisted_chan_data = persister.read_all_channel_monitors_with_updates().unwrap();
+ assert_eq!(persisted_chan_data.len(), 1);
+ let (_, closed_monitor) = &persisted_chan_data[0];
+ assert_eq!(closed_monitor.get_latest_update_id(), u64::MAX);
+
+ let update_list = KVStoreSync::list(
+ &kv_store,
+ CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
+ &monitor_key,
+ )
+ .unwrap();
+ assert!(!update_list.is_empty());
+
+ // Writing a sentinel-id update should do a full monitor write rather than a standalone
+ // update file, and purge all stale update files.
+ let status =
+ persister.update_persisted_channel(monitor_name, Some(&legacy_update), closed_monitor);
+ assert_eq!(status, ChannelMonitorUpdateStatus::Completed);
+
+ let update_list = KVStoreSync::list(
+ &kv_store,
+ CHANNEL_MONITOR_UPDATE_PERSISTENCE_PRIMARY_NAMESPACE,
+ &monitor_key,
+ )
+ .unwrap();
+ assert!(update_list.is_empty());
+
+ let persisted_chan_data = persister.read_all_channel_monitors_with_updates().unwrap();
+ assert_eq!(persisted_chan_data.len(), 1);
+ assert_eq!(persisted_chan_data[0].1.get_latest_update_id(), u64::MAX);
+ }
+
fn persist_fn<P: Deref, ChannelSigner: EcdsaChannelSigner>(_persist: P) -> bool
where
P::Target: Persist<ChannelSigner>,
Why this scored 17/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.