← Watch feed
test: Modify test_peer_storage to check latest changes
Public commit record
87/100 · Strong
What the developer wrote
Authored by Aditya Sharma
test: Modify test_peer_storage to check latest changes
Node should now determine lost states using retrieved peer storage.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides an explanatory body✓ Mentions testing or verification
What changed, and why it matters
This commit only changes a test file. It updates an existing unit test to exercise a new behavior: after a node reloads from an older backup, it should detect that it is behind by comparing against peer storage data. The test is gated behind a compile-time feature flag and does not alter production code. There is no indication this fixes or introduces a security vulnerability in live code.
Recommended action
No security action required. Treat as normal test maintenance. If reviewing the broader peer-storage feature, ensure the production panic paths and state-detection logic are separately audited, but this commit itself does not change them.
Security signals we found
01
Test-only change
02
Compile-time feature gated: #[cfg(peer_storage)]
03
Peer storage retrieval used to detect stale node state after reload
04
Assertions expect panic on duplicate peer storage retrieval handling
Evidence from the diff
The diff modifies lightning/src/ln/channelmanager.rs, specifically the test_peer_storage unit test, adding #[cfg(peer_storage)] and restructuring the test scenario. The new test flow: creates a channel, sends payments, captures node-0’s serialized state and an old channel monitor, advances blocks to generate peer-storage messages, exchanges peer storage, disconnects peers, reloads node-0 from the old backup, reconnects, and asserts that node-0 (now stale) sends only a channel reestablish and panics when receiving duplicate peer-storage retrieval data. This is a defensive test verifying that lost/fallback state can be detected via peer storage. No production logic is changed.
Changed components
lightning/src/ln/channelmanager.rstest_peer_storage unit testInspect captured patch +55 / −56
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 6e7c191..16014d2 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -17544,38 +17544,58 @@ mod tests {
#[test]
#[rustfmt::skip]
+ #[cfg(peer_storage)]
fn test_peer_storage() {
let chanmon_cfgs = create_chanmon_cfgs(2);
+ let (persister, chain_monitor);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let nodes_0_deserialized;
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
- let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+ let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
- create_announced_chan_between_nodes(&nodes, 0, 1);
+ let (_, _, cid, _) = create_announced_chan_between_nodes(&nodes, 0, 1);
+ send_payment(&nodes[0], &[&nodes[1]], 1000);
+ let nodes_0_serialized = nodes[0].node.encode();
+ let old_state_monitor = get_monitor!(nodes[0], cid).encode();
+ send_payment(&nodes[0], &[&nodes[1]], 10000);
+ send_payment(&nodes[0], &[&nodes[1]], 9999);
+
+ // Update peer storage with latest commitment txns
+ connect_blocks(&nodes[0], 1);
+ connect_blocks(&nodes[0], 1);
let peer_storage_msg_events_node0 = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_msg_events();
let peer_storage_msg_events_node1 = nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_msg_events();
assert_ne!(peer_storage_msg_events_node0.len(), 0);
assert_ne!(peer_storage_msg_events_node1.len(), 0);
- match peer_storage_msg_events_node0[0] {
- MessageSendEvent::SendPeerStorage { ref node_id, ref msg } => {
- assert_eq!(*node_id, nodes[1].node.get_our_node_id());
- nodes[1].node.handle_peer_storage(nodes[0].node.get_our_node_id(), msg.clone());
+ for ps_msg in peer_storage_msg_events_node0 {
+ match ps_msg {
+ MessageSendEvent::SendPeerStorage { ref node_id, ref msg } => {
+ assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+ nodes[1].node.handle_peer_storage(nodes[0].node.get_our_node_id(), msg.clone());
+ }
+ _ => panic!("Unexpected event"),
}
- _ => panic!("Unexpected event"),
}
- match peer_storage_msg_events_node1[0] {
- MessageSendEvent::SendPeerStorage { ref node_id, ref msg } => {
- assert_eq!(*node_id, nodes[0].node.get_our_node_id());
- nodes[0].node.handle_peer_storage(nodes[1].node.get_our_node_id(), msg.clone());
+ for ps_msg in peer_storage_msg_events_node1 {
+ match ps_msg {
+ MessageSendEvent::SendPeerStorage { ref node_id, ref msg } => {
+ assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+ nodes[0].node.handle_peer_storage(nodes[1].node.get_our_node_id(), msg.clone());
+ }
+ _ => panic!("Unexpected event"),
}
- _ => panic!("Unexpected event"),
}
nodes[0].node.peer_disconnected(nodes[1].node.get_our_node_id());
nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());
+ // Reload Node!
+ // TODO: Handle the case where we've completely forgotten about an active channel.
+ reload_node!(nodes[0], test_default_channel_config(), &nodes_0_serialized, &[&old_state_monitor[..]], persister, chain_monitor, nodes_0_deserialized);
+
nodes[0].node.peer_connected(nodes[1].node.get_our_node_id(), &msgs::Init {
features: nodes[1].node.init_features(), networks: None, remote_network_address: None
}, true).unwrap();
@@ -17586,62 +17606,41 @@ mod tests {
let node_1_events = nodes[1].node.get_and_clear_pending_msg_events();
assert_eq!(node_1_events.len(), 2);
+ // Since, node-0 does not have any memory it would not send any message.
let node_0_events = nodes[0].node.get_and_clear_pending_msg_events();
- assert_eq!(node_0_events.len(), 2);
+ assert_eq!(node_0_events.len(), 1);
- for msg in node_1_events{
+ match node_0_events[0] {
+ MessageSendEvent::SendChannelReestablish { ref node_id, .. } => {
+ assert_eq!(*node_id, nodes[1].node.get_our_node_id());
+ // nodes[0] would send a bogus channel reestablish, so there's no need to handle this.
+ }
+ _ => panic!("Unexpected event"),
+ }
+
+ for msg in node_1_events {
if let MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } = msg {
nodes[0].node.handle_channel_reestablish(nodes[1].node.get_our_node_id(), msg);
assert_eq!(*node_id, nodes[0].node.get_our_node_id());
} else if let MessageSendEvent::SendPeerStorageRetrieval { ref node_id, ref msg } = msg {
+ // Should Panic here!
nodes[0].node.handle_peer_storage_retrieval(nodes[1].node.get_our_node_id(), msg.clone());
assert_eq!(*node_id, nodes[0].node.get_our_node_id());
+ // Should Panic here!
+ let res = std::panic::catch_unwind(|| {
+ nodes[0]
+ .node
+ .handle_peer_storage_retrieval(nodes[1].node.get_our_node_id(), msg.clone())
+ });
+ assert!(res.is_err());
+ break;
} else {
panic!("Unexpected event")
}
}
-
- for msg in node_0_events{
- if let MessageSendEvent::SendChannelReestablish { ref node_id, ref msg } = msg {
- nodes[1].node.handle_channel_reestablish(nodes[0].node.get_our_node_id(), msg);
- assert_eq!(*node_id, nodes[1].node.get_our_node_id());
- } else if let MessageSendEvent::SendPeerStorageRetrieval { ref node_id, ref msg } = msg {
- nodes[1].node.handle_peer_storage_retrieval(nodes[0].node.get_our_node_id(), msg.clone());
- assert_eq!(*node_id, nodes[1].node.get_our_node_id());
- } else {
- panic!("Unexpected event")
- }
- }
-
- let node_1_msg_events = nodes[1].node.get_and_clear_pending_msg_events();
- let node_0_msg_events = nodes[0].node.get_and_clear_pending_msg_events();
-
- assert_eq!(node_1_msg_events.len(), 3);
- assert_eq!(node_0_msg_events.len(), 3);
-
- for msg in node_1_msg_events {
- if let MessageSendEvent::SendChannelReady { ref node_id, .. } = msg {
- assert_eq!(*node_id, nodes[0].node.get_our_node_id());
- } else if let MessageSendEvent::SendAnnouncementSignatures { ref node_id, .. } = msg {
- assert_eq!(*node_id, nodes[0].node.get_our_node_id());
- } else if let MessageSendEvent::SendChannelUpdate { ref node_id, .. } = msg {
- assert_eq!(*node_id, nodes[0].node.get_our_node_id());
- } else {
- panic!("Unexpected event")
- }
- }
-
- for msg in node_0_msg_events {
- if let MessageSendEvent::SendChannelReady { ref node_id, .. } = msg {
- assert_eq!(*node_id, nodes[1].node.get_our_node_id());
- } else if let MessageSendEvent::SendAnnouncementSignatures { ref node_id, .. } = msg {
- assert_eq!(*node_id, nodes[1].node.get_our_node_id());
- } else if let MessageSendEvent::SendChannelUpdate { ref node_id, .. } = msg {
- assert_eq!(*node_id, nodes[1].node.get_our_node_id());
- } else {
- panic!("Unexpected event")
- }
- }
+ // When we panic'd, we expect to panic on `Drop`.
+ let res = std::panic::catch_unwind(|| drop(nodes));
+ assert!(res.is_err());
}
#[test]
Risk score
Our methodology →Why this scored 12/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.