test: more complex input queues scenario for queue compatibility tests

rs/replicated_state/src/canister_state/queues/tests.rs

@@ -10,6 +10,7 @@ use ic_test_utilities_types::ids::{canister_test_id, message_test_id, user_test_
 use ic_test_utilities_types::messages::{IngressBuilder, RequestBuilder, ResponseBuilder};
 use ic_types::messages::{CallbackId, CanisterMessage};
 use ic_types::time::{expiry_time_from_now, CoarseTime, UNIX_EPOCH};
+use ic_types::UserId;
 use maplit::btreemap;
 use proptest::prelude::*;
 use std::cell::RefCell;
@@ -390,6 +391,18 @@ impl CanisterQueuesMultiFixture {
         )
     }
 
+    fn reserve_and_push_input_response(
+        &mut self,
+        other: CanisterId,
+        input_queue_type: InputQueueType,
+    ) -> Result<(), (StateError, RequestOrResponse)> {
+        self.push_output_request(other)
+            .map_err(|(se, req)| (se, (*req).clone().into()))?;
+        self.pop_output()
+            .expect("Just pushed an output request, but nothing popped");
+        self.push_input_response(other, input_queue_type)
+    }
+
     fn push_ingress(&mut self, msg: Ingress) {
         self.queues.push_ingress(msg)
     }
@@ -2348,4 +2361,88 @@ mod mainnet_compatibility_tests {
             assert!(!queues.queues.has_output());
         }
     }
+
+    /// Test that, with multiple input queues of different types, the order in which they
+    /// are consumed stays the same
+    mod input_order_test {
+        use super::super::*;
+        use super::*;
+
+        const OUTPUT_NAME: &str = "queues.pbuf";
+        const CANISTER_ID: CanisterId = CanisterId::from_u64(42);
+        const LOCAL_CANISTER_ID: CanisterId = CanisterId::from_u64(13);
+        const REMOTE_CANISTER_ID: CanisterId = CanisterId::from_u64(666);
+        const USER_ID: UserId = user_test_id(7);
+
+        #[test]
+        #[ignore]
+        fn serialize() {
+            let mut queues = CanisterQueuesMultiFixture::new();
+            queues.this = CANISTER_ID;
+
+            // Put a request and a response from a local canister in the input queues
+            queues
+                .push_input_request(LOCAL_CANISTER_ID, InputQueueType::LocalSubnet)
+                .unwrap();
+            queues
+                .reserve_and_push_input_response(LOCAL_CANISTER_ID, InputQueueType::LocalSubnet)
+                .unwrap();
+
+            // Put a request and a response from a remote canister in the input queues
+            queues
+                .push_input_request(REMOTE_CANISTER_ID, InputQueueType::RemoteSubnet)
+                .unwrap();
+            queues
+                .reserve_and_push_input_response(REMOTE_CANISTER_ID, InputQueueType::RemoteSubnet)
+                .unwrap();
+
+            // Put a request from the canister itself in the input queues
+            queues
+                .push_input_request(CANISTER_ID, InputQueueType::LocalSubnet)
+                .unwrap();
+
+            // Put an ingress message in the input queues
+            queues.push_ingress(
+                IngressBuilder::default()
+                    .source(USER_ID)
+                    .receiver(CANISTER_ID)
+                    .build(),
+            );
+
+            let pb_queues: pb_queues::CanisterQueues = (&queues.queues).into();
+            let serialized = pb_queues.encode_to_vec();
+
+            let output_path = std::path::Path::new(OUTPUT_NAME);
+            File::create(output_path)
+                .unwrap()
+                .write_all(&serialized)
+                .unwrap();
+        }
+
+        #[test]
+        #[ignore]
+        fn deserialize() {
+            let serialized = std::fs::read(OUTPUT_NAME).expect("Could not read file");
+            let pb_queues = pb_queues::CanisterQueues::decode(&serialized as &[u8])
+                .expect("Failed to deserialize the protobuf");
+            let c_queues = CanisterQueues::try_from((
+                pb_queues,
+                &StrictMetrics as &dyn CheckpointLoadingMetrics,
+            ))
+            .expect("Failed to convert the protobuf to CanisterQueues");
+
+            let mut queues = CanisterQueuesMultiFixture::new();
+            queues.queues = c_queues;
+            queues.this = CANISTER_ID;
+
+            assert_matches!(queues.pop_input().unwrap(), CanisterMessage::Request(ref req) if req.sender == LOCAL_CANISTER_ID);
+            assert_matches!(queues.pop_input().unwrap(), CanisterMessage::Ingress(ref ing) if ing.source == USER_ID);
+            assert_matches!(queues.pop_input().unwrap(), CanisterMessage::Request(ref req) if req.sender == REMOTE_CANISTER_ID);
+            assert_matches!(queues.pop_input().unwrap(), CanisterMessage::Request(ref req) if req.sender == CANISTER_ID);
+            assert_matches!(queues.pop_input().unwrap(), CanisterMessage::Response(ref req) if req.respondent == REMOTE_CANISTER_ID);
+            assert_matches!(queues.pop_input().unwrap(), CanisterMessage::Response(ref req) if req.respondent == LOCAL_CANISTER_ID);
+
+            assert!(!queues.has_input());
+        }
+    }
 }

rs/sns/root/src/lib.rs

@@ -1685,7 +1685,7 @@ mod tests {
     async fn register_dapp_canisters_redundant() {
         // Step 1: Prepare the world.
         thread_local! {
-            static DAPP_CANISTER_ID: PrincipalId = PrincipalId::new_user_test_id(4);
+            static DAPP_CANISTER_ID: PrincipalId = const { PrincipalId::new_user_test_id(4) };
             static SNS_ROOT_CANISTER: RefCell<SnsRootCanister> = RefCell::new(SnsRootCanister {
                 governance_canister_id: Some(PrincipalId::new_user_test_id(1)),
                 ledger_canister_id: Some(PrincipalId::new_user_test_id(2)),

rs/tests/message_routing/BUILD.bazel

@@ -87,11 +87,19 @@ system_test(
 
 system_test(
     name = "queues_compatibility_test",
-    proc_macro_deps = MACRO_DEPENDENCIES,
     target_compatible_with = ["@platforms//os:linux"],  # requires libssh that does not build on Mac OS
     runtime_deps = [
         "//rs/replicated_state:replicated_state_test_binary",
         "//testnet:mainnet_revisions",
     ],
-    deps = DEPENDENCIES + ["//rs/tests"],
+    deps = [
+        "//rs/recovery",
+        "//rs/tests/driver:ic-system-test-driver",
+        "//rs/types/types",
+        "@crate_index//:anyhow",
+        "@crate_index//:serde",
+        "@crate_index//:serde_json",
+        "@crate_index//:slog",
+        "@crate_index//:tempfile",
+    ],
 )

rs/tests/message_routing/queues_compatibility_test.rs

@@ -67,6 +67,11 @@ fn run_unit_test(
         .output()
         .unwrap_or_else(|e| panic!("Could not execute unit test binary {binary:?}: {e:?}"));
     info!(logger, "Command output: {:?}", output);
+    info!(
+        logger,
+        "Command output stderr: {}",
+        String::from_utf8_lossy(&output.stderr)
+    );
     assert!(
         output.status.success(),
         "Command failed: with status {:?}",
@@ -86,7 +91,7 @@ fn run_unit_test(
 
 fn download_mainnet_binary(
     binary_name: &str,
-    version: String,
+    version: &str,
     target_dir: &Path,
     log: &Logger,
 ) -> PathBuf {
@@ -98,7 +103,7 @@ fn download_mainnet_binary(
         || async {
             download_binary(
                 log,
-                ReplicaVersion::try_from(version.clone()).unwrap(),
+                ReplicaVersion::try_from(version).unwrap(),
                 binary_name.into(),
                 target_dir,
             )
@@ -140,27 +145,72 @@ fn test_one_direction(
     );
 }
 
+enum TestType {
+    SelfTestOnly,
+    Bidirectional {
+        published_binary: String,
+        mainnet_version: String,
+    },
+}
+
 struct TestCase {
-    published_binary: String,
     test_binary: String,
     test_module: String,
+    test_type: TestType,
 }
 
 impl TestCase {
-    fn new(published_binary: &str, test_binary: &str, test_module: &str) -> Self {
+    fn new(test_type: TestType, test_binary: &str, test_module: &str) -> Self {
         Self {
-            published_binary: published_binary.to_string(),
             test_binary: test_binary.to_string(),
             test_module: test_module.to_string(),
+            test_type,
+        }
+    }
+
+    pub fn run(&self, logger: &Logger) {
+        match &self.test_type {
+            TestType::Bidirectional {
+                published_binary,
+                mainnet_version,
+            } => {
+                self.self_test(logger);
+                self.bidirectional_test(mainnet_version, published_binary, logger)
+            }
+            TestType::SelfTestOnly => {
+                self.self_test(logger);
+            }
         }
     }
 
-    pub fn bidirectional_test(&self, mainnet_version: String, logger: &Logger) {
+    fn self_test(&self, logger: &Logger) {
+        let test_binary = data_dependency_file(&self.test_binary);
+        info!(
+            logger,
+            "Testing self-compatibility of module {}", self.test_module
+        );
+        let tmp_dir = tempfile::tempdir().unwrap();
+        let tmp_dir_path = tmp_dir.path();
+        test_one_direction(
+            &test_binary,
+            &test_binary,
+            &self.test_module,
+            tmp_dir_path,
+            logger,
+        );
+    }
+
+    fn bidirectional_test(
+        &self,
+        mainnet_version: &str,
+        published_binary_name: &str,
+        logger: &Logger,
+    ) {
         let download_dir = tempfile::tempdir().unwrap();
         let download_dir_path = download_dir.path();
         let published_binary = download_mainnet_binary(
-            &self.published_binary,
-            mainnet_version.clone(),
+            published_binary_name,
+            mainnet_version,
             download_dir_path,
             logger,
         );
@@ -170,11 +220,10 @@ impl TestCase {
             "Testing module {} with the mainnet commit {} and published binary {}",
             self.test_module,
             mainnet_version,
-            self.published_binary
+            published_binary_name,
         );
         for (direction, from, to) in [
             ("upgrade", &published_binary, &test_binary),
-            ("current self-compatibility", &test_binary, &test_binary),
             ("downgrade", &test_binary, &published_binary),
         ] {
             info!(logger, "Testing {}", direction);
@@ -193,12 +242,6 @@ struct Subnets {
 fn test(env: TestEnv) {
     let logger = env.logger();
 
-    let test_case = TestCase::new(
-        "replicated-state-test",
-        "ic/rs/replicated_state/replicated_state_test_binary/replicated_state_test_binary",
-        "canister_state::queues::tests::mainnet_compatibility_tests::basic_test",
-    );
-
     let versions_json = env
         .read_dependency_to_string("testnet/mainnet_revisions.json")
         .expect("mainnet IC versions");
@@ -209,8 +252,26 @@ fn test(env: TestEnv) {
 
     info!(logger, "Mainnet versions: {:?}", mainnet_versions);
 
-    for mainnet_version in mainnet_versions {
-        test_case.bidirectional_test(mainnet_version, &logger)
+    let tests = mainnet_versions
+        .iter()
+        .map(|v| {
+            TestCase::new(
+                TestType::Bidirectional {
+                    published_binary: "replicated-state-test".to_string(),
+                    mainnet_version: v.clone(),
+                },
+                "ic/rs/replicated_state/replicated_state_test_binary/replicated_state_test_binary",
+                "canister_state::queues::tests::mainnet_compatibility_tests::basic_test",
+            )
+        })
+        .chain([TestCase::new(
+            TestType::SelfTestOnly,
+            "ic/rs/replicated_state/replicated_state_test_binary/replicated_state_test_binary",
+            "canister_state::queues::tests::mainnet_compatibility_tests::input_order_test",
+        )]);
+
+    for t in tests {
+        t.run(&logger);
     }
 }
 

rs/types/base_types/src/principal_id.rs

@@ -196,11 +196,25 @@ impl PrincipalId {
         PrincipalId::new(len + 1, blob)
     }
 
-    pub fn new_user_test_id(n: u64) -> Self {
-        let mut bytes = n.to_le_bytes().to_vec();
-        bytes.push(0xfe); // internal marker for user test ids
-        Self::new_opaque(&bytes[..])
+    pub const fn new_user_test_id(n: u64) -> Self {
+        let mut bytes = [0u8; Self::MAX_LENGTH_IN_BYTES];
+        let n_bytes = n.to_le_bytes();
+
+        // Copy the u64 bytes into the array
+        // Need a while loop since one can't use a for loop in const functions, see:
+        // https://github.com/rust-lang/rust/issues/87575
+        let mut i = 0;
+        while i < n_bytes.len() {
+            bytes[i] = n_bytes[i];
+            i += 1;
+        }
+
+        // Append the internal marker
+        bytes[n_bytes.len()] = 0xfe;
+
+        Self::new_opaque_from_array(bytes, n_bytes.len() + 1)
     }
+
     pub fn new_node_test_id(n: u64) -> Self {
         let mut bytes = n.to_le_bytes().to_vec();
         bytes.push(0xfd); // internal marker for node test ids

rs/types/types_test_utils/src/ids.rs

@@ -174,8 +174,8 @@ pub fn new_node_test_id(i: u64) -> NodeId {
 }
 
 /// Returns a [`UserId`] that can be used in tests.
-pub fn user_test_id(i: u64) -> UserId {
-    UserId::from(PrincipalId::new_user_test_id(i))
+pub const fn user_test_id(i: u64) -> UserId {
+    UserId::new(PrincipalId::new_user_test_id(i))
 }
 
 /// Returns the user id of the anonymous user.