Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvements over plonk #671

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions provers/plonk/src/prover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,33 +341,38 @@ where
fn round_2(
&self,
witness: &Witness<F>,
common_preprocessed_input: &CommonPreprocessedInput<F>,
cpi: &CommonPreprocessedInput<F>,
beta: FieldElement<F>,
gamma: FieldElement<F>,
) -> Round2Result<F, CS::Commitment> {
let cpi = common_preprocessed_input;
let mut coefficients: Vec<FieldElement<F>> = vec![FieldElement::one()];
let mut z_evaluations: Vec<FieldElement<F>> = vec![FieldElement::one()];
let (s1, s2, s3) = (&cpi.s1_lagrange, &cpi.s2_lagrange, &cpi.s3_lagrange);

let k2 = &cpi.k1 * &cpi.k1;

let lp = |w: &FieldElement<F>, eta: &FieldElement<F>| w + &beta * eta + &gamma;

let z_nums: Vec<_> = (0..&cpi.n - 1)
.map(|i| {
lp(&witness.a[i], &cpi.domain[i])
* lp(&witness.b[i], &(&cpi.domain[i] * &cpi.k1))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this could be more efficient just by just iterating over cpi.domain zipped with witness.a, witness.b, witness.c

* lp(&witness.c[i], &(&cpi.domain[i] * &k2))
})
.collect();
let mut z_denoms: Vec<_> = (0..&cpi.n - 1)
.map(|i| {
lp(&witness.a[i], &s1[i]) * lp(&witness.b[i], &s2[i]) * lp(&witness.c[i], &s3[i])
})
.collect();
FieldElement::inplace_batch_inverse(&mut z_denoms).unwrap();

for i in 0..&cpi.n - 1 {
let (a_i, b_i, c_i) = (&witness.a[i], &witness.b[i], &witness.c[i]);
let num = lp(a_i, &cpi.domain[i])
* lp(b_i, &(&cpi.domain[i] * &cpi.k1))
* lp(c_i, &(&cpi.domain[i] * &k2));
let den = lp(a_i, &s1[i]) * lp(b_i, &s2[i]) * lp(c_i, &s3[i]);
let new_factor = num / den;
let new_term = coefficients.last().unwrap() * &new_factor;
coefficients.push(new_term);
z_evaluations.push(z_evaluations.last().unwrap() * (&z_nums[i] * &z_denoms[i]));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be done better using a scan iterator. That avoids pushing every evaluation into the vector, which is not super efficient

}

let p_z = Polynomial::interpolate_fft(&coefficients)
let p_z = Polynomial::interpolate_fft(&z_evaluations)
.expect("xs and ys have equal length and xs are unique");
let z_h = Polynomial::new_monomial(FieldElement::one(), common_preprocessed_input.n)
- FieldElement::one();
let z_h = Polynomial::new_monomial(FieldElement::one(), cpi.n) - FieldElement::one();
let p_z = self.blind_polynomial(&p_z, &z_h, 3);
let z_1 = self.commitment_scheme.commit(&p_z);
Round2Result {
Expand Down
2 changes: 1 addition & 1 deletion provers/plonk/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ impl<F: IsFFTField> CommonPreprocessedInput<F> {

let permutation = get_permutation(&lro);
let permuted =
generate_permutation_coefficients(&omega, n, &permutation, order_r_minus_1_root_unity);
generate_permutation_coefficients(&domain, &permutation, order_r_minus_1_root_unity);

let s1_lagrange: Vec<_> = permuted[..n].to_vec();
let s2_lagrange: Vec<_> = permuted[n..2 * n].to_vec();
Expand Down
3 changes: 1 addition & 2 deletions provers/plonk/src/test_utils/circuit_1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ pub fn test_common_preprocessed_input_1() -> CommonPreprocessedInput<FrField> {
let omega = FrField::get_primitive_root_of_unity(2).unwrap();
let domain = generate_domain(&omega, n);
let permuted = generate_permutation_coefficients(
&omega,
n,
&domain,
&[11, 3, 0, 1, 2, 4, 6, 10, 5, 8, 7, 9],
&ORDER_R_MINUS_1_ROOT_UNITY,
);
Expand Down
2 changes: 1 addition & 1 deletion provers/plonk/src/test_utils/circuit_2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub fn test_common_preprocessed_input_2() -> CommonPreprocessedInput<FrField> {
23, 4, 0, 18, 1, 2, 5, 6, 7, 8, 10, 9, 19, 11, 13, 14, 15, 16, 3, 12, 17, 20, 21, 22,
];
let permuted =
generate_permutation_coefficients(&omega, n, permutation, &ORDER_R_MINUS_1_ROOT_UNITY);
generate_permutation_coefficients(&domain, permutation, &ORDER_R_MINUS_1_ROOT_UNITY);

let s1_lagrange: Vec<FrElement> = permuted[..8].to_vec();
let s2_lagrange: Vec<FrElement> = permuted[8..16].to_vec();
Expand Down
3 changes: 1 addition & 2 deletions provers/plonk/src/test_utils/circuit_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ pub fn common_preprocessed_input_from_json(
let omega = FrField::get_primitive_root_of_unity(n.trailing_zeros() as u64).unwrap();
let domain = generate_domain(&omega, n);
let permuted = generate_permutation_coefficients(
&omega,
n,
&domain,
&json_input.Permutation,
&ORDER_R_MINUS_1_ROOT_UNITY,
);
Expand Down
50 changes: 27 additions & 23 deletions provers/plonk/src/test_utils/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,51 +34,55 @@ pub fn test_srs(n: usize) -> StructuredReferenceString<G1Point, G2Point> {
let g1 = <BLS12381Curve as IsEllipticCurve>::generator();
let g2 = <BLS12381TwistCurve as IsEllipticCurve>::generator();

let powers_main_group: Vec<G1Point> = (0..n + 3)
.map(|exp| g1.operate_with_self(s.pow(exp as u64).representative()))
.collect();
let powers_main_group: Vec<G1Point> =
core::iter::successors(Some(FieldElement::from(1)), |acc| Some(acc * &s))
.take(n + 3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need for two maps; we can just fuse them into a single one with g1.operate_with_self(power.representative()).
Yet another version could use successors starting from the generator and having s directly as representative, so at every step we have s*previous

.map(|power| power.representative())
.map(|power| g1.operate_with_self(power))
.collect();
let powers_secondary_group = [g2.clone(), g2.operate_with_self(s.representative())];

StructuredReferenceString::new(&powers_main_group, &powers_secondary_group)
}

/// Generates a domain to interpolate: 1, omega, omega², ..., omega^size
pub fn generate_domain<F: IsField>(omega: &FieldElement<F>, size: usize) -> Vec<FieldElement<F>> {
(1..size).fold(vec![FieldElement::one()], |mut acc, _| {
acc.push(acc.last().unwrap() * omega);
acc
})
core::iter::successors(Some(FieldElement::one()), |prev| Some(prev * omega))
.take(size)
.collect()
}

/// Generates the permutation coefficients for the copy constraints.
/// polynomials S1, S2, S3.
pub fn generate_permutation_coefficients<F: IsField>(
omega: &FieldElement<F>,
n: usize,
domain: &[FieldElement<F>],
permutation: &[usize],
order_r_minus_1_root_unity: &FieldElement<F>,
) -> Vec<FieldElement<F>> {
let identity = identity_permutation(omega, n, order_r_minus_1_root_unity);
let permuted: Vec<FieldElement<F>> = (0..n * 3)
.map(|i| identity[permutation[i]].clone())
.collect();
permuted
let identity = identity_permutation(domain, order_r_minus_1_root_unity);
permutation
.iter()
.map(|perm| identity[*perm].clone())
.collect()
}

/// The identity permutation, auxiliary function to generate the copy constraints.
fn identity_permutation<F: IsField>(
w: &FieldElement<F>,
n: usize,
domain: &[FieldElement<F>],
order_r_minus_1_root_unity: &FieldElement<F>,
) -> Vec<FieldElement<F>> {
let u = order_r_minus_1_root_unity;
let mut result: Vec<FieldElement<F>> = vec![];
for index_column in 0..=2 {
for index_row in 0..n {
result.push(w.pow(index_row) * u.pow(index_column as u64));
}
}
result
let u_powers = [&FieldElement::one(), &u, &(u * u)];

(0..=2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to start from zero if it is just multiplying with one

.map(|col| {
domain
.iter()
.map(|elem| elem * u_powers[col])
.collect::<Vec<_>>()
})
.collect::<Vec<_>>()
.concat()
}

/// A mock of a random number generator, to have deterministic tests.
Expand Down