From a204177ef1e9d8692a4af9e353b3c905b51a9c65 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 6 Dec 2023 11:02:51 -0500 Subject: [PATCH 1/9] Make MerkleStream handle branches with values --- firewood/src/merkle/stream.rs | 507 +++++++++++++++++++++------------- 1 file changed, 310 insertions(+), 197 deletions(-) diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index 4186d7e22..ddd0bffd1 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -1,13 +1,13 @@ // Copyright (C) 2023, Ava Labs, Inc. All rights reserved. // See the file LICENSE.md for licensing terms. -use super::{node::Node, LeafNode, Merkle, MerkleError, NodeType, ObjRef}; +use super::{node::Node, BranchNode, Merkle, NodeType, ObjRef}; use crate::{ shale::{DiskAddress, ShaleStore}, v2::api, }; use futures::Stream; -use std::task::Poll; +use std::{ops::Not, task::Poll}; pub(super) enum IteratorState<'a> { /// Start iterating at the beginning of the trie, @@ -15,9 +15,10 @@ pub(super) enum IteratorState<'a> { StartAtBeginning, /// Start iterating at the specified key StartAtKey(Vec), - /// Continue iterating after the given last_node and parents + /// Continue iterating after the given `next_node` and parents Iterating { - last_node: ObjRef<'a>, + visit_last: bool, + next_result: Option<(Vec, Vec)>, parents: Vec<(ObjRef<'a>, u8)>, }, } @@ -38,9 +39,6 @@ impl<'a> Default for IteratorState<'a> { } /// A MerkleKeyValueStream iterates over keys/values for a merkle trie. -/// This iterator is not fused. If you read past the None value, you start -/// over at the beginning. If you need a fused iterator, consider using -/// std::iter::fuse pub struct MerkleKeyValueStream<'a, S, T> { pub(super) key_state: IteratorState<'a>, pub(super) merkle_root: DiskAddress, @@ -54,236 +52,353 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' mut self: std::pin::Pin<&mut Self>, _cx: &mut std::task::Context<'_>, ) -> Poll> { - // Note that this sets the key_state to StartAtBeginning temporarily - let found_key = match std::mem::take(&mut self.key_state) { + let MerkleKeyValueStream { + key_state, + merkle_root, + merkle, + } = &mut *self; + + match key_state { IteratorState::StartAtBeginning => { - let root_node = self - .merkle - .get_node(self.merkle_root) + let root = merkle + .get_node(*merkle_root) .map_err(|e| api::Error::InternalError(Box::new(e)))?; - let mut last_node = root_node; - let mut parents = vec![]; - let leaf = loop { - match last_node.inner() { - NodeType::Branch(branch) => { - let Some((leftmost_position, leftmost_address)) = branch - .children - .iter() - .enumerate() - .filter_map(|(i, addr)| addr.map(|addr| (i, addr))) - .next() - else { - // we already exhausted the branch node. This happens with an empty trie - // ... or a corrupt one - return if parents.is_empty() { - // empty trie - Poll::Ready(None) - } else { - // branch with NO children, not at the top - Poll::Ready(Some(Err(api::Error::InternalError(Box::new( - MerkleError::ParentLeafBranch, - ))))) - }; - }; - - let next = self - .merkle - .get_node(leftmost_address) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; - - parents.push((last_node, leftmost_position as u8)); - - last_node = next; - } - NodeType::Leaf(leaf) => break leaf, - NodeType::Extension(_) => todo!(), - } - }; - // last_node should have a leaf; compute the key and value - let current_key = key_from_parents_and_leaf(&parents, leaf); + // always put the sentinal node in parents + let mut parents = vec![(root, 0)]; - self.key_state = IteratorState::Iterating { last_node, parents }; + let next_result = find_next_result(merkle, &mut parents, true) + .map_err(|e| api::Error::InternalError(Box::new(e)))?; + + let (next_result, visit_last) = next_result + .map(|(next, visit)| (Some(next), visit)) + .unwrap_or_default(); + + self.key_state = IteratorState::Iterating { + visit_last, + next_result, + parents, + }; - current_key + self.poll_next(_cx) } + IteratorState::StartAtKey(key) => { - // TODO: support finding the next key after K - let root_node = self - .merkle - .get_node(self.merkle_root) + let root_node = merkle + .get_node(*merkle_root) .map_err(|e| api::Error::InternalError(Box::new(e)))?; - let (found_node, parents) = self - .merkle - .get_node_and_parents_by_key(root_node, &key) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; + let (next_result, parents) = { + let (found_node, parents) = { + let mut parents = vec![]; + + let found_node = merkle + .get_node_by_key_with_callbacks( + root_node, + &key, + |addr, nib| { + parents.push((addr, nib)); + }, + |_, _| {}, + ) + .map_err(|e| api::Error::InternalError(Box::new(e)))?; - let Some(last_node) = found_node else { - return Poll::Ready(None); - }; + (found_node, parents) + }; - let returned_key_value = match last_node.inner() { - NodeType::Branch(branch) => (key, branch.value.to_owned().unwrap().to_vec()), - NodeType::Leaf(leaf) => (key, leaf.data.to_vec()), - NodeType::Extension(_) => todo!(), - }; + let mut parents = parents + .into_iter() + .map(|(addr, nib)| merkle.get_node(addr).map(|node| (node, nib))) + .collect::, _>>() + .map_err(|e| api::Error::InternalError(Box::new(e)))?; - self.key_state = IteratorState::Iterating { last_node, parents }; + // means the keys definitely match + if let Some(found_node) = found_node { + let (value, visit_last) = match found_node.inner() { + NodeType::Extension(_) => unreachable!(), - return Poll::Ready(Some(Ok(returned_key_value))); - } - IteratorState::Iterating { - last_node, - mut parents, - } => { - match last_node.inner() { - NodeType::Branch(branch) => { - // previously rendered the value from a branch node, so walk down to the first available child - let Some((child_position, child_address)) = branch - .children - .iter() - .enumerate() - .filter_map(|(child_position, &addr)| { - addr.map(|addr| (child_position, addr)) - }) - .next() - else { - // Branch node with no children? - return Poll::Ready(Some(Err(api::Error::InternalError(Box::new( - MerkleError::ParentLeafBranch, - ))))); - }; + NodeType::Branch(branch) => { + let value = branch.value.as_ref().map(|v| v.to_vec()); + let visit_last = true; + parents.push((found_node, 0)); - parents.push((last_node, child_position as u8)); // remember where we walked down from + (value, visit_last) + } - let current_node = self - .merkle - .get_node(child_address) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; + NodeType::Leaf(leaf) => { + let value = leaf.data.to_vec(); + let visit_last = false; - let found_key = key_from_parents(&parents); + (Some(value), visit_last) + } + }; + + let next_result = value.map(|value| (std::mem::take(key), value)); self.key_state = IteratorState::Iterating { - // continue iterating from here - last_node: current_node, + visit_last, + next_result, parents, }; - found_key + return self.poll_next(_cx); } - NodeType::Leaf(leaf) => { - let mut next = parents.pop().map(|(node, position)| (node, Some(position))); - loop { - match next { - None => return Poll::Ready(None), - Some((parent, child_position)) => { - // Assume all parents are branch nodes - let children = parent.inner().as_branch().unwrap().chd(); - - // we use wrapping_add here because the value might be u8::MAX indicating that - // we want to go down branch - let start_position = - child_position.map(|pos| pos + 1).unwrap_or_default(); - - let Some((found_position, found_address)) = children - .iter() - .enumerate() - .skip(start_position as usize) - .filter_map(|(offset, addr)| { - addr.map(|addr| (offset as u8, addr)) - }) - .next() - else { - next = parents - .pop() - .map(|(node, position)| (node, Some(position))); - continue; - }; - - // we push (node, None) which will start at the beginning of the next branch node - let child = self - .merkle - .get_node(found_address) - .map(|node| (node, None)) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; - - // stop_descending if: - // - on a branch and it has a value; OR - // - on a leaf - let stop_descending = match child.0.inner() { - NodeType::Branch(branch) => branch.value.is_some(), - NodeType::Leaf(_) => true, - NodeType::Extension(_) => todo!(), - }; - - next = Some(child); - - parents.push((parent, found_position)); - - if stop_descending { - break; - } - } - } - } - // recompute current_key - // TODO: Can we keep current_key updated as we walk the tree instead of building it from the top all the time? - let current_key = key_from_parents_and_leaf(&parents, leaf); - self.key_state = IteratorState::Iterating { - last_node: next.unwrap().0, - parents, - }; + let key_from_parents = key_from_nibble_iter(nibble_iter_from_parents(&parents)); + let mut visit_last = key == &key_from_parents; - current_key + if key.as_slice() < key_from_parents.as_slice() { + let _ = parents.pop(); + visit_last = true; } - NodeType::Extension(_) => todo!(), - } + let next_result = find_next_result(merkle, &mut parents, visit_last) + .map_err(|e| api::Error::InternalError(Box::new(e)))?; + + (next_result, parents) + }; + + let (next_result, visit_last) = next_result + .map(|(next, visit)| (Some(next), visit)) + .unwrap_or_default(); + + self.key_state = IteratorState::Iterating { + visit_last, + next_result, + parents, + }; + + self.poll_next(_cx) } - }; - // figure out the value to return from the state - // if we get here, we're sure to have something to return - // TODO: It's possible to return a reference to the data since the last_node is - // saved in the iterator - let return_value = match &self.key_state { IteratorState::Iterating { - last_node, - parents: _, + visit_last, + next_result, + parents, } => { - let value = match last_node.inner() { - NodeType::Branch(branch) => branch.value.to_owned().unwrap().to_vec(), - NodeType::Leaf(leaf) => leaf.data.to_vec(), - NodeType::Extension(_) => todo!(), + let Some((next_key, next_value)) = next_result.take() else { + return Poll::Ready(None); }; - (found_key, value) + let next = find_next_result(merkle, parents, *visit_last) + .map_err(|e| api::Error::InternalError(Box::new(e)))?; + + (*next_result, *visit_last) = next + .map(|(next, visit)| (Some(next), visit)) + .unwrap_or_default(); + + Poll::Ready(Some(Ok((next_key, next_value)))) } - _ => unreachable!(), + } + } +} + +enum ParentNode<'a> { + New(ObjRef<'a>), + Visited(ObjRef<'a>), +} + +#[derive(Debug)] +enum InnerNode<'a> { + New(&'a NodeType), + Visited(&'a NodeType), +} + +impl<'a> ParentNode<'a> { + fn inner(&self) -> InnerNode<'_> { + match self { + Self::New(node) => InnerNode::New(node.inner()), + Self::Visited(node) => InnerNode::Visited(node.inner()), + } + } + + fn into_node(self) -> ObjRef<'a> { + match self { + Self::New(node) => node, + Self::Visited(node) => node, + } + } +} + +type NextResult = Option<((Vec, Vec), bool)>; + +fn find_next_result<'a, S: ShaleStore, T>( + merkle: &'a Merkle, + parents: &mut Vec<(ObjRef<'a>, u8)>, + visit_last: bool, +) -> Result { + let next = find_next_node_with_data(merkle, parents, visit_last)?.map(|next| { + let (next_node, value) = next; + let node_path_iter = match next_node.inner() { + NodeType::Leaf(leaf) => leaf.path.iter().copied(), + _ => [].iter().copied(), }; - Poll::Ready(Some(Ok(return_value))) + let key = key_from_nibble_iter(nibble_iter_from_parents(parents).chain(node_path_iter)); + let visit_last = matches!(next_node.inner(), NodeType::Branch(_)); + + if visit_last { + parents.push((next_node, 0)); + } + + ((key, value), visit_last) + }); + + Ok(next) +} + +fn find_next_node_with_data<'a, S: ShaleStore, T>( + merkle: &'a Merkle, + visited_parents: &mut Vec<(ObjRef<'a>, u8)>, + visit_last: bool, +) -> Result, Vec)>, super::MerkleError> { + use InnerNode::*; + + let Some((visited_parent, visited_pos)) = visited_parents.pop() else { + return Ok(None); + }; + + let mut node = ParentNode::Visited(visited_parent); + let mut pos = visited_pos; + + loop { + match node.inner() { + // TODO: find a better way to handle this impossible case + Visited(NodeType::Leaf(_)) => unreachable!(), + + New(NodeType::Leaf(leaf)) => { + let value = leaf.data.to_vec(); + return Ok(Some((node.into_node(), value))); + } + + Visited(NodeType::Extension(_)) => { + let Some((next_parent, next_pos)) = visited_parents.pop() else { + return Ok(None); + }; + + node = ParentNode::Visited(next_parent); + pos = next_pos; + } + + New(NodeType::Extension(extension)) => { + let child = merkle.get_node(extension.chd())?; + + pos = 0; + visited_parents.push((node.into_node(), pos)); + + node = ParentNode::New(child); + } + + Visited(NodeType::Branch(branch)) => { + let compare_op = if visit_last { + ::ge + } else { + ::gt + }; + + let children = get_children_iter(branch) + .filter(move |(_, child_pos)| compare_op(child_pos, &pos)); + + let next_child_success = + next_child(merkle, children, visited_parents, &mut node, &mut pos)?; + + if !next_child_success { + return Ok(None); + } + } + + New(NodeType::Branch(branch)) => { + if let Some(value) = branch.value.as_ref() { + let value = value.to_vec(); + return Ok(Some((node.into_node(), value))); + } + + let children = get_children_iter(branch); + + let next_child_success = + next_child(merkle, children, visited_parents, &mut node, &mut pos)?; + + if !next_child_success { + return Ok(None); + } + } + } + } +} + +fn get_children_iter(branch: &BranchNode) -> impl Iterator { + branch + .children + .into_iter() + .enumerate() + .filter_map(|(pos, child_addr)| child_addr.map(|child_addr| (child_addr, pos as u8))) +} + +#[must_use] +struct MustUse(T); + +impl From for MustUse { + fn from(t: T) -> Self { + Self(t) } } -/// Compute a key from a set of parents -fn key_from_parents(parents: &[(ObjRef, u8)]) -> Vec { - parents[1..] - .chunks_exact(2) - .map(|parents| (parents[0].1 << 4) + parents[1].1) - .collect::>() +impl Not for MustUse { + type Output = T::Output; + + fn not(self) -> Self::Output { + self.0.not() + } } -fn key_from_parents_and_leaf(parents: &[(ObjRef, u8)], leaf: &LeafNode) -> Vec { - let mut iter = parents[1..] + +fn next_child<'a, S, T, Iter>( + merkle: &'a Merkle, + mut children: Iter, + parents: &mut Vec<(ObjRef<'a>, u8)>, + node: &mut ParentNode<'a>, + pos: &mut u8, +) -> Result, super::MerkleError> +where + Iter: Iterator, + S: ShaleStore, +{ + if let Some((child_addr, child_pos)) = children.next() { + let child = merkle.get_node(child_addr)?; + + *pos = child_pos; + let node = std::mem::replace(node, ParentNode::New(child)); + parents.push((node.into_node(), *pos)); + } else { + let Some((next_parent, next_pos)) = parents.pop() else { + return Ok(false.into()); + }; + + *node = ParentNode::Visited(next_parent); + *pos = next_pos; + } + + Ok(true.into()) +} + +/// create an iterator over the key-nibbles from all parents _excluding_ the sentinal node. +fn nibble_iter_from_parents<'a>(parents: &'a [(ObjRef, u8)]) -> impl Iterator + 'a { + parents .iter() - .map(|parent| parent.1) - .chain(leaf.path.to_vec()); - let mut data = Vec::with_capacity(iter.size_hint().0); - while let (Some(hi), Some(lo)) = (iter.next(), iter.next()) { + .skip(1) // always skip the sentinal node + .flat_map(|(parent, child_nibble)| match parent.inner() { + NodeType::Branch(_) => vec![*child_nibble], + NodeType::Extension(extension) => extension.path.to_vec(), + NodeType::Leaf(leaf) => leaf.path.to_vec(), + }) +} + +fn key_from_nibble_iter>(mut nibbles: Iter) -> Vec { + let mut data = Vec::with_capacity(nibbles.size_hint().0); + + while let (Some(hi), Some(lo)) = (nibbles.next(), nibbles.next()) { data.push((hi << 4) + lo); } + data } @@ -334,7 +449,6 @@ mod tests { assert!(it.next().await.is_none()); } - #[ignore] #[tokio::test] async fn get_branch_and_leaf() { let mut merkle = create_test_merkle(); @@ -371,7 +485,6 @@ mod tests { ); } - #[ignore] #[tokio::test] async fn start_at_key_not_in_trie() { let mut merkle = create_test_merkle(); From 4efdd14cc35869b945d5842b126cb02b28db1cf3 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Mon, 11 Dec 2023 15:32:31 -0500 Subject: [PATCH 2/9] Stop prefetching next node in iterator --- firewood/src/merkle/stream.rs | 207 ++++++++++++++-------------------- 1 file changed, 83 insertions(+), 124 deletions(-) diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index ddd0bffd1..6c0d128b8 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -16,11 +16,7 @@ pub(super) enum IteratorState<'a> { /// Start iterating at the specified key StartAtKey(Vec), /// Continue iterating after the given `next_node` and parents - Iterating { - visit_last: bool, - next_result: Option<(Vec, Vec)>, - parents: Vec<(ObjRef<'a>, u8)>, - }, + Iterating { parents: Vec<(ObjRef<'a>, u8)> }, } impl IteratorState<'_> { pub(super) fn new>(starting: Option) -> Self { @@ -67,127 +63,60 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' // always put the sentinal node in parents let mut parents = vec![(root, 0)]; - let next_result = find_next_result(merkle, &mut parents, true) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; - - let (next_result, visit_last) = next_result - .map(|(next, visit)| (Some(next), visit)) - .unwrap_or_default(); + let next_result = find_next_result(merkle, &mut parents) + .map_err(|e| api::Error::InternalError(Box::new(e))) + .transpose(); - self.key_state = IteratorState::Iterating { - visit_last, - next_result, - parents, - }; + self.key_state = IteratorState::Iterating { parents }; - self.poll_next(_cx) + Poll::Ready(next_result) } IteratorState::StartAtKey(key) => { + if key.is_empty() { + self.key_state = IteratorState::StartAtBeginning; + return self.poll_next(_cx); + } + let root_node = merkle .get_node(*merkle_root) .map_err(|e| api::Error::InternalError(Box::new(e)))?; - let (next_result, parents) = { - let (found_node, parents) = { - let mut parents = vec![]; - - let found_node = merkle - .get_node_by_key_with_callbacks( - root_node, - &key, - |addr, nib| { - parents.push((addr, nib)); - }, - |_, _| {}, - ) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; - - (found_node, parents) - }; - - let mut parents = parents - .into_iter() - .map(|(addr, nib)| merkle.get_node(addr).map(|node| (node, nib))) - .collect::, _>>() - .map_err(|e| api::Error::InternalError(Box::new(e)))?; - - // means the keys definitely match - if let Some(found_node) = found_node { - let (value, visit_last) = match found_node.inner() { - NodeType::Extension(_) => unreachable!(), - - NodeType::Branch(branch) => { - let value = branch.value.as_ref().map(|v| v.to_vec()); - let visit_last = true; - parents.push((found_node, 0)); - - (value, visit_last) - } - - NodeType::Leaf(leaf) => { - let value = leaf.data.to_vec(); - let visit_last = false; - - (Some(value), visit_last) - } - }; - - let next_result = value.map(|value| (std::mem::take(key), value)); - - self.key_state = IteratorState::Iterating { - visit_last, - next_result, - parents, - }; + let (found_node, mut parents) = merkle + .get_node_and_parents_by_key(root_node, &key) + .map_err(|e| api::Error::InternalError(Box::new(e)))?; - return self.poll_next(_cx); - } + if let Some(found_node) = found_node { + let value = match found_node.inner() { + NodeType::Branch(branch) => branch.value.as_ref(), + NodeType::Leaf(leaf) => Some(&leaf.data), + NodeType::Extension(_) => None, + }; - let key_from_parents = key_from_nibble_iter(nibble_iter_from_parents(&parents)); - let mut visit_last = key == &key_from_parents; + let next_result = value.map(|value| { + let value = value.to_vec(); - if key.as_slice() < key_from_parents.as_slice() { - let _ = parents.pop(); - visit_last = true; - } + Ok((std::mem::take(key), value)) + }); - let next_result = find_next_result(merkle, &mut parents, visit_last) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; + parents.push((found_node, 0)); - (next_result, parents) - }; + self.key_state = IteratorState::Iterating { parents }; - let (next_result, visit_last) = next_result - .map(|(next, visit)| (Some(next), visit)) - .unwrap_or_default(); + return Poll::Ready(next_result); + } - self.key_state = IteratorState::Iterating { - visit_last, - next_result, - parents, - }; + self.key_state = IteratorState::Iterating { parents }; self.poll_next(_cx) } - IteratorState::Iterating { - visit_last, - next_result, - parents, - } => { - let Some((next_key, next_value)) = next_result.take() else { - return Poll::Ready(None); - }; - - let next = find_next_result(merkle, parents, *visit_last) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; - - (*next_result, *visit_last) = next - .map(|(next, visit)| (Some(next), visit)) - .unwrap_or_default(); + IteratorState::Iterating { parents } => { + let next = find_next_result(merkle, parents) + .map_err(|e| api::Error::InternalError(Box::new(e))) + .transpose(); - Poll::Ready(Some(Ok((next_key, next_value)))) + Poll::Ready(next) } } } @@ -220,28 +149,24 @@ impl<'a> ParentNode<'a> { } } -type NextResult = Option<((Vec, Vec), bool)>; +type NextResult = Option<(Vec, Vec)>; fn find_next_result<'a, S: ShaleStore, T>( merkle: &'a Merkle, parents: &mut Vec<(ObjRef<'a>, u8)>, - visit_last: bool, ) -> Result { - let next = find_next_node_with_data(merkle, parents, visit_last)?.map(|next| { - let (next_node, value) = next; + let next = find_next_node_with_data(merkle, parents)?.map(|(next_node, value)| { let node_path_iter = match next_node.inner() { NodeType::Leaf(leaf) => leaf.path.iter().copied(), + NodeType::Extension(extension) => extension.path.iter().copied(), _ => [].iter().copied(), }; let key = key_from_nibble_iter(nibble_iter_from_parents(parents).chain(node_path_iter)); - let visit_last = matches!(next_node.inner(), NodeType::Branch(_)); - if visit_last { - parents.push((next_node, 0)); - } + parents.push((next_node, 0)); - ((key, value), visit_last) + (key, value) }); Ok(next) @@ -250,7 +175,6 @@ fn find_next_result<'a, S: ShaleStore, T>( fn find_next_node_with_data<'a, S: ShaleStore, T>( merkle: &'a Merkle, visited_parents: &mut Vec<(ObjRef<'a>, u8)>, - visit_last: bool, ) -> Result, Vec)>, super::MerkleError> { use InnerNode::*; @@ -260,18 +184,16 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( let mut node = ParentNode::Visited(visited_parent); let mut pos = visited_pos; + let mut first_loop = true; loop { match node.inner() { - // TODO: find a better way to handle this impossible case - Visited(NodeType::Leaf(_)) => unreachable!(), - New(NodeType::Leaf(leaf)) => { let value = leaf.data.to_vec(); return Ok(Some((node.into_node(), value))); } - Visited(NodeType::Extension(_)) => { + Visited(NodeType::Leaf(_)) | Visited(NodeType::Extension(_)) => { let Some((next_parent, next_pos)) = visited_parents.pop() else { return Ok(None); }; @@ -290,7 +212,7 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( } Visited(NodeType::Branch(branch)) => { - let compare_op = if visit_last { + let compare_op = if first_loop { ::ge } else { ::gt @@ -323,6 +245,8 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( } } } + + first_loop = false; } } @@ -386,14 +310,34 @@ fn nibble_iter_from_parents<'a>(parents: &'a [(ObjRef, u8)]) -> impl Iterator vec![*child_nibble], - NodeType::Extension(extension) => extension.path.to_vec(), - NodeType::Leaf(leaf) => leaf.path.to_vec(), + NodeType::Branch(_) => Either::Left(std::iter::once(*child_nibble)), + NodeType::Extension(extension) => Either::Right(extension.path.iter().copied()), + NodeType::Leaf(leaf) => Either::Right(leaf.path.iter().copied()), }) } +enum Either { + Left(T), + Right(U), +} + +impl Iterator for Either +where + T: Iterator, + U: Iterator, +{ + type Item = T::Item; + + fn next(&mut self) -> Option { + match self { + Self::Left(left) => left.next(), + Self::Right(right) => right.next(), + } + } +} + fn key_from_nibble_iter>(mut nibbles: Iter) -> Vec { - let mut data = Vec::with_capacity(nibbles.size_hint().0); + let mut data = Vec::with_capacity(nibbles.size_hint().0 / 2); while let (Some(hi), Some(lo)) = (nibbles.next(), nibbles.next()) { data.push((hi << 4) + lo); @@ -449,6 +393,21 @@ mod tests { assert!(it.next().await.is_none()); } + #[tokio::test] + async fn root_with_empty_data() { + let mut merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + + let key = vec![]; + let value = vec![0x00]; + + merkle.insert(&key, value.clone(), root).unwrap(); + + let mut stream = merkle.get_iter(None::<&[u8]>, root).unwrap(); + + assert_eq!(stream.next().await.unwrap().unwrap(), (key, value)); + } + #[tokio::test] async fn get_branch_and_leaf() { let mut merkle = create_test_merkle(); From 4119d2c3c0b7a2e2a48fb9454f7ccc0343080a74 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 14 Dec 2023 12:26:11 -0500 Subject: [PATCH 3/9] Make MerkleKeyValueStream fused --- firewood/src/db.rs | 12 +++++- firewood/src/merkle.rs | 25 ++++++++--- firewood/src/merkle/stream.rs | 78 ++++++++++++++++++++++++++++++----- 3 files changed, 96 insertions(+), 19 deletions(-) diff --git a/firewood/src/db.rs b/firewood/src/db.rs index 282cf5a18..d3e4eb0dc 100644 --- a/firewood/src/db.rs +++ b/firewood/src/db.rs @@ -320,10 +320,18 @@ impl + Send + Sync> api::DbView for DbRev { impl + Send + Sync> DbRev { pub fn stream( &self, - start_key: Option, ) -> Result, api::Error> { self.merkle - .get_iter(start_key, self.header.kv_root) + .iter(self.header.kv_root) + .map_err(|e| api::Error::InternalError(Box::new(e))) + } + + pub fn stream_from( + &self, + start_key: K, + ) -> Result, api::Error> { + self.merkle + .iter_from(start_key, self.header.kv_root) .map_err(|e| api::Error::InternalError(Box::new(e))) } diff --git a/firewood/src/merkle.rs b/firewood/src/merkle.rs index f567c193f..70d8aa26b 100644 --- a/firewood/src/merkle.rs +++ b/firewood/src/merkle.rs @@ -1221,13 +1221,24 @@ impl + Send + Sync, T> Merkle { self.store.flush_dirty() } - pub(crate) fn get_iter>( + pub(crate) fn iter( &self, - key: Option, root: DiskAddress, ) -> Result, MerkleError> { Ok(MerkleKeyValueStream { - key_state: IteratorState::new(key), + key_state: IteratorState::new(), + merkle_root: root, + merkle: self, + }) + } + + pub(crate) fn iter_from>( + &self, + key: K, + root: DiskAddress, + ) -> Result, MerkleError> { + Ok(MerkleKeyValueStream { + key_state: IteratorState::with_key(key), merkle_root: root, merkle: self, }) @@ -1245,9 +1256,11 @@ impl + Send + Sync, T> Merkle { return Ok(None); } - let mut stream = self - .get_iter(first_key, root) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; + let mut stream = match first_key { + Some(key) => self.iter_from(key, root), + None => self.iter(root), + } + .map_err(|e| api::Error::InternalError(Box::new(e)))?; // fetch the first key from the stream let first_result = stream.next().await; diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index 6c0d128b8..f74d717fe 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -6,7 +6,7 @@ use crate::{ shale::{DiskAddress, ShaleStore}, v2::api, }; -use futures::Stream; +use futures::{stream::FusedStream, Stream}; use std::{ops::Not, task::Poll}; pub(super) enum IteratorState<'a> { @@ -19,11 +19,12 @@ pub(super) enum IteratorState<'a> { Iterating { parents: Vec<(ObjRef<'a>, u8)> }, } impl IteratorState<'_> { - pub(super) fn new>(starting: Option) -> Self { - match starting { - None => Self::StartAtBeginning, - Some(key) => Self::StartAtKey(key.as_ref().to_vec()), - } + pub(super) fn new() -> Self { + Self::StartAtBeginning + } + + pub(super) fn with_key>(key: K) -> Self { + Self::StartAtKey(key.as_ref().to_vec()) } } @@ -41,6 +42,15 @@ pub struct MerkleKeyValueStream<'a, S, T> { pub(super) merkle: &'a Merkle, } +impl<'a, S: ShaleStore + Send + Sync, T> FusedStream for MerkleKeyValueStream<'a, S, T> { + fn is_terminated(&self) -> bool { + match &self.key_state { + IteratorState::Iterating { parents } if parents.is_empty() => true, + _ => false, + } + } +} + impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<'a, S, T> { type Item = Result<(Vec, Vec), api::Error>; @@ -359,7 +369,7 @@ mod tests { async fn iterate_empty() { let merkle = create_test_merkle(); let root = merkle.init_root().unwrap(); - let mut it = merkle.get_iter(Some(b"x"), root).unwrap(); + let mut it = merkle.iter_from(b"x", root).unwrap(); let next = it.next().await; assert!(next.is_none()); } @@ -378,7 +388,12 @@ mod tests { merkle.insert([k], vec![k], root).unwrap(); } - let mut it = merkle.get_iter(start, root).unwrap(); + let mut it = match start { + Some(start) => merkle.iter_from(start, root), + None => merkle.iter(root), + } + .unwrap(); + // we iterate twice because we should get a None then start over for k in start.map(|r| r[0]).unwrap_or_default()..=u8::MAX { let next = it.next().await.map(|kv| { @@ -393,6 +408,47 @@ mod tests { assert!(it.next().await.is_none()); } + #[tokio::test] + async fn fused_empty() { + let merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + let mut it = merkle.iter(root).unwrap(); + assert!(it.next().await.is_none()); + assert!(it.is_terminated()); + } + + #[tokio::test] + async fn fused_full() { + let mut merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + + let last = vec![0x00, 0x00, 0x00]; + + let mut key_values = vec![vec![0x00], vec![0x00, 0x00], last.clone()]; + + // branchs with paths (or extensions) will be present as well as leaves with siblings + for kv in u8::MIN..=u8::MAX { + let mut last = last.clone(); + last.push(kv); + key_values.push(last); + } + + for kv in key_values.iter() { + merkle.insert(kv, kv.clone(), root).unwrap(); + } + + let mut it = merkle.iter(root).unwrap(); + + for kv in key_values.iter() { + let next = it.next().await.unwrap().unwrap(); + assert_eq!(&next.0, kv); + assert_eq!(&next.1, kv); + } + + assert!(it.next().await.is_none()); + assert!(it.is_terminated()); + } + #[tokio::test] async fn root_with_empty_data() { let mut merkle = create_test_merkle(); @@ -403,7 +459,7 @@ mod tests { merkle.insert(&key, value.clone(), root).unwrap(); - let mut stream = merkle.get_iter(None::<&[u8]>, root).unwrap(); + let mut stream = merkle.iter(root).unwrap(); assert_eq!(stream.next().await.unwrap().unwrap(), (key, value)); } @@ -426,7 +482,7 @@ mod tests { merkle.insert(branch, branch.to_vec(), root).unwrap(); - let mut stream = merkle.get_iter(None::<&[u8]>, root).unwrap(); + let mut stream = merkle.iter(root).unwrap(); assert_eq!( stream.next().await.unwrap().unwrap(), @@ -466,7 +522,7 @@ mod tests { merkle.insert(key, key.to_vec(), root).unwrap(); } - let mut stream = merkle.get_iter(Some([intermediate]), root).unwrap(); + let mut stream = merkle.iter_from([intermediate], root).unwrap(); let first_expected = key_values[1].as_slice(); let first = stream.next().await.unwrap().unwrap(); From fb67626e65355ddbe3d707653f8a3ebdc3ddadd5 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 14 Dec 2023 13:13:01 -0500 Subject: [PATCH 4/9] Cleanup apis for stream --- firewood/src/db.rs | 17 ++----- firewood/src/merkle.rs | 31 ++++-------- firewood/src/merkle/stream.rs | 91 ++++++++++++++++------------------- 3 files changed, 55 insertions(+), 84 deletions(-) diff --git a/firewood/src/db.rs b/firewood/src/db.rs index d3e4eb0dc..e2cb9d638 100644 --- a/firewood/src/db.rs +++ b/firewood/src/db.rs @@ -318,21 +318,12 @@ impl + Send + Sync> api::DbView for DbRev { } impl + Send + Sync> DbRev { - pub fn stream( - &self, - ) -> Result, api::Error> { - self.merkle - .iter(self.header.kv_root) - .map_err(|e| api::Error::InternalError(Box::new(e))) + pub fn stream(&self) -> merkle::MerkleKeyValueStream<'_, S, Bincode> { + self.merkle.iter(self.header.kv_root) } - pub fn stream_from( - &self, - start_key: K, - ) -> Result, api::Error> { - self.merkle - .iter_from(start_key, self.header.kv_root) - .map_err(|e| api::Error::InternalError(Box::new(e))) + pub fn stream_from(&self, start_key: Vec) -> merkle::MerkleKeyValueStream<'_, S, Bincode> { + self.merkle.iter_from(self.header.kv_root, start_key) } fn flush_dirty(&mut self) -> Option<()> { diff --git a/firewood/src/merkle.rs b/firewood/src/merkle.rs index 70d8aa26b..46f00f068 100644 --- a/firewood/src/merkle.rs +++ b/firewood/src/merkle.rs @@ -24,7 +24,6 @@ pub use node::{ NodeType, PartialPath, }; pub use proof::{Proof, ProofError}; -use stream::IteratorState; pub use stream::MerkleKeyValueStream; pub use trie_hash::{TrieHash, TRIE_HASH_LEN}; @@ -1221,27 +1220,16 @@ impl + Send + Sync, T> Merkle { self.store.flush_dirty() } - pub(crate) fn iter( - &self, - root: DiskAddress, - ) -> Result, MerkleError> { - Ok(MerkleKeyValueStream { - key_state: IteratorState::new(), - merkle_root: root, - merkle: self, - }) + pub(crate) fn iter(&self, root: DiskAddress) -> MerkleKeyValueStream<'_, S, T> { + MerkleKeyValueStream::new(self, root) } - pub(crate) fn iter_from>( + pub(crate) fn iter_from( &self, - key: K, root: DiskAddress, - ) -> Result, MerkleError> { - Ok(MerkleKeyValueStream { - key_state: IteratorState::with_key(key), - merkle_root: root, - merkle: self, - }) + key: Vec, + ) -> MerkleKeyValueStream<'_, S, T> { + MerkleKeyValueStream::from_key(self, root, key) } pub(super) async fn range_proof( @@ -1252,15 +1240,14 @@ impl + Send + Sync, T> Merkle { limit: Option, ) -> Result, Vec>>, api::Error> { // limit of 0 is always an empty RangeProof - if let Some(0) = limit { + if limit == Some(0) { return Ok(None); } let mut stream = match first_key { - Some(key) => self.iter_from(key, root), + Some(key) => self.iter_from(root, key.as_ref().to_vec()), None => self.iter(root), - } - .map_err(|e| api::Error::InternalError(Box::new(e)))?; + }; // fetch the first key from the stream let first_result = stream.next().await; diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index f74d717fe..ced26fd72 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -9,37 +9,27 @@ use crate::{ use futures::{stream::FusedStream, Stream}; use std::{ops::Not, task::Poll}; -pub(super) enum IteratorState<'a> { - /// Start iterating at the beginning of the trie, - /// returning the lowest key/value pair first - StartAtBeginning, +enum IteratorState<'a> { /// Start iterating at the specified key StartAtKey(Vec), /// Continue iterating after the given `next_node` and parents Iterating { parents: Vec<(ObjRef<'a>, u8)> }, } impl IteratorState<'_> { - pub(super) fn new() -> Self { - Self::StartAtBeginning + fn new() -> Self { + Self::StartAtKey(vec![]) } - pub(super) fn with_key>(key: K) -> Self { - Self::StartAtKey(key.as_ref().to_vec()) - } -} - -// The default state is to start at the beginning -impl<'a> Default for IteratorState<'a> { - fn default() -> Self { - Self::StartAtBeginning + fn with_key(key: Vec) -> Self { + Self::StartAtKey(key) } } /// A MerkleKeyValueStream iterates over keys/values for a merkle trie. pub struct MerkleKeyValueStream<'a, S, T> { - pub(super) key_state: IteratorState<'a>, - pub(super) merkle_root: DiskAddress, - pub(super) merkle: &'a Merkle, + key_state: IteratorState<'a>, + merkle_root: DiskAddress, + merkle: &'a Merkle, } impl<'a, S: ShaleStore + Send + Sync, T> FusedStream for MerkleKeyValueStream<'a, S, T> { @@ -51,6 +41,32 @@ impl<'a, S: ShaleStore + Send + Sync, T> FusedStream for MerkleKeyValueStr } } +impl<'a, S, T> MerkleKeyValueStream<'a, S, T> { + pub(super) fn new(merkle: &'a Merkle, merkle_root: DiskAddress) -> Self { + let key_state = IteratorState::new(); + + Self { + merkle, + key_state, + merkle_root, + } + } + + pub(super) fn from_key( + merkle: &'a Merkle, + merkle_root: DiskAddress, + key: Vec, + ) -> Self { + let key_state = IteratorState::with_key(key); + + Self { + merkle, + key_state, + merkle_root, + } + } +} + impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<'a, S, T> { type Item = Result<(Vec, Vec), api::Error>; @@ -65,29 +81,7 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' } = &mut *self; match key_state { - IteratorState::StartAtBeginning => { - let root = merkle - .get_node(*merkle_root) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; - - // always put the sentinal node in parents - let mut parents = vec![(root, 0)]; - - let next_result = find_next_result(merkle, &mut parents) - .map_err(|e| api::Error::InternalError(Box::new(e))) - .transpose(); - - self.key_state = IteratorState::Iterating { parents }; - - Poll::Ready(next_result) - } - IteratorState::StartAtKey(key) => { - if key.is_empty() { - self.key_state = IteratorState::StartAtBeginning; - return self.poll_next(_cx); - } - let root_node = merkle .get_node(*merkle_root) .map_err(|e| api::Error::InternalError(Box::new(e)))?; @@ -369,7 +363,7 @@ mod tests { async fn iterate_empty() { let merkle = create_test_merkle(); let root = merkle.init_root().unwrap(); - let mut it = merkle.iter_from(b"x", root).unwrap(); + let mut it = merkle.iter_from(root, b"x".to_vec()); let next = it.next().await; assert!(next.is_none()); } @@ -389,10 +383,9 @@ mod tests { } let mut it = match start { - Some(start) => merkle.iter_from(start, root), + Some(start) => merkle.iter_from(root, start.to_vec()), None => merkle.iter(root), - } - .unwrap(); + }; // we iterate twice because we should get a None then start over for k in start.map(|r| r[0]).unwrap_or_default()..=u8::MAX { @@ -412,7 +405,7 @@ mod tests { async fn fused_empty() { let merkle = create_test_merkle(); let root = merkle.init_root().unwrap(); - let mut it = merkle.iter(root).unwrap(); + let mut it = merkle.iter(root); assert!(it.next().await.is_none()); assert!(it.is_terminated()); } @@ -437,7 +430,7 @@ mod tests { merkle.insert(kv, kv.clone(), root).unwrap(); } - let mut it = merkle.iter(root).unwrap(); + let mut it = merkle.iter(root); for kv in key_values.iter() { let next = it.next().await.unwrap().unwrap(); @@ -459,7 +452,7 @@ mod tests { merkle.insert(&key, value.clone(), root).unwrap(); - let mut stream = merkle.iter(root).unwrap(); + let mut stream = merkle.iter(root); assert_eq!(stream.next().await.unwrap().unwrap(), (key, value)); } @@ -482,7 +475,7 @@ mod tests { merkle.insert(branch, branch.to_vec(), root).unwrap(); - let mut stream = merkle.iter(root).unwrap(); + let mut stream = merkle.iter(root); assert_eq!( stream.next().await.unwrap().unwrap(), @@ -522,7 +515,7 @@ mod tests { merkle.insert(key, key.to_vec(), root).unwrap(); } - let mut stream = merkle.iter_from([intermediate], root).unwrap(); + let mut stream = merkle.iter_from(root, vec![intermediate]); let first_expected = key_values[1].as_slice(); let first = stream.next().await.unwrap().unwrap(); From cc3b0e2e8306f4fa4b86064db24e8b48e509ea72 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 14 Dec 2023 14:06:02 -0500 Subject: [PATCH 5/9] Use boxed-slice as key --- firewood/src/db.rs | 5 +- firewood/src/merkle.rs | 10 +- firewood/src/merkle/stream.rs | 189 ++++++++++++++++++---------------- 3 files changed, 111 insertions(+), 93 deletions(-) diff --git a/firewood/src/db.rs b/firewood/src/db.rs index e2cb9d638..ad05f24ee 100644 --- a/firewood/src/db.rs +++ b/firewood/src/db.rs @@ -322,7 +322,10 @@ impl + Send + Sync> DbRev { self.merkle.iter(self.header.kv_root) } - pub fn stream_from(&self, start_key: Vec) -> merkle::MerkleKeyValueStream<'_, S, Bincode> { + pub fn stream_from( + &self, + start_key: Box<[u8]>, + ) -> merkle::MerkleKeyValueStream<'_, S, Bincode> { self.merkle.iter_from(self.header.kv_root, start_key) } diff --git a/firewood/src/merkle.rs b/firewood/src/merkle.rs index 46f00f068..65ed1832b 100644 --- a/firewood/src/merkle.rs +++ b/firewood/src/merkle.rs @@ -1227,7 +1227,7 @@ impl + Send + Sync, T> Merkle { pub(crate) fn iter_from( &self, root: DiskAddress, - key: Vec, + key: Box<[u8]>, ) -> MerkleKeyValueStream<'_, S, T> { MerkleKeyValueStream::from_key(self, root, key) } @@ -1245,7 +1245,8 @@ impl + Send + Sync, T> Merkle { } let mut stream = match first_key { - Some(key) => self.iter_from(root, key.as_ref().to_vec()), + // TODO: fix the call-site to force the caller to do the allocation + Some(key) => self.iter_from(root, key.as_ref().to_vec().into_boxed_slice()), None => self.iter(root), }; @@ -1264,7 +1265,7 @@ impl + Send + Sync, T> Merkle { .map_err(|e| api::Error::InternalError(Box::new(e)))?; let limit = limit.map(|old_limit| old_limit - 1); - let mut middle = vec![(first_key, first_data)]; + let mut middle = vec![(first_key.into_vec(), first_data)]; // we stop streaming if either we hit the limit or the key returned was larger // than the largest key requested @@ -1283,8 +1284,9 @@ impl + Send + Sync, T> Merkle { }; // keep going if the key returned is less than the last key requested - ready(kv.0.as_slice() <= last_key.as_ref()) + ready(&*kv.0 <= last_key.as_ref()) }) + .map(|kv_result| kv_result.map(|(k, v)| (k.into_vec(), v))) .try_collect::, Vec)>>() .await?, ); diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index ced26fd72..8fb24bfb1 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -7,20 +7,24 @@ use crate::{ v2::api, }; use futures::{stream::FusedStream, Stream}; -use std::{ops::Not, task::Poll}; +use helper_types::{Either, MustUse}; +use std::task::Poll; + +type Key = Box<[u8]>; +type Value = Vec; enum IteratorState<'a> { /// Start iterating at the specified key - StartAtKey(Vec), + StartAtKey(Key), /// Continue iterating after the given `next_node` and parents Iterating { parents: Vec<(ObjRef<'a>, u8)> }, } impl IteratorState<'_> { fn new() -> Self { - Self::StartAtKey(vec![]) + Self::StartAtKey(vec![].into_boxed_slice()) } - fn with_key(key: Vec) -> Self { + fn with_key(key: Key) -> Self { Self::StartAtKey(key) } } @@ -52,11 +56,7 @@ impl<'a, S, T> MerkleKeyValueStream<'a, S, T> { } } - pub(super) fn from_key( - merkle: &'a Merkle, - merkle_root: DiskAddress, - key: Vec, - ) -> Self { + pub(super) fn from_key(merkle: &'a Merkle, merkle_root: DiskAddress, key: Key) -> Self { let key_state = IteratorState::with_key(key); Self { @@ -68,13 +68,13 @@ impl<'a, S, T> MerkleKeyValueStream<'a, S, T> { } impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<'a, S, T> { - type Item = Result<(Vec, Vec), api::Error>; + type Item = Result<(Key, Value), api::Error>; fn poll_next( mut self: std::pin::Pin<&mut Self>, _cx: &mut std::task::Context<'_>, ) -> Poll> { - let MerkleKeyValueStream { + let Self { key_state, merkle_root, merkle, @@ -126,7 +126,7 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' } } -enum ParentNode<'a> { +enum NodeRef<'a> { New(ObjRef<'a>), Visited(ObjRef<'a>), } @@ -137,7 +137,7 @@ enum InnerNode<'a> { Visited(&'a NodeType), } -impl<'a> ParentNode<'a> { +impl<'a> NodeRef<'a> { fn inner(&self) -> InnerNode<'_> { match self { Self::New(node) => InnerNode::New(node.inner()), @@ -153,22 +153,20 @@ impl<'a> ParentNode<'a> { } } -type NextResult = Option<(Vec, Vec)>; - fn find_next_result<'a, S: ShaleStore, T>( merkle: &'a Merkle, - parents: &mut Vec<(ObjRef<'a>, u8)>, -) -> Result { - let next = find_next_node_with_data(merkle, parents)?.map(|(next_node, value)| { - let node_path_iter = match next_node.inner() { + visited_path: &mut Vec<(ObjRef<'a>, u8)>, +) -> Result, super::MerkleError> { + let next = find_next_node_with_data(merkle, visited_path)?.map(|(next_node, value)| { + let partial_path = match next_node.inner() { NodeType::Leaf(leaf) => leaf.path.iter().copied(), NodeType::Extension(extension) => extension.path.iter().copied(), _ => [].iter().copied(), }; - let key = key_from_nibble_iter(nibble_iter_from_parents(parents).chain(node_path_iter)); + let key = key_from_nibble_iter(nibble_iter_from_parents(visited_path).chain(partial_path)); - parents.push((next_node, 0)); + visited_path.push((next_node, 0)); (key, value) }); @@ -178,15 +176,15 @@ fn find_next_result<'a, S: ShaleStore, T>( fn find_next_node_with_data<'a, S: ShaleStore, T>( merkle: &'a Merkle, - visited_parents: &mut Vec<(ObjRef<'a>, u8)>, + visited_path: &mut Vec<(ObjRef<'a>, u8)>, ) -> Result, Vec)>, super::MerkleError> { use InnerNode::*; - let Some((visited_parent, visited_pos)) = visited_parents.pop() else { + let Some((visited_parent, visited_pos)) = visited_path.pop() else { return Ok(None); }; - let mut node = ParentNode::Visited(visited_parent); + let mut node = NodeRef::Visited(visited_parent); let mut pos = visited_pos; let mut first_loop = true; @@ -198,11 +196,11 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( } Visited(NodeType::Leaf(_)) | Visited(NodeType::Extension(_)) => { - let Some((next_parent, next_pos)) = visited_parents.pop() else { + let Some((next_parent, next_pos)) = visited_path.pop() else { return Ok(None); }; - node = ParentNode::Visited(next_parent); + node = NodeRef::Visited(next_parent); pos = next_pos; } @@ -210,9 +208,9 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( let child = merkle.get_node(extension.chd())?; pos = 0; - visited_parents.push((node.into_node(), pos)); + visited_path.push((node.into_node(), pos)); - node = ParentNode::New(child); + node = NodeRef::New(child); } Visited(NodeType::Branch(branch)) => { @@ -225,10 +223,10 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( let children = get_children_iter(branch) .filter(move |(_, child_pos)| compare_op(child_pos, &pos)); - let next_child_success = - next_child(merkle, children, visited_parents, &mut node, &mut pos)?; + let next_node_success = + next_node(merkle, children, visited_path, &mut node, &mut pos)?; - if !next_child_success { + if !next_node_success { return Ok(None); } } @@ -241,10 +239,10 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( let children = get_children_iter(branch); - let next_child_success = - next_child(merkle, children, visited_parents, &mut node, &mut pos)?; + let next_node_success = + next_node(merkle, children, visited_path, &mut node, &mut pos)?; - if !next_child_success { + if !next_node_success { return Ok(None); } } @@ -262,28 +260,14 @@ fn get_children_iter(branch: &BranchNode) -> impl Iterator(T); - -impl From for MustUse { - fn from(t: T) -> Self { - Self(t) - } -} - -impl Not for MustUse { - type Output = T::Output; - - fn not(self) -> Self::Output { - self.0.not() - } -} - -fn next_child<'a, S, T, Iter>( +/// This function is a little complicated because we need to be able to early return from the parent +/// when we return `false`. `MustUse` forces the caller to check the inner value of `Result::Ok`. +/// It also replaces `node` +fn next_node<'a, S, T, Iter>( merkle: &'a Merkle, mut children: Iter, parents: &mut Vec<(ObjRef<'a>, u8)>, - node: &mut ParentNode<'a>, + node: &mut NodeRef<'a>, pos: &mut u8, ) -> Result, super::MerkleError> where @@ -294,14 +278,14 @@ where let child = merkle.get_node(child_addr)?; *pos = child_pos; - let node = std::mem::replace(node, ParentNode::New(child)); + let node = std::mem::replace(node, NodeRef::New(child)); parents.push((node.into_node(), *pos)); } else { let Some((next_parent, next_pos)) = parents.pop() else { return Ok(false.into()); }; - *node = ParentNode::Visited(next_parent); + *node = NodeRef::Visited(next_parent); *pos = next_pos; } @@ -320,34 +304,60 @@ fn nibble_iter_from_parents<'a>(parents: &'a [(ObjRef, u8)]) -> impl Iterator { - Left(T), - Right(U), +fn key_from_nibble_iter>(mut nibbles: Iter) -> Key { + let mut data = Vec::with_capacity(nibbles.size_hint().0 / 2); + + while let (Some(hi), Some(lo)) = (nibbles.next(), nibbles.next()) { + data.push((hi << 4) + lo); + } + + data.into_boxed_slice() } -impl Iterator for Either -where - T: Iterator, - U: Iterator, -{ - type Item = T::Item; +mod helper_types { + use std::ops::Not; + + /// Enums enable stack-based dynamic-dispatch as opposed to heap-based `Box`. + /// This helps us with match arms that return different types that implement the same trait. + /// It's possible that https://github.com/rust-lang/rust/issues/63065 will make this unnecessary. + /// + /// And this can be replaced by the `either` crate from crates.io if we ever need more functionality. + pub(super) enum Either { + Left(T), + Right(U), + } - fn next(&mut self) -> Option { - match self { - Self::Left(left) => left.next(), - Self::Right(right) => right.next(), + impl Iterator for Either + where + T: Iterator, + U: Iterator, + { + type Item = T::Item; + + fn next(&mut self) -> Option { + match self { + Self::Left(left) => left.next(), + Self::Right(right) => right.next(), + } } } -} -fn key_from_nibble_iter>(mut nibbles: Iter) -> Vec { - let mut data = Vec::with_capacity(nibbles.size_hint().0 / 2); + #[must_use] + pub(super) struct MustUse(T); - while let (Some(hi), Some(lo)) = (nibbles.next(), nibbles.next()) { - data.push((hi << 4) + lo); + impl From for MustUse { + fn from(t: T) -> Self { + Self(t) + } } - data + impl Not for MustUse { + type Output = T::Output; + + fn not(self) -> Self::Output { + self.0.not() + } + } } #[cfg(test)] @@ -363,7 +373,7 @@ mod tests { async fn iterate_empty() { let merkle = create_test_merkle(); let root = merkle.init_root().unwrap(); - let mut it = merkle.iter_from(root, b"x".to_vec()); + let mut it = merkle.iter_from(root, b"x".to_vec().into_boxed_slice()); let next = it.next().await; assert!(next.is_none()); } @@ -383,7 +393,7 @@ mod tests { } let mut it = match start { - Some(start) => merkle.iter_from(root, start.to_vec()), + Some(start) => merkle.iter_from(root, start.to_vec().into_boxed_slice()), None => merkle.iter(root), }; @@ -391,11 +401,11 @@ mod tests { for k in start.map(|r| r[0]).unwrap_or_default()..=u8::MAX { let next = it.next().await.map(|kv| { let (k, v) = kv.unwrap(); - assert_eq!(k, v); + assert_eq!(&*k, &*v); k }); - assert_eq!(next, Some(vec![k])); + assert_eq!(next, Some(vec![k].into_boxed_slice())); } assert!(it.next().await.is_none()); @@ -434,7 +444,7 @@ mod tests { for kv in key_values.iter() { let next = it.next().await.unwrap().unwrap(); - assert_eq!(&next.0, kv); + assert_eq!(&*next.0, &*next.1); assert_eq!(&next.1, kv); } @@ -447,7 +457,7 @@ mod tests { let mut merkle = create_test_merkle(); let root = merkle.init_root().unwrap(); - let key = vec![]; + let key = vec![].into_boxed_slice(); let value = vec![0x00]; merkle.insert(&key, value.clone(), root).unwrap(); @@ -479,17 +489,20 @@ mod tests { assert_eq!( stream.next().await.unwrap().unwrap(), - (branch.to_vec(), branch.to_vec()) + (branch.to_vec().into_boxed_slice(), branch.to_vec()) ); assert_eq!( stream.next().await.unwrap().unwrap(), - (first_leaf.to_vec(), first_leaf.to_vec()) + (first_leaf.to_vec().into_boxed_slice(), first_leaf.to_vec()) ); assert_eq!( stream.next().await.unwrap().unwrap(), - (second_leaf.to_vec(), second_leaf.to_vec()) + ( + second_leaf.to_vec().into_boxed_slice(), + second_leaf.to_vec() + ) ); } @@ -515,19 +528,19 @@ mod tests { merkle.insert(key, key.to_vec(), root).unwrap(); } - let mut stream = merkle.iter_from(root, vec![intermediate]); + let mut stream = merkle.iter_from(root, vec![intermediate].into_boxed_slice()); let first_expected = key_values[1].as_slice(); let first = stream.next().await.unwrap().unwrap(); - assert_eq!(first.0, first.1); - assert_eq!(first.0, first_expected); + assert_eq!(&*first.0, &*first.1); + assert_eq!(first.1, first_expected); let second_expected = key_values[2].as_slice(); let second = stream.next().await.unwrap().unwrap(); - assert_eq!(second.0, second.1); - assert_eq!(second.0, second_expected); + assert_eq!(&*second.0, &*second.1); + assert_eq!(second.1, second_expected); let done = stream.next().await; From 980fed55d9a565615dcb4cd61222b33c92e9f354 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 14 Dec 2023 14:15:09 -0500 Subject: [PATCH 6/9] Rename parents to visited_node_path --- firewood/src/merkle/stream.rs | 299 ++++++++++++++++++++++++++++++---- fwdctl/src/dump.rs | 2 +- 2 files changed, 266 insertions(+), 35 deletions(-) diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index 8fb24bfb1..01d03c918 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -16,9 +16,12 @@ type Value = Vec; enum IteratorState<'a> { /// Start iterating at the specified key StartAtKey(Key), - /// Continue iterating after the given `next_node` and parents - Iterating { parents: Vec<(ObjRef<'a>, u8)> }, + /// Continue iterating after the last node in the `visited_node_path` + Iterating { + visited_node_path: Vec<(ObjRef<'a>, u8)>, + }, } + impl IteratorState<'_> { fn new() -> Self { Self::StartAtKey(vec![].into_boxed_slice()) @@ -38,10 +41,7 @@ pub struct MerkleKeyValueStream<'a, S, T> { impl<'a, S: ShaleStore + Send + Sync, T> FusedStream for MerkleKeyValueStream<'a, S, T> { fn is_terminated(&self) -> bool { - match &self.key_state { - IteratorState::Iterating { parents } if parents.is_empty() => true, - _ => false, - } + matches!(&self.key_state, IteratorState::Iterating { visited_node_path } if visited_node_path.is_empty()) } } @@ -74,6 +74,8 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' mut self: std::pin::Pin<&mut Self>, _cx: &mut std::task::Context<'_>, ) -> Poll> { + // destructuring is necessary here because we need mutable access to `key_state` + // at the same time as immutable access to `merkle` let Self { key_state, merkle_root, @@ -86,9 +88,40 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' .get_node(*merkle_root) .map_err(|e| api::Error::InternalError(Box::new(e)))?; - let (found_node, mut parents) = merkle - .get_node_and_parents_by_key(root_node, &key) - .map_err(|e| api::Error::InternalError(Box::new(e)))?; + // traverse the trie along each nibble until we find a node with a value + // TODO: merkle.iter_by_key(key) will simplify this entire code-block. + let (found_node, mut visited_node_path) = { + let mut visited_node_path = vec![]; + + let found_node = merkle + .get_node_by_key_with_callbacks( + root_node, + &key, + |node_addr, i| visited_node_path.push((node_addr, i)), + |_, _| {}, + ) + .map_err(|e| api::Error::InternalError(Box::new(e)))?; + + let mut visited_node_path = visited_node_path + .into_iter() + .map(|(node, pos)| merkle.get_node(node).map(|node| (node, pos))) + .collect::, _>>() + .map_err(|e| api::Error::InternalError(Box::new(e)))?; + + let last_visited_node_not_branch = visited_node_path + .last() + .map(|(node, _)| { + matches!(node.inner(), NodeType::Leaf(_) | NodeType::Extension(_)) + }) + .unwrap_or_default(); + + // we only want branch in the visited node-path to start + if last_visited_node_not_branch { + visited_node_path.pop(); + } + + (found_node, visited_node_path) + }; if let Some(found_node) = found_node { let value = match found_node.inner() { @@ -103,20 +136,25 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' Ok((std::mem::take(key), value)) }); - parents.push((found_node, 0)); + visited_node_path.push((found_node, 0)); - self.key_state = IteratorState::Iterating { parents }; + self.key_state = IteratorState::Iterating { visited_node_path }; return Poll::Ready(next_result); } - self.key_state = IteratorState::Iterating { parents }; + self.key_state = IteratorState::Iterating { visited_node_path }; self.poll_next(_cx) } - IteratorState::Iterating { parents } => { - let next = find_next_result(merkle, parents) + IteratorState::Iterating { visited_node_path } => { + visited_node_path + .last() + .as_ref() + .map(|(last, pos)| (last.inner(), pos)); + + let next = find_next_result(merkle, visited_node_path) .map_err(|e| api::Error::InternalError(Box::new(e))) .transpose(); @@ -214,8 +252,10 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( } Visited(NodeType::Branch(branch)) => { + // if the first node that we check is a visited branch, that means that the branch had a value + // and we need to visit the first child, for all other cases, we need to visit the next child let compare_op = if first_loop { - ::ge + ::ge // >= } else { ::gt }; @@ -223,10 +263,10 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( let children = get_children_iter(branch) .filter(move |(_, child_pos)| compare_op(child_pos, &pos)); - let next_node_success = + let found_next_node = next_node(merkle, children, visited_path, &mut node, &mut pos)?; - if !next_node_success { + if !found_next_node { return Ok(None); } } @@ -239,10 +279,10 @@ fn find_next_node_with_data<'a, S: ShaleStore, T>( let children = get_children_iter(branch); - let next_node_success = + let found_next_node = next_node(merkle, children, visited_path, &mut node, &mut pos)?; - if !next_node_success { + if !found_next_node { return Ok(None); } } @@ -373,9 +413,8 @@ mod tests { async fn iterate_empty() { let merkle = create_test_merkle(); let root = merkle.init_root().unwrap(); - let mut it = merkle.iter_from(root, b"x".to_vec().into_boxed_slice()); - let next = it.next().await; - assert!(next.is_none()); + let stream = merkle.iter_from(root, b"x".to_vec().into_boxed_slice()); + check_stream_is_done(stream).await; } #[test_case(Some(&[u8::MIN]); "Starting at first key")] @@ -392,14 +431,14 @@ mod tests { merkle.insert([k], vec![k], root).unwrap(); } - let mut it = match start { + let mut stream = match start { Some(start) => merkle.iter_from(root, start.to_vec().into_boxed_slice()), None => merkle.iter(root), }; // we iterate twice because we should get a None then start over for k in start.map(|r| r[0]).unwrap_or_default()..=u8::MAX { - let next = it.next().await.map(|kv| { + let next = stream.next().await.map(|kv| { let (k, v) = kv.unwrap(); assert_eq!(&*k, &*v); k @@ -408,16 +447,14 @@ mod tests { assert_eq!(next, Some(vec![k].into_boxed_slice())); } - assert!(it.next().await.is_none()); + check_stream_is_done(stream).await; } #[tokio::test] async fn fused_empty() { let merkle = create_test_merkle(); let root = merkle.init_root().unwrap(); - let mut it = merkle.iter(root); - assert!(it.next().await.is_none()); - assert!(it.is_terminated()); + check_stream_is_done(merkle.iter(root)).await; } #[tokio::test] @@ -440,16 +477,15 @@ mod tests { merkle.insert(kv, kv.clone(), root).unwrap(); } - let mut it = merkle.iter(root); + let mut stream = merkle.iter(root); for kv in key_values.iter() { - let next = it.next().await.unwrap().unwrap(); + let next = stream.next().await.unwrap().unwrap(); assert_eq!(&*next.0, &*next.1); assert_eq!(&next.1, kv); } - assert!(it.next().await.is_none()); - assert!(it.is_terminated()); + check_stream_is_done(stream).await; } #[tokio::test] @@ -542,8 +578,203 @@ mod tests { assert_eq!(&*second.0, &*second.1); assert_eq!(second.1, second_expected); - let done = stream.next().await; + check_stream_is_done(stream).await; + } + + #[tokio::test] + async fn start_at_key_on_branch_with_no_value() { + let sibling_path = 0x00; + let branch_path = 0x0f; + let children = 0..=0x0f; + + let mut merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + + children.clone().for_each(|child_path| { + let key = vec![sibling_path, child_path]; + + merkle.insert(&key, key.clone(), root).unwrap(); + }); + + let mut keys: Vec<_> = children + .map(|child_path| { + let key = vec![branch_path, child_path]; + + merkle.insert(&key, key.clone(), root).unwrap(); + + key + }) + .collect(); + + keys.sort(); + + let start = keys.iter().position(|key| key[0] == branch_path).unwrap(); + let keys = &keys[start..]; + + let mut stream = merkle.iter_from(root, vec![branch_path].into_boxed_slice()); + + for key in keys { + let next = stream.next().await.unwrap().unwrap(); + + assert_eq!(&*next.0, &*next.1); + assert_eq!(&*next.0, key); + } + + check_stream_is_done(stream).await; + } + + #[tokio::test] + async fn start_at_key_on_branch_with_value() { + let sibling_path = 0x00; + let branch_path = 0x0f; + let branch_key = vec![branch_path]; + + let children = (0..=0xf).map(|val| (val << 4) + val); // 0x00, 0x11, ... 0xff + + let mut merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + + merkle + .insert(&branch_key, branch_key.clone(), root) + .unwrap(); + + children.clone().for_each(|child_path| { + let key = vec![sibling_path, child_path]; + + merkle.insert(&key, key.clone(), root).unwrap(); + }); + + let mut keys: Vec<_> = children + .map(|child_path| { + let key = vec![branch_path, child_path]; + + merkle.insert(&key, key.clone(), root).unwrap(); + + key + }) + .chain(Some(branch_key.clone())) + .collect(); + + keys.sort(); + + let start = keys.iter().position(|key| key == &branch_key).unwrap(); + let keys = &keys[start..]; + + let mut stream = merkle.iter_from(root, branch_key.into_boxed_slice()); + + for key in keys { + let next = stream.next().await.unwrap().unwrap(); + + assert_eq!(&*next.0, &*next.1); + assert_eq!(&*next.0, key); + } + + check_stream_is_done(stream).await; + } + + #[tokio::test] + async fn start_at_key_on_extension() { + let missing = 0x0a; + let children = (0..=0x0f).filter(|x| *x != missing); + let mut merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + + let keys: Vec<_> = children + .map(|child_path| { + let key = vec![child_path]; + + merkle.insert(&key, key.clone(), root).unwrap(); + + key + }) + .collect(); + + let keys = &keys[(missing as usize)..]; + + let mut stream = merkle.iter_from(root, vec![missing].into_boxed_slice()); + + for key in keys { + let next = stream.next().await.unwrap().unwrap(); + + assert_eq!(&*next.0, &*next.1); + assert_eq!(&*next.0, key); + } + + check_stream_is_done(stream).await; + } + + #[tokio::test] + async fn start_at_key_between_siblings() { + let missing = 0xaa; + let children = (0..=0xf) + .map(|val| (val << 4) + val) // 0x00, 0x11, ... 0xff + .filter(|x| *x != missing); + let mut merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + + let keys: Vec<_> = children + .map(|child_path| { + let key = vec![child_path]; + + merkle.insert(&key, key.clone(), root).unwrap(); + + key + }) + .collect(); + + let keys = &keys[((missing >> 4) as usize)..]; - assert!(done.is_none()); + let mut stream = merkle.iter_from(root, vec![missing].into_boxed_slice()); + + for key in keys { + let next = stream.next().await.unwrap().unwrap(); + + assert_eq!(&*next.0, &*next.1); + assert_eq!(&*next.0, key); + } + + check_stream_is_done(stream).await; + } + + // TODO: start key greater than all keys + #[tokio::test] + async fn start_at_key_greater_than_all_others() { + let greatest = 0xff; + let children = (0..=0xf) + .map(|val| (val << 4) + val) // 0x00, 0x11, ... 0xff + .filter(|x| *x != greatest); + let mut merkle = create_test_merkle(); + let root = merkle.init_root().unwrap(); + + let keys: Vec<_> = children + .map(|child_path| { + let key = vec![child_path]; + + merkle.insert(&key, key.clone(), root).unwrap(); + + key + }) + .collect(); + + let keys = &keys[((greatest >> 4) as usize)..]; + + let mut stream = merkle.iter_from(root, vec![greatest].into_boxed_slice()); + + for key in keys { + let next = stream.next().await.unwrap().unwrap(); + + assert_eq!(&*next.0, &*next.1); + assert_eq!(&*next.0, key); + } + + check_stream_is_done(stream).await; + } + + async fn check_stream_is_done(mut stream: S) + where + S: FusedStream + Unpin, + { + assert!(stream.next().await.is_none()); + assert!(stream.is_terminated()); } } diff --git a/fwdctl/src/dump.rs b/fwdctl/src/dump.rs index 3ae12358a..346858993 100644 --- a/fwdctl/src/dump.rs +++ b/fwdctl/src/dump.rs @@ -32,7 +32,7 @@ pub(super) async fn run(opts: &Options) -> Result<(), api::Error> { let db = Db::new(opts.db.clone(), &cfg.build()).await?; let latest_hash = db.root_hash().await?; let latest_rev = db.revision(latest_hash).await?; - let mut stream = latest_rev.stream::>(None)?; + let mut stream = latest_rev.stream(); loop { match stream.next().await { None => break, From 9a7743927555c3bb82a5e0253faf25be45fd8e21 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 14 Dec 2023 19:11:26 -0500 Subject: [PATCH 7/9] Fix doc --- firewood/src/merkle/stream.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index 01d03c918..7276f1ae4 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -359,7 +359,7 @@ mod helper_types { /// Enums enable stack-based dynamic-dispatch as opposed to heap-based `Box`. /// This helps us with match arms that return different types that implement the same trait. - /// It's possible that https://github.com/rust-lang/rust/issues/63065 will make this unnecessary. + /// It's possible that [rust-lang/rust#63065](https://github.com/rust-lang/rust/issues/63065) will make this unnecessary. /// /// And this can be replaced by the `either` crate from crates.io if we ever need more functionality. pub(super) enum Either { From 821a8b168e34803ca44ef7c324ad6208dbe852d9 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 14 Dec 2023 19:18:35 -0500 Subject: [PATCH 8/9] Remove dead code --- firewood/src/merkle/stream.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index 7276f1ae4..366346dff 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -149,11 +149,6 @@ impl<'a, S: ShaleStore + Send + Sync, T> Stream for MerkleKeyValueStream<' } IteratorState::Iterating { visited_node_path } => { - visited_node_path - .last() - .as_ref() - .map(|(last, pos)| (last.inner(), pos)); - let next = find_next_result(merkle, visited_node_path) .map_err(|e| api::Error::InternalError(Box::new(e))) .transpose(); From 5800ccde8a3af4f463dc477cd627d5b2c74448ff Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 14 Dec 2023 19:21:04 -0500 Subject: [PATCH 9/9] Remove TODO that was implemented --- firewood/src/merkle/stream.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/firewood/src/merkle/stream.rs b/firewood/src/merkle/stream.rs index 366346dff..748c3eaf1 100644 --- a/firewood/src/merkle/stream.rs +++ b/firewood/src/merkle/stream.rs @@ -731,7 +731,6 @@ mod tests { check_stream_is_done(stream).await; } - // TODO: start key greater than all keys #[tokio::test] async fn start_at_key_greater_than_all_others() { let greatest = 0xff;