From 782c43753e896ebd228cc9a1a1ad7c19498d2fff Mon Sep 17 00:00:00 2001 From: Yan Chen <48968912+chenyan-dfinity@users.noreply.github.com> Date: Thu, 26 Oct 2023 11:15:19 -0700 Subject: [PATCH 1/5] fix service_equal error reporting (#486) --- rust/candid/src/types/subtype.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/rust/candid/src/types/subtype.rs b/rust/candid/src/types/subtype.rs index 552c9b85..4101cdbc 100644 --- a/rust/candid/src/types/subtype.rs +++ b/rust/candid/src/types/subtype.rs @@ -143,7 +143,8 @@ pub fn equal(gamma: &mut Gamma, env: &TypeEnv, t1: &Type, t2: &Type) -> Result<( (Opt(ty1), Opt(ty2)) => equal(gamma, env, ty1, ty2), (Vec(ty1), Vec(ty2)) => equal(gamma, env, ty1, ty2), (Record(fs1), Record(fs2)) | (Variant(fs1), Variant(fs2)) => { - assert_length(fs1, fs2, |x| x.to_string()).context("Different field length")?; + assert_length(fs1, fs2, |x| x.id.clone(), |x| x.to_string()) + .context("Different field length")?; for (f1, f2) in fs1.iter().zip(fs2.iter()) { if f1.id != f2.id { return Err(Error::msg(format!( @@ -159,7 +160,7 @@ pub fn equal(gamma: &mut Gamma, env: &TypeEnv, t1: &Type, t2: &Type) -> Result<( Ok(()) } (Service(ms1), Service(ms2)) => { - assert_length(ms1, ms2, |x| format!("method {} : {}", x.0, x.1)) + assert_length(ms1, ms2, |x| x.0.clone(), |x| format!("method {x}")) .context("Different method length")?; for (m1, m2) in ms1.iter().zip(ms2.iter()) { if m1.0 != m2.0 { @@ -203,18 +204,19 @@ pub fn equal(gamma: &mut Gamma, env: &TypeEnv, t1: &Type, t2: &Type) -> Result<( } } -fn assert_length(left: &[I], right: &[I], display: F) -> Result<()> +fn assert_length(left: &[I], right: &[I], get_key: F, display: D) -> Result<()> where - F: Fn(&I) -> String, - I: Clone + std::hash::Hash + std::cmp::Eq, + F: Fn(&I) -> K + Clone, + K: std::hash::Hash + std::cmp::Eq, + D: Fn(&K) -> String, { let l = left.len(); let r = right.len(); if l == r { return Ok(()); } - let left: HashSet<_> = left.iter().cloned().collect(); - let right: HashSet<_> = right.iter().cloned().collect(); + let left: HashSet<_> = left.iter().map(get_key.clone()).collect(); + let right: HashSet<_> = right.iter().map(get_key).collect(); if l < r { let mut diff = right.difference(&left); Err(Error::msg(format!( From a968a8281338af291c5e5e9837b76c19d15cc4a0 Mon Sep 17 00:00:00 2001 From: Venkkatesh Sekar Date: Fri, 27 Oct 2023 09:33:39 +0000 Subject: [PATCH 2/5] Add Arbitrary for Principal and extend random for Func --- rust/candid/src/parser/random.rs | 5 ++++- rust/candid/src/types/principal.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/rust/candid/src/parser/random.rs b/rust/candid/src/parser/random.rs index 79ce9019..4b38dc59 100644 --- a/rust/candid/src/parser/random.rs +++ b/rust/candid/src/parser/random.rs @@ -140,7 +140,7 @@ impl<'a> GenState<'a> { TypeInner::Int64 => IDLValue::Int64(arbitrary_num(u, self.config.range)?), TypeInner::Float32 => IDLValue::Float32(u.arbitrary()?), TypeInner::Float64 => IDLValue::Float64(u.arbitrary()?), - TypeInner::Principal => IDLValue::Principal(crate::Principal::anonymous()), + TypeInner::Principal => IDLValue::Principal(crate::Principal::arbitrary(u)?), TypeInner::Text => { IDLValue::Text(arbitrary_text(u, &self.config.text, &self.config.width)?) } @@ -204,6 +204,9 @@ impl<'a> GenState<'a> { }; IDLValue::Variant(VariantValue(Box::new(field), idx as u64)) } + TypeInner::Func(_) => { + IDLValue::Func(crate::Principal::arbitrary(u)?, String::arbitrary(u)?) + } _ => unimplemented!(), }); self.pop_state(old_config, ty, false); diff --git a/rust/candid/src/types/principal.rs b/rust/candid/src/types/principal.rs index f5c8d776..623ade5d 100644 --- a/rust/candid/src/types/principal.rs +++ b/rust/candid/src/types/principal.rs @@ -1,4 +1,6 @@ use super::{CandidType, Serializer, Type, TypeInner}; +#[cfg(feature = "random")] +use arbitrary::{Arbitrary, Result as ArbitraryResult, Unstructured}; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha224}; use std::convert::TryFrom; @@ -365,3 +367,28 @@ impl<'de> serde::Deserialize<'de> for Principal { } } } + +#[cfg(feature = "random")] +impl<'a> Arbitrary<'a> for Principal { + fn arbitrary(u: &mut Unstructured<'a>) -> ArbitraryResult { + let principal = match u8::arbitrary(u)? { + u8::MAX => Principal::management_canister(), + 254u8 => Principal::anonymous(), + _ => { + let length: usize = u.int_in_range(1..=Principal::MAX_LENGTH_IN_BYTES)?; + let mut result: Vec = Vec::with_capacity(length); + for _ in 0..length { + result.push(u8::arbitrary(u)?); + } + // non-anonymous principal cannot have type ANONYMOUS + // adapt by changing the last byte. + let last = result.last_mut().unwrap(); + if *last == 4_u8 { + *last = u8::MAX + } + Principal::try_from(&result[..]).unwrap() + } + }; + Ok(principal) + } +} From 9c2dac9d808bbad445ac07395c40951636806ee5 Mon Sep 17 00:00:00 2001 From: Venkkatesh Sekar Date: Fri, 27 Oct 2023 11:55:59 +0000 Subject: [PATCH 3/5] Use instead of --- rust/candid/src/parser/random.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust/candid/src/parser/random.rs b/rust/candid/src/parser/random.rs index 4b38dc59..681f4a01 100644 --- a/rust/candid/src/parser/random.rs +++ b/rust/candid/src/parser/random.rs @@ -205,7 +205,7 @@ impl<'a> GenState<'a> { IDLValue::Variant(VariantValue(Box::new(field), idx as u64)) } TypeInner::Func(_) => { - IDLValue::Func(crate::Principal::arbitrary(u)?, String::arbitrary(u)?) + IDLValue::Func(crate::Principal::arbitrary(u)?, arbitrary_text(u, &self.config.text, &self.config.width)?) } _ => unimplemented!(), }); From 6da45c07e9d9d674939480f1a207dc1e2bfcfa42 Mon Sep 17 00:00:00 2001 From: Venkkatesh Sekar Date: Fri, 27 Oct 2023 12:02:28 +0000 Subject: [PATCH 4/5] fmt --- rust/candid/src/parser/random.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/rust/candid/src/parser/random.rs b/rust/candid/src/parser/random.rs index 681f4a01..a7bdccb9 100644 --- a/rust/candid/src/parser/random.rs +++ b/rust/candid/src/parser/random.rs @@ -204,9 +204,10 @@ impl<'a> GenState<'a> { }; IDLValue::Variant(VariantValue(Box::new(field), idx as u64)) } - TypeInner::Func(_) => { - IDLValue::Func(crate::Principal::arbitrary(u)?, arbitrary_text(u, &self.config.text, &self.config.width)?) - } + TypeInner::Func(_) => IDLValue::Func( + crate::Principal::arbitrary(u)?, + arbitrary_text(u, &self.config.text, &self.config.width)?, + ), _ => unimplemented!(), }); self.pop_state(old_config, ty, false); From c59d0109e54cf19d88a8a777ac71936748914471 Mon Sep 17 00:00:00 2001 From: Venkkatesh Sekar Date: Tue, 31 Oct 2023 09:18:33 +0000 Subject: [PATCH 5/5] add arbitrary for principal --- Cargo.lock | 1 + rust/candid/Cargo.toml | 2 +- rust/ic_principal/Cargo.toml | 5 +++++ rust/ic_principal/src/lib.rs | 28 ++++++++++++++++++++++++++++ 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 6f025f8a..98eaec75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -855,6 +855,7 @@ dependencies = [ name = "ic_principal" version = "0.1.0" dependencies = [ + "arbitrary", "crc32fast", "data-encoding", "hex", diff --git a/rust/candid/Cargo.toml b/rust/candid/Cargo.toml index dd263d93..e0ff00c2 100644 --- a/rust/candid/Cargo.toml +++ b/rust/candid/Cargo.toml @@ -50,7 +50,7 @@ bignum = ["num-bigint", "num-traits"] value = ["bignum"] printer = ["pretty"] default = ["serde_bytes", "printer", "bignum"] -all = ["default", "value"] +all = ["default", "value", "ic_principal/arbitrary"] [[test]] name = "types" diff --git a/rust/ic_principal/Cargo.toml b/rust/ic_principal/Cargo.toml index 8164b80d..157f5f43 100644 --- a/rust/ic_principal/Cargo.toml +++ b/rust/ic_principal/Cargo.toml @@ -34,6 +34,11 @@ optional = true version = "0.11.5" optional = true +[dependencies.arbitrary] +version = "1.0" +optional = true + [features] # Default features include serde support. default = ['serde', 'serde_bytes'] +arbitrary = ['default', 'dep:arbitrary'] diff --git a/rust/ic_principal/src/lib.rs b/rust/ic_principal/src/lib.rs index b128ccce..a0fa9b92 100644 --- a/rust/ic_principal/src/lib.rs +++ b/rust/ic_principal/src/lib.rs @@ -5,6 +5,9 @@ use std::convert::TryFrom; use std::fmt::Write; use thiserror::Error; +#[cfg(feature = "arbitrary")] +use arbitrary::{Arbitrary, Result as ArbitraryResult, Unstructured}; + /// An error happened while encoding, decoding or serializing a [`Principal`]. #[derive(Error, Clone, Debug, Eq, PartialEq)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -358,3 +361,28 @@ impl<'de> serde::Deserialize<'de> for Principal { } } } + +#[cfg(feature = "arbitrary")] +impl<'a> Arbitrary<'a> for Principal { + fn arbitrary(u: &mut Unstructured<'a>) -> ArbitraryResult { + let principal = match u8::arbitrary(u)? { + u8::MAX => Principal::management_canister(), + 254u8 => Principal::anonymous(), + _ => { + let length: usize = u.int_in_range(1..=Principal::MAX_LENGTH_IN_BYTES)?; + let mut result: Vec = Vec::with_capacity(length); + for _ in 0..length { + result.push(u8::arbitrary(u)?); + } + // non-anonymous principal cannot have type ANONYMOUS + // adapt by changing the last byte. + let last = result.last_mut().unwrap(); + if *last == 4_u8 { + *last = u8::MAX + } + Principal::try_from(&result[..]).unwrap() + } + }; + Ok(principal) + } +}