From 7cc123e0a442d41a3610a57a2ee4f8008ef5811f Mon Sep 17 00:00:00 2001 From: David Palm Date: Wed, 29 Nov 2023 09:09:36 +0100 Subject: [PATCH 1/3] chore: from_values takes ref --- evm/src/keccak/keccak_stark.rs | 6 +----- evm/src/prover.rs | 11 ++--------- plonky2/src/fri/oracle.rs | 8 ++++++-- plonky2/src/plonk/circuit_builder.rs | 2 +- plonky2/src/plonk/prover.rs | 4 ++-- starky/src/prover.rs | 6 ++---- 6 files changed, 14 insertions(+), 23 deletions(-) diff --git a/evm/src/keccak/keccak_stark.rs b/evm/src/keccak/keccak_stark.rs index 9870b906..019b8083 100644 --- a/evm/src/keccak/keccak_stark.rs +++ b/evm/src/keccak/keccak_stark.rs @@ -722,15 +722,11 @@ mod tests { stark.generate_trace(input, 8, &mut timing) ); - // TODO: Cloning this isn't great; consider having `from_values` accept a reference, - // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. - let cloned_trace_poly_values = timed!(timing, "clone", trace_poly_values.clone()); - let trace_commitments = timed!( timing, "compute trace commitment", PolynomialBatch::::from_values( - cloned_trace_poly_values, + &trace_poly_values, config.fri_config.rate_bits, false, config.fri_config.cap_height, diff --git a/evm/src/prover.rs b/evm/src/prover.rs index fe8ad517..77985a23 100644 --- a/evm/src/prover.rs +++ b/evm/src/prover.rs @@ -102,14 +102,7 @@ where timing, &format!("compute trace commitment for {:?}", table), PolynomialBatch::::from_values( - // TODO: Cloning this isn't great; consider having `from_values` accept a reference, - // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. - trace.clone(), - rate_bits, - false, - cap_height, - timing, - None, + trace, rate_bits, false, cap_height, timing, None, ) ) }) @@ -380,7 +373,7 @@ where timing, "compute auxiliary polynomials commitment", PolynomialBatch::from_values( - auxiliary_polys, + &auxiliary_polys, rate_bits, false, config.fri_config.cap_height, diff --git a/plonky2/src/fri/oracle.rs b/plonky2/src/fri/oracle.rs index 8642a6c5..bb065b7c 100644 --- a/plonky2/src/fri/oracle.rs +++ b/plonky2/src/fri/oracle.rs @@ -55,7 +55,7 @@ impl, C: GenericConfig, const D: usize> { /// Creates a list polynomial commitment for the polynomials interpolating the values in `values`. pub fn from_values( - values: Vec>, + values: &[PolynomialValues], rate_bits: usize, blinding: bool, cap_height: usize, @@ -65,7 +65,11 @@ impl, C: GenericConfig, const D: usize> let coeffs = timed!( timing, "IFFT", - values.into_par_iter().map(|v| v.ifft()).collect::>() + values + .into_par_iter() + .cloned() + .map(|v| v.ifft()) + .collect::>() ); Self::from_coeffs( diff --git a/plonky2/src/plonk/circuit_builder.rs b/plonky2/src/plonk/circuit_builder.rs index 67db6864..279adac9 100644 --- a/plonky2/src/plonk/circuit_builder.rs +++ b/plonky2/src/plonk/circuit_builder.rs @@ -1029,7 +1029,7 @@ impl, const D: usize> CircuitBuilder { let constants_sigmas_commitment = if commit_to_sigma { let constants_sigmas_vecs = [constant_vecs, sigma_vecs.clone()].concat(); PolynomialBatch::::from_values( - constants_sigmas_vecs, + &constants_sigmas_vecs, rate_bits, PlonkOracle::CONSTANTS_SIGMAS.blinding, cap_height, diff --git a/plonky2/src/plonk/prover.rs b/plonky2/src/plonk/prover.rs index 41aebdb1..208d6450 100644 --- a/plonky2/src/plonk/prover.rs +++ b/plonky2/src/plonk/prover.rs @@ -171,7 +171,7 @@ where timing, "compute wires commitment", PolynomialBatch::::from_values( - wires_values, + &wires_values, config.fri_config.rate_bits, config.zero_knowledge && PlonkOracle::WIRES.blinding, config.fri_config.cap_height, @@ -238,7 +238,7 @@ where timing, "commit to partial products, Z's and, if any, lookup polynomials", PolynomialBatch::from_values( - zs_partial_products_lookups, + &zs_partial_products_lookups, config.fri_config.rate_bits, config.zero_knowledge && PlonkOracle::ZS_PARTIAL_PRODUCTS.blinding, config.fri_config.cap_height, diff --git a/starky/src/prover.rs b/starky/src/prover.rs index 23808e0f..066319c8 100644 --- a/starky/src/prover.rs +++ b/starky/src/prover.rs @@ -55,9 +55,7 @@ where timing, "compute trace commitment", PolynomialBatch::::from_values( - // TODO: Cloning this isn't great; consider having `from_values` accept a reference, - // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. - trace_poly_values.clone(), + &trace_poly_values, rate_bits, false, cap_height, @@ -88,7 +86,7 @@ where timing, "compute permutation Z commitments", PolynomialBatch::from_values( - permutation_z_polys, + &permutation_z_polys, rate_bits, false, config.fri_config.cap_height, From 37918cccfd148814605d936155ef58714c81dc91 Mon Sep 17 00:00:00 2001 From: David Palm Date: Thu, 30 Nov 2023 15:07:10 +0100 Subject: [PATCH 2/3] Revert "chore: from_values takes ref" This reverts commit 7cc123e0a442d41a3610a57a2ee4f8008ef5811f. --- evm/src/keccak/keccak_stark.rs | 6 +++++- evm/src/prover.rs | 11 +++++++++-- plonky2/src/fri/oracle.rs | 8 ++------ plonky2/src/plonk/circuit_builder.rs | 2 +- plonky2/src/plonk/prover.rs | 4 ++-- starky/src/prover.rs | 6 ++++-- 6 files changed, 23 insertions(+), 14 deletions(-) diff --git a/evm/src/keccak/keccak_stark.rs b/evm/src/keccak/keccak_stark.rs index 019b8083..9870b906 100644 --- a/evm/src/keccak/keccak_stark.rs +++ b/evm/src/keccak/keccak_stark.rs @@ -722,11 +722,15 @@ mod tests { stark.generate_trace(input, 8, &mut timing) ); + // TODO: Cloning this isn't great; consider having `from_values` accept a reference, + // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. + let cloned_trace_poly_values = timed!(timing, "clone", trace_poly_values.clone()); + let trace_commitments = timed!( timing, "compute trace commitment", PolynomialBatch::::from_values( - &trace_poly_values, + cloned_trace_poly_values, config.fri_config.rate_bits, false, config.fri_config.cap_height, diff --git a/evm/src/prover.rs b/evm/src/prover.rs index 77985a23..fe8ad517 100644 --- a/evm/src/prover.rs +++ b/evm/src/prover.rs @@ -102,7 +102,14 @@ where timing, &format!("compute trace commitment for {:?}", table), PolynomialBatch::::from_values( - trace, rate_bits, false, cap_height, timing, None, + // TODO: Cloning this isn't great; consider having `from_values` accept a reference, + // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. + trace.clone(), + rate_bits, + false, + cap_height, + timing, + None, ) ) }) @@ -373,7 +380,7 @@ where timing, "compute auxiliary polynomials commitment", PolynomialBatch::from_values( - &auxiliary_polys, + auxiliary_polys, rate_bits, false, config.fri_config.cap_height, diff --git a/plonky2/src/fri/oracle.rs b/plonky2/src/fri/oracle.rs index bb065b7c..8642a6c5 100644 --- a/plonky2/src/fri/oracle.rs +++ b/plonky2/src/fri/oracle.rs @@ -55,7 +55,7 @@ impl, C: GenericConfig, const D: usize> { /// Creates a list polynomial commitment for the polynomials interpolating the values in `values`. pub fn from_values( - values: &[PolynomialValues], + values: Vec>, rate_bits: usize, blinding: bool, cap_height: usize, @@ -65,11 +65,7 @@ impl, C: GenericConfig, const D: usize> let coeffs = timed!( timing, "IFFT", - values - .into_par_iter() - .cloned() - .map(|v| v.ifft()) - .collect::>() + values.into_par_iter().map(|v| v.ifft()).collect::>() ); Self::from_coeffs( diff --git a/plonky2/src/plonk/circuit_builder.rs b/plonky2/src/plonk/circuit_builder.rs index 279adac9..67db6864 100644 --- a/plonky2/src/plonk/circuit_builder.rs +++ b/plonky2/src/plonk/circuit_builder.rs @@ -1029,7 +1029,7 @@ impl, const D: usize> CircuitBuilder { let constants_sigmas_commitment = if commit_to_sigma { let constants_sigmas_vecs = [constant_vecs, sigma_vecs.clone()].concat(); PolynomialBatch::::from_values( - &constants_sigmas_vecs, + constants_sigmas_vecs, rate_bits, PlonkOracle::CONSTANTS_SIGMAS.blinding, cap_height, diff --git a/plonky2/src/plonk/prover.rs b/plonky2/src/plonk/prover.rs index 208d6450..41aebdb1 100644 --- a/plonky2/src/plonk/prover.rs +++ b/plonky2/src/plonk/prover.rs @@ -171,7 +171,7 @@ where timing, "compute wires commitment", PolynomialBatch::::from_values( - &wires_values, + wires_values, config.fri_config.rate_bits, config.zero_knowledge && PlonkOracle::WIRES.blinding, config.fri_config.cap_height, @@ -238,7 +238,7 @@ where timing, "commit to partial products, Z's and, if any, lookup polynomials", PolynomialBatch::from_values( - &zs_partial_products_lookups, + zs_partial_products_lookups, config.fri_config.rate_bits, config.zero_knowledge && PlonkOracle::ZS_PARTIAL_PRODUCTS.blinding, config.fri_config.cap_height, diff --git a/starky/src/prover.rs b/starky/src/prover.rs index 066319c8..23808e0f 100644 --- a/starky/src/prover.rs +++ b/starky/src/prover.rs @@ -55,7 +55,9 @@ where timing, "compute trace commitment", PolynomialBatch::::from_values( - &trace_poly_values, + // TODO: Cloning this isn't great; consider having `from_values` accept a reference, + // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. + trace_poly_values.clone(), rate_bits, false, cap_height, @@ -86,7 +88,7 @@ where timing, "compute permutation Z commitments", PolynomialBatch::from_values( - &permutation_z_polys, + permutation_z_polys, rate_bits, false, config.fri_config.cap_height, From e68195fc0d4613f32e3874ca198a8be6acbb6ec0 Mon Sep 17 00:00:00 2001 From: David Palm Date: Thu, 30 Nov 2023 15:10:34 +0100 Subject: [PATCH 3/3] chore: Remove TODOs about `from_values` taking a reference --- evm/src/keccak/keccak_stark.rs | 2 -- evm/src/prover.rs | 2 -- starky/src/prover.rs | 2 -- 3 files changed, 6 deletions(-) diff --git a/evm/src/keccak/keccak_stark.rs b/evm/src/keccak/keccak_stark.rs index 9870b906..e81d2f8a 100644 --- a/evm/src/keccak/keccak_stark.rs +++ b/evm/src/keccak/keccak_stark.rs @@ -722,8 +722,6 @@ mod tests { stark.generate_trace(input, 8, &mut timing) ); - // TODO: Cloning this isn't great; consider having `from_values` accept a reference, - // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. let cloned_trace_poly_values = timed!(timing, "clone", trace_poly_values.clone()); let trace_commitments = timed!( diff --git a/evm/src/prover.rs b/evm/src/prover.rs index fe8ad517..c61361d6 100644 --- a/evm/src/prover.rs +++ b/evm/src/prover.rs @@ -102,8 +102,6 @@ where timing, &format!("compute trace commitment for {:?}", table), PolynomialBatch::::from_values( - // TODO: Cloning this isn't great; consider having `from_values` accept a reference, - // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. trace.clone(), rate_bits, false, diff --git a/starky/src/prover.rs b/starky/src/prover.rs index 23808e0f..866bb635 100644 --- a/starky/src/prover.rs +++ b/starky/src/prover.rs @@ -55,8 +55,6 @@ where timing, "compute trace commitment", PolynomialBatch::::from_values( - // TODO: Cloning this isn't great; consider having `from_values` accept a reference, - // or having `compute_permutation_z_polys` read trace values from the `PolynomialBatch`. trace_poly_values.clone(), rate_bits, false,