Address (simple) review comments

Cargo.toml

@@ -87,6 +87,7 @@ sanity-checks = []
 circuit-params = []
 cost-estimator = ["serde", "serde_derive"]
 derive_serde = ["halo2curves/derive_serde"]
+truncated-challenges = [] # This feature truncates challenges to half the size of the scalar field.
 
 [lib]
 bench = false

src/plonk/circuit.rs

@@ -481,14 +481,18 @@ impl Selector {
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub struct FixedQuery {
     /// Query index
-    pub index: Option<usize>,
+    pub(crate) index: Option<usize>,
     /// Column index
     pub(crate) column_index: usize,
     /// Rotation of this query
     pub(crate) rotation: Rotation,
 }
 
 impl FixedQuery {
+    /// Return the query index
+    pub fn index(&self) -> Option<usize> {
+        self.index
+    }
     /// Column index
     pub fn column_index(&self) -> usize {
         self.column_index

src/plonk/keygen.rs

@@ -201,7 +201,6 @@ impl<F: Field> Assignment<F> for Assembly<F> {
 
 /// Compute the minimal `k` to compute a circuit.
 pub fn k_from_circuit<F: Ord + Field + FromUniformBytes<64>, C: Circuit<F>>(circuit: &C) -> u32 {
-    // TODO: We could optimize the order here.
     (1..25)
         .find(|k| {
             let n = 2usize.pow(*k);
@@ -260,7 +259,7 @@ where
     let k = k_from_circuit(circuit);
 
     if params.max_k() != k {
-        return Err(Error::SrsError)
+        return Err(Error::SrsError);
     }
 
     keygen_vk_with_k(params, circuit, k)

src/plonk/permutation.rs

@@ -88,7 +88,7 @@ pub struct VerifyingKey<F: PrimeField, CS: PolynomialCommitmentScheme<F>> {
 }
 
 impl<F: PrimeField, CS: PolynomialCommitmentScheme<F>> VerifyingKey<F, CS> {
-    /// Returns the commitments of the verifying key.
+    /// Returns the (permutation argument) commitments of the verifying key.
     pub fn commitments(&self) -> &Vec<CS::Commitment> {
         &self.commitments
     }

src/plonk/prover.rs

@@ -610,8 +610,8 @@ fn test_create_proof() {
         }
     }
 
-    let mut params: ParamsKZG<Bn256> = ParamsKZG::unsafe_setup(3, OsRng);
-    let vk = keygen_vk(&mut params, &MyCircuit).expect("keygen_vk should not fail");
+    let params: ParamsKZG<Bn256> = ParamsKZG::unsafe_setup(3, OsRng);
+    let vk = keygen_vk(&params, &MyCircuit).expect("keygen_vk should not fail");
     let pk = keygen_pk(vk, &MyCircuit).expect("keygen_pk should not fail");
     let mut transcript = CircuitTranscript::<_>::init();
 

src/poly/commitment.rs

@@ -1,10 +1,10 @@
 //! Trait for a commitment scheme
+use crate::plonk::{k_from_circuit, Circuit};
 use crate::poly::{Coeff, Error, LagrangeCoeff, Polynomial, ProverQuery, VerifierQuery};
 use crate::transcript::{Hashable, Sampleable, Transcript};
 use crate::utils::helpers::ProcessedSerdeObject;
 use ff::{FromUniformBytes, PrimeField};
 use std::fmt::Debug;
-use crate::plonk::{Circuit, k_from_circuit};
 
 /// Public interface for a Polynomial Commitment Scheme (PCS)
 pub trait PolynomialCommitmentScheme<F: PrimeField>: Clone + Debug {
@@ -87,7 +87,13 @@ pub trait Params {
     /// Downsize the params to work with a circuit of unknown length. The
     /// function first computes the `k` of the provided circuit, and then
     /// downsizes the SRS.
-    fn downsize_from_circuit<F: PrimeField + Ord + FromUniformBytes<64>, ConcreCircuit: Circuit<F>>(&mut self, circuit: &ConcreCircuit) {
+    fn downsize_from_circuit<
+        F: PrimeField + Ord + FromUniformBytes<64>,
+        ConcreCircuit: Circuit<F>,
+    >(
+        &mut self,
+        circuit: &ConcreCircuit,
+    ) {
         let k = k_from_circuit(circuit);
         self.downsize(k);
     }

src/poly/kzg/mod.rs

@@ -23,9 +23,12 @@ use crate::poly::query::VerifierQuery;
 use crate::poly::{Coeff, Error, LagrangeCoeff, Polynomial, ProverQuery};
 use crate::utils::arithmetic::{
     eval_polynomial, evals_inner_product, inner_product, kate_division, lagrange_interpolate,
-    msm_inner_product, powers, truncate, truncated_powers, MSM,
+    msm_inner_product, powers, MSM,
 };
 
+#[cfg(feature = "truncated-challenges")]
+use crate::utils::arithmetic::{truncate, truncated_powers};
+
 use crate::poly::commitment::{Params, PolynomialCommitmentScheme};
 use crate::poly::kzg::utils::construct_intermediate_sets;
 use crate::transcript::{Hashable, Sampleable, Transcript};
@@ -109,7 +112,15 @@ where
 
         let q_polys = q_polys
             .iter()
-            .map(|polys| inner_product(polys, truncated_powers(x1)))
+            .map(|polys| {
+                #[cfg(feature = "truncated-challenges")]
+                let x1 = truncated_powers(x1);
+
+                #[cfg(not(feature = "truncated-challenges"))]
+                let x1 = powers(x1);
+
+                inner_product(polys, x1)
+            })
             .collect::<Vec<_>>();
 
         let f_poly = {
@@ -134,6 +145,7 @@ where
         transcript.write(&f_com).map_err(|_| Error::OpeningError)?;
 
         let x3: E::Fr = transcript.squeeze_challenge();
+        #[cfg(feature = "truncated-challenges")]
         let x3 = truncate(x3);
 
         for q_poly in q_polys.iter() {
@@ -147,7 +159,13 @@ where
         let final_poly = {
             let mut polys = q_polys;
             polys.push(f_poly);
-            inner_product(&polys, truncated_powers(x4))
+            #[cfg(feature = "truncated-challenges")]
+            let powers = truncated_powers(x4);
+
+            #[cfg(not(feature = "truncated-challenges"))]
+            let powers = powers(x4);
+
+            inner_product(&polys, powers)
         };
         let v = eval_polynomial(&final_poly, x3);
 
@@ -189,19 +207,36 @@ where
 
         let q_coms = q_coms
             .iter()
-            .map(|msms| msm_inner_product(msms, truncated_powers(x1)))
+            .map(|msms| {
+                #[cfg(feature = "truncated-challenges")]
+                let powers = truncated_powers(x1);
+
+                #[cfg(not(feature = "truncated-challenges"))]
+                let powers = powers(x1);
+
+                msm_inner_product(msms, powers)
+            })
             .collect::<Vec<_>>();
 
         let q_eval_sets = q_eval_sets
             .iter()
-            .map(|evals| evals_inner_product(evals, truncated_powers(x1)))
+            .map(|evals| {
+                #[cfg(feature = "truncated-challenges")]
+                let powers = truncated_powers(x1);
+
+                #[cfg(not(feature = "truncated-challenges"))]
+                let powers = powers(x1);
+
+                evals_inner_product(evals, powers)
+            })
             .collect::<Vec<_>>();
 
         let f_com: E::G1Affine = transcript.read().map_err(|_| Error::SamplingError)?;
 
         // Sample a challenge x_3 for checking that f(X) was committed to
         // correctly.
         let x3: E::Fr = transcript.squeeze_challenge();
+        #[cfg(feature = "truncated-challenges")]
         let x3 = truncate(x3);
 
         let mut q_evals_on_x3 = Vec::<E::Fr>::with_capacity(q_eval_sets.len());
@@ -232,13 +267,27 @@ where
             let mut f_com_as_msm = MSMKZG::new();
             f_com_as_msm.append_term(E::Fr::ONE, f_com.into());
             polys.push(f_com_as_msm);
-            msm_inner_product(&polys, truncated_powers(x4))
+
+            #[cfg(feature = "truncated-challenges")]
+            let powers = truncated_powers(x4);
+
+            #[cfg(not(feature = "truncated-challenges"))]
+            let powers = powers(x4);
+
+            msm_inner_product(&polys, powers)
         };
 
         let v = {
             let mut evals = q_evals_on_x3;
             evals.push(f_eval);
-            inner_product(&evals, truncated_powers(x4))
+
+            #[cfg(feature = "truncated-challenges")]
+            let powers = truncated_powers(x4);
+
+            #[cfg(not(feature = "truncated-challenges"))]
+            let powers = powers(x4);
+
+            inner_product(&evals, powers)
         };
 
         let pi: E::G1Affine = transcript.read().map_err(|_| Error::SamplingError)?;

src/poly/kzg/utils.rs

@@ -27,7 +27,7 @@ pub(super) type IntermediateSets<F, Q> = (
     Vec<Vec<F>>,
 );
 
-pub(super) fn construct_intermediate_sets<F: Field + Ord, I, Q: Query<F>>(
+pub fn construct_intermediate_sets<F: Field + Ord, I, Q: Query<F>>(
     queries: I,
 ) -> IntermediateSets<F, Q>
 where

src/transcript/mod.rs

@@ -77,7 +77,6 @@ pub struct CircuitTranscript<H: TranscriptHash> {
 impl<H: TranscriptHash> CircuitTranscript<H> {
     /// Returns the buffer for non default reading of the buffer (such as for
     /// reading an empty proof)
-    /// TODO: SHOULD WE REMOVE THIS AND WRITE A FUNCTION THAT RETURNS THE PROOF SIZE?
     pub fn buffer(&mut self) -> &mut Cursor<Vec<u8>> {
         &mut self.buffer
     }

src/utils/arithmetic.rs

@@ -232,7 +232,9 @@ pub fn lagrange_interpolate<F: Field>(points: &[F], evals: &[F]) -> Vec<F> {
     }
 }
 
+#[cfg(feature = "truncated-challenges")]
 use num_bigint::BigUint;
+
 /// Truncates a scalar field element to half its byte size.
 ///
 /// This function reduces a scalar field element `scalar` to half its size by
@@ -243,13 +245,19 @@ use num_bigint::BigUint;
 /// approximately twice the size of the security parameter. When scalars are
 /// sampled uniformly at random, truncating to half the field size retains
 /// sufficient entropy for security while reducing computational overhead.
+///
+/// # Warning
+/// 128 bits may not be enough entropy depending on the application. For example,
+/// it makes a collision attack feasible with 2^64 memory and ~2^64 operations.
+#[cfg(feature = "truncated-challenges")]
 pub(crate) fn truncate<F: PrimeField>(scalar: F) -> F {
     let nb_bytes = F::NUM_BITS.div_ceil(8).div_ceil(2) as usize;
     let bytes = scalar.to_repr().as_ref()[..nb_bytes].to_vec();
     let bi = BigUint::from_bytes_le(&bytes);
     F::from_str_vartime(&BigUint::to_string(&bi)).unwrap()
 }
 
+#[cfg(feature = "truncated-challenges")]
 pub(crate) fn truncated_powers<F: PrimeField>(base: F) -> impl Iterator<Item = F> {
     powers(base).map(truncate)
 }