From fa8e6a38c021d943b7cf073b686743edfc70a27f Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Fri, 4 Oct 2024 23:06:45 +0200 Subject: [PATCH 1/2] Poly-commitment/KZG: uniform definition of `SRS::create` And include additional documentation for the method. KZG won't be used in production in the near future, and there is no plan for now. Therefore, it seems reasonable to have these changes, with the warning. Note that we should get rid of create_trusted_setup in IPA::SRS, and stop relying on IPA::SRS to define the KZG SRS. It was initially introduced to avoid to reimplement all methods and have something quick for testing. --- ivc/tests/folding_ivc.rs | 5 +--- kimchi/src/tests/generic.rs | 9 ++----- msm/src/precomputed_srs.rs | 8 ++---- msm/src/test/logup.rs | 5 +--- o1vm/src/legacy/main.rs | 5 ++-- poly-commitment/src/kzg.rs | 49 ++++++++++++++++++------------------ poly-commitment/src/lib.rs | 9 +++++++ poly-commitment/tests/kzg.rs | 6 +---- 8 files changed, 43 insertions(+), 53 deletions(-) diff --git a/ivc/tests/folding_ivc.rs b/ivc/tests/folding_ivc.rs index 608501d1f3..1420cd4f1b 100644 --- a/ivc/tests/folding_ivc.rs +++ b/ivc/tests/folding_ivc.rs @@ -1,4 +1,3 @@ -use ark_ff::UniformRand; use ark_poly::{EvaluationDomain, Radix2EvaluationDomain}; use folding::{ instance_witness::Foldable, Alphas, FoldingCompatibleExpr, FoldingConfig, FoldingEnv, @@ -123,10 +122,8 @@ fn test_regression_additional_columns_reduction_to_degree_2() { constrain_ivc::(&mut constraint_env); let constraints = constraint_env.get_relation_constraints(); - let mut rng = o1_utils::tests::make_test_rng(None); - let toxic_waste = Fp::rand(&mut rng); let domain = Radix2EvaluationDomain::::new(2).unwrap(); - let mut srs = unsafe { PairingSRS::create(toxic_waste, 2) }; + let mut srs = PairingSRS::create(2); srs.add_lagrange_basis(domain); let folding_compat_expresions: Vec> = constraints diff --git a/kimchi/src/tests/generic.rs b/kimchi/src/tests/generic.rs index e0df4bc520..6abba1b23c 100644 --- a/kimchi/src/tests/generic.rs +++ b/kimchi/src/tests/generic.rs @@ -98,14 +98,9 @@ fn test_generic_gate_kzg() { type BaseSponge = DefaultFqSponge; type ScalarSponge = DefaultFrSponge; - use ark_ff::UniformRand; - let public = vec![Fp::from(3u8); 5]; let gates = create_circuit(0, public.len()); - let rng = &mut rand::rngs::OsRng; - let x = Fp::rand(rng); - // create witness let mut witness: [Vec; COLUMNS] = array::from_fn(|_| vec![Fp::zero(); gates.len()]); fill_in_witness(0, &mut witness, &public); @@ -118,8 +113,8 @@ fn test_generic_gate_kzg() { .gates(gates) .witness(witness) .public_inputs(public) - .setup_with_custom_srs(|d1, usize| { - let mut srs = unsafe { poly_commitment::kzg::PairingSRS::create(x, usize) }; + .setup_with_custom_srs(|d1, srs_size| { + let mut srs = poly_commitment::kzg::PairingSRS::create(srs_size); srs.full_srs.add_lagrange_basis(d1); srs }) diff --git a/msm/src/precomputed_srs.rs b/msm/src/precomputed_srs.rs index 5f1b12536d..635eb6a89b 100644 --- a/msm/src/precomputed_srs.rs +++ b/msm/src/precomputed_srs.rs @@ -2,11 +2,9 @@ use crate::{Fp, BN254, DOMAIN_SIZE}; use ark_ec::pairing::Pairing; -use ark_ff::UniformRand; use ark_serialize::Write; use kimchi::{circuits::domains::EvaluationDomains, precomputed_srs::TestSRS}; use poly_commitment::{kzg::PairingSRS, SRS as _}; -use rand::{prelude::StdRng, SeedableRng}; use serde::{Deserialize, Serialize}; use std::{fs::File, io::BufReader, path::PathBuf}; @@ -40,7 +38,7 @@ pub fn get_bn254_srs(domain: EvaluationDomains) -> PairingSRS { let mut srs = if domain.d1.size as usize == DOMAIN_SIZE { read_bn254_srs_from_disk(get_bn254_srs_path()) } else { - unsafe { PairingSRS::create(Fp::rand(&mut rand::rngs::OsRng), domain.d1.size as usize) } + PairingSRS::create(domain.d1.size as usize) }; srs.full_srs.add_lagrange_basis(domain.d1); // not added if already present. srs @@ -73,9 +71,7 @@ fn create_and_store_srs_with_path( srs_path: PathBuf, ) -> PairingSRS { // We generate with a fixed-seed RNG, only used for testing. - let mut rng = &mut StdRng::from_seed([42u8; 32]); - let trapdoor = Fp::rand(&mut rng); - let mut srs = unsafe { PairingSRS::create(trapdoor, domain_size) }; + let mut srs = PairingSRS::create(domain_size); for sub_domain_size in 1..=domain_size { let domain = EvaluationDomains::::create(sub_domain_size).unwrap(); diff --git a/msm/src/test/logup.rs b/msm/src/test/logup.rs index 57fd2af967..f2e2dfd28d 100644 --- a/msm/src/test/logup.rs +++ b/msm/src/test/logup.rs @@ -24,10 +24,7 @@ mod tests { let domain_size = 1 << 8; let domain = EvaluationDomains::::create(domain_size).unwrap(); - let mut srs: PairingSRS = { - let toxic_waste = Fp::rand(&mut rng); - unsafe { PairingSRS::create(toxic_waste, domain.d1.size as usize) } - }; + let mut srs: PairingSRS = { PairingSRS::create(domain.d1.size as usize) }; srs.full_srs.add_lagrange_basis(domain.d1); let mut inputs = ProofInputs::random(domain); diff --git a/o1vm/src/legacy/main.rs b/o1vm/src/legacy/main.rs index 03bfc34d56..630786ff05 100644 --- a/o1vm/src/legacy/main.rs +++ b/o1vm/src/legacy/main.rs @@ -77,9 +77,8 @@ pub fn main() -> ExitCode { let mut rng = o1_utils::tests::make_test_rng(None); let srs = { - let toxic_waste = Fp::rand(&mut rand::rngs::OsRng); - - let mut srs = unsafe { poly_commitment::kzg::PairingSRS::create(toxic_waste, DOMAIN_SIZE) }; + // FIXME: toxic waste is generated in `create`. This is unsafe for prod. + let mut srs = poly_commitment::kzg::PairingSRS::create(DOMAIN_SIZE); srs.full_srs.add_lagrange_basis(domain.d1); srs }; diff --git a/poly-commitment/src/kzg.rs b/poly-commitment/src/kzg.rs index de53804f23..ae96f7f62b 100644 --- a/poly-commitment/src/kzg.rs +++ b/poly-commitment/src/kzg.rs @@ -22,6 +22,7 @@ use ark_poly::{ DenseUVPolynomial, EvaluationDomain, Evaluations, Polynomial, Radix2EvaluationDomain as D, }; use mina_poseidon::FqSponge; +use rand::thread_rng; use rand_core::{CryptoRng, RngCore}; use serde::{Deserialize, Serialize}; use serde_with::serde_as; @@ -128,6 +129,26 @@ pub struct PairingSRS { pub verifier_srs: SRS, } +impl< + F: PrimeField, + G: CommitmentCurve, + G2: CommitmentCurve, + Pair: Pairing, + > PairingSRS +{ + /// Create a trusted setup for the KZG protocol. + /// The setup is created using a toxic waste `toxic_waste` and a depth + /// `depth`. + fn create_trusted_setup(toxic_waste: F, depth: usize) -> Self { + let full_srs = unsafe { SRS::create_trusted_setup(toxic_waste, depth) }; + let verifier_srs = unsafe { SRS::create_trusted_setup(toxic_waste, depth) }; + Self { + full_srs, + verifier_srs, + } + } +} + impl Default for PairingSRS { fn default() -> Self { Self { @@ -146,28 +167,6 @@ impl Clone for PairingSRS { } } -impl< - F: PrimeField, - G: CommitmentCurve, - G2: CommitmentCurve, - Pair: Pairing, - > PairingSRS -{ - /// Create a new SRS for the KZG protocol. - /// - /// # Safety - /// - /// The method is annotated as unsafe because it does use a method - /// generating the toxic waste. A safe method would be to load an existing - /// SRS where it is broadly accepted that the trapdoor is not recoverable. - pub unsafe fn create(x: F, n: usize) -> Self { - PairingSRS { - full_srs: unsafe { SRS::create_trusted_setup(x, n) }, - verifier_srs: unsafe { SRS::create_trusted_setup(x, 3) }, - } - } -} - impl< F: PrimeField, G: CommitmentCurve, @@ -318,8 +317,10 @@ impl< .commit_evaluations_custom(domain, plnm, blinders) } - fn create(_depth: usize) -> Self { - todo!() + fn create(depth: usize) -> Self { + let mut rng = thread_rng(); + let toxic_waste = G::ScalarField::rand(&mut rng); + Self::create_trusted_setup(toxic_waste, depth) } fn add_lagrange_basis(&mut self, domain: D<::ScalarField>) { diff --git a/poly-commitment/src/lib.rs b/poly-commitment/src/lib.rs index 2d175e828e..be3f97e4da 100644 --- a/poly-commitment/src/lib.rs +++ b/poly-commitment/src/lib.rs @@ -149,6 +149,15 @@ pub trait SRS: Clone + Sized { blinders: &PolyComm, ) -> Result, CommitmentError>; + /// Create an SRS of size `depth`. + /// + /// Warning: in the case of a trusted setup, as it is required for a + /// polynomial commitment scheme like KZG, a toxic waste is generated using + /// `rand::thread_rng()`. This is not the behavior you would expect in a + /// production environment. + /// However, we do accept this behavior for the sake of simplicity in the + /// interface, and this method will only be supposed to be used in tests in + /// this case. fn create(depth: usize) -> Self; fn add_lagrange_basis(&mut self, domain: D); diff --git a/poly-commitment/tests/kzg.rs b/poly-commitment/tests/kzg.rs index b380f54768..53f5ae4c12 100644 --- a/poly-commitment/tests/kzg.rs +++ b/poly-commitment/tests/kzg.rs @@ -210,12 +210,8 @@ fn test_kzg_proof() { fn check_srs_g2_valid_and_serializes() { type BN254 = Bn; type BN254G2BaseField = ::BaseField; - type Fp = ark_bn254::Fr; - let mut rng = o1_utils::tests::make_test_rng(None); - - let x = Fp::rand(&mut rng); - let srs: PairingSRS = unsafe { PairingSRS::create(x, 1 << 5) }; + let srs: PairingSRS = PairingSRS::create(1 << 5); let mut vec: Vec = vec![0u8; 1024]; From ff3359eecd9af797bb604547ff5da5d31458b75e Mon Sep 17 00:00:00 2001 From: Danny Willems Date: Fri, 4 Oct 2024 23:08:49 +0200 Subject: [PATCH 2/2] Poly-commitment/KZG: add tests regarding exp nb of chunks --- poly-commitment/tests/kzg.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/poly-commitment/tests/kzg.rs b/poly-commitment/tests/kzg.rs index 53f5ae4c12..4599266dff 100644 --- a/poly-commitment/tests/kzg.rs +++ b/poly-commitment/tests/kzg.rs @@ -11,7 +11,7 @@ use poly_commitment::{ commitment::Evaluation, ipa::{DensePolynomialOrEvaluations, SRS}, kzg::{combine_evaluations, KZGProof, PairingSRS}, - PolyComm, SRS as _, + pbt_srs, PolyComm, SRS as _, }; #[test] @@ -234,3 +234,13 @@ fn check_srs_g2_valid_and_serializes() { assert!(expected == actual_y, "serialization failed"); } } + +// Testing how many chunks are generated with different polynomial sizes and +// different number of chunks requested. +#[test] +fn test_regression_commit_non_hiding_expected_number_of_chunks() { + type BN254 = Bn; + type Srs = PairingSRS; + + pbt_srs::test_regression_commit_non_hiding_expected_number_of_chunks::(); +}