test(consensus): move the IngressPayload serialization/deserializat…

…ion unit test from `test_utilities/types/` to `rs/types/types` (#3384)

It makes little sense to me to have unit tests for `IngressPayload` in a
test utilities crate

Note: to avoid cyclic dependency I'm not using `SignedIngressBuilder` to
construct a `SignedIngress`

rs/test_utilities/types/BUILD.bazel

@@ -25,7 +25,6 @@ rust_test(
     crate = ":types",
     deps = [
         # Keep sorted.
-        "@crate_index//:assert_matches",
         "@crate_index//:bincode",
         "@crate_index//:serde_cbor",
     ],

rs/test_utilities/types/Cargo.toml

@@ -14,6 +14,5 @@ ic-types-test-utils = { path = "../../types/types_test_utils" }
 rand = { workspace = true }
 
 [dev-dependencies]
-assert_matches = { workspace = true }
 bincode = { workspace = true }
 serde_cbor = { workspace = true }

rs/test_utilities/types/src/batch/ingress_payload.rs

@@ -36,77 +36,3 @@ impl IngressPayloadBuilder {
         self.ingress_payload.into()
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::messages::SignedIngressBuilder;
-    use assert_matches::assert_matches;
-    use ic_types::{batch::IngressPayloadError, time::expiry_time_from_now};
-    use std::convert::TryFrom;
-    use std::time::Duration;
-
-    #[test]
-    fn test_ingress_payload_deserialization() {
-        // serialization/deserialization of empty payload.
-        let payload = IngressPayload::default();
-        let bytes = bincode::serialize(&payload).unwrap();
-        assert_eq!(
-            bincode::deserialize::<IngressPayload>(&bytes).unwrap(),
-            payload
-        );
-        let time = expiry_time_from_now();
-
-        // Some test messages.
-        let m1 = SignedIngressBuilder::new()
-            .method_name("m1".to_string())
-            .expiry_time(time + Duration::from_secs(1))
-            .build();
-        let m1_id = m1.id();
-        let m2 = SignedIngressBuilder::new()
-            .method_name("m2".to_string())
-            .expiry_time(time + Duration::from_secs(2))
-            .build();
-        let m3 = SignedIngressBuilder::new()
-            .method_name("m3".to_string())
-            .expiry_time(time + Duration::from_secs(3))
-            .build();
-
-        let msgs = vec![m1, m2, m3];
-        let payload = IngressPayload::from(msgs.clone());
-        // Serialization/deserialization works.
-        let mut bytes = bincode::serialize(&payload).unwrap();
-        assert_eq!(
-            bincode::deserialize::<IngressPayload>(&bytes).unwrap(),
-            payload
-        );
-        // Individual lookup works.
-        assert_matches!(payload.get(0).unwrap(), (_, msg) if msg == msgs[0]);
-        assert_matches!(payload.get(1).unwrap(), (_, msg) if msg == msgs[1]);
-        assert_matches!(payload.get(2).unwrap(), (_, msg) if msg == msgs[2]);
-        // Test IndexOutOfBound.
-        assert_matches!(payload.get(3), Err(IngressPayloadError::IndexOutOfBound(3)));
-        // Converting back to messages should match original
-        assert_eq!(msgs, <Vec<SignedIngress>>::try_from(payload).unwrap());
-
-        // A sub-sequence search function
-        fn find(array: &[u8], subseq: &[u8]) -> Option<usize> {
-            (0..array.len() - subseq.len() + 1).find(|&i| array[i..i + subseq.len()] == subseq[..])
-        }
-
-        // Mutate some byte, deserialization works, but casting back to messages fail.
-        let pos = find(&bytes, m1_id.as_bytes()).unwrap();
-        // `+= 1` may overflow in debug mode.
-        bytes[pos] ^= 1;
-        let payload = bincode::deserialize::<IngressPayload>(&bytes);
-        assert!(payload.is_ok());
-        let payload = payload.unwrap();
-        // get(0) should return error.
-        assert_matches!(
-            payload.get(0),
-            Err(IngressPayloadError::MismatchedMessageIdAtIndex(0))
-        );
-        // Conversion should also fail.
-        assert!(<Vec<_>>::try_from(payload).is_err());
-    }
-}

rs/types/types/src/batch/ingress.rs

@@ -204,41 +204,45 @@ mod tests {
         },
         time::expiry_time_from_now,
     };
+    use assert_matches::assert_matches;
+    use std::convert::TryFrom;
 
-    /// Build a Vec<SignedIngress>.  Convert to IngressPayload and then back to
-    /// Vec<SignedIngress>.  Ensure that the two vectors are identical.
-    #[test]
-    fn into_ingress_payload_and_back() {
-        let ingress_expiry = expiry_time_from_now();
-        let content = HttpCallContent::Call {
+    fn fake_http_call_content(method_name: &str) -> HttpCallContent {
+        HttpCallContent::Call {
             update: HttpCanisterUpdate {
                 canister_id: Blob(vec![42; 8]),
-                method_name: "some_method".to_string(),
+                method_name: method_name.to_string(),
                 arg: Blob(b"".to_vec()),
                 sender: Blob(vec![0x05]),
                 nonce: Some(Blob(vec![1, 2, 3, 4])),
-                ingress_expiry: ingress_expiry.as_nanos_since_unix_epoch(),
+                ingress_expiry: expiry_time_from_now().as_nanos_since_unix_epoch(),
             },
-        };
+        }
+    }
+
+    /// Build a Vec<SignedIngress>.  Convert to IngressPayload and then back to
+    /// Vec<SignedIngress>.  Ensure that the two vectors are identical.
+    #[test]
+    fn into_ingress_payload_and_back() {
         let update_messages = vec![
             HttpRequestEnvelope::<HttpCallContent> {
-                content: content.clone(),
+                content: fake_http_call_content("1"),
                 sender_pubkey: Some(Blob(vec![2; 32])),
                 sender_sig: Some(Blob(vec![1; 32])),
                 sender_delegation: None,
             },
             HttpRequestEnvelope::<HttpCallContent> {
-                content: content.clone(),
+                content: fake_http_call_content("2"),
                 sender_pubkey: None,
                 sender_sig: None,
                 sender_delegation: None,
             },
             HttpRequestEnvelope::<HttpCallContent> {
-                content,
+                content: fake_http_call_content("3"),
                 sender_pubkey: Some(Blob(vec![2; 32])),
                 sender_sig: Some(Blob(vec![1; 32])),
                 sender_delegation: Some(vec![SignedDelegation::new(
-                    Delegation::new(vec![1, 2], ingress_expiry),
+                    Delegation::new(vec![1, 2], expiry_time_from_now()),
                     vec![3, 4],
                 )]),
             },
@@ -251,4 +255,69 @@ mod tests {
         let signed_ingresses1 = Vec::<SignedIngress>::try_from(ingress_payload).unwrap();
         assert_eq!(signed_ingresses, signed_ingresses1);
     }
+
+    #[test]
+    fn test_ingress_payload_deserialization() {
+        // serialization/deserialization of empty payload.
+        let payload = IngressPayload::default();
+        let bytes = bincode::serialize(&payload).unwrap();
+        assert_eq!(
+            bincode::deserialize::<IngressPayload>(&bytes).unwrap(),
+            payload
+        );
+
+        let fake_ingress_message = |method_name| {
+            let message = HttpRequestEnvelope::<HttpCallContent> {
+                content: fake_http_call_content(method_name),
+                sender_pubkey: None,
+                sender_sig: None,
+                sender_delegation: None,
+            };
+
+            SignedIngress::try_from(message).unwrap()
+        };
+
+        // Some test messages.
+        let m1 = fake_ingress_message("m1");
+        let m1_id = m1.id();
+        let m2 = fake_ingress_message("m2");
+        let m3 = fake_ingress_message("m3");
+
+        let msgs = vec![m1, m2, m3];
+        let payload = IngressPayload::from(msgs.clone());
+        // Serialization/deserialization works.
+        let mut bytes = bincode::serialize(&payload).unwrap();
+        assert_eq!(
+            bincode::deserialize::<IngressPayload>(&bytes).unwrap(),
+            payload
+        );
+        // Individual lookup works.
+        assert_matches!(payload.get(0).unwrap(), (_, msg) if msg == msgs[0]);
+        assert_matches!(payload.get(1).unwrap(), (_, msg) if msg == msgs[1]);
+        assert_matches!(payload.get(2).unwrap(), (_, msg) if msg == msgs[2]);
+        // Test IndexOutOfBound.
+        assert_matches!(payload.get(3), Err(IngressPayloadError::IndexOutOfBound(3)));
+        // Converting back to messages should match original
+        assert_eq!(msgs, <Vec<SignedIngress>>::try_from(payload).unwrap());
+
+        // A sub-sequence search function
+        fn find(array: &[u8], subseq: &[u8]) -> Option<usize> {
+            (0..array.len() - subseq.len() + 1).find(|&i| array[i..i + subseq.len()] == subseq[..])
+        }
+
+        // Mutate some byte, deserialization works, but casting back to messages fail.
+        let pos = find(&bytes, m1_id.as_bytes()).unwrap();
+        // `+= 1` may overflow in debug mode.
+        bytes[pos] ^= 1;
+        let payload = bincode::deserialize::<IngressPayload>(&bytes);
+        assert!(payload.is_ok());
+        let payload = payload.unwrap();
+        // get(0) should return error.
+        assert_matches!(
+            payload.get(0),
+            Err(IngressPayloadError::MismatchedMessageIdAtIndex(0))
+        );
+        // Conversion should also fail.
+        assert!(<Vec<_>>::try_from(payload).is_err());
+    }
 }