From 2d34ac56545079908201017ed2fd89f43f351943 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 10 Mar 2022 09:06:59 -0700 Subject: [PATCH 1/5] Update incrementalmerkletree version. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 5456f0e36..9ea5722f9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ codegen-units = 1 [patch.crates-io] hdwallet = { git = "https://github.com/nuttycom/hdwallet", rev = "576683b9f2865f1118c309017ff36e01f84420c9" } -incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "dd57b430dee7c0b163f4035fef2280cd1935036c" } +incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "62c33e4480a71170b02b9eb7d4b0160194f414ee" } orchard = { git = "https://github.com/zcash/orchard.git", rev = "a5f701f3186fb619b40f2dee9939e64e4c9eb7cc" } zcash_encoding = { path = "components/zcash_encoding" } zcash_note_encryption = { path = "components/zcash_note_encryption" } From d602c01ef6ac1356fbe7a01b1d25d516a57d6a17 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 10 Mar 2022 09:06:59 -0700 Subject: [PATCH 2/5] Standardize how we write and read usize values for incrementalmerkletree. Also, make consistent use of helper functions for reading and writing `Position` values. --- .../src/merkle_tree/incremental.rs | 68 +++++++++++-------- 1 file changed, 41 insertions(+), 27 deletions(-) diff --git a/zcash_primitives/src/merkle_tree/incremental.rs b/zcash_primitives/src/merkle_tree/incremental.rs index 94e80bebb..57946b035 100644 --- a/zcash_primitives/src/merkle_tree/incremental.rs +++ b/zcash_primitives/src/merkle_tree/incremental.rs @@ -35,6 +35,39 @@ impl HashSer for MerkleHashOrchard { } } +/// Writes a usize value encoded as a u64 in little-endian order. Since usize +/// is platform-dependent, we consistently represent it as u64 in serialized +/// formats. +pub fn write_usize_leu64(mut writer: W, value: usize) -> io::Result<()> { + // Panic if we get a usize value that can't fit into a u64. + writer.write_u64::(value.try_into().unwrap()) +} + +/// Reads a usize value encoded as a u64 in little-endian order. Since usize +/// is platform-dependent, we consistently represent it as u64 in serialized +/// formats. +pub fn read_leu64_usize(mut reader: R) -> io::Result { + reader.read_u64::()?.try_into().map_err(|e| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "usize could not be decoded from a 64-bit value on this platform: {:?}", + e + ), + ) + }) +} + +pub fn write_position(mut writer: W, position: Position) -> io::Result<()> { + // The following `unwrap` is safe because we should never encounter + // a position that can't fit into a u64. + write_usize_leu64(&mut writer, position.into()) +} + +pub fn read_position(mut reader: R) -> io::Result { + read_leu64_usize(&mut reader).map(Position::from) +} + pub fn read_frontier_v0( mut reader: R, ) -> io::Result> { @@ -47,7 +80,7 @@ pub fn write_nonempty_frontier_v1( mut writer: W, frontier: &NonEmptyFrontier, ) -> io::Result<()> { - writer.write_u64::(::from(frontier.position()))?; + write_position(&mut writer, frontier.position())?; match frontier.leaf() { Leaf::Left(a) => { a.write(&mut writer)?; @@ -105,36 +138,19 @@ pub fn read_frontier_v1(reader: R) -> io::Result(mut writer: W, position: Position) -> io::Result<()> { - writer.write_u64::(position.try_into().unwrap()) -} - -pub fn read_position(mut reader: R) -> io::Result { - let p = reader.read_u64::()?; - ::try_from(p).map(Position::from).map_err(|err| { - io::Error::new( - io::ErrorKind::Unsupported, - format!( - "usize could not be decoded to a 64-bit value on this platform: {:?}", - err - ), - ) - }) -} - pub fn write_auth_fragment_v1( mut writer: W, fragment: &AuthFragment, ) -> io::Result<()> { write_position(&mut writer, fragment.position())?; - writer.write_u64::(fragment.altitudes_observed().try_into().unwrap())?; + 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> { let position = read_position(&mut reader)?; - let alts_observed = reader.read_u64::()? as usize; + 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)) @@ -144,16 +160,14 @@ pub fn write_bridge_v1( mut writer: W, bridge: &MerkleBridge, ) -> io::Result<()> { - Optional::write( - &mut writer, - bridge.prior_position().map(::from), - |w, n| w.write_u64::(n), - )?; + Optional::write(&mut writer, bridge.prior_position(), |w, pos| { + write_position(w, pos) + })?; Vector::write( &mut writer, &bridge.auth_fragments().iter().collect::>(), - |w, (i, a)| { - w.write_u64::(u64::from(**i))?; + |mut w, (pos, a)| { + write_position(&mut w, **pos)?; write_auth_fragment_v1(w, a) }, )?; From 71657b4f182c5b6851a0591fe054b7498f88675e Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Thu, 10 Mar 2022 11:22:24 -0700 Subject: [PATCH 3/5] Generalize vector and array writes & reads. In a number of places, we transform other kinds of collections with known length information into vectors simply to be able to use them with `Vector::write` or `Vector::read`. We can avoid these extra allocations by writing from iterators directly, and similarly by reading directly into our desired collection types. --- components/zcash_encoding/src/lib.rs | 70 ++++++++++++++++--- .../src/merkle_tree/incremental.rs | 2 - 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/components/zcash_encoding/src/lib.rs b/components/zcash_encoding/src/lib.rs index 58c3e5389..79049b48a 100644 --- a/components/zcash_encoding/src/lib.rs +++ b/components/zcash_encoding/src/lib.rs @@ -12,6 +12,7 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use nonempty::NonEmpty; use std::convert::TryFrom; use std::io::{self, Read, Write}; +use std::iter::FromIterator; /// The maximum allowed value representable as a `[CompactSize]` pub const MAX_COMPACT_SIZE: u32 = 0x02000000; @@ -104,23 +105,34 @@ pub struct Vector; impl Vector { /// Reads a vector, assuming the encoding written by [`Vector::write`], using the provided /// function to decode each element of the vector. - pub fn read(mut reader: R, func: F) -> io::Result> + pub fn read(reader: R, func: F) -> io::Result> + where + F: Fn(&mut R) -> io::Result, + { + Self::read_collected(reader, func) + } + + /// Reads a CompactSize-prefixed series of elements into a collection, assuming the encoding + /// written by [`Vector::write`], using the provided function to decode each element. + pub fn read_collected>( + mut reader: R, + func: F, + ) -> io::Result where F: Fn(&mut R) -> io::Result, { let count: usize = CompactSize::read_t(&mut reader)?; - Array::read(reader, count, func) + Array::read_collected(reader, count, func) } - /// Writes a slice of values by writing [`CompactSize`]-encoded integer specifying the length of - /// the slice to the stream, followed by the encoding of each element of the slice as performed - /// by the provided function. - pub fn write(mut writer: W, vec: &[E], func: F) -> io::Result<()> + /// Writes a slice of values by writing [`CompactSize`]-encoded integer specifying the length + /// of the slice to the stream, followed by the encoding of each element of the slice as + /// performed by the provided function. + pub fn write(writer: W, vec: &[E], func: F) -> io::Result<()> where F: Fn(&mut W, &E) -> io::Result<()>, { - CompactSize::write(&mut writer, vec.len())?; - vec.iter().try_for_each(|e| func(&mut writer, e)) + Self::write_sized(writer, vec.iter(), func) } /// Writes a NonEmpty container of values to the stream using the same encoding as @@ -136,6 +148,29 @@ impl Vector { CompactSize::write(&mut writer, vec.len())?; vec.iter().try_for_each(|e| func(&mut writer, e)) } + + /// Writes at most `items_to_write` values from the provided iterator to the stream + /// in [`CompactSize`]-prefixed format. + /// + /// If not enough items are available, this will return an error; all available + /// elements will already have been written to the stream but the [`CompactSize`] + /// prefix that was written will be incorrect, and so the data written will not be + /// able to be correctly decoded as a vector. + pub fn write_sized<'a, W: Write, E: 'a, F, I: Iterator + ExactSizeIterator>( + mut writer: W, + items: I, + func: F, + ) -> io::Result<()> + where + F: Fn(&mut W, E) -> io::Result<()>, + { + CompactSize::write(&mut writer, items.len())?; + for item in items { + func(&mut writer, item)?; + } + + Ok(()) + } } /// Namespace for functions that perform encoding of array contents. @@ -146,9 +181,22 @@ impl Vector { pub struct Array; impl Array { - /// Reads a vector, assuming the encoding written by [`Array::write`], using the provided - /// function to decode each element of the vector. - pub fn read(mut reader: R, count: usize, func: F) -> io::Result> + /// Reads `count` elements from a stream into a vector, assuming the encoding written by + /// [`Array::write`], using the provided function to decode each element. + pub fn read(reader: R, count: usize, func: F) -> io::Result> + where + F: Fn(&mut R) -> io::Result, + { + Self::read_collected(reader, count, func) + } + + /// Reads `count` elements into a collection, assuming the encoding written by + /// [`Array::write`], using the provided function to decode each element. + pub fn read_collected>( + mut reader: R, + count: usize, + func: F, + ) -> io::Result where F: Fn(&mut R) -> io::Result, { diff --git a/zcash_primitives/src/merkle_tree/incremental.rs b/zcash_primitives/src/merkle_tree/incremental.rs index 57946b035..c6cc09d58 100644 --- a/zcash_primitives/src/merkle_tree/incremental.rs +++ b/zcash_primitives/src/merkle_tree/incremental.rs @@ -59,8 +59,6 @@ pub fn read_leu64_usize(mut reader: R) -> io::Result { } pub fn write_position(mut writer: W, position: Position) -> io::Result<()> { - // The following `unwrap` is safe because we should never encounter - // a position that can't fit into a u64. write_usize_leu64(&mut writer, position.into()) } From abd63166c0ca11763cfaf8f0a79fb2cf2e35f765 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 11 Mar 2022 17:16:38 -0700 Subject: [PATCH 4/5] Apply suggestions from code review Co-authored-by: str4d --- components/zcash_encoding/src/lib.rs | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/components/zcash_encoding/src/lib.rs b/components/zcash_encoding/src/lib.rs index 79049b48a..7db5e0987 100644 --- a/components/zcash_encoding/src/lib.rs +++ b/components/zcash_encoding/src/lib.rs @@ -149,27 +149,19 @@ impl Vector { vec.iter().try_for_each(|e| func(&mut writer, e)) } - /// Writes at most `items_to_write` values from the provided iterator to the stream - /// in [`CompactSize`]-prefixed format. - /// - /// If not enough items are available, this will return an error; all available - /// elements will already have been written to the stream but the [`CompactSize`] - /// prefix that was written will be incorrect, and so the data written will not be - /// able to be correctly decoded as a vector. - pub fn write_sized<'a, W: Write, E: 'a, F, I: Iterator + ExactSizeIterator>( + /// Writes an iterator of values by writing [`CompactSize`]-encoded integer specifying + /// the length of the iterator to the stream, followed by the encoding of each element + /// of the iterator as performed by the provided function. + pub fn write_sized + ExactSizeIterator>( mut writer: W, - items: I, + mut items: I, func: F, ) -> io::Result<()> where F: Fn(&mut W, E) -> io::Result<()>, { CompactSize::write(&mut writer, items.len())?; - for item in items { - func(&mut writer, item)?; - } - - Ok(()) + items.try_for_each(|e| func(&mut writer, e)) } } From 5668804629d42885bd20489b65f647c086e4b925 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 23 Mar 2022 00:04:32 +0000 Subject: [PATCH 5/5] Migrate to beta releases of incrementalmerkletree and orchard --- Cargo.toml | 2 -- zcash_primitives/Cargo.toml | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5081427be..9d13d5403 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,5 @@ codegen-units = 1 [patch.crates-io] hdwallet = { git = "https://github.com/nuttycom/hdwallet", rev = "576683b9f2865f1118c309017ff36e01f84420c9" } -incrementalmerkletree = { git = "https://github.com/zcash/incrementalmerkletree.git", rev = "62c33e4480a71170b02b9eb7d4b0160194f414ee" } -orchard = { git = "https://github.com/zcash/orchard.git", rev = "3ddf6c49f7484ed1295bd5351317bbfe49e14472" } zcash_encoding = { path = "components/zcash_encoding" } zcash_note_encryption = { path = "components/zcash_note_encryption" } diff --git a/zcash_primitives/Cargo.toml b/zcash_primitives/Cargo.toml index 1a0eada2c..62fb37ca9 100644 --- a/zcash_primitives/Cargo.toml +++ b/zcash_primitives/Cargo.toml @@ -31,12 +31,12 @@ fpe = "0.5" group = "0.11" hdwallet = { version = "0.3.0", optional = true } hex = "0.4" -incrementalmerkletree = "0.2" +incrementalmerkletree = "=0.3.0-beta.1" jubjub = "0.8" lazy_static = "1" memuse = "0.2" nonempty = "0.7" -orchard = "=0.1.0-beta.1" +orchard = "=0.1.0-beta.2" proptest = { version = "1.0.0", optional = true } rand = "0.8" rand_core = "0.6" @@ -55,7 +55,7 @@ features = ["pre-zip-212"] criterion = "0.3" proptest = "1.0.0" rand_xorshift = "0.3" -orchard = { version = "=0.1.0-beta.1", features = ["test-dependencies"] } +orchard = { version = "=0.1.0-beta.2", features = ["test-dependencies"] } [target.'cfg(unix)'.dev-dependencies] pprof = { version = "=0.6.1", features = ["criterion", "flamegraph"] }