Unify circuit types names and interfaces (privacy-scaling-exploration…

…s#937)

Given a circuit Foo:
- Unify the struct that contains the columns to FooCircuitConfig
- Unify the struct that contains the witness to FooCircuit
- Unify the config constructor to FooCircuitConfig::new, following the new SubCircuitConfig trait
- Unify the creation of a circuit from a block to FooCircuit::new_from_block following the new SubCircuit trait
- Unify the loading of fixed internal tables to load_aux_tables
- Unify the synthesis step of subcircuits to synthesize_sub following the new SubCircuit trait
- Remove generic associated const from circuit+config types as much as possible
    - Only possible in TxCircuit.  Not possible in PiCircuit due to halo2 Circuit trait
- Implement the Circuit trait to all subcircuits uniformly
- Move circuit tunning parameters to CircuitParams so that they can be taken from a single source
- Add an instance function to all circuits following the new SubCircuit trait

Clean up

Don't unwrap in non-test code

Address some comments from pinkie

Address some comments from pinkie and han

bus-mapping/src/circuit_input_builder.rs

@@ -29,6 +29,7 @@ pub use execution::{
 };
 pub use input_state_ref::CircuitInputStateRef;
 use itertools::Itertools;
+use log::warn;
 use std::collections::HashMap;
 pub use transaction::{Transaction, TransactionContext};
 
@@ -42,9 +43,13 @@ pub struct CircuitsParams {
     // TODO: evm_rows: Maximum number of rows in the EVM Circuit
     /// Maximum number of txs in the Tx Circuit
     pub max_txs: usize,
-    // TODO: max_calldata: Maximum number of bytes from all txs calldata in the Tx Circuit
+    /// Maximum number of bytes from all txs calldata in the Tx Circuit
+    pub max_calldata: usize,
+    /// Maximum number of bytes supported in the Bytecode Circuit
+    pub max_bytecode: usize,
+    // TODO: Rename for consistency
     /// Pad the keccak circuit with this number of invocations to a static
-    /// capacity
+    /// capacity.  Number of keccak_f that the Keccak circuit will support.
     pub keccak_padding: Option<usize>,
 }
 
@@ -54,6 +59,8 @@ impl Default for CircuitsParams {
         CircuitsParams {
             max_rws: 1000,
             max_txs: 1,
+            max_calldata: 256,
+            max_bytecode: 512,
             keccak_padding: None,
         }
     }
@@ -271,27 +278,24 @@ impl<'a> CircuitInputBuilder {
 
         Ok(())
     }
+}
 
-    /// Return all the keccak inputs used during the processing of the current
-    /// block.
-    pub fn keccak_inputs(&self) -> Result<Vec<Vec<u8>>, Error> {
-        let mut keccak_inputs = Vec::new();
-        // Tx Circuit
-        let txs: Vec<geth_types::Transaction> = self.block.txs.iter().map(|tx| tx.into()).collect();
-        keccak_inputs.extend_from_slice(&keccak_inputs_tx_circuit(
-            &txs,
-            self.block.chain_id.as_u64(),
-        )?);
-        // Bytecode Circuit
-        for bytecode in self.code_db.0.values() {
-            keccak_inputs.push(bytecode.clone());
-        }
-        // EVM Circuit
-        keccak_inputs.extend_from_slice(&self.block.sha3_inputs);
-        // MPT Circuit
-        // TODO https://github.com/privacy-scaling-explorations/zkevm-circuits/issues/696
-        Ok(keccak_inputs)
+/// Return all the keccak inputs used during the processing of the current
+/// block.
+pub fn keccak_inputs(block: &Block, code_db: &CodeDB) -> Result<Vec<Vec<u8>>, Error> {
+    let mut keccak_inputs = Vec::new();
+    // Tx Circuit
+    let txs: Vec<geth_types::Transaction> = block.txs.iter().map(|tx| tx.into()).collect();
+    keccak_inputs.extend_from_slice(&keccak_inputs_tx_circuit(&txs, block.chain_id.as_u64())?);
+    // Bytecode Circuit
+    for bytecode in code_db.0.values() {
+        keccak_inputs.push(bytecode.clone());
     }
+    // EVM Circuit
+    keccak_inputs.extend_from_slice(&block.sha3_inputs);
+    // MPT Circuit
+    // TODO https://github.com/privacy-scaling-explorations/zkevm-circuits/issues/696
+    Ok(keccak_inputs)
 }
 
 /// Generate the keccak inputs required by the SignVerify Chip from the
@@ -316,7 +320,19 @@ pub fn keccak_inputs_tx_circuit(
     chain_id: u64,
 ) -> Result<Vec<Vec<u8>>, Error> {
     let mut inputs = Vec::new();
-    let sign_datas: Vec<SignData> = txs.iter().map(|tx| tx.sign_data(chain_id)).try_collect()?;
+    let sign_datas: Vec<SignData> = txs
+        .iter()
+        .enumerate()
+        .filter(|(i, tx)| {
+            if tx.v == 0 && tx.r.is_zero() && tx.s.is_zero() {
+                warn!("tx {} is not signed, skipping tx circuit keccak input", i);
+                false
+            } else {
+                true
+            }
+        })
+        .map(|(_, tx)| tx.sign_data(chain_id))
+        .try_collect()?;
     // Keccak inputs from SignVerify Chip
     let sign_verify_inputs = keccak_inputs_sign_verify(&sign_datas);
     inputs.extend_from_slice(&sign_verify_inputs);

bus-mapping/src/circuit_input_builder/block.rs

@@ -51,6 +51,7 @@ pub struct BlockSteps {
     pub end_block_last: ExecStep,
 }
 
+// TODO: Remove fields that are duplicated in`eth_block`
 /// Circuit Input related to a block.
 #[derive(Debug)]
 pub struct Block {
@@ -88,15 +89,17 @@ pub struct Block {
     code: HashMap<Hash, Vec<u8>>,
     /// Circuits Setup Paramteres
     pub circuits_params: CircuitsParams,
+    /// Original block from geth
+    pub eth_block: eth_types::Block<eth_types::Transaction>,
 }
 
 impl Block {
     /// Create a new block.
-    pub fn new<TX>(
+    pub fn new(
         chain_id: Word,
         history_hashes: Vec<Word>,
         prev_state_root: Word,
-        eth_block: &eth_types::Block<TX>,
+        eth_block: &eth_types::Block<eth_types::Transaction>,
         circuits_params: CircuitsParams,
     ) -> Result<Self, Error> {
         if eth_block.base_fee_per_gas.is_none() {
@@ -139,6 +142,7 @@ impl Block {
             code: HashMap::new(),
             sha3_inputs: Vec::new(),
             circuits_params,
+            eth_block: eth_block.clone(),
         })
     }
 

circuit-benchmarks/src/bytecode_circuit.rs

@@ -19,8 +19,9 @@ mod tests {
     use rand::SeedableRng;
     use rand_xorshift::XorShiftRng;
     use std::env::var;
-    use zkevm_circuits::bytecode_circuit::bytecode_unroller::{unroll, UnrolledBytecode};
-    use zkevm_circuits::bytecode_circuit::dev::BytecodeCircuitTester;
+    use zkevm_circuits::bytecode_circuit::bytecode_unroller::{
+        unroll, BytecodeCircuit, UnrolledBytecode,
+    };
 
     #[cfg_attr(not(feature = "benches"), ignore)]
     #[test]
@@ -40,7 +41,7 @@ mod tests {
         let bytecodes_num: usize = max_bytecode_row_num / bytecode_len;
 
         // Create the circuit
-        let bytecode_circuit = BytecodeCircuitTester::<Fr>::new(
+        let bytecode_circuit = BytecodeCircuit::<Fr>::new(
             fillup_codebytes(bytecodes_num, bytecode_len),
             2usize.pow(degree),
         );
@@ -74,7 +75,7 @@ mod tests {
             Challenge255<G1Affine>,
             XorShiftRng,
             Blake2bWrite<Vec<u8>, G1Affine, Challenge255<G1Affine>>,
-            BytecodeCircuitTester<Fr>,
+            BytecodeCircuit<Fr>,
         >(
             &general_params,
             &pk,

circuit-benchmarks/src/evm_circuit.rs

@@ -1,71 +1,7 @@
 //! Evm circuit benchmarks
 
-use eth_types::Field;
-use halo2_proofs::{
-    circuit::{Layouter, SimpleFloorPlanner},
-    plonk::{Circuit, ConstraintSystem, Error, Expression},
-};
-use zkevm_circuits::evm_circuit::{witness::Block, EvmCircuit};
-use zkevm_circuits::table::{BlockTable, BytecodeTable, RwTable, TxTable};
-
-#[derive(Debug, Default)]
-pub struct TestCircuit<F> {
-    block: Option<Block<F>>,
-}
-
-impl<F: Field> TestCircuit<F> {
-    pub fn new(block: Block<F>) -> Self {
-        TestCircuit { block: Some(block) }
-    }
-}
-
-impl<F: Field> Circuit<F> for TestCircuit<F> {
-    type Config = EvmCircuit<F>;
-    type FloorPlanner = SimpleFloorPlanner;
-
-    fn without_witnesses(&self) -> Self {
-        Self::default()
-    }
-
-    fn configure(meta: &mut ConstraintSystem<F>) -> Self::Config {
-        let tx_table = TxTable::construct(meta);
-        let rw_table = RwTable::construct(meta);
-        let bytecode_table = BytecodeTable::construct(meta);
-        let block_table = BlockTable::construct(meta);
-        let copy_table = [(); 11].map(|_| meta.advice_column());
-        let keccak_table = [(); 4].map(|_| meta.advice_column());
-        let exp_table = [(); 9].map(|_| meta.advice_column());
-        // Use constant expression to mock constant instance column for a more
-        // reasonable benchmark.
-        let power_of_randomness = [(); 31].map(|_| Expression::Constant(F::one()));
-
-        EvmCircuit::configure(
-            meta,
-            power_of_randomness,
-            &tx_table,
-            &rw_table,
-            &bytecode_table,
-            &block_table,
-            &copy_table,
-            &keccak_table,
-            &exp_table,
-        )
-    }
-
-    fn synthesize(
-        &self,
-        config: Self::Config,
-        mut layouter: impl Layouter<F>,
-    ) -> Result<(), Error> {
-        let block = self.block.as_ref().unwrap();
-        config.assign_block(&mut layouter, block)?;
-        Ok(())
-    }
-}
-
 #[cfg(test)]
 mod evm_circ_benches {
-    use super::*;
     use ark_std::{end_timer, start_timer};
     use bus_mapping::{circuit_input_builder::CircuitsParams, mock::BlockData};
     use eth_types::geth_types::GethData;
@@ -84,7 +20,7 @@ mod evm_circ_benches {
     use rand::SeedableRng;
     use rand_xorshift::XorShiftRng;
     use std::env::var;
-    use zkevm_circuits::evm_circuit::witness::block_convert;
+    use zkevm_circuits::evm_circuit::{witness::block_convert, EvmCircuit};
 
     #[cfg_attr(not(feature = "benches"), ignore)]
     #[test]
@@ -108,9 +44,9 @@ mod evm_circ_benches {
             .handle_block(&empty_data.eth_block, &empty_data.geth_traces)
             .unwrap();
 
-        let block = block_convert(&builder.block, &builder.code_db);
+        let block = block_convert(&builder.block, &builder.code_db).unwrap();
 
-        let circuit = TestCircuit::<Fr>::new(block);
+        let circuit = EvmCircuit::<Fr>::new(block);
         let mut rng = XorShiftRng::from_seed([
             0x59, 0x62, 0xbe, 0x5d, 0x76, 0x3d, 0x31, 0x8d, 0x17, 0xdb, 0x37, 0x32, 0x54, 0x06,
             0xbc, 0xe5,
@@ -138,7 +74,7 @@ mod evm_circ_benches {
             Challenge255<G1Affine>,
             XorShiftRng,
             Blake2bWrite<Vec<u8>, G1Affine, Challenge255<G1Affine>>,
-            TestCircuit<Fr>,
+            EvmCircuit<Fr>,
         >(
             &general_params,
             &pk,

circuit-benchmarks/src/packed_multi_keccak.rs

@@ -17,7 +17,7 @@ mod tests {
     use rand::SeedableRng;
     use rand_xorshift::XorShiftRng;
     use std::env::var;
-    use zkevm_circuits::keccak_circuit::keccak_packed_multi::KeccakPackedCircuit;
+    use zkevm_circuits::keccak_circuit::keccak_packed_multi::KeccakCircuit;
 
     #[cfg_attr(not(feature = "benches"), ignore)]
     #[test]
@@ -31,7 +31,7 @@ mod tests {
         let inputs = vec![(0u8..135).collect::<Vec<_>>(); 3];
 
         // Create the circuit. Leave last dozens of rows for blinding.
-        let circuit = KeccakPackedCircuit::new(2usize.pow(degree) - 64, inputs.to_vec());
+        let circuit = KeccakCircuit::new(Some(2usize.pow(degree) - 64), inputs);
 
         // Initialize the polynomial commitment parameters
         let mut rng = XorShiftRng::from_seed([
@@ -64,7 +64,7 @@ mod tests {
             Challenge255<G1Affine>,
             XorShiftRng,
             Blake2bWrite<Vec<u8>, G1Affine, Challenge255<G1Affine>>,
-            KeccakPackedCircuit<Fr>,
+            KeccakCircuit<Fr>,
         >(
             &general_params,
             &pk,

circuit-benchmarks/src/pi_circuit.rs

@@ -19,8 +19,9 @@ mod tests {
     use rand_chacha::ChaCha20Rng;
     use rand_xorshift::XorShiftRng;
     use std::env::var;
-    use zkevm_circuits::pi_circuit::{PiCircuit, PublicData};
+    use zkevm_circuits::pi_circuit::{PiCircuit, PiTestCircuit, PublicData};
     use zkevm_circuits::test_util::rand_tx;
+    use zkevm_circuits::util::SubCircuit;
 
     #[cfg_attr(not(feature = "benches"), ignore)]
     #[test]
@@ -37,9 +38,14 @@ mod tests {
         let randomness = Fr::random(&mut rng);
         let rand_rpi = Fr::random(&mut rng);
         let public_data = generate_publicdata::<MAX_TXS, MAX_CALLDATA>();
-        let circuit =
-            PiCircuit::<Fr, MAX_TXS, MAX_CALLDATA>::new(randomness, rand_rpi, public_data);
-        let public_inputs = circuit.instance();
+        let circuit = PiTestCircuit::<Fr, MAX_TXS, MAX_CALLDATA>(PiCircuit::<Fr>::new(
+            MAX_TXS,
+            MAX_CALLDATA,
+            randomness,
+            rand_rpi,
+            public_data,
+        ));
+        let public_inputs = circuit.0.instance();
         let instance: Vec<&[Fr]> = public_inputs.iter().map(|input| &input[..]).collect();
         let instances = &[&instance[..]][..];
 
@@ -70,7 +76,7 @@ mod tests {
             Challenge255<G1Affine>,
             XorShiftRng,
             Blake2bWrite<Vec<u8>, G1Affine, Challenge255<G1Affine>>,
-            PiCircuit<Fr, MAX_TXS, MAX_CALLDATA>,
+            PiTestCircuit<Fr, MAX_TXS, MAX_CALLDATA>,
         >(
             &general_params,
             &pk,
@@ -114,7 +120,7 @@ mod tests {
         let n_tx = MAX_TXS;
         for _ in 0..n_tx {
             let eth_tx = eth_types::Transaction::from(&rand_tx(&mut rng, chain_id));
-            public_data.eth_block.transactions.push(eth_tx);
+            public_data.transactions.push(eth_tx);
         }
         public_data
     }

circuit-benchmarks/src/state_circuit.rs

@@ -19,6 +19,7 @@ mod tests {
     use std::env::var;
     use zkevm_circuits::evm_circuit::witness::RwMap;
     use zkevm_circuits::state_circuit::StateCircuit;
+    use zkevm_circuits::util::SubCircuit;
 
     #[cfg_attr(not(feature = "benches"), ignore)]
     #[test]

circuit-benchmarks/src/super_circuit.rs

@@ -71,8 +71,7 @@ mod tests {
 
         block.sign(&wallets);
 
-        let (_, circuit, instance, _) =
-            SuperCircuit::<_, 1, 32, 512>::build(block, &mut ChaChaRng::seed_from_u64(2)).unwrap();
+        let (_, circuit, instance, _) = SuperCircuit::<_, 1, 32, 512>::build(block).unwrap();
         let instance_refs: Vec<&[Fr]> = instance.iter().map(|v| &v[..]).collect();
 
         // Bench setup generation