Skip to content

Commit 589a88e

Browse files
committed
Fix data_loss_protect test to actually test DLP
The data loss protect test was panicking in a message assertion which should be passing, but because the test was marked only `#[should_panic]` it was being treated as a successful outcome. Instead, we use `catch_unwind` on exactly the line we expect to panic to ensure we are hitting the right one.
1 parent 1e25979 commit 589a88e

File tree

1 file changed

+21
-10
lines changed

1 file changed

+21
-10
lines changed

lightning/src/ln/reload_tests.rs

+21-10
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() {
493493
assert!(found_err);
494494
}
495495

496+
#[cfg(feature = "std")]
496497
fn do_test_data_loss_protect(reconnect_panicing: bool) {
497498
// When we get a data_loss_protect proving we're behind, we immediately panic as the
498499
// chain::Watch API requirements have been violated (e.g. the user restored from a backup). The
@@ -539,18 +540,32 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) {
539540
// Check that we sent the warning message when we detected that A has fallen behind,
540541
// and give the possibility for A to recover from the warning.
541542
nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
542-
let warn_msg = "Peer attempted to reestablish channel with a very old local commitment transaction".to_owned();
543-
assert!(check_warn_msg!(nodes[1], nodes[0].node.get_our_node_id(), chan.2).contains(&warn_msg));
543+
let warn_msg = "Peer attempted to reestablish channel with a very old local commitment transaction: 0 (received) vs 4 (expected)".to_owned();
544+
545+
let warn_reestablish = nodes[1].node.get_and_clear_pending_msg_events();
546+
assert_eq!(warn_reestablish.len(), 2);
547+
match warn_reestablish[1] {
548+
MessageSendEvent::HandleError { action: ErrorAction::SendWarningMessage { ref msg, .. }, .. } => {
549+
assert_eq!(msg.data, warn_msg);
550+
},
551+
_ => panic!("Unexpected event"),
552+
}
553+
let reestablish_msg = match &warn_reestablish[0] {
554+
MessageSendEvent::SendChannelReestablish { msg, .. } => msg.clone(),
555+
_ => panic!("Unexpected event"),
556+
};
544557

545558
{
546559
let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
547560
// The node B should not broadcast the transaction to force close the channel!
548561
assert!(node_txn.is_empty());
549562
}
550563

551-
let reestablish_0 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
552564
// Check A panics upon seeing proof it has fallen behind.
553-
nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_0[0]);
565+
assert!(std::panic::catch_unwind(|| {
566+
nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_msg);
567+
}).is_err());
568+
std::mem::forget(nodes); // Skip the `Drop` handler for `Node`
554569
return; // By this point we should have panic'ed!
555570
}
556571

@@ -610,13 +625,9 @@ fn do_test_data_loss_protect(reconnect_panicing: bool) {
610625
}
611626

612627
#[test]
613-
#[should_panic]
614-
fn test_data_loss_protect_showing_stale_state_panics() {
628+
#[cfg(feature = "std")]
629+
fn test_data_loss_protect() {
615630
do_test_data_loss_protect(true);
616-
}
617-
618-
#[test]
619-
fn test_force_close_without_broadcast() {
620631
do_test_data_loss_protect(false);
621632
}
622633

0 commit comments

Comments
 (0)