From 9a4a27038a01841c88e6e2218e7162b4cadd9c3f Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Sun, 19 May 2024 23:56:15 +0000 Subject: [PATCH 01/16] chore: add benchmarks for pedersen/schnorr --- .../benches/criterion.rs | 53 ++++++++++++++++++- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/benches/criterion.rs b/acvm-repo/bn254_blackbox_solver/benches/criterion.rs index eb529ed2c11..c4d83b447b8 100644 --- a/acvm-repo/bn254_blackbox_solver/benches/criterion.rs +++ b/acvm-repo/bn254_blackbox_solver/benches/criterion.rs @@ -2,7 +2,8 @@ use criterion::{criterion_group, criterion_main, Criterion}; use std::{hint::black_box, time::Duration}; use acir::FieldElement; -use bn254_blackbox_solver::poseidon2_permutation; +use acvm_blackbox_solver::BlackBoxFunctionSolver; +use bn254_blackbox_solver::{poseidon2_permutation, Bn254BlackBoxSolver}; use pprof::criterion::{Output, PProfProfiler}; @@ -12,10 +13,58 @@ fn bench_poseidon2(c: &mut Criterion) { c.bench_function("poseidon2", |b| b.iter(|| poseidon2_permutation(black_box(&inputs), 4))); } +fn bench_pedersen_commitment(c: &mut Criterion) { + let inputs = [FieldElement::one(); 2]; + let solver = Bn254BlackBoxSolver::new(); + + c.bench_function("pedersen_commitment", |b| { + b.iter(|| solver.pedersen_commitment(black_box(&inputs), 0)) + }); +} + +fn bench_pedersen_hash(c: &mut Criterion) { + let inputs = [FieldElement::one(); 2]; + let solver = Bn254BlackBoxSolver::new(); + + c.bench_function("pedersen_hash", |b| b.iter(|| solver.pedersen_hash(black_box(&inputs), 0))); +} + +fn bench_schnorr_verify(c: &mut Criterion) { + let solver = Bn254BlackBoxSolver::new(); + + let pub_key_x = FieldElement::from_hex( + "0x04b260954662e97f00cab9adb773a259097f7a274b83b113532bce27fa3fb96a", + ) + .unwrap(); + let pub_key_y = FieldElement::from_hex( + "0x2fd51571db6c08666b0edfbfbc57d432068bccd0110a39b166ab243da0037197", + ) + .unwrap(); + let sig_bytes: [u8; 64] = [ + 1, 13, 119, 112, 212, 39, 233, 41, 84, 235, 255, 93, 245, 172, 186, 83, 157, 253, 76, 77, + 33, 128, 178, 15, 214, 67, 105, 107, 177, 234, 77, 48, 27, 237, 155, 84, 39, 84, 247, 27, + 22, 8, 176, 230, 24, 115, 145, 220, 254, 122, 135, 179, 171, 4, 214, 202, 64, 199, 19, 84, + 239, 138, 124, 12, + ]; + + let message: &[u8] = &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]; + + c.bench_function("schnorr_verify", |b| { + b.iter(|| { + solver.schnorr_verify( + black_box(&pub_key_x), + black_box(&pub_key_y), + black_box(&sig_bytes), + black_box(&message), + ) + }) + }); +} + criterion_group!( name = benches; config = Criterion::default().sample_size(40).measurement_time(Duration::from_secs(20)).with_profiler(PProfProfiler::new(100, Output::Flamegraph(None))); - targets = bench_poseidon2 + targets = bench_poseidon2, bench_pedersen_commitment, bench_pedersen_hash, bench_schnorr_verify ); criterion_main!(benches); From 39363a0969a115b3b80a52317ac325dbe22400bf Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 22 Apr 2024 12:05:50 +0000 Subject: [PATCH 02/16] feat: add native rust implementations of pedersen functions --- Cargo.lock | 1 + acvm-repo/bn254_blackbox_solver/Cargo.toml | 1 + .../src/generator/generators.rs | 95 ++++++++++++++ .../src/generator/hash_to_curve.rs | 110 ++++++++++++++++ .../src/generator/mod.rs | 119 ++++++++++++++++++ acvm-repo/bn254_blackbox_solver/src/lib.rs | 2 + .../src/pedersen/commitment.rs | 59 +++++++++ .../src/pedersen/hash.rs | 85 +++++++++++++ .../bn254_blackbox_solver/src/pedersen/mod.rs | 8 ++ cspell.json | 1 + 10 files changed, 481 insertions(+) create mode 100644 acvm-repo/bn254_blackbox_solver/src/generator/generators.rs create mode 100644 acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs create mode 100644 acvm-repo/bn254_blackbox_solver/src/generator/mod.rs create mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs create mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs create mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs diff --git a/Cargo.lock b/Cargo.lock index a8c63c032aa..2e8eeb10a58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -614,6 +614,7 @@ dependencies = [ "acvm_blackbox_solver", "ark-ec", "ark-ff", + "ark-std", "cfg-if 1.0.0", "criterion", "getrandom 0.2.10", diff --git a/acvm-repo/bn254_blackbox_solver/Cargo.toml b/acvm-repo/bn254_blackbox_solver/Cargo.toml index 3a6c9b1d55b..b261be65735 100644 --- a/acvm-repo/bn254_blackbox_solver/Cargo.toml +++ b/acvm-repo/bn254_blackbox_solver/Cargo.toml @@ -40,6 +40,7 @@ getrandom.workspace = true wasmer = "4.2.6" [dev-dependencies] +ark-std = { version = "^0.4.0", default-features = false } criterion = "0.5.0" pprof = { version = "0.12", features = [ "flamegraph", diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs new file mode 100644 index 00000000000..7c6f232dfe7 --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -0,0 +1,95 @@ +// Taken from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rshttps://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/group.rs + +use ark_ec::short_weierstrass::{Affine, SWCurveConfig}; + +use acvm_blackbox_solver::blake3; + +use super::hash_to_curve::hash_to_curve; + +/// Derives generator points via hash-to-curve +/// +/// # ALGORITHM DESCRIPTION +/// +/// 1. Each generator has an associated "generator index" described by its location in the vector +/// 2. a 64-byte preimage buffer is generated with the following structure: +/// - bytes 0-31: BLAKE3 hash of domain_separator +/// - bytes 32-63: generator index in big-endian form +/// 3. The hash-to-curve algorithm is used to hash the above into a group element: +/// +/// a. Iterate `count` upwards from `0` +/// +/// b. Append `count` to the preimage buffer as a 1-byte integer in big-endian form +/// +/// c. Compute `hi = BLAKE3(preimage buffer | 0)` +/// +/// d. Compute `low = BLAKE3(preimage buffer | 1)` +/// +/// e. Interpret `(hi, low)` as limbs of a 512-bit integer +/// +/// f. Reduce 512-bit integer modulo coordinate_field to produce x-coordinate +/// +/// g. Attempt to derive y-coordinate. If not successful go to step (a) and continue +/// +/// h. If parity of y-coordinate's least significant bit does not match parity of most significant bit of +/// (d), invert y-coordinate. +/// +/// j. Return `(x, y)` +/// +/// NOTE: In step 3b it is sufficient to use 1 byte to store `count`. +/// Step 3 has a 50% chance of returning, the probability of `count` exceeding 256 is 1 in 2^256 +/// NOTE: The domain separator is included to ensure that it is possible to derive independent sets of +/// index-addressable generators. +/// NOTE: we produce 64 bytes of BLAKE3 output when producing x-coordinate field +/// element, to ensure that x-coordinate is uniformly randomly distributed in the field. Using a 256-bit input adds +/// significant bias when reducing modulo a ~256-bit coordinate_field +/// NOTE: We ensure y-parity is linked to preimage +/// hash because there is no canonical deterministic square root algorithm (i.e. if a field element has a square +/// root, there are two of them and `field::sqrt` may return either one) +pub(crate) fn derive_generators( + domain_separator_bytes: &[u8], + num_generators: u32, + starting_index: u32, +) -> Vec> { + let mut generator_preimage = [0u8; 64]; + let domain_hash = blake3(&domain_separator_bytes).expect("hash should succeed"); + //1st 32 bytes are blake3 domain_hash + generator_preimage[..32].copy_from_slice(&domain_hash); + + // Convert generator index in big-endian form + let mut res = Vec::with_capacity(num_generators as usize); + for i in starting_index..(starting_index + num_generators) { + let generator_be_bytes: [u8; 4] = i.to_be_bytes(); + generator_preimage[32] = generator_be_bytes[0]; + generator_preimage[33] = generator_be_bytes[1]; + generator_preimage[34] = generator_be_bytes[2]; + generator_preimage[35] = generator_be_bytes[3]; + res.push(hash_to_curve(&generator_preimage, 0)); + } + res +} + +#[cfg(test)] +mod test { + + use super::*; + + #[test] + fn test_derive_generators() { + let res = + derive_generators::("test domain".as_bytes(), 128, 0); + + let is_unique = |y: Affine, j: usize| -> bool { + for (i, res) in res.iter().enumerate() { + if i != j && *res == y { + return false; + } + } + true + }; + + for (i, res) in res.iter().enumerate() { + assert_eq!(is_unique(*res, i), true); + assert_eq!(res.is_on_curve(), true); + } + } +} diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs new file mode 100644 index 00000000000..699c31dde43 --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs @@ -0,0 +1,110 @@ +// Taken from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rs + +use acvm_blackbox_solver::blake3; + +use ark_ec::short_weierstrass::{Affine, SWCurveConfig}; +use ark_ff::Field; + +/// Hash a seed buffer into a point +/// +/// # ALGORITHM DESCRIPTION +/// +/// 1. Initialize unsigned integer `attempt_count = 0` +/// 2. Copy seed into a buffer whose size is 2 bytes greater than `seed` (initialized to `0`) +/// 3. Interpret `attempt_count` as a byte and write into buffer at `[buffer.size() - 2]` +/// 4. Compute Blake3 hash of buffer +/// 5. Set the end byte of the buffer to `1` +/// 6. Compute Blake3 hash of buffer +/// 7. Interpret the two hash outputs as the high / low 256 bits of a 512-bit integer (big-endian) +/// 8. Derive x-coordinate of point by reducing the 512-bit integer modulo the curve's field modulus (Fq) +/// 9. Compute `y^2`` from the curve formula `y^2 = x^3 + ax + b` (`a``, `b`` are curve params. for BN254, `a = 0``, `b = 3``) +/// 10. IF `y^2`` IS NOT A QUADRATIC RESIDUE +/// +/// a. increment `attempt_count` by 1 and go to step 2 +/// +/// 11. IF `y^2`` IS A QUADRATIC RESIDUE +/// +/// a. derive y coordinate via `y = sqrt(y)`` +/// +/// b. Interpret most significant bit of 512-bit integer as a 'parity' bit +/// +/// In Barretenberg: +/// 11c. If parity bit is set AND `y`'s most significant bit is not set, invert `y` +/// 11d. If parity bit is not set AND `y`'s most significant bit is set, invert `y` +/// In Noir we use arkworks https://github.com/arkworks-rs/algebra/blob/master/ec/src/models/short_weierstrass/affine.rs#L110: +/// 11c. If parity bit is set AND `y < -y` lexographically, invert `y` +/// 11d. If parity bit is not set AND `y >= -y` lexographically, invert `y` +/// N.B. last 2 steps are because the `sqrt()` algorithm can return 2 values, +/// we need to a way to canonically distinguish between these 2 values and select a "preferred" one +/// 11e. return (x, y) +/// +pub(crate) fn hash_to_curve(seed: &[u8], attempt_count: u8) -> Affine { + let seed_size = seed.len(); + // expand by 2 bytes to cover incremental hash attempts + let mut target_seed = seed.to_vec(); + target_seed.extend_from_slice(&[0u8; 2]); + + target_seed[seed_size] = attempt_count; + target_seed[seed_size + 1] = 0; + let hash_hi = blake3(&target_seed).expect("hash should succeed"); + target_seed[seed_size + 1] = 1; + let hash_lo = blake3(&target_seed).expect("hash should succeed"); + + let mut hash = hash_hi.to_vec(); + hash.extend_from_slice(&hash_lo); + if let Some(x) = E::BaseField::from_random_bytes(&hash) { + let sign_bit = hash_hi[0] > 127; + if let Some(res) = Affine::get_point_from_x_unchecked(x, sign_bit) { + res + } else { + hash_to_curve(seed, attempt_count + 1) + } + } else { + hash_to_curve(seed, attempt_count + 1) + } +} + +#[cfg(test)] +mod test { + use ark_ec::AffineRepr; + + use super::hash_to_curve; + + #[test] + fn smoke_test() { + // NOTE: test cases are generated from the code above. These need to be checked against barretenberg for consistency! + let test_cases: [(&[u8], u8, (&str, &str)); 3] = [ + ( + &[], + 0, + ( + "15438301111419613682326485500296565426422599273113990340744393358036656182298", + "15066231519297765468214811000625377804446448682562673817844412647425235265754", + ), + ), + ( + &[], + 1, + ( + "11160790032913623338486215764792634663295827616463659385219429623803366672381", + "12758691381125683379000979886391432575683958275657258381114933677979946860616", + ), + ), + ( + &[42], + 0, + ( + "4089244622675092895231715601045494990393179656118214530182684755585786168822", + "13201656830459730190234714570396481523049991863856219427417574483450907106352", + ), + ), + ]; + + for (seed, attempt_count, expected_point) in test_cases { + let point = hash_to_curve::(seed, attempt_count); + assert!(point.is_on_curve()); + assert_eq!(point.x().unwrap().to_string(), expected_point.0); + assert_eq!(point.y().unwrap().to_string(), expected_point.1); + } + } +} diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs b/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs new file mode 100644 index 00000000000..7514d92b084 --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs @@ -0,0 +1,119 @@ +//! This module is adapted from the [Barustenberg][barustenberg] Rust implementation of the Barretenberg library. +//! +//! Code is used under the MIT license +//! +//! [barustenberg]: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/ + +use lazy_static::lazy_static; + +use std::{collections::HashMap, fmt::Debug, sync::Mutex}; + +use ark_ec::{ + short_weierstrass::{Affine, SWCurveConfig}, + AffineRepr, +}; + +mod generators; +mod hash_to_curve; + +pub(crate) use generators::derive_generators; + +pub(crate) const DEFAULT_NUM_GENERATORS: usize = 8; +pub(crate) const DEFAULT_DOMAIN_SEPARATOR: &str = "DEFAULT_DOMAIN_SEPARATOR"; + +//Ref that can be imported to access pre-computed generators +lazy_static! { + pub(crate) static ref GENERATOR_CONTEXT: Mutex> = + Mutex::new(GeneratorContext::default()); +} + +#[derive(Debug, Clone)] +pub(crate) struct GeneratorList(Vec>); + +// In barustenberg there exists a shared ladder storing cached precomputed values. +#[derive(Clone, Debug)] +pub(crate) struct GeneratorData { + pub(crate) precomputed_generators: [Affine; DEFAULT_NUM_GENERATORS], + pub(crate) generator_map: HashMap>, +} + +impl Default for GeneratorData { + fn default() -> Self { + Self { + precomputed_generators: Self::make_precomputed_generators(), + generator_map: HashMap::new(), + } + } +} + +impl GeneratorData { + fn make_precomputed_generators() -> [Affine; DEFAULT_NUM_GENERATORS] { + let mut output: [Affine; DEFAULT_NUM_GENERATORS] = + [Affine::zero(); DEFAULT_NUM_GENERATORS]; + let res: Vec> = derive_generators( + DEFAULT_DOMAIN_SEPARATOR.as_bytes(), + DEFAULT_NUM_GENERATORS as u32, + 0, + ); + output.copy_from_slice(&res[..DEFAULT_NUM_GENERATORS]); + output + } + + //NOTE: can add default arguments by wrapping function parameters with options + pub(crate) fn get( + &mut self, + num_generators: usize, + generator_offset: usize, + domain_separator: &str, + ) -> Vec> { + let is_default_domain = domain_separator == DEFAULT_DOMAIN_SEPARATOR; + if is_default_domain && (num_generators + generator_offset) < DEFAULT_NUM_GENERATORS { + return self.precomputed_generators.to_vec(); + } + + // Case 2: we want default generators, but more than we precomputed at compile time. If we have not yet copied + // the default generators into the map, do so. + if is_default_domain && !self.generator_map.is_empty() { + let _ = self + .generator_map + .insert( + DEFAULT_DOMAIN_SEPARATOR.to_string(), + GeneratorList(self.precomputed_generators.to_vec()), + ) + .unwrap(); + } + + //TODO: open to suggestions for this + let mut generators = self.generator_map.get(DEFAULT_DOMAIN_SEPARATOR).unwrap().0.clone(); + + if num_generators + generator_offset > generators.len() { + let num_extra_generators = num_generators + generator_offset - generators.len(); + let extended_generators = derive_generators( + domain_separator.as_bytes(), + num_extra_generators as u32, + generators.len() as u32, + ); + + generators.extend_from_slice(&extended_generators); + } + + generators + } +} + +#[derive(Debug, Clone)] +pub(crate) struct GeneratorContext { + pub(crate) offset: usize, + pub(crate) domain_separator: &'static str, + pub(crate) generators: GeneratorData, +} + +impl Default for GeneratorContext { + fn default() -> Self { + Self { + offset: 0, + domain_separator: DEFAULT_DOMAIN_SEPARATOR, + generators: GeneratorData::default(), + } + } +} diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index 4cb51b59755..1e411491e7c 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -6,6 +6,8 @@ use acir::{BlackBoxFunc, FieldElement}; use acvm_blackbox_solver::{BlackBoxFunctionSolver, BlackBoxResolutionError}; mod embedded_curve_ops; +mod generator; +mod pedersen; mod poseidon2; mod wasm; diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs new file mode 100644 index 00000000000..8c9c22be938 --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs @@ -0,0 +1,59 @@ +// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson.rs + +use ark_ec::{short_weierstrass::Affine, AffineRepr, CurveGroup}; +use ark_ff::PrimeField; +use grumpkin::{Fq, Fr, GrumpkinParameters}; + +use crate::generator::GeneratorContext; + +/** + * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. + * + * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating + * grumpkin commitments to field elements inside a BN254 SNARK circuit. + * @param inputs + * @param context + * @return Curve::AffineElement + */ +// NOTE: this could be generalized using SWCurveConfig but since we perform the operation over grumpkin its explicit +pub(crate) fn commit_native( + inputs: &[Fq], + context: &mut GeneratorContext, +) -> Affine { + let generators = context.generators.get(inputs.len(), context.offset, context.domain_separator); + + inputs.iter().enumerate().fold(Affine::zero(), |mut acc, (i, input)| { + //TODO: this is a sketch conversion do better + acc = (acc + (generators[i] * Fr::from_bigint(input.into_bigint()).unwrap()).into_affine()) + .into_affine(); + acc + }) +} + +#[cfg(test)] +mod test { + use super::commit_native; + use crate::generator::GENERATOR_CONTEXT; + + use ark_ec::short_weierstrass::Affine; + use ark_ff::MontFp; + use ark_std::One; + use grumpkin::Fq; + + //TODO: double check that the generators are the same. They could be slightly different due to the way we canonically + // decide how to invert y which was done to prevent a headache of having to deseialize an Fq element... Long story. + #[test] + fn commitment() { + let res = commit_native(&[Fq::one(), Fq::one()], &mut GENERATOR_CONTEXT.lock().unwrap()); + let expected = Affine::new( + // 2f7a8f9a6c96926682205fb73ee43215bf13523c19d7afe36f12760266cdfe15 + MontFp!( + "21475250338311530111088781112432132511855209292730670949974692984887182229013" + ), + // 01916b316adbbf0e10e39b18c1d24b33ec84b46daddf72f43878bcc92b6057e6 + MontFp!("709245492126126701709902506217603794644991322680146492508959813283461748710"), + ); + + assert_eq!(res, expected); + } +} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs new file mode 100644 index 00000000000..f5c24c8ab8e --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs @@ -0,0 +1,85 @@ +// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson_hash.rs + +use ark_ec::{ + short_weierstrass::{Affine, SWCurveConfig}, + CurveConfig, CurveGroup, +}; +use grumpkin::GrumpkinParameters; + +use crate::generator::{derive_generators, GeneratorContext}; + +use super::commitment::commit_native; + +/** + * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. + * + * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating + * grumpkin commitments to field elements inside a BN254 SNARK circuit. + * @param inputs + * @param context + * @return Curve::AffineElement + */ +//TODO: confirm we can do this with scalar field +pub(crate) fn hash( + inputs: &[grumpkin::Fq], + context: &mut GeneratorContext, +) -> ::BaseField { + let res: Affine = (length_generator(0) + * ::ScalarField::from(inputs.len() as u64)) + .into_affine(); + (res + commit_native(inputs, context)).x +} + +pub(crate) fn hash_with_index( + inputs: &[grumpkin::Fq], + starting_index: u32, + context: &mut GeneratorContext, +) -> ::BaseField { + let res: Affine = (length_generator(starting_index) + * ::ScalarField::from(inputs.len() as u64)) + .into_affine(); + (res + commit_native(inputs, context)).x +} + +//Note: this can be abstracted to a lazy_static!() +fn length_generator(starting_index: u32) -> Affine { + derive_generators::("pedersen_hash_length".as_bytes(), 1, starting_index)[0] +} + + + +#[cfg(test)] +pub(crate) mod test { + use crate::generator::GENERATOR_CONTEXT; + + use super::*; + + use ark_ff::MontFp; + use ark_std::One; + use grumpkin::Fq; + + //reference: https://github.com/AztecProtocol/barretenberg/blob/master/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp + #[test] + fn hash_one() { + let res = hash(&[Fq::one(), Fq::one()], &mut GENERATOR_CONTEXT.lock().unwrap()); + //TODO: double check that the generators are the same. They could be slightly different due to the way we canonically invert y + //TODO: compute correct value from generators + // 07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b + assert_eq!( + res, + MontFp!("3583137940367543141169889198758850326673923325182598243450662697654714313083") + ); + } + + #[test] + fn hash_with_index() { + let res = hash(&[Fq::one(), Fq::one()], &mut GENERATOR_CONTEXT.lock().unwrap()); + //TODO: double check that the generators are the same. They could be slightly different due to the way we canonically invert y + //TODO: compute correct value from generators + // 07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b + assert_eq!( + res, + MontFp!("3583137940367543141169889198758850326673923325182598243450662697654714313083") + ); + } +} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs new file mode 100644 index 00000000000..1382a78f9ec --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs @@ -0,0 +1,8 @@ +//! This module is adapted from the [Barustenberg][barustenberg] Rust implementation of the Barretenberg library. +//! +//! Code is used under the MIT license +//! +//! [barustenberg]: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/ + +mod commitment; +mod hash; diff --git a/cspell.json b/cspell.json index bf3040265c2..1fbbe5c428d 100644 --- a/cspell.json +++ b/cspell.json @@ -18,6 +18,7 @@ "Backpropagation", "barebones", "barretenberg", + "barustenberg", "bincode", "bindgen", "bitand", From 755c1cdf8573c06fa19fff44f2faac85834dba37 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 29 Apr 2024 10:10:41 +0100 Subject: [PATCH 03/16] chore: linter fixes --- acvm-repo/bn254_blackbox_solver/src/generator/generators.rs | 6 +++--- acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index 7c6f232dfe7..faa4d543f36 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -51,7 +51,7 @@ pub(crate) fn derive_generators( starting_index: u32, ) -> Vec> { let mut generator_preimage = [0u8; 64]; - let domain_hash = blake3(&domain_separator_bytes).expect("hash should succeed"); + let domain_hash = blake3(domain_separator_bytes).expect("hash should succeed"); //1st 32 bytes are blake3 domain_hash generator_preimage[..32].copy_from_slice(&domain_hash); @@ -88,8 +88,8 @@ mod test { }; for (i, res) in res.iter().enumerate() { - assert_eq!(is_unique(*res, i), true); - assert_eq!(res.is_on_curve(), true); + assert!(is_unique(*res, i)); + assert!(res.is_on_curve()); } } } diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs index f5c24c8ab8e..1e3bf2497e8 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs @@ -46,8 +46,6 @@ fn length_generator(starting_index: u32) -> Affine { derive_generators::("pedersen_hash_length".as_bytes(), 1, starting_index)[0] } - - #[cfg(test)] pub(crate) mod test { use crate::generator::GENERATOR_CONTEXT; From 28547d8bec8516c219d9972d6bb34ea135ef907b Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 5 May 2024 01:48:38 +0100 Subject: [PATCH 04/16] chore: apply modulo properly in `hash_to_curve` --- .../src/generator/hash_to_curve.rs | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs index 699c31dde43..2657169000d 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs @@ -3,7 +3,7 @@ use acvm_blackbox_solver::blake3; use ark_ec::short_weierstrass::{Affine, SWCurveConfig}; -use ark_ff::Field; +use ark_ff::{Field, PrimeField}; /// Hash a seed buffer into a point /// @@ -52,16 +52,14 @@ pub(crate) fn hash_to_curve(seed: &[u8], attempt_count: u8) -> let mut hash = hash_hi.to_vec(); hash.extend_from_slice(&hash_lo); - if let Some(x) = E::BaseField::from_random_bytes(&hash) { - let sign_bit = hash_hi[0] > 127; - if let Some(res) = Affine::get_point_from_x_unchecked(x, sign_bit) { - res - } else { - hash_to_curve(seed, attempt_count + 1) - } - } else { - hash_to_curve(seed, attempt_count + 1) - } + + // Here we reduce the 512 bit number modulo the base field modulus to calculate `x` + let x = ::BasePrimeField::from_be_bytes_mod_order(&hash); + let x = E::BaseField::from_base_prime_field(x); + + let parity_bit = hash_hi[0] > 127; + Affine::get_point_from_x_unchecked(x, parity_bit) + .unwrap_or_else(|| hash_to_curve(seed, attempt_count + 1)) } #[cfg(test)] @@ -78,24 +76,24 @@ mod test { &[], 0, ( - "15438301111419613682326485500296565426422599273113990340744393358036656182298", - "15066231519297765468214811000625377804446448682562673817844412647425235265754", + "16630969835852596693293552682274346428810960863289591405364736021328503685422", + "2898904879751428755315857271136646918777181217826622916610882064326950914862", ), ), ( &[], 1, ( - "11160790032913623338486215764792634663295827616463659385219429623803366672381", - "12758691381125683379000979886391432575683958275657258381114933677979946860616", + "16630969835852596693293552682274346428810960863289591405364736021328503685422", + "2898904879751428755315857271136646918777181217826622916610882064326950914862", ), ), ( &[42], 0, ( - "4089244622675092895231715601045494990393179656118214530182684755585786168822", - "13201656830459730190234714570396481523049991863856219427417574483450907106352", + "10062871819776274344541726704728265607389658727752533003234345551987599533646", + "4336697243017116919088392067104139397869626428691238548002362745837171062453", ), ), ]; From f09ae7f387b871a6caa182737b7df393b230ddab Mon Sep 17 00:00:00 2001 From: Tom French Date: Sun, 5 May 2024 02:03:54 +0100 Subject: [PATCH 05/16] chore: update generator docs --- .../src/generator/generators.rs | 33 +++---------------- 1 file changed, 4 insertions(+), 29 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index faa4d543f36..b8279c059c5 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -6,7 +6,7 @@ use acvm_blackbox_solver::blake3; use super::hash_to_curve::hash_to_curve; -/// Derives generator points via hash-to-curve +/// Derives generator points via [hash-to-curve][hash_to_curve]. /// /// # ALGORITHM DESCRIPTION /// @@ -14,37 +14,12 @@ use super::hash_to_curve::hash_to_curve; /// 2. a 64-byte preimage buffer is generated with the following structure: /// - bytes 0-31: BLAKE3 hash of domain_separator /// - bytes 32-63: generator index in big-endian form -/// 3. The hash-to-curve algorithm is used to hash the above into a group element: -/// -/// a. Iterate `count` upwards from `0` +/// 3. The [hash-to-curve algorithm][hash_to_curve] is used to hash the above into a curve point. /// -/// b. Append `count` to the preimage buffer as a 1-byte integer in big-endian form -/// -/// c. Compute `hi = BLAKE3(preimage buffer | 0)` -/// -/// d. Compute `low = BLAKE3(preimage buffer | 1)` -/// -/// e. Interpret `(hi, low)` as limbs of a 512-bit integer -/// -/// f. Reduce 512-bit integer modulo coordinate_field to produce x-coordinate -/// -/// g. Attempt to derive y-coordinate. If not successful go to step (a) and continue -/// -/// h. If parity of y-coordinate's least significant bit does not match parity of most significant bit of -/// (d), invert y-coordinate. -/// -/// j. Return `(x, y)` -/// -/// NOTE: In step 3b it is sufficient to use 1 byte to store `count`. -/// Step 3 has a 50% chance of returning, the probability of `count` exceeding 256 is 1 in 2^256 /// NOTE: The domain separator is included to ensure that it is possible to derive independent sets of /// index-addressable generators. -/// NOTE: we produce 64 bytes of BLAKE3 output when producing x-coordinate field -/// element, to ensure that x-coordinate is uniformly randomly distributed in the field. Using a 256-bit input adds -/// significant bias when reducing modulo a ~256-bit coordinate_field -/// NOTE: We ensure y-parity is linked to preimage -/// hash because there is no canonical deterministic square root algorithm (i.e. if a field element has a square -/// root, there are two of them and `field::sqrt` may return either one) +/// +/// [hash_to_curve]: super::hash_to_curve::hash_to_curve pub(crate) fn derive_generators( domain_separator_bytes: &[u8], num_generators: u32, From 4b0b1ff974dfb50ce9bf91221aac8488103d920c Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Sat, 18 May 2024 20:19:41 +0000 Subject: [PATCH 06/16] chore: fix generator derivation to match barretenberg --- .../src/generator/generators.rs | 76 +++++++++++- .../src/generator/hash_to_curve.rs | 97 ++++++++------- .../src/generator/mod.rs | 112 +----------------- acvm-repo/bn254_blackbox_solver/src/lib.rs | 1 - .../src/pedersen/commitment.rs | 59 --------- .../src/pedersen/hash.rs | 83 ------------- .../bn254_blackbox_solver/src/pedersen/mod.rs | 8 -- 7 files changed, 129 insertions(+), 307 deletions(-) delete mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs delete mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs delete mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index b8279c059c5..ce8b2ec77d1 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -1,8 +1,9 @@ // Taken from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rshttps://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/group.rs -use ark_ec::short_weierstrass::{Affine, SWCurveConfig}; +use ark_ec::short_weierstrass::Affine; use acvm_blackbox_solver::blake3; +use grumpkin::GrumpkinParameters; use super::hash_to_curve::hash_to_curve; @@ -20,11 +21,11 @@ use super::hash_to_curve::hash_to_curve; /// index-addressable generators. /// /// [hash_to_curve]: super::hash_to_curve::hash_to_curve -pub(crate) fn derive_generators( +pub(crate) fn derive_generators( domain_separator_bytes: &[u8], num_generators: u32, starting_index: u32, -) -> Vec> { +) -> Vec> { let mut generator_preimage = [0u8; 64]; let domain_hash = blake3(domain_separator_bytes).expect("hash should succeed"); //1st 32 bytes are blake3 domain_hash @@ -38,7 +39,8 @@ pub(crate) fn derive_generators( generator_preimage[33] = generator_be_bytes[1]; generator_preimage[34] = generator_be_bytes[2]; generator_preimage[35] = generator_be_bytes[3]; - res.push(hash_to_curve(&generator_preimage, 0)); + let generator = hash_to_curve(&generator_preimage, 0); + res.push(generator); } res } @@ -46,12 +48,15 @@ pub(crate) fn derive_generators( #[cfg(test)] mod test { + use ark_ff::{BigInteger, PrimeField}; + use ark_ec::AffineRepr; + use super::*; #[test] fn test_derive_generators() { let res = - derive_generators::("test domain".as_bytes(), 128, 0); + derive_generators("test domain".as_bytes(), 128, 0); let is_unique = |y: Affine, j: usize| -> bool { for (i, res) in res.iter().enumerate() { @@ -67,4 +72,65 @@ mod test { assert!(res.is_on_curve()); } } + + #[test] + fn derive_length_generator() { + let domain_separator = "pedersen_hash_length"; + let length_generator = derive_generators(domain_separator.as_bytes(), 1, 0)[0]; + + let expected_generator = ( + "2df8b940e5890e4e1377e05373fae69a1d754f6935e6a780b666947431f2cdcd", + "2ecd88d15967bc53b885912e0d16866154acb6aac2d3f85e27ca7eefb2c19083" + ); + assert_eq!(hex::encode(length_generator.x().unwrap().into_bigint().to_bytes_be()), expected_generator.0, "Failed on x component"); + assert_eq!(hex::encode(length_generator.y().unwrap().into_bigint().to_bytes_be()), expected_generator.1, "Failed on y component"); + + } + + #[test] + fn derives_default_generators() { + let domain_separator = "DEFAULT_DOMAIN_SEPARATOR"; + + const DEFAULT_GENERATORS: &'static [[&str; 2]] = &[[ + "083e7911d835097629f0067531fc15cafd79a89beecb39903f69572c636f4a5a", + "1a7f5efaad7f315c25a918f30cc8d7333fccab7ad7c90f14de81bcc528f9935d", + ], + [ + "054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402", + "209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126", + ], + [ + "1c44f2a5207c81c28a8321a5815ce8b1311024bbed131819bbdaf5a2ada84748", + "03aaee36e6422a1d0191632ac6599ae9eba5ac2c17a8c920aa3caf8b89c5f8a8", + ], + [ + "26d8b1160c6821a30c65f6cb47124afe01c29f4338f44d4a12c9fccf22fb6fb2", + "05c70c3b9c0d25a4c100e3a27bf3cc375f8af8cdd9498ec4089a823d7464caff", + ], + [ + "20ed9c6a1d27271c4498bfce0578d59db1adbeaa8734f7facc097b9b994fcf6e", + "29cd7d370938b358c62c4a00f73a0d10aba7e5aaa04704a0713f891ebeb92371", + ], + [ + "0224a8abc6c8b8d50373d64cd2a1ab1567bf372b3b1f7b861d7f01257052d383", + "2358629b90eafb299d6650a311e79914b0215eb0a790810b26da5a826726d711", + ], + [ + "0f106f6d46bc904a5290542490b2f238775ff3c445b2f8f704c466655f460a2a", + "29ab84d472f1d33f42fe09c47b8f7710f01920d6155250126731e486877bcf27", + ], + [ + "0298f2e42249f0519c8a8abd91567ebe016e480f219b8c19461d6a595cc33696", + "035bec4b8520a4ece27bd5aafabee3dfe1390d7439c419a8c55aceb207aac83b", + ], + ]; + + let generated_generators = derive_generators(domain_separator.as_bytes(), DEFAULT_GENERATORS.len() as u32, 0); + for (i, (generator, expected_generator)) in generated_generators.iter().zip(DEFAULT_GENERATORS).enumerate() { + assert_eq!(hex::encode(generator.x().unwrap().into_bigint().to_bytes_be()), expected_generator[0], "Failed on x component of generator {i}"); + assert_eq!(hex::encode(generator.y().unwrap().into_bigint().to_bytes_be()), expected_generator[1], "Failed on y component of generator {i}"); + } + + } + } diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs index 2657169000d..65457d9d498 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs @@ -2,8 +2,10 @@ use acvm_blackbox_solver::blake3; -use ark_ec::short_weierstrass::{Affine, SWCurveConfig}; -use ark_ff::{Field, PrimeField}; +use ark_ff::{BigInteger, PrimeField}; +use ark_ec::{short_weierstrass::Affine, AffineRepr, CurveConfig}; +use ark_ff::Field; +use grumpkin::GrumpkinParameters; /// Hash a seed buffer into a point /// @@ -17,28 +19,26 @@ use ark_ff::{Field, PrimeField}; /// 6. Compute Blake3 hash of buffer /// 7. Interpret the two hash outputs as the high / low 256 bits of a 512-bit integer (big-endian) /// 8. Derive x-coordinate of point by reducing the 512-bit integer modulo the curve's field modulus (Fq) -/// 9. Compute `y^2`` from the curve formula `y^2 = x^3 + ax + b` (`a``, `b`` are curve params. for BN254, `a = 0``, `b = 3``) -/// 10. IF `y^2`` IS NOT A QUADRATIC RESIDUE -/// -/// a. increment `attempt_count` by 1 and go to step 2 +/// 9. Compute `y^2` from the curve formula `y^2 = x^3 + ax + b` (`a`, `b` are curve params. for BN254, `a = 0`, `b = 3`) +/// 10. IF `y^2` IS NOT A QUADRATIC RESIDUE: +/// +/// a. increment `attempt_count` by 1 and go to step 2 /// -/// 11. IF `y^2`` IS A QUADRATIC RESIDUE -/// -/// a. derive y coordinate via `y = sqrt(y)`` -/// -/// b. Interpret most significant bit of 512-bit integer as a 'parity' bit -/// -/// In Barretenberg: -/// 11c. If parity bit is set AND `y`'s most significant bit is not set, invert `y` -/// 11d. If parity bit is not set AND `y`'s most significant bit is set, invert `y` -/// In Noir we use arkworks https://github.com/arkworks-rs/algebra/blob/master/ec/src/models/short_weierstrass/affine.rs#L110: -/// 11c. If parity bit is set AND `y < -y` lexographically, invert `y` -/// 11d. If parity bit is not set AND `y >= -y` lexographically, invert `y` -/// N.B. last 2 steps are because the `sqrt()` algorithm can return 2 values, -/// we need to a way to canonically distinguish between these 2 values and select a "preferred" one -/// 11e. return (x, y) +/// 11. IF `y^2` IS A QUADRATIC RESIDUE: +/// +/// a. derive y coordinate via `y = sqrt(y)` +/// +/// b. Interpret most significant bit of 512-bit integer as a 'parity' bit /// -pub(crate) fn hash_to_curve(seed: &[u8], attempt_count: u8) -> Affine { +/// c. If parity bit is set AND `y`'s most significant bit is not set, invert `y` +/// +/// d. If parity bit is not set AND `y`'s most significant bit is set, invert `y` +/// +/// e. return (x, y) +/// +/// N.B. steps c. and e. are because the `sqrt()` algorithm can return 2 values, +/// we need to a way to canonically distinguish between these 2 values and select a "preferred" one +pub(crate) fn hash_to_curve(seed: &[u8], attempt_count: u8) -> Affine { let seed_size = seed.len(); // expand by 2 bytes to cover incremental hash attempts let mut target_seed = seed.to_vec(); @@ -54,55 +54,72 @@ pub(crate) fn hash_to_curve(seed: &[u8], attempt_count: u8) -> hash.extend_from_slice(&hash_lo); // Here we reduce the 512 bit number modulo the base field modulus to calculate `x` - let x = ::BasePrimeField::from_be_bytes_mod_order(&hash); - let x = E::BaseField::from_base_prime_field(x); + let x = <::BaseField as Field>::BasePrimeField::from_be_bytes_mod_order(&hash); + let x = ::BaseField::from_base_prime_field(x); - let parity_bit = hash_hi[0] > 127; - Affine::get_point_from_x_unchecked(x, parity_bit) - .unwrap_or_else(|| hash_to_curve(seed, attempt_count + 1)) + if let Some(point) = Affine::::get_point_from_x_unchecked(x, false) { + let parity_bit = hash_hi[0] > 127; + let y_bit_set = point.y().unwrap().into_bigint().get_bit(0); + if (parity_bit && !y_bit_set) || (!parity_bit && y_bit_set) { + -point + } else { + point + } + } else { + hash_to_curve(seed, attempt_count + 1) + } } #[cfg(test)] mod test { - use ark_ec::AffineRepr; + use ark_ff::{BigInteger, PrimeField}; + use ark_ec::AffineRepr; + use super::hash_to_curve; #[test] fn smoke_test() { - // NOTE: test cases are generated from the code above. These need to be checked against barretenberg for consistency! - let test_cases: [(&[u8], u8, (&str, &str)); 3] = [ + let test_cases: [(&[u8], u8, (&str, &str)); 4] = [ ( &[], 0, ( - "16630969835852596693293552682274346428810960863289591405364736021328503685422", - "2898904879751428755315857271136646918777181217826622916610882064326950914862", + "24c4cb9c1206ab5470592f237f1698abe684dadf0ab4d7a132c32b2134e2c12e", + "0668b8d61a317fb34ccad55c930b3554f1828a0e5530479ecab4defe6bbc0b2e", ), ), ( &[], 1, ( - "16630969835852596693293552682274346428810960863289591405364736021328503685422", - "2898904879751428755315857271136646918777181217826622916610882064326950914862", + "24c4cb9c1206ab5470592f237f1698abe684dadf0ab4d7a132c32b2134e2c12e", + "0668b8d61a317fb34ccad55c930b3554f1828a0e5530479ecab4defe6bbc0b2e", + ), + ), + ( + &[1], + 0, + ( + "107f1b633c6113f3222f39f6256f0546b41a4880918c86864b06471afb410454", + "050cd3823d0c01590b6a50adcc85d2ee4098668fd28805578aa05a423ea938c6", ), ), ( - &[42], + &[0x68, 0x65, 0x6c, 0x6c, 0x6f, 0x20, 0x77, 0x6f, 0x72, 0x6c, 0x64], 0, ( - "10062871819776274344541726704728265607389658727752533003234345551987599533646", - "4336697243017116919088392067104139397869626428691238548002362745837171062453", + "037c5c229ae495f6e8d1b4bf7723fafb2b198b51e27602feb8a4d1053d685093", + "10cf9596c5b2515692d930efa2cf3817607e4796856a79f6af40c949b066969f", ), ), ]; for (seed, attempt_count, expected_point) in test_cases { - let point = hash_to_curve::(seed, attempt_count); + let point = hash_to_curve(seed, attempt_count); assert!(point.is_on_curve()); - assert_eq!(point.x().unwrap().to_string(), expected_point.0); - assert_eq!(point.y().unwrap().to_string(), expected_point.1); + assert_eq!(hex::encode(point.x().unwrap().into_bigint().to_bytes_be()), expected_point.0, "Failed on x component with seed {seed:?}, attempt_count {attempt_count}"); + assert_eq!(hex::encode(point.y().unwrap().into_bigint().to_bytes_be()), expected_point.1, "Failed on y component with seed {seed:?}, attempt_count {attempt_count}"); } } } diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs b/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs index 7514d92b084..39da51d212f 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs @@ -4,116 +4,6 @@ //! //! [barustenberg]: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/ -use lazy_static::lazy_static; - -use std::{collections::HashMap, fmt::Debug, sync::Mutex}; - -use ark_ec::{ - short_weierstrass::{Affine, SWCurveConfig}, - AffineRepr, -}; mod generators; -mod hash_to_curve; - -pub(crate) use generators::derive_generators; - -pub(crate) const DEFAULT_NUM_GENERATORS: usize = 8; -pub(crate) const DEFAULT_DOMAIN_SEPARATOR: &str = "DEFAULT_DOMAIN_SEPARATOR"; - -//Ref that can be imported to access pre-computed generators -lazy_static! { - pub(crate) static ref GENERATOR_CONTEXT: Mutex> = - Mutex::new(GeneratorContext::default()); -} - -#[derive(Debug, Clone)] -pub(crate) struct GeneratorList(Vec>); - -// In barustenberg there exists a shared ladder storing cached precomputed values. -#[derive(Clone, Debug)] -pub(crate) struct GeneratorData { - pub(crate) precomputed_generators: [Affine; DEFAULT_NUM_GENERATORS], - pub(crate) generator_map: HashMap>, -} - -impl Default for GeneratorData { - fn default() -> Self { - Self { - precomputed_generators: Self::make_precomputed_generators(), - generator_map: HashMap::new(), - } - } -} - -impl GeneratorData { - fn make_precomputed_generators() -> [Affine; DEFAULT_NUM_GENERATORS] { - let mut output: [Affine; DEFAULT_NUM_GENERATORS] = - [Affine::zero(); DEFAULT_NUM_GENERATORS]; - let res: Vec> = derive_generators( - DEFAULT_DOMAIN_SEPARATOR.as_bytes(), - DEFAULT_NUM_GENERATORS as u32, - 0, - ); - output.copy_from_slice(&res[..DEFAULT_NUM_GENERATORS]); - output - } - - //NOTE: can add default arguments by wrapping function parameters with options - pub(crate) fn get( - &mut self, - num_generators: usize, - generator_offset: usize, - domain_separator: &str, - ) -> Vec> { - let is_default_domain = domain_separator == DEFAULT_DOMAIN_SEPARATOR; - if is_default_domain && (num_generators + generator_offset) < DEFAULT_NUM_GENERATORS { - return self.precomputed_generators.to_vec(); - } - - // Case 2: we want default generators, but more than we precomputed at compile time. If we have not yet copied - // the default generators into the map, do so. - if is_default_domain && !self.generator_map.is_empty() { - let _ = self - .generator_map - .insert( - DEFAULT_DOMAIN_SEPARATOR.to_string(), - GeneratorList(self.precomputed_generators.to_vec()), - ) - .unwrap(); - } - - //TODO: open to suggestions for this - let mut generators = self.generator_map.get(DEFAULT_DOMAIN_SEPARATOR).unwrap().0.clone(); - - if num_generators + generator_offset > generators.len() { - let num_extra_generators = num_generators + generator_offset - generators.len(); - let extended_generators = derive_generators( - domain_separator.as_bytes(), - num_extra_generators as u32, - generators.len() as u32, - ); - - generators.extend_from_slice(&extended_generators); - } - - generators - } -} - -#[derive(Debug, Clone)] -pub(crate) struct GeneratorContext { - pub(crate) offset: usize, - pub(crate) domain_separator: &'static str, - pub(crate) generators: GeneratorData, -} - -impl Default for GeneratorContext { - fn default() -> Self { - Self { - offset: 0, - domain_separator: DEFAULT_DOMAIN_SEPARATOR, - generators: GeneratorData::default(), - } - } -} +mod hash_to_curve; \ No newline at end of file diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index 1e411491e7c..b90cac52f0e 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -7,7 +7,6 @@ use acvm_blackbox_solver::{BlackBoxFunctionSolver, BlackBoxResolutionError}; mod embedded_curve_ops; mod generator; -mod pedersen; mod poseidon2; mod wasm; diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs deleted file mode 100644 index 8c9c22be938..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs +++ /dev/null @@ -1,59 +0,0 @@ -// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson.rs - -use ark_ec::{short_weierstrass::Affine, AffineRepr, CurveGroup}; -use ark_ff::PrimeField; -use grumpkin::{Fq, Fr, GrumpkinParameters}; - -use crate::generator::GeneratorContext; - -/** - * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. - * - * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating - * grumpkin commitments to field elements inside a BN254 SNARK circuit. - * @param inputs - * @param context - * @return Curve::AffineElement - */ -// NOTE: this could be generalized using SWCurveConfig but since we perform the operation over grumpkin its explicit -pub(crate) fn commit_native( - inputs: &[Fq], - context: &mut GeneratorContext, -) -> Affine { - let generators = context.generators.get(inputs.len(), context.offset, context.domain_separator); - - inputs.iter().enumerate().fold(Affine::zero(), |mut acc, (i, input)| { - //TODO: this is a sketch conversion do better - acc = (acc + (generators[i] * Fr::from_bigint(input.into_bigint()).unwrap()).into_affine()) - .into_affine(); - acc - }) -} - -#[cfg(test)] -mod test { - use super::commit_native; - use crate::generator::GENERATOR_CONTEXT; - - use ark_ec::short_weierstrass::Affine; - use ark_ff::MontFp; - use ark_std::One; - use grumpkin::Fq; - - //TODO: double check that the generators are the same. They could be slightly different due to the way we canonically - // decide how to invert y which was done to prevent a headache of having to deseialize an Fq element... Long story. - #[test] - fn commitment() { - let res = commit_native(&[Fq::one(), Fq::one()], &mut GENERATOR_CONTEXT.lock().unwrap()); - let expected = Affine::new( - // 2f7a8f9a6c96926682205fb73ee43215bf13523c19d7afe36f12760266cdfe15 - MontFp!( - "21475250338311530111088781112432132511855209292730670949974692984887182229013" - ), - // 01916b316adbbf0e10e39b18c1d24b33ec84b46daddf72f43878bcc92b6057e6 - MontFp!("709245492126126701709902506217603794644991322680146492508959813283461748710"), - ); - - assert_eq!(res, expected); - } -} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs deleted file mode 100644 index 1e3bf2497e8..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs +++ /dev/null @@ -1,83 +0,0 @@ -// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson_hash.rs - -use ark_ec::{ - short_weierstrass::{Affine, SWCurveConfig}, - CurveConfig, CurveGroup, -}; -use grumpkin::GrumpkinParameters; - -use crate::generator::{derive_generators, GeneratorContext}; - -use super::commitment::commit_native; - -/** - * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. - * - * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating - * grumpkin commitments to field elements inside a BN254 SNARK circuit. - * @param inputs - * @param context - * @return Curve::AffineElement - */ -//TODO: confirm we can do this with scalar field -pub(crate) fn hash( - inputs: &[grumpkin::Fq], - context: &mut GeneratorContext, -) -> ::BaseField { - let res: Affine = (length_generator(0) - * ::ScalarField::from(inputs.len() as u64)) - .into_affine(); - (res + commit_native(inputs, context)).x -} - -pub(crate) fn hash_with_index( - inputs: &[grumpkin::Fq], - starting_index: u32, - context: &mut GeneratorContext, -) -> ::BaseField { - let res: Affine = (length_generator(starting_index) - * ::ScalarField::from(inputs.len() as u64)) - .into_affine(); - (res + commit_native(inputs, context)).x -} - -//Note: this can be abstracted to a lazy_static!() -fn length_generator(starting_index: u32) -> Affine { - derive_generators::("pedersen_hash_length".as_bytes(), 1, starting_index)[0] -} - -#[cfg(test)] -pub(crate) mod test { - use crate::generator::GENERATOR_CONTEXT; - - use super::*; - - use ark_ff::MontFp; - use ark_std::One; - use grumpkin::Fq; - - //reference: https://github.com/AztecProtocol/barretenberg/blob/master/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp - #[test] - fn hash_one() { - let res = hash(&[Fq::one(), Fq::one()], &mut GENERATOR_CONTEXT.lock().unwrap()); - //TODO: double check that the generators are the same. They could be slightly different due to the way we canonically invert y - //TODO: compute correct value from generators - // 07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b - assert_eq!( - res, - MontFp!("3583137940367543141169889198758850326673923325182598243450662697654714313083") - ); - } - - #[test] - fn hash_with_index() { - let res = hash(&[Fq::one(), Fq::one()], &mut GENERATOR_CONTEXT.lock().unwrap()); - //TODO: double check that the generators are the same. They could be slightly different due to the way we canonically invert y - //TODO: compute correct value from generators - // 07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b - assert_eq!( - res, - MontFp!("3583137940367543141169889198758850326673923325182598243450662697654714313083") - ); - } -} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs deleted file mode 100644 index 1382a78f9ec..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs +++ /dev/null @@ -1,8 +0,0 @@ -//! This module is adapted from the [Barustenberg][barustenberg] Rust implementation of the Barretenberg library. -//! -//! Code is used under the MIT license -//! -//! [barustenberg]: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/ - -mod commitment; -mod hash; From 28fd8a74e0e5b8ba30d04bf5a8581bf2fffa3d69 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Sat, 18 May 2024 21:25:03 +0000 Subject: [PATCH 07/16] feat: switch over to using native pedersen functions --- .../src/generator/generators.rs | 6 +- .../src/generator/mod.rs | 2 +- acvm-repo/bn254_blackbox_solver/src/lib.rs | 21 +++--- .../src/pedersen/commitment.rs | 72 ++++++++++++++++++ .../src/pedersen/hash.rs | 68 +++++++++++++++++ .../bn254_blackbox_solver/src/pedersen/mod.rs | 2 + .../src/wasm/barretenberg_structures.rs | 25 ------- .../bn254_blackbox_solver/src/wasm/mod.rs | 20 ----- .../src/wasm/pedersen.rs | 73 ------------------- 9 files changed, 159 insertions(+), 130 deletions(-) create mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs create mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs create mode 100644 acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs delete mode 100644 acvm-repo/bn254_blackbox_solver/src/wasm/barretenberg_structures.rs delete mode 100644 acvm-repo/bn254_blackbox_solver/src/wasm/pedersen.rs diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index ce8b2ec77d1..9bab1189b64 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -7,6 +7,9 @@ use grumpkin::GrumpkinParameters; use super::hash_to_curve::hash_to_curve; +pub(crate) const DEFAULT_DOMAIN_SEPARATOR: &[u8] = "DEFAULT_DOMAIN_SEPARATOR".as_bytes(); + + /// Derives generator points via [hash-to-curve][hash_to_curve]. /// /// # ALGORITHM DESCRIPTION @@ -89,7 +92,6 @@ mod test { #[test] fn derives_default_generators() { - let domain_separator = "DEFAULT_DOMAIN_SEPARATOR"; const DEFAULT_GENERATORS: &'static [[&str; 2]] = &[[ "083e7911d835097629f0067531fc15cafd79a89beecb39903f69572c636f4a5a", @@ -125,7 +127,7 @@ mod test { ], ]; - let generated_generators = derive_generators(domain_separator.as_bytes(), DEFAULT_GENERATORS.len() as u32, 0); + let generated_generators = derive_generators(DEFAULT_DOMAIN_SEPARATOR, DEFAULT_GENERATORS.len() as u32, 0); for (i, (generator, expected_generator)) in generated_generators.iter().zip(DEFAULT_GENERATORS).enumerate() { assert_eq!(hex::encode(generator.x().unwrap().into_bigint().to_bytes_be()), expected_generator[0], "Failed on x component of generator {i}"); assert_eq!(hex::encode(generator.y().unwrap().into_bigint().to_bytes_be()), expected_generator[1], "Failed on y component of generator {i}"); diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs b/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs index 39da51d212f..bde58615ffc 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/mod.rs @@ -5,5 +5,5 @@ //! [barustenberg]: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/ -mod generators; +pub(crate) mod generators; mod hash_to_curve; \ No newline at end of file diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index b90cac52f0e..7ca2d4277b8 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -9,12 +9,14 @@ mod embedded_curve_ops; mod generator; mod poseidon2; mod wasm; +mod pedersen; +use ark_ec::AffineRepr; pub use embedded_curve_ops::{embedded_curve_add, multi_scalar_mul}; pub use poseidon2::poseidon2_permutation; use wasm::Barretenberg; -use self::wasm::{Pedersen, SchnorrSig}; +use self::wasm::SchnorrSig; pub struct Bn254BlackBoxSolver { blackbox_vendor: Barretenberg, @@ -73,10 +75,11 @@ impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { inputs: &[FieldElement], domain_separator: u32, ) -> Result<(FieldElement, FieldElement), BlackBoxResolutionError> { - #[allow(deprecated)] - self.blackbox_vendor.encrypt(inputs.to_vec(), domain_separator).map_err(|err| { - BlackBoxResolutionError::Failed(BlackBoxFunc::PedersenCommitment, err.to_string()) - }) + let inputs: Vec = inputs.iter().map(|input| input.into_repr()).collect(); + let result = pedersen::commitment::commit_native_with_index(&inputs, domain_separator); + let res_x = FieldElement::from_repr(*result.x().expect("should not commit to point at infinity")); + let res_y = FieldElement::from_repr(*result.y().expect("should not commit to point at infinity")); + Ok((res_x, res_y)) } fn pedersen_hash( @@ -84,10 +87,10 @@ impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { inputs: &[FieldElement], domain_separator: u32, ) -> Result { - #[allow(deprecated)] - self.blackbox_vendor.hash(inputs.to_vec(), domain_separator).map_err(|err| { - BlackBoxResolutionError::Failed(BlackBoxFunc::PedersenCommitment, err.to_string()) - }) + let inputs: Vec = inputs.iter().map(|input| input.into_repr()).collect(); + let result = pedersen::hash::hash_with_index(&inputs, domain_separator); + let result = FieldElement::from_repr(result); + Ok(result) } fn multi_scalar_mul( diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs new file mode 100644 index 00000000000..37c71c1accc --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs @@ -0,0 +1,72 @@ +// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson.rs + +use ark_ec::{short_weierstrass::Affine, AffineRepr, CurveGroup}; +use ark_ff::PrimeField; +use grumpkin::{Fq, Fr, GrumpkinParameters}; + +use crate::generator::generators::{derive_generators, DEFAULT_DOMAIN_SEPARATOR}; + +/** + * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. + * + * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating + * grumpkin commitments to field elements inside a BN254 SNARK circuit. + * @param inputs + * @param context + * @return Curve::AffineElement + */ +// NOTE: this could be generalized using SWCurveConfig but since we perform the operation over grumpkin its explicit +pub(crate) fn commit_native_with_index( + inputs: &[Fq], + starting_index: u32 +) -> Affine { + let generators = derive_generators(DEFAULT_DOMAIN_SEPARATOR, inputs.len() as u32, starting_index); + + inputs.iter().enumerate().fold(Affine::zero(), |mut acc, (i, input)| { + //TODO: this is a sketch conversion do better + acc = (acc + (generators[i] * Fr::from_bigint(input.into_bigint()).unwrap()).into_affine()) + .into_affine(); + acc + }) +} + +#[cfg(test)] +mod test { + + use ark_ec::short_weierstrass::Affine; + use ark_ff::MontFp; + use ark_std::{One, Zero}; + use grumpkin::Fq; + + use crate::pedersen::commitment::commit_native_with_index; + + #[test] + fn commitment() { + let res = commit_native_with_index(&[Fq::one(), Fq::one()], 0); + let expected = Affine::new( + // 2f7a8f9a6c96926682205fb73ee43215bf13523c19d7afe36f12760266cdfe15 + MontFp!( + "21475250338311530111088781112432132511855209292730670949974692984887182229013" + ), + // 01916b316adbbf0e10e39b18c1d24b33ec84b46daddf72f43878bcc92b6057e6 + MontFp!("709245492126126701709902506217603794644991322680146492508959813283461748710"), + ); + + assert_eq!(res, expected); + } + + #[test] + fn commitment_with_zero() { + let res = commit_native_with_index(&[Fq::zero(), Fq::one()], 0); + let expected = Affine::new( + // 054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402 + MontFp!( + "2393473289045184898987089634332637236754766663897650125720167164137088869378" + ), + // 209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126 + MontFp!("14752839959415467457196082350231122454649853219840744672802853620609001898278"), + ); + + assert_eq!(res, expected); + } +} \ No newline at end of file diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs new file mode 100644 index 00000000000..3a68b6f5600 --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs @@ -0,0 +1,68 @@ +// Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson_hash.rs + +use ark_ec::{ + short_weierstrass::Affine, + CurveConfig, CurveGroup, +}; +use grumpkin::GrumpkinParameters; + +use crate::generator::generators::derive_generators; + +use super::commitment::commit_native_with_index; + +/** + * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. + * + * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating + * grumpkin commitments to field elements inside a BN254 SNARK circuit. + * @param inputs + * @param context + * @return Curve::AffineElement + */ +//TODO: confirm we can do this with scalar field +pub(crate) fn hash_with_index( + inputs: &[grumpkin::Fq], + starting_index: u32, +) -> ::BaseField { + let length_as_scalar: ::ScalarField = (inputs.len() as u64).into(); + let length_prefix = length_generator(0) * length_as_scalar; + let result = length_prefix + commit_native_with_index(inputs, starting_index); + result.into_affine().x +} + +//Note: this can be abstracted to a lazy_static!() +fn length_generator(starting_index: u32) -> Affine { + derive_generators("pedersen_hash_length".as_bytes(), 1, starting_index)[0] +} + +#[cfg(test)] +pub(crate) mod test { + + use super::*; + + use ark_ff::MontFp; + use ark_std::One; + use grumpkin::Fq; + + //reference: https://github.com/AztecProtocol/barretenberg/blob/master/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp + #[test] + fn hash_one() { + let res = hash_with_index(&[Fq::one(), Fq::one()], 0); + + // 07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b + assert_eq!( + res, + MontFp!("3583137940367543141169889198758850326673923325182598243450662697654714313083") + ); + } + + #[test] + fn test_hash_with_index() { + let res = hash_with_index(&[Fq::one(), Fq::one()], 5); + // 1c446df60816b897cda124524e6b03f36df0cec333fad87617aab70d7861daa6 + assert_eq!( + res, + MontFp!("12785664284086914537273210116175139764153812914951498056047869066787449592486") + ); + } +} \ No newline at end of file diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs new file mode 100644 index 00000000000..615690fddbf --- /dev/null +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs @@ -0,0 +1,2 @@ +pub(crate) mod commitment; +pub(crate) mod hash; \ No newline at end of file diff --git a/acvm-repo/bn254_blackbox_solver/src/wasm/barretenberg_structures.rs b/acvm-repo/bn254_blackbox_solver/src/wasm/barretenberg_structures.rs deleted file mode 100644 index 302ffa8af9b..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/wasm/barretenberg_structures.rs +++ /dev/null @@ -1,25 +0,0 @@ -use acir::FieldElement; - -#[derive(Debug, Default)] -pub(crate) struct Assignments(Vec); - -impl Assignments { - pub(crate) fn to_bytes(&self) -> Vec { - let mut buffer = Vec::new(); - - let witness_len = self.0.len() as u32; - buffer.extend_from_slice(&witness_len.to_be_bytes()); - - for assignment in self.0.iter() { - buffer.extend_from_slice(&assignment.to_be_bytes()); - } - - buffer - } -} - -impl From> for Assignments { - fn from(w: Vec) -> Assignments { - Assignments(w) - } -} diff --git a/acvm-repo/bn254_blackbox_solver/src/wasm/mod.rs b/acvm-repo/bn254_blackbox_solver/src/wasm/mod.rs index f4f6f56aa99..e0a5c4c9069 100644 --- a/acvm-repo/bn254_blackbox_solver/src/wasm/mod.rs +++ b/acvm-repo/bn254_blackbox_solver/src/wasm/mod.rs @@ -4,13 +4,8 @@ //! //! As [`acvm`] includes rust implementations for these opcodes, this module can be removed. -mod barretenberg_structures; -mod pedersen; mod schnorr; -use barretenberg_structures::Assignments; - -pub(crate) use pedersen::Pedersen; pub(crate) use schnorr::SchnorrSig; /// The number of bytes necessary to store a `FieldElement`. @@ -208,10 +203,6 @@ impl Barretenberg { buf } - pub(crate) fn call(&self, name: &str, param: &WASMValue) -> Result { - self.call_multiple(name, vec![param]) - } - pub(crate) fn call_multiple( &self, name: &str, @@ -236,17 +227,6 @@ impl Barretenberg { Ok(WASMValue(option_value)) } - - /// Creates a pointer and allocates the bytes that the pointer references to, to the heap - pub(crate) fn allocate(&self, bytes: &[u8]) -> Result { - let ptr: i32 = self.call("bbmalloc", &bytes.len().into())?.try_into()?; - - let i32_bytes = ptr.to_be_bytes(); - let u32_bytes = u32::from_be_bytes(i32_bytes); - - self.transfer_to_heap(bytes, u32_bytes as usize); - Ok(ptr.into()) - } } fn init_memory_and_state() -> (Memory, Store, Imports) { diff --git a/acvm-repo/bn254_blackbox_solver/src/wasm/pedersen.rs b/acvm-repo/bn254_blackbox_solver/src/wasm/pedersen.rs deleted file mode 100644 index c816e5b4d1b..00000000000 --- a/acvm-repo/bn254_blackbox_solver/src/wasm/pedersen.rs +++ /dev/null @@ -1,73 +0,0 @@ -use acir::FieldElement; - -use super::{Assignments, Barretenberg, Error, FIELD_BYTES}; - -pub(crate) trait Pedersen { - fn encrypt( - &self, - inputs: Vec, - hash_index: u32, - ) -> Result<(FieldElement, FieldElement), Error>; - - fn hash(&self, inputs: Vec, hash_index: u32) -> Result; -} - -impl Pedersen for Barretenberg { - fn encrypt( - &self, - inputs: Vec, - hash_index: u32, - ) -> Result<(FieldElement, FieldElement), Error> { - let input_buf = Assignments::from(inputs).to_bytes(); - let input_ptr = self.allocate(&input_buf)?; - let result_ptr: usize = 0; - - self.call_multiple( - "pedersen_plookup_commit_with_hash_index", - vec![&input_ptr, &result_ptr.into(), &hash_index.into()], - )?; - - let result_bytes: [u8; 2 * FIELD_BYTES] = self.read_memory(result_ptr); - let (point_x_bytes, point_y_bytes) = result_bytes.split_at(FIELD_BYTES); - - let point_x = FieldElement::from_be_bytes_reduce(point_x_bytes); - let point_y = FieldElement::from_be_bytes_reduce(point_y_bytes); - - Ok((point_x, point_y)) - } - - fn hash(&self, inputs: Vec, hash_index: u32) -> Result { - let input_buf = Assignments::from(inputs).to_bytes(); - let input_ptr = self.allocate(&input_buf)?; - let result_ptr: usize = 0; - - self.call_multiple( - "pedersen_plookup_compress_with_hash_index", - vec![&input_ptr, &result_ptr.into(), &hash_index.into()], - )?; - - let result_bytes: [u8; FIELD_BYTES] = self.read_memory(result_ptr); - - let hash = FieldElement::from_be_bytes_reduce(&result_bytes); - - Ok(hash) - } -} - -#[test] -fn pedersen_hash_to_point() -> Result<(), Error> { - let barretenberg = Barretenberg::new(); - let (x, y) = barretenberg.encrypt(vec![FieldElement::one(), FieldElement::one()], 1)?; - let expected_x = FieldElement::from_hex( - "0x12afb43195f5c621d1d2cabb5f629707095c5307fd4185a663d4e80bb083e878", - ) - .unwrap(); - let expected_y = FieldElement::from_hex( - "0x25793f5b5e62beb92fd18a66050293a9fd554a2ff13bceba0339cae1a038d7c1", - ) - .unwrap(); - - assert_eq!(expected_x.to_hex(), x.to_hex()); - assert_eq!(expected_y.to_hex(), y.to_hex()); - Ok(()) -} From 43030525b49127c961a68709a3d19bcb3acffee7 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Sat, 18 May 2024 21:26:48 +0000 Subject: [PATCH 08/16] chore: clippy --- acvm-repo/bn254_blackbox_solver/src/generator/generators.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index 9bab1189b64..1413df69f0c 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -93,7 +93,7 @@ mod test { #[test] fn derives_default_generators() { - const DEFAULT_GENERATORS: &'static [[&str; 2]] = &[[ + const DEFAULT_GENERATORS: &[[&str; 2]] = &[[ "083e7911d835097629f0067531fc15cafd79a89beecb39903f69572c636f4a5a", "1a7f5efaad7f315c25a918f30cc8d7333fccab7ad7c90f14de81bcc528f9935d", ], From e8d9f5840957f46d3e4af33c76cd296c6f8a389b Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Sat, 18 May 2024 21:29:00 +0000 Subject: [PATCH 09/16] chore: cargo fmt --- .../src/generator/generators.rs | 110 ++++++++++-------- .../src/generator/hash_to_curve.rs | 24 ++-- .../src/generator/mod.rs | 3 +- acvm-repo/bn254_blackbox_solver/src/lib.rs | 8 +- .../src/pedersen/commitment.rs | 15 +-- .../src/pedersen/hash.rs | 14 +-- .../bn254_blackbox_solver/src/pedersen/mod.rs | 2 +- 7 files changed, 100 insertions(+), 76 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index 1413df69f0c..6f9c72f7ebb 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -9,7 +9,6 @@ use super::hash_to_curve::hash_to_curve; pub(crate) const DEFAULT_DOMAIN_SEPARATOR: &[u8] = "DEFAULT_DOMAIN_SEPARATOR".as_bytes(); - /// Derives generator points via [hash-to-curve][hash_to_curve]. /// /// # ALGORITHM DESCRIPTION @@ -51,15 +50,14 @@ pub(crate) fn derive_generators( #[cfg(test)] mod test { - use ark_ff::{BigInteger, PrimeField}; use ark_ec::AffineRepr; + use ark_ff::{BigInteger, PrimeField}; use super::*; #[test] fn test_derive_generators() { - let res = - derive_generators("test domain".as_bytes(), 128, 0); + let res = derive_generators("test domain".as_bytes(), 128, 0); let is_unique = |y: Affine, j: usize| -> bool { for (i, res) in res.iter().enumerate() { @@ -80,59 +78,75 @@ mod test { fn derive_length_generator() { let domain_separator = "pedersen_hash_length"; let length_generator = derive_generators(domain_separator.as_bytes(), 1, 0)[0]; - + let expected_generator = ( "2df8b940e5890e4e1377e05373fae69a1d754f6935e6a780b666947431f2cdcd", - "2ecd88d15967bc53b885912e0d16866154acb6aac2d3f85e27ca7eefb2c19083" + "2ecd88d15967bc53b885912e0d16866154acb6aac2d3f85e27ca7eefb2c19083", + ); + assert_eq!( + hex::encode(length_generator.x().unwrap().into_bigint().to_bytes_be()), + expected_generator.0, + "Failed on x component" + ); + assert_eq!( + hex::encode(length_generator.y().unwrap().into_bigint().to_bytes_be()), + expected_generator.1, + "Failed on y component" ); - assert_eq!(hex::encode(length_generator.x().unwrap().into_bigint().to_bytes_be()), expected_generator.0, "Failed on x component"); - assert_eq!(hex::encode(length_generator.y().unwrap().into_bigint().to_bytes_be()), expected_generator.1, "Failed on y component"); - } #[test] fn derives_default_generators() { - - const DEFAULT_GENERATORS: &[[&str; 2]] = &[[ - "083e7911d835097629f0067531fc15cafd79a89beecb39903f69572c636f4a5a", - "1a7f5efaad7f315c25a918f30cc8d7333fccab7ad7c90f14de81bcc528f9935d", - ], - [ - "054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402", - "209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126", - ], - [ - "1c44f2a5207c81c28a8321a5815ce8b1311024bbed131819bbdaf5a2ada84748", - "03aaee36e6422a1d0191632ac6599ae9eba5ac2c17a8c920aa3caf8b89c5f8a8", - ], - [ - "26d8b1160c6821a30c65f6cb47124afe01c29f4338f44d4a12c9fccf22fb6fb2", - "05c70c3b9c0d25a4c100e3a27bf3cc375f8af8cdd9498ec4089a823d7464caff", - ], - [ - "20ed9c6a1d27271c4498bfce0578d59db1adbeaa8734f7facc097b9b994fcf6e", - "29cd7d370938b358c62c4a00f73a0d10aba7e5aaa04704a0713f891ebeb92371", - ], - [ - "0224a8abc6c8b8d50373d64cd2a1ab1567bf372b3b1f7b861d7f01257052d383", - "2358629b90eafb299d6650a311e79914b0215eb0a790810b26da5a826726d711", - ], - [ - "0f106f6d46bc904a5290542490b2f238775ff3c445b2f8f704c466655f460a2a", - "29ab84d472f1d33f42fe09c47b8f7710f01920d6155250126731e486877bcf27", - ], - [ - "0298f2e42249f0519c8a8abd91567ebe016e480f219b8c19461d6a595cc33696", - "035bec4b8520a4ece27bd5aafabee3dfe1390d7439c419a8c55aceb207aac83b", - ], + const DEFAULT_GENERATORS: &[[&str; 2]] = &[ + [ + "083e7911d835097629f0067531fc15cafd79a89beecb39903f69572c636f4a5a", + "1a7f5efaad7f315c25a918f30cc8d7333fccab7ad7c90f14de81bcc528f9935d", + ], + [ + "054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402", + "209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126", + ], + [ + "1c44f2a5207c81c28a8321a5815ce8b1311024bbed131819bbdaf5a2ada84748", + "03aaee36e6422a1d0191632ac6599ae9eba5ac2c17a8c920aa3caf8b89c5f8a8", + ], + [ + "26d8b1160c6821a30c65f6cb47124afe01c29f4338f44d4a12c9fccf22fb6fb2", + "05c70c3b9c0d25a4c100e3a27bf3cc375f8af8cdd9498ec4089a823d7464caff", + ], + [ + "20ed9c6a1d27271c4498bfce0578d59db1adbeaa8734f7facc097b9b994fcf6e", + "29cd7d370938b358c62c4a00f73a0d10aba7e5aaa04704a0713f891ebeb92371", + ], + [ + "0224a8abc6c8b8d50373d64cd2a1ab1567bf372b3b1f7b861d7f01257052d383", + "2358629b90eafb299d6650a311e79914b0215eb0a790810b26da5a826726d711", + ], + [ + "0f106f6d46bc904a5290542490b2f238775ff3c445b2f8f704c466655f460a2a", + "29ab84d472f1d33f42fe09c47b8f7710f01920d6155250126731e486877bcf27", + ], + [ + "0298f2e42249f0519c8a8abd91567ebe016e480f219b8c19461d6a595cc33696", + "035bec4b8520a4ece27bd5aafabee3dfe1390d7439c419a8c55aceb207aac83b", + ], ]; - let generated_generators = derive_generators(DEFAULT_DOMAIN_SEPARATOR, DEFAULT_GENERATORS.len() as u32, 0); - for (i, (generator, expected_generator)) in generated_generators.iter().zip(DEFAULT_GENERATORS).enumerate() { - assert_eq!(hex::encode(generator.x().unwrap().into_bigint().to_bytes_be()), expected_generator[0], "Failed on x component of generator {i}"); - assert_eq!(hex::encode(generator.y().unwrap().into_bigint().to_bytes_be()), expected_generator[1], "Failed on y component of generator {i}"); + let generated_generators = + derive_generators(DEFAULT_DOMAIN_SEPARATOR, DEFAULT_GENERATORS.len() as u32, 0); + for (i, (generator, expected_generator)) in + generated_generators.iter().zip(DEFAULT_GENERATORS).enumerate() + { + assert_eq!( + hex::encode(generator.x().unwrap().into_bigint().to_bytes_be()), + expected_generator[0], + "Failed on x component of generator {i}" + ); + assert_eq!( + hex::encode(generator.y().unwrap().into_bigint().to_bytes_be()), + expected_generator[1], + "Failed on y component of generator {i}" + ); } - } - } diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs index 65457d9d498..cfa25ec0b4d 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs @@ -2,9 +2,9 @@ use acvm_blackbox_solver::blake3; -use ark_ff::{BigInteger, PrimeField}; use ark_ec::{short_weierstrass::Affine, AffineRepr, CurveConfig}; use ark_ff::Field; +use ark_ff::{BigInteger, PrimeField}; use grumpkin::GrumpkinParameters; /// Hash a seed buffer into a point @@ -33,9 +33,9 @@ use grumpkin::GrumpkinParameters; /// c. If parity bit is set AND `y`'s most significant bit is not set, invert `y` /// /// d. If parity bit is not set AND `y`'s most significant bit is set, invert `y` -/// +/// /// e. return (x, y) -/// +/// /// N.B. steps c. and e. are because the `sqrt()` algorithm can return 2 values, /// we need to a way to canonically distinguish between these 2 values and select a "preferred" one pub(crate) fn hash_to_curve(seed: &[u8], attempt_count: u8) -> Affine { @@ -67,15 +67,15 @@ pub(crate) fn hash_to_curve(seed: &[u8], attempt_count: u8) -> Affine Result<(FieldElement, FieldElement), BlackBoxResolutionError> { let inputs: Vec = inputs.iter().map(|input| input.into_repr()).collect(); let result = pedersen::commitment::commit_native_with_index(&inputs, domain_separator); - let res_x = FieldElement::from_repr(*result.x().expect("should not commit to point at infinity")); - let res_y = FieldElement::from_repr(*result.y().expect("should not commit to point at infinity")); + let res_x = + FieldElement::from_repr(*result.x().expect("should not commit to point at infinity")); + let res_y = + FieldElement::from_repr(*result.y().expect("should not commit to point at infinity")); Ok((res_x, res_y)) } diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs index 37c71c1accc..2cf349fe350 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs @@ -18,9 +18,10 @@ use crate::generator::generators::{derive_generators, DEFAULT_DOMAIN_SEPARATOR}; // NOTE: this could be generalized using SWCurveConfig but since we perform the operation over grumpkin its explicit pub(crate) fn commit_native_with_index( inputs: &[Fq], - starting_index: u32 + starting_index: u32, ) -> Affine { - let generators = derive_generators(DEFAULT_DOMAIN_SEPARATOR, inputs.len() as u32, starting_index); + let generators = + derive_generators(DEFAULT_DOMAIN_SEPARATOR, inputs.len() as u32, starting_index); inputs.iter().enumerate().fold(Affine::zero(), |mut acc, (i, input)| { //TODO: this is a sketch conversion do better @@ -32,7 +33,7 @@ pub(crate) fn commit_native_with_index( #[cfg(test)] mod test { - + use ark_ec::short_weierstrass::Affine; use ark_ff::MontFp; use ark_std::{One, Zero}; @@ -60,13 +61,13 @@ mod test { let res = commit_native_with_index(&[Fq::zero(), Fq::one()], 0); let expected = Affine::new( // 054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402 + MontFp!("2393473289045184898987089634332637236754766663897650125720167164137088869378"), + // 209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126 MontFp!( - "2393473289045184898987089634332637236754766663897650125720167164137088869378" + "14752839959415467457196082350231122454649853219840744672802853620609001898278" ), - // 209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126 - MontFp!("14752839959415467457196082350231122454649853219840744672802853620609001898278"), ); assert_eq!(res, expected); } -} \ No newline at end of file +} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs index 3a68b6f5600..7c049849743 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs @@ -1,9 +1,6 @@ // Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson_hash.rs -use ark_ec::{ - short_weierstrass::Affine, - CurveConfig, CurveGroup, -}; +use ark_ec::{short_weierstrass::Affine, CurveConfig, CurveGroup}; use grumpkin::GrumpkinParameters; use crate::generator::generators::derive_generators; @@ -24,7 +21,8 @@ pub(crate) fn hash_with_index( inputs: &[grumpkin::Fq], starting_index: u32, ) -> ::BaseField { - let length_as_scalar: ::ScalarField = (inputs.len() as u64).into(); + let length_as_scalar: ::ScalarField = + (inputs.len() as u64).into(); let length_prefix = length_generator(0) * length_as_scalar; let result = length_prefix + commit_native_with_index(inputs, starting_index); result.into_affine().x @@ -62,7 +60,9 @@ pub(crate) mod test { // 1c446df60816b897cda124524e6b03f36df0cec333fad87617aab70d7861daa6 assert_eq!( res, - MontFp!("12785664284086914537273210116175139764153812914951498056047869066787449592486") + MontFp!( + "12785664284086914537273210116175139764153812914951498056047869066787449592486" + ) ); } -} \ No newline at end of file +} diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs index 615690fddbf..c3c4ed56450 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/mod.rs @@ -1,2 +1,2 @@ pub(crate) mod commitment; -pub(crate) mod hash; \ No newline at end of file +pub(crate) mod hash; From 52b8b3af913b10c4468b3a38a2de103e74931624 Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Sun, 19 May 2024 23:42:19 +0000 Subject: [PATCH 10/16] feat: cache default generators to avoid recalculating --- .../src/generator/generators.rs | 28 +++++++++++++++++++ .../src/pedersen/hash.rs | 10 ++++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index 6f9c72f7ebb..a4f195a7229 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -1,5 +1,7 @@ // Taken from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rshttps://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/group.rs +use std::sync::OnceLock; + use ark_ec::short_weierstrass::Affine; use acvm_blackbox_solver::blake3; @@ -8,6 +10,17 @@ use grumpkin::GrumpkinParameters; use super::hash_to_curve::hash_to_curve; pub(crate) const DEFAULT_DOMAIN_SEPARATOR: &[u8] = "DEFAULT_DOMAIN_SEPARATOR".as_bytes(); +const NUM_DEFAULT_GENERATORS: usize = 8; + +fn default_generators() -> &'static [Affine; NUM_DEFAULT_GENERATORS] { + static INSTANCE: OnceLock<[Affine; NUM_DEFAULT_GENERATORS]> = + OnceLock::new(); + INSTANCE.get_or_init(|| { + _derive_generators(DEFAULT_DOMAIN_SEPARATOR, NUM_DEFAULT_GENERATORS as u32, 0) + .try_into() + .expect("Should generate `NUM_DEFAULT_GENERATORS`") + }) +} /// Derives generator points via [hash-to-curve][hash_to_curve]. /// @@ -27,6 +40,21 @@ pub(crate) fn derive_generators( domain_separator_bytes: &[u8], num_generators: u32, starting_index: u32, +) -> Vec> { + // We cache a small number of the default generators so we can reuse them without needing to repeatedly recalculate them. + if domain_separator_bytes == DEFAULT_DOMAIN_SEPARATOR && starting_index + num_generators <= NUM_DEFAULT_GENERATORS as u32 { + let start_index = starting_index as usize; + let end_index = (starting_index + num_generators) as usize; + default_generators()[start_index..end_index].to_vec() + } else { + _derive_generators(domain_separator_bytes, num_generators, starting_index) + } +} + +fn _derive_generators( + domain_separator_bytes: &[u8], + num_generators: u32, + starting_index: u32, ) -> Vec> { let mut generator_preimage = [0u8; 64]; let domain_hash = blake3(domain_separator_bytes).expect("hash should succeed"); diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs index 7c049849743..40a1aeb3c9e 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs @@ -1,5 +1,7 @@ // Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson_hash.rs +use std::sync::OnceLock; + use ark_ec::{short_weierstrass::Affine, CurveConfig, CurveGroup}; use grumpkin::GrumpkinParameters; @@ -23,14 +25,14 @@ pub(crate) fn hash_with_index( ) -> ::BaseField { let length_as_scalar: ::ScalarField = (inputs.len() as u64).into(); - let length_prefix = length_generator(0) * length_as_scalar; + let length_prefix = *length_generator() * length_as_scalar; let result = length_prefix + commit_native_with_index(inputs, starting_index); result.into_affine().x } -//Note: this can be abstracted to a lazy_static!() -fn length_generator(starting_index: u32) -> Affine { - derive_generators("pedersen_hash_length".as_bytes(), 1, starting_index)[0] +fn length_generator() -> &'static Affine { + static INSTANCE: OnceLock> = OnceLock::new(); + INSTANCE.get_or_init(|| derive_generators("pedersen_hash_length".as_bytes(), 1, 0)[0]) } #[cfg(test)] From 13dcb0255f0caeb91810cb5340e860aa2845eacb Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 20 May 2024 00:03:45 +0000 Subject: [PATCH 11/16] chore: cargo fmt --- acvm-repo/bn254_blackbox_solver/src/generator/generators.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index a4f195a7229..517f99cea09 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -42,7 +42,9 @@ pub(crate) fn derive_generators( starting_index: u32, ) -> Vec> { // We cache a small number of the default generators so we can reuse them without needing to repeatedly recalculate them. - if domain_separator_bytes == DEFAULT_DOMAIN_SEPARATOR && starting_index + num_generators <= NUM_DEFAULT_GENERATORS as u32 { + if domain_separator_bytes == DEFAULT_DOMAIN_SEPARATOR + && starting_index + num_generators <= NUM_DEFAULT_GENERATORS as u32 + { let start_index = starting_index as usize; let end_index = (starting_index + num_generators) as usize; default_generators()[start_index..end_index].to_vec() From 0d59ab7bf4dc3d2529ecc5e9deed0c611be3a51f Mon Sep 17 00:00:00 2001 From: TomAFrench Date: Mon, 20 May 2024 00:10:20 +0000 Subject: [PATCH 12/16] chore: clippy --- acvm-repo/bn254_blackbox_solver/benches/criterion.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/bn254_blackbox_solver/benches/criterion.rs b/acvm-repo/bn254_blackbox_solver/benches/criterion.rs index c4d83b447b8..a8fa7d8aae4 100644 --- a/acvm-repo/bn254_blackbox_solver/benches/criterion.rs +++ b/acvm-repo/bn254_blackbox_solver/benches/criterion.rs @@ -55,7 +55,7 @@ fn bench_schnorr_verify(c: &mut Criterion) { black_box(&pub_key_x), black_box(&pub_key_y), black_box(&sig_bytes), - black_box(&message), + black_box(message), ) }) }); From db35ae698bad3ce78b7463c332beee662e2fbc4a Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 20 May 2024 14:34:22 +0100 Subject: [PATCH 13/16] Apply suggestions from code review --- acvm-repo/bn254_blackbox_solver/src/generator/generators.rs | 4 +++- .../bn254_blackbox_solver/src/generator/hash_to_curve.rs | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs index 517f99cea09..f89d582d167 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/generators.rs @@ -1,4 +1,6 @@ -// Taken from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rshttps://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/group.rs +// Adapted from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rshttps://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/group.rs +//! +//! Code is used under the MIT license use std::sync::OnceLock; diff --git a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs index cfa25ec0b4d..c0197883442 100644 --- a/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs +++ b/acvm-repo/bn254_blackbox_solver/src/generator/hash_to_curve.rs @@ -1,4 +1,6 @@ -// Taken from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rs +// Adapted from https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/ecc/groups/affine_element.rs +//! +//! Code is used under the MIT license use acvm_blackbox_solver::blake3; From 1dd0f298ab12beef2830bc03edf23b3519344863 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 May 2024 15:16:49 +0100 Subject: [PATCH 14/16] chore: fix doc comments --- .../bn254_blackbox_solver/src/pedersen/commitment.rs | 11 +---------- acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs | 11 +---------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs index 2cf349fe350..afd8139ce58 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs @@ -6,16 +6,7 @@ use grumpkin::{Fq, Fr, GrumpkinParameters}; use crate::generator::generators::{derive_generators, DEFAULT_DOMAIN_SEPARATOR}; -/** - * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. - * - * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating - * grumpkin commitments to field elements inside a BN254 SNARK circuit. - * @param inputs - * @param context - * @return Curve::AffineElement - */ -// NOTE: this could be generalized using SWCurveConfig but since we perform the operation over grumpkin its explicit +/// Given a vector of fields, generate a pedersen commitment using the indexed generators. pub(crate) fn commit_native_with_index( inputs: &[Fq], starting_index: u32, diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs index 40a1aeb3c9e..eee8b29730a 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs @@ -9,16 +9,7 @@ use crate::generator::generators::derive_generators; use super::commitment::commit_native_with_index; -/** - * @brief Given a vector of fields, generate a pedersen commitment using the indexed generators. - * - * @details This method uses `Curve::BaseField` members as inputs. This aligns with what we expect when creating - * grumpkin commitments to field elements inside a BN254 SNARK circuit. - * @param inputs - * @param context - * @return Curve::AffineElement - */ -//TODO: confirm we can do this with scalar field +/// Given a vector of fields, generate a pedersen hash using the indexed generators. pub(crate) fn hash_with_index( inputs: &[grumpkin::Fq], starting_index: u32, From e390cf374558f028e8ff784bc9b5a6ca82717641 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 May 2024 15:41:27 +0100 Subject: [PATCH 15/16] chore: use hex in test suite --- .../src/pedersen/commitment.rs | 36 ++++++++++++------- .../src/pedersen/hash.rs | 19 ++++++---- 2 files changed, 36 insertions(+), 19 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs index afd8139ce58..23991f9d219 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs @@ -25,8 +25,8 @@ pub(crate) fn commit_native_with_index( #[cfg(test)] mod test { + use acir::FieldElement; use ark_ec::short_weierstrass::Affine; - use ark_ff::MontFp; use ark_std::{One, Zero}; use grumpkin::Fq; @@ -34,14 +34,19 @@ mod test { #[test] fn commitment() { + // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.test.cpp#L10-L18 let res = commit_native_with_index(&[Fq::one(), Fq::one()], 0); let expected = Affine::new( - // 2f7a8f9a6c96926682205fb73ee43215bf13523c19d7afe36f12760266cdfe15 - MontFp!( - "21475250338311530111088781112432132511855209292730670949974692984887182229013" - ), - // 01916b316adbbf0e10e39b18c1d24b33ec84b46daddf72f43878bcc92b6057e6 - MontFp!("709245492126126701709902506217603794644991322680146492508959813283461748710"), + FieldElement::from_hex( + "0x2f7a8f9a6c96926682205fb73ee43215bf13523c19d7afe36f12760266cdfe15", + ) + .unwrap() + .into_repr(), + FieldElement::from_hex( + "0x01916b316adbbf0e10e39b18c1d24b33ec84b46daddf72f43878bcc92b6057e6", + ) + .unwrap() + .into_repr(), ); assert_eq!(res, expected); @@ -49,14 +54,19 @@ mod test { #[test] fn commitment_with_zero() { + // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_commitment/pedersen.test.cpp#L20-L29 let res = commit_native_with_index(&[Fq::zero(), Fq::one()], 0); let expected = Affine::new( - // 054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402 - MontFp!("2393473289045184898987089634332637236754766663897650125720167164137088869378"), - // 209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126 - MontFp!( - "14752839959415467457196082350231122454649853219840744672802853620609001898278" - ), + FieldElement::from_hex( + "0x054aa86a73cb8a34525e5bbed6e43ba1198e860f5f3950268f71df4591bde402", + ) + .unwrap() + .into_repr(), + FieldElement::from_hex( + "0x209dcfbf2cfb57f9f6046f44d71ac6faf87254afc7407c04eb621a6287cac126", + ) + .unwrap() + .into_repr(), ); assert_eq!(res, expected); diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs index eee8b29730a..28bf354edc9 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/hash.rs @@ -31,31 +31,38 @@ pub(crate) mod test { use super::*; - use ark_ff::MontFp; + use acir::FieldElement; use ark_std::One; use grumpkin::Fq; //reference: https://github.com/AztecProtocol/barretenberg/blob/master/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp #[test] fn hash_one() { + // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp#L21-L26 let res = hash_with_index(&[Fq::one(), Fq::one()], 0); - // 07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b assert_eq!( res, - MontFp!("3583137940367543141169889198758850326673923325182598243450662697654714313083") + FieldElement::from_hex( + "0x07ebfbf4df29888c6cd6dca13d4bb9d1a923013ddbbcbdc3378ab8845463297b", + ) + .unwrap() + .into_repr(), ); } #[test] fn test_hash_with_index() { + // https://github.com/AztecProtocol/aztec-packages/blob/72931bdb8202c34042cdfb8cee2ef44b75939879/barretenberg/cpp/src/barretenberg/crypto/pedersen_hash/pedersen.test.cpp#L28-L33 let res = hash_with_index(&[Fq::one(), Fq::one()], 5); - // 1c446df60816b897cda124524e6b03f36df0cec333fad87617aab70d7861daa6 + assert_eq!( res, - MontFp!( - "12785664284086914537273210116175139764153812914951498056047869066787449592486" + FieldElement::from_hex( + "0x1c446df60816b897cda124524e6b03f36df0cec333fad87617aab70d7861daa6", ) + .unwrap() + .into_repr(), ); } } From 15414b9e60b6d9d73d39f84b3cdf48a1e7f83582 Mon Sep 17 00:00:00 2001 From: Tom French Date: Mon, 20 May 2024 16:28:47 +0100 Subject: [PATCH 16/16] chore: add justification for why we can convert from Fq to Fr --- .../bn254_blackbox_solver/src/pedersen/commitment.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs index 23991f9d219..6769150508a 100644 --- a/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs +++ b/acvm-repo/bn254_blackbox_solver/src/pedersen/commitment.rs @@ -1,8 +1,8 @@ // Taken from: https://github.com/laudiacay/barustenberg/blob/df6bc6f095fe7f288bf6a12e7317fd8eb33d68ae/barustenberg/src/crypto/pedersen/pederson.rs use ark_ec::{short_weierstrass::Affine, AffineRepr, CurveGroup}; -use ark_ff::PrimeField; -use grumpkin::{Fq, Fr, GrumpkinParameters}; +use ark_ff::{MontConfig, PrimeField}; +use grumpkin::{Fq, FqConfig, Fr, FrConfig, GrumpkinParameters}; use crate::generator::generators::{derive_generators, DEFAULT_DOMAIN_SEPARATOR}; @@ -14,8 +14,10 @@ pub(crate) fn commit_native_with_index( let generators = derive_generators(DEFAULT_DOMAIN_SEPARATOR, inputs.len() as u32, starting_index); + // As |F_r| > |F_q|, we can safely convert any `F_q` into an `F_r` uniquely. + assert!(FrConfig::MODULUS > FqConfig::MODULUS); + inputs.iter().enumerate().fold(Affine::zero(), |mut acc, (i, input)| { - //TODO: this is a sketch conversion do better acc = (acc + (generators[i] * Fr::from_bigint(input.into_bigint()).unwrap()).into_affine()) .into_affine(); acc