From fac65fe8cfd2d00f9adf34819db68c7d681d55ae Mon Sep 17 00:00:00 2001 From: Jeremy Wall Date: Wed, 24 Aug 2022 10:00:08 -0400 Subject: [PATCH] Get rid of the const generics --- src/blake2.rs | 16 ++--- src/dag.rs | 54 +++++++------- src/hash.rs | 10 +-- src/leveldb/mod.rs | 8 +-- src/node.rs | 172 +++++---------------------------------------- src/proptest.rs | 8 +-- src/store.rs | 22 +++--- src/test.rs | 44 ++++++++---- 8 files changed, 108 insertions(+), 226 deletions(-) diff --git a/src/blake2.rs b/src/blake2.rs index 63a43df..063b176 100644 --- a/src/blake2.rs +++ b/src/blake2.rs @@ -16,24 +16,24 @@ use blake2::digest::Digest; pub use blake2::{Blake2b512, Blake2s256}; macro_rules! hash_writer_impl { - ($tname:ident, $size:expr) => { - impl HashWriter<$size> for $tname { + ($tname:ident) => { + impl HashWriter for $tname { fn record>(&mut self, bs: I) { let vec: Vec = bs.collect(); self.update(&vec); } - fn hash(&self) -> [u8; $size] { - let mut out: [u8; $size] = Default::default(); + fn hash(&self) -> Vec { + let mut out = Vec::new(); // This is gross but Blake2 doesn't support the // non consuming version of this. - let mut arr = self.clone().finalize(); - arr.swap_with_slice(&mut out); + let arr = self.clone().finalize(); + out.extend(arr); out } } }; } -hash_writer_impl!(Blake2b512, 8); -hash_writer_impl!(Blake2s256, 4); +hash_writer_impl!(Blake2b512); +hash_writer_impl!(Blake2s256); diff --git a/src/dag.rs b/src/dag.rs index d700b6c..93115e4 100644 --- a/src/dag.rs +++ b/src/dag.rs @@ -44,20 +44,20 @@ pub enum NodeCompare { /// A merkle DAG instance is tied to a specific implementation of the HashWriter interface to ensure /// that all hash identifiers are of the same hash algorithm. #[derive(Clone, Debug)] -pub struct Merkle +pub struct Merkle where - HW: HashWriter, - S: Store, + HW: HashWriter, + S: Store, { - roots: BTreeSet<[u8; HASH_LEN]>, + roots: BTreeSet>, nodes: S, - _phantom_node: PhantomData>, + _phantom_node: PhantomData>, } -impl Merkle +impl Merkle where - HW: HashWriter, - S: Store, + HW: HashWriter, + S: Store, { /// Construct a new empty DAG. The empty DAG is also the default for a DAG. pub fn new() -> Self { @@ -73,13 +73,19 @@ where pub fn add_node<'a, N: Into>>( &'a mut self, item: N, - dependency_ids: BTreeSet<[u8; HASH_LEN]>, - ) -> Result<[u8; HASH_LEN]> { - let node = Node::::new(item.into(), dependency_ids.clone()); - let id = node.id().clone(); - if self.nodes.contains(&id)? { + dependency_ids: BTreeSet>, + ) -> Result> { + let node = Node::::new(item.into(), dependency_ids.clone()); + let id = node.id().to_vec(); + if self.nodes.contains(id.as_slice())? { // We've already added this node so there is nothing left to do. - return Ok(id); + return Ok(self + .nodes + .get(id.as_slice()) + .unwrap() + .unwrap() + .id() + .to_vec()); } let mut root_removals = Vec::new(); for dep_id in dependency_ids.iter() { @@ -96,22 +102,22 @@ where for removal in root_removals { self.roots.remove(removal); } - self.roots.insert(id); - Ok(id) + self.roots.insert(id.to_vec()); + Ok(id.to_vec()) } /// Check if we already have a copy of a node. - pub fn check_for_node(&self, id: &[u8; HASH_LEN]) -> Result { + pub fn check_for_node(&self, id: &[u8]) -> Result { return self.nodes.contains(id); } /// Get a node from the DAG by it's hash identifier if it exists. - pub fn get_node_by_id(&self, id: &[u8; HASH_LEN]) -> Result>> { + pub fn get_node_by_id(&self, id: &[u8]) -> Result>> { self.nodes.get(id) } /// Get the set of root node ids. - pub fn get_roots(&self) -> &BTreeSet<[u8; HASH_LEN]> { + pub fn get_roots(&self) -> &BTreeSet> { &self.roots } @@ -125,7 +131,7 @@ where /// then returns `NodeCompare::After`. If both id's are equal then the returns /// `NodeCompare::Equivalent`. If neither id are parts of the same subgraph then returns /// `NodeCompare::Uncomparable`. - pub fn compare(&self, left: &[u8; HASH_LEN], right: &[u8; HASH_LEN]) -> Result { + pub fn compare(&self, left: &[u8], right: &[u8]) -> Result { Ok(if left == right { NodeCompare::Equivalent } else { @@ -141,7 +147,7 @@ where }) } - fn search_graph(&self, root_id: &[u8; HASH_LEN], search_id: &[u8; HASH_LEN]) -> Result { + fn search_graph(&self, root_id: &[u8], search_id: &[u8]) -> Result { if root_id == search_id { return Ok(true); } @@ -169,10 +175,10 @@ where } } -impl Default for Merkle +impl Default for Merkle where - HW: HashWriter, - S: Store, + HW: HashWriter, + S: Store, { fn default() -> Self { Self { diff --git a/src/hash.rs b/src/hash.rs index a497b46..7f073de 100644 --- a/src/hash.rs +++ b/src/hash.rs @@ -17,21 +17,21 @@ use std::hash::Hasher; /// Utility Trait to specify the hashing algorithm and provide a common /// interface for that algorithm to provide. This interface is expected to /// be stateful. -pub trait HashWriter: Default { +pub trait HashWriter: Default { /// Record bytes from an iterator into our hash algorithm. fn record>(&mut self, bs: I); /// Provide the current hash value based on the bytes that have so far been recorded. - fn hash(&self) -> [u8; LEN]; + fn hash(&self) -> Vec; } -impl HashWriter<8> for DefaultHasher { +impl HashWriter for DefaultHasher { fn record>(&mut self, iter: I) { let bytes = iter.collect::>(); self.write(bytes.as_slice()); } - fn hash(&self) -> [u8; 8] { - self.finish().to_le_bytes() + fn hash(&self) -> Vec { + self.finish().to_le_bytes().to_vec() } } diff --git a/src/leveldb/mod.rs b/src/leveldb/mod.rs index 23a740a..b4656b3 100644 --- a/src/leveldb/mod.rs +++ b/src/leveldb/mod.rs @@ -37,12 +37,12 @@ impl LevelStore { }) } } -impl Store for LevelStore { - fn contains(&self, id: &[u8; 8]) -> Result { +impl Store for LevelStore { + fn contains(&self, id: &[u8]) -> Result { Ok(self.store.borrow_mut().get(id).is_some()) } - fn get(&self, id: &[u8; 8]) -> Result>> { + fn get(&self, id: &[u8]) -> Result>> { Ok(match self.store.borrow_mut().get(id) { Some(bs) => ciborium::de::from_reader(bs.as_slice()) .map_err(|e| StoreError::StoreFailure(format!("Invalid serialization {:?}", e)))?, @@ -50,7 +50,7 @@ impl Store for LevelStore { }) } - fn store(&mut self, node: Node) -> Result<()> { + fn store(&mut self, node: Node) -> Result<()> { let mut buf = Vec::new(); ciborium::ser::into_writer(&node, &mut buf).unwrap(); self.store.borrow_mut().put(node.id(), &buf)?; diff --git a/src/node.rs b/src/node.rs index 7291890..5d6a760 100644 --- a/src/node.rs +++ b/src/node.rs @@ -13,7 +13,7 @@ // limitations under the License. use std::{collections::BTreeSet, marker::PhantomData}; -use serde::{de::Visitor, ser::SerializeStruct, Deserialize, Deserializer, Serialize, Serializer}; +use serde::{Deserialize, Serialize}; use crate::hash::HashWriter; @@ -27,21 +27,21 @@ use crate::hash::HashWriter; /// Nodes are tied to a specific implementation of the HashWriter trait which is itself tied /// to the DAG they are stored in guaranteeing that the same Hashing implementation is used /// for each node in the DAG. -#[derive(Debug, PartialEq, Eq)] -pub struct Node +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct Node where - HW: HashWriter, + HW: HashWriter, { - id: [u8; HASH_LEN], + id: Vec, item: Vec, - item_id: [u8; HASH_LEN], - dependency_ids: BTreeSet<[u8; HASH_LEN]>, + item_id: Vec, + dependency_ids: BTreeSet>, _phantom: PhantomData, } -impl Clone for Node +impl Clone for Node where - HW: HashWriter, + HW: HashWriter, { fn clone(&self) -> Self { Self { @@ -54,149 +54,12 @@ where } } -fn coerce_non_const_generic_set( - set: &BTreeSet<[u8; HASH_LEN]>, -) -> BTreeSet<&[u8]> { - let mut coerced_item = BTreeSet::new(); - for arr in set { - coerced_item.insert(arr.as_slice()); - } - coerced_item -} - -impl Serialize for Node +impl Node where - HW: HashWriter, -{ - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let mut structor = serializer.serialize_struct("Node", 4)?; - structor.serialize_field("item", &self.item)?; - structor.serialize_field( - "dependency_ids", - &coerce_non_const_generic_set(&self.dependency_ids), - )?; - structor.end() - } -} - -fn coerce_const_generic_array( - slice: &[u8], -) -> Result<[u8; HASH_LEN], String> { - let mut coerced_item: [u8; HASH_LEN] = [0; HASH_LEN]; - if slice.len() > coerced_item.len() { - return Err(format!( - "Expected slice of length: {} but got slice of length: {}", - coerced_item.len(), - slice.len() - )); - } else { - coerced_item.copy_from_slice(slice); - } - Ok(coerced_item) -} - -fn coerce_const_generic_set( - set: BTreeSet<&[u8]>, -) -> Result, String> { - let mut coerced_item = BTreeSet::new(); - for slice in set { - coerced_item.insert(coerce_const_generic_array(slice)?); - } - Ok(coerced_item) -} - -impl<'de, HW, const HASH_LEN: usize> Deserialize<'de> for Node -where - HW: HashWriter, -{ - fn deserialize(deserializer: D) -> Result - where - D: Deserializer<'de>, - { - #[derive(Deserialize)] - #[serde(field_identifier, rename_all = "lowercase")] - #[allow(non_camel_case_types)] - enum Field { - Item, - Dependency_Ids, - } - - struct NodeVisitor(PhantomData); - - impl<'de, HW, const HASH_LEN: usize> Visitor<'de> for NodeVisitor - where - HW: HashWriter, - { - type Value = Node; - - fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { - formatter.write_str("struct Node") - } - - fn visit_seq(self, mut seq: A) -> Result - where - A: serde::de::SeqAccess<'de>, - { - let item = seq - .next_element::>()? - .ok_or_else(|| serde::de::Error::invalid_length(1, &self))?; - let dependency_ids: BTreeSet<[u8; HASH_LEN]> = coerce_const_generic_set( - seq.next_element::>()? - .ok_or_else(|| serde::de::Error::invalid_length(3, &self))?, - ) - .map_err(serde::de::Error::custom)?; - Ok(Self::Value::new(item, dependency_ids)) - } - - fn visit_map(self, mut map: A) -> Result - where - A: serde::de::MapAccess<'de>, - { - let mut item: Option> = None; - let mut dependency_ids: Option> = None; - while let Some(key) = map.next_key()? { - match key { - Field::Item => { - if item.is_some() { - return Err(serde::de::Error::duplicate_field("item")); - } else { - item = Some(map.next_value()?); - } - } - Field::Dependency_Ids => { - if dependency_ids.is_some() { - return Err(serde::de::Error::duplicate_field("dependency_ids")); - } else { - dependency_ids = Some( - coerce_const_generic_set(map.next_value()?) - .map_err(serde::de::Error::custom)?, - ); - } - } - } - } - let item = item.ok_or_else(|| serde::de::Error::missing_field("item"))?; - let dependency_ids = dependency_ids - .ok_or_else(|| serde::de::Error::missing_field("dependency_ids"))?; - - Ok(Self::Value::new(item, dependency_ids)) - } - } - - const FIELDS: &'static [&'static str] = &["item", "dependency_ids"]; - deserializer.deserialize_struct("Node", FIELDS, NodeVisitor::(PhantomData)) - } -} - -impl Node -where - HW: HashWriter, + HW: HashWriter, { /// Construct a new node with a payload and a set of dependency_ids. - pub fn new>>(item: P, dependency_ids: BTreeSet<[u8; HASH_LEN]>) -> Self { + pub fn new>>(item: P, dependency_ids: BTreeSet>) -> Self { let mut hw = HW::default(); let item = item.into(); // NOTE(jwall): The order here is important. Our reliable id creation must be stable @@ -205,10 +68,7 @@ where hw.record(item.iter().cloned()); let item_id = hw.hash(); // 2. Sort the dependency ids before recording them into our node id hash. - let mut dependency_list = dependency_ids - .iter() - .cloned() - .collect::>(); + let mut dependency_list = dependency_ids.iter().cloned().collect::>>(); dependency_list.sort(); // 3. record the dependency ids into our node id hash in the sorted order. for d in dependency_list.iter() { @@ -223,7 +83,7 @@ where } } - pub fn id(&self) -> &[u8; HASH_LEN] { + pub fn id(&self) -> &[u8] { &self.id } @@ -231,11 +91,11 @@ where &self.item } - pub fn item_id(&self) -> &[u8; HASH_LEN] { + pub fn item_id(&self) -> &[u8] { &self.item_id } - pub fn dependency_ids(&self) -> &BTreeSet<[u8; HASH_LEN]> { + pub fn dependency_ids(&self) -> &BTreeSet> { &self.dependency_ids } } diff --git a/src/proptest.rs b/src/proptest.rs index add11c9..e2a0859 100644 --- a/src/proptest.rs +++ b/src/proptest.rs @@ -17,7 +17,7 @@ use proptest::prelude::*; use crate::prelude::*; -type TestDag = Merkle>, DefaultHasher, 8>; +type TestDag = Merkle, Node>, DefaultHasher>; fn simple_edge_strategy( nodes_count: usize, @@ -42,10 +42,10 @@ fn complex_dag_strategy( let nodes_len = payloads.len(); let mut dag = TestDag::new(); // partition the payloads into depth pieces - let mut id_stack: Vec<[u8; 8]> = Vec::new(); + let mut id_stack: Vec> = Vec::new(); for chunk in payloads.chunks(nodes_len / depth) { // loop through the partions adding each partions nodes to the dag. - let dep_sets: Vec> = if id_stack.is_empty() { + let dep_sets: Vec>> = if id_stack.is_empty() { vec![BTreeSet::new()] } else { let mut dep_sets = Vec::new(); @@ -83,7 +83,7 @@ proptest! { node_set.insert(node_id.clone()); let parent = idx % parent_count; if dependents.contains_key(&parent) { - dependents.get_mut(&parent).map(|v: &mut BTreeSet<[u8; 8]>| v.insert(node_id)); + dependents.get_mut(&parent).map(|v: &mut BTreeSet>| v.insert(node_id)); } else { dependents.insert(parent, BTreeSet::from([node_id])); } diff --git a/src/store.rs b/src/store.rs index 3aea7de..df841ba 100644 --- a/src/store.rs +++ b/src/store.rs @@ -23,29 +23,29 @@ pub enum StoreError { NoSuchDependents, } -pub trait Store: Default +pub trait Store: Default where - HW: HashWriter, + HW: HashWriter, { - fn contains(&self, id: &[u8; HASH_LEN]) -> Result; - fn get(&self, id: &[u8; HASH_LEN]) -> Result>>; - fn store(&mut self, node: Node) -> Result<()>; + fn contains(&self, id: &[u8]) -> Result; + fn get(&self, id: &[u8]) -> Result>>; + fn store(&mut self, node: Node) -> Result<()>; } -impl Store for BTreeMap<[u8; HASH_LEN], Node> +impl Store for BTreeMap, Node> where - HW: HashWriter, + HW: HashWriter, { - fn contains(&self, id: &[u8; HASH_LEN]) -> Result { + fn contains(&self, id: &[u8]) -> Result { Ok(self.contains_key(id)) } - fn get(&self, id: &[u8; HASH_LEN]) -> Result>> { + fn get(&self, id: &[u8]) -> Result>> { Ok(self.get(id).cloned()) } - fn store(&mut self, node: Node) -> Result<()> { - self.insert(node.id().clone(), node); + fn store(&mut self, node: Node) -> Result<()> { + self.insert(node.id().to_vec(), node); Ok(()) } } diff --git a/src/test.rs b/src/test.rs index 143ade3..0721918 100644 --- a/src/test.rs +++ b/src/test.rs @@ -17,9 +17,8 @@ use std::collections::{BTreeMap, BTreeSet}; use crate::prelude::*; type TestDag<'a> = Merkle< - BTreeMap<[u8; 8], Node>, + BTreeMap, Node>, std::collections::hash_map::DefaultHasher, - 8, >; #[test] @@ -32,7 +31,7 @@ fn test_root_pointer_hygiene() { ); assert!(dag.get_roots().contains(&quax_node_id)); let mut dep_set = BTreeSet::new(); - dep_set.insert(quax_node_id); + dep_set.insert(quax_node_id.clone()); let quux_node_id = dag.add_node("quux", dep_set).unwrap(); assert!(!dag.get_roots().contains(&quax_node_id)); assert!(dag.get_roots().contains(&quux_node_id)); @@ -45,10 +44,10 @@ fn test_root_pointer_hygiene() { #[test] fn test_insert_no_such_dependents_error() { let missing_dependent = - Node::::new("missing".as_bytes().to_vec(), BTreeSet::new()); + Node::::new("missing".as_bytes().to_vec(), BTreeSet::new()); let mut dag = TestDag::new(); let mut dep_set = BTreeSet::new(); - dep_set.insert(*missing_dependent.id()); + dep_set.insert(missing_dependent.id().to_vec()); assert!(dag.add_node("foo", dep_set).is_err()); assert!(dag.get_roots().is_empty()); assert!(dag.get_nodes().is_empty()); @@ -76,17 +75,29 @@ fn test_adding_nodes_is_idempotent_regardless_of_dep_order() { let quake_node_id = dag.add_node("quake", BTreeSet::new()).unwrap(); let qualm_node_id = dag.add_node("qualm", BTreeSet::new()).unwrap(); let quell_node_id = dag.add_node("quell", BTreeSet::new()).unwrap(); - let dep_ids = BTreeSet::from([quake_node_id, qualm_node_id, quell_node_id]); + let dep_ids = BTreeSet::from([ + quake_node_id.clone(), + qualm_node_id.clone(), + quell_node_id.clone(), + ]); dag.add_node("foo", dep_ids).unwrap(); let root_size = dag.get_roots().len(); let nodes_size = dag.get_nodes().len(); - let dep_ids = BTreeSet::from([quell_node_id, quake_node_id, qualm_node_id]); + let dep_ids = BTreeSet::from([ + quell_node_id.clone(), + quake_node_id.clone(), + qualm_node_id.clone(), + ]); dag.add_node("foo", dep_ids).unwrap(); assert_eq!(root_size, dag.get_roots().len()); assert_eq!(nodes_size, dag.get_nodes().len()); - let dep_ids = BTreeSet::from([qualm_node_id, quell_node_id, quake_node_id]); + let dep_ids = BTreeSet::from([ + qualm_node_id.clone(), + quell_node_id.clone(), + quake_node_id.clone(), + ]); dag.add_node("foo", dep_ids).unwrap(); assert_eq!(root_size, dag.get_roots().len()); assert_eq!(nodes_size, dag.get_nodes().len()); @@ -174,20 +185,25 @@ mod cbor_serialization_tests { let mut dag = TestDag::new(); let simple_node_id = dag.add_node("simple", BTreeSet::new()).unwrap(); let mut dep_set = BTreeSet::new(); - dep_set.insert(simple_node_id); + dep_set.insert(simple_node_id.clone()); let root_node_id = dag.add_node("root", dep_set).unwrap(); - let simple_node_to_serialize = dag.get_node_by_id(&simple_node_id).unwrap().unwrap(); - let root_node_to_serialize = dag.get_node_by_id(&root_node_id).unwrap().unwrap(); + let simple_node_to_serialize = dag + .get_node_by_id(simple_node_id.as_slice()) + .unwrap() + .unwrap(); + let root_node_to_serialize = dag + .get_node_by_id(root_node_id.as_slice()) + .unwrap() + .unwrap(); let mut simple_node_vec: Vec = Vec::new(); let mut root_node_vec: Vec = Vec::new(); into_writer(&simple_node_to_serialize, &mut simple_node_vec).unwrap(); into_writer(&root_node_to_serialize, &mut root_node_vec).unwrap(); - let simple_node_de: Node = - from_reader(simple_node_vec.as_slice()).unwrap(); - let root_node_de: Node = from_reader(root_node_vec.as_slice()).unwrap(); + let simple_node_de: Node = from_reader(simple_node_vec.as_slice()).unwrap(); + let root_node_de: Node = from_reader(root_node_vec.as_slice()).unwrap(); assert_eq!(simple_node_to_serialize.id(), simple_node_de.id()); assert_eq!(simple_node_to_serialize.item_id(), simple_node_de.item_id()); assert_eq!(simple_node_to_serialize.item(), simple_node_de.item());