Skip to content

Commit

Permalink
Merge pull request #2811 from o1-labs/volhovm/rename-pickles-vars
Browse files Browse the repository at this point in the history
Refactoring: Improve poly-commitment docs & decrease confusing variable names
  • Loading branch information
volhovm authored Dec 3, 2024
2 parents eb330fc + 2e4d754 commit 57f62ae
Show file tree
Hide file tree
Showing 14 changed files with 291 additions and 250 deletions.
10 changes: 5 additions & 5 deletions ivc/src/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ use mina_poseidon::{sponge::ScalarChallenge, FqSponge};
use o1_utils::ExtendedDensePolynomial;
use poly_commitment::{
commitment::{absorb_commitment, CommitmentCurve, PolyComm},
ipa::DensePolynomialOrEvaluations,
kzg::{KZGProof, PairingSRS},
utils::DensePolynomialOrEvaluations,
OpenProof, SRS,
};
use rand::{CryptoRng, RngCore};
Expand Down Expand Up @@ -437,11 +437,11 @@ where
let u = u_chal.to_field(endo_r);

let coefficients_form = DensePolynomialOrEvaluations::DensePolynomial;
let non_hiding = |d1_size| PolyComm {
chunks: vec![Fp::zero(); d1_size],
let non_hiding = |n_chunks| PolyComm {
chunks: vec![Fp::zero(); n_chunks],
};
let hiding = |d1_size| PolyComm {
chunks: vec![Fp::one(); d1_size],
let hiding = |n_chunks| PolyComm {
chunks: vec![Fp::one(); n_chunks],
};

// Gathering all polynomials_to_open to use in the opening proof
Expand Down
16 changes: 8 additions & 8 deletions kimchi/src/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ use poly_commitment::{
commitment::{
absorb_commitment, b_poly_coefficients, BlindedCommitment, CommitmentCurve, PolyComm,
},
ipa::DensePolynomialOrEvaluations,
utils::DensePolynomialOrEvaluations,
OpenProof, SRS as _,
};
use rand_core::{CryptoRng, RngCore};
Expand Down Expand Up @@ -1238,22 +1238,22 @@ where
//~ 1. Create a list of all polynomials that will require evaluations
//~ (and evaluation proofs) in the protocol.
//~ First, include the previous challenges, in case we are in a recursive prover.
let non_hiding = |d1_size: usize| PolyComm {
chunks: vec![G::ScalarField::zero(); d1_size],
let non_hiding = |n_chunks: usize| PolyComm {
chunks: vec![G::ScalarField::zero(); n_chunks],
};

let fixed_hiding = |n_chunks: usize| PolyComm {
chunks: vec![G::ScalarField::one(); n_chunks],
};

let coefficients_form = DensePolynomialOrEvaluations::DensePolynomial;
let evaluations_form = |e| DensePolynomialOrEvaluations::Evaluations(e, index.cs.domain.d1);

let mut polynomials = polys
.iter()
.map(|(p, d1_size)| (coefficients_form(p), non_hiding(*d1_size)))
.map(|(p, n_chunks)| (coefficients_form(p), non_hiding(*n_chunks)))
.collect::<Vec<_>>();

let fixed_hiding = |d1_size: usize| PolyComm {
chunks: vec![G::ScalarField::one(); d1_size],
};

//~ 1. Then, include:
//~~ * the negated public polynomial
//~~ * the ft polynomial
Expand Down
10 changes: 5 additions & 5 deletions msm/src/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use mina_poseidon::{sponge::ScalarChallenge, FqSponge};
use o1_utils::ExtendedDensePolynomial;
use poly_commitment::{
commitment::{absorb_commitment, PolyComm},
ipa::DensePolynomialOrEvaluations,
utils::DensePolynomialOrEvaluations,
OpenProof, SRS,
};
use rand::{CryptoRng, RngCore};
Expand Down Expand Up @@ -490,11 +490,11 @@ where
let u = u_chal.to_field(endo_r);

let coefficients_form = DensePolynomialOrEvaluations::DensePolynomial;
let non_hiding = |d1_size| PolyComm {
chunks: vec![G::ScalarField::zero(); d1_size],
let non_hiding = |n_chunks| PolyComm {
chunks: vec![G::ScalarField::zero(); n_chunks],
};
let hiding = |d1_size| PolyComm {
chunks: vec![G::ScalarField::one(); d1_size],
let hiding = |n_chunks| PolyComm {
chunks: vec![G::ScalarField::one(); n_chunks],
};

// Gathering all polynomials to use in the opening proof
Expand Down
3 changes: 2 additions & 1 deletion o1vm/src/pickles/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ use mina_poseidon::{sponge::ScalarChallenge, FqSponge};
use o1_utils::ExtendedDensePolynomial;
use poly_commitment::{
commitment::{absorb_commitment, PolyComm},
ipa::{DensePolynomialOrEvaluations, OpeningProof, SRS},
ipa::{OpeningProof, SRS},
utils::DensePolynomialOrEvaluations,
OpenProof as _, SRS as _,
};
use rand::{CryptoRng, RngCore};
Expand Down
4 changes: 1 addition & 3 deletions poly-commitment/benches/ipa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use groupmap::GroupMap;
use mina_curves::pasta::{Fp, Vesta, VestaParameters};
use mina_poseidon::{constants::PlonkSpongeConstantsKimchi, sponge::DefaultFqSponge, FqSponge};
use poly_commitment::{
commitment::CommitmentCurve,
ipa::{DensePolynomialOrEvaluations, SRS},
PolyComm, SRS as _,
commitment::CommitmentCurve, ipa::SRS, utils::DensePolynomialOrEvaluations, PolyComm, SRS as _,
};

fn benchmark_ipa_open(c: &mut Criterion) {
Expand Down
14 changes: 7 additions & 7 deletions poly-commitment/src/commitment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -511,7 +511,7 @@ pub fn combined_inner_product<F: PrimeField>(
// final combined evaluation result
let mut res = F::zero();
// polyscale^i
let mut xi_i = F::one();
let mut polyscale_i = F::one();

for evals_tr in polys.iter().filter(|evals_tr| !evals_tr[0].is_empty()) {
// Transpose the evaluations.
Expand All @@ -523,16 +523,16 @@ pub fn combined_inner_product<F: PrimeField>(

// Iterating over the polynomial segments.
// Each segment gets its own polyscale^i, each segment element j is multiplied by evalscale^j.
// Given that xi_i = polyscale^i0 at this point, after this loop we have:
// Given that polyscale_i = polyscale^i0 at this point, after this loop we have:
//
// res += Σ polyscale^{i0+i} ( Σ evals_tr[j][i] * evalscale^j )
// i j
//
for eval in &evals {
// p_i(evalscale)
let term = DensePolynomial::<F>::eval_polynomial(eval, *evalscale);
res += &(xi_i * term);
xi_i *= polyscale;
res += &(polyscale_i * term);
polyscale_i *= polyscale;
}
}
res
Expand Down Expand Up @@ -607,16 +607,16 @@ pub fn combine_commitments<G: CommitmentCurve>(
rand_base: G::ScalarField,
) {
// will contain the power of polyscale
let mut xi_i = G::ScalarField::one();
let mut polyscale_i = G::ScalarField::one();

for Evaluation { commitment, .. } in evaluations.iter().filter(|x| !x.commitment.is_empty()) {
// iterating over the polynomial segments
for comm_ch in &commitment.chunks {
scalars.push(rand_base * xi_i);
scalars.push(rand_base * polyscale_i);
points.push(*comm_ch);

// compute next power of polyscale
xi_i *= polyscale;
polyscale_i *= polyscale;
}
}
}
Expand Down
Loading

0 comments on commit 57f62ae

Please sign in to comment.