[chore] clean up; fix clippy; fix cargo fmt

zhenfeizhang · zhenfeizhang · commit c0f5b9420648 · 2023-06-06T15:17:41.000-04:00
diff --git a/aggregator/src/core.rs b/aggregator/src/core.rs
@@ -2,10 +2,8 @@ use ark_std::{end_timer, start_timer};
 use eth_types::Field;
 use halo2_proofs::{
     circuit::{AssignedCell, Layouter, Value},
-    plonk::Error,
-};
-use halo2_proofs::{
     halo2curves::bn256::{Bn256, G1Affine},
+    plonk::Error,
     poly::{commitment::ParamsProver, kzg::commitment::ParamsKZG},
 };
 use rand::Rng;
diff --git a/aggregator/src/lib.rs b/aggregator/src/lib.rs
@@ -7,7 +7,7 @@ mod batch;
 /// Core module for circuit assignment
 mod core;
 /// Parameters for compression circuit
-pub mod param;
+mod param;
 /// proof aggregation
 mod proof_aggregation;
 /// proof compression
@@ -20,5 +20,6 @@ mod tests;
 
 pub use batch::BatchHash;
 pub use chunk::ChunkHash;
+pub use param::*;
 pub use proof_aggregation::*;
 pub use proof_compression::*;
diff --git a/aggregator/src/proof_aggregation/circuit.rs b/aggregator/src/proof_aggregation/circuit.rs
@@ -1,30 +1,30 @@
 use ark_std::{end_timer, start_timer};
-use halo2_proofs::plonk::Error;
 use halo2_proofs::{
     circuit::{Layouter, SimpleFloorPlanner, Value},
     halo2curves::bn256::{Bn256, Fq, Fr, G1Affine},
-    plonk::{Circuit, ConstraintSystem},
+    plonk::{Circuit, ConstraintSystem, Error},
     poly::{commitment::ParamsProver, kzg::commitment::ParamsKZG},
 };
 use itertools::Itertools;
 use rand::Rng;
-use snark_verifier::loader::halo2::halo2_ecc::halo2_base::{
-    self, AssignedValue, Context, ContextParams,
-};
-use snark_verifier::loader::halo2::Halo2Loader;
 use snark_verifier::{
+    loader::halo2::{
+        halo2_ecc::halo2_base::{self, AssignedValue, Context, ContextParams},
+        Halo2Loader,
+    },
     pcs::kzg::{Bdfg21, Kzg, KzgAccumulator, KzgSuccinctVerifyingKey},
     util::arithmetic::fe_to_limbs,
 };
-use snark_verifier_sdk::halo2::aggregation::{aggregate, flatten_accumulator};
-use snark_verifier_sdk::CircuitExt;
-use snark_verifier_sdk::{halo2::aggregation::Svk, NativeLoader, Snark, SnarkWitness};
+use snark_verifier_sdk::{
+    halo2::aggregation::{aggregate, flatten_accumulator, Svk},
+    CircuitExt, NativeLoader, Snark, SnarkWitness,
+};
 use zkevm_circuits::util::Challenges;
 
-use crate::core::{assign_batch_hashes, extract_accumulators_and_proof};
-use crate::proof_aggregation::config::AggregationConfig;
 use crate::{
+    core::{assign_batch_hashes, extract_accumulators_and_proof},
     param::{ConfigParams, BITS, LIMBS},
+    proof_aggregation::config::AggregationConfig,
     BatchHashCircuit, ChunkHash,
 };
 
@@ -104,7 +104,7 @@ impl AggregationCircuit {
 
         Self {
             svk,
-            snarks: snarks.into_iter().cloned().map_into().collect(),
+            snarks: snarks.iter().cloned().map_into().collect(),
             flattened_instances,
             as_proof: Value::known(as_proof),
             batch_hash_circuit,
@@ -157,18 +157,18 @@ impl Circuit<Fr> for AggregationCircuit {
             .expect("load range lookup table");
         let mut first_pass = halo2_base::SKIP_FIRST_PASS;
 
-        // This circuit takes 2 steps
+        // This circuit takes 3 steps
         // - 1. use aggregation circuit to aggregate the multiple snarks into a single one;
-        //      re-export all the public input of the snarks, denoted by [snarks_instances], and
-        //      the accumulator [acc_instances]
-        // - 2. use public input aggregation circuit to aggregate the chunks;
-        //      expose the instance dentoed by [pi_agg_instances]
-        // - 3. assert [snarks_instances] are private inputs used for public input aggregation circuit
+        //   re-export all the public input of the snarks, denoted by [snarks_instances], and the
+        //   accumulator [acc_instances]
+        // - 2. use public input aggregation circuit to aggregate the chunks; expose the instance
+        //   dentoed by [pi_agg_instances]
+        // - 3. assert [snarks_instances] are private inputs used for public input aggregation
+        //   circuit
 
         // ==============================================
         // Step 1: aggregation circuit
         // ==============================================
-        // let mut aggregation_instances = vec![];
         let mut accumulator_instances: Vec<AssignedValue<Fr>> = vec![];
         let mut snark_inputs: Vec<AssignedValue<Fr>> = vec![];
         layouter.assign_region(
@@ -192,7 +192,8 @@ impl Circuit<Fr> for AggregationCircuit {
 
                 //
                 // extract the assigned values for
-                // - instances which are the public inputs of each chunk (prefixed with 12 instances from previous accumualtors)
+                // - instances which are the public inputs of each chunk (prefixed with 12 instances
+                //   from previous accumualtors)
                 // - new accumulator to be verified on chain
                 //
                 let (assigned_aggreation_instances, acc) = aggregate::<Kzg<Bn256, Bdfg21>>(
@@ -209,17 +210,13 @@ impl Circuit<Fr> for AggregationCircuit {
                 // extract the following cells for later constraints
                 // - the accumulators
                 // - the public input from snark
-                accumulator_instances.extend(
-                    flatten_accumulator(acc)
-                        .iter()
-                        .map(|assigned| assigned.clone()),
-                );
+                accumulator_instances.extend(flatten_accumulator(acc).iter().copied());
                 // - the snark is not a fresh one, assigned_instances already contains an
                 //   accumulator so we want to skip the first 12 elements from the public input
                 snark_inputs.extend(
                     assigned_aggreation_instances
                         .iter()
-                        .flat_map(|instance_column| instance_column.iter().skip(12).map(|x| x)),
+                        .flat_map(|instance_column| instance_column.iter().skip(12)),
                 );
 
                 config.range().finalize(&mut loader.ctx_mut());
@@ -243,7 +240,6 @@ impl Circuit<Fr> for AggregationCircuit {
         // step 2: public input aggregation circuit
         // ==============================================
         // extract all the hashes and load them to the hash table
-        // assert the public input matches that of the pi_aggregation_circuit
         let challenges = challenge.values(&layouter);
 
         let timer = start_timer!(|| ("extract hash").to_string());
@@ -297,9 +293,7 @@ impl Circuit<Fr> for AggregationCircuit {
 
                 for chunk_idx in 0..self.snarks.len() {
                     // step 3.1, data hash
-                    // - batch_data_hash := keccak(chunk_0.data_hash
-                    //                   || ...
-                    //                   || chunk_k-1.data_hash)
+                    // - batch_data_hash := keccak(chunk_0.data_hash || ... || chunk_k-1.data_hash)
                     // where batch_data_hash is the second hash for pi aggregation
                     for i in 0..32 {
                         region.constrain_equal(
diff --git a/aggregator/src/proof_aggregation/config.rs b/aggregator/src/proof_aggregation/config.rs
@@ -19,8 +19,9 @@ use zkevm_circuits::{
 use crate::param::{ConfigParams, BITS, LIMBS};
 
 #[derive(Debug, Clone)]
-/// Configurations for aggregation circuit
-/// This config is hard coded for BN256 curve
+#[rustfmt::skip]
+/// Configurations for aggregation circuit.
+/// This config is hard coded for BN256 curve.
 pub struct AggregationConfig {
     /// Non-native field chip configurations
     pub base_field_config: FpConfig<Fr, Fq>,
@@ -30,9 +31,9 @@ pub struct AggregationConfig {
     /// - accumulator from aggregation (12 elements)
     /// - aggregated public inputs (132 elements):
     ///     chain_id ||
-    ///     chunk[0].prev_state_root ||
-    ///     chunk[k-1].post_state_root ||
-    ///     chunk[k-1].withdraw_root ||
+    ///     chunk\[0\].prev_state_root ||
+    ///     chunk\[k-1\].post_state_root ||
+    ///     chunk\[k-1\].withdraw_root ||
     ///     batch_data_hash
     pub instance: Column<Instance>,
 }
diff --git a/aggregator/src/proof_aggregation/public_input_aggregation.rs b/aggregator/src/proof_aggregation/public_input_aggregation.rs
@@ -11,22 +11,16 @@
 //! Those 4 hashes are obtained from the caller.
 //!
 //! A chunk's public input hash is then derived from the above 4 attributes via
-//! - chunk_pi_hash   := keccak(chain_id    
-//!                   || prev_state_root
-//!                   || post_state_root
-//!                   || withdraw_root
-//!                   || chunk_data_hash)
+//!
+//! - chunk_pi_hash   := keccak(chain_id || prev_state_root || post_state_root || withdraw_root ||
+//!   chunk_data_hash)
 //!
 //! A batch is a list of continuous chunks. It consists of 2 hashes
-//! - batch_data_hash := keccak(chunk_0.data_hash      
-//!                   || ...                        
-//!                   || chunk_k-1.data_hash)
 //!
-//! - batch_pi_hash   := keccak(chain_id               
-//!                   || chunk_0.prev_state_root    
-//!                   || chunk_k-1.post_state_root  
-//!                   || chunk_k-1.withdraw_root    
-//!                   || batch_data_hash)
+//! - batch_data_hash := keccak(chunk_0.data_hash || ... || chunk_k-1.data_hash)
+//!
+//! - batch_pi_hash   := keccak(chain_id || chunk_0.prev_state_root || chunk_k-1.post_state_root ||
+//!   chunk_k-1.withdraw_root || batch_data_hash)
 //!
 //! Note that chain_id is used for all public input hashes. But not for any data hashes.
 //!
diff --git a/aggregator/src/proof_aggregation/public_input_aggregation/circuit.rs b/aggregator/src/proof_aggregation/public_input_aggregation/circuit.rs
@@ -77,7 +77,7 @@ impl<F: Field> BatchHashCircuit<F> {
     /// orders:
     /// - batch_public_input_hash
     /// - batch_data_hash_preimage
-    /// - chunk[i].piHash for i in [0, k)
+    /// - chunk\[i\].piHash for i in \[0, k)
     pub(crate) fn extract_hash_preimages(&self) -> Vec<Vec<u8>> {
         let mut res = vec![];
 
diff --git a/aggregator/src/proof_aggregation/public_input_aggregation/circuit_ext.rs b/aggregator/src/proof_aggregation/public_input_aggregation/circuit_ext.rs
@@ -5,7 +5,7 @@ use crate::BatchHashCircuit;
 
 impl<F: Field> CircuitExt<F> for BatchHashCircuit<F> {
     fn num_instance(&self) -> Vec<usize> {
-        vec![self.instances()[0].len()]
+        vec![132]
     }
 
     /// Compute the public inputs for this circuit.
diff --git a/aggregator/src/proof_compression/circuit.rs b/aggregator/src/proof_compression/circuit.rs
@@ -9,16 +9,18 @@ use halo2_proofs::{
     plonk::{Circuit, ConstraintSystem, Error},
 };
 use rand::Rng;
-use snark_verifier::loader::halo2::halo2_ecc::halo2_base::{
-    self,
-    halo2_proofs::{
-        halo2curves::bn256::{Bn256, Fr},
-        poly::{commitment::ParamsProver, kzg::commitment::ParamsKZG},
-    },
-    Context, ContextParams,
-};
 use snark_verifier::{
-    loader::halo2::Halo2Loader,
+    loader::halo2::{
+        halo2_ecc::halo2_base::{
+            self,
+            halo2_proofs::{
+                halo2curves::bn256::{Bn256, Fr},
+                poly::{commitment::ParamsProver, kzg::commitment::ParamsKZG},
+            },
+            Context, ContextParams,
+        },
+        Halo2Loader,
+    },
     pcs::kzg::{Bdfg21, Kzg, KzgAccumulator, KzgSuccinctVerifyingKey},
     util::arithmetic::fe_to_limbs,
 };
@@ -98,7 +100,7 @@ impl Circuit<Fr> for CompressionCircuit {
         let mut first_pass = halo2_base::SKIP_FIRST_PASS;
         let mut instances = vec![];
         layouter.assign_region(
-            || "",
+            || "compression circuit",
             |region| {
                 if first_pass {
                     first_pass = false;
diff --git a/aggregator/src/proof_compression/config.rs b/aggregator/src/proof_compression/config.rs
@@ -2,11 +2,13 @@ use halo2_proofs::{
     halo2curves::bn256::{Fq, Fr, G1Affine},
     plonk::{Column, ConstraintSystem, Instance},
 };
-use snark_verifier::loader::halo2::halo2_ecc::ecc::{BaseFieldEccChip, EccChip};
-use snark_verifier::loader::halo2::halo2_ecc::fields::fp::FpConfig;
-use snark_verifier::loader::halo2::halo2_ecc::halo2_base::{
-    gates::{flex_gate::FlexGateConfig, range::RangeConfig},
-    utils::modulus,
+use snark_verifier::loader::halo2::halo2_ecc::{
+    ecc::{BaseFieldEccChip, EccChip},
+    fields::fp::FpConfig,
+    halo2_base::{
+        gates::{flex_gate::FlexGateConfig, range::RangeConfig},
+        utils::modulus,
+    },
 };
 
 use crate::param::{ConfigParams, BITS, LIMBS};
diff --git a/aggregator/src/tests/mock_chunk.rs b/aggregator/src/tests/mock_chunk.rs
@@ -1,10 +1,10 @@
 use ark_std::{end_timer, start_timer, test_rng};
 use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr};
 use snark_verifier::loader::halo2::halo2_ecc::halo2_base::utils::fs::gen_srs;
-use snark_verifier_sdk::CircuitExt;
 use snark_verifier_sdk::{
     gen_pk,
     halo2::{gen_snark_shplonk, verify_snark_shplonk},
+    CircuitExt,
 };
 
 use crate::{ChunkHash, LOG_DEGREE};
@@ -15,6 +15,11 @@ mod config;
 
 #[derive(Debug, Default, Clone, Copy)]
 /// A mock chunk circuit
+///
+/// This mock chunk circuit simulates a zkEVM circuit.
+/// It's public inputs consists of 64 elements:
+/// - data hash
+/// - public input hash
 pub struct MockChunkCircuit {
     pub(crate) chunk: ChunkHash,
 }
diff --git a/aggregator/src/tests/mock_chunk/circuit.rs b/aggregator/src/tests/mock_chunk/circuit.rs
@@ -35,6 +35,7 @@ impl Circuit<Fr> for MockChunkCircuit {
     fn without_witnesses(&self) -> Self {
         Self::default()
     }
+
     fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {
         let challenges = Challenges::construct(meta);
         let challenges_exprs = challenges.exprs(meta);
diff --git a/aggregator/src/tests/proof_aggregation.rs b/aggregator/src/tests/proof_aggregation.rs
@@ -66,7 +66,7 @@ fn test_aggregation_circuit() {
                 let snark = gen_snark_shplonk(
                     &layer_0_params,
                     &layer_0_pk,
-                    circuit.clone(),
+                    *circuit,
                     &mut rng,
                     Some(&path.join(Path::new(format!("layer_0_{}.snark", i).as_str()))),
                 );
@@ -102,11 +102,7 @@ fn test_aggregation_circuit() {
         let compression_circuit =
             CompressionCircuit::new(&layer_1_params, layer_0_snarks[0].clone(), true, &mut rng);
 
-        let layer_1_pk = gen_pk(
-            &layer_1_params,
-            &compression_circuit,
-            None
-        );
+        let layer_1_pk = gen_pk(&layer_1_params, &compression_circuit, None);
 
         log::trace!("finished layer 1 pk gen");
         let mut layer_1_snarks = vec![];
@@ -154,8 +150,7 @@ fn test_aggregation_circuit() {
         let instances = aggregation_circuit.instances();
 
         log::trace!("start mock proving");
-        let mock_prover =
-            MockProver::<Fr>::run(k1, &aggregation_circuit, instances.clone()).unwrap();
+        let mock_prover = MockProver::<Fr>::run(k1, &aggregation_circuit, instances).unwrap();
 
         mock_prover.assert_satisfied_par();
     }
diff --git a/aggregator/src/tests/proof_compression.rs b/aggregator/src/tests/proof_compression.rs

Original file line number	Diff line number	Diff line change
`@@ -5,7 +5,7 @@ use crate::BatchHashCircuit;`
`5`	`5`
`6`	`6`	`impl<F: Field> CircuitExt<F> for BatchHashCircuit<F> {`
`7`	`7`	`fn num_instance(&self) -> Vec<usize> {`
`8`		`- vec![self.instances()[0].len()]`
	`8`	`+ vec![132]`
`9`	`9`	`}`
`10`	`10`
`11`	`11`	`/// Compute the public inputs for this circuit.`
Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,7 @@ impl Circuit<Fr> for MockChunkCircuit {`
`35`	`35`	`fn without_witnesses(&self) -> Self {`
`36`	`36`	`Self::default()`
`37`	`37`	`}`
	`38`	`+`
`38`	`39`	`fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {`
`39`	`40`	`let challenges = Challenges::construct(meta);`
`40`	`41`	`let challenges_exprs = challenges.exprs(meta);`