diff --git a/Cargo.toml b/Cargo.toml index 0ddd568e4..1aeb9090c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,3 +21,6 @@ codegen-units = 1 [patch.crates-io] zcash_encoding = { path = "components/zcash_encoding" } zcash_note_encryption = { path = "components/zcash_note_encryption" } +bridgetree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "ea1686e8f8f6c1e41aa97251a7eb4fadfd33df47" } +incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "ea1686e8f8f6c1e41aa97251a7eb4fadfd33df47" } +orchard = { git = "https://github.com/zcash/orchard.git", rev = "8bc53ecbde0944f59f2321f06f2f4171975c7288" } diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index a0403b3e8..31bdbbb0f 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -7,6 +7,23 @@ and this library adheres to Rust's notion of ## [Unreleased] +### Removed +- `merkle_tree::Hashable` has been removed and its uses have been replaced by + `incrementalmerkletree::Hashable` and `merkle_tree::HashSer`. +- The `Hashable` bound on the `Node` parameter to the `IncrementalWitness` + type has been removed. +- `merkle_tree::incremental::write_auth_fragment_v1` + +### Added +- `merkle_tree::incremental::{read_address, write_address}` +- `merkle_tree::incremental::read_bridge_v2` + +### Changed +- The bounds on the `H` parameter to the following methods have changed: + - `merkle_tree::incremental::read_frontier_v0` + - `merkle_tree::incremental::read_auth_fragment_v1` + + ## [0.11.0] - 2023-04-15 ### Added - `zcash_primitives::zip32::fingerprint` module, containing types for deriving diff --git a/zcash_primitives/Cargo.toml b/zcash_primitives/Cargo.toml index 7c2f9e4c5..0356e4426 100644 --- a/zcash_primitives/Cargo.toml +++ b/zcash_primitives/Cargo.toml @@ -42,11 +42,14 @@ subtle = "2.2.3" bls12_381 = "0.8" ff = "0.13" group = { version = "0.13", features = ["wnaf-memuse"] } -incrementalmerkletree = "0.3" jubjub = "0.10" nonempty = "0.7" orchard = { version = "0.4", default-features = false } +# - Note Commitment Trees +bridgetree = "0.2" +incrementalmerkletree = "0.3" + # - Static constants lazy_static = "1" @@ -86,6 +89,7 @@ features = ["pre-zip-212"] [dev-dependencies] chacha20poly1305 = "0.10" criterion = "0.4" +incrementalmerkletree = { version = "0.3", features = ["test-dependencies"] } proptest = "1.0.0" assert_matches = "1.3.0" rand_xorshift = "0.3" diff --git a/zcash_primitives/proptest-regressions/merkle_tree.txt b/zcash_primitives/proptest-regressions/merkle_tree.txt new file mode 100644 index 000000000..d7e77e33c --- /dev/null +++ b/zcash_primitives/proptest-regressions/merkle_tree.txt @@ -0,0 +1,7 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +cc ad8649940f45fef5aad3355d7ca193e7d018285b04f9d976197ff8e1fec8687a # shrinks to ct = CommitmentTree { left: Some(Node { repr: [31, 136, 59, 216, 239, 236, 65, 132, 70, 226, 212, 35, 165, 194, 10, 57, 188, 94, 141, 22, 236, 68, 38, 57, 219, 28, 160, 173, 156, 62, 220, 34] }), right: Some(Node { repr: [117, 7, 10, 72, 207, 40, 193, 79, 126, 156, 158, 193, 63, 190, 49, 179, 227, 188, 144, 85, 30, 143, 186, 203, 154, 76, 5, 113, 42, 16, 60, 41] }), parents: [Some(Node { repr: [92, 253, 34, 29, 13, 202, 250, 201, 54, 210, 88, 207, 183, 117, 166, 126, 63, 101, 34, 44, 119, 88, 122, 240, 51, 77, 53, 148, 190, 141, 226, 103] }), Some(Node { repr: [91, 140, 162, 31, 9, 135, 96, 209, 170, 45, 143, 251, 108, 37, 94, 84, 95, 22, 28, 109, 140, 61, 249, 41, 220, 207, 149, 136, 93, 220, 161, 87] }), Some(Node { repr: [21, 173, 87, 125, 78, 242, 185, 197, 174, 47, 114, 229, 189, 35, 154, 221, 164, 232, 181, 26, 232, 216, 228, 244, 81, 127, 222, 10, 22, 241, 134, 106] }), None, Some(Node { repr: [88, 145, 84, 46, 146, 135, 252, 5, 21, 50, 137, 85, 36, 136, 101, 55, 153, 134, 71, 41, 95, 73, 17, 151, 170, 141, 195, 95, 11, 204, 181, 85] }), None, None] } diff --git a/zcash_primitives/src/merkle_tree.rs b/zcash_primitives/src/merkle_tree.rs index 2e55cab39..7a3dd48b7 100644 --- a/zcash_primitives/src/merkle_tree.rs +++ b/zcash_primitives/src/merkle_tree.rs @@ -1,38 +1,17 @@ //! Implementation of a Merkle tree of commitments used to prove the existence of notes. use byteorder::{LittleEndian, ReadBytesExt}; -use incrementalmerkletree::{ - self, - bridgetree::{self, Leaf}, - Altitude, -}; +use incrementalmerkletree::Hashable; use std::collections::VecDeque; +use std::convert::TryInto; use std::io::{self, Read, Write}; use std::iter::repeat; use zcash_encoding::{Optional, Vector}; -use crate::sapling::SAPLING_COMMITMENT_TREE_DEPTH; +use crate::sapling::SAPLING_COMMITMENT_TREE_DEPTH_U8; pub mod incremental; -/// A hashable node within a Merkle tree. -pub trait Hashable: Clone + Copy { - /// Parses a node from the given byte source. - fn read(reader: R) -> io::Result; - - /// Serializes this node. - fn write(&self, writer: W) -> io::Result<()>; - - /// Returns the parent node within the tree of the two given nodes. - fn combine(_: usize, _: &Self, _: &Self) -> Self; - - /// Returns a blank leaf node. - fn blank() -> Self; - - /// Returns the empty root for the given depth. - fn empty_root(_: usize) -> Self; -} - /// A hashable node within a Merkle tree. pub trait HashSer { /// Parses a node from the given byte source. @@ -44,45 +23,7 @@ pub trait HashSer { fn write(&self, writer: W) -> io::Result<()>; } -impl Hashable for T -where - T: incrementalmerkletree::Hashable + HashSer + Copy, -{ - /// Parses a node from the given byte source. - fn read(reader: R) -> io::Result { - ::read(reader) - } - - /// Serializes this node. - fn write(&self, writer: W) -> io::Result<()> { - ::write(self, writer) - } - - /// Returns the parent node within the tree of the two given nodes. - fn combine(alt: usize, lhs: &Self, rhs: &Self) -> Self { - ::combine( - Altitude::from( - u8::try_from(alt).expect("Tree heights greater than 255 are unsupported."), - ), - lhs, - rhs, - ) - } - - /// Returns a blank leaf node. - fn blank() -> Self { - ::empty_leaf() - } - - /// Returns the empty root for the given depth. - fn empty_root(alt: usize) -> Self { - ::empty_root(Altitude::from( - u8::try_from(alt).expect("Tree heights greater than 255 are unsupported."), - )) - } -} - -struct PathFiller { +struct PathFiller { queue: VecDeque, } @@ -93,10 +34,10 @@ impl PathFiller { } } - fn next(&mut self, depth: usize) -> Node { + fn next(&mut self, depth: u8) -> Node { self.queue .pop_front() - .unwrap_or_else(|| Node::empty_root(depth)) + .unwrap_or_else(|| Node::empty_root(depth.into())) } } @@ -126,16 +67,23 @@ impl CommitmentTree { Node: Clone, { frontier.value().map_or_else(Self::empty, |f| { - let (left, right) = match f.leaf() { - Leaf::Left(v) => (Some(v.clone()), None), - Leaf::Right(l, r) => (Some(l.clone()), Some(r.clone())), - }; let mut ommers_iter = f.ommers().iter().cloned(); + let (left, right) = if f.position().is_odd() { + ( + ommers_iter + .next() + .expect("An ommer must exist if the frontier position is odd"), + Some(f.leaf().clone()), + ) + } else { + (f.leaf().clone(), None) + }; + let upos: usize = f.position().into(); Self { - left, + left: Some(left), right, - parents: (1..DEPTH) + parents: (1u8..DEPTH) .into_iter() .map(|i| { if upos & (1 << i) == 0 { @@ -151,24 +99,21 @@ impl CommitmentTree { pub fn to_frontier(&self) -> bridgetree::Frontier where - Node: incrementalmerkletree::Hashable + Clone, + Node: Hashable + Clone, { if self.size() == 0 { bridgetree::Frontier::empty() } else { - let leaf = match (self.left.as_ref(), self.right.as_ref()) { - (Some(a), None) => bridgetree::Leaf::Left(a.clone()), - (Some(a), Some(b)) => bridgetree::Leaf::Right(a.clone(), b.clone()), + let ommers_iter = self.parents.iter().filter_map(|v| v.as_ref()).cloned(); + let (leaf, ommers) = match (self.left.as_ref(), self.right.as_ref()) { + (Some(a), None) => (a.clone(), ommers_iter.collect()), + (Some(a), Some(b)) => ( + b.clone(), + Some(a.clone()).into_iter().chain(ommers_iter).collect(), + ), _ => unreachable!(), }; - let ommers = self - .parents - .iter() - .filter_map(|v| v.as_ref()) - .cloned() - .collect(); - // If a frontier cannot be successfully constructed from the // parts of a commitment tree, it is a programming error. bridgetree::Frontier::from_parts((self.size() - 1).into(), leaf, ommers) @@ -193,7 +138,7 @@ impl CommitmentTree { ) } - fn is_complete(&self, depth: usize) -> bool { + fn is_complete(&self, depth: u8) -> bool { if depth == 0 { self.left.is_some() && self.right.is_none() && self.parents.is_empty() } else { @@ -203,13 +148,13 @@ impl CommitmentTree { .parents .iter() .chain(repeat(&None)) - .take(depth - 1) + .take((depth - 1).into()) .all(|p| p.is_some()) } } } -impl CommitmentTree { +impl CommitmentTree { /// Reads a `CommitmentTree` from its serialized form. pub fn read(mut reader: R) -> io::Result { let left = Optional::read(&mut reader, Node::read)?; @@ -225,41 +170,44 @@ impl CommitmentTree { /// Serializes this tree as an array of bytes. pub fn write(&self, mut writer: W) -> io::Result<()> { - Optional::write(&mut writer, self.left, |w, n| n.write(w))?; - Optional::write(&mut writer, self.right, |w, n| n.write(w))?; + Optional::write(&mut writer, self.left.as_ref(), |w, n| n.write(w))?; + Optional::write(&mut writer, self.right.as_ref(), |w, n| n.write(w))?; Vector::write(&mut writer, &self.parents, |w, e| { - Optional::write(w, *e, |w, n| n.write(w)) + Optional::write(w, e.as_ref(), |w, n| n.write(w)) }) } +} +impl CommitmentTree { /// Adds a leaf node to the tree. /// /// Returns an error if the tree is full. pub fn append(&mut self, node: Node) -> Result<(), ()> { - self.append_inner(node, SAPLING_COMMITMENT_TREE_DEPTH) + self.append_inner(node, SAPLING_COMMITMENT_TREE_DEPTH_U8) } - fn append_inner(&mut self, node: Node, depth: usize) -> Result<(), ()> { + fn append_inner(&mut self, node: Node, depth: u8) -> Result<(), ()> { if self.is_complete(depth) { // Tree is full return Err(()); } - match (self.left, self.right) { + match (&self.left, &self.right) { (None, _) => self.left = Some(node), (_, None) => self.right = Some(node), (Some(l), Some(r)) => { - let mut combined = Node::combine(0, &l, &r); + let mut combined = Node::combine(0.into(), l, r); self.left = Some(node); self.right = None; for i in 0..depth { - if i < self.parents.len() { - if let Some(p) = self.parents[i] { - combined = Node::combine(i + 1, &p, &combined); - self.parents[i] = None; + let i_usize = usize::from(i); + if i_usize < self.parents.len() { + if let Some(p) = &self.parents[i_usize] { + combined = Node::combine((i + 1).into(), p, &combined); + self.parents[i_usize] = None; } else { - self.parents[i] = Some(combined); + self.parents[i_usize] = Some(combined); break; } } else { @@ -275,18 +223,24 @@ impl CommitmentTree { /// Returns the current root of the tree. pub fn root(&self) -> Node { - self.root_inner(SAPLING_COMMITMENT_TREE_DEPTH, PathFiller::empty()) + self.root_inner(SAPLING_COMMITMENT_TREE_DEPTH_U8, PathFiller::empty()) } - fn root_inner(&self, depth: usize, mut filler: PathFiller) -> Node { + fn root_inner(&self, depth: u8, mut filler: PathFiller) -> Node { assert!(depth > 0); // 1) Hash left and right leaves together. // - Empty leaves are used as needed. let leaf_root = Node::combine( - 0, - &self.left.unwrap_or_else(|| filler.next(0)), - &self.right.unwrap_or_else(|| filler.next(0)), + 0.into(), + &self + .left + .as_ref() + .map_or_else(|| filler.next(0), |n| n.clone()), + &self + .right + .as_ref() + .map_or_else(|| filler.next(0), |n| n.clone()), ); // 2) Extend the parents to the desired depth with None values, then hash from leaf to @@ -294,11 +248,14 @@ impl CommitmentTree { self.parents .iter() .chain(repeat(&None)) - .take(depth - 1) - .enumerate() - .fold(leaf_root, |root, (i, p)| match p { - Some(node) => Node::combine(i + 1, node, &root), - None => Node::combine(i + 1, &root, &filler.next(i + 1)), + .take((depth - 1).into()) + .zip(0u8..) + .fold(leaf_root, |root, (p, i)| { + let parent_level = i + 1; + match p { + Some(node) => Node::combine(parent_level.into(), node, &root), + None => Node::combine(parent_level.into(), &root, &filler.next(parent_level)), + } }) } } @@ -334,15 +291,15 @@ impl CommitmentTree { /// witness.append(cmu); /// assert_eq!(tree.root(), witness.root()); /// ``` -#[derive(Clone)] -pub struct IncrementalWitness { +#[derive(Clone, Debug)] +pub struct IncrementalWitness { tree: CommitmentTree, filled: Vec, - cursor_depth: usize, + cursor_depth: u8, cursor: Option>, } -impl IncrementalWitness { +impl IncrementalWitness { /// Creates an `IncrementalWitness` for the most recent commitment added to the given /// [`CommitmentTree`]. pub fn from_tree(tree: &CommitmentTree) -> IncrementalWitness { @@ -397,8 +354,12 @@ impl IncrementalWitness { } /// Finds the next "depth" of an unfilled subtree. - fn next_depth(&self) -> usize { - let mut skip = self.filled.len(); + fn next_depth(&self) -> u8 { + let mut skip: u8 = self + .filled + .len() + .try_into() + .expect("Merkle tree depths may not exceed the bounds of a u8"); if self.tree.left.is_none() { if skip > 0 { @@ -435,10 +396,10 @@ impl IncrementalWitness { /// /// Returns an error if the tree is full. pub fn append(&mut self, node: Node) -> Result<(), ()> { - self.append_inner(node, SAPLING_COMMITMENT_TREE_DEPTH) + self.append_inner(node, SAPLING_COMMITMENT_TREE_DEPTH_U8) } - fn append_inner(&mut self, node: Node, depth: usize) -> Result<(), ()> { + fn append_inner(&mut self, node: Node, depth: u8) -> Result<(), ()> { if let Some(mut cursor) = self.cursor.take() { cursor .append_inner(node, depth) @@ -472,25 +433,25 @@ impl IncrementalWitness { /// Returns the current root of the tree corresponding to the witness. pub fn root(&self) -> Node { - self.root_inner(SAPLING_COMMITMENT_TREE_DEPTH) + self.root_inner(SAPLING_COMMITMENT_TREE_DEPTH_U8) } - fn root_inner(&self, depth: usize) -> Node { + fn root_inner(&self, depth: u8) -> Node { self.tree.root_inner(depth, self.filler()) } /// Returns the current witness, or None if the tree is empty. pub fn path(&self) -> Option> { - self.path_inner(SAPLING_COMMITMENT_TREE_DEPTH) + self.path_inner(SAPLING_COMMITMENT_TREE_DEPTH_U8) } - fn path_inner(&self, depth: usize) -> Option> { + fn path_inner(&self, depth: u8) -> Option> { let mut filler = self.filler(); let mut auth_path = Vec::new(); - if let Some(node) = self.tree.left { + if let Some(node) = &self.tree.left { if self.tree.right.is_some() { - auth_path.push((node, true)); + auth_path.push((node.clone(), true)); } else { auth_path.push((filler.next(0), false)); } @@ -499,21 +460,21 @@ impl IncrementalWitness { return None; } - for (i, p) in self + for (p, i) in self .tree .parents .iter() .chain(repeat(&None)) - .take(depth - 1) - .enumerate() + .take((depth - 1).into()) + .zip(0u8..) { auth_path.push(match p { - Some(node) => (*node, true), + Some(node) => (node.clone(), true), None => (filler.next(i + 1), false), }); } - assert_eq!(auth_path.len(), depth); + assert_eq!(auth_path.len(), depth.into()); Some(MerklePath::from_path(auth_path, self.position() as u64)) } @@ -526,7 +487,7 @@ pub struct MerklePath { pub position: u64, } -impl MerklePath { +impl MerklePath { /// Constructs a Merkle path directly from a path and position. pub fn from_path(auth_path: Vec<(Node, bool)>, position: u64) -> Self { MerklePath { @@ -537,13 +498,13 @@ impl MerklePath { /// Reads a Merkle path from its serialized form. pub fn from_slice(witness: &[u8]) -> Result { - Self::from_slice_with_depth(witness, SAPLING_COMMITMENT_TREE_DEPTH) + Self::from_slice_with_depth(witness, SAPLING_COMMITMENT_TREE_DEPTH_U8) } - fn from_slice_with_depth(mut witness: &[u8], depth: usize) -> Result { + fn from_slice_with_depth(mut witness: &[u8], depth: u8) -> Result { // Skip the first byte, which should be "depth" to signify the length of // the following vector of Pedersen hashes. - if witness[0] != depth as u8 { + if witness[0] != depth { return Err(()); } witness = &witness[1..]; @@ -572,7 +533,7 @@ impl MerklePath { } }) .collect::, _>>()?; - if auth_path.len() != depth { + if auth_path.len() != depth.into() { return Err(()); } @@ -604,12 +565,12 @@ impl MerklePath { pub fn root(&self, leaf: Node) -> Node { self.auth_path .iter() - .enumerate() + .zip(0u8..) .fold( leaf, - |root, (i, (p, leaf_is_on_right))| match leaf_is_on_right { - false => Node::combine(i, &root, p), - true => Node::combine(i, p, &root), + |root, ((p, leaf_is_on_right), i)| match leaf_is_on_right { + false => Node::combine(i.into(), &root, p), + true => Node::combine(i.into(), p, &root), }, ) } @@ -617,15 +578,18 @@ impl MerklePath { #[cfg(test)] mod tests { - use incrementalmerkletree::bridgetree::Frontier; + use assert_matches::assert_matches; + use bridgetree::Frontier; + use incrementalmerkletree::Hashable; use proptest::prelude::*; + use proptest::strategy::Strategy; use std::io::{self, Read, Write}; use crate::sapling::{testing::arb_node, Node}; use super::{ testing::{arb_commitment_tree, TestNode}, - CommitmentTree, Hashable, IncrementalWitness, MerklePath, PathFiller, + CommitmentTree, HashSer, IncrementalWitness, MerklePath, PathFiller, }; const HEX_EMPTY_ROOTS: [&str; 33] = [ @@ -664,8 +628,9 @@ mod tests { "fbc2f4300c01f0b7820d00e3347c8da4ee614674376cbc45359daa54f9b5493e", ]; - const TESTING_DEPTH: usize = 4; + const TESTING_DEPTH: u8 = 4; + #[derive(Debug)] struct TestCommitmentTree(CommitmentTree); impl TestCommitmentTree { @@ -695,6 +660,7 @@ mod tests { } } + #[derive(Debug)] struct TestIncrementalWitness(IncrementalWitness); impl TestIncrementalWitness { @@ -727,8 +693,8 @@ mod tests { #[test] fn empty_root_test_vectors() { let mut tmp = [0u8; 32]; - for (i, &expected) in HEX_EMPTY_ROOTS.iter().enumerate() { - Node::empty_root(i) + for (&expected, i) in HEX_EMPTY_ROOTS.iter().zip(0u8..) { + Node::empty_root(i.into()) .write(&mut tmp[..]) .expect("length is 32 bytes"); assert_eq!(hex::encode(tmp), expected); @@ -752,7 +718,7 @@ mod tests { fn empty_commitment_tree_roots() { let tree = CommitmentTree::::empty(); let mut tmp = [0u8; 32]; - for (i, &expected) in HEX_EMPTY_ROOTS.iter().enumerate().skip(1) { + for (&expected, i) in HEX_EMPTY_ROOTS.iter().zip(0u8..).skip(1) { tree.root_inner(i, PathFiller::empty()) .write(&mut tmp[..]) .expect("length is 32 bytes"); @@ -1159,6 +1125,7 @@ mod tests { ) .unwrap(); assert_eq!(path, expected); + assert_eq!(path.root(*leaf), witness.root()); paths_i += 1; } else { @@ -1177,22 +1144,61 @@ mod tests { } // Tree should be full now - let node = Node::blank(); + let node = Node::empty_leaf(); assert!(tree.append(node).is_err()); for (witness, _) in witnesses.as_mut_slice() { assert!(witness.append(node).is_err()); } } + #[test] + fn test_commitment_tree_roundtrip() { + let ct = CommitmentTree { + left: Some("a".to_string()), + right: Some("b".to_string()), + parents: vec![ + Some("c".to_string()), + Some("d".to_string()), + Some("e".to_string()), + Some("f".to_string()), + None, + None, + None, + ], + }; + + let frontier: Frontier = ct.to_frontier(); + let ct0 = CommitmentTree::from_frontier(&frontier); + assert_eq!(ct, ct0); + let frontier0: Frontier = ct0.to_frontier(); + assert_eq!(frontier, frontier0); + } + proptest! { #[test] - fn prop_commitment_tree_roundtrip(ct in arb_commitment_tree(32, arb_node(), 8)) { + fn prop_commitment_tree_roundtrip_str(ct in arb_commitment_tree(32, any::().prop_map(|c| c.to_string()), 8)) { + let frontier: Frontier = ct.to_frontier(); + let ct0 = CommitmentTree::from_frontier(&frontier); + assert_eq!(ct, ct0); + let frontier0: Frontier = ct0.to_frontier(); + assert_eq!(frontier, frontier0); + } + + #[test] + fn prop_commitment_tree_roundtrip_node(ct in arb_commitment_tree(32, arb_node(), 8)) { let frontier: Frontier = ct.to_frontier(); let ct0 = CommitmentTree::from_frontier(&frontier); assert_eq!(ct, ct0); let frontier0: Frontier = ct0.to_frontier(); assert_eq!(frontier, frontier0); } + + #[test] + fn prop_commitment_tree_roundtrip_ser(ct in arb_commitment_tree(32, arb_node(), 8)) { + let mut serialized = vec![]; + assert_matches!(ct.write(&mut serialized), Ok(())); + assert_matches!(CommitmentTree::read(&serialized[..]), Ok(ct_out) if ct == ct_out); + } } #[test] @@ -1223,16 +1229,16 @@ mod tests { pub mod testing { use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use core::fmt::Debug; - use incrementalmerkletree::{self, Altitude}; + use incrementalmerkletree::{Hashable, Level}; use proptest::collection::vec; use proptest::prelude::*; use std::collections::hash_map::DefaultHasher; use std::hash::Hasher; use std::io::{self, Read, Write}; - use super::{CommitmentTree, HashSer, Hashable}; + use super::{CommitmentTree, HashSer}; - pub fn arb_commitment_tree>( + pub fn arb_commitment_tree>( min_size: usize, arb_node: T, depth: u8, @@ -1249,12 +1255,12 @@ pub mod testing { } #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] - pub(crate) struct TestNode(pub(crate) u64); + pub struct TestNode(pub u64); impl incrementalmerkletree::Hashable for TestNode { - fn combine(alt: Altitude, a: &TestNode, b: &TestNode) -> TestNode { + fn combine(level: Level, a: &TestNode, b: &TestNode) -> TestNode { let mut hasher = DefaultHasher::new(); - hasher.write_u8(alt.into()); + hasher.write_u8(level.into()); hasher.write_u64(a.0); hasher.write_u64(b.0); TestNode(hasher.finish()) @@ -1274,4 +1280,10 @@ pub mod testing { writer.write_u64::(self.0) } } + + prop_compose! { + pub fn arb_test_node()(i in any::()) -> TestNode { + TestNode(i) + } + } } diff --git a/zcash_primitives/src/merkle_tree/incremental.rs b/zcash_primitives/src/merkle_tree/incremental.rs index b70718b50..e7efa4a1a 100644 --- a/zcash_primitives/src/merkle_tree/incremental.rs +++ b/zcash_primitives/src/merkle_tree/incremental.rs @@ -1,12 +1,11 @@ //! Implementations of serialization and parsing for Orchard note commitment trees. use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; +use std::collections::{BTreeMap, BTreeSet}; use std::io::{self, Read, Write}; -use incrementalmerkletree::{ - bridgetree::{AuthFragment, Frontier, Leaf, MerkleBridge, NonEmptyFrontier}, - Hashable, Position, -}; +use bridgetree::{Frontier, MerkleBridge, NonEmptyFrontier}; +use incrementalmerkletree::{Address, Hashable, Level, Position}; use orchard::tree::MerkleHashOrchard; use zcash_encoding::{Optional, Vector}; @@ -66,7 +65,18 @@ pub fn read_position(mut reader: R) -> io::Result { read_leu64_usize(&mut reader).map(Position::from) } -pub fn read_frontier_v0( +pub fn write_address(mut writer: W, addr: Address) -> io::Result<()> { + writer.write_u8(addr.level().into())?; + write_usize_leu64(&mut writer, addr.index()) +} + +pub fn read_address(mut reader: R) -> io::Result
{ + let level = reader.read_u8().map(Level::from)?; + let index = read_leu64_usize(&mut reader)?; + Ok(Address::from_parts(level, index)) +} + +pub fn read_frontier_v0( mut reader: R, ) -> io::Result> { let tree = CommitmentTree::read(&mut reader)?; @@ -79,17 +89,21 @@ pub fn write_nonempty_frontier_v1( frontier: &NonEmptyFrontier, ) -> io::Result<()> { write_position(&mut writer, frontier.position())?; - match frontier.leaf() { - Leaf::Left(a) => { - a.write(&mut writer)?; - Optional::write(&mut writer, None, |w, n: &H| n.write(w))?; - } - Leaf::Right(a, b) => { - a.write(&mut writer)?; - Optional::write(&mut writer, Some(b), |w, n| n.write(w))?; - } + if frontier.position().is_odd() { + // The v1 serialization wrote the sibling of a right-hand leaf as a non-optional value, + // rather than as part of the ommers vector. + frontier + .ommers() + .get(0) + .expect("ommers vector cannot be empty for right-hand nodes") + .write(&mut writer)?; + Optional::write(&mut writer, Some(frontier.leaf()), |w, n: &H| n.write(w))?; + Vector::write(&mut writer, &frontier.ommers()[1..], |w, e| e.write(w))?; + } else { + frontier.leaf().write(&mut writer)?; + Optional::write(&mut writer, None, |w, n: &H| n.write(w))?; + Vector::write(&mut writer, frontier.ommers(), |w, e| e.write(w))?; } - Vector::write(&mut writer, frontier.ommers(), |w, e| e.write(w))?; Ok(()) } @@ -101,12 +115,15 @@ pub fn read_nonempty_frontier_v1( let position = read_position(&mut reader)?; let left = H::read(&mut reader)?; let right = Optional::read(&mut reader, H::read)?; + let mut ommers = Vector::read(&mut reader, |r| H::read(r))?; - let leaf = right.map_or_else( - || Leaf::Left(left.clone()), - |r| Leaf::Right(left.clone(), r), - ); - let ommers = Vector::read(&mut reader, |r| H::read(r))?; + let leaf = if let Some(right) = right { + // if the frontier has a right leaf, then the left leaf is the first ommer + ommers.insert(0, left); + right + } else { + left + }; NonEmptyFrontier::from_parts(position, leaf, ommers).map_err(|err| { io::Error::new( @@ -136,37 +153,42 @@ pub fn read_frontier_v1(reader: R) -> io::Result( - mut writer: W, - fragment: &AuthFragment, -) -> io::Result<()> { - write_position(&mut writer, fragment.position())?; - write_usize_leu64(&mut writer, fragment.altitudes_observed())?; - Vector::write(&mut writer, fragment.values(), |w, a| a.write(w)) -} +// pub fn write_auth_fragment_v1( +// mut writer: W, +// fragment: &AuthFragment, +// ) -> io::Result<()> { +// write_position(&mut writer, fragment.position())?; +// write_usize_leu64(&mut writer, fragment.altitudes_observed())?; +// Vector::write(&mut writer, fragment.values(), |w, a| a.write(w)) +// } #[allow(clippy::redundant_closure)] -pub fn read_auth_fragment_v1(mut reader: R) -> io::Result> { +pub fn read_auth_fragment_v1( + mut reader: R, +) -> io::Result<(Position, usize, Vec)> { let position = read_position(&mut reader)?; let alts_observed = read_leu64_usize(&mut reader)?; let values = Vector::read(&mut reader, |r| H::read(r))?; - Ok(AuthFragment::from_parts(position, alts_observed, values)) + Ok((position, alts_observed, values)) } -pub fn write_bridge_v1( +pub fn write_bridge_v2( mut writer: W, bridge: &MerkleBridge, ) -> io::Result<()> { Optional::write(&mut writer, bridge.prior_position(), |w, pos| { write_position(w, pos) })?; - Vector::write( + Vector::write_sized(&mut writer, bridge.tracking().iter(), |mut w, addr| { + write_address(&mut w, *addr) + })?; + Vector::write_sized( &mut writer, - &bridge.auth_fragments().iter().collect::>(), - |mut w, (pos, a)| { - write_position(&mut w, **pos)?; - write_auth_fragment_v1(w, a) + bridge.ommers().iter(), + |mut w, (addr, value)| { + write_address(&mut w, *addr)?; + value.write(&mut w) }, )?; write_nonempty_frontier_v1(&mut writer, bridge.frontier())?; @@ -177,17 +199,85 @@ pub fn write_bridge_v1( pub fn read_bridge_v1( mut reader: R, ) -> io::Result> { + fn levels_required(pos: Position) -> impl Iterator { + (0u8..64).into_iter().filter_map(move |i| { + if usize::from(pos) == 0 || usize::from(pos) & (1 << i) == 0 { + Some(Level::from(i)) + } else { + None + } + }) + } + let prior_position = Optional::read(&mut reader, read_position)?; - let auth_fragments = Vector::read(&mut reader, |mut r| { - Ok((read_position(&mut r)?, read_auth_fragment_v1(r)?)) - })? - .into_iter() - .collect(); + + let fragments = Vector::read(&mut reader, |mut r| { + let fragment_position = read_position(&mut r)?; + let (pos, levels_observed, values) = read_auth_fragment_v1(r)?; + + if fragment_position == pos { + Ok((pos, levels_observed, values)) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "Auth fragment position mismatch: {:?} != {:?}", + fragment_position, pos + ), + )) + } + })?; + + let frontier = read_nonempty_frontier_v1(&mut reader)?; + + let mut tracking = BTreeSet::new(); + let mut ommers = BTreeMap::new(); + for (pos, levels_observed, values) in fragments.into_iter() { + // get the list of levels at which we expect to find future ommers for the position being + // tracked + let levels = levels_required(pos) + .take(levels_observed + 1) + .collect::>(); + + // track the currently-incomplete parent of the tracked position at max height (the one + // we're currently building) + tracking.insert(Address::above_position(*levels.last().unwrap(), pos)); + + for (level, ommer_value) in levels + .into_iter() + .rev() + .skip(1) + .zip(values.into_iter().rev()) + { + let ommer_address = Address::above_position(level, pos).sibling(); + ommers.insert(ommer_address, ommer_value); + } + } + + Ok(MerkleBridge::from_parts( + prior_position, + tracking, + ommers, + frontier, + )) +} + +pub fn read_bridge_v2( + mut reader: R, +) -> io::Result> { + let prior_position = Optional::read(&mut reader, read_position)?; + let tracking = Vector::read_collected(&mut reader, |mut r| read_address(&mut r))?; + let ommers = Vector::read_collected(&mut reader, |mut r| { + let addr = read_address(&mut r)?; + let value = H::read(&mut r)?; + Ok((addr, value)) + })?; let frontier = read_nonempty_frontier_v1(&mut reader)?; Ok(MerkleBridge::from_parts( prior_position, - auth_fragments, + tracking, + ommers, frontier, )) } @@ -196,8 +286,8 @@ pub fn write_bridge( mut writer: W, bridge: &MerkleBridge, ) -> io::Result<()> { - writer.write_u8(SER_V1)?; - write_bridge_v1(writer, bridge) + writer.write_u8(SER_V2)?; + write_bridge_v2(writer, bridge) } pub fn read_bridge( @@ -205,6 +295,7 @@ pub fn read_bridge( ) -> io::Result> { match reader.read_u8()? { SER_V1 => read_bridge_v1(&mut reader), + SER_V2 => read_bridge_v2(&mut reader), flag => Err(io::Error::new( io::ErrorKind::InvalidInput, format!("Unrecognized serialization version: {:?}", flag), @@ -214,14 +305,10 @@ pub fn read_bridge( #[cfg(test)] mod tests { + use bridgetree::{BridgeTree, Frontier}; use hex; use proptest::prelude::*; - use incrementalmerkletree::{ - bridgetree::{BridgeTree, Frontier}, - Tree, - }; - use super::*; use crate::{ merkle_tree::testing::{arb_commitment_tree, TestNode}, @@ -255,28 +342,28 @@ mod tests { #[test] fn test_bridge_roundtrip() { - let mut t: BridgeTree = BridgeTree::new(10); + let mut t: BridgeTree = BridgeTree::new(10, 0); let mut to_unwitness = vec![]; let mut has_auth_path = vec![]; for i in 0usize..100 { assert!( - t.append(&TestNode(i.try_into().unwrap())), + t.append(TestNode(i.try_into().unwrap())), "Append should succeed." ); - if i % 5 == 0 { - t.checkpoint(); - } if i % 7 == 0 { - t.witness(); + t.mark(); if i > 0 && i % 2 == 0 { to_unwitness.push(Position::from(i)); } else { has_auth_path.push(Position::from(i)); } } + if i % 5 == 0 { + t.checkpoint(i + 1); + } if i % 11 == 0 && !to_unwitness.is_empty() { let pos = to_unwitness.remove(0); - t.remove_witness(pos); + t.remove_mark(pos); } } @@ -290,16 +377,25 @@ mod tests { write_bridge(&mut buffer, b).unwrap(); let b0 = read_bridge(&buffer[..]).unwrap(); assert_eq!(b, &b0); + let buffer2 = hex::decode(&BRIDGE_V1_VECTORS[i]).unwrap(); let b2 = read_bridge(&buffer2[..]).unwrap(); - assert_eq!(b, &b2); + assert_eq!(b.prior_position(), b2.prior_position()); + assert_eq!(b.frontier(), b2.frontier()); + // Due to the changed nature of garbage collection, bridgetree-v0.2.0 and later + // MerkleBridge values may track elements that incrementalmerkletree-v0.3.0 bridges did + // not; in the case that we remove the mark on a leaf, the ommers being tracked related + // to that mark may be retained until the next garbage collection pass. Therefore, we + // can only verify that the legacy tracking set is fully contained in the new tracking + // set. + assert!(b.tracking().is_superset(b2.tracking())); + for (k, v) in b2.ommers() { + assert_eq!(b.ommers().get(k), Some(v)); + } } - let latest_root = t.root(0).unwrap(); for (pos, witness) in BRIDGE_V1_WITNESSES { - let path = t - .authentication_path(Position::from(*pos), &latest_root) - .unwrap(); + let path = t.witness(Position::from(*pos), 0).unwrap(); assert_eq!(witness.to_vec(), path); } } diff --git a/zcash_primitives/src/sapling/tree.rs b/zcash_primitives/src/sapling/tree.rs index 976f5709d..33d8e0e50 100644 --- a/zcash_primitives/src/sapling/tree.rs +++ b/zcash_primitives/src/sapling/tree.rs @@ -1,6 +1,6 @@ use bitvec::{order::Lsb0, view::AsBits}; use group::{ff::PrimeField, Curve}; -use incrementalmerkletree::Altitude; +use incrementalmerkletree::{Hashable, Level}; use lazy_static::lazy_static; use std::io::{self, Read, Write}; @@ -8,7 +8,7 @@ use super::{ note::ExtractedNoteCommitment, pedersen_hash::{pedersen_hash, Personalization}, }; -use crate::merkle_tree::{HashSer, Hashable}; +use crate::merkle_tree::HashSer; pub const SAPLING_COMMITMENT_TREE_DEPTH: usize = 32; pub const SAPLING_COMMITMENT_TREE_DEPTH_U8: u8 = 32; @@ -16,9 +16,9 @@ pub const SAPLING_COMMITMENT_TREE_DEPTH_U8: u8 = 32; lazy_static! { static ref UNCOMMITTED_SAPLING: bls12_381::Scalar = bls12_381::Scalar::one(); static ref EMPTY_ROOTS: Vec = { - let mut v = vec![Node::blank()]; - for d in 0..SAPLING_COMMITMENT_TREE_DEPTH { - let next = Node::combine(d, &v[d], &v[d]); + let mut v = vec![Node::empty_leaf()]; + for d in 0..SAPLING_COMMITMENT_TREE_DEPTH_U8 { + let next = Node::combine(d.into(), &v[usize::from(d)], &v[usize::from(d)]); v.push(next); } v @@ -93,14 +93,14 @@ impl incrementalmerkletree::Hashable for Node { } } - fn combine(altitude: Altitude, lhs: &Self, rhs: &Self) -> Self { + fn combine(level: Level, lhs: &Self, rhs: &Self) -> Self { Node { - repr: merkle_hash(altitude.into(), &lhs.repr, &rhs.repr), + repr: merkle_hash(level.into(), &lhs.repr, &rhs.repr), } } - fn empty_root(altitude: Altitude) -> Self { - EMPTY_ROOTS[::from(altitude)] + fn empty_root(level: Level) -> Self { + EMPTY_ROOTS[::from(level)] } }