diff --git a/eth2/utils/ssz/src/cached_tree_hash.rs b/eth2/utils/ssz/src/cached_tree_hash.rs index e7f2114e4..0889718a2 100644 --- a/eth2/utils/ssz/src/cached_tree_hash.rs +++ b/eth2/utils/ssz/src/cached_tree_hash.rs @@ -1,4 +1,5 @@ use hashing::hash; +use std::fmt::Debug; use std::iter::IntoIterator; use std::iter::Iterator; use std::ops::Range; @@ -29,9 +30,8 @@ pub enum ItemType { Composite, } -pub trait CachedTreeHash { - type Item: CachedTreeHash; - +// TODO: remove debug requirement. +pub trait CachedTreeHash: Debug { fn item_type() -> ItemType; fn build_tree_hash_cache(&self) -> Result; @@ -50,7 +50,7 @@ pub trait CachedTreeHash { fn cached_hash_tree_root( &self, - other: &Self::Item, + other: &Item, cache: &mut TreeHashCache, chunk: usize, ) -> Result; @@ -71,7 +71,7 @@ impl Into> for TreeHashCache { impl TreeHashCache { pub fn new(item: &T) -> Result where - T: CachedTreeHash, + T: CachedTreeHash, { item.build_tree_hash_cache() } @@ -81,7 +81,7 @@ impl TreeHashCache { leaves_and_subtrees: Vec, ) -> Result where - T: CachedTreeHash, + T: CachedTreeHash, { let offset_handler = OffsetHandler::new(item, 0)?; @@ -114,7 +114,6 @@ impl TreeHashCache { // Merkleize the leaves, then split the leaf nodes off them. Then, replace all-zeros // internal nodes created earlier with the internal nodes generated by `merkleize`. let mut merkleized = merkleize(leaves); - dbg!(&merkleized); merkleized.split_off(internal_node_bytes); cache.splice(0..internal_node_bytes, merkleized); @@ -207,16 +206,6 @@ impl TreeHashCache { == other) } - pub fn set_changed(&mut self, chunk: usize, to: bool) -> Result<(), Error> { - if chunk < self.chunk_modified.len() { - self.chunk_modified[chunk] = to; - - Ok(()) - } else { - Err(Error::NoModifiedFieldForChunk(chunk)) - } - } - pub fn changed(&self, chunk: usize) -> Result { self.chunk_modified .get(chunk) @@ -256,7 +245,7 @@ fn num_nodes(num_leaves: usize) -> usize { #[derive(Debug)] pub struct OffsetHandler { num_internal_nodes: usize, - num_leaf_nodes: usize, + pub num_leaf_nodes: usize, next_node: usize, offsets: Vec, } @@ -264,7 +253,7 @@ pub struct OffsetHandler { impl OffsetHandler { pub fn new(item: &T, initial_offset: usize) -> Result where - T: CachedTreeHash, + T: CachedTreeHash, { Self::from_lengths(initial_offset, item.offsets()?) } @@ -314,6 +303,8 @@ impl OffsetHandler { self.next_node } + /// Returns an iterator visiting each internal node, providing the left and right child chunks + /// for the node. pub fn iter_internal_nodes<'a>( &'a self, ) -> impl DoubleEndedIterator { @@ -328,6 +319,7 @@ impl OffsetHandler { }) } + /// Returns an iterator visiting each leaf node, providing the chunk for that node. pub fn iter_leaf_nodes<'a>(&'a self) -> impl DoubleEndedIterator { let leaf_nodes = &self.offsets[self.num_internal_nodes..]; diff --git a/eth2/utils/ssz/src/cached_tree_hash/impls.rs b/eth2/utils/ssz/src/cached_tree_hash/impls.rs index 621c5d02b..58343de3a 100644 --- a/eth2/utils/ssz/src/cached_tree_hash/impls.rs +++ b/eth2/utils/ssz/src/cached_tree_hash/impls.rs @@ -1,9 +1,7 @@ use super::*; use crate::{ssz_encode, Encodable}; -impl CachedTreeHash for u64 { - type Item = Self; - +impl CachedTreeHash for u64 { fn item_type() -> ItemType { ItemType::Basic } @@ -47,12 +45,10 @@ impl CachedTreeHash for u64 { } } -impl CachedTreeHash for Vec +impl CachedTreeHash> for Vec where - T: CachedTreeHash, + T: CachedTreeHash, { - type Item = Self; - fn item_type() -> ItemType { ItemType::List } @@ -78,7 +74,7 @@ where let mut offsets = vec![]; for item in self { - offsets.push(item.offsets()?.iter().sum()) + offsets.push(OffsetHandler::new(item, 0)?.total_nodes()) } offsets @@ -107,32 +103,51 @@ where fn cached_hash_tree_root( &self, - other: &Self::Item, + other: &Vec, cache: &mut TreeHashCache, chunk: usize, ) -> Result { let offset_handler = OffsetHandler::new(self, chunk)?; + if self.len() != other.len() { + panic!("variable sized lists not implemented"); + } + match T::item_type() { ItemType::Basic => { let leaves = get_packed_leaves(self); for (i, chunk) in offset_handler.iter_leaf_nodes().enumerate() { if let Some(latest) = leaves.get(i * HASHSIZE..(i + 1) * HASHSIZE) { - if !cache.chunk_equals(*chunk, latest)? { - dbg!(chunk); - cache.set_changed(*chunk, true)?; - } + cache.maybe_update_chunk(*chunk, latest)?; } } let first_leaf_chunk = offset_handler.first_leaf_node()?; cache.chunk_splice(first_leaf_chunk..offset_handler.next_node, leaves); } - _ => panic!("not implemented"), + ItemType::Composite | ItemType::List => { + let mut i = offset_handler.num_leaf_nodes; + for start_chunk in offset_handler.iter_leaf_nodes().rev() { + i -= 1; + match (other.get(i), self.get(i)) { + // The item existed in the previous list and exsits in the current list. + (Some(old), Some(new)) => { + new.cached_hash_tree_root(old, cache, *start_chunk)?; + }, + // The item didn't exist in the old list and doesn't exist in the new list, + // nothing to do. + (None, None) => {}, + _ => panic!("variable sized lists not implemented") + }; + } + // this thing + } } for (&parent, children) in offset_handler.iter_internal_nodes().rev() { if cache.either_modified(children)? { + dbg!(parent); + dbg!(children); cache.modify_chunk(parent, &cache.hash_children(children)?)?; } } @@ -143,7 +158,7 @@ where fn get_packed_leaves(vec: &Vec) -> Vec where - T: CachedTreeHash, + T: CachedTreeHash, { let num_packed_bytes = vec.num_bytes(); let num_leaves = num_sanitized_leaves(num_packed_bytes); diff --git a/eth2/utils/ssz/src/cached_tree_hash/tests.rs b/eth2/utils/ssz/src/cached_tree_hash/tests.rs index e65c87bbd..156e2c2e5 100644 --- a/eth2/utils/ssz/src/cached_tree_hash/tests.rs +++ b/eth2/utils/ssz/src/cached_tree_hash/tests.rs @@ -2,7 +2,7 @@ use super::*; use crate::Encodable; use int_to_bytes::{int_to_bytes32, int_to_bytes8}; -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Inner { pub a: u64, pub b: u64, @@ -10,9 +10,7 @@ pub struct Inner { pub d: u64, } -impl CachedTreeHash for Inner { - type Item = Self; - +impl CachedTreeHash for Inner { fn item_type() -> ItemType { ItemType::Composite } @@ -100,16 +98,14 @@ impl CachedTreeHash for Inner { } } -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct Outer { pub a: u64, pub b: Inner, pub c: u64, } -impl CachedTreeHash for Outer { - type Item = Self; - +impl CachedTreeHash for Outer { fn item_type() -> ItemType { ItemType::Composite } @@ -398,6 +394,66 @@ fn large_vec_of_u64_builds() { assert_eq!(expected, cache); } +#[test] +fn partial_modification_of_vec_of_inner() { + let original_vec = vec![ + Inner { + a: 0, + b: 1, + c: 2, + d: 3, + }, + Inner { + a: 4, + b: 5, + c: 6, + d: 7, + }, + Inner { + a: 8, + b: 9, + c: 10, + d: 11, + }, + ]; + let mut cache = TreeHashCache::new(&original_vec).unwrap(); + + let mut modified_vec = original_vec.clone(); + modified_vec[1].a = 42; + + modified_vec + .cached_hash_tree_root(&original_vec, &mut cache, 0) + .unwrap(); + let modified_cache: Vec = cache.into(); + + // Build the reference vec. + + let mut numbers: Vec = (0..12).collect(); + numbers[4] = 42; + + let mut leaves = vec![]; + let mut full_bytes = vec![]; + + for n in numbers.chunks(4) { + let mut merkle = merkleize(join(vec![ + int_to_bytes32(n[0]), + int_to_bytes32(n[1]), + int_to_bytes32(n[2]), + int_to_bytes32(n[3]), + ])); + leaves.append(&mut merkle[0..HASHSIZE].to_vec()); + full_bytes.append(&mut merkle); + } + + let mut expected = merkleize(leaves); + expected.splice(3 * HASHSIZE.., full_bytes); + expected.append(&mut vec![0; HASHSIZE]); + + // Compare the cached tree to the reference tree. + + assert_trees_eq(&expected, &modified_cache); +} + #[test] fn vec_of_inner_builds() { let numbers: Vec = (0..12).collect(); @@ -449,9 +505,17 @@ fn vec_of_inner_builds() { /// Provides detailed assertions when comparing merkle trees. fn assert_trees_eq(a: &[u8], b: &[u8]) { assert_eq!(a.len(), b.len(), "Byte lens different"); - for i in 0..a.len() / HASHSIZE { + for i in (0..a.len() / HASHSIZE).rev() { let range = i * HASHSIZE..(i + 1) * HASHSIZE; - assert_eq!(a[range.clone()], b[range], "Chunk {} different", i); + assert_eq!( + a[range.clone()], + b[range], + "Chunk {}/{} different \n\n a: {:?} \n\n b: {:?}", + i, + a.len() / HASHSIZE, + a, + b, + ); } }