Clean up test handling of resending responding commitment_signed

When we need to rebroadcast a `commitment_signed` on reconnect in
response to a previous update (ie not one which contains any
updates) we previously hacked in support for it by passing a `-1`
for the number of expected update_add_htlcs. This is a mess, and
with the introduction of `ReconnectArgs` we can now clean it up
easily with a new bool.

Author: TheBlueMatt

lightning/src/ln/functional_test_utils.rs

@@ -3050,7 +3050,11 @@ pub struct ReconnectArgs<'a, 'b, 'c, 'd> {
 	pub node_a: &'a Node<'b, 'c, 'd>,
 	pub node_b: &'a Node<'b, 'c, 'd>,
 	pub send_channel_ready: (bool, bool),
-	pub pending_htlc_adds: (i64, i64),
+	pub pending_responding_commitment_signed: (bool, bool),
+	/// Indicates that the pending responding commitment signed will be a dup for the recipient,
+	/// and no monitor update is expected
+	pub pending_responding_commitment_signed_dup_monitor: (bool, bool),
+	pub pending_htlc_adds: (usize, usize),
 	pub pending_htlc_claims: (usize, usize),
 	pub pending_htlc_fails: (usize, usize),
 	pub pending_cell_htlc_claims: (usize, usize),
@@ -3064,6 +3068,8 @@ impl<'a, 'b, 'c, 'd> ReconnectArgs<'a, 'b, 'c, 'd> {
 			node_a,
 			node_b,
 			send_channel_ready: (false, false),
+			pending_responding_commitment_signed: (false, false),
+			pending_responding_commitment_signed_dup_monitor: (false, false),
 			pending_htlc_adds: (0, 0),
 			pending_htlc_claims: (0, 0),
 			pending_htlc_fails: (0, 0),
@@ -3079,7 +3085,8 @@ impl<'a, 'b, 'c, 'd> ReconnectArgs<'a, 'b, 'c, 'd> {
 pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 	let ReconnectArgs {
 		node_a, node_b, send_channel_ready, pending_htlc_adds, pending_htlc_claims, pending_htlc_fails,
-		pending_cell_htlc_claims, pending_cell_htlc_fails, pending_raa
+		pending_cell_htlc_claims, pending_cell_htlc_fails, pending_raa,
+		pending_responding_commitment_signed, pending_responding_commitment_signed_dup_monitor,
 	} = args;
 	node_a.node.peer_connected(&node_b.node.get_our_node_id(), &msgs::Init {
 		features: node_b.node.init_features(), networks: None, remote_network_address: None
@@ -3164,13 +3171,12 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 		} else {
 			assert!(chan_msgs.1.is_none());
 		}
-		if pending_htlc_adds.0 != 0 || pending_htlc_claims.0 != 0 || pending_htlc_fails.0 != 0 || pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 {
+		if pending_htlc_adds.0 != 0 || pending_htlc_claims.0 != 0 || pending_htlc_fails.0 != 0 ||
+			pending_cell_htlc_claims.0 != 0 || pending_cell_htlc_fails.0 != 0 ||
+			pending_responding_commitment_signed.0
+		{
 			let commitment_update = chan_msgs.2.unwrap();
-			if pending_htlc_adds.0 != -1 { // We use -1 to denote a response commitment_signed
-				assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.0 as usize);
-			} else {
-				assert!(commitment_update.update_add_htlcs.is_empty());
-			}
+			assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.0);
 			assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.0 + pending_cell_htlc_claims.0);
 			assert_eq!(commitment_update.update_fail_htlcs.len(), pending_htlc_fails.0 + pending_cell_htlc_fails.0);
 			assert!(commitment_update.update_fail_malformed_htlcs.is_empty());
@@ -3184,7 +3190,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 				node_a.node.handle_update_fail_htlc(&node_b.node.get_our_node_id(), &update_fail);
 			}
 
-			if pending_htlc_adds.0 != -1 { // We use -1 to denote a response commitment_signed
+			if !pending_responding_commitment_signed.0 {
 				commitment_signed_dance!(node_a, node_b, commitment_update.commitment_signed, false);
 			} else {
 				node_a.node.handle_commitment_signed(&node_b.node.get_our_node_id(), &commitment_update.commitment_signed);
@@ -3193,7 +3199,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 				// No commitment_signed so get_event_msg's assert(len == 1) passes
 				node_b.node.handle_revoke_and_ack(&node_a.node.get_our_node_id(), &as_revoke_and_ack);
 				assert!(node_b.node.get_and_clear_pending_msg_events().is_empty());
-				check_added_monitors!(node_b, 1);
+				check_added_monitors!(node_b, if pending_responding_commitment_signed_dup_monitor.0 { 0 } else { 1 });
 			}
 		} else {
 			assert!(chan_msgs.2.is_none());
@@ -3223,11 +3229,12 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 		} else {
 			assert!(chan_msgs.1.is_none());
 		}
-		if pending_htlc_adds.1 != 0 || pending_htlc_claims.1 != 0 || pending_htlc_fails.1 != 0 || pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 {
+		if pending_htlc_adds.1 != 0 || pending_htlc_claims.1 != 0 || pending_htlc_fails.1 != 0 ||
+			pending_cell_htlc_claims.1 != 0 || pending_cell_htlc_fails.1 != 0 ||
+			pending_responding_commitment_signed.1
+		{
 			let commitment_update = chan_msgs.2.unwrap();
-			if pending_htlc_adds.1 != -1 { // We use -1 to denote a response commitment_signed
-				assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.1 as usize);
-			}
+			assert_eq!(commitment_update.update_add_htlcs.len(), pending_htlc_adds.1);
 			assert_eq!(commitment_update.update_fulfill_htlcs.len(), pending_htlc_claims.1 + pending_cell_htlc_claims.1);
 			assert_eq!(commitment_update.update_fail_htlcs.len(), pending_htlc_fails.1 + pending_cell_htlc_fails.1);
 			assert!(commitment_update.update_fail_malformed_htlcs.is_empty());
@@ -3241,7 +3248,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 				node_b.node.handle_update_fail_htlc(&node_a.node.get_our_node_id(), &update_fail);
 			}
 
-			if pending_htlc_adds.1 != -1 { // We use -1 to denote a response commitment_signed
+			if !pending_responding_commitment_signed.1 {
 				commitment_signed_dance!(node_b, node_a, commitment_update.commitment_signed, false);
 			} else {
 				node_b.node.handle_commitment_signed(&node_a.node.get_our_node_id(), &commitment_update.commitment_signed);
@@ -3250,7 +3257,7 @@ pub fn reconnect_nodes<'a, 'b, 'c, 'd>(args: ReconnectArgs<'a, 'b, 'c, 'd>) {
 				// No commitment_signed so get_event_msg's assert(len == 1) passes
 				node_a.node.handle_revoke_and_ack(&node_b.node.get_our_node_id(), &bs_revoke_and_ack);
 				assert!(node_a.node.get_and_clear_pending_msg_events().is_empty());
-				check_added_monitors!(node_a, 1);
+				check_added_monitors!(node_a, if pending_responding_commitment_signed_dup_monitor.1 { 0 } else { 1 });
 			}
 		} else {
 			assert!(chan_msgs.2.is_none());

lightning/src/ln/functional_tests.rs

@@ -3867,13 +3867,13 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
 	} else if messages_delivered == 3 {
 		// nodes[0] still wants its RAA + commitment_signed
 		let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
-		reconnect_args.pending_htlc_adds.0 = -1;
+		reconnect_args.pending_responding_commitment_signed.0 = true;
 		reconnect_args.pending_raa.0 = true;
 		reconnect_nodes(reconnect_args);
 	} else if messages_delivered == 4 {
 		// nodes[0] still wants its commitment_signed
 		let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
-		reconnect_args.pending_htlc_adds.0 = -1;
+		reconnect_args.pending_responding_commitment_signed.0 = true;
 		reconnect_nodes(reconnect_args);
 	} else if messages_delivered == 5 {
 		// nodes[1] still wants its final RAA
@@ -4001,13 +4001,13 @@ fn do_test_drop_messages_peer_disconnect(messages_delivered: u8, simulate_broken
 	} else if messages_delivered == 2 {
 		// nodes[0] still wants its RAA + commitment_signed
 		let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
-		reconnect_args.pending_htlc_adds.1 = -1;
+		reconnect_args.pending_responding_commitment_signed.1 = true;
 		reconnect_args.pending_raa.1 = true;
 		reconnect_nodes(reconnect_args);
 	} else if messages_delivered == 3 {
 		// nodes[0] still wants its commitment_signed
 		let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
-		reconnect_args.pending_htlc_adds.1 = -1;
+		reconnect_args.pending_responding_commitment_signed.1 = true;
 		reconnect_nodes(reconnect_args);
 	} else if messages_delivered == 4 {
 		// nodes[1] still wants its final RAA