diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 8b79fa966..9da1d48bb 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -64,7 +64,7 @@ and this library adheres to Rust's notion of - `merkle_tree::write_incremental_witness` replaces `merkle_tree::IncrementalWitness::write` - `merkle_tree::read_incremental_witness` replaces `merkle_tree::IncrementalWitness::read` - `merkle_tree::merkle_path_from_slice` replaces `merkle_tree::MerklePath::from_slice` -- `sapling::{CommitmentTree, IncrementalWitness, MerklePath}` +- `sapling::{CommitmentTree, IncrementalWitness, MerklePath, NOTE_COMMITMENT_TREE_DEPTH}` ### Changed - The bounds on the `H` parameter to the following methods have changed: diff --git a/zcash_primitives/src/merkle_tree.rs b/zcash_primitives/src/merkle_tree.rs index ab2def81f..798649540 100644 --- a/zcash_primitives/src/merkle_tree.rs +++ b/zcash_primitives/src/merkle_tree.rs @@ -32,7 +32,7 @@ impl HashSer for MerkleHashOrchard { reader.read_exact(&mut repr)?; >::from(Self::from_bytes(&repr)).ok_or_else(|| { io::Error::new( - io::ErrorKind::InvalidInput, + io::ErrorKind::InvalidData, "Non-canonical encoding of Pallas base field value.", ) }) @@ -57,7 +57,7 @@ pub fn write_usize_leu64(mut writer: W, value: usize) -> io::Result<() pub fn read_leu64_usize(mut reader: R) -> io::Result { reader.read_u64::()?.try_into().map_err(|e| { io::Error::new( - io::ErrorKind::InvalidInput, + io::ErrorKind::InvalidData, format!( "usize could not be decoded from a 64-bit value on this platform: {:?}", e @@ -136,7 +136,7 @@ pub fn read_nonempty_frontier_v1( NonEmptyFrontier::from_parts(position, leaf, ommers).map_err(|err| { io::Error::new( - io::ErrorKind::InvalidInput, + io::ErrorKind::InvalidData, format!("Parsing resulted in an invalid Merkle frontier: {:?}", err), ) }) @@ -155,7 +155,7 @@ pub fn read_frontier_v1(reader: R) -> io::Result Ok(Frontier::empty()), Some(f) => Frontier::try_from(f).map_err(|err| { io::Error::new( - io::ErrorKind::InvalidInput, + io::ErrorKind::InvalidData, format!("Parsing resulted in an invalid Merkle frontier: {:?}", err), ) }), @@ -217,11 +217,14 @@ pub fn write_incremental_witness( /// Reads a Merkle path from its serialized form. pub fn merkle_path_from_slice( mut witness: &[u8], -) -> Result, ()> { +) -> io::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(()); + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "depth is not as expected", + )); } witness = &witness[1..]; @@ -236,29 +239,51 @@ pub fn merkle_path_from_slice( // 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_err(|_| ()) + Node::read(&bytes[1..]) } else { - Err(()) + Err(io::Error::new( + io::ErrorKind::InvalidData, + "length of auth path element is not the expected 32 bytes", + )) } }) - .collect::, _>>()?; + .collect::>>()?; if auth_path.len() != usize::from(DEPTH) { - return Err(()); + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("length of auth path is not the expected {} elements", DEPTH), + )); } // Read the position from the witness - let position = witness - .read_u64::() - .map_err(|_| ()) - .and_then(|p| Position::try_from(p).map_err(|_| ()))?; + let position = witness.read_u64::().and_then(|p| { + Position::try_from(p).map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidData, + format!("decoded position {} exceeded the range of a `usize`", p), + ) + }) + })?; // 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() { - MerklePath::from_parts(auth_path, position) + let path_len = auth_path.len(); + MerklePath::from_parts(auth_path, position).map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidData, + format!( + "auth path expected to contain {} elements, got {}", + DEPTH, path_len + ), + ) + }) } else { - Err(()) + Err(io::Error::new( + io::ErrorKind::InvalidData, + "trailing data found in auth path decoding", + )) } }