f - test enabling signer before reestablihs, no double send

lightning/src/ln/async_signer_tests.rs

@@ -332,20 +332,33 @@ fn test_async_commitment_signature_for_peer_disconnect() {
 
 #[test]
 fn test_async_commitment_signature_ordering_reestablish() {
-	do_test_async_commitment_signature_ordering(false, false);
+	do_test_async_commitment_signature_ordering(false, false, false);
 }
 
 #[test]
 fn test_async_commitment_signature_ordering_monitor_restored() {
-	do_test_async_commitment_signature_ordering(true, false);
+	do_test_async_commitment_signature_ordering(true, false, false);
 }
 
 #[test]
-fn test_async_commitment_signature_ordering_monitor_restored_signer_restored_early() {
-	do_test_async_commitment_signature_ordering(true, true);
+fn test_async_commitment_signature_ordering_monitor_restored_signer_restored_before_completion() {
+	// This tests that if we were pending a monitor update completion across a restart,
+	// and needed to send a CS then RAA, that if our signer is unblocked before the monitor
+	// update completes, then we don't send duplicate messages upon calling `signer_unblocked`
+	// after the monitor update completes.
+	do_test_async_commitment_signature_ordering(true, true, false);
 }
 
-fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, enable_signer_before_monitor_completion: bool) {
+#[test]
+fn test_async_commitment_signature_ordering_signer_restored_before_reestablish() {
+	// This tests that if we tried to send a commitment_signed, but our signer was blocked,
+	// if we disconnect, reconnect, unblock the signer, then handle channel_reestablish,
+	// that we don't send duplicate messages upon calling `signer_unblocked`.
+	do_test_async_commitment_signature_ordering(false, false, true);
+}
+
+fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool,
+	enable_signer_before_monitor_completion: bool, enable_signer_before_reestablish: bool) {
 	// Across disconnects we may end up in a situation where we need to send a
 	// commitment_signed and then revoke_and_ack. We need to make sure that if
 	// the signer is pending for commitment_signed but not revoke_and_ack, we don't
@@ -365,14 +378,24 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena
 
 	let (payment_preimage_1, payment_hash_1, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
 
+	if enable_signer_before_reestablish {
+		// When the node goes to sign the second commitment update, it will set that
+		// it's pending a commitment update, which will still be set when the peer
+		// reconnects.
+		nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment);
+	}
 	// Start to send the second update_add_htlc + commitment_signed, but don't actually make it
 	// to the peer.
 	let (route, payment_hash_2, payment_preimage_2, payment_secret_2) = get_route_and_payment_hash!(nodes[0], nodes[1], 1000000);
 	nodes[0].node.send_payment_with_route(&route, payment_hash_2,
 		RecipientOnionFields::secret_only(payment_secret_2), PaymentId(payment_hash_2.0)).unwrap();
 	check_added_monitors!(nodes[0], 1);
 
-	get_htlc_update_msgs(&nodes[0], &nodes[1].node.get_our_node_id());
+	if enable_signer_before_reestablish {
+		assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+	} else {
+		get_htlc_update_msgs(&nodes[0], &nodes[1].node.get_our_node_id());
+	}
 
 	// Send back update_fulfill_htlc + commitment_signed for the first payment.
 	nodes[1].node.claim_funds(payment_preimage_1);
@@ -391,7 +414,7 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena
 				chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
 			}
 			nodes[0].node.handle_commitment_signed(&nodes[1].node.get_our_node_id(), commitment_signed);
-			if monitor_update_failure {
+			if monitor_update_failure || enable_signer_before_reestablish {
 				assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 			} else {
 				let _ = get_event_msg!(nodes[0], MessageSendEvent::SendRevokeAndACK, nodes[1].node.get_our_node_id());
@@ -421,22 +444,38 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena
 	// With a fully working signer, here we would send a commitment_signed,
 	// and then revoke_and_ack. With commitment_signed disabled, since
 	// our ordering is CS then RAA, we should make sure we don't send the RAA.
-	nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment);
+	if enable_signer_before_reestablish {
+		nodes[0].enable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment);
+	} else {
+		nodes[0].disable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment);
+	}
 	nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &reestablish_2[0]);
-	let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
-	assert!(as_resp.0.is_none());
-	assert!(as_resp.1.is_none());
-	assert!(as_resp.2.is_none());
 
-	let as_resp = if monitor_update_failure && enable_signer_before_monitor_completion {
+	let as_resp = if enable_signer_before_reestablish {
+		// We should send the CS + RAA now, and not again on `signer_unblocked`.
+		let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+
+		nodes[0].node.signer_unblocked(Some((nodes[1].node.get_our_node_id(), chan_id)));
+		let as_resp_empty = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+		assert!(as_resp_empty.0.is_none());
+		assert!(as_resp_empty.1.is_none());
+		assert!(as_resp_empty.2.is_none());
+
+		as_resp
+	} else if monitor_update_failure && enable_signer_before_monitor_completion {
+		let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+		assert!(as_resp.0.is_none());
+		assert!(as_resp.1.is_none());
+		assert!(as_resp.2.is_none());
+
 		nodes[0].enable_channel_signer_op(&nodes[1].node.get_our_node_id(), &chan_id, SignerOp::SignCounterpartyCommitment);
 
 		chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
 		let (outpoint, latest_update, _) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id).unwrap().clone();
 		nodes[0].chain_monitor.chain_monitor.force_channel_monitor_updated(outpoint, latest_update);
 		check_added_monitors!(nodes[0], 0);
 
-		// We should send the CS now, and not send it again on `signer_unblocked`.
+		// We should send the CS + RAA now, and not again on `signer_unblocked`.
 		let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
 
 		nodes[0].node.signer_unblocked(Some((nodes[1].node.get_our_node_id(), chan_id)));
@@ -446,6 +485,11 @@ fn do_test_async_commitment_signature_ordering(monitor_update_failure: bool, ena
 		assert!(as_resp_empty.2.is_none());
 		as_resp
 	} else {
+		let as_resp = handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
+		assert!(as_resp.0.is_none());
+		assert!(as_resp.1.is_none());
+		assert!(as_resp.2.is_none());
+
 		if monitor_update_failure {
 			chanmon_cfgs[0].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
 			let (outpoint, latest_update, _) = nodes[0].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&chan_id).unwrap().clone();