From 6f2cbfc7de8d73f74495d1d76634667198e9dd4c Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 16 Mar 2023 10:28:21 -0600 Subject: [PATCH] Factor serialization out from merkle tree data structures. --- zcash_client_sqlite/src/prepared.rs | 5 +- zcash_client_sqlite/src/wallet.rs | 5 +- zcash_client_sqlite/src/wallet/transact.rs | 4 +- zcash_primitives/CHANGELOG.md | 12 +- zcash_primitives/src/merkle_tree.rs | 252 ++++++++++-------- .../src/merkle_tree/incremental.rs | 12 +- 6 files changed, 158 insertions(+), 132 deletions(-) diff --git a/zcash_client_sqlite/src/prepared.rs b/zcash_client_sqlite/src/prepared.rs index f1fee7043..da97faa6c 100644 --- a/zcash_client_sqlite/src/prepared.rs +++ b/zcash_client_sqlite/src/prepared.rs @@ -12,6 +12,7 @@ use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight}, memo::MemoBytes, + merkle_tree::{write_commitment_tree, write_incremental_witness}, sapling::{self, Diversifier, Nullifier}, transaction::{components::Amount, TxId}, zip32::{AccountId, DiversifierIndex}, @@ -279,7 +280,7 @@ impl<'a, P> DataConnStmtCache<'a, P> { commitment_tree: &sapling::CommitmentTree, ) -> Result<(), SqliteClientError> { let mut encoded_tree = Vec::new(); - commitment_tree.write(&mut encoded_tree).unwrap(); + write_commitment_tree(commitment_tree, &mut encoded_tree).unwrap(); self.stmt_insert_block.execute(params![ u32::from(block_height), @@ -784,7 +785,7 @@ impl<'a, P> DataConnStmtCache<'a, P> { }?; let mut encoded = Vec::new(); - witness.write(&mut encoded).unwrap(); + write_incremental_witness(witness, &mut encoded).unwrap(); self.stmt_insert_witness .execute(params![note_id, u32::from(height), encoded])?; diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 9407a60f1..0ec2e5933 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -73,6 +73,7 @@ use zcash_primitives::{ block::BlockHash, consensus::{self, BlockHeight, BranchId, NetworkUpgrade, Parameters}, memo::{Memo, MemoBytes}, + merkle_tree::{read_commitment_tree, read_incremental_witness}, sapling::{self, Note, Nullifier}, transaction::{components::Amount, Transaction, TxId}, zip32::{ @@ -693,7 +694,7 @@ pub(crate) fn get_sapling_commitment_tree

( [u32::from(block_height)], |row| { let row_data: Vec = row.get(0)?; - sapling::CommitmentTree::read(&row_data[..]).map_err(|e| { + read_commitment_tree(&row_data[..]).map_err(|e| { rusqlite::Error::FromSqlConversionFailure( row_data.len(), rusqlite::types::Type::Blob, @@ -719,7 +720,7 @@ pub(crate) fn get_sapling_witnesses

( .query_map([u32::from(block_height)], |row| { let id_note = NoteId::ReceivedNoteId(row.get(0)?); let wdb: Vec = row.get(1)?; - Ok(sapling::IncrementalWitness::read(&wdb[..]).map(|witness| (id_note, witness))) + Ok(read_incremental_witness(&wdb[..]).map(|witness| (id_note, witness))) }) .map_err(SqliteClientError::from)?; diff --git a/zcash_client_sqlite/src/wallet/transact.rs b/zcash_client_sqlite/src/wallet/transact.rs index 3e2754dc5..613749c68 100644 --- a/zcash_client_sqlite/src/wallet/transact.rs +++ b/zcash_client_sqlite/src/wallet/transact.rs @@ -7,7 +7,7 @@ use group::ff::PrimeField; use zcash_primitives::{ consensus::BlockHeight, - merkle_tree::IncrementalWitness, + merkle_tree::read_incremental_witness, sapling::{Diversifier, Rseed}, transaction::components::Amount, zip32::AccountId, @@ -50,7 +50,7 @@ fn to_spendable_note(row: &Row) -> Result, SqliteClientErr let witness = { let d: Vec<_> = row.get(4)?; - IncrementalWitness::read(&d[..])? + read_incremental_witness(&d[..])? }; Ok(SpendableNote { diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 8e485f987..0cf64704b 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -14,11 +14,17 @@ and this library adheres to Rust's notion of type has been removed. - `sapling::SAPLING_COMMITMENT_TREE_DEPTH_U8` and `sapling::SAPLING_COMMITMENT_TREE_DEPTH` have been removed; use `sapling::NOTE_COMMITMENT_TREE_DEPTH` instead. -- `merkle_tree::incremental::write_auth_fragment_v1` +- `merkle_tree:` removals: + - `incremental::write_auth_fragment_v1` + - `CommitmentTree::write` and `CommitmentTree::read` have been replaced by + `write_commitment_tree` and `read_commitment_tree` respectively. + - `IncrementalWitness::write` and `IncrementalWitness::read` have been + replaced by `write_incremental_witness` and `read_incremental_witness` + respectively. + - `MerklePath::from_slice` has been replaced by `merkle_path_from_slice` ### Added -- `merkle_tree::incremental::{read_address, write_address}` -- `merkle_tree::incremental::read_bridge_v2` +- `merkle_tree::incremental::{read_address, write_address, read_bridge_v2}` - `sapling::{CommitmentTree, IncrementalWitness, MerklePath, NOTE_COMMITMENT_TREE_DEPTH}` ### Changed diff --git a/zcash_primitives/src/merkle_tree.rs b/zcash_primitives/src/merkle_tree.rs index ee6e92b25..f11a43c51 100644 --- a/zcash_primitives/src/merkle_tree.rs +++ b/zcash_primitives/src/merkle_tree.rs @@ -150,35 +150,34 @@ impl CommitmentTree { } } +/// Reads a `CommitmentTree` from its serialized form. +pub fn read_commitment_tree( + mut reader: R, +) -> io::Result> { + let left = Optional::read(&mut reader, Node::read)?; + let right = Optional::read(&mut reader, Node::read)?; + let parents = Vector::read(&mut reader, |r| Optional::read(r, Node::read))?; + + Ok(CommitmentTree { + left, + right, + parents, + }) +} + +/// Serializes this tree as an array of bytes. +pub fn write_commitment_tree( + tree: &CommitmentTree, + mut writer: W, +) -> io::Result<()> { + Optional::write(&mut writer, tree.left.as_ref(), |w, n| n.write(w))?; + Optional::write(&mut writer, tree.right.as_ref(), |w, n| n.write(w))?; + Vector::write(&mut writer, &tree.parents, |w, e| { + Optional::write(w, e.as_ref(), |w, n| n.write(w)) + }) +} + impl CommitmentTree { - /// Reads a `CommitmentTree` from its serialized form. - pub fn read(mut reader: R) -> io::Result - where - Node: HashSer, - { - let left = Optional::read(&mut reader, Node::read)?; - let right = Optional::read(&mut reader, Node::read)?; - let parents = Vector::read(&mut reader, |r| Optional::read(r, Node::read))?; - - Ok(CommitmentTree { - left, - right, - parents, - }) - } - - /// Serializes this tree as an array of bytes. - pub fn write(&self, mut writer: W) -> io::Result<()> - where - Node: HashSer, - { - 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.as_ref(), |w, n| n.write(w)) - }) - } - /// Adds a leaf node to the tree. /// /// Returns an error if the tree is full. @@ -307,38 +306,47 @@ impl IncrementalWitness { } } -impl IncrementalWitness { - /// Reads an `IncrementalWitness` from its serialized form. - #[allow(clippy::redundant_closure)] - pub fn read(mut reader: R) -> io::Result> { - let tree = CommitmentTree::::read(&mut reader)?; - let filled = Vector::read(&mut reader, |r| Node::read(r))?; - let cursor = Optional::read(&mut reader, CommitmentTree::::read)?; +/// Reads an `IncrementalWitness` from its serialized form. +#[allow(clippy::redundant_closure)] +pub fn read_incremental_witness( + mut reader: R, +) -> io::Result> { + let tree = read_commitment_tree(&mut reader)?; + let filled = Vector::read(&mut reader, |r| Node::read(r))?; + let cursor = Optional::read(&mut reader, read_commitment_tree)?; - let mut witness = IncrementalWitness { - tree, - filled, - cursor_depth: 0, - cursor, - }; + let mut witness = IncrementalWitness { + tree, + filled, + cursor_depth: 0, + cursor, + }; - witness.cursor_depth = witness.next_depth(); + witness.cursor_depth = witness.next_depth(); - Ok(witness) - } + Ok(witness) +} - /// Serializes this `IncrementalWitness` as an array of bytes. - pub fn write(&self, mut writer: W) -> io::Result<()> { - self.tree.write(&mut writer)?; - Vector::write(&mut writer, &self.filled, |w, n| n.write(w))?; - Optional::write(&mut writer, self.cursor.as_ref(), |w, t| t.write(w)) - } +/// Serializes this `IncrementalWitness` as an array of bytes. +pub fn write_incremental_witness( + witness: &IncrementalWitness, + mut writer: W, +) -> io::Result<()> { + write_commitment_tree(&witness.tree, &mut writer)?; + Vector::write(&mut writer, &witness.filled, |w, n| n.write(w))?; + Optional::write(&mut writer, witness.cursor.as_ref(), |w, t| { + write_commitment_tree(t, w) + }) +} +impl IncrementalWitness { /// Returns the position of the witnessed leaf node in the commitment tree. pub fn position(&self) -> usize { self.tree.size() - 1 } +} +impl IncrementalWitness { fn filler(&self) -> PathFiller { let cursor_root = self .cursor @@ -349,7 +357,9 @@ impl IncrementalWitness IncrementalWitness { /// Finds the next "depth" of an unfilled subtree. fn next_depth(&self) -> u8 { let mut skip: u8 = self @@ -388,7 +398,9 @@ impl IncrementalWitness IncrementalWitness { /// Tracks a leaf node that has been added to the underlying tree. /// /// Returns an error if the tree is full. @@ -499,68 +511,70 @@ impl MerklePath { } } -impl MerklePath { - /// Reads a Merkle path from its serialized form. - pub fn from_slice(mut witness: &[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 { - return Err(()); - } - witness = &witness[1..]; +/// Reads a Merkle path from its serialized form. +pub fn merkle_path_from_slice( + mut witness: &[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 { + return Err(()); + } + witness = &witness[1..]; - // Begin to construct the authentication path - let iter = witness.chunks_exact(33); - witness = iter.remainder(); + // Begin to construct the authentication path + let iter = witness.chunks_exact(33); + witness = iter.remainder(); - // The vector works in reverse - let mut auth_path = iter - .rev() - .map(|bytes| { - // Length of inner vector should be the length of a Pedersen hash - if bytes[0] == 32 { - // Sibling node should be an element of Fr - Node::read(&bytes[1..]) - .map(|sibling| { - // Set the value in the auth path; we put false here - // for now (signifying the position bit) which we'll - // fill in later. - (sibling, false) - }) - .map_err(|_| ()) - } else { - Err(()) - } - }) - .collect::, _>>()?; - if auth_path.len() != usize::from(DEPTH) { - return Err(()); - } - - // Read the position from the witness - let position = witness.read_u64::().map_err(|_| ())?; - - // Given the position, let's finish constructing the authentication - // path - let mut tmp = position; - for entry in auth_path.iter_mut() { - entry.1 = (tmp & 1) == 1; - tmp >>= 1; - } - - // The witness should be empty now; if it wasn't, the caller would - // have provided more information than they should have, indicating - // a bug downstream - if witness.is_empty() { - Ok(MerklePath { - auth_path, - position, - }) - } else { - Err(()) - } + // The vector works in reverse + let mut auth_path = iter + .rev() + .map(|bytes| { + // Length of inner vector should be the length of a Pedersen hash + if bytes[0] == 32 { + // Sibling node should be an element of Fr + Node::read(&bytes[1..]) + .map(|sibling| { + // Set the value in the auth path; we put false here + // for now (signifying the position bit) which we'll + // fill in later. + (sibling, false) + }) + .map_err(|_| ()) + } else { + Err(()) + } + }) + .collect::, _>>()?; + if auth_path.len() != usize::from(DEPTH) { + return Err(()); } + // Read the position from the witness + let position = witness.read_u64::().map_err(|_| ())?; + + // Given the position, let's finish constructing the authentication + // path + let mut tmp = position; + for entry in auth_path.iter_mut() { + entry.1 = (tmp & 1) == 1; + tmp >>= 1; + } + + // The witness should be empty now; if it wasn't, the caller would + // have provided more information than they should have, indicating + // a bug downstream + if witness.is_empty() { + Ok(MerklePath { + auth_path, + position, + }) + } else { + Err(()) + } +} + +impl MerklePath { /// Returns the root of the tree corresponding to this path applied to `leaf`. pub fn root(&self, leaf: Node) -> Node { self.auth_path @@ -587,8 +601,10 @@ mod tests { use crate::sapling::{self, testing::arb_node, Node}; use super::{ + merkle_path_from_slice, read_commitment_tree, read_incremental_witness, testing::{arb_commitment_tree, TestNode}, - CommitmentTree, HashSer, IncrementalWitness, MerklePath, PathFiller, + write_commitment_tree, write_incremental_witness, CommitmentTree, HashSer, + IncrementalWitness, PathFiller, }; const HEX_EMPTY_ROOTS: [&str; 33] = [ @@ -1001,14 +1017,14 @@ mod tests { fn assert_tree_ser_eq(tree: &CommitmentTree, expected: &str) { // Check that the tree matches its encoding let mut tmp = Vec::new(); - tree.write(&mut tmp).unwrap(); + write_commitment_tree(tree, &mut tmp).unwrap(); assert_eq!(hex::encode(&tmp[..]), expected); // Check round-trip encoding let decoded: CommitmentTree = - CommitmentTree::read(&hex::decode(expected).unwrap()[..]).unwrap(); + read_commitment_tree(&hex::decode(expected).unwrap()[..]).unwrap(); tmp.clear(); - decoded.write(&mut tmp).unwrap(); + write_commitment_tree(&decoded, &mut tmp).unwrap(); assert_eq!(hex::encode(tmp), expected); } @@ -1018,14 +1034,14 @@ mod tests { ) { // Check that the witness matches its encoding let mut tmp = Vec::new(); - witness.write(&mut tmp).unwrap(); + write_incremental_witness(witness, &mut tmp).unwrap(); assert_eq!(hex::encode(&tmp[..]), expected); // Check round-trip encoding let decoded: IncrementalWitness = - IncrementalWitness::read(&hex::decode(expected).unwrap()[..]).unwrap(); + read_incremental_witness(&hex::decode(expected).unwrap()[..]).unwrap(); tmp.clear(); - decoded.write(&mut tmp).unwrap(); + write_incremental_witness(&decoded, &mut tmp).unwrap(); assert_eq!(hex::encode(tmp), expected); } @@ -1063,7 +1079,7 @@ mod tests { if let Some(leaf) = leaf { let path = witness.path().expect("should be able to create a path"); let expected = - MerklePath::from_slice(&hex::decode(paths[paths_i]).unwrap()).unwrap(); + merkle_path_from_slice(&hex::decode(paths[paths_i]).unwrap()).unwrap(); assert_eq!(path, expected); assert_eq!(path.root(*leaf), witness.root()); @@ -1136,8 +1152,8 @@ mod tests { #[test] fn prop_commitment_tree_roundtrip_ser(ct in arb_commitment_tree::<_, _, 8>(32, arb_node())) { let mut serialized = vec![]; - assert_matches!(ct.write(&mut serialized), Ok(())); - assert_matches!(CommitmentTree::<_, 8>::read(&serialized[..]), Ok(ct_out) if ct == ct_out); + assert_matches!(write_commitment_tree(&ct, &mut serialized), Ok(())); + assert_matches!(read_commitment_tree::<_, _, 8>(&serialized[..]), Ok(ct_out) if ct == ct_out); } } diff --git a/zcash_primitives/src/merkle_tree/incremental.rs b/zcash_primitives/src/merkle_tree/incremental.rs index a58ab75e4..b6ece1559 100644 --- a/zcash_primitives/src/merkle_tree/incremental.rs +++ b/zcash_primitives/src/merkle_tree/incremental.rs @@ -9,7 +9,7 @@ use incrementalmerkletree::{Address, Hashable, Level, Position}; use orchard::tree::MerkleHashOrchard; use zcash_encoding::{Optional, Vector}; -use super::{CommitmentTree, HashSer}; +use super::{read_commitment_tree, HashSer}; use crate::sapling; pub const SER_V1: u8 = 1; @@ -80,8 +80,7 @@ pub fn read_address(mut reader: R) -> io::Result

{ pub fn read_frontier_v0( mut reader: R, ) -> io::Result> { - let tree = CommitmentTree::::read(&mut reader)?; - + let tree = read_commitment_tree(&mut reader)?; Ok(tree.to_frontier()) } @@ -312,7 +311,10 @@ mod tests { use super::*; use crate::{ - merkle_tree::testing::{arb_commitment_tree, TestNode}, + merkle_tree::{ + testing::{arb_commitment_tree, TestNode}, + write_commitment_tree, + }, sapling::{testing as sapling, Node}, }; @@ -321,7 +323,7 @@ mod tests { fn frontier_serialization_v0(t in arb_commitment_tree::<_, _, 32>(0, sapling::arb_node())) { let mut buffer = vec![]; - t.write(&mut buffer).unwrap(); + write_commitment_tree(&t, &mut buffer).unwrap(); let frontier: Frontier = read_frontier_v0(&buffer[..]).unwrap(); let expected: Frontier = t.to_frontier();