Address some of the review comments from @CPerezz

halo2_proofs/Cargo.toml

@@ -95,7 +95,7 @@ thread-safe-region = []
 sanity-checks = []
 batch = ["rand_core/getrandom"]
 circuit-params = []
-dhat-heap = []
+heap-profiling = []
 cost-estimator = ["serde", "serde_derive"]
 derive_serde = ["halo2curves/derive_serde"]
 

halo2_proofs/src/plonk.rs

@@ -108,8 +108,7 @@ pub struct VerifyingKey<C: CurveAffine> {
     domain: EvaluationDomain<C::Scalar>,
     fixed_commitments: Vec<C>,
     permutation: permutation::VerifyingKey<C>,
-    /// TODO: Remove pub
-    pub cs: ConstraintSystem<C::Scalar>,
+    cs: ConstraintSystem<C::Scalar>,
     /// Cached maximum degree of `cs` (which doesn't change after construction).
     cs_degree: usize,
     /// The representative of this `VerifyingKey` in transcripts.

halo2_proofs/src/plonk/circuit.rs

@@ -4,7 +4,7 @@ use crate::dev::metadata;
 use crate::plonk::WitnessCollection;
 use crate::{
     circuit::{Layouter, Region, Value},
-    poly::{batch_invert_assigned, LagrangeCoeff, Polynomial, Rotation},
+    poly::{batch_invert_assigned, Polynomial, Rotation},
 };
 use core::cmp::max;
 use core::ops::{Add, Mul};
@@ -1690,13 +1690,10 @@ impl<F: Field> Gate<F> {
 pub struct PreprocessingV2<F: Field> {
     // TODO(Edu): Can we replace this by a simpler structure?
     pub(crate) permutation: permutation::keygen::Assembly,
-    // TODO(Edu): Replace this by Vec<Vec<F>>.  Requires some methods of Polynomial to take Vec<F>
-    // instead
-    // pub(crate) fixed: Vec<Polynomial<F, LagrangeCoeff>>,
     pub(crate) fixed: Vec<Vec<F>>,
 }
 
-/// This is a description of a low level Plonkish compiled circuit.  Contains the Constraint System
+/// This is a description of a low level Plonkish compiled circuit. Contains the Constraint System
 /// as well as the fixed columns and copy constraints information.
 #[derive(Debug, Clone)]
 pub struct CompiledCircuitV2<F: Field> {
@@ -1859,7 +1856,6 @@ impl<'a, F: Field, ConcreteCircuit: Circuit<F>> WitnessCalculator<'a, F, Concret
         &mut self,
         phase: u8,
         challenges: &HashMap<usize, F>,
-        // ) -> Result<Vec<Option<Polynomial<Assigned<F>, LagrangeCoeff>>>, Error> {
     ) -> Result<Vec<Option<Vec<Assigned<F>>>>, Error> {
         if phase != self.next_phase {
             return Err(Error::Other(format!(
@@ -1873,6 +1869,7 @@ impl<'a, F: Field, ConcreteCircuit: Circuit<F>> WitnessCalculator<'a, F, Concret
             2 => ThirdPhase.to_sealed(),
             _ => unreachable!("only phase [0,2] supported"),
         };
+
         let mut witness = WitnessCollection {
             k: self.k,
             current_phase,
@@ -1968,7 +1965,7 @@ pub fn compile_circuit<F: Field, ConcreteCircuit: Circuit<F>>(
         cs.constants.clone(),
     )?;
 
-    let mut fixed = batch_invert_assigned(assembly.fixed);
+    let fixed = batch_invert_assigned(assembly.fixed);
     let (cs, selector_polys) = if compress_selectors {
         cs.compress_selectors(assembly.selectors.clone())
     } else {
@@ -2146,7 +2143,6 @@ impl<F: Field> ConstraintSystemV2Backend<F> {
             fixed: queries.fixed,
             num_advice_queries,
         };
-        // println!("DBG collected queries\n{:#?}", queries);
         (queries, gates, lookups, shuffles)
     }
 }

halo2_proofs/src/plonk/prover.rs

@@ -92,9 +92,6 @@ impl<
     pub fn commit_phase(
         &mut self,
         phase: u8,
-        // TODO: Turn this into Vec<Option<Vec<F>>>.  Requires batch_invert_assigned to work with
-        // Vec<F>
-        // witness: Vec<Option<Polynomial<Assigned<Scheme::Scalar>, LagrangeCoeff>>>,
         witness: Vec<Option<Vec<Assigned<Scheme::Scalar>>>>,
     ) -> Result<HashMap<usize, Scheme::Scalar>, Error>
     where
@@ -161,7 +158,6 @@ impl<
     where
         Scheme::Scalar: WithSmallOrderMulGroup<3> + FromUniformBytes<64>,
     {
-        // println!("DBG prove vk.queries.advices {:?}", pk.vk.queries.advice);
         for instance in instances.iter() {
             if instance.len() != pk.vk.cs.num_instance_columns {
                 return Err(Error::InvalidInstances);
@@ -172,7 +168,6 @@ impl<
         pk.vk.hash_into(transcript)?;
 
         let meta = &pk.vk.cs;
-        // let queries = &pk.vk.queries;
         let phases = meta.phases().collect();
 
         let domain = &pk.vk.domain;
@@ -239,7 +234,11 @@ impl<
 
         let advice = vec![
             AdviceSingle::<Scheme::Curve, LagrangeCoeff> {
-                advice_polys: vec![domain.empty_lagrange(); meta.num_advice_columns],
+                // Create vectors with empty polynomials to free space while they are not being used
+                advice_polys: vec![
+                    Polynomial::new_empty(0, Scheme::Scalar::ZERO);
+                    meta.num_advice_columns
+                ],
                 advice_blinds: vec![Blind::default(); meta.num_advice_columns],
             };
             instances.len()
@@ -264,9 +263,6 @@ impl<
     pub fn commit_phase(
         &mut self,
         phase: u8,
-        // TODO: Turn this into Vec<Option<Vec<F>>>.  Requires batch_invert_assigned to work with
-        // Vec<F>
-        // witness: Vec<Vec<Option<Polynomial<Assigned<Scheme::Scalar>, LagrangeCoeff>>>>,
         witness: Vec<Vec<Option<Vec<Assigned<Scheme::Scalar>>>>>,
     ) -> Result<HashMap<usize, Scheme::Scalar>, Error>
     where
@@ -284,9 +280,6 @@ impl<
 
         let params = self.params;
         let meta = &self.pk.vk.cs;
-        // let queries = &self.pk.vk.queries;
-        // println!("DBG commit_phase gate {:?}", meta.gates()[0]);
-        // println!("DBG commit_phase queries {:?}", meta.advice_queries());
 
         let mut rng = &mut self.rng;
 
@@ -922,15 +915,12 @@ where
     let mut challenges = HashMap::new();
     let phases = prover.phases.clone();
     for phase in &phases {
-        // for phase in [0] {
         println!("DBG phase {}", phase.0);
         let mut witnesses = Vec::with_capacity(circuits.len());
         for witness_calc in witness_calcs.iter_mut() {
             witnesses.push(witness_calc.calc(phase.0, &challenges)?);
         }
-        // println!("DBG witness: {:?}", witness);
         challenges = prover.commit_phase(phase.0, witnesses).unwrap();
-        // println!("DBG challenges {:?}", challenges);
     }
     prover.create_proof()
 }

halo2_proofs/src/plonk/verifier.rs

@@ -383,8 +383,6 @@ where
         vanishing.verify(params, expressions, y, xn)
     };
 
-    // println!("DBG verify fixed_queries:\n{:#?}", vk.cs.fixed_queries);
-
     let queries = instance_commitments
         .iter()
         .zip(instance_evals.iter())

halo2_proofs/src/transcript.rs

@@ -358,13 +358,11 @@ where
     fn write_point(&mut self, point: C) -> io::Result<()> {
         self.common_point(point)?;
         let compressed = point.to_bytes();
-        // println!("DBG write_point\n{:02x?}", compressed.as_ref());
         self.writer.write_all(compressed.as_ref())
     }
     fn write_scalar(&mut self, scalar: C::Scalar) -> io::Result<()> {
         self.common_scalar(scalar)?;
         let data = scalar.to_repr();
-        // println!("DBG write_scalar\n{:02x?}", data.as_ref());
         self.writer.write_all(data.as_ref())
     }
 }

halo2_proofs/tests/frontend_backend_split.rs

@@ -1,7 +1,7 @@
 #![allow(clippy::many_single_char_names)]
 #![allow(clippy::op_ref)]
 
-#[cfg(feature = "dhat-heap")]
+#[cfg(feature = "heap-profiling")]
 #[global_allocator]
 static ALLOC: dhat::Alloc = dhat::Alloc;
 
@@ -468,14 +468,14 @@ fn test_mycircuit_mock() {
 
 use std::time::Instant;
 
-const K: u32 = 8;
-const WIDTH_FACTOR: usize = 1;
-// const K: u32 = 16;
-// const WIDTH_FACTOR: usize = 4;
+// const K: u32 = 8;
+// const WIDTH_FACTOR: usize = 1;
+const K: u32 = 16;
+const WIDTH_FACTOR: usize = 4;
 
 #[test]
 fn test_mycircuit_full_legacy() {
-    #[cfg(feature = "dhat-heap")]
+    #[cfg(feature = "heap-profiling")]
     let _profiler = dhat::Profiler::new_heap();
 
     use halo2_proofs::plonk::{create_proof, keygen_pk, keygen_vk};
@@ -489,8 +489,6 @@ fn test_mycircuit_full_legacy() {
     let verifier_params = params.verifier_params();
     let start = Instant::now();
     let vk = keygen_vk(&params, &circuit).expect("keygen_vk should not fail");
-    // println!("DBG gate {:?}", vk.cs.gates()[0]);
-    // println!("DBG queries {:?}", vk.cs.advice_queries());
     let pk = keygen_pk(&params, vk.clone(), &circuit).expect("keygen_pk should not fail");
     println!("Keygen: {:?}", start.elapsed());
 
@@ -513,10 +511,6 @@ fn test_mycircuit_full_legacy() {
     )
     .expect("proof generation should not fail");
     let proof = transcript.finalize();
-    // println!("DBG proof.len={} ", proof.len());
-    // for word in proof.chunks(32) {
-    //     println!("  {:02x?}", word);
-    // }
     println!("Prove: {:?}", start.elapsed());
 
     // Verify
@@ -538,7 +532,7 @@ fn test_mycircuit_full_legacy() {
 
 #[test]
 fn test_mycircuit_full_split() {
-    #[cfg(feature = "dhat-heap")]
+    #[cfg(feature = "heap-profiling")]
     let _profiler = dhat::Profiler::new_heap();
 
     let k = K;
@@ -551,14 +545,13 @@ fn test_mycircuit_full_split() {
     let verifier_params = params.verifier_params();
     let start = Instant::now();
     let vk = keygen_vk_v2(&params, &compiled_circuit).expect("keygen_vk should not fail");
-    // println!("vk: {:#?}", vk);
     let pk =
         keygen_pk_v2(&params, vk.clone(), &compiled_circuit).expect("keygen_pk should not fail");
     println!("Keygen: {:?}", start.elapsed());
     drop(compiled_circuit);
 
     // Proving
-    println!("DBG Proving...");
+    println!("Proving...");
     let instances = circuit.instances();
     let instances_slice: &[&[Fr]] = &(instances
         .iter()
@@ -579,24 +572,17 @@ fn test_mycircuit_full_split() {
         .unwrap();
     let mut challenges = HashMap::new();
     for phase in 0..cs.phases().count() {
-        // for phase in [0] {
-        println!("DBG phase {}", phase);
+        println!("phase {}", phase);
         let witness = witness_calc.calc(phase as u8, &challenges).unwrap();
-        // println!("DBG witness: {:?}", witness);
         challenges = prover.commit_phase(phase as u8, witness).unwrap();
-        // println!("DBG challenges {:?}", challenges);
     }
     prover.create_proof().unwrap();
     let proof = transcript.finalize();
-    // println!("DBG proof.len={} ", proof.len());
-    // for word in proof.chunks(32) {
-    //     println!("  {:02x?}", word);
-    // }
     println!("Prove: {:?}", start.elapsed());
 
     // Verify
     let start = Instant::now();
-    println!("DBG Verifying...");
+    println!("Verifying...");
     let mut verifier_transcript =
         Blake2bRead::<_, G1Affine, Challenge255<_>>::init(proof.as_slice());
     let strategy = SingleStrategy::new(verifier_params);