Correct comments and flow in `test_peer_storage`
What changed, and why it matters
This commit only rewrites comments and reorders assertions inside a single test function. It does not change any production code, cryptographic logic, network handling, or behavior visible to users. There is no security issue here.
No action needed. This is a non-functional test cleanup.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors test_peer_storage in lightning/src/ln/channelmanager.rs. It removes a misleading comment, changes ‘bogus’ to ‘stale’ in a comment, and replaces a for loop over two message events with explicit if let checks on node_1_events[0] and node_1_events[1]. The test still expects a panic when a peer-storage-retrieval message is delivered to a node that has lost state. No runtime behavior of the library is altered.
Changed components
lightning/src/ln/channelmanager.rs (test code only)Inspect captured patch +20 / −20
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index af82f86..b5f4c95 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -17634,36 +17634,36 @@ 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(), 1);
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.
+ // nodes[0] would send a stale 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
- {
- 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")
- }
+ if let MessageSendEvent::SendPeerStorageRetrieval { node_id, msg } = &node_1_events[0] {
+ 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());
+ } else {
+ panic!("Unexpected event {node_1_events:?}")
+ }
+
+ if let MessageSendEvent::SendChannelReestablish { .. } = &node_1_events[1] {
+ // After the `peer_storage_retreival` message would come a `channel_reestablish` (which
+ // would also cause nodes[0] to panic) but it already went down due to lost state so
+ // there's nothing to deliver.
+ } else {
+ panic!("Unexpected event {node_1_events:?}")
}
// When we panic'd, we expect to panic on `Drop`.
let res = std::panic::catch_unwind(|| drop(nodes));
Why this scored 15/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.