From b4fc7f6a19b8c53bd0009467bc61c4c0beec6f9d Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Thu, 13 Jul 2023 13:56:25 +0200 Subject: [PATCH 1/2] cosmwasm_std: remove dependency on DefaultHasher in tests DefaultHasher is part of std module and is not available on no_std builds. Remove dependency on that type to pave the path for no_std support. This is done by adding test_utils::check_hash_impl helper function which tests whether type implements Hash trait. That function uses crc32fast::Hasher instead of DefaultHasher thus eliminating need for std. Furthermore, Binary and HexBinary tests checking AsRef implementation call AsRef::as_ref directly and explicitly check the result rather than unnecessarily hashing the data. As an added bonus, this diff has negative delta. \o/ --- Cargo.lock | 1 + packages/std/Cargo.toml | 1 + packages/std/src/addresses.rs | 20 ++------------ packages/std/src/binary.rs | 48 +++++----------------------------- packages/std/src/hex_binary.rs | 48 +++++----------------------------- packages/std/src/lib.rs | 4 +++ packages/std/src/test_utils.rs | 24 +++++++++++++++++ 7 files changed, 44 insertions(+), 102 deletions(-) create mode 100644 packages/std/src/test_utils.rs diff --git a/Cargo.lock b/Cargo.lock index 0071b3775a..2d51248c97 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -489,6 +489,7 @@ dependencies = [ "cosmwasm-crypto", "cosmwasm-derive", "cosmwasm-schema", + "crc32fast", "derivative", "forward_ref", "hex", diff --git a/packages/std/Cargo.toml b/packages/std/Cargo.toml index 0c34b84a5c..aadcbb4488 100644 --- a/packages/std/Cargo.toml +++ b/packages/std/Cargo.toml @@ -63,5 +63,6 @@ cosmwasm-crypto = { path = "../crypto", version = "1.3.0-rc.0" } cosmwasm-schema = { path = "../schema" } # The chrono dependency is only used in an example, which Rust compiles for us. If this causes trouble, remove it. chrono = { version = "0.4", default-features = false, features = ["alloc", "std"] } +crc32fast = "1.3.2" hex-literal = "0.3.1" serde_json = "1.0.81" diff --git a/packages/std/src/addresses.rs b/packages/std/src/addresses.rs index 8ca10d0544..7eec12502c 100644 --- a/packages/std/src/addresses.rs +++ b/packages/std/src/addresses.rs @@ -401,9 +401,7 @@ mod tests { use super::*; use crate::HexBinary; use hex_literal::hex; - use std::collections::hash_map::DefaultHasher; use std::collections::HashSet; - use std::hash::{Hash, Hasher}; #[test] fn addr_unchecked_works() { @@ -651,23 +649,9 @@ mod tests { #[test] fn canonical_addr_implements_hash() { - let alice1 = CanonicalAddr::from([0, 187, 61, 11, 250, 0]); - let mut hasher = DefaultHasher::new(); - alice1.hash(&mut hasher); - let alice1_hash = hasher.finish(); - - let alice2 = CanonicalAddr::from([0, 187, 61, 11, 250, 0]); - let mut hasher = DefaultHasher::new(); - alice2.hash(&mut hasher); - let alice2_hash = hasher.finish(); - + let alice = CanonicalAddr::from([0, 187, 61, 11, 250, 0]); let bob = CanonicalAddr::from([16, 21, 33, 0, 255, 9]); - let mut hasher = DefaultHasher::new(); - bob.hash(&mut hasher); - let bob_hash = hasher.finish(); - - assert_eq!(alice1_hash, alice2_hash); - assert_ne!(alice1_hash, bob_hash); + crate::test_utils::check_hash_impl(alice, bob) } /// This requires Hash and Eq to be implemented diff --git a/packages/std/src/binary.rs b/packages/std/src/binary.rs index 15bac355d8..9696063422 100644 --- a/packages/std/src/binary.rs +++ b/packages/std/src/binary.rs @@ -240,9 +240,7 @@ mod tests { use super::*; use crate::errors::StdError; use crate::serde::{from_slice, to_vec}; - use std::collections::hash_map::DefaultHasher; use std::collections::HashSet; - use std::hash::{Hash, Hasher}; #[test] fn encode_decode() { @@ -505,51 +503,17 @@ mod tests { #[test] fn binary_implements_as_ref() { - // Can use as_ref (this we already get via the Deref implementation) - let data = Binary(vec![7u8, 35, 49, 101, 0, 255]); - assert_eq!(data.as_ref(), &[7u8, 35, 49, 101, 0, 255]); - - let data = Binary(vec![7u8, 35, 49, 101, 0, 255]); - let data_ref = &data; - assert_eq!(data_ref.as_ref(), &[7u8, 35, 49, 101, 0, 255]); - - // Implements as ref - - // This is a dummy function to mimic the signature of - // https://docs.rs/sha2/0.10.6/sha2/trait.Digest.html#tymethod.digest - fn hash(data: impl AsRef<[u8]>) -> u64 { - let mut hasher = DefaultHasher::new(); - data.as_ref().hash(&mut hasher); - hasher.finish() - } - - let data = Binary(vec![7u8, 35, 49, 101, 0, 255]); - hash(data); - - let data = Binary(vec![7u8, 35, 49, 101, 0, 255]); - let data_ref = &data; - hash(data_ref); + let want = &[7u8, 35, 49, 101, 0, 255]; + let data = Binary(want.to_vec()); + assert_eq!(want, AsRef::<[u8]>::as_ref(&data)); + assert_eq!(want, AsRef::<[u8]>::as_ref(&&data)); } #[test] fn binary_implements_hash() { - let a1 = Binary::from([0, 187, 61, 11, 250, 0]); - let mut hasher = DefaultHasher::new(); - a1.hash(&mut hasher); - let a1_hash = hasher.finish(); - - let a2 = Binary::from([0, 187, 61, 11, 250, 0]); - let mut hasher = DefaultHasher::new(); - a2.hash(&mut hasher); - let a2_hash = hasher.finish(); - + let a = Binary::from([0, 187, 61, 11, 250, 0]); let b = Binary::from([16, 21, 33, 0, 255, 9]); - let mut hasher = DefaultHasher::new(); - b.hash(&mut hasher); - let b_hash = hasher.finish(); - - assert_eq!(a1_hash, a2_hash); - assert_ne!(a1_hash, b_hash); + crate::test_utils::check_hash_impl(a, b) } /// This requires Hash and Eq to be implemented diff --git a/packages/std/src/hex_binary.rs b/packages/std/src/hex_binary.rs index bc3bc5ba21..e81ee35ede 100644 --- a/packages/std/src/hex_binary.rs +++ b/packages/std/src/hex_binary.rs @@ -248,9 +248,7 @@ mod tests { use super::*; use crate::{from_slice, to_vec, StdError}; - use std::collections::hash_map::DefaultHasher; use std::collections::HashSet; - use std::hash::{Hash, Hasher}; #[test] fn from_hex_works() { @@ -569,51 +567,17 @@ mod tests { #[test] fn hex_binary_implements_as_ref() { - // Can use as_ref (this we already get via the Deref implementation) - let data = HexBinary(vec![7u8, 35, 49, 101, 0, 255]); - assert_eq!(data.as_ref(), &[7u8, 35, 49, 101, 0, 255]); - - let data = HexBinary(vec![7u8, 35, 49, 101, 0, 255]); - let data_ref = &data; - assert_eq!(data_ref.as_ref(), &[7u8, 35, 49, 101, 0, 255]); - - // Implements as ref - - // This is a dummy function to mimic the signature of - // https://docs.rs/sha2/0.10.6/sha2/trait.Digest.html#tymethod.digest - fn hash(data: impl AsRef<[u8]>) -> u64 { - let mut hasher = DefaultHasher::new(); - data.as_ref().hash(&mut hasher); - hasher.finish() - } - - let data = HexBinary(vec![7u8, 35, 49, 101, 0, 255]); - hash(data); - - let data = HexBinary(vec![7u8, 35, 49, 101, 0, 255]); - let data_ref = &data; - hash(data_ref); + let want = &[7u8, 35, 49, 101, 0, 255]; + let data = HexBinary(want.to_vec()); + assert_eq!(want, AsRef::<[u8]>::as_ref(&data)); + assert_eq!(want, AsRef::<[u8]>::as_ref(&&data)); } #[test] fn hex_binary_implements_hash() { - let a1 = HexBinary::from([0, 187, 61, 11, 250, 0]); - let mut hasher = DefaultHasher::new(); - a1.hash(&mut hasher); - let a1_hash = hasher.finish(); - - let a2 = HexBinary::from([0, 187, 61, 11, 250, 0]); - let mut hasher = DefaultHasher::new(); - a2.hash(&mut hasher); - let a2_hash = hasher.finish(); - + let a = HexBinary::from([0, 187, 61, 11, 250, 0]); let b = HexBinary::from([16, 21, 33, 0, 255, 9]); - let mut hasher = DefaultHasher::new(); - b.hash(&mut hasher); - let b_hash = hasher.finish(); - - assert_eq!(a1_hash, a2_hash); - assert_ne!(a1_hash, b_hash); + crate::test_utils::check_hash_impl(a, b) } /// This requires Hash and Eq to be implemented diff --git a/packages/std/src/lib.rs b/packages/std/src/lib.rs index 0f26389b38..326e6f1132 100644 --- a/packages/std/src/lib.rs +++ b/packages/std/src/lib.rs @@ -121,6 +121,10 @@ pub use crate::imports::{ExternalApi, ExternalQuerier, ExternalStorage}; #[cfg(not(target_arch = "wasm32"))] pub mod testing; +// Internal testing utilities +#[cfg(test)] +mod test_utils; + // Re-exports pub use cosmwasm_derive::entry_point; diff --git a/packages/std/src/test_utils.rs b/packages/std/src/test_utils.rs new file mode 100644 index 0000000000..3da93c189b --- /dev/null +++ b/packages/std/src/test_utils.rs @@ -0,0 +1,24 @@ +//! Module with common routines used internally by the library in unit tests. + +use core::hash::{Hash, Hasher}; + +/// Tests that type `T` implements `Hash` trait correctly. +/// +/// `foo` and `bar` must be different objects. +/// +/// Some object pairs may produce the same hash causing test failure. In those +/// cases try different objects. The test uses stable hasher so once working +/// pair is identified, the test’s going to continue passing. +pub(crate) fn check_hash_impl(foo: T, bar: T) { + let foo_copy = foo.clone(); + + fn hash(value: &T) -> u64 { + let mut hasher = crc32fast::Hasher::default(); + value.hash(&mut hasher); + hasher.finish() + } + + let foo_hash = hash(&foo); + assert_eq!(foo_hash, hash(&foo_copy)); + assert_ne!(foo_hash, hash(&bar)); +} From a3c95971002e291ff8c18d3a990e87f9d6cec74c Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Sat, 15 Jul 2023 12:26:53 +0200 Subject: [PATCH 2/2] Convert test helper into assert_hash_works --- packages/std/src/addresses.rs | 4 +- packages/std/src/binary.rs | 3 +- packages/std/src/hex_binary.rs | 4 +- packages/std/src/lib.rs | 4 -- packages/std/src/test_utils.rs | 24 ----------- packages/std/src/testing/assertions.rs | 60 ++++++++++++++++++++++++++ packages/std/src/testing/mod.rs | 2 + 7 files changed, 68 insertions(+), 33 deletions(-) delete mode 100644 packages/std/src/test_utils.rs diff --git a/packages/std/src/addresses.rs b/packages/std/src/addresses.rs index 7eec12502c..3110b4d634 100644 --- a/packages/std/src/addresses.rs +++ b/packages/std/src/addresses.rs @@ -399,7 +399,7 @@ fn hash(ty: &str, key: &[u8]) -> Vec { #[cfg(test)] mod tests { use super::*; - use crate::HexBinary; + use crate::{assert_hash_works, HexBinary}; use hex_literal::hex; use std::collections::HashSet; @@ -651,7 +651,7 @@ mod tests { fn canonical_addr_implements_hash() { let alice = CanonicalAddr::from([0, 187, 61, 11, 250, 0]); let bob = CanonicalAddr::from([16, 21, 33, 0, 255, 9]); - crate::test_utils::check_hash_impl(alice, bob) + assert_hash_works!(alice, bob); } /// This requires Hash and Eq to be implemented diff --git a/packages/std/src/binary.rs b/packages/std/src/binary.rs index 9696063422..368d5f6f84 100644 --- a/packages/std/src/binary.rs +++ b/packages/std/src/binary.rs @@ -238,6 +238,7 @@ impl<'de> de::Visitor<'de> for Base64Visitor { #[cfg(test)] mod tests { use super::*; + use crate::assert_hash_works; use crate::errors::StdError; use crate::serde::{from_slice, to_vec}; use std::collections::HashSet; @@ -513,7 +514,7 @@ mod tests { fn binary_implements_hash() { let a = Binary::from([0, 187, 61, 11, 250, 0]); let b = Binary::from([16, 21, 33, 0, 255, 9]); - crate::test_utils::check_hash_impl(a, b) + assert_hash_works!(a, b); } /// This requires Hash and Eq to be implemented diff --git a/packages/std/src/hex_binary.rs b/packages/std/src/hex_binary.rs index e81ee35ede..26048dc30b 100644 --- a/packages/std/src/hex_binary.rs +++ b/packages/std/src/hex_binary.rs @@ -247,7 +247,7 @@ impl<'de> de::Visitor<'de> for HexVisitor { mod tests { use super::*; - use crate::{from_slice, to_vec, StdError}; + use crate::{assert_hash_works, from_slice, to_vec, StdError}; use std::collections::HashSet; #[test] @@ -577,7 +577,7 @@ mod tests { fn hex_binary_implements_hash() { let a = HexBinary::from([0, 187, 61, 11, 250, 0]); let b = HexBinary::from([16, 21, 33, 0, 255, 9]); - crate::test_utils::check_hash_impl(a, b) + assert_hash_works!(a, b); } /// This requires Hash and Eq to be implemented diff --git a/packages/std/src/lib.rs b/packages/std/src/lib.rs index 326e6f1132..0f26389b38 100644 --- a/packages/std/src/lib.rs +++ b/packages/std/src/lib.rs @@ -121,10 +121,6 @@ pub use crate::imports::{ExternalApi, ExternalQuerier, ExternalStorage}; #[cfg(not(target_arch = "wasm32"))] pub mod testing; -// Internal testing utilities -#[cfg(test)] -mod test_utils; - // Re-exports pub use cosmwasm_derive::entry_point; diff --git a/packages/std/src/test_utils.rs b/packages/std/src/test_utils.rs deleted file mode 100644 index 3da93c189b..0000000000 --- a/packages/std/src/test_utils.rs +++ /dev/null @@ -1,24 +0,0 @@ -//! Module with common routines used internally by the library in unit tests. - -use core::hash::{Hash, Hasher}; - -/// Tests that type `T` implements `Hash` trait correctly. -/// -/// `foo` and `bar` must be different objects. -/// -/// Some object pairs may produce the same hash causing test failure. In those -/// cases try different objects. The test uses stable hasher so once working -/// pair is identified, the test’s going to continue passing. -pub(crate) fn check_hash_impl(foo: T, bar: T) { - let foo_copy = foo.clone(); - - fn hash(value: &T) -> u64 { - let mut hasher = crc32fast::Hasher::default(); - value.hash(&mut hasher); - hasher.finish() - } - - let foo_hash = hash(&foo); - assert_eq!(foo_hash, hash(&foo_copy)); - assert_ne!(foo_hash, hash(&bar)); -} diff --git a/packages/std/src/testing/assertions.rs b/packages/std/src/testing/assertions.rs index bac91d0138..78df8450a3 100644 --- a/packages/std/src/testing/assertions.rs +++ b/packages/std/src/testing/assertions.rs @@ -1,4 +1,6 @@ use crate::{Decimal, Uint128}; +#[cfg(test)] +use core::hash::{Hash, Hasher}; use std::str::FromStr as _; /// Asserts that two expressions are approximately equal to each other. @@ -21,6 +23,25 @@ macro_rules! assert_approx_eq { }}; } +/// Asserts that type `T` implements `Hash` trait correctly. +/// +/// `left` and `right` must be unequal objects. +/// +/// Some object pairs may produce the same hash causing test failure. +/// In those cases try different objects. The test uses stable hasher +/// so once working pair is identified, the test’s going to continue +/// passing. +#[macro_export] +#[cfg(test)] +macro_rules! assert_hash_works { + ($left:expr, $right:expr $(,)?) => {{ + $crate::testing::assert_hash_works_impl($left, $right, None); + }}; + ($left:expr, $right:expr, $($args:tt)+) => {{ + $crate::testing::assert_hash_works_impl($left, $right, Some(format!($($args)*))); + }}; +} + /// Implementation for the [`cosmwasm_std::assert_approx_eq`] macro. This does not provide any /// stability guarantees and may change any time. #[track_caller] @@ -50,6 +71,45 @@ pub fn assert_approx_eq_impl>( } } +/// Tests that type `T` implements `Hash` trait correctly. +/// +/// `left` and `right` must be unequal objects. +/// +/// Some object pairs may produce the same hash causing test failure. +/// In those cases try different objects. The test uses stable hasher +/// so once working pair is identified, the test’s going to continue +/// passing. +#[track_caller] +#[doc(hidden)] +#[cfg(test)] +pub fn assert_hash_works_impl(left: T, right: T, panic_msg: Option) { + fn hash(value: &impl Hash) -> u64 { + let mut hasher = crc32fast::Hasher::default(); + value.hash(&mut hasher); + hasher.finish() + } + + // Check clone + if hash(&left) != hash(&left.clone()) { + match panic_msg { + Some(panic_msg) => { + panic!("assertion failed: `hash(left) == hash(left.clone())`\n: {panic_msg}") + } + None => panic!("assertion failed: `hash(left) == hash(left.clone())`"), + } + } + + // Check different object + if hash(&left) == hash(&right) { + match panic_msg { + Some(panic_msg) => { + panic!("assertion failed: `hash(left) != hash(right)`\n: {panic_msg}") + } + None => panic!("assertion failed: `hash(left) != hash(right)`"), + } + } +} + #[cfg(test)] mod tests { #[test] diff --git a/packages/std/src/testing/mod.rs b/packages/std/src/testing/mod.rs index 550486a0b6..14b362ce42 100644 --- a/packages/std/src/testing/mod.rs +++ b/packages/std/src/testing/mod.rs @@ -8,6 +8,8 @@ mod mock; mod shuffle; pub use assertions::assert_approx_eq_impl; +#[cfg(test)] +pub use assertions::assert_hash_works_impl; #[cfg(feature = "cosmwasm_1_3")] pub use mock::DistributionQuerier;