update functional test and check warn message instead of a close channel

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>

lightning/src/ln/functional_test_utils.rs

@@ -893,6 +893,17 @@ macro_rules! get_closing_signed_broadcast {
 
 #[cfg(test)]
 macro_rules! check_warn_msg {
+	($node: expr, $recipient_node_id: expr) => {{
+		let msg_events = $node.node.get_and_clear_pending_msg_events();
+		assert_eq!(msg_events.len(), 1);
+		match msg_events[0] {
+			MessageSendEvent::HandleError { action: ErrorAction::SendWarningMessage { ref msg, log_level: _ }, node_id } => {
+				assert_eq!(node_id, $recipient_node_id);
+				msg.data.clone()
+			},
+			_ => panic!("Unexpected event"),
+		}
+	}};
 	($node: expr, $recipient_node_id: expr, $chan_id: expr) => {{
 		let msg_events = $node.node.get_and_clear_pending_msg_events();
 		assert_eq!(msg_events.len(), 1);

lightning/src/ln/functional_tests.rs

@@ -7282,8 +7282,20 @@ fn test_user_configurable_csv_delay() {
 	} else { assert!(false); }
 }
 
-#[test]
-fn test_data_loss_protect() {
+/// describe the test case as a enum, istead as boolean in this case!
+/// with the enum there is the possibility to pass more data and
+/// check more corner case.
+#[derive(Debug)]
+enum DataLossProtectTestCase {
+	/// The node that send the warning message, will try to
+	/// use the channel, but it can't, because after the
+	/// warning message we don't change the channel state.
+	UseChannel,
+	/// Try to reconnect to the node that have send the warning message
+	TryToReconnect,
+}
+
+fn do_test_data_loss_protect(case: DataLossProtectTestCase) {
 	// We want to be sure that :
 	// * we don't broadcast our Local Commitment Tx in case of fallen behind
 	//   (but this is not quite true - we broadcast during Drop because chanmon is out of sync with chanmgr)
@@ -7380,22 +7392,45 @@ fn test_data_loss_protect() {
 	}
 
 	// Check we close channel detecting A is fallen-behind
+	// Check if we sent the warning message when we detecting that A is fallen-behind,
+	// and we give the possibility to A to  be able to recover from error.
 	nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &reestablish_1[0]);
-	check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: "Peer attempted to reestablish channel with a very old local commitment transaction".to_string() });
-	assert_eq!(check_closed_broadcast!(nodes[1], true).unwrap().data, "Peer attempted to reestablish channel with a very old local commitment transaction");
-	check_added_monitors!(nodes[1], 1);
+	let warn_msg = "Peer attempted to reestablish channel with a very old local commitment transaction".to_owned();
+	assert!(check_warn_msg!(nodes[1], nodes[0].node.get_our_node_id()).contains(&warn_msg));
+
 
 	// Check A is able to claim to_remote output
-	let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
-	assert_eq!(node_txn.len(), 1);
-	check_spends!(node_txn[0], chan.3);
-	assert_eq!(node_txn[0].output.len(), 2);
-	mine_transaction(&nodes[0], &node_txn[0]);
-	connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
-	check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can\'t do any automated broadcasting".to_string() });
-	let spend_txn = check_spendable_outputs!(nodes[0], node_cfgs[0].keys_manager);
-	assert_eq!(spend_txn.len(), 1);
-	check_spends!(spend_txn[0], node_txn[0]);
+	let mut node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().clone();
+	// The node B should not broadcast the transaction to force close the channel!
+	assert!(node_txn.is_empty());
+	// B should now detect that there is something wrong and should force close the channel.
+	let exp_err = "We have fallen behind - we have received proof that if we broadcast remote is going to claim our funds - we can\'t do any automated broadcasting";
+	check_closed_event!(nodes[0], 1, ClosureReason::ProcessingError { err: exp_err.to_string() });
+
+	// after the waring message sent by B, we should not able to
+	// use the channel, or reconnect with success to the channel.
+	match case {
+		DataLossProtectTestCase::UseChannel => {
+			assert!(nodes[0].node.list_usable_channels().is_empty());
+		},
+		DataLossProtectTestCase::TryToReconnect => {
+			nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
+			nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty(), remote_network_address: None });
+			let retry_reestablish = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
+
+			nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &retry_reestablish[0]);
+			check_added_monitors!(nodes[1], 1);
+			let exp_err = "Peer sent a garbage channel_reestablish with secret key not matching the commitment height provided";
+			check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: exp_err.to_string() });
+			nodes[1].node.get_and_clear_pending_msg_events();
+		},
+	}
+}
+
+#[test]
+fn test_data_loss_protect() {
+	do_test_data_loss_protect(DataLossProtectTestCase::UseChannel);
+	do_test_data_loss_protect(DataLossProtectTestCase::TryToReconnect);
 }
 
 #[test]