From ff057d42b1cfd3543b69703acbcb8075682b5ab1 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 4 Jun 2024 12:02:37 +1000 Subject: [PATCH 1/4] Fix bugs in Builder --- Cargo.toml | 2 +- src/builder.rs | 38 ++++++++++++++++++++------- src/error.rs | 1 + src/lib.rs | 20 ++++++++++++++ src/list.rs | 45 ++++++++++++++++---------------- src/repeat.rs | 5 ++-- src/serde.rs | 5 ++-- src/tests/proptest/operations.rs | 10 +++---- src/tests/repeat.rs | 6 ++--- src/vector.rs | 43 +++++++++++++++--------------- 10 files changed, 106 insertions(+), 69 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 3692bc2..9ad1609 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ rayon = "1.5.1" serde = { version = "1.0.0", features = ["derive"] } tree_hash = "0.6.0" triomphe = "0.1.5" -typenum = "1.14.0" +typenum = { version = "1.17.0", features = ["const-generics"] } vec_map = "0.8.2" smallvec = "1.8.0" arbitrary = { version = "1.2.3", features = ["derive"] } diff --git a/src/builder.rs b/src/builder.rs index 5e3e620..06f9d03 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1,6 +1,7 @@ use crate::utils::{opt_packing_depth, opt_packing_factor, Length, MaybeArced}; -use crate::{Arc, Error, PackedLeaf, Tree, Value}; +use crate::{Arc, Error, PackedLeaf, Tree, Value, MAX_TREE_DEPTH}; +#[derive(Debug)] pub struct Builder { stack: Vec>>, depth: usize, @@ -13,14 +14,19 @@ pub struct Builder { } impl Builder { - pub fn new(depth: usize, level: usize) -> Self { - Self { - stack: Vec::with_capacity(depth), - depth, - level, - length: Length(0), - packing_factor: opt_packing_factor::(), - packing_depth: opt_packing_depth::().unwrap_or(0), + pub fn new(depth: usize, level: usize) -> Result { + let packing_depth = opt_packing_depth::().unwrap_or(0); + if depth.saturating_add(packing_depth) > MAX_TREE_DEPTH { + Err(Error::BuilderInvalidDepth { depth }) + } else { + Ok(Self { + stack: Vec::with_capacity(depth), + depth, + level, + length: Length(0), + packing_factor: opt_packing_factor::(), + packing_depth, + }) } } @@ -168,3 +174,17 @@ impl Builder { Ok((tree, self.depth, self.length)) } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn depth_upper_limit() { + assert_eq!( + Builder::::new(62, 0).unwrap_err(), + Error::BuilderInvalidDepth { depth: 62 } + ); + assert_eq!(Builder::::new(61, 0).unwrap().depth, 61); + } +} diff --git a/src/error.rs b/src/error.rs index 8d88d39..bcd8d79 100644 --- a/src/error.rs +++ b/src/error.rs @@ -17,6 +17,7 @@ pub enum Error { UpdateLeavesError, InvalidRebaseNode, InvalidRebaseLeaf, + BuilderInvalidDepth { depth: usize }, BuilderExpectedLeaf, BuilderStackEmptyMerge, BuilderStackEmptyMergeLeft, diff --git a/src/lib.rs b/src/lib.rs index 33a167d..6ba49c2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,6 +32,26 @@ pub use vector::Vector; use ssz::{Decode, Encode}; use tree_hash::TreeHash; +use typenum::{ + assert_type_eq, generic_const_mappings::U, IsLessOrEqual, Unsigned, B1, U9223372036854775808, +}; + +/// Maximum depth for a tree. +/// +/// We limit trees to 2^63 elements so we can avoid overflow when calculating 2^depth. +pub const MAX_TREE_DEPTH: usize = u64::BITS as usize - 1; + +pub const MAX_TREE_LENGTH: usize = 1 << MAX_TREE_DEPTH; + +/// Maximum length of lists and vectors. +pub type MaxTreeLength = U9223372036854775808; + +// Consistency check on `MAX_TREE_LENGTH` and `MaxTreeLength`. +assert_type_eq!(MaxTreeLength, U); + +/// Trait to assert the bounds on list and vector lengths +pub trait ValidN: Unsigned + IsLessOrEqual {} +impl> ValidN for N {} #[cfg(feature = "debug")] pub trait Value: Encode + Decode + TreeHash + PartialEq + Clone + std::fmt::Debug {} diff --git a/src/list.rs b/src/list.rs index 5a35b45..f8f6b38 100644 --- a/src/list.rs +++ b/src/list.rs @@ -7,7 +7,7 @@ use crate::serde::ListVisitor; use crate::tree::RebaseAction; use crate::update_map::MaxMap; use crate::utils::{arb_arc, compute_level, int_log, opt_packing_depth, updated_length, Length}; -use crate::{Arc, Cow, Error, Tree, UpdateMap, Value}; +use crate::{Arc, Cow, Error, Tree, UpdateMap, ValidN, Value}; use arbitrary::Arbitrary; use derivative::Derivative; use itertools::process_results; @@ -16,21 +16,20 @@ use ssz::{Decode, Encode, SszEncoder, TryFromIter, BYTES_PER_LENGTH_OFFSET}; use std::collections::BTreeMap; use std::marker::PhantomData; use tree_hash::{Hash256, PackedEncoding, TreeHash}; -use typenum::Unsigned; use vec_map::VecMap; #[derive(Debug, Clone, Derivative, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: Unsigned, U: UpdateMap + PartialEq"))] +#[derivative(PartialEq(bound = "T: Value, N: ValidN, U: UpdateMap + PartialEq"))] #[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value")] -#[arbitrary(bound = "N: Unsigned, U: Arbitrary<'arbitrary> + UpdateMap + PartialEq")] -pub struct List = MaxMap>> { +#[arbitrary(bound = "N: ValidN, U: Arbitrary<'arbitrary> + UpdateMap + PartialEq")] +pub struct List = MaxMap>> { pub(crate) interface: Interface, U>, } #[derive(Debug, Clone, Derivative, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: Unsigned"))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: Unsigned")] -pub struct ListInner { +#[derivative(PartialEq(bound = "T: Value, N: ValidN"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: ValidN")] +pub struct ListInner { #[arbitrary(with = arb_arc)] pub(crate) tree: Arc>, pub(crate) length: Length, @@ -40,7 +39,7 @@ pub struct ListInner { _phantom: PhantomData, } -impl> List { +impl> List { pub fn new(vec: Vec) -> Result { Self::try_from_iter(vec) } @@ -73,12 +72,12 @@ impl> List { Self::try_from_iter(std::iter::repeat(elem).take(n)) } - pub fn builder() -> Builder { + pub fn builder() -> Result, Error> { Builder::new(Self::depth(), 0) } pub fn try_from_iter(iter: impl IntoIterator) -> Result { - let mut builder = Self::builder(); + let mut builder = Self::builder()?; for item in iter.into_iter() { builder.push(item)?; @@ -204,7 +203,7 @@ impl> List { let depth = Self::depth(); let packing_depth = opt_packing_depth::().unwrap_or(0); let level = compute_level(n, depth, packing_depth); - let mut builder = Builder::new(Self::depth(), level); + let mut builder = Builder::new(Self::depth(), level)?; let mut level_iter = self.level_iter_from(n)?.peekable(); while let Some(item) = level_iter.next() { @@ -232,7 +231,7 @@ impl> List { } } -impl ImmList for ListInner { +impl ImmList for ListInner { fn get(&self, index: usize) -> Option<&T> { if index < self.len().as_usize() { self.tree @@ -258,7 +257,7 @@ impl ImmList for ListInner { impl MutList for ListInner where T: Value, - N: Unsigned, + N: ValidN, { fn validate_push(current_len: usize) -> Result<(), Error> { if current_len == N::to_usize() { @@ -304,7 +303,7 @@ where } } -impl> List { +impl> List { pub fn rebase(&self, base: &Self) -> Result { let mut rebased = self.clone(); rebased.rebase_on(base)?; @@ -330,13 +329,13 @@ impl> List { } } -impl Default for List { +impl Default for List { fn default() -> Self { Self::empty() } } -impl TreeHash for List { +impl TreeHash for List { fn tree_hash_type() -> tree_hash::TreeHashType { tree_hash::TreeHashType::List } @@ -358,7 +357,7 @@ impl TreeHash for List { } } -impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a List { +impl<'a, T: Value, N: ValidN, U: UpdateMap> IntoIterator for &'a List { type Item = &'a T; type IntoIter = InterfaceIter<'a, T, U>; @@ -367,7 +366,7 @@ impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a List> Serialize for List +impl> Serialize for List where T: Serialize, { @@ -386,7 +385,7 @@ where impl<'de, T, N, U> Deserialize<'de> for List where T: Deserialize<'de> + Value, - N: Unsigned, + N: ValidN, U: UpdateMap, { fn deserialize(deserializer: D) -> Result @@ -398,7 +397,7 @@ where } // FIXME: duplicated from `ssz::encode::impl_for_vec` -impl Encode for List { +impl Encode for List { fn is_ssz_fixed_len() -> bool { false } @@ -435,7 +434,7 @@ impl Encode for List { impl TryFromIter for List where T: Value, - N: Unsigned, + N: ValidN, { type Error = Error; @@ -450,7 +449,7 @@ where impl Decode for List where T: Value, - N: Unsigned, + N: ValidN, { fn is_ssz_fixed_len() -> bool { false diff --git a/src/repeat.rs b/src/repeat.rs index e6a70f3..e7a2926 100644 --- a/src/repeat.rs +++ b/src/repeat.rs @@ -1,14 +1,13 @@ use crate::utils::{opt_packing_factor, Length}; -use crate::{Arc, Error, Leaf, List, PackedLeaf, Tree, UpdateMap, Value}; +use crate::{Arc, Error, Leaf, List, PackedLeaf, Tree, UpdateMap, ValidN, Value}; use smallvec::{smallvec, SmallVec}; use tree_hash::Hash256; -use typenum::Unsigned; /// Efficiently construct a list from `n` copies of `elem`. pub fn repeat_list(elem: T, n: usize) -> Result, Error> where T: Value, - N: Unsigned, + N: ValidN, U: UpdateMap, { if n == 0 { diff --git a/src/serde.rs b/src/serde.rs index 282896f..4065834 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -1,8 +1,7 @@ -use crate::{List, UpdateMap, Value}; +use crate::{List, UpdateMap, ValidN, Value}; use itertools::process_results; use serde::Deserialize; use std::marker::PhantomData; -use typenum::Unsigned; pub struct ListVisitor { _phantom: PhantomData<(T, N, U)>, @@ -19,7 +18,7 @@ impl Default for ListVisitor { impl<'a, T, N, U> serde::de::Visitor<'a> for ListVisitor where T: Deserialize<'a> + Value, - N: Unsigned, + N: ValidN, U: UpdateMap, { type Value = List; diff --git a/src/tests/proptest/operations.rs b/src/tests/proptest/operations.rs index f24504f..9fa9629 100644 --- a/src/tests/proptest/operations.rs +++ b/src/tests/proptest/operations.rs @@ -1,5 +1,5 @@ use super::{arb_hash256, arb_index, arb_large, arb_list, arb_vect, Large}; -use crate::{Error, List, Value, Vector}; +use crate::{Error, List, ValidN, Value, Vector}; use proptest::prelude::*; use ssz::{Decode, Encode}; use std::fmt::Debug; @@ -11,13 +11,13 @@ const OP_LIMIT: usize = 128; /// Simple specification for `List` and `Vector` behaviour. #[derive(Debug, Clone)] -pub struct Spec { +pub struct Spec { values: Vec, allow_push: bool, _phantom: PhantomData, } -impl Spec { +impl Spec { pub fn list(values: Vec) -> Self { assert!(values.len() <= N::to_usize()); Self { @@ -173,7 +173,7 @@ where fn apply_ops_list(list: &mut List, spec: &mut Spec, ops: Vec>) where T: Value + Debug + Send + Sync, - N: Unsigned + Debug, + N: ValidN + Debug, { let mut checkpoint = list.clone(); @@ -259,7 +259,7 @@ where fn apply_ops_vect(vect: &mut Vector, spec: &mut Spec, ops: Vec>) where T: Value + Debug + Send + Sync, - N: Unsigned + Debug, + N: ValidN + Debug, { let mut checkpoint = vect.clone(); diff --git a/src/tests/repeat.rs b/src/tests/repeat.rs index 552041f..51c8f4a 100644 --- a/src/tests/repeat.rs +++ b/src/tests/repeat.rs @@ -1,9 +1,9 @@ -use crate::{List, Value}; +use crate::{List, ValidN, Value}; use std::fmt::Debug; use tree_hash::TreeHash; -use typenum::{Unsigned, U1024, U64, U8}; +use typenum::{U1024, U64, U8}; -fn list_test(val: T) { +fn list_test(val: T) { for n in 96..=N::to_usize() { let fast = List::::repeat(val.clone(), n).unwrap(); let slow = List::::repeat_slow(val.clone(), n).unwrap(); diff --git a/src/vector.rs b/src/vector.rs index f5b6711..6f36e3c 100644 --- a/src/vector.rs +++ b/src/vector.rs @@ -5,7 +5,7 @@ use crate::level_iter::LevelIter; use crate::tree::RebaseAction; use crate::update_map::MaxMap; use crate::utils::{arb_arc, Length}; -use crate::{Arc, Cow, Error, List, Tree, UpdateMap, Value}; +use crate::{Arc, Cow, Error, List, Tree, UpdateMap, ValidN, Value}; use arbitrary::Arbitrary; use derivative::Derivative; use serde::{Deserialize, Serialize}; @@ -14,25 +14,24 @@ use std::collections::BTreeMap; use std::convert::TryFrom; use std::marker::PhantomData; use tree_hash::{Hash256, PackedEncoding}; -use typenum::Unsigned; use vec_map::VecMap; #[derive(Debug, Derivative, Clone, Serialize, Deserialize, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: Unsigned, U: UpdateMap + PartialEq"))] +#[derivative(PartialEq(bound = "T: Value, N: ValidN, U: UpdateMap + PartialEq"))] #[serde(try_from = "List")] #[serde(into = "List")] -#[serde(bound(serialize = "T: Value + Serialize, N: Unsigned, U: UpdateMap"))] -#[serde(bound(deserialize = "T: Value + Deserialize<'de>, N: Unsigned, U: UpdateMap"))] +#[serde(bound(serialize = "T: Value + Serialize, N: ValidN, U: UpdateMap"))] +#[serde(bound(deserialize = "T: Value + Deserialize<'de>, N: ValidN, U: UpdateMap"))] #[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value")] -#[arbitrary(bound = "N: Unsigned, U: Arbitrary<'arbitrary> + UpdateMap")] -pub struct Vector = MaxMap>> { +#[arbitrary(bound = "N: ValidN, U: Arbitrary<'arbitrary> + UpdateMap")] +pub struct Vector = MaxMap>> { pub(crate) interface: Interface, U>, } #[derive(Debug, Derivative, Clone, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: Unsigned"))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: Unsigned")] -pub struct VectorInner { +#[derivative(PartialEq(bound = "T: Value, N: ValidN"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: ValidN")] +pub struct VectorInner { #[arbitrary(with = arb_arc)] pub(crate) tree: Arc>, pub(crate) depth: usize, @@ -41,7 +40,7 @@ pub struct VectorInner { _phantom: PhantomData, } -impl> Vector { +impl> Vector { pub fn new(vec: Vec) -> Result { if vec.len() == N::to_usize() { Self::try_from(List::new(vec)?) @@ -109,7 +108,7 @@ impl> Vector { } } -impl> TryFrom> for Vector { +impl> TryFrom> for Vector { type Error = Error; fn try_from(list: List) -> Result { @@ -137,7 +136,7 @@ impl> TryFrom> for Vector> Vector { +impl> Vector { pub fn rebase(&self, base: &Self) -> Result { let mut rebased = self.clone(); rebased.rebase_on(base)?; @@ -163,7 +162,7 @@ impl> Vector { } } -impl> From> for List { +impl> From> for List { fn from(vector: Vector) -> Self { let mut list = List::from_parts( vector.interface.backing.tree, @@ -175,7 +174,7 @@ impl> From> for List ImmList for VectorInner { +impl ImmList for VectorInner { fn get(&self, index: usize) -> Option<&T> { if index < self.len().as_usize() { self.tree @@ -201,7 +200,7 @@ impl ImmList for VectorInner { impl MutList for VectorInner where T: Value, - N: Unsigned, + N: ValidN, { fn validate_push(_current_len: usize) -> Result<(), Error> { Err(Error::PushNotSupported) @@ -238,7 +237,7 @@ where } } -impl Default for Vector { +impl Default for Vector { fn default() -> Self { Self::from_elem(T::default()).unwrap_or_else(|e| { panic!( @@ -250,7 +249,7 @@ impl Default for Vector { } } -impl tree_hash::TreeHash for Vector { +impl tree_hash::TreeHash for Vector { fn tree_hash_type() -> tree_hash::TreeHashType { tree_hash::TreeHashType::Vector } @@ -273,7 +272,7 @@ impl tree_hash::TreeHash for Vector { impl TryFromIter for Vector where T: Value, - N: Unsigned, + N: ValidN, { type Error = Error; @@ -285,7 +284,7 @@ where } } -impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a Vector { +impl<'a, T: Value, N: ValidN, U: UpdateMap> IntoIterator for &'a Vector { type Item = &'a T; type IntoIter = InterfaceIter<'a, T, U>; @@ -295,7 +294,7 @@ impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a Vector Encode for Vector { +impl Encode for Vector { fn is_ssz_fixed_len() -> bool { ::is_ssz_fixed_len() } @@ -337,7 +336,7 @@ impl Encode for Vector { } } -impl Decode for Vector { +impl Decode for Vector { fn is_ssz_fixed_len() -> bool { ::is_ssz_fixed_len() } From 8b912fc75606244104b078f38af4abaf43b25d62 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 4 Jun 2024 15:03:32 +1000 Subject: [PATCH 2/4] Check capacity when pushing --- src/builder.rs | 16 ++++++++++++++-- src/error.rs | 1 + 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 06f9d03..46d090d 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -4,13 +4,17 @@ use crate::{Arc, Error, PackedLeaf, Tree, Value, MAX_TREE_DEPTH}; #[derive(Debug)] pub struct Builder { stack: Vec>>, + /// The depth of the tree excluding the packing depth. depth: usize, + /// The level (depth) in the tree at which nodes level: usize, length: Length, /// Cached value of `opt_packing_factor`. packing_factor: Option, /// Cached value of `opt_packing_depth`. packing_depth: usize, + /// Cached value of capacity: 2^(depth + packing_depth). + capacity: usize, } impl Builder { @@ -19,6 +23,7 @@ impl Builder { if depth.saturating_add(packing_depth) > MAX_TREE_DEPTH { Err(Error::BuilderInvalidDepth { depth }) } else { + let capacity = 1 << depth + packing_depth; Ok(Self { stack: Vec::with_capacity(depth), depth, @@ -26,11 +31,15 @@ impl Builder { length: Length(0), packing_factor: opt_packing_factor::(), packing_depth, + capacity, }) } } pub fn push(&mut self, value: T) -> Result<(), Error> { + if self.length.as_usize() == self.capacity { + return Err(Error::BuilderFull); + } let index = self.length.as_usize(); let next_index = index + 1; @@ -65,6 +74,10 @@ impl Builder { } pub fn push_node(&mut self, node: Arc>, len: usize) -> Result<(), Error> { + if self.length.as_usize() == self.capacity { + return Err(Error::BuilderFull); + } + let index_on_level = self.length.as_usize() >> self.level; let next_index_on_level = index_on_level + 1; @@ -99,7 +112,6 @@ impl Builder { return Ok((Tree::zero(self.depth), self.depth, Length(0))); } - let capacity = 2usize.pow((self.depth + self.packing_depth) as u32); let length = self.length.as_usize(); let level_capacity = 1 << self.level; let mut next_index_on_level = (length + level_capacity - 1) / level_capacity; @@ -129,7 +141,7 @@ impl Builder { } } - while next_index_on_level << self.level != capacity { + while next_index_on_level << self.level != self.capacity { // Push a new zero padding node on the right of the top-most stack element. let depth = (next_index_on_level.trailing_zeros() as usize) .saturating_add(self.level) diff --git a/src/error.rs b/src/error.rs index bcd8d79..1757bac 100644 --- a/src/error.rs +++ b/src/error.rs @@ -27,6 +27,7 @@ pub enum Error { BuilderStackEmptyFinishRight, BuilderStackEmptyFinalize, BuilderStackLeftover, + BuilderFull, BulkUpdateUnclean, CowMissingEntry, LevelIterPendingUpdates, From 2dc1c8e33c236242df7e413b3e6035050be5e8e5 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Tue, 4 Jun 2024 16:47:23 +1000 Subject: [PATCH 3/4] Remove type-length bound, it's too clunky --- Cargo.toml | 2 +- src/lib.rs | 13 ---------- src/list.rs | 39 +++++++++++++++-------------- src/repeat.rs | 5 ++-- src/serde.rs | 5 ++-- src/tests/proptest/operations.rs | 10 ++++---- src/tests/repeat.rs | 6 ++--- src/vector.rs | 43 ++++++++++++++++---------------- 8 files changed, 57 insertions(+), 66 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 9ad1609..3692bc2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ rayon = "1.5.1" serde = { version = "1.0.0", features = ["derive"] } tree_hash = "0.6.0" triomphe = "0.1.5" -typenum = { version = "1.17.0", features = ["const-generics"] } +typenum = "1.14.0" vec_map = "0.8.2" smallvec = "1.8.0" arbitrary = { version = "1.2.3", features = ["derive"] } diff --git a/src/lib.rs b/src/lib.rs index 6ba49c2..f093028 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,9 +32,6 @@ pub use vector::Vector; use ssz::{Decode, Encode}; use tree_hash::TreeHash; -use typenum::{ - assert_type_eq, generic_const_mappings::U, IsLessOrEqual, Unsigned, B1, U9223372036854775808, -}; /// Maximum depth for a tree. /// @@ -43,16 +40,6 @@ pub const MAX_TREE_DEPTH: usize = u64::BITS as usize - 1; pub const MAX_TREE_LENGTH: usize = 1 << MAX_TREE_DEPTH; -/// Maximum length of lists and vectors. -pub type MaxTreeLength = U9223372036854775808; - -// Consistency check on `MAX_TREE_LENGTH` and `MaxTreeLength`. -assert_type_eq!(MaxTreeLength, U); - -/// Trait to assert the bounds on list and vector lengths -pub trait ValidN: Unsigned + IsLessOrEqual {} -impl> ValidN for N {} - #[cfg(feature = "debug")] pub trait Value: Encode + Decode + TreeHash + PartialEq + Clone + std::fmt::Debug {} diff --git a/src/list.rs b/src/list.rs index f8f6b38..bb20dc6 100644 --- a/src/list.rs +++ b/src/list.rs @@ -7,7 +7,7 @@ use crate::serde::ListVisitor; use crate::tree::RebaseAction; use crate::update_map::MaxMap; use crate::utils::{arb_arc, compute_level, int_log, opt_packing_depth, updated_length, Length}; -use crate::{Arc, Cow, Error, Tree, UpdateMap, ValidN, Value}; +use crate::{Arc, Cow, Error, Tree, UpdateMap, Value}; use arbitrary::Arbitrary; use derivative::Derivative; use itertools::process_results; @@ -16,20 +16,21 @@ use ssz::{Decode, Encode, SszEncoder, TryFromIter, BYTES_PER_LENGTH_OFFSET}; use std::collections::BTreeMap; use std::marker::PhantomData; use tree_hash::{Hash256, PackedEncoding, TreeHash}; +use typenum::Unsigned; use vec_map::VecMap; #[derive(Debug, Clone, Derivative, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: ValidN, U: UpdateMap + PartialEq"))] +#[derivative(PartialEq(bound = "T: Value, N: Unsigned, U: UpdateMap + PartialEq"))] #[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value")] -#[arbitrary(bound = "N: ValidN, U: Arbitrary<'arbitrary> + UpdateMap + PartialEq")] -pub struct List = MaxMap>> { +#[arbitrary(bound = "N: Unsigned, U: Arbitrary<'arbitrary> + UpdateMap + PartialEq")] +pub struct List = MaxMap>> { pub(crate) interface: Interface, U>, } #[derive(Debug, Clone, Derivative, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: ValidN"))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: ValidN")] -pub struct ListInner { +#[derivative(PartialEq(bound = "T: Value, N: Unsigned"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: Unsigned")] +pub struct ListInner { #[arbitrary(with = arb_arc)] pub(crate) tree: Arc>, pub(crate) length: Length, @@ -39,7 +40,7 @@ pub struct ListInner { _phantom: PhantomData, } -impl> List { +impl> List { pub fn new(vec: Vec) -> Result { Self::try_from_iter(vec) } @@ -231,7 +232,7 @@ impl> List { } } -impl ImmList for ListInner { +impl ImmList for ListInner { fn get(&self, index: usize) -> Option<&T> { if index < self.len().as_usize() { self.tree @@ -257,7 +258,7 @@ impl ImmList for ListInner { impl MutList for ListInner where T: Value, - N: ValidN, + N: Unsigned, { fn validate_push(current_len: usize) -> Result<(), Error> { if current_len == N::to_usize() { @@ -303,7 +304,7 @@ where } } -impl> List { +impl> List { pub fn rebase(&self, base: &Self) -> Result { let mut rebased = self.clone(); rebased.rebase_on(base)?; @@ -329,13 +330,13 @@ impl> List { } } -impl Default for List { +impl Default for List { fn default() -> Self { Self::empty() } } -impl TreeHash for List { +impl TreeHash for List { fn tree_hash_type() -> tree_hash::TreeHashType { tree_hash::TreeHashType::List } @@ -357,7 +358,7 @@ impl TreeHash for List { } } -impl<'a, T: Value, N: ValidN, U: UpdateMap> IntoIterator for &'a List { +impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a List { type Item = &'a T; type IntoIter = InterfaceIter<'a, T, U>; @@ -366,7 +367,7 @@ impl<'a, T: Value, N: ValidN, U: UpdateMap> IntoIterator for &'a List> Serialize for List +impl> Serialize for List where T: Serialize, { @@ -385,7 +386,7 @@ where impl<'de, T, N, U> Deserialize<'de> for List where T: Deserialize<'de> + Value, - N: ValidN, + N: Unsigned, U: UpdateMap, { fn deserialize(deserializer: D) -> Result @@ -397,7 +398,7 @@ where } // FIXME: duplicated from `ssz::encode::impl_for_vec` -impl Encode for List { +impl Encode for List { fn is_ssz_fixed_len() -> bool { false } @@ -434,7 +435,7 @@ impl Encode for List { impl TryFromIter for List where T: Value, - N: ValidN, + N: Unsigned, { type Error = Error; @@ -449,7 +450,7 @@ where impl Decode for List where T: Value, - N: ValidN, + N: Unsigned, { fn is_ssz_fixed_len() -> bool { false diff --git a/src/repeat.rs b/src/repeat.rs index e7a2926..e6a70f3 100644 --- a/src/repeat.rs +++ b/src/repeat.rs @@ -1,13 +1,14 @@ use crate::utils::{opt_packing_factor, Length}; -use crate::{Arc, Error, Leaf, List, PackedLeaf, Tree, UpdateMap, ValidN, Value}; +use crate::{Arc, Error, Leaf, List, PackedLeaf, Tree, UpdateMap, Value}; use smallvec::{smallvec, SmallVec}; use tree_hash::Hash256; +use typenum::Unsigned; /// Efficiently construct a list from `n` copies of `elem`. pub fn repeat_list(elem: T, n: usize) -> Result, Error> where T: Value, - N: ValidN, + N: Unsigned, U: UpdateMap, { if n == 0 { diff --git a/src/serde.rs b/src/serde.rs index 4065834..282896f 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -1,7 +1,8 @@ -use crate::{List, UpdateMap, ValidN, Value}; +use crate::{List, UpdateMap, Value}; use itertools::process_results; use serde::Deserialize; use std::marker::PhantomData; +use typenum::Unsigned; pub struct ListVisitor { _phantom: PhantomData<(T, N, U)>, @@ -18,7 +19,7 @@ impl Default for ListVisitor { impl<'a, T, N, U> serde::de::Visitor<'a> for ListVisitor where T: Deserialize<'a> + Value, - N: ValidN, + N: Unsigned, U: UpdateMap, { type Value = List; diff --git a/src/tests/proptest/operations.rs b/src/tests/proptest/operations.rs index 9fa9629..f24504f 100644 --- a/src/tests/proptest/operations.rs +++ b/src/tests/proptest/operations.rs @@ -1,5 +1,5 @@ use super::{arb_hash256, arb_index, arb_large, arb_list, arb_vect, Large}; -use crate::{Error, List, ValidN, Value, Vector}; +use crate::{Error, List, Value, Vector}; use proptest::prelude::*; use ssz::{Decode, Encode}; use std::fmt::Debug; @@ -11,13 +11,13 @@ const OP_LIMIT: usize = 128; /// Simple specification for `List` and `Vector` behaviour. #[derive(Debug, Clone)] -pub struct Spec { +pub struct Spec { values: Vec, allow_push: bool, _phantom: PhantomData, } -impl Spec { +impl Spec { pub fn list(values: Vec) -> Self { assert!(values.len() <= N::to_usize()); Self { @@ -173,7 +173,7 @@ where fn apply_ops_list(list: &mut List, spec: &mut Spec, ops: Vec>) where T: Value + Debug + Send + Sync, - N: ValidN + Debug, + N: Unsigned + Debug, { let mut checkpoint = list.clone(); @@ -259,7 +259,7 @@ where fn apply_ops_vect(vect: &mut Vector, spec: &mut Spec, ops: Vec>) where T: Value + Debug + Send + Sync, - N: ValidN + Debug, + N: Unsigned + Debug, { let mut checkpoint = vect.clone(); diff --git a/src/tests/repeat.rs b/src/tests/repeat.rs index 51c8f4a..552041f 100644 --- a/src/tests/repeat.rs +++ b/src/tests/repeat.rs @@ -1,9 +1,9 @@ -use crate::{List, ValidN, Value}; +use crate::{List, Value}; use std::fmt::Debug; use tree_hash::TreeHash; -use typenum::{U1024, U64, U8}; +use typenum::{Unsigned, U1024, U64, U8}; -fn list_test(val: T) { +fn list_test(val: T) { for n in 96..=N::to_usize() { let fast = List::::repeat(val.clone(), n).unwrap(); let slow = List::::repeat_slow(val.clone(), n).unwrap(); diff --git a/src/vector.rs b/src/vector.rs index 6f36e3c..f5b6711 100644 --- a/src/vector.rs +++ b/src/vector.rs @@ -5,7 +5,7 @@ use crate::level_iter::LevelIter; use crate::tree::RebaseAction; use crate::update_map::MaxMap; use crate::utils::{arb_arc, Length}; -use crate::{Arc, Cow, Error, List, Tree, UpdateMap, ValidN, Value}; +use crate::{Arc, Cow, Error, List, Tree, UpdateMap, Value}; use arbitrary::Arbitrary; use derivative::Derivative; use serde::{Deserialize, Serialize}; @@ -14,24 +14,25 @@ use std::collections::BTreeMap; use std::convert::TryFrom; use std::marker::PhantomData; use tree_hash::{Hash256, PackedEncoding}; +use typenum::Unsigned; use vec_map::VecMap; #[derive(Debug, Derivative, Clone, Serialize, Deserialize, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: ValidN, U: UpdateMap + PartialEq"))] +#[derivative(PartialEq(bound = "T: Value, N: Unsigned, U: UpdateMap + PartialEq"))] #[serde(try_from = "List")] #[serde(into = "List")] -#[serde(bound(serialize = "T: Value + Serialize, N: ValidN, U: UpdateMap"))] -#[serde(bound(deserialize = "T: Value + Deserialize<'de>, N: ValidN, U: UpdateMap"))] +#[serde(bound(serialize = "T: Value + Serialize, N: Unsigned, U: UpdateMap"))] +#[serde(bound(deserialize = "T: Value + Deserialize<'de>, N: Unsigned, U: UpdateMap"))] #[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value")] -#[arbitrary(bound = "N: ValidN, U: Arbitrary<'arbitrary> + UpdateMap")] -pub struct Vector = MaxMap>> { +#[arbitrary(bound = "N: Unsigned, U: Arbitrary<'arbitrary> + UpdateMap")] +pub struct Vector = MaxMap>> { pub(crate) interface: Interface, U>, } #[derive(Debug, Derivative, Clone, Arbitrary)] -#[derivative(PartialEq(bound = "T: Value, N: ValidN"))] -#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: ValidN")] -pub struct VectorInner { +#[derivative(PartialEq(bound = "T: Value, N: Unsigned"))] +#[arbitrary(bound = "T: Arbitrary<'arbitrary> + Value, N: Unsigned")] +pub struct VectorInner { #[arbitrary(with = arb_arc)] pub(crate) tree: Arc>, pub(crate) depth: usize, @@ -40,7 +41,7 @@ pub struct VectorInner { _phantom: PhantomData, } -impl> Vector { +impl> Vector { pub fn new(vec: Vec) -> Result { if vec.len() == N::to_usize() { Self::try_from(List::new(vec)?) @@ -108,7 +109,7 @@ impl> Vector { } } -impl> TryFrom> for Vector { +impl> TryFrom> for Vector { type Error = Error; fn try_from(list: List) -> Result { @@ -136,7 +137,7 @@ impl> TryFrom> for Vector> Vector { +impl> Vector { pub fn rebase(&self, base: &Self) -> Result { let mut rebased = self.clone(); rebased.rebase_on(base)?; @@ -162,7 +163,7 @@ impl> Vector { } } -impl> From> for List { +impl> From> for List { fn from(vector: Vector) -> Self { let mut list = List::from_parts( vector.interface.backing.tree, @@ -174,7 +175,7 @@ impl> From> for List ImmList for VectorInner { +impl ImmList for VectorInner { fn get(&self, index: usize) -> Option<&T> { if index < self.len().as_usize() { self.tree @@ -200,7 +201,7 @@ impl ImmList for VectorInner { impl MutList for VectorInner where T: Value, - N: ValidN, + N: Unsigned, { fn validate_push(_current_len: usize) -> Result<(), Error> { Err(Error::PushNotSupported) @@ -237,7 +238,7 @@ where } } -impl Default for Vector { +impl Default for Vector { fn default() -> Self { Self::from_elem(T::default()).unwrap_or_else(|e| { panic!( @@ -249,7 +250,7 @@ impl Default for Vector { } } -impl tree_hash::TreeHash for Vector { +impl tree_hash::TreeHash for Vector { fn tree_hash_type() -> tree_hash::TreeHashType { tree_hash::TreeHashType::Vector } @@ -272,7 +273,7 @@ impl tree_hash::TreeHash for Vector { impl TryFromIter for Vector where T: Value, - N: ValidN, + N: Unsigned, { type Error = Error; @@ -284,7 +285,7 @@ where } } -impl<'a, T: Value, N: ValidN, U: UpdateMap> IntoIterator for &'a Vector { +impl<'a, T: Value, N: Unsigned, U: UpdateMap> IntoIterator for &'a Vector { type Item = &'a T; type IntoIter = InterfaceIter<'a, T, U>; @@ -294,7 +295,7 @@ impl<'a, T: Value, N: ValidN, U: UpdateMap> IntoIterator for &'a Vector Encode for Vector { +impl Encode for Vector { fn is_ssz_fixed_len() -> bool { ::is_ssz_fixed_len() } @@ -336,7 +337,7 @@ impl Encode for Vector { } } -impl Decode for Vector { +impl Decode for Vector { fn is_ssz_fixed_len() -> bool { ::is_ssz_fixed_len() } From 1a9e28e571c1dac22fd274c3b473974281d1a282 Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Fri, 7 Jun 2024 12:22:04 +1000 Subject: [PATCH 4/4] Satisfy clippy --- src/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/builder.rs b/src/builder.rs index 46d090d..61741a0 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -23,7 +23,7 @@ impl Builder { if depth.saturating_add(packing_depth) > MAX_TREE_DEPTH { Err(Error::BuilderInvalidDepth { depth }) } else { - let capacity = 1 << depth + packing_depth; + let capacity = 1 << (depth + packing_depth); Ok(Self { stack: Vec::with_capacity(depth), depth,