From 645563347eb1b692aa6515ef3b6ac5c15975fcb9 Mon Sep 17 00:00:00 2001 From: Jens Date: Tue, 3 Dec 2024 11:07:37 +0100 Subject: [PATCH 1/4] Remove derivative dependency --- Cargo.toml | 1 - src/bitfield.rs | 33 ++++++++++++++++++++++++++++++--- src/fixed_vector.rs | 19 ++++++++++++++++--- src/variable_list.rs | 19 ++++++++++++++++--- 4 files changed, 62 insertions(+), 10 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0384d55..c81fc0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,6 @@ ethereum_ssz = "0.7.0" serde = "1.0.0" serde_derive = "1.0.0" typenum = "1.12.0" -derivative = "2.1.1" smallvec = "1.8.0" arbitrary = { version = "1.0", features = ["derive"], optional = true } itertools = "0.13.0" diff --git a/src/bitfield.rs b/src/bitfield.rs index dedaf7a..aa3a495 100644 --- a/src/bitfield.rs +++ b/src/bitfield.rs @@ -1,7 +1,6 @@ use crate::tree_hash::bitfield_bytes_tree_hash_root; use crate::Error; use core::marker::PhantomData; -use derivative::Derivative; use serde::de::{Deserialize, Deserializer}; use serde::ser::{Serialize, Serializer}; use serde_utils::hex::{encode as hex_encode, PrefixedHexVisitor}; @@ -95,14 +94,29 @@ pub type BitVector = Bitfield>; /// The internal representation of the bitfield is the same as that required by SSZ. The lowest /// byte (by `Vec` index) stores the lowest bit-indices and the right-most bit stores the lowest /// bit-index. E.g., `smallvec![0b0000_0001, 0b0000_0010]` has bits `0, 9` set. -#[derive(Clone, Debug, Derivative)] -#[derivative(PartialEq, Eq, Hash(bound = ""))] +#[derive(Clone, Debug)] pub struct Bitfield { bytes: SmallVec<[u8; SMALLVEC_LEN]>, len: usize, _phantom: PhantomData, } +// Implement comparison functions even if T doesn't implement them (since we don't sotre T) +impl PartialEq for Bitfield { + fn eq(&self, other: &Self) -> bool { + // T is already compared because other must have the same T + // We don't store values of T, so we don't have to compare it either. + self.len == other.len && self.bytes == other.bytes + } +} +impl Eq for Bitfield {} +impl std::hash::Hash for Bitfield { + fn hash(&self, state: &mut H) { + self.bytes.hash(state); + self.len.hash(state); + } +} + impl Bitfield> { /// Instantiate with capacity for `num_bits` boolean values. The length cannot be grown or /// shrunk after instantiation. @@ -1438,4 +1452,17 @@ mod bitlist { // Can't extend a BitList to a smaller BitList resized_bit_list.resize::().unwrap_err(); } + + /// Test if Eq and Hash are implemented regardless of whether T does + /// (backwards compatibility). + #[test] + fn can_use_in_hashmaps() { + let mut map: std::collections::HashSet> = std::collections::HashSet::new(); + map.reserve(5); + + struct NoDerives {} + let mut map: std::collections::HashSet> = + std::collections::HashSet::new(); + map.reserve(5); + } } diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 76e182d..05c028a 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -1,6 +1,5 @@ use crate::tree_hash::vec_tree_hash_root; use crate::Error; -use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; use std::marker::PhantomData; use std::ops::{Deref, DerefMut, Index, IndexMut}; @@ -45,14 +44,28 @@ pub use typenum; /// let long: FixedVector<_, typenum::U5> = FixedVector::from(base); /// assert_eq!(&long[..], &[1, 2, 3, 4, 0]); /// ``` -#[derive(Debug, Clone, Serialize, Deserialize, Derivative)] -#[derivative(PartialEq, Eq, Hash(bound = "T: std::hash::Hash"))] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(transparent)] pub struct FixedVector { vec: Vec, _phantom: PhantomData, } +// Implement comparison functions even if T doesn't implement them (since we don't sotre T) +impl PartialEq for FixedVector { + fn eq(&self, other: &Self) -> bool { + // T and N are already compared because other must have the same T/N + // We don't store values of T or N, so we don't have to compare them either. + self.vec == other.vec + } +} +impl Eq for FixedVector {} +impl std::hash::Hash for FixedVector { + fn hash(&self, state: &mut H) { + self.vec.hash(state); + } +} + impl FixedVector { /// Returns `Ok` if the given `vec` equals the fixed length of `Self`. Otherwise returns /// `Err`. diff --git a/src/variable_list.rs b/src/variable_list.rs index 9816839..85316ee 100644 --- a/src/variable_list.rs +++ b/src/variable_list.rs @@ -1,6 +1,5 @@ use crate::tree_hash::vec_tree_hash_root; use crate::Error; -use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; use std::marker::PhantomData; use std::ops::{Deref, DerefMut, Index, IndexMut}; @@ -47,14 +46,28 @@ pub use typenum; /// // Push a value to if it _does_ exceed the maximum. /// assert!(long.push(6).is_err()); /// ``` -#[derive(Debug, Clone, Serialize, Deserialize, Derivative)] -#[derivative(PartialEq, Eq, Hash(bound = "T: std::hash::Hash"))] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(transparent)] pub struct VariableList { vec: Vec, _phantom: PhantomData, } +// Implement comparison functions even if T doesn't implement them (since we don't sotre T) +impl PartialEq for VariableList { + fn eq(&self, other: &Self) -> bool { + // T and N are already compared because other must have the same T/N + // We don't store values of T or N, so we don't have to compare them either. + self.vec == other.vec + } +} +impl Eq for VariableList {} +impl std::hash::Hash for VariableList { + fn hash(&self, state: &mut H) { + self.vec.hash(state); + } +} + /// Maximum number of elements to pre-allocate in `try_from_iter`. /// /// Some variable lists have *very long* maximum lengths such that we can't actually fit them From f4419f54c8bf0b67ad88e8e4947ddf797b5c024c Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 5 Dec 2024 11:06:09 +1100 Subject: [PATCH 2/4] Tweak doc comments --- src/fixed_vector.rs | 4 +--- src/variable_list.rs | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 05c028a..0354c1e 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -51,11 +51,9 @@ pub struct FixedVector { _phantom: PhantomData, } -// Implement comparison functions even if T doesn't implement them (since we don't sotre T) +// Implement comparison functions even if N doesn't implement PartialEq impl PartialEq for FixedVector { fn eq(&self, other: &Self) -> bool { - // T and N are already compared because other must have the same T/N - // We don't store values of T or N, so we don't have to compare them either. self.vec == other.vec } } diff --git a/src/variable_list.rs b/src/variable_list.rs index 85316ee..578e743 100644 --- a/src/variable_list.rs +++ b/src/variable_list.rs @@ -53,11 +53,9 @@ pub struct VariableList { _phantom: PhantomData, } -// Implement comparison functions even if T doesn't implement them (since we don't sotre T) +// Implement comparison functions even if N doesn't implement PartialEq impl PartialEq for VariableList { fn eq(&self, other: &Self) -> bool { - // T and N are already compared because other must have the same T/N - // We don't store values of T or N, so we don't have to compare them either. self.vec == other.vec } } From 699252afe6b75e4f8078cf543ae2436d9fb9aa43 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 5 Dec 2024 11:40:09 +1100 Subject: [PATCH 3/4] Test hash impl --- src/fixed_vector.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/fixed_vector.rs b/src/fixed_vector.rs index 0354c1e..f4a2029 100644 --- a/src/fixed_vector.rs +++ b/src/fixed_vector.rs @@ -358,6 +358,7 @@ impl<'a, T: arbitrary::Arbitrary<'a>, N: 'static + Unsigned> arbitrary::Arbitrar mod test { use super::*; use ssz::*; + use std::collections::HashSet; use tree_hash::{merkle_root, TreeHash}; use tree_hash_derive::TreeHash; use typenum::*; @@ -513,4 +514,18 @@ mod test { merkle_root(&repeat(a.tree_hash_root().as_slice(), 16), 0) ); } + + #[test] + fn std_hash() { + let x: FixedVector = FixedVector::from(vec![3; 16]); + let y: FixedVector = FixedVector::from(vec![4; 16]); + let mut hashset = HashSet::new(); + + for value in [x.clone(), y.clone()] { + assert!(hashset.insert(value.clone())); + assert!(!hashset.insert(value.clone())); + assert!(hashset.contains(&value)); + } + assert_eq!(hashset.len(), 2); + } } From 5ad5afd866a3c85bacb3ed3e37ac61a36e621b65 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Thu, 5 Dec 2024 11:44:55 +1100 Subject: [PATCH 4/4] Test variable list Hash impl --- src/variable_list.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/variable_list.rs b/src/variable_list.rs index 578e743..7c1140b 100644 --- a/src/variable_list.rs +++ b/src/variable_list.rs @@ -332,6 +332,7 @@ impl<'a, T: arbitrary::Arbitrary<'a>, N: 'static + Unsigned> arbitrary::Arbitrar mod test { use super::*; use ssz::*; + use std::collections::HashSet; use tree_hash::{merkle_root, TreeHash}; use tree_hash_derive::TreeHash; use typenum::*; @@ -559,4 +560,18 @@ mod test { List::try_from_iter(wonky_iter).unwrap() ); } + + #[test] + fn std_hash() { + let x: VariableList = VariableList::from(vec![3; 16]); + let y: VariableList = VariableList::from(vec![4; 16]); + let mut hashset = HashSet::new(); + + for value in [x.clone(), y.clone()] { + assert!(hashset.insert(value.clone())); + assert!(!hashset.insert(value.clone())); + assert!(hashset.contains(&value)); + } + assert_eq!(hashset.len(), 2); + } }