Merge branch 'dimitris/structured-enums' into 'master'

test: Add protobuf round trip tests for some structured enums

This MR adds some protobuf round trip tests for structured enums using the `ExhaustiveSet` derive to produce a list of possible values. It's done for the ones that are in the `ic_types` (unless if they are using named field variants which is not supported yet -- will be added later).

There are also a few cases of enums that live outside the `ic_types` crate. This is an issue in general because we cannot have this `derive(ExhaustiveSet)` be a `cfg_attr(test)` unless everything is in the same crate (if we move it to its own crate, then we could depend on it but through production code, so these macros would expand there are well). There are some cases which we cannot move (e.g. types in the more specific `ic_management_canister_types` crate) but some others could be moved (e.g. that are currently in `ic_replicated_state`) and we can follow up for them in another MR. 

See merge request dfinity-lab/public/ic!19034

rs/types/types/src/ingress.rs

@@ -3,6 +3,8 @@
 use crate::artifact::IngressMessageId;
 use crate::{CanisterId, CountBytes, PrincipalId, Time, UserId};
 use ic_error_types::{ErrorCode, UserError};
+#[cfg(test)]
+use ic_exhaustive_derive::ExhaustiveSet;
 use ic_protobuf::{
     proxy::{try_from_option_field, ProxyDecodeError},
     state::ingress::v1 as pb_ingress,
@@ -165,6 +167,7 @@ impl IngressSets {
 /// This struct describes the different types that executing a Wasm function in
 /// a canister can produce
 #[derive(PartialOrd, Ord, Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[cfg_attr(test, derive(ExhaustiveSet))]
 pub enum WasmResult {
     /// Raw response, returned in a "happy" case
     Reply(#[serde(with = "serde_bytes")] Vec<u8>),
@@ -366,3 +369,20 @@ impl TryFrom<pb_ingress::IngressStatus> for IngressStatus {
         )
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::exhaustive::ExhaustiveSet;
+    use ic_crypto_test_utils_reproducible_rng::reproducible_rng;
+
+    #[test]
+    fn wasm_result_proto_round_trip() {
+        for result in WasmResult::exhaustive_set(&mut reproducible_rng()) {
+            let encoded = pb_ingress::ingress_status_completed::WasmResult::from(&result);
+            let round_trip = WasmResult::from(encoded);
+
+            assert_eq!(result, round_trip);
+        }
+    }
+}

rs/types/types/src/messages/inter_canister.rs

@@ -49,6 +49,7 @@ pub enum CallContextIdTag {}
 pub type CallContextId = Id<CallContextIdTag, u64>;
 
 #[derive(Clone, Debug, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[cfg_attr(test, derive(ExhaustiveSet))]
 pub struct RequestMetadata {
     /// Indicates how many steps down the call tree a request is, starting at 0.
     call_tree_depth: u64,
@@ -87,6 +88,7 @@ impl RequestMetadata {
 
 /// Canister-to-canister request message.
 #[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
+#[cfg_attr(test, derive(ExhaustiveSet))]
 pub struct Request {
     pub receiver: CanisterId,
     pub sender: CanisterId,
@@ -367,6 +369,26 @@ impl From<Result<Option<WasmResult>, UserError>> for Payload {
     }
 }
 
+impl From<&Payload> for pb_queues::response::ResponsePayload {
+    fn from(value: &Payload) -> Self {
+        match value {
+            Payload::Data(d) => pb_queues::response::ResponsePayload::Data(d.clone()),
+            Payload::Reject(r) => pb_queues::response::ResponsePayload::Reject(r.into()),
+        }
+    }
+}
+
+impl TryFrom<pb_queues::response::ResponsePayload> for Payload {
+    type Error = ProxyDecodeError;
+
+    fn try_from(value: pb_queues::response::ResponsePayload) -> Result<Self, Self::Error> {
+        match value {
+            pb_queues::response::ResponsePayload::Data(d) => Ok(Payload::Data(d)),
+            pb_queues::response::ResponsePayload::Reject(r) => Ok(Payload::Reject(r.try_into()?)),
+        }
+    }
+}
+
 /// Canister-to-canister response message.
 #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[cfg_attr(test, derive(ExhaustiveSet))]
@@ -420,6 +442,7 @@ impl Hash for Response {
 /// The underlying request / response is wrapped within an `Arc`, for cheap
 /// cloning.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
+#[cfg_attr(test, derive(ExhaustiveSet))]
 pub enum RequestOrResponse {
     Request(Arc<Request>),
     Response(Arc<Response>),
@@ -625,16 +648,14 @@ impl TryFrom<pb_queues::RejectContext> for RejectContext {
 
 impl From<&Response> for pb_queues::Response {
     fn from(rep: &Response) -> Self {
-        let p = match &rep.response_payload {
-            Payload::Data(d) => pb_queues::response::ResponsePayload::Data(d.clone()),
-            Payload::Reject(r) => pb_queues::response::ResponsePayload::Reject(r.into()),
-        };
         Self {
             originator: Some(pb_types::CanisterId::from(rep.originator)),
             respondent: Some(pb_types::CanisterId::from(rep.respondent)),
             originator_reply_callback: rep.originator_reply_callback.get(),
             refund: Some((&Funds::new(rep.refund)).into()),
-            response_payload: Some(p),
+            response_payload: Some(pb_queues::response::ResponsePayload::from(
+                &rep.response_payload,
+            )),
             cycles_refund: Some((rep.refund).into()),
             deadline_seconds: rep.deadline.as_secs_since_unix_epoch(),
         }
@@ -645,14 +666,6 @@ impl TryFrom<pb_queues::Response> for Response {
     type Error = ProxyDecodeError;
 
     fn try_from(rep: pb_queues::Response) -> Result<Self, Self::Error> {
-        let response_payload = match rep
-            .response_payload
-            .ok_or(ProxyDecodeError::MissingField("Response::response_payload"))?
-        {
-            pb_queues::response::ResponsePayload::Data(d) => Payload::Data(d),
-            pb_queues::response::ResponsePayload::Reject(r) => Payload::Reject(r.try_into()?),
-        };
-
         // To maintain backwards compatibility we fall back to reading from `refund` if
         // `cycles_refund` is not set.
         let refund = match try_from_option_field(rep.cycles_refund, "Response::cycles_refund") {
@@ -666,7 +679,10 @@ impl TryFrom<pb_queues::Response> for Response {
             respondent: try_from_option_field(rep.respondent, "Response::respondent")?,
             originator_reply_callback: rep.originator_reply_callback.into(),
             refund,
-            response_payload,
+            response_payload: try_from_option_field(
+                rep.response_payload,
+                "Response::response_payload",
+            )?,
             deadline: CoarseTime::from_secs_since_unix_epoch(rep.deadline_seconds),
         })
     }

rs/types/types/src/messages/inter_canister/tests.rs

@@ -4,6 +4,8 @@ use crate::messages::{
 };
 
 use super::*;
+use crate::exhaustive::ExhaustiveSet;
+use ic_crypto_test_utils_reproducible_rng::reproducible_rng;
 use ic_types_test_utils::ids::canister_test_id;
 use std::hash::{DefaultHasher, Hash, Hasher};
 
@@ -118,3 +120,23 @@ fn max_response_count_bytes() {
     // And its total size must be exactly `MAX_RESPONSE_COUNT_BYTES`.
     assert_eq!(MAX_RESPONSE_COUNT_BYTES, reject.count_bytes());
 }
+
+#[test]
+fn response_payload_proto_round_trip() {
+    for payload in Payload::exhaustive_set(&mut reproducible_rng()) {
+        let encoded = pb_queues::response::ResponsePayload::from(&payload);
+        let round_trip = Payload::try_from(encoded).unwrap();
+
+        assert_eq!(payload, round_trip);
+    }
+}
+
+#[test]
+fn request_or_response_proto_round_trip() {
+    for r in RequestOrResponse::exhaustive_set(&mut reproducible_rng()) {
+        let encoded = pb_queues::RequestOrResponse::from(&r);
+        let round_trip = RequestOrResponse::try_from(encoded).unwrap();
+
+        assert_eq!(r, round_trip);
+    }
+}

rs/types/types/src/methods.rs

@@ -3,6 +3,8 @@
 
 use crate::{messages::CallContextId, time::CoarseTime, Cycles};
 use ic_base_types::{CanisterId, PrincipalId};
+#[cfg(test)]
+use ic_exhaustive_derive::ExhaustiveSet;
 use ic_protobuf::proxy::{try_from_option_field, ProxyDecodeError};
 use ic_protobuf::state::{canister_state_bits::v1 as pb, queues::v1::Cycles as PbCycles};
 use ic_protobuf::types::v1 as pb_types;
@@ -15,6 +17,7 @@ use strum_macros::EnumIter;
 
 /// Represents the types of methods that a Wasm module can export.
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
+#[cfg_attr(test, derive(ExhaustiveSet))]
 pub enum WasmMethod {
     /// An exported update method along with its name.
     ///
@@ -127,6 +130,7 @@ impl TryFrom<pb::WasmMethod> for WasmMethod {
 
 /// The various system methods available to canisters.
 #[derive(Clone, Debug, PartialEq, Eq, EnumIter, PartialOrd, Ord, Serialize, Deserialize)]
+#[cfg_attr(test, derive(ExhaustiveSet))]
 pub enum SystemMethod {
     /// A system method for initializing a Wasm module.
     CanisterStart = 1,
@@ -381,6 +385,8 @@ pub enum FuncRef {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::exhaustive::ExhaustiveSet;
+    use ic_crypto_test_utils_reproducible_rng::reproducible_rng;
     use ic_protobuf::state::canister_state_bits::v1 as pb;
     use strum::IntoEnumIterator;
 
@@ -403,4 +409,14 @@ mod tests {
             [1, 2, 3, 4, 5, 6, 7]
         );
     }
+
+    #[test]
+    fn wasm_method_proto_round_trip() {
+        for method in WasmMethod::exhaustive_set(&mut reproducible_rng()) {
+            let encoded = pb::WasmMethod::from(&method);
+            let round_trip = WasmMethod::try_from(encoded).unwrap();
+
+            assert_eq!(method, round_trip);
+        }
+    }
 }