diff --git a/librustzcash/src/rustzcash.rs b/librustzcash/src/rustzcash.rs index 9878becf0..084d5b0ff 100644 --- a/librustzcash/src/rustzcash.rs +++ b/librustzcash/src/rustzcash.rs @@ -50,7 +50,7 @@ use zcash_primitives::{ fs::{Fs, FsRepr}, FixedGenerators, JubjubEngine, JubjubParams, PrimeOrder, ToUniform, Unknown, }, - merkle_tree::CommitmentTreeWitness, + merkle_tree::MerklePath, note_encryption::sapling_ka_agree, primitives::{Diversifier, Note, PaymentAddress, ProofGenerationKey, ViewingKey}, redjubjub::{self, Signature}, @@ -980,7 +980,7 @@ pub extern "C" fn librustzcash_sapling_spend_proof( ar: *const [c_uchar; 32], value: u64, anchor: *const [c_uchar; 32], - witness: *const [c_uchar; 1 + 33 * SAPLING_TREE_DEPTH + 8], + merkle_path: *const [c_uchar; 1 + 33 * SAPLING_TREE_DEPTH + 8], cv: *mut [c_uchar; 32], rk_out: *mut [c_uchar; 32], zkproof: *mut [c_uchar; GROTH_PROOF_SIZE], @@ -1030,9 +1030,8 @@ pub extern "C" fn librustzcash_sapling_spend_proof( Err(_) => return false, }; - // The witness contains the incremental tree witness information, in a - // weird serialized format. - let witness = match CommitmentTreeWitness::from_slice(unsafe { &(&*witness)[..] }) { + // Parse the Merkle path from the caller + let merkle_path = match MerklePath::from_slice(unsafe { &(&*merkle_path)[..] }) { Ok(w) => w, Err(_) => return false, }; @@ -1046,7 +1045,7 @@ pub extern "C" fn librustzcash_sapling_spend_proof( ar, value, anchor, - witness, + merkle_path, unsafe { SAPLING_SPEND_PARAMS.as_ref() }.unwrap(), unsafe { SAPLING_SPEND_VK.as_ref() }.unwrap(), &JUBJUB, diff --git a/zcash_primitives/src/merkle_tree.rs b/zcash_primitives/src/merkle_tree.rs index d41f05c51..53600e5f3 100644 --- a/zcash_primitives/src/merkle_tree.rs +++ b/zcash_primitives/src/merkle_tree.rs @@ -375,19 +375,19 @@ impl IncrementalWitness { } /// Returns the current witness, or None if the tree is empty. - pub fn path(&self) -> Option> { + pub fn path(&self) -> Option> { self.path_inner(SAPLING_COMMITMENT_TREE_DEPTH) } - fn path_inner(&self, depth: usize) -> Option> { + fn path_inner(&self, depth: usize) -> Option> { let mut filler = self.filler(); let mut auth_path = Vec::new(); if let Some(node) = self.tree.left { if self.tree.right.is_some() { - auth_path.push(Some((node, true))); + auth_path.push((node, true)); } else { - auth_path.push(Some((filler.next(0), false))); + auth_path.push((filler.next(0), false)); } } else { // Can't create an authentication path for the beginning of the tree @@ -396,41 +396,37 @@ impl IncrementalWitness { for (i, p) in self.tree.parents.iter().enumerate() { auth_path.push(match p { - Some(node) => Some((*node, true)), - None => Some((filler.next(i + 1), false)), + Some(node) => (*node, true), + None => (filler.next(i + 1), false), }); } for i in self.tree.parents.len()..(depth - 1) { - auth_path.push(Some((filler.next(i + 1), false))); + auth_path.push((filler.next(i + 1), false)); } assert_eq!(auth_path.len(), depth); - Some(CommitmentTreeWitness::from_path( - auth_path, - self.position() as u64, - )) + Some(MerklePath::from_path(auth_path, self.position() as u64)) } } -/// A witness to a path from a position in a particular commitment tree to the root of -/// that tree. +/// A path from a position in a particular commitment tree to the root of that tree. #[derive(Clone, Debug, PartialEq)] -pub struct CommitmentTreeWitness { - pub auth_path: Vec>, +pub struct MerklePath { + pub auth_path: Vec<(Node, bool)>, pub position: u64, } -impl CommitmentTreeWitness { - /// Constructs a witness directly from its path and position. - pub fn from_path(auth_path: Vec>, position: u64) -> Self { - CommitmentTreeWitness { +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 { auth_path, position, } } - /// Reads a witness from its serialized form. + /// 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) } @@ -444,48 +440,41 @@ impl CommitmentTreeWitness { witness = &witness[1..]; // Begin to construct the authentication path - let mut auth_path = vec![None; depth]; + let iter = witness.chunks_exact(33); + witness = iter.remainder(); // The vector works in reverse - for i in (0..depth).rev() { - // skip length of inner vector - if witness[0] != 32 { - // the length of a pedersen hash - return Err(()); - } - witness = &witness[1..]; - - // Grab the sibling node at this depth in the tree - let mut sibling = [0u8; 32]; - sibling.copy_from_slice(&witness[0..32]); - witness = &witness[32..]; - - // Sibling node should be an element of Fr - let sibling = match Node::read(&sibling[..]) { - Ok(p) => p, - Err(_) => return Err(()), - }; - - // Set the value in the auth path; we put false here - // for now (signifying the position bit) which we'll - // fill in later. - auth_path[i] = Some((sibling, false)); + 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() != depth { + return Err(()); } // Read the position from the witness - let position = match witness.read_u64::() { - Ok(pos) => pos, - Err(_) => return Err(()), - }; + 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() { - if let Some(p) = entry { - p.1 = (tmp & 1) == 1; - } - + entry.1 = (tmp & 1) == 1; tmp >>= 1; } @@ -493,7 +482,7 @@ impl CommitmentTreeWitness { // have provided more information than they should have, indicating // a bug downstream if witness.is_empty() { - Ok(CommitmentTreeWitness { + Ok(MerklePath { auth_path, position, }) @@ -501,11 +490,25 @@ impl CommitmentTreeWitness { Err(()) } } + + /// Returns the root of the tree corresponding to this path applied to `leaf`. + pub fn root(&self, leaf: Node) -> Node { + self.auth_path + .iter() + .enumerate() + .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), + }, + ) + } } #[cfg(test)] mod tests { - use super::{CommitmentTree, CommitmentTreeWitness, Hashable, IncrementalWitness, PathFiller}; + use super::{CommitmentTree, Hashable, IncrementalWitness, MerklePath, PathFiller}; use crate::sapling::Node; use ff::PrimeFieldRepr; @@ -604,7 +607,7 @@ mod tests { self.0.root_inner(TESTING_DEPTH) } - fn path(&self) -> Option> { + fn path(&self) -> Option> { self.0.path_inner(TESTING_DEPTH) } } @@ -1009,6 +1012,7 @@ mod tests { assert_eq!(tree.size(), 0); let mut witnesses = vec![]; + let mut last_cm = None; let mut paths_i = 0; let mut witness_ser_i = 0; for i in 0..16 { @@ -1019,7 +1023,7 @@ mod tests { let cm = Node::new(cm); // Witness here - witnesses.push(TestIncrementalWitness::from_tree(&tree)); + witnesses.push((TestIncrementalWitness::from_tree(&tree), last_cm)); // Now append a commitment to the tree assert!(tree.append(cm).is_ok()); @@ -1033,22 +1037,23 @@ mod tests { // Check serialization of tree assert_tree_ser_eq(&tree, tree_ser[i]); - let mut first = true; // The first witness can never form a path - for witness in witnesses.as_mut_slice() { + for (witness, leaf) in witnesses.as_mut_slice() { // Append the same commitment to all the witnesses assert!(witness.append(cm).is_ok()); - if first { - assert!(witness.path().is_none()); - } else { + if let Some(leaf) = leaf { let path = witness.path().expect("should be able to create a path"); - let expected = CommitmentTreeWitness::from_slice_with_depth( + let expected = MerklePath::from_slice_with_depth( &mut hex::decode(paths[paths_i]).unwrap(), TESTING_DEPTH, ) .unwrap(); assert_eq!(path, expected); + assert_eq!(path.root(*leaf), witness.root()); paths_i += 1; + } else { + // The first witness can never form a path + assert!(witness.path().is_none()); } // Check witness serialization @@ -1056,15 +1061,15 @@ mod tests { witness_ser_i += 1; assert_eq!(witness.root(), tree.root()); - - first = false; } + + last_cm = Some(cm); } // Tree should be full now let node = Node::blank(); assert!(tree.append(node).is_err()); - for witness in witnesses.as_mut_slice() { + for (witness, _) in witnesses.as_mut_slice() { assert!(witness.append(node).is_err()); } } diff --git a/zcash_primitives/src/prover.rs b/zcash_primitives/src/prover.rs index d07181690..932573d99 100644 --- a/zcash_primitives/src/prover.rs +++ b/zcash_primitives/src/prover.rs @@ -7,7 +7,7 @@ use crate::{ use pairing::bls12_381::{Bls12, Fr}; use crate::{ - merkle_tree::CommitmentTreeWitness, + merkle_tree::MerklePath, redjubjub::{PublicKey, Signature}, sapling::Node, transaction::components::{Amount, GROTH_PROOF_SIZE}, @@ -35,7 +35,7 @@ pub trait TxProver { ar: Fs, value: u64, anchor: Fr, - witness: CommitmentTreeWitness, + merkle_path: MerklePath, ) -> Result< ( [u8; GROTH_PROOF_SIZE], @@ -82,7 +82,7 @@ pub(crate) mod mock { }; use crate::{ - merkle_tree::CommitmentTreeWitness, + merkle_tree::MerklePath, redjubjub::{PublicKey, Signature}, sapling::Node, transaction::components::{Amount, GROTH_PROOF_SIZE}, @@ -108,7 +108,7 @@ pub(crate) mod mock { ar: Fs, value: u64, _anchor: Fr, - _witness: CommitmentTreeWitness, + _merkle_path: MerklePath, ) -> Result< ( [u8; GROTH_PROOF_SIZE], diff --git a/zcash_primitives/src/transaction/builder.rs b/zcash_primitives/src/transaction/builder.rs index ce0bfe9b6..9acb996d4 100644 --- a/zcash_primitives/src/transaction/builder.rs +++ b/zcash_primitives/src/transaction/builder.rs @@ -13,7 +13,7 @@ use crate::{ consensus, keys::OutgoingViewingKey, legacy::TransparentAddress, - merkle_tree::{CommitmentTreeWitness, IncrementalWitness}, + merkle_tree::MerklePath, note_encryption::{generate_esk, Memo, SaplingNoteEncryption}, prover::TxProver, redjubjub::PrivateKey, @@ -44,7 +44,6 @@ pub enum Error { ChangeIsNegative(Amount), InvalidAddress, InvalidAmount, - InvalidWitness, NoChangeAddress, SpendProof, } @@ -54,7 +53,7 @@ struct SpendDescriptionInfo { diversifier: Diversifier, note: Note, alpha: Fs, - witness: CommitmentTreeWitness, + merkle_path: MerklePath, } pub struct SaplingOutput { @@ -335,25 +334,25 @@ impl Builder { /// Adds a Sapling note to be spent in this transaction. /// - /// Returns an error if the given witness does not have the same anchor as previous - /// witnesses, or has no path. + /// Returns an error if the given Merkle path does not have the same anchor as the + /// paths for previous Sapling notes. pub fn add_sapling_spend( &mut self, extsk: ExtendedSpendingKey, diversifier: Diversifier, note: Note, - witness: IncrementalWitness, + merkle_path: MerklePath, ) -> Result<(), Error> { // Consistency check: all anchors must equal the first one + let cm = Node::new(note.cm(&JUBJUB).into()); if let Some(anchor) = self.anchor { - let witness_root: Fr = witness.root().into(); - if witness_root != anchor { + let path_root: Fr = merkle_path.root(cm).into(); + if path_root != anchor { return Err(Error::AnchorMismatch); } } else { - self.anchor = Some(witness.root().into()) + self.anchor = Some(merkle_path.root(cm).into()) } - let witness = witness.path().ok_or(Error::InvalidWitness)?; let alpha = Fs::random(&mut self.rng); @@ -364,7 +363,7 @@ impl Builder { diversifier, note, alpha, - witness, + merkle_path, }); Ok(()) @@ -436,7 +435,7 @@ impl Builder { pub fn build( mut self, consensus_branch_id: consensus::BranchId, - prover: impl TxProver, + prover: &impl TxProver, ) -> Result<(Transaction, TransactionMetadata), Error> { let mut tx_metadata = TransactionMetadata::new(); @@ -522,7 +521,7 @@ impl Builder { let mut nullifier = [0u8; 32]; nullifier.copy_from_slice(&spend.note.nf( &proof_generation_key.to_viewing_key(&JUBJUB), - spend.witness.position, + spend.merkle_path.position, &JUBJUB, )); @@ -535,7 +534,7 @@ impl Builder { spend.alpha, spend.note.value, anchor, - spend.witness.clone(), + spend.merkle_path.clone(), ) .map_err(|()| Error::SpendProof)?; @@ -559,7 +558,7 @@ impl Builder { // Record the post-randomized output location tx_metadata.output_indices[pos] = i; - output.build(&prover, &mut ctx, &mut self.rng) + output.build(prover, &mut ctx, &mut self.rng) } else { // This is a dummy output let (dummy_to, dummy_note) = { @@ -719,7 +718,7 @@ mod tests { { let builder = Builder::new(0); assert_eq!( - builder.build(consensus::BranchId::Sapling, MockTxProver), + builder.build(consensus::BranchId::Sapling, &MockTxProver), Err(Error::ChangeIsNegative(Amount::from_i64(-10000).unwrap())) ); } @@ -741,7 +740,7 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(consensus::BranchId::Sapling, MockTxProver), + builder.build(consensus::BranchId::Sapling, &MockTxProver), Err(Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap())) ); } @@ -757,7 +756,7 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(consensus::BranchId::Sapling, MockTxProver), + builder.build(consensus::BranchId::Sapling, &MockTxProver), Err(Error::ChangeIsNegative(Amount::from_i64(-60000).unwrap())) ); } @@ -779,7 +778,7 @@ mod tests { extsk.clone(), *to.diversifier(), note1.clone(), - witness1.clone(), + witness1.path().unwrap(), ) .unwrap(); builder @@ -797,7 +796,7 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(consensus::BranchId::Sapling, MockTxProver), + builder.build(consensus::BranchId::Sapling, &MockTxProver), Err(Error::ChangeIsNegative(Amount::from_i64(-1).unwrap())) ); } @@ -816,10 +815,15 @@ mod tests { { let mut builder = Builder::new(0); builder - .add_sapling_spend(extsk.clone(), *to.diversifier(), note1, witness1) + .add_sapling_spend( + extsk.clone(), + *to.diversifier(), + note1, + witness1.path().unwrap(), + ) .unwrap(); builder - .add_sapling_spend(extsk, *to.diversifier(), note2, witness2) + .add_sapling_spend(extsk, *to.diversifier(), note2, witness2.path().unwrap()) .unwrap(); builder .add_sapling_output(ovk, to, Amount::from_u64(30000).unwrap(), None) @@ -831,7 +835,7 @@ mod tests { ) .unwrap(); assert_eq!( - builder.build(consensus::BranchId::Sapling, MockTxProver), + builder.build(consensus::BranchId::Sapling, &MockTxProver), Err(Error::BindingSig) ) } diff --git a/zcash_proofs/src/prover.rs b/zcash_proofs/src/prover.rs index 6dd5767df..c4608f703 100644 --- a/zcash_proofs/src/prover.rs +++ b/zcash_proofs/src/prover.rs @@ -9,7 +9,7 @@ use zcash_primitives::{ primitives::{Diversifier, PaymentAddress, ProofGenerationKey}, }; use zcash_primitives::{ - merkle_tree::CommitmentTreeWitness, + merkle_tree::MerklePath, prover::TxProver, redjubjub::{PublicKey, Signature}, sapling::Node, @@ -127,7 +127,7 @@ impl TxProver for LocalTxProver { ar: Fs, value: u64, anchor: Fr, - witness: CommitmentTreeWitness, + merkle_path: MerklePath, ) -> Result< ( [u8; GROTH_PROOF_SIZE], @@ -143,7 +143,7 @@ impl TxProver for LocalTxProver { ar, value, anchor, - witness, + merkle_path, &self.spend_params, &self.spend_vk, &JUBJUB, diff --git a/zcash_proofs/src/sapling/prover.rs b/zcash_proofs/src/sapling/prover.rs index be73dedb3..32e42297b 100644 --- a/zcash_proofs/src/sapling/prover.rs +++ b/zcash_proofs/src/sapling/prover.rs @@ -10,7 +10,7 @@ use zcash_primitives::{ primitives::{Diversifier, Note, PaymentAddress, ProofGenerationKey, ValueCommitment}, }; use zcash_primitives::{ - merkle_tree::CommitmentTreeWitness, + merkle_tree::MerklePath, redjubjub::{PrivateKey, PublicKey, Signature}, sapling::Node, transaction::components::Amount, @@ -46,7 +46,7 @@ impl SaplingProvingContext { ar: Fs, value: u64, anchor: Fr, - witness: CommitmentTreeWitness, + merkle_path: MerklePath, proving_key: &Parameters, verifying_key: &PreparedVerifyingKey, params: &JubjubBls12, @@ -104,7 +104,7 @@ impl SaplingProvingContext { r: rcm, }; - let nullifier = note.nf(&viewing_key, witness.position, params); + let nullifier = note.nf(&viewing_key, merkle_path.position, params); // We now have the full witness for our circuit let instance = Spend { @@ -114,10 +114,10 @@ impl SaplingProvingContext { payment_address: Some(payment_address), commitment_randomness: Some(rcm), ar: Some(ar), - auth_path: witness + auth_path: merkle_path .auth_path .iter() - .map(|n| n.map(|(node, b)| (node.into(), b))) + .map(|(node, b)| Some(((*node).into(), *b))) .collect(), anchor: Some(anchor), };