zec
/
incrementalmerkletree
mirror of https://github.com/zcash/incrementalmerkletree.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Address comments from code review.

This commit is contained in:
Kris Nuttycombe 2023-06-16 17:04:05 -06:00
parent cb8ef79648
commit f5889dffa5
6 changed files with 60 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
incrementalmerkletree/Cargo.toml
View File

@ -21,5 +21,10 @@ proptest = { version = "1.0.0", optional = true }
proptest = "1.0.0"
[features]
# The legacy-api feature guards types and functions that were previously
# part of the `zcash_primitives` crate. Those types were removed in the
# `zcash_primitives` 0.12 release and are now maintained here.
legacy-api = []
# The test-depenencies feature guards types and functions that are
# useful for testing incremental Merkle trees and Merkle tree frontiers.
test-dependencies = ["proptest"]

8
incrementalmerkletree/src/frontier.rs
View File

@ -84,9 +84,9 @@ impl<H: Hashable + Clone> NonEmptyFrontier<H> {
let prior_leaf = self.leaf.clone();
self.position += 1;
self.leaf = leaf;
if self.position.is_odd() {
// if the new position is odd, the current leaf will directly become
// an ommer at level 0, and there is no other mutation made to the tree.
if self.position.is_right_child() {
// if the new position is a right-hand leaf, the current leaf will directly become an
// ommer at level 0, and there is no other mutation made to the tree.
self.ommers.insert(0, prior_leaf);
} else {
// if the new position is even, then the current leaf will be hashed
@ -411,7 +411,7 @@ impl<H: Hashable + Clone, const DEPTH: u8> CommitmentTree<H, DEPTH> {
pub fn from_frontier(frontier: &Frontier<H, DEPTH>) -> Self {
frontier.value().map_or_else(Self::empty, |f| {
let mut ommers_iter = f.ommers().iter().cloned();
let (left, right) = if f.position().is_odd() {
let (left, right) = if f.position().is_right_child() {
(
ommers_iter
.next()

10
incrementalmerkletree/src/lib.rs
View File

@ -94,8 +94,9 @@ impl Iterator for WitnessAddrsIter {
pub struct Position(u64);
impl Position {
/// Return whether the position is odd-valued.
pub fn is_odd(&self) -> bool {
/// Return whether the position refers to the right-hand child of a subtree with
/// its root at level 1.
pub fn is_right_child(&self) -> bool {
self.0 & 0x1 == 1
}
@ -424,6 +425,11 @@ impl Address {
}
}
/// Returns whether this address is the right-hand child of its parent
pub fn is_left_child(&self) -> bool {
self.index & 0x1 == 0
}
/// Returns whether this address is the right-hand child of its parent
pub fn is_right_child(&self) -> bool {
self.index & 0x1 == 1

8
incrementalmerkletree/src/testing.rs
View File

@ -1061,7 +1061,9 @@ pub fn check_remove_mark<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usize) ->
}
}
pub fn check_rewind_remove_mark<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usize) -> T>(new_tree: F) {
pub fn check_rewind_remove_mark<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usize) -> T>(
new_tree: F,
) {
// rewinding doesn't remove a mark
let mut tree = new_tree(100);
tree.append("e".to_string(), Retention::Marked);
@ -1139,7 +1141,9 @@ pub fn check_rewind_remove_mark<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usi
}
}
pub fn check_witness_consistency<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usize) -> T>(new_tree: F) {
pub fn check_witness_consistency<C: TestCheckpoint, T: Tree<String, C>, F: Fn(usize) -> T>(
new_tree: F,
) {
let samples = vec![
// Reduced examples
vec![

5
shardtree/Cargo.toml
View File

@ -24,7 +24,12 @@ incrementalmerkletree = { version = "0.4", path = "../incrementalmerkletree", fe
proptest = "1.0.0"
[features]
# The legacy-api feature guards types and functions that are useful for
# migrating data previously managed using `incrementalmerkletree/legacy-api`
# types into the `ShardTree` data structure.
legacy-api = ["incrementalmerkletree/legacy-api"]
# The test-depenencies feature can be enabled to expose types and functions
# that are useful for testing `shardtree` functionality.
test-dependencies = ["proptest"]
[target.'cfg(unix)'.dev-dependencies]

75
shardtree/src/lib.rs
View File

@ -1,6 +1,6 @@
use bitflags::bitflags;
use core::convert::TryFrom;
use core::fmt::{self, Debug};
use core::fmt::{self, Debug, Display};
use core::ops::{Deref, Range};
use either::Either;
use std::collections::{BTreeMap, BTreeSet};
@ -722,11 +722,7 @@ impl fmt::Display for InsertionError {
}
}
impl std::error::Error for InsertionError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
None
}
}
impl std::error::Error for InsertionError {}
/// Errors that may be returned in the process of querying a [`ShardTree`]
#[derive(Clone, Debug, PartialEq, Eq)]
@ -765,11 +761,7 @@ impl fmt::Display for QueryError {
}
}
impl std::error::Error for QueryError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
None
}
}
impl std::error::Error for QueryError {}
/// Operations on [`LocatedTree`]s that are annotated with Merkle hashes.
impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
@ -871,7 +863,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
} else {
// we handle the level 0 leaves here by adding the sibling of our desired
// leaf to the witness
if position.is_odd() {
if position.is_right_child() {
if right.is_marked_leaf() {
left.leaf_value()
.map(|v| vec![v.clone()])
@ -1345,9 +1337,9 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
}),
};
if position.is_odd() {
// At odd positions, we are completing a subtree and so we unite fragments
// up the stack until we get the largest possible subtree
if position.is_right_child() {
// At right-hand positions, we are completing a subtree and so we unite
// fragments up the stack until we get the largest possible subtree
while let Some((potential_sibling, marked)) = fragments.pop() {
if potential_sibling.root_addr.parent() == subtree.root_addr.parent() {
subtree = unite(potential_sibling, subtree, prune_below);
@ -1416,11 +1408,10 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
}
}
// Constructs a pair of trees contains the leaf and ommers of the given frontier.
// The first element of the result is a tree with its root at a level less than
// or equal to `split_at`; the second element is a tree with its leaves at level
// `split_at` that is only returned if the frontier contains sufficient data to
// fill the first tree to the `split_at` level.
// Constructs a pair of trees that contain the leaf and ommers of the given frontier. The first
// element of the result is a tree with its root at a level less than or equal to `split_at`;
// the second element is a tree with its leaves at level `split_at` that is only returned if
// the frontier contains sufficient data to fill the first tree to the `split_at` level.
fn from_frontier<C>(
frontier: NonEmptyFrontier<H>,
leaf_retention: &Retention<C>,
@ -1446,7 +1437,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
});
while addr.level() < split_at {
if addr.index() & 0x1 == 0 {
if addr.is_left_child() {
// the current address is a left child, so create a parent with
// an empty right-hand tree
subtree = Tree::parent(None, subtree, Tree::empty());
@ -1472,7 +1463,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
let mut supertree = None;
for left in ommers {
// build up the left-biased tree until we get a right-hand node
while addr.index() & 0x1 == 0 {
while addr.is_left_child() {
supertree = supertree.map(|t| Tree::parent(None, t, Tree::empty()));
addr = addr.parent();
}
@ -1511,7 +1502,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
let mut addr = leaf_addr;
let mut subtree = Tree::empty();
while addr.level() < split_at {
if addr.index() & 0x1 == 0 {
if addr.is_left_child() {
// the current root is a left child, so take the right sibling from the
// filled iterator
if let Some(right) = filled.next() {
@ -1543,7 +1534,7 @@ impl<H: Hashable + Clone + PartialEq> LocatedPrunableTree<H> {
let mut supertree = None;
for right in filled {
// build up the right-biased tree until we get a left-hand node
while addr.index() & 0x1 == 1 {
while addr.is_right_child() {
supertree = supertree.map(|t| Tree::parent(None, Tree::empty(), t));
addr = addr.parent();
}
@ -1818,12 +1809,10 @@ pub trait ShardStore {
/// provided has level `SHARD_HEIGHT`.
fn truncate(&mut self, from: Address) -> Result<(), Self::Error>;
/// A tree that is used to cache the known roots of subtrees in the "cap" of nodes between
/// `SHARD_HEIGHT` and `DEPTH` that are otherwise not directly represented in the tree.
///
/// This tree only contains data that is expected to be stable; i.e. any node whose value
/// might be altered as a consequence of the discovery of additional information from
/// branches of the tree will be `Nil` in this cache.
/// A tree that is used to cache the known roots of subtrees in the "cap" - the top part of the
/// tree, which contains parent nodes produced by hashing the roots of the individual shards.
/// Nodes in the cap have levels in the range `SHARD_HEIGHT..DEPTH`. Note that the cap may be
/// sparse, in the same way that individual shards may be sparse.
fn get_cap(&self) -> Result<PrunableTree<Self::H>, Self::Error>;
/// Persists the provided cap to the data store.
@ -2176,12 +2165,8 @@ impl<S> From<InsertionError> for ShardTreeError<S> {
impl<S: fmt::Display> fmt::Display for ShardTreeError<S> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match &self {
ShardTreeError::Query(q) => {
write!(f, "{}", q)
}
ShardTreeError::Insert(i) => {
write!(f, "{}", i)
}
ShardTreeError::Query(q) => Display::fmt(&q, f),
ShardTreeError::Insert(i) => Display::fmt(&i, f),
ShardTreeError::Storage(s) => {
write!(
f,
@ -2749,16 +2734,15 @@ impl<
&mut self,
checkpoint_id: &C,
) -> Result<bool, ShardTreeError<S::Error>> {
match self
if let Some(c) = self
.store
.get_checkpoint(checkpoint_id)
.map_err(ShardTreeError::Storage)?
{
Some(c) => {
self.truncate_removing_checkpoint_internal(checkpoint_id, &c)?;
Ok(true)
}
None => Ok(false),
self.truncate_removing_checkpoint_internal(checkpoint_id, &c)?;
Ok(true)
} else {
Ok(false)
}
}
@ -3221,6 +3205,11 @@ impl<
}
}
/// Computes the witness for the leaf at the specified position.
///
/// This implementation will mutate the tree to cache intermediate root (ommer) values that are
/// computed in the process of constructing the witness, so as to avoid the need to recompute
/// those values from potentially large numbers of subtree roots in the future.
pub fn witness_caching(
&mut self,
position: Position,
@ -3499,7 +3488,7 @@ pub mod testing {
}
fn marked_positions(&self) -> BTreeSet<Position> {
match ShardTree::marked_positions(&self) {
match ShardTree::marked_positions(self) {
Ok(v) => v,
Err(err) => panic!("marked positions query failed: {:?}", err),
}