From ca5b7e534148f7e68c8e2ec7f699510984e0e2ab Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Fri, 29 Sep 2023 14:19:05 -0400 Subject: [PATCH 01/30] derive common bounds for VidDisperse --- primitives/src/vid/mod.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/primitives/src/vid/mod.rs b/primitives/src/vid/mod.rs index d2fa3cb51..99ebde913 100644 --- a/primitives/src/vid/mod.rs +++ b/primitives/src/vid/mod.rs @@ -9,6 +9,7 @@ use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; use ark_std::{error::Error, fmt::Debug, string::String, vec::Vec}; use displaydoc::Display; +use serde::{Deserialize, Serialize}; pub mod advz; @@ -78,6 +79,10 @@ pub trait VidScheme { /// /// # Why the `?Sized` bound? /// Rust hates you: +#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq, Hash)] +#[serde(bound = "V::Share: Serialize + for<'a> Deserialize<'a>, + V::Common: Serialize + for<'a> Deserialize<'a>, + V::Commit: Serialize + for<'a> Deserialize<'a>,")] pub struct VidDisperse { /// VID disperse shares to send to the storage nodes. pub shares: Vec, From b1238f3e51cde2cd1fcc6b7e7c0ea75fb3f06246 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Fri, 29 Sep 2023 14:33:53 -0400 Subject: [PATCH 02/30] more downstream-friendly derivations for VidDisperse --- primitives/src/vid/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/primitives/src/vid/mod.rs b/primitives/src/vid/mod.rs index 99ebde913..967da7cb1 100644 --- a/primitives/src/vid/mod.rs +++ b/primitives/src/vid/mod.rs @@ -7,7 +7,7 @@ //! Trait and implementation for a Verifiable Information Retrieval (VID). /// See section 1.3--1.4 for intro to VID semantics. use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; -use ark_std::{error::Error, fmt::Debug, string::String, vec::Vec}; +use ark_std::{error::Error, fmt::Debug, hash::Hash, string::String, vec::Vec}; use displaydoc::Display; use serde::{Deserialize, Serialize}; @@ -79,10 +79,18 @@ pub trait VidScheme { /// /// # Why the `?Sized` bound? /// Rust hates you: -#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq, Hash)] +#[derive(Derivative, Deserialize, Serialize)] #[serde(bound = "V::Share: Serialize + for<'a> Deserialize<'a>, V::Common: Serialize + for<'a> Deserialize<'a>, V::Commit: Serialize + for<'a> Deserialize<'a>,")] +// Somehow these bizarre bounds suffice for downstream derivations +#[derivative( + Clone(bound = ""), + Debug(bound = "V::Share: Debug, V::Common: Debug, V::Commit: Debug"), + Eq(bound = ""), + Hash(bound = "V::Share: Hash, V::Common: Hash, V::Commit: Hash"), + PartialEq(bound = "") +)] pub struct VidDisperse { /// VID disperse shares to send to the storage nodes. pub shares: Vec, From 6de787781fad5d3431ec74c4deb81ce4d2fab33e Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Tue, 26 Sep 2023 15:42:59 -0400 Subject: [PATCH 03/30] make itertools a workspace dep --- Cargo.toml | 10 ++++------ plonk/Cargo.toml | 32 ++++++++++++++++++++++++-------- primitives/Cargo.toml | 4 +--- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b21d6bb22..2d92ff1e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,10 +1,5 @@ [workspace] -members = [ - "plonk", - "primitives", - "relation", - "utilities", -] +members = ["plonk", "primitives", "relation", "utilities"] [workspace.package] version = "0.4.0-pre.0" @@ -15,3 +10,6 @@ rust-version = "1.64.0" homepage = "https://github.com/EspressoSystems/jellyfish" documentation = "https://jellyfish.docs.espressosys.com" repository = "https://github.com/EspressoSystems/jellyfish" + +[workspace.dependencies] +itertools = { version = "0.10.1", default-features = false } diff --git a/plonk/Cargo.toml b/plonk/Cargo.toml index 4672cfed4..a6f6aaba0 100644 --- a/plonk/Cargo.toml +++ b/plonk/Cargo.toml @@ -11,7 +11,7 @@ rust-version = { workspace = true } [dependencies] ark-ec = "0.4.0" -ark-ff = { version = "0.4.0", features = [ "asm" ] } +ark-ff = { version = "0.4.0", features = ["asm"] } ark-poly = "0.4.0" ark-serialize = "0.4.0" ark-std = { version = "0.4.0", default-features = false } @@ -21,7 +21,7 @@ downcast-rs = { version = "1.2.0", default-features = false } dyn-clone = "^1.0" espresso-systems-common = { git = "https://github.com/espressosystems/espresso-systems-common", tag = "0.4.0" } hashbrown = "0.13.2" -itertools = { version = "0.10.1", default-features = false } +itertools = { workspace = true } jf-primitives = { path = "../primitives", default-features = false } jf-relation = { path = "../relation", default-features = false } jf-utils = { path = "../utilities" } @@ -52,14 +52,30 @@ harness = false [features] default = ["parallel"] std = [ - "ark-std/std", "ark-serialize/std", "ark-ff/std", "ark-ec/std", "ark-poly/std", - "downcast-rs/std", "itertools/use_std", "jf-primitives/std", "jf-relation/std", - "jf-utils/std", "num-bigint/std", "rand_chacha/std", "sha3/std" + "ark-std/std", + "ark-serialize/std", + "ark-ff/std", + "ark-ec/std", + "ark-poly/std", + "downcast-rs/std", + "itertools/use_std", + "jf-primitives/std", + "jf-relation/std", + "jf-utils/std", + "num-bigint/std", + "rand_chacha/std", + "sha3/std", ] test_apis = [] # exposing apis for testing purpose -parallel = ["ark-ff/parallel", "ark-ec/parallel", "ark-poly/parallel", - "jf-utils/parallel", "jf-relation/parallel", "jf-primitives/parallel", - "dep:rayon" ] +parallel = [ + "ark-ff/parallel", + "ark-ec/parallel", + "ark-poly/parallel", + "jf-utils/parallel", + "jf-relation/parallel", + "jf-primitives/parallel", + "dep:rayon", +] test-srs = [] [[example]] diff --git a/primitives/Cargo.toml b/primitives/Cargo.toml index 72fc66cd3..6fd804592 100644 --- a/primitives/Cargo.toml +++ b/primitives/Cargo.toml @@ -36,9 +36,7 @@ digest = { version = "0.10.1", default-features = false, features = ["alloc"] } displaydoc = { version = "0.2.3", default-features = false } espresso-systems-common = { git = "https://github.com/espressosystems/espresso-systems-common", tag = "0.4.0" } hashbrown = "0.13.1" -itertools = { version = "0.10.1", default-features = false, features = [ - "use_alloc", -] } +itertools = { workspace = true, features = ["use_alloc"] } jf-relation = { path = "../relation", default-features = false } jf-utils = { path = "../utilities" } merlin = { version = "3.0.0", default-features = false } From b0d49cea607885075437126cbb53a7b99dd05d6a Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 27 Sep 2023 09:46:03 -0400 Subject: [PATCH 04/30] WIP can't use itertools because https://github.com/rust-itertools/itertools/issues/499 --- utilities/Cargo.toml | 20 +++++++++++++++++--- utilities/src/conversion.rs | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/utilities/Cargo.toml b/utilities/Cargo.toml index 6aba0e3ee..67843b3d0 100644 --- a/utilities/Cargo.toml +++ b/utilities/Cargo.toml @@ -9,10 +9,11 @@ rust-version = { workspace = true } [dependencies] ark-ec = { version = "0.4.0", default-features = false } -ark-ff = { version = "0.4.0", default-features = false, features = [ "asm" ] } +ark-ff = { version = "0.4.0", default-features = false, features = ["asm"] } ark-serialize = { version = "0.4.0", default-features = false } ark-std = { version = "0.4.0", default-features = false } digest = { version = "0.10.1", default-features = false } +itertools = { workspace = true } rayon = { version = "1.5.0", optional = true } serde = { version = "1.0", default-features = false, features = ["derive"] } sha2 = { version = "0.10.1", default-features = false } @@ -28,5 +29,18 @@ ark-ed-on-bn254 = "0.4.0" [features] default = [] -std = ["ark-ff/std", "ark-std/std", "ark-ec/std", "ark-serialize/std", "digest/std", "serde/std", "sha2/std"] -parallel = ["ark-ff/parallel", "ark-std/parallel", "ark-ec/parallel", "dep:rayon"] +std = [ + "ark-ff/std", + "ark-std/std", + "ark-ec/std", + "ark-serialize/std", + "digest/std", + "serde/std", + "sha2/std", +] +parallel = [ + "ark-ff/parallel", + "ark-std/parallel", + "ark-ec/parallel", + "dep:rayon", +] diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index a6d73bc0f..a3d60a182 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -10,9 +10,11 @@ use ark_std::{ borrow::Borrow, cmp::min, iter::{once, repeat}, + marker::PhantomData, mem, vec::Vec, }; +use itertools::{Chunks, IntoChunks, Itertools}; use sha2::{Digest, Sha512}; /// Convert a scalar field element to a base field element. @@ -289,6 +291,41 @@ fn compile_time_checks() -> (usize, usize, usize) { (primefield_bytes_len, extension_degree, field_bytes_len) } +// pub fn bytes_to_field_elements(bytes: B) -> impl Iterator +// where +// F: Field, +// B: IntoIterator, +// B::Item: Borrow, +// { +// } + +pub struct FieldElems<'a, I: Iterator, F> { + into_chunks_iter: IntoChunks, + chunks_iter: Chunks<'a, I>, + final_byte_len: Option, + _phantom: PhantomData, + primefield_bytes_len: usize, + extension_degree: usize, + field_bytes_len: usize, +} + +impl FieldElems<'_, I, F> { + pub(crate) fn new(iter: I) -> Self { + let (primefield_bytes_len, extension_degree, field_bytes_len) = compile_time_checks::(); + let into_chunks_iter = iter.chunks(field_bytes_len); + let chunks_iter = into_chunks_iter.into_iter(); + Self { + into_chunks_iter, + chunks_iter, + final_byte_len: None, + _phantom: PhantomData, + primefield_bytes_len, + extension_degree, + field_bytes_len, + } + } +} + #[cfg(test)] mod tests { use crate::test_rng; From bb8dc038f828fbbc0200e537e4adc7e83bed3849 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 28 Sep 2023 10:16:13 -0400 Subject: [PATCH 05/30] BytesToField and FieldToBytes iterators pass tests, lots of cleanup todo --- utilities/Cargo.toml | 1 - utilities/src/conversion.rs | 266 ++++++++++++++++++++++++++++++++---- 2 files changed, 240 insertions(+), 27 deletions(-) diff --git a/utilities/Cargo.toml b/utilities/Cargo.toml index 67843b3d0..9844e0f6a 100644 --- a/utilities/Cargo.toml +++ b/utilities/Cargo.toml @@ -13,7 +13,6 @@ ark-ff = { version = "0.4.0", default-features = false, features = ["asm"] } ark-serialize = { version = "0.4.0", default-features = false } ark-std = { version = "0.4.0", default-features = false } digest = { version = "0.10.1", default-features = false } -itertools = { workspace = true } rayon = { version = "1.5.0", optional = true } serde = { version = "1.0", default-features = false, features = ["derive"] } sha2 = { version = "0.10.1", default-features = false } diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index a3d60a182..543dd2b4b 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -9,12 +9,11 @@ use ark_ff::{BigInteger, Field, PrimeField}; use ark_std::{ borrow::Borrow, cmp::min, - iter::{once, repeat}, + iter::{once, repeat, Take}, marker::PhantomData, - mem, - vec::Vec, + mem, println, vec, + vec::{IntoIter, Vec}, }; -use itertools::{Chunks, IntoChunks, Itertools}; use sha2::{Digest, Sha512}; /// Convert a scalar field element to a base field element. @@ -291,37 +290,206 @@ fn compile_time_checks() -> (usize, usize, usize) { (primefield_bytes_len, extension_degree, field_bytes_len) } -// pub fn bytes_to_field_elements(bytes: B) -> impl Iterator -// where -// F: Field, -// B: IntoIterator, -// B::Item: Borrow, -// { -// } - -pub struct FieldElems<'a, I: Iterator, F> { - into_chunks_iter: IntoChunks, - chunks_iter: Chunks<'a, I>, +pub fn bytes_to_field2(bytes: I) -> impl Iterator +where + F: PrimeField, + I: IntoIterator, +{ + BytesToField::new(bytes.into_iter()) +} + +pub fn bytes_from_field2(elems: I) -> impl Iterator +where + F: PrimeField, + I: IntoIterator, +{ + FieldToBytes::new(elems.into_iter()) +} + +struct BytesToField { + iter: I, final_byte_len: Option, + done: bool, _phantom: PhantomData, primefield_bytes_len: usize, - extension_degree: usize, - field_bytes_len: usize, } -impl FieldElems<'_, I, F> { - pub(crate) fn new(iter: I) -> Self { - let (primefield_bytes_len, extension_degree, field_bytes_len) = compile_time_checks::(); - let into_chunks_iter = iter.chunks(field_bytes_len); - let chunks_iter = into_chunks_iter.into_iter(); +impl BytesToField { + fn new(iter: I) -> Self { + let (primefield_bytes_len, _, _) = compile_time_checks::(); Self { - into_chunks_iter, - chunks_iter, + iter, final_byte_len: None, + done: false, _phantom: PhantomData, primefield_bytes_len, - extension_degree, - field_bytes_len, + } + } +} + +impl, F: PrimeField> Iterator for BytesToField { + type Item = F; + + fn next(&mut self) -> Option { + if self.done { + // we don't support iterators that return `Some` after returning `None` + return None; + } + + if let Some(len) = self.final_byte_len { + // iterator is done. final field elem encodes length. + self.done = true; + return Some(F::from(len as u64)); + } + + // TODO const generics: use [u8; primefield_bytes_len] + let mut field_elem_bytes = vec![0u8; self.primefield_bytes_len]; // TODO const generics + for (i, b) in field_elem_bytes.iter_mut().enumerate() { + if let Some(byte) = self.iter.next() { + *b = byte; + } else { + self.final_byte_len = Some(i); + break; + } + } + Some(F::from_le_bytes_mod_order(&field_elem_bytes)) + } +} + +struct FieldToBytes { + elems_iter: I, + state: FieldToBytesState, + primefield_bytes_len: usize, +} + +enum FieldToBytesState { + New, + Typical { + bytes_iter: Take>, + next_elem: F, + next_next_elem: F, + }, + Final { + bytes_iter: Take>, + }, +} + +impl FieldToBytes { + fn new(elems_iter: I) -> Self { + let (primefield_bytes_len, _, _) = compile_time_checks::(); + Self { + elems_iter, + state: FieldToBytesState::New, + primefield_bytes_len, + } + } +} + +impl, F: PrimeField> Iterator for FieldToBytes { + type Item = u8; + + fn next(&mut self) -> Option { + use FieldToBytesState::{Final, New, Typical}; + match &mut self.state { + New => { + // println!("state: New"); + let cur_elem = if let Some(elem) = self.elems_iter.next() { + elem + } else { + // length-0 iterator. move to LengthOne state with an empty iterator + self.state = Final { + bytes_iter: Vec::new().into_iter().take(0), + }; + return None; + }; + + // TODO did I forget to drop the last byte? + let bytes_iter = cur_elem.into_bigint().to_bytes_le().into_iter(); + + let next_elem = if let Some(elem) = self.elems_iter.next() { + elem + } else { + // length-1 iterator: we never produced this. just return all the bytes of the sole elem (minus 1) + let mut bytes_iter = bytes_iter.take(self.primefield_bytes_len); + let ret = bytes_iter.next(); + self.state = Final { bytes_iter }; + return ret; + }; + + let next_next_elem = if let Some(elem) = self.elems_iter.next() { + elem + } else { + // length-2 iterator + let final_byte_len = usize::try_from(u64::from_le_bytes( + next_elem.into_bigint().to_bytes_le()[..mem::size_of::()] + .try_into() + .expect("conversion from [u8] to u64 should succeed"), + )) + .expect("result len conversion from u64 to usize should succeed"); + let mut bytes_iter = bytes_iter.take(final_byte_len); + let ret = bytes_iter.next(); + self.state = Final { bytes_iter }; + return ret; + }; + + let mut bytes_iter = bytes_iter.take(self.primefield_bytes_len); + let ret = bytes_iter.next(); + self.state = Typical { + bytes_iter, + next_elem, + next_next_elem, + }; + return ret; + }, + Typical { + bytes_iter, + next_elem, + next_next_elem, + } => { + // println!("state: Typical"); + + let ret = bytes_iter.next(); + if ret.is_some() { + return ret; + } + + if let Some(elem) = self.elems_iter.next() { + // advance to the next field element + let mut bytes_iter = next_elem + .into_bigint() + .to_bytes_le() + .into_iter() + .take(self.primefield_bytes_len); + let ret = bytes_iter.next(); + self.state = Typical { + bytes_iter, + next_elem: *next_next_elem, + next_next_elem: elem, + }; + return ret; + } + + // done + let final_byte_len = usize::try_from(u64::from_le_bytes( + next_next_elem.into_bigint().to_bytes_le()[..mem::size_of::()] + .try_into() + .expect("conversion from [u8] to u64 should succeed"), + )) + .expect("result len conversion from u64 to usize should succeed"); + + let mut bytes_iter = next_elem + .into_bigint() + .to_bytes_le() + .into_iter() + .take(final_byte_len); + let ret = bytes_iter.next(); + self.state = Final { bytes_iter }; + return ret; + }, + Final { bytes_iter } => { + // println!("state: Final"); + return bytes_iter.next(); + }, } } } @@ -337,6 +505,7 @@ mod tests { use ark_ed_on_bls12_377::{EdwardsConfig as Param377, Fr as Fr377}; use ark_ed_on_bls12_381::{EdwardsConfig as Param381, Fr as Fr381}; use ark_ed_on_bn254::{EdwardsConfig as Param254, Fr as Fr254}; + use ark_ff::{Field, PrimeField}; use ark_std::{rand::RngCore, UniformRand}; #[test] @@ -400,6 +569,49 @@ mod tests { } } + fn bytes_field_elems2() { + // copied from bytes_field_elems() + + let lengths = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200]; + let trailing_zeros_lengths = [0, 1, 2, 5, 50]; + + let max_len = *lengths.iter().max().unwrap(); + let max_trailing_zeros_len = *trailing_zeros_lengths.iter().max().unwrap(); + let mut bytes = Vec::with_capacity(max_len + max_trailing_zeros_len); + let mut elems: Vec = Vec::with_capacity(max_len); + let mut rng = test_rng(); + + for len in lengths { + for trailing_zeros_len in trailing_zeros_lengths { + // fill bytes with random bytes and trailing zeros + bytes.resize(len + trailing_zeros_len, 0); + rng.fill_bytes(&mut bytes[..len]); + bytes[len..].fill(0); + + println!("byte_len: {}, trailing_zeros: {}", len, trailing_zeros_len); + + // debug + println!("bytes: {:?}", bytes); + let encoded: Vec = bytes_to_field2(bytes.clone()).collect(); + println!("encoded: {:?}", encoded); + let result: Vec<_> = bytes_from_field2(encoded).collect(); + println!("result: {:?}", result); + + // round trip + // let result: Vec<_> = + // bytes_from_field2::<_, F>(bytes_to_field2(bytes.clone())).collect(); + // TODO make the iterators over Borrow + assert_eq!(result, bytes); + } + + // test infallibility of bytes_from_field_elements + // with random field elements + elems.resize(len, F::zero()); + elems.iter_mut().for_each(|e| *e = F::rand(&mut rng)); + bytes_from_field_elements(elems.as_ref()); + } + } + #[test] fn test_bytes_field_elems() { bytes_field_elems::(); @@ -408,5 +620,7 @@ mod tests { bytes_field_elems::(); bytes_field_elems::(); bytes_field_elems::(); + + bytes_field_elems2::(); } } From 1056b053e1bcb4de7c8842d84b2e378ca2cc81ba Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 15:37:11 -0400 Subject: [PATCH 06/30] add large test length 5000, separate test for bytes_field_elems2, remove println --- utilities/src/conversion.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 543dd2b4b..6f18eeec8 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -11,7 +11,7 @@ use ark_std::{ cmp::min, iter::{once, repeat, Take}, marker::PhantomData, - mem, println, vec, + mem, vec, vec::{IntoIter, Vec}, }; use sha2::{Digest, Sha512}; @@ -392,7 +392,6 @@ impl, F: PrimeField> Iterator for FieldToBytes { use FieldToBytesState::{Final, New, Typical}; match &mut self.state { New => { - // println!("state: New"); let cur_elem = if let Some(elem) = self.elems_iter.next() { elem } else { @@ -446,8 +445,6 @@ impl, F: PrimeField> Iterator for FieldToBytes { next_elem, next_next_elem, } => { - // println!("state: Typical"); - let ret = bytes_iter.next(); if ret.is_some() { return ret; @@ -487,7 +484,6 @@ impl, F: PrimeField> Iterator for FieldToBytes { return ret; }, Final { bytes_iter } => { - // println!("state: Final"); return bytes_iter.next(); }, } @@ -572,7 +568,7 @@ mod tests { fn bytes_field_elems2() { // copied from bytes_field_elems() - let lengths = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200]; + let lengths = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200, 5000]; let trailing_zeros_lengths = [0, 1, 2, 5, 50]; let max_len = *lengths.iter().max().unwrap(); @@ -588,18 +584,17 @@ mod tests { rng.fill_bytes(&mut bytes[..len]); bytes[len..].fill(0); - println!("byte_len: {}, trailing_zeros: {}", len, trailing_zeros_len); - // debug - println!("bytes: {:?}", bytes); - let encoded: Vec = bytes_to_field2(bytes.clone()).collect(); - println!("encoded: {:?}", encoded); - let result: Vec<_> = bytes_from_field2(encoded).collect(); - println!("result: {:?}", result); + // println!("byte_len: {}, trailing_zeros: {}", len, trailing_zeros_len); + // println!("bytes: {:?}", bytes); + // let encoded: Vec = bytes_to_field2(bytes.clone()).collect(); + // println!("encoded: {:?}", encoded); + // let result: Vec<_> = bytes_from_field2(encoded).collect(); + // println!("result: {:?}", result); // round trip - // let result: Vec<_> = - // bytes_from_field2::<_, F>(bytes_to_field2(bytes.clone())).collect(); + let result: Vec<_> = + bytes_from_field2::<_, F>(bytes_to_field2(bytes.clone())).collect(); // TODO make the iterators over Borrow assert_eq!(result, bytes); } @@ -620,7 +615,12 @@ mod tests { bytes_field_elems::(); bytes_field_elems::(); bytes_field_elems::(); + } + #[test] + fn test_bytes_field_elems2() { bytes_field_elems2::(); + bytes_field_elems2::(); + bytes_field_elems2::(); } } From 4e437415587eb3ee2724e908b09e0696c9321db4 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 15:58:40 -0400 Subject: [PATCH 07/30] make bytes_to_field2 generic over Borrow --- utilities/src/conversion.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 6f18eeec8..fcbb56e24 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -293,7 +293,8 @@ fn compile_time_checks() -> (usize, usize, usize) { pub fn bytes_to_field2(bytes: I) -> impl Iterator where F: PrimeField, - I: IntoIterator, + I: IntoIterator, + I::Item: Borrow, { BytesToField::new(bytes.into_iter()) } @@ -327,7 +328,12 @@ impl BytesToField { } } -impl, F: PrimeField> Iterator for BytesToField { +impl Iterator for BytesToField +where + I: Iterator, + I::Item: Borrow, + F: PrimeField, +{ type Item = F; fn next(&mut self) -> Option { @@ -346,7 +352,7 @@ impl, F: PrimeField> Iterator for BytesToField { let mut field_elem_bytes = vec![0u8; self.primefield_bytes_len]; // TODO const generics for (i, b) in field_elem_bytes.iter_mut().enumerate() { if let Some(byte) = self.iter.next() { - *b = byte; + *b = *byte.borrow(); } else { self.final_byte_len = Some(i); break; @@ -592,11 +598,15 @@ mod tests { // let result: Vec<_> = bytes_from_field2(encoded).collect(); // println!("result: {:?}", result); - // round trip - let result: Vec<_> = + // round trip: bytes as Iterator + let result_clone: Vec<_> = bytes_from_field2::<_, F>(bytes_to_field2(bytes.clone())).collect(); - // TODO make the iterators over Borrow - assert_eq!(result, bytes); + assert_eq!(result_clone, bytes); + + // round trip: bytes as Iterator + let result_borrow: Vec<_> = + bytes_from_field2::<_, F>(bytes_to_field2(bytes.iter())).collect(); + assert_eq!(result_borrow, bytes); } // test infallibility of bytes_from_field_elements From e67f68f70ad54497d400d8a5e9982676060f24f9 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 16:02:16 -0400 Subject: [PATCH 08/30] rename bytes_to_field2 -> bytes_to_field, etc --- utilities/src/conversion.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index fcbb56e24..4abf6ad9f 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -290,7 +290,7 @@ fn compile_time_checks() -> (usize, usize, usize) { (primefield_bytes_len, extension_degree, field_bytes_len) } -pub fn bytes_to_field2(bytes: I) -> impl Iterator +pub fn bytes_to_field(bytes: I) -> impl Iterator where F: PrimeField, I: IntoIterator, @@ -299,7 +299,7 @@ where BytesToField::new(bytes.into_iter()) } -pub fn bytes_from_field2(elems: I) -> impl Iterator +pub fn bytes_from_field(elems: I) -> impl Iterator where F: PrimeField, I: IntoIterator, @@ -571,7 +571,7 @@ mod tests { } } - fn bytes_field_elems2() { + fn bytes_field_elems_iter() { // copied from bytes_field_elems() let lengths = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200, 5000]; @@ -600,12 +600,12 @@ mod tests { // round trip: bytes as Iterator let result_clone: Vec<_> = - bytes_from_field2::<_, F>(bytes_to_field2(bytes.clone())).collect(); + bytes_from_field::<_, F>(bytes_to_field(bytes.clone())).collect(); assert_eq!(result_clone, bytes); // round trip: bytes as Iterator let result_borrow: Vec<_> = - bytes_from_field2::<_, F>(bytes_to_field2(bytes.iter())).collect(); + bytes_from_field::<_, F>(bytes_to_field(bytes.iter())).collect(); assert_eq!(result_borrow, bytes); } @@ -628,9 +628,9 @@ mod tests { } #[test] - fn test_bytes_field_elems2() { - bytes_field_elems2::(); - bytes_field_elems2::(); - bytes_field_elems2::(); + fn test_bytes_field_elems_iter() { + bytes_field_elems_iter::(); + bytes_field_elems_iter::(); + bytes_field_elems_iter::(); } } From ca72b0adf70ceb6c7553929fda835f943d18fd6a Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 16:07:56 -0400 Subject: [PATCH 09/30] cargo fmt, cargo clippy --- utilities/src/conversion.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 4abf6ad9f..db2733309 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -317,7 +317,7 @@ struct BytesToField { impl BytesToField { fn new(iter: I) -> Self { - let (primefield_bytes_len, _, _) = compile_time_checks::(); + let (primefield_bytes_len, ..) = compile_time_checks::(); Self { iter, final_byte_len: None, @@ -382,7 +382,7 @@ enum FieldToBytesState { impl FieldToBytes { fn new(elems_iter: I) -> Self { - let (primefield_bytes_len, _, _) = compile_time_checks::(); + let (primefield_bytes_len, ..) = compile_time_checks::(); Self { elems_iter, state: FieldToBytesState::New, @@ -414,7 +414,8 @@ impl, F: PrimeField> Iterator for FieldToBytes { let next_elem = if let Some(elem) = self.elems_iter.next() { elem } else { - // length-1 iterator: we never produced this. just return all the bytes of the sole elem (minus 1) + // length-1 iterator: we never produced this. just return all the bytes of the + // sole elem (minus 1) let mut bytes_iter = bytes_iter.take(self.primefield_bytes_len); let ret = bytes_iter.next(); self.state = Final { bytes_iter }; @@ -444,7 +445,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { next_elem, next_next_elem, }; - return ret; + ret }, Typical { bytes_iter, @@ -487,11 +488,9 @@ impl, F: PrimeField> Iterator for FieldToBytes { .take(final_byte_len); let ret = bytes_iter.next(); self.state = Final { bytes_iter }; - return ret; - }, - Final { bytes_iter } => { - return bytes_iter.next(); + ret }, + Final { bytes_iter } => bytes_iter.next(), } } } From e7fdb101c25905cb1ee6455cde92cdda923418b0 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 16:53:55 -0400 Subject: [PATCH 10/30] comments --- utilities/src/conversion.rs | 40 +++++++++++++++++++++++++++++++++---- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index db2733309..1fc8dcedd 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -290,6 +290,32 @@ fn compile_time_checks() -> (usize, usize, usize) { (primefield_bytes_len, extension_degree, field_bytes_len) } +/// Deterministic, infallible, invertible iterator adaptor to convert from +/// arbitrary bytes to field elements. +/// +/// # TODO doctest example +/// +/// # How it works +/// +/// Returns an iterator over [`PrimeField`] items defined as follows: +/// - For each call to `next()`: +/// - Consume P-1 items from `bytes` where P is the field characteristic byte +/// length. (Consume all remaining B items from `bytes` if B < P-1.) +/// - Convert the consumed bytes into a [`PrimeField`] via +/// [`from_le_bytes_mod_order`]. Reduction modulo the field characteristic +/// is guaranteed not to occur because we consumed at most P-1 bytes. +/// - Return the resulting [`PrimeField`] item. +/// - The returned iterator has an additional item that encodes the number of +/// input items consumed in order to produce the final output item. +/// +/// # Panics +/// +/// Panics only under conditions that should be checkable at compile time: +/// +/// - The [`PrimeField`] modulus bit length is too small to hold a `u64`. +/// - The [`PrimeField`] byte length is too large to fit inside a `usize`. +/// +/// If any of the above conditions holds then this function *always* panics. pub fn bytes_to_field(bytes: I) -> impl Iterator where F: PrimeField, @@ -299,6 +325,13 @@ where BytesToField::new(bytes.into_iter()) } +/// Deterministic, infallible inverse of [`bytes_to_field`]. +/// +/// This function is not invertible because [`bytes_to_field`] is not onto. +/// +/// ## Panics +/// +/// Panics under the conditions listed at [`bytes_to_field`]. pub fn bytes_from_field(elems: I) -> impl Iterator where F: PrimeField, @@ -401,14 +434,13 @@ impl, F: PrimeField> Iterator for FieldToBytes { let cur_elem = if let Some(elem) = self.elems_iter.next() { elem } else { - // length-0 iterator. move to LengthOne state with an empty iterator + // length-0 iterator. move to Final state with an empty iterator self.state = Final { bytes_iter: Vec::new().into_iter().take(0), }; return None; }; - // TODO did I forget to drop the last byte? let bytes_iter = cur_elem.into_bigint().to_bytes_le().into_iter(); let next_elem = if let Some(elem) = self.elems_iter.next() { @@ -592,9 +624,9 @@ mod tests { // debug // println!("byte_len: {}, trailing_zeros: {}", len, trailing_zeros_len); // println!("bytes: {:?}", bytes); - // let encoded: Vec = bytes_to_field2(bytes.clone()).collect(); + // let encoded: Vec = bytes_to_field(bytes.iter()).collect(); // println!("encoded: {:?}", encoded); - // let result: Vec<_> = bytes_from_field2(encoded).collect(); + // let result: Vec<_> = bytes_from_field(encoded).collect(); // println!("result: {:?}", result); // round trip: bytes as Iterator From 813ebfe1d9b92ec18d8198fb59ff0a01e7d5440c Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 16:56:57 -0400 Subject: [PATCH 11/30] test for zero-length input --- utilities/src/conversion.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 1fc8dcedd..987dbd7de 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -646,6 +646,16 @@ mod tests { elems.iter_mut().for_each(|e| *e = F::rand(&mut rng)); bytes_from_field_elements(elems.as_ref()); } + + // empty iterator + let bytes = Vec::new(); + assert!(bytes.iter().next().is_none()); + let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); + assert_eq!(elems_iter.next().unwrap(), F::ZERO); + assert_eq!(elems_iter.next().unwrap(), F::ZERO); + assert!(elems_iter.next().is_none()); + // let result: Vec<_> = bytes_from_field::<_, F>(elems_iter).collect(); + // assert_eq!(result, bytes); } #[test] From 89b9bcc19eaf6f942bcb7c8608bc0938f734681f Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 17:25:13 -0400 Subject: [PATCH 12/30] 0-length input -> 0-length output --- utilities/src/conversion.rs | 41 +++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 987dbd7de..80b76e0c9 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -9,7 +9,7 @@ use ark_ff::{BigInteger, Field, PrimeField}; use ark_std::{ borrow::Borrow, cmp::min, - iter::{once, repeat, Take}, + iter::{once, repeat, Peekable, Take}, marker::PhantomData, mem, vec, vec::{IntoIter, Vec}, @@ -307,6 +307,7 @@ fn compile_time_checks() -> (usize, usize, usize) { /// - Return the resulting [`PrimeField`] item. /// - The returned iterator has an additional item that encodes the number of /// input items consumed in order to produce the final output item. +/// - If `bytes` is empty then result is empty. /// /// # Panics /// @@ -340,21 +341,29 @@ where FieldToBytes::new(elems.into_iter()) } -struct BytesToField { - iter: I, +struct BytesToField +where + I: Iterator, +{ + bytes_iter: Peekable, final_byte_len: Option, done: bool, + new: bool, _phantom: PhantomData, primefield_bytes_len: usize, } -impl BytesToField { +impl BytesToField +where + I: Iterator, +{ fn new(iter: I) -> Self { let (primefield_bytes_len, ..) = compile_time_checks::(); Self { - iter, + bytes_iter: iter.peekable(), final_byte_len: None, done: false, + new: true, _phantom: PhantomData, primefield_bytes_len, } @@ -381,10 +390,16 @@ where return Some(F::from(len as u64)); } + if self.new && self.bytes_iter.peek().is_none() { + // zero-length iterator + self.done = true; + return None; + } + // TODO const generics: use [u8; primefield_bytes_len] let mut field_elem_bytes = vec![0u8; self.primefield_bytes_len]; // TODO const generics for (i, b) in field_elem_bytes.iter_mut().enumerate() { - if let Some(byte) = self.iter.next() { + if let Some(byte) = self.bytes_iter.next() { *b = *byte.borrow(); } else { self.final_byte_len = Some(i); @@ -458,6 +473,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { elem } else { // length-2 iterator + // TODO refactor repeated code let final_byte_len = usize::try_from(u64::from_le_bytes( next_elem.into_bigint().to_bytes_le()[..mem::size_of::()] .try_into() @@ -647,15 +663,18 @@ mod tests { bytes_from_field_elements(elems.as_ref()); } - // empty iterator + // empty input -> empty output let bytes = Vec::new(); assert!(bytes.iter().next().is_none()); let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); - assert_eq!(elems_iter.next().unwrap(), F::ZERO); - assert_eq!(elems_iter.next().unwrap(), F::ZERO); assert!(elems_iter.next().is_none()); - // let result: Vec<_> = bytes_from_field::<_, F>(elems_iter).collect(); - // assert_eq!(result, bytes); + + // smallest non-empty input -> 2-item output + let bytes = [42u8; 1]; + let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); + assert_eq!(elems_iter.next().unwrap(), F::from(42u64)); + assert_eq!(elems_iter.next().unwrap(), F::from(1u64)); + assert!(elems_iter.next().is_none()); } #[test] From 175cbf97a273f330e83f3d4e5a26e6c56eda3c15 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 18:35:33 -0400 Subject: [PATCH 13/30] refactor elem_to_usize --- utilities/src/conversion.rs | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 80b76e0c9..d6e556985 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -397,7 +397,7 @@ where } // TODO const generics: use [u8; primefield_bytes_len] - let mut field_elem_bytes = vec![0u8; self.primefield_bytes_len]; // TODO const generics + let mut field_elem_bytes = vec![0u8; self.primefield_bytes_len]; for (i, b) in field_elem_bytes.iter_mut().enumerate() { if let Some(byte) = self.bytes_iter.next() { *b = *byte.borrow(); @@ -428,7 +428,7 @@ enum FieldToBytesState { }, } -impl FieldToBytes { +impl FieldToBytes { fn new(elems_iter: I) -> Self { let (primefield_bytes_len, ..) = compile_time_checks::(); Self { @@ -437,6 +437,15 @@ impl FieldToBytes { primefield_bytes_len, } } + + fn elem_to_usize(elem: F) -> usize { + usize::try_from(u64::from_le_bytes( + elem.into_bigint().to_bytes_le()[..mem::size_of::()] + .try_into() + .expect("conversion from [u8] to u64 should succeed"), + )) + .expect("result len conversion from u64 to usize should succeed") + } } impl, F: PrimeField> Iterator for FieldToBytes { @@ -474,12 +483,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { } else { // length-2 iterator // TODO refactor repeated code - let final_byte_len = usize::try_from(u64::from_le_bytes( - next_elem.into_bigint().to_bytes_le()[..mem::size_of::()] - .try_into() - .expect("conversion from [u8] to u64 should succeed"), - )) - .expect("result len conversion from u64 to usize should succeed"); + let final_byte_len = Self::elem_to_usize(next_elem); let mut bytes_iter = bytes_iter.take(final_byte_len); let ret = bytes_iter.next(); self.state = Final { bytes_iter }; @@ -522,13 +526,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { } // done - let final_byte_len = usize::try_from(u64::from_le_bytes( - next_next_elem.into_bigint().to_bytes_le()[..mem::size_of::()] - .try_into() - .expect("conversion from [u8] to u64 should succeed"), - )) - .expect("result len conversion from u64 to usize should succeed"); - + let final_byte_len = Self::elem_to_usize(*next_next_elem); let mut bytes_iter = next_elem .into_bigint() .to_bytes_le() From f41a36aabe0cdfd82387f4acfe9299b46cd8a01d Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 19:13:20 -0400 Subject: [PATCH 14/30] refactor elem_to_bytes_iter --- utilities/src/conversion.rs | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index d6e556985..87c2fad98 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -446,6 +446,10 @@ impl FieldToBytes { )) .expect("result len conversion from u64 to usize should succeed") } + + fn elem_to_bytes_iter(elem: F) -> IntoIter { + elem.into_bigint().to_bytes_le().into_iter() + } } impl, F: PrimeField> Iterator for FieldToBytes { @@ -458,20 +462,21 @@ impl, F: PrimeField> Iterator for FieldToBytes { let cur_elem = if let Some(elem) = self.elems_iter.next() { elem } else { - // length-0 iterator. move to Final state with an empty iterator + // length-0 iterator + // move to `Final` state with an empty iterator self.state = Final { bytes_iter: Vec::new().into_iter().take(0), }; return None; }; - let bytes_iter = cur_elem.into_bigint().to_bytes_le().into_iter(); + let bytes_iter = Self::elem_to_bytes_iter(cur_elem); let next_elem = if let Some(elem) = self.elems_iter.next() { elem } else { - // length-1 iterator: we never produced this. just return all the bytes of the - // sole elem (minus 1) + // length-1 iterator: we never produced this + // move to `Final` state with primefield_bytes_len bytes from the sole elem let mut bytes_iter = bytes_iter.take(self.primefield_bytes_len); let ret = bytes_iter.next(); self.state = Final { bytes_iter }; @@ -482,7 +487,6 @@ impl, F: PrimeField> Iterator for FieldToBytes { elem } else { // length-2 iterator - // TODO refactor repeated code let final_byte_len = Self::elem_to_usize(next_elem); let mut bytes_iter = bytes_iter.take(final_byte_len); let ret = bytes_iter.next(); @@ -490,6 +494,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { return ret; }; + // length >2 iterator let mut bytes_iter = bytes_iter.take(self.primefield_bytes_len); let ret = bytes_iter.next(); self.state = Typical { @@ -509,13 +514,11 @@ impl, F: PrimeField> Iterator for FieldToBytes { return ret; } + let bytes_iter = Self::elem_to_bytes_iter(*next_elem); + if let Some(elem) = self.elems_iter.next() { // advance to the next field element - let mut bytes_iter = next_elem - .into_bigint() - .to_bytes_le() - .into_iter() - .take(self.primefield_bytes_len); + let mut bytes_iter = bytes_iter.take(self.primefield_bytes_len); let ret = bytes_iter.next(); self.state = Typical { bytes_iter, @@ -527,11 +530,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { // done let final_byte_len = Self::elem_to_usize(*next_next_elem); - let mut bytes_iter = next_elem - .into_bigint() - .to_bytes_le() - .into_iter() - .take(final_byte_len); + let mut bytes_iter = bytes_iter.take(final_byte_len); let ret = bytes_iter.next(); self.state = Final { bytes_iter }; ret From e73ee1120829db9ceb7dd0f29b3c3140bd1812f3 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 19:31:35 -0400 Subject: [PATCH 15/30] make bytes_from_field generic over Borrow --- utilities/src/conversion.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 87c2fad98..8f0069fab 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -336,7 +336,8 @@ where pub fn bytes_from_field(elems: I) -> impl Iterator where F: PrimeField, - I: IntoIterator, + I: IntoIterator, + I::Item: Borrow, { FieldToBytes::new(elems.into_iter()) } @@ -452,7 +453,12 @@ impl FieldToBytes { } } -impl, F: PrimeField> Iterator for FieldToBytes { +impl Iterator for FieldToBytes +where + I: Iterator, + I::Item: Borrow, + F: PrimeField, +{ type Item = u8; fn next(&mut self) -> Option { @@ -460,7 +466,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { match &mut self.state { New => { let cur_elem = if let Some(elem) = self.elems_iter.next() { - elem + *elem.borrow() } else { // length-0 iterator // move to `Final` state with an empty iterator @@ -473,7 +479,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { let bytes_iter = Self::elem_to_bytes_iter(cur_elem); let next_elem = if let Some(elem) = self.elems_iter.next() { - elem + *elem.borrow() } else { // length-1 iterator: we never produced this // move to `Final` state with primefield_bytes_len bytes from the sole elem @@ -484,7 +490,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { }; let next_next_elem = if let Some(elem) = self.elems_iter.next() { - elem + *elem.borrow() } else { // length-2 iterator let final_byte_len = Self::elem_to_usize(next_elem); @@ -523,7 +529,7 @@ impl, F: PrimeField> Iterator for FieldToBytes { self.state = Typical { bytes_iter, next_elem: *next_next_elem, - next_next_elem: elem, + next_next_elem: *elem.borrow(), }; return ret; } @@ -644,12 +650,12 @@ mod tests { // round trip: bytes as Iterator let result_clone: Vec<_> = - bytes_from_field::<_, F>(bytes_to_field(bytes.clone())).collect(); + bytes_from_field(bytes_to_field::<_, F>(bytes.clone())).collect(); assert_eq!(result_clone, bytes); // round trip: bytes as Iterator let result_borrow: Vec<_> = - bytes_from_field::<_, F>(bytes_to_field(bytes.iter())).collect(); + bytes_from_field(bytes_to_field::<_, F>(bytes.iter())).collect(); assert_eq!(result_borrow, bytes); } From a8ab070fc3a98adafdf1bad2d0b3f5f2c25c2b10 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Wed, 4 Oct 2023 19:38:42 -0400 Subject: [PATCH 16/30] test for iterators over both (u8, F) and (&u8, &F) --- utilities/src/conversion.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 8f0069fab..d83481e0c 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -648,14 +648,14 @@ mod tests { // let result: Vec<_> = bytes_from_field(encoded).collect(); // println!("result: {:?}", result); - // round trip: bytes as Iterator + // round trip: bytes as Iterator, elems as Iterator let result_clone: Vec<_> = bytes_from_field(bytes_to_field::<_, F>(bytes.clone())).collect(); assert_eq!(result_clone, bytes); - // round trip: bytes as Iterator - let result_borrow: Vec<_> = - bytes_from_field(bytes_to_field::<_, F>(bytes.iter())).collect(); + // round trip: bytes as Iterator, elems as Iterator + let encoded: Vec<_> = bytes_to_field::<_, F>(bytes.iter()).collect(); + let result_borrow: Vec<_> = bytes_from_field::<_, F>(encoded.iter()).collect(); assert_eq!(result_borrow, bytes); } From 89c0ee78b8b41c1b08f83ee0f998df76135e3239 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 05:44:42 -0400 Subject: [PATCH 17/30] fix infallibility test for bytes_from_field --- utilities/src/conversion.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index d83481e0c..564dcedc8 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -659,11 +659,11 @@ mod tests { assert_eq!(result_borrow, bytes); } - // test infallibility of bytes_from_field_elements + // test infallibility of bytes_from_field // with random field elements elems.resize(len, F::zero()); elems.iter_mut().for_each(|e| *e = F::rand(&mut rng)); - bytes_from_field_elements(elems.as_ref()); + let _: Vec = bytes_from_field::<_, F>(elems.iter()).collect(); } // empty input -> empty output From 1328fd8be22f2549d4414ead4fa51cc58349a6b7 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 11:42:40 -0400 Subject: [PATCH 18/30] WIP disperse() take iterator, tests fail because commit() still does not --- primitives/src/vid/advz.rs | 33 ++++++++++++++++++++++++--------- primitives/src/vid/mod.rs | 7 +++++-- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index b5b811b05..d9e155534 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -21,7 +21,7 @@ use anyhow::anyhow; use ark_ec::{pairing::Pairing, AffineRepr}; use ark_ff::{ fields::field_hashers::{DefaultFieldHasher, HashToField}, - FftField, Field, + FftField, Field, PrimeField, }; use ark_poly::{DenseUVPolynomial, EvaluationDomain}; use ark_serialize::{CanonicalDeserialize, CanonicalSerialize, Write}; @@ -37,7 +37,7 @@ use ark_std::{ }; use derivative::Derivative; use digest::{crypto_common::Output, Digest, DynDigest}; -use jf_utils::{bytes_from_field_elements, bytes_to_field_elements, canonical}; +use jf_utils::{bytes_from_field_elements, bytes_to_field, bytes_to_field_elements, canonical}; use serde::{Deserialize, Serialize}; /// The [ADVZ VID scheme](https://eprint.iacr.org/2021/1500), a concrete impl for [`VidScheme`]. @@ -147,10 +147,13 @@ where // 1,2: `Polynomial` is univariate: domain (`Point`) same field as range // (`Evaluation'). 3,4: `Commitment` is (convertible to/from) an elliptic curve // group in affine form. 5: `H` is a hasher +// +// `PrimeField` needed only because `bytes_to_field` needs it. +// Otherwise we could relax to `FftField`. impl VidScheme for GenericAdvz where P: UnivariatePCS::Evaluation>, - P::Evaluation: FftField, + P::Evaluation: PrimeField, P::Polynomial: DenseUVPolynomial, // 2 P::Commitment: From + AsRef, // 3 T: AffineRepr, // 4 @@ -181,8 +184,12 @@ where Ok(hasher.finalize()) } - fn disperse(&self, payload: &[u8]) -> VidResult> { - self.disperse_from_elems(&bytes_to_field_elements(payload)) + fn disperse(&self, payload: I) -> VidResult> + where + I: IntoIterator, + I::Item: Borrow, + { + self.disperse_from_elems(bytes_to_field::<_, P::Evaluation>(payload)) } fn verify_share( @@ -263,7 +270,7 @@ where impl GenericAdvz where P: UnivariatePCS::Evaluation>, - P::Evaluation: FftField, + P::Evaluation: PrimeField, P::Polynomial: DenseUVPolynomial, P::Commitment: From + AsRef, T: AffineRepr, @@ -274,14 +281,22 @@ where { /// Same as [`VidScheme::disperse`] except `payload` is a slice of /// field elements. - pub fn disperse_from_elems(&self, payload: &[P::Evaluation]) -> VidResult> { + pub fn disperse_from_elems(&self, payload: I) -> VidResult> + where + I: IntoIterator, + I::Item: Borrow, + { + let domain = P::multi_open_rou_eval_domain(self.payload_chunk_size, self.num_storage_nodes) + .map_err(vid)?; + + // TODO is it possible to avoid collect() here? + let payload: Vec<_> = payload.into_iter().map(|elem| *elem.borrow()).collect(); + let num_polys = if payload.is_empty() { 0 } else { (payload.len() - 1) / self.payload_chunk_size + 1 }; - let domain = P::multi_open_rou_eval_domain(self.payload_chunk_size, self.num_storage_nodes) - .map_err(vid)?; // partition payload into polynomial coefficients let polys: Vec = payload diff --git a/primitives/src/vid/mod.rs b/primitives/src/vid/mod.rs index 967da7cb1..c48d479fc 100644 --- a/primitives/src/vid/mod.rs +++ b/primitives/src/vid/mod.rs @@ -7,7 +7,7 @@ //! Trait and implementation for a Verifiable Information Retrieval (VID). /// See section 1.3--1.4 for intro to VID semantics. use ark_serialize::{CanonicalDeserialize, CanonicalSerialize}; -use ark_std::{error::Error, fmt::Debug, hash::Hash, string::String, vec::Vec}; +use ark_std::{borrow::Borrow, error::Error, fmt::Debug, hash::Hash, string::String, vec::Vec}; use displaydoc::Display; use serde::{Deserialize, Serialize}; @@ -57,7 +57,10 @@ pub trait VidScheme { fn commit_only(&self, payload: &[u8]) -> VidResult; /// Compute shares to send to the storage nodes - fn disperse(&self, payload: &[u8]) -> VidResult>; + fn disperse(&self, payload: I) -> VidResult> + where + I: IntoIterator, + I::Item: Borrow; /// Verify a share. Used by both storage node and retrieval client. /// Why is return type a nested `Result`? See From f2dde8b6504b6761152d16fbb30dc28ad09b3f92 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 12:06:18 -0400 Subject: [PATCH 19/30] WIP commit() take iterator, tests fail because recover() still does not --- primitives/src/vid/advz.rs | 21 ++++++++++++++------- primitives/src/vid/mod.rs | 7 +++++-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index d9e155534..0554f1191 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -37,7 +37,7 @@ use ark_std::{ }; use derivative::Derivative; use digest::{crypto_common::Output, Digest, DynDigest}; -use jf_utils::{bytes_from_field_elements, bytes_to_field, bytes_to_field_elements, canonical}; +use jf_utils::{bytes_from_field_elements, bytes_to_field, canonical}; use serde::{Deserialize, Serialize}; /// The [ADVZ VID scheme](https://eprint.iacr.org/2021/1500), a concrete impl for [`VidScheme`]. @@ -166,15 +166,22 @@ where type Share = Share; type Common = Common; - fn commit_only(&self, payload: &[u8]) -> VidResult { + fn commit_only(&self, payload: I) -> VidResult + where + I: IntoIterator, + I::Item: Borrow, + { let mut hasher = H::new(); - // TODO perf: DenseUVPolynomial::from_coefficients_slice copies the slice. - // We could avoid unnecessary mem copies if bytes_to_field_elements returned - // Vec> - let elems = bytes_to_field_elements(payload); + // TODO perf: is it possible to avoid collect() here? + let elems: Vec<_> = bytes_to_field::<_, P::Evaluation>(payload).collect(); + for coeffs in elems.chunks(self.payload_chunk_size) { + // TODO perf: DenseUVPolynomial::from_coefficients_slice copies the slice. + // We could avoid unnecessary mem copies if bytes_to_field_elements returned + // Vec> let poly = DenseUVPolynomial::from_coefficients_slice(coeffs); + let commitment = P::commit(&self.ck, &poly).map_err(vid)?; commitment .serialize_uncompressed(&mut hasher) @@ -289,7 +296,7 @@ where let domain = P::multi_open_rou_eval_domain(self.payload_chunk_size, self.num_storage_nodes) .map_err(vid)?; - // TODO is it possible to avoid collect() here? + // TODO perf: is it possible to avoid collect() here? let payload: Vec<_> = payload.into_iter().map(|elem| *elem.borrow()).collect(); let num_polys = if payload.is_empty() { diff --git a/primitives/src/vid/mod.rs b/primitives/src/vid/mod.rs index c48d479fc..00d1a7a40 100644 --- a/primitives/src/vid/mod.rs +++ b/primitives/src/vid/mod.rs @@ -53,8 +53,11 @@ pub trait VidScheme { /// Common data sent to all storage nodes. type Common: CanonicalSerialize + CanonicalDeserialize + Clone + Eq + PartialEq + Sync; // TODO https://github.com/EspressoSystems/jellyfish/issues/253 - /// Compute a payload commitment. - fn commit_only(&self, payload: &[u8]) -> VidResult; + /// Compute a payload commitment + fn commit_only(&self, payload: I) -> VidResult + where + I: IntoIterator, + I::Item: Borrow; /// Compute shares to send to the storage nodes fn disperse(&self, payload: I) -> VidResult> From e256ec5a8510ee3e607328bce27d028eeb65c8be Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 12:24:38 -0400 Subject: [PATCH 20/30] WIP recover_payload() use bytes_from_field, tests fail due to a bug --- primitives/src/vid/advz.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index 0554f1191..20a4db830 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -37,7 +37,7 @@ use ark_std::{ }; use derivative::Derivative; use digest::{crypto_common::Output, Digest, DynDigest}; -use jf_utils::{bytes_from_field_elements, bytes_to_field, canonical}; +use jf_utils::{bytes_from_field, bytes_to_field, canonical}; use serde::{Deserialize, Serialize}; /// The [ADVZ VID scheme](https://eprint.iacr.org/2021/1500), a concrete impl for [`VidScheme`]. @@ -268,9 +268,8 @@ where } fn recover_payload(&self, shares: &[Self::Share], common: &Self::Common) -> VidResult> { - Ok(bytes_from_field_elements( - self.recover_elems(shares, common)?, - )) + // TODO can we avoid collect() here? + Ok(bytes_from_field(self.recover_elems(shares, common)?).collect()) } } From 71ff65c7b896aa12008583025296af04e967838c Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 14:28:04 -0400 Subject: [PATCH 21/30] rename: add 'invertible' --- primitives/src/vid/advz.rs | 8 +++--- utilities/src/conversion.rs | 52 +++++++++++++++++++------------------ 2 files changed, 31 insertions(+), 29 deletions(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index 20a4db830..57406c3fe 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -37,7 +37,7 @@ use ark_std::{ }; use derivative::Derivative; use digest::{crypto_common::Output, Digest, DynDigest}; -use jf_utils::{bytes_from_field, bytes_to_field, canonical}; +use jf_utils::{bytes_from_field_invertible, bytes_to_field_invertible, canonical}; use serde::{Deserialize, Serialize}; /// The [ADVZ VID scheme](https://eprint.iacr.org/2021/1500), a concrete impl for [`VidScheme`]. @@ -174,7 +174,7 @@ where let mut hasher = H::new(); // TODO perf: is it possible to avoid collect() here? - let elems: Vec<_> = bytes_to_field::<_, P::Evaluation>(payload).collect(); + let elems: Vec<_> = bytes_to_field_invertible::<_, P::Evaluation>(payload).collect(); for coeffs in elems.chunks(self.payload_chunk_size) { // TODO perf: DenseUVPolynomial::from_coefficients_slice copies the slice. @@ -196,7 +196,7 @@ where I: IntoIterator, I::Item: Borrow, { - self.disperse_from_elems(bytes_to_field::<_, P::Evaluation>(payload)) + self.disperse_from_elems(bytes_to_field_invertible::<_, P::Evaluation>(payload)) } fn verify_share( @@ -269,7 +269,7 @@ where fn recover_payload(&self, shares: &[Self::Share], common: &Self::Common) -> VidResult> { // TODO can we avoid collect() here? - Ok(bytes_from_field(self.recover_elems(shares, common)?).collect()) + Ok(bytes_from_field_invertible(self.recover_elems(shares, common)?).collect()) } } diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 564dcedc8..f5abea810 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -317,13 +317,13 @@ fn compile_time_checks() -> (usize, usize, usize) { /// - The [`PrimeField`] byte length is too large to fit inside a `usize`. /// /// If any of the above conditions holds then this function *always* panics. -pub fn bytes_to_field(bytes: I) -> impl Iterator +pub fn bytes_to_field_invertible(bytes: I) -> impl Iterator where F: PrimeField, I: IntoIterator, I::Item: Borrow, { - BytesToField::new(bytes.into_iter()) + BytesToFieldInvertible::new(bytes.into_iter()) } /// Deterministic, infallible inverse of [`bytes_to_field`]. @@ -333,16 +333,16 @@ where /// ## Panics /// /// Panics under the conditions listed at [`bytes_to_field`]. -pub fn bytes_from_field(elems: I) -> impl Iterator +pub fn bytes_from_field_invertible(elems: I) -> impl Iterator where F: PrimeField, I: IntoIterator, I::Item: Borrow, { - FieldToBytes::new(elems.into_iter()) + FieldToBytesInvertible::new(elems.into_iter()) } -struct BytesToField +struct BytesToFieldInvertible where I: Iterator, { @@ -354,7 +354,7 @@ where primefield_bytes_len: usize, } -impl BytesToField +impl BytesToFieldInvertible where I: Iterator, { @@ -371,7 +371,7 @@ where } } -impl Iterator for BytesToField +impl Iterator for BytesToFieldInvertible where I: Iterator, I::Item: Borrow, @@ -411,13 +411,13 @@ where } } -struct FieldToBytes { +struct FieldToBytesInvertible { elems_iter: I, - state: FieldToBytesState, + state: FieldToBytesInvertibleState, primefield_bytes_len: usize, } -enum FieldToBytesState { +enum FieldToBytesInvertibleState { New, Typical { bytes_iter: Take>, @@ -429,12 +429,12 @@ enum FieldToBytesState { }, } -impl FieldToBytes { +impl FieldToBytesInvertible { fn new(elems_iter: I) -> Self { let (primefield_bytes_len, ..) = compile_time_checks::(); Self { elems_iter, - state: FieldToBytesState::New, + state: FieldToBytesInvertibleState::New, primefield_bytes_len, } } @@ -453,7 +453,7 @@ impl FieldToBytes { } } -impl Iterator for FieldToBytes +impl Iterator for FieldToBytesInvertible where I: Iterator, I::Item: Borrow, @@ -462,7 +462,7 @@ where type Item = u8; fn next(&mut self) -> Option { - use FieldToBytesState::{Final, New, Typical}; + use FieldToBytesInvertibleState::{Final, New, Typical}; match &mut self.state { New => { let cur_elem = if let Some(elem) = self.elems_iter.next() { @@ -621,7 +621,7 @@ mod tests { } } - fn bytes_field_elems_iter() { + fn bytes_field_elems_iter_invertible() { // copied from bytes_field_elems() let lengths = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200, 5000]; @@ -650,12 +650,14 @@ mod tests { // round trip: bytes as Iterator, elems as Iterator let result_clone: Vec<_> = - bytes_from_field(bytes_to_field::<_, F>(bytes.clone())).collect(); + bytes_from_field_invertible(bytes_to_field_invertible::<_, F>(bytes.clone())) + .collect(); assert_eq!(result_clone, bytes); // round trip: bytes as Iterator, elems as Iterator - let encoded: Vec<_> = bytes_to_field::<_, F>(bytes.iter()).collect(); - let result_borrow: Vec<_> = bytes_from_field::<_, F>(encoded.iter()).collect(); + let encoded: Vec<_> = bytes_to_field_invertible::<_, F>(bytes.iter()).collect(); + let result_borrow: Vec<_> = + bytes_from_field_invertible::<_, F>(encoded.iter()).collect(); assert_eq!(result_borrow, bytes); } @@ -663,18 +665,18 @@ mod tests { // with random field elements elems.resize(len, F::zero()); elems.iter_mut().for_each(|e| *e = F::rand(&mut rng)); - let _: Vec = bytes_from_field::<_, F>(elems.iter()).collect(); + let _: Vec = bytes_from_field_invertible::<_, F>(elems.iter()).collect(); } // empty input -> empty output let bytes = Vec::new(); assert!(bytes.iter().next().is_none()); - let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); + let mut elems_iter = bytes_to_field_invertible::<_, F>(bytes.iter()); assert!(elems_iter.next().is_none()); // smallest non-empty input -> 2-item output let bytes = [42u8; 1]; - let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); + let mut elems_iter = bytes_to_field_invertible::<_, F>(bytes.iter()); assert_eq!(elems_iter.next().unwrap(), F::from(42u64)); assert_eq!(elems_iter.next().unwrap(), F::from(1u64)); assert!(elems_iter.next().is_none()); @@ -691,9 +693,9 @@ mod tests { } #[test] - fn test_bytes_field_elems_iter() { - bytes_field_elems_iter::(); - bytes_field_elems_iter::(); - bytes_field_elems_iter::(); + fn test_bytes_field_elems_iter_invertible() { + bytes_field_elems_iter_invertible::(); + bytes_field_elems_iter_invertible::(); + bytes_field_elems_iter_invertible::(); } } From 03d7d87e24a05804b293c40b611df0fd076c8bf4 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 16:25:28 -0400 Subject: [PATCH 22/30] new alternate simpler bytes_to_field --- utilities/src/conversion.rs | 212 +++++++++++++++++++++++++++++++++++- 1 file changed, 211 insertions(+), 1 deletion(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index f5abea810..45f56f0b8 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -293,7 +293,168 @@ fn compile_time_checks() -> (usize, usize, usize) { /// Deterministic, infallible, invertible iterator adaptor to convert from /// arbitrary bytes to field elements. /// -/// # TODO doctest example +/// The final field element is padded with zero bytes as needed. +/// +/// # Example +/// +/// ``` +/// # use jf_utils::bytes_to_field; +/// # use ark_ed_on_bn254::Fr as Fr254; +/// let bytes = [1, 2, 3]; +/// let mut elems_iter = bytes_to_field::<_, Fr254>(bytes); +/// assert_eq!(elems_iter.next(), Some(Fr254::from(197121u64))); +/// assert_eq!(elems_iter.next(), None); +/// ``` +/// +/// # Panics +/// +/// Panics only under conditions that should be checkable at compile time: +/// +/// - The [`PrimeField`] modulus bit length is too small to hold a `u64`. +/// - The [`PrimeField`] byte length is too large to fit inside a `usize`. +/// +/// If any of the above conditions holds then this function *always* panics. +pub fn bytes_to_field(bytes: I) -> impl Iterator +where + F: PrimeField, + I: IntoIterator, + I::Item: Borrow, +{ + BytesToField::new(bytes.into_iter()) +} + +/// Deterministic, infallible inverse of [`bytes_to_field`]. +/// +/// The composition of [`field_to_bytes`] with [`bytes_to_field`] might contain +/// extra zero bytes. +/// +/// # Example +/// +/// ``` +/// # use jf_utils::{bytes_to_field, field_to_bytes}; +/// # use ark_ed_on_bn254::Fr as Fr254; +/// let bytes = [1, 2, 3]; +/// let mut bytes_iter = field_to_bytes(bytes_to_field::<_, Fr254>(bytes)); +/// assert_eq!(bytes_iter.next(), Some(1)); +/// assert_eq!(bytes_iter.next(), Some(2)); +/// assert_eq!(bytes_iter.next(), Some(3)); +/// for _ in 0..28 { +/// assert_eq!(bytes_iter.next(), Some(0)); +/// } +/// assert_eq!(bytes_iter.next(), None); +/// ``` +/// +/// ## Panics +/// +/// Panics under the conditions listed at [`bytes_to_field`]. +pub fn field_to_bytes(elems: I) -> impl Iterator +where + F: PrimeField, + I: IntoIterator, + I::Item: Borrow, +{ + FieldToBytes::new(elems.into_iter()) +} + +struct BytesToField { + bytes_iter: I, + primefield_bytes_len: usize, + _phantom: PhantomData, +} + +impl BytesToField +where + I: Iterator, + F: Field, +{ + fn new(bytes_iter: I) -> Self { + let (primefield_bytes_len, ..) = compile_time_checks::(); + Self { + bytes_iter, + primefield_bytes_len, + _phantom: PhantomData, + } + } +} + +impl Iterator for BytesToField +where + I: Iterator, + I::Item: Borrow, + F: PrimeField, +{ + type Item = F; + + fn next(&mut self) -> Option { + let mut elem_bytes = Vec::with_capacity(self.primefield_bytes_len); + for _ in 0..elem_bytes.capacity() { + if let Some(byte) = self.bytes_iter.next() { + elem_bytes.push(*byte.borrow()); + } else { + break; + } + } + if elem_bytes.is_empty() { + None + } else { + Some(F::from_le_bytes_mod_order(&elem_bytes)) + } + } +} + +struct FieldToBytes { + elems_iter: I, + bytes_iter: Take>, + primefield_bytes_len: usize, + _phantom: PhantomData, +} + +impl FieldToBytes +where + I: Iterator, + F: Field, +{ + fn new(elems_iter: I) -> Self { + let (primefield_bytes_len, ..) = compile_time_checks::(); + Self { + elems_iter, + bytes_iter: Vec::new().into_iter().take(0), + primefield_bytes_len, + _phantom: PhantomData, + } + } +} + +impl Iterator for FieldToBytes +where + I: Iterator, + I::Item: Borrow, + F: PrimeField, +{ + type Item = u8; + + fn next(&mut self) -> Option { + if let Some(byte) = self.bytes_iter.next() { + return Some(byte); + } + if let Some(elem) = self.elems_iter.next() { + self.bytes_iter = elem + .borrow() + .into_bigint() + .to_bytes_le() + .into_iter() + .take(self.primefield_bytes_len); + return self.bytes_iter.next(); + } + None + } +} + +/// Deterministic, infallible, invertible iterator adaptor to convert from +/// arbitrary bytes to field elements. +/// +/// # Example +/// /// /// # How it works /// @@ -682,6 +843,48 @@ mod tests { assert!(elems_iter.next().is_none()); } + fn bytes_to_field_iter() { + let byte_lens = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200, 5000]; + + let max_len = *byte_lens.iter().max().unwrap(); + let mut bytes = Vec::with_capacity(max_len); + // TODO pre-allocate space for elems, owned, borrowed + let mut rng = test_rng(); + + for len in byte_lens { + // fill bytes with random bytes and trailing zeros + bytes.resize(len, 0); + rng.fill_bytes(&mut bytes); + + // round trip, owned: + // bytes as Iterator, elems as Iterator + let owned: Vec<_> = field_to_bytes(bytes_to_field::<_, F>(bytes.clone())) + .take(bytes.len()) + .collect(); + assert_eq!(owned, bytes); + + // round trip, borrowed: + // bytes as Iterator, elems as Iterator + let elems: Vec<_> = bytes_to_field::<_, F>(bytes.iter()).collect(); + let borrowed: Vec<_> = field_to_bytes::<_, F>(elems.iter()) + .take(bytes.len()) + .collect(); + assert_eq!(borrowed, bytes); + } + + // empty input -> empty output + let bytes = Vec::new(); + assert!(bytes.iter().next().is_none()); + let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); + assert!(elems_iter.next().is_none()); + + // 1-item input -> 1-item output + let bytes = [42u8; 1]; + let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); + assert_eq!(elems_iter.next().unwrap(), F::from(42u64)); + assert!(elems_iter.next().is_none()); + } + #[test] fn test_bytes_field_elems() { bytes_field_elems::(); @@ -698,4 +901,11 @@ mod tests { bytes_field_elems_iter_invertible::(); bytes_field_elems_iter_invertible::(); } + + #[test] + fn test_bytes_field_elems_iter() { + bytes_to_field_iter::(); + bytes_to_field_iter::(); + bytes_to_field_iter::(); + } } From 279d7e11d69caf42256a1f3da90cd00e822fd877 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 16:58:25 -0400 Subject: [PATCH 23/30] fix: add elems_len to common so as to discard excess elements returned by RS decoding --- primitives/src/vid/advz.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index 57406c3fe..ef33ef5bf 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -140,6 +140,7 @@ where #[serde(with = "canonical")] poly_commits: Vec, all_evals_digest: V::NodeValue, + elems_len: usize, } // We take great pains to maintain abstraction by relying only on traits and not @@ -297,6 +298,7 @@ where // TODO perf: is it possible to avoid collect() here? let payload: Vec<_> = payload.into_iter().map(|elem| *elem.borrow()).collect(); + let elems_len = payload.len(); let num_polys = if payload.is_empty() { 0 @@ -359,6 +361,7 @@ where .collect::>() .map_err(vid)?, all_evals_digest: all_evals_commit.commitment().digest(), + elems_len, }; let commit = { @@ -415,7 +418,7 @@ where pub fn recover_elems( &self, shares: &[::Share], - _common: &::Common, + common: &::Common, ) -> VidResult> { if shares.len() < self.payload_chunk_size { return Err(VidError::Argument(format!( @@ -459,6 +462,7 @@ where result.append(&mut coeffs); } assert_eq!(result.len(), result_len); + result.truncate(common.elems_len); Ok(result) } From 6d2111202b10251a8cfe770b438d984cb3b6cec7 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 17:14:27 -0400 Subject: [PATCH 24/30] remove simpler bytes_to_field (boo) --- utilities/src/conversion.rs | 209 ------------------------------------ 1 file changed, 209 deletions(-) diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 45f56f0b8..84d2a8376 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -290,166 +290,6 @@ fn compile_time_checks() -> (usize, usize, usize) { (primefield_bytes_len, extension_degree, field_bytes_len) } -/// Deterministic, infallible, invertible iterator adaptor to convert from -/// arbitrary bytes to field elements. -/// -/// The final field element is padded with zero bytes as needed. -/// -/// # Example -/// -/// ``` -/// # use jf_utils::bytes_to_field; -/// # use ark_ed_on_bn254::Fr as Fr254; -/// let bytes = [1, 2, 3]; -/// let mut elems_iter = bytes_to_field::<_, Fr254>(bytes); -/// assert_eq!(elems_iter.next(), Some(Fr254::from(197121u64))); -/// assert_eq!(elems_iter.next(), None); -/// ``` -/// -/// # Panics -/// -/// Panics only under conditions that should be checkable at compile time: -/// -/// - The [`PrimeField`] modulus bit length is too small to hold a `u64`. -/// - The [`PrimeField`] byte length is too large to fit inside a `usize`. -/// -/// If any of the above conditions holds then this function *always* panics. -pub fn bytes_to_field(bytes: I) -> impl Iterator -where - F: PrimeField, - I: IntoIterator, - I::Item: Borrow, -{ - BytesToField::new(bytes.into_iter()) -} - -/// Deterministic, infallible inverse of [`bytes_to_field`]. -/// -/// The composition of [`field_to_bytes`] with [`bytes_to_field`] might contain -/// extra zero bytes. -/// -/// # Example -/// -/// ``` -/// # use jf_utils::{bytes_to_field, field_to_bytes}; -/// # use ark_ed_on_bn254::Fr as Fr254; -/// let bytes = [1, 2, 3]; -/// let mut bytes_iter = field_to_bytes(bytes_to_field::<_, Fr254>(bytes)); -/// assert_eq!(bytes_iter.next(), Some(1)); -/// assert_eq!(bytes_iter.next(), Some(2)); -/// assert_eq!(bytes_iter.next(), Some(3)); -/// for _ in 0..28 { -/// assert_eq!(bytes_iter.next(), Some(0)); -/// } -/// assert_eq!(bytes_iter.next(), None); -/// ``` -/// -/// ## Panics -/// -/// Panics under the conditions listed at [`bytes_to_field`]. -pub fn field_to_bytes(elems: I) -> impl Iterator -where - F: PrimeField, - I: IntoIterator, - I::Item: Borrow, -{ - FieldToBytes::new(elems.into_iter()) -} - -struct BytesToField { - bytes_iter: I, - primefield_bytes_len: usize, - _phantom: PhantomData, -} - -impl BytesToField -where - I: Iterator, - F: Field, -{ - fn new(bytes_iter: I) -> Self { - let (primefield_bytes_len, ..) = compile_time_checks::(); - Self { - bytes_iter, - primefield_bytes_len, - _phantom: PhantomData, - } - } -} - -impl Iterator for BytesToField -where - I: Iterator, - I::Item: Borrow, - F: PrimeField, -{ - type Item = F; - - fn next(&mut self) -> Option { - let mut elem_bytes = Vec::with_capacity(self.primefield_bytes_len); - for _ in 0..elem_bytes.capacity() { - if let Some(byte) = self.bytes_iter.next() { - elem_bytes.push(*byte.borrow()); - } else { - break; - } - } - if elem_bytes.is_empty() { - None - } else { - Some(F::from_le_bytes_mod_order(&elem_bytes)) - } - } -} - -struct FieldToBytes { - elems_iter: I, - bytes_iter: Take>, - primefield_bytes_len: usize, - _phantom: PhantomData, -} - -impl FieldToBytes -where - I: Iterator, - F: Field, -{ - fn new(elems_iter: I) -> Self { - let (primefield_bytes_len, ..) = compile_time_checks::(); - Self { - elems_iter, - bytes_iter: Vec::new().into_iter().take(0), - primefield_bytes_len, - _phantom: PhantomData, - } - } -} - -impl Iterator for FieldToBytes -where - I: Iterator, - I::Item: Borrow, - F: PrimeField, -{ - type Item = u8; - - fn next(&mut self) -> Option { - if let Some(byte) = self.bytes_iter.next() { - return Some(byte); - } - if let Some(elem) = self.elems_iter.next() { - self.bytes_iter = elem - .borrow() - .into_bigint() - .to_bytes_le() - .into_iter() - .take(self.primefield_bytes_len); - return self.bytes_iter.next(); - } - None - } -} - /// Deterministic, infallible, invertible iterator adaptor to convert from /// arbitrary bytes to field elements. /// @@ -843,48 +683,6 @@ mod tests { assert!(elems_iter.next().is_none()); } - fn bytes_to_field_iter() { - let byte_lens = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200, 5000]; - - let max_len = *byte_lens.iter().max().unwrap(); - let mut bytes = Vec::with_capacity(max_len); - // TODO pre-allocate space for elems, owned, borrowed - let mut rng = test_rng(); - - for len in byte_lens { - // fill bytes with random bytes and trailing zeros - bytes.resize(len, 0); - rng.fill_bytes(&mut bytes); - - // round trip, owned: - // bytes as Iterator, elems as Iterator - let owned: Vec<_> = field_to_bytes(bytes_to_field::<_, F>(bytes.clone())) - .take(bytes.len()) - .collect(); - assert_eq!(owned, bytes); - - // round trip, borrowed: - // bytes as Iterator, elems as Iterator - let elems: Vec<_> = bytes_to_field::<_, F>(bytes.iter()).collect(); - let borrowed: Vec<_> = field_to_bytes::<_, F>(elems.iter()) - .take(bytes.len()) - .collect(); - assert_eq!(borrowed, bytes); - } - - // empty input -> empty output - let bytes = Vec::new(); - assert!(bytes.iter().next().is_none()); - let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); - assert!(elems_iter.next().is_none()); - - // 1-item input -> 1-item output - let bytes = [42u8; 1]; - let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); - assert_eq!(elems_iter.next().unwrap(), F::from(42u64)); - assert!(elems_iter.next().is_none()); - } - #[test] fn test_bytes_field_elems() { bytes_field_elems::(); @@ -901,11 +699,4 @@ mod tests { bytes_field_elems_iter_invertible::(); bytes_field_elems_iter_invertible::(); } - - #[test] - fn test_bytes_field_elems_iter() { - bytes_to_field_iter::(); - bytes_to_field_iter::(); - bytes_to_field_iter::(); - } } From 474fcdf6a40102ce0acfc5c3729ed539562c5051 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 17:17:17 -0400 Subject: [PATCH 25/30] rename: remove invertible from bytes_to_field name --- primitives/src/vid/advz.rs | 8 +++--- utilities/src/conversion.rs | 55 ++++++++++++++++++------------------- 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index ef33ef5bf..93a326608 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -37,7 +37,7 @@ use ark_std::{ }; use derivative::Derivative; use digest::{crypto_common::Output, Digest, DynDigest}; -use jf_utils::{bytes_from_field_invertible, bytes_to_field_invertible, canonical}; +use jf_utils::{bytes_to_field, canonical, field_to_bytes}; use serde::{Deserialize, Serialize}; /// The [ADVZ VID scheme](https://eprint.iacr.org/2021/1500), a concrete impl for [`VidScheme`]. @@ -175,7 +175,7 @@ where let mut hasher = H::new(); // TODO perf: is it possible to avoid collect() here? - let elems: Vec<_> = bytes_to_field_invertible::<_, P::Evaluation>(payload).collect(); + let elems: Vec<_> = bytes_to_field::<_, P::Evaluation>(payload).collect(); for coeffs in elems.chunks(self.payload_chunk_size) { // TODO perf: DenseUVPolynomial::from_coefficients_slice copies the slice. @@ -197,7 +197,7 @@ where I: IntoIterator, I::Item: Borrow, { - self.disperse_from_elems(bytes_to_field_invertible::<_, P::Evaluation>(payload)) + self.disperse_from_elems(bytes_to_field::<_, P::Evaluation>(payload)) } fn verify_share( @@ -270,7 +270,7 @@ where fn recover_payload(&self, shares: &[Self::Share], common: &Self::Common) -> VidResult> { // TODO can we avoid collect() here? - Ok(bytes_from_field_invertible(self.recover_elems(shares, common)?).collect()) + Ok(field_to_bytes(self.recover_elems(shares, common)?).collect()) } } diff --git a/utilities/src/conversion.rs b/utilities/src/conversion.rs index 84d2a8376..377901059 100644 --- a/utilities/src/conversion.rs +++ b/utilities/src/conversion.rs @@ -293,8 +293,7 @@ fn compile_time_checks() -> (usize, usize, usize) { /// Deterministic, infallible, invertible iterator adaptor to convert from /// arbitrary bytes to field elements. /// -/// # Example -/// +/// # TODO doc test /// /// # How it works /// @@ -318,13 +317,13 @@ fn compile_time_checks() -> (usize, usize, usize) { /// - The [`PrimeField`] byte length is too large to fit inside a `usize`. /// /// If any of the above conditions holds then this function *always* panics. -pub fn bytes_to_field_invertible(bytes: I) -> impl Iterator +pub fn bytes_to_field(bytes: I) -> impl Iterator where F: PrimeField, I: IntoIterator, I::Item: Borrow, { - BytesToFieldInvertible::new(bytes.into_iter()) + BytesToField::new(bytes.into_iter()) } /// Deterministic, infallible inverse of [`bytes_to_field`]. @@ -334,16 +333,16 @@ where /// ## Panics /// /// Panics under the conditions listed at [`bytes_to_field`]. -pub fn bytes_from_field_invertible(elems: I) -> impl Iterator +pub fn field_to_bytes(elems: I) -> impl Iterator where F: PrimeField, I: IntoIterator, I::Item: Borrow, { - FieldToBytesInvertible::new(elems.into_iter()) + FieldToBytes::new(elems.into_iter()) } -struct BytesToFieldInvertible +struct BytesToField where I: Iterator, { @@ -355,7 +354,7 @@ where primefield_bytes_len: usize, } -impl BytesToFieldInvertible +impl BytesToField where I: Iterator, { @@ -372,7 +371,7 @@ where } } -impl Iterator for BytesToFieldInvertible +impl Iterator for BytesToField where I: Iterator, I::Item: Borrow, @@ -412,13 +411,13 @@ where } } -struct FieldToBytesInvertible { +struct FieldToBytes { elems_iter: I, - state: FieldToBytesInvertibleState, + state: FieldToBytesState, primefield_bytes_len: usize, } -enum FieldToBytesInvertibleState { +enum FieldToBytesState { New, Typical { bytes_iter: Take>, @@ -430,12 +429,12 @@ enum FieldToBytesInvertibleState { }, } -impl FieldToBytesInvertible { +impl FieldToBytes { fn new(elems_iter: I) -> Self { let (primefield_bytes_len, ..) = compile_time_checks::(); Self { elems_iter, - state: FieldToBytesInvertibleState::New, + state: FieldToBytesState::New, primefield_bytes_len, } } @@ -454,7 +453,7 @@ impl FieldToBytesInvertible { } } -impl Iterator for FieldToBytesInvertible +impl Iterator for FieldToBytes where I: Iterator, I::Item: Borrow, @@ -463,7 +462,7 @@ where type Item = u8; fn next(&mut self) -> Option { - use FieldToBytesInvertibleState::{Final, New, Typical}; + use FieldToBytesState::{Final, New, Typical}; match &mut self.state { New => { let cur_elem = if let Some(elem) = self.elems_iter.next() { @@ -622,7 +621,7 @@ mod tests { } } - fn bytes_field_elems_iter_invertible() { + fn bytes_field_elems_iter() { // copied from bytes_field_elems() let lengths = [0, 1, 2, 16, 31, 32, 33, 48, 65, 100, 200, 5000]; @@ -651,14 +650,12 @@ mod tests { // round trip: bytes as Iterator, elems as Iterator let result_clone: Vec<_> = - bytes_from_field_invertible(bytes_to_field_invertible::<_, F>(bytes.clone())) - .collect(); + field_to_bytes(bytes_to_field::<_, F>(bytes.clone())).collect(); assert_eq!(result_clone, bytes); // round trip: bytes as Iterator, elems as Iterator - let encoded: Vec<_> = bytes_to_field_invertible::<_, F>(bytes.iter()).collect(); - let result_borrow: Vec<_> = - bytes_from_field_invertible::<_, F>(encoded.iter()).collect(); + let encoded: Vec<_> = bytes_to_field::<_, F>(bytes.iter()).collect(); + let result_borrow: Vec<_> = field_to_bytes::<_, F>(encoded.iter()).collect(); assert_eq!(result_borrow, bytes); } @@ -666,18 +663,18 @@ mod tests { // with random field elements elems.resize(len, F::zero()); elems.iter_mut().for_each(|e| *e = F::rand(&mut rng)); - let _: Vec = bytes_from_field_invertible::<_, F>(elems.iter()).collect(); + let _: Vec = field_to_bytes::<_, F>(elems.iter()).collect(); } // empty input -> empty output let bytes = Vec::new(); assert!(bytes.iter().next().is_none()); - let mut elems_iter = bytes_to_field_invertible::<_, F>(bytes.iter()); + let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); assert!(elems_iter.next().is_none()); // smallest non-empty input -> 2-item output let bytes = [42u8; 1]; - let mut elems_iter = bytes_to_field_invertible::<_, F>(bytes.iter()); + let mut elems_iter = bytes_to_field::<_, F>(bytes.iter()); assert_eq!(elems_iter.next().unwrap(), F::from(42u64)); assert_eq!(elems_iter.next().unwrap(), F::from(1u64)); assert!(elems_iter.next().is_none()); @@ -694,9 +691,9 @@ mod tests { } #[test] - fn test_bytes_field_elems_iter_invertible() { - bytes_field_elems_iter_invertible::(); - bytes_field_elems_iter_invertible::(); - bytes_field_elems_iter_invertible::(); + fn test_bytes_field_elems_iter() { + bytes_field_elems_iter::(); + bytes_field_elems_iter::(); + bytes_field_elems_iter::(); } } From 5dbc66b7869c919e15551cb33241a18ed076398c Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Thu, 5 Oct 2023 17:59:01 -0400 Subject: [PATCH 26/30] update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 970fa8033..e5bad3316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ and follow [semantic versioning](https://semver.org/) for our releases. - [#341](https://github.com/EspressoSystems/jellyfish/pull/341) Port VDF from another repo - [#343](https://github.com/EspressoSystems/jellyfish/pull/343) Rescue parameter for `ark_bn254::Fq` - [#362](https://github.com/EspressoSystems/jellyfish/pull/362) Derive Eq, Hash at a bunch of places +- [#381](https://github.com/EspressoSystems/jellyfish/pull/381) VID take iterator instead of slice ### Changed From 4ca08abd6013b1feeb20613b365e3d9f4fc8879e Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Fri, 6 Oct 2023 13:59:24 -0400 Subject: [PATCH 27/30] update flake.lock, fix flake.nix --- flake.lock | 30 +++++++++++++++--------------- flake.nix | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/flake.lock b/flake.lock index 262ffc9d5..93aea596e 100644 --- a/flake.lock +++ b/flake.lock @@ -3,11 +3,11 @@ "flake-compat": { "flake": false, "locked": { - "lastModified": 1673956053, - "narHash": "sha256-4gtG9iQuiKITOjNQQeQIpoIB6b16fm+504Ch3sNKLd8=", + "lastModified": 1696426674, + "narHash": "sha256-kvjfFW7WAETZlt09AgDn1MrtKzP7t90Vf7vypd3OL1U=", "owner": "edolstra", "repo": "flake-compat", - "rev": "35bb57c0c8d8b62bbfd284272c928ceb64ddbde9", + "rev": "0f9255e01c2351cc7d116c072cb317785dd33b33", "type": "github" }, "original": { @@ -37,11 +37,11 @@ "systems": "systems" }, "locked": { - "lastModified": 1689068808, - "narHash": "sha256-6ixXo3wt24N/melDWjq70UuHQLxGV8jZvooRanIHXw0=", + "lastModified": 1694529238, + "narHash": "sha256-zsNZZGTGnMOf9YpHKJqMSsa0dXbfmxeoJ7xHlrt+xmY=", "owner": "numtide", "repo": "flake-utils", - "rev": "919d646de7be200f3bf08cb76ae1f09402b6f9b4", + "rev": "ff7b65b44d01cf9ba6a71320833626af21126384", "type": "github" }, "original": { @@ -109,11 +109,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1690031011, - "narHash": "sha256-kzK0P4Smt7CL53YCdZCBbt9uBFFhE0iNvCki20etAf4=", + "lastModified": 1696375444, + "narHash": "sha256-Sv0ICt/pXfpnFhTGYTsX6lUr1SljnuXWejYTI2ZqHa4=", "owner": "nixos", "repo": "nixpkgs", - "rev": "12303c652b881435065a98729eb7278313041e49", + "rev": "81e8f48ebdecf07aab321182011b067aafc78896", "type": "github" }, "original": { @@ -166,11 +166,11 @@ "nixpkgs-stable": "nixpkgs-stable" }, "locked": { - "lastModified": 1689668210, - "narHash": "sha256-XAATwDkaUxH958yXLs1lcEOmU6pSEIkatY3qjqk8X0E=", + "lastModified": 1696516544, + "narHash": "sha256-8rKE8Je6twTNFRTGF63P9mE3lZIq917RAicdc4XJO80=", "owner": "cachix", "repo": "pre-commit-hooks.nix", - "rev": "eb433bff05b285258be76513add6f6c57b441775", + "rev": "66c352d33e0907239e4a69416334f64af2c685cc", "type": "github" }, "original": { @@ -194,11 +194,11 @@ "nixpkgs": "nixpkgs_2" }, "locked": { - "lastModified": 1689992383, - "narHash": "sha256-x/MSjx2aA9aJqQA3fUHxiH0l8uG+1vxnkRNkqAZHQ2U=", + "lastModified": 1696558324, + "narHash": "sha256-TnnP4LGwDB8ZGE7h2n4nA9Faee8xPkMdNcyrzJ57cbw=", "owner": "oxalica", "repo": "rust-overlay", - "rev": "97fcdd4793778cf8e4f9007079cb9d2b836d7ea9", + "rev": "fdb37574a04df04aaa8cf7708f94a9309caebe2b", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index 8a8960d40..c659ef0d4 100644 --- a/flake.nix +++ b/flake.nix @@ -70,7 +70,7 @@ buildInputs = [ argbash openssl - pkgconfig + pkg-config git stableToolchain From befa38dc3a105be413ae086b68ad1f3027787fb4 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Fri, 6 Oct 2023 13:59:33 -0400 Subject: [PATCH 28/30] clippy appeasement --- plonk/src/circuit/plonk_verifier/gadgets.rs | 2 +- plonk/src/circuit/plonk_verifier/poly.rs | 6 +++--- primitives/src/merkle_tree/hasher.rs | 4 ++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/plonk/src/circuit/plonk_verifier/gadgets.rs b/plonk/src/circuit/plonk_verifier/gadgets.rs index 987874c08..8d00151c4 100644 --- a/plonk/src/circuit/plonk_verifier/gadgets.rs +++ b/plonk/src/circuit/plonk_verifier/gadgets.rs @@ -177,7 +177,7 @@ where } // ensure all the buffer has been consumed if v_and_uv_basis.next().is_some() { - return Err(PlonkError::IteratorOutOfRange)?; + Err(PlonkError::IteratorOutOfRange)?; } Ok(result) } diff --git a/plonk/src/circuit/plonk_verifier/poly.rs b/plonk/src/circuit/plonk_verifier/poly.rs index 7eec2fabb..5835b4db5 100644 --- a/plonk/src/circuit/plonk_verifier/poly.rs +++ b/plonk/src/circuit/plonk_verifier/poly.rs @@ -340,7 +340,7 @@ where let pi = public_inputs[0]; for &pi_i in public_inputs.iter().skip(1) { if pi != pi_i { - return Err(PlonkError::PublicInputsDoNotMatch)?; + Err(PlonkError::PublicInputsDoNotMatch)?; } } @@ -462,7 +462,7 @@ where } // ensure all the buffer has been consumed if alpha_bases_elem_var.next().is_some() { - return Err(PlonkError::IteratorOutOfRange)?; + Err(PlonkError::IteratorOutOfRange)?; } // ===================================================== // second statement @@ -690,7 +690,7 @@ where // ensure all the buffer has been consumed if alpha_bases_elem_var.next().is_some() { - return Err(PlonkError::IteratorOutOfRange)?; + Err(PlonkError::IteratorOutOfRange)?; } // ============================================ // Add splitted quotient commitments diff --git a/primitives/src/merkle_tree/hasher.rs b/primitives/src/merkle_tree/hasher.rs index 93b9882d2..7a8df6ee1 100644 --- a/primitives/src/merkle_tree/hasher.rs +++ b/primitives/src/merkle_tree/hasher.rs @@ -35,6 +35,10 @@ //! Use [`GenericHasherMerkleTree`] if you prefer to specify your own `Arity` //! and node [`Index`] types. +// clippy is freaking out about `HasherNode` and this is the only thing I +// could do to stop it +#![allow(clippy::incorrect_partial_ord_impl_on_ord_type)] + use crate::errors::PrimitivesError; use super::{append_only::MerkleTree, DigestAlgorithm, Element, Index}; From 8d2a83dfdca1091a5c36d907ef7a8324f446c89e Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Fri, 6 Oct 2023 14:19:45 -0400 Subject: [PATCH 29/30] iterate elems directly into polynomials in commit_only --- primitives/src/vid/advz.rs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index 93a326608..f58b0f90b 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -37,6 +37,7 @@ use ark_std::{ }; use derivative::Derivative; use digest::{crypto_common::Output, Digest, DynDigest}; +use itertools::Itertools; use jf_utils::{bytes_to_field, canonical, field_to_bytes}; use serde::{Deserialize, Serialize}; @@ -173,22 +174,14 @@ where I::Item: Borrow, { let mut hasher = H::new(); - - // TODO perf: is it possible to avoid collect() here? - let elems: Vec<_> = bytes_to_field::<_, P::Evaluation>(payload).collect(); - - for coeffs in elems.chunks(self.payload_chunk_size) { - // TODO perf: DenseUVPolynomial::from_coefficients_slice copies the slice. - // We could avoid unnecessary mem copies if bytes_to_field_elements returned - // Vec> - let poly = DenseUVPolynomial::from_coefficients_slice(coeffs); - + let elems_iter = bytes_to_field::<_, P::Evaluation>(payload); + for coeffs in elems_iter.chunks(self.payload_chunk_size).into_iter() { + let poly = DenseUVPolynomial::from_coefficients_vec(coeffs.collect()); let commitment = P::commit(&self.ck, &poly).map_err(vid)?; commitment .serialize_uncompressed(&mut hasher) .map_err(vid)?; } - Ok(hasher.finalize()) } From da87289bd99978a237966a7fddea1e56d0c3ea51 Mon Sep 17 00:00:00 2001 From: Gus Gutoski Date: Fri, 6 Oct 2023 14:50:02 -0400 Subject: [PATCH 30/30] iterate elems directly into polynomials in disperse_from_elems --- primitives/src/vid/advz.rs | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/primitives/src/vid/advz.rs b/primitives/src/vid/advz.rs index f58b0f90b..9000dbc33 100644 --- a/primitives/src/vid/advz.rs +++ b/primitives/src/vid/advz.rs @@ -289,26 +289,21 @@ where let domain = P::multi_open_rou_eval_domain(self.payload_chunk_size, self.num_storage_nodes) .map_err(vid)?; - // TODO perf: is it possible to avoid collect() here? - let payload: Vec<_> = payload.into_iter().map(|elem| *elem.borrow()).collect(); - let elems_len = payload.len(); - - let num_polys = if payload.is_empty() { - 0 - } else { - (payload.len() - 1) / self.payload_chunk_size + 1 - }; - // partition payload into polynomial coefficients - let polys: Vec = payload - .chunks(self.payload_chunk_size) - .map(DenseUVPolynomial::from_coefficients_slice) - .collect(); + // and count `elems_len` for later + let elems_iter = payload.into_iter().map(|elem| *elem.borrow()); + let mut elems_len = 0; + let mut polys = Vec::new(); + for coeffs_iter in elems_iter.chunks(self.payload_chunk_size).into_iter() { + let coeffs: Vec<_> = coeffs_iter.collect(); + elems_len += coeffs.len(); + polys.push(DenseUVPolynomial::from_coefficients_vec(coeffs)); + } // evaluate polynomials let all_storage_node_evals = { let mut all_storage_node_evals = - vec![Vec::with_capacity(num_polys); self.num_storage_nodes]; + vec![Vec::with_capacity(polys.len()); self.num_storage_nodes]; for poly in polys.iter() { let poly_evals = @@ -324,7 +319,7 @@ where // sanity checks assert_eq!(all_storage_node_evals.len(), self.num_storage_nodes); for storage_node_evals in all_storage_node_evals.iter() { - assert_eq!(storage_node_evals.len(), num_polys); + assert_eq!(storage_node_evals.len(), polys.len()); } all_storage_node_evals