From cdcff9adaf80b7c4fd1ec9c48029965268b63b7e Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Mon, 5 Feb 2024 11:30:48 +0800 Subject: [PATCH 01/16] better naming --- zkevm-circuits/src/witness/chunk.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index 6e8fac5d5e..c954f5cd44 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -113,7 +113,7 @@ pub fn chunk_convert( ); // Compute fingerprints of all chunks - let mut alpha_gamas = Vec::with_capacity(builder.chunks.len()); + let mut challenges = Vec::with_capacity(builder.chunks.len()); let mut rw_fingerprints: Vec> = Vec::with_capacity(builder.chunks.len()); let mut chrono_rw_fingerprints: Vec> = Vec::with_capacity(builder.chunks.len()); @@ -154,7 +154,7 @@ pub fn chunk_convert( true, ); - alpha_gamas.push(vec![alpha, gamma]); + challenges.push(vec![alpha, gamma]); rw_fingerprints.push(cur_fingerprints); chrono_rw_fingerprints.push(cur_chrono_fingerprints); if i == idx { @@ -164,8 +164,8 @@ pub fn chunk_convert( // TODO(Cecilia): if we chunk across blocks then need to store the prev_block let chunck = Chunk { - permu_alpha: alpha_gamas[idx][0], - permu_gamma: alpha_gamas[idx][1], + permu_alpha: challenges[idx][0], + permu_gamma: challenges[idx][1], rw_fingerprints: rw_fingerprints[idx].clone(), chrono_rw_fingerprints: chrono_rw_fingerprints[idx].clone(), begin_chunk: chunk.begin_chunk.clone(), From 031c0fc842902e2360e16bc8ae465d7f25257e67 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Thu, 14 Dec 2023 07:46:04 +0800 Subject: [PATCH 02/16] WIP: rw_table witness commitment --- zkevm-circuits/src/root_circuit.rs | 31 ++- zkevm-circuits/src/super_circuit/test.rs | 71 ++++- zkevm-circuits/src/table/rw_table.rs | 317 ++++++++++++++++++++++- 3 files changed, 409 insertions(+), 10 deletions(-) diff --git a/zkevm-circuits/src/root_circuit.rs b/zkevm-circuits/src/root_circuit.rs index 249ad5b7a7..337a7ddd79 100644 --- a/zkevm-circuits/src/root_circuit.rs +++ b/zkevm-circuits/src/root_circuit.rs @@ -285,7 +285,7 @@ where config.aggregate::(ctx, &key.clone(), &self.snark_witnesses)?; // aggregate user challenge for rwtable permutation challenge - let (alpha, gamma) = { + let (_alpha, _gamma) = { let mut challenges = config.aggregate_user_challenges::( loader.clone(), self.user_challenges, @@ -325,9 +325,24 @@ where .scalar_chip() .assign_constant(&mut loader.ctx_mut(), M::Fr::from(1)) .unwrap(); + (zero_const, one_const, total_chunk_const) }; + // TODO remove me + let (_hardcode_alpha, _hardcode_gamma) = { + ( + loader + .scalar_chip() + .assign_constant(&mut loader.ctx_mut(), M::Scalar::from(101)) + .unwrap(), + loader + .scalar_chip() + .assign_constant(&mut loader.ctx_mut(), M::Scalar::from(103)) + .unwrap(), + ) + }; + // `first.sc_rwtable_row_prev_fingerprint == // first.ec_rwtable_row_prev_fingerprint` will be checked inside circuit vec![ @@ -338,11 +353,17 @@ where (first_chunk.initial_rwc.assigned(), &one_const), // constraint permutation fingerprint // challenge: alpha - (first_chunk.sc_permu_alpha.assigned(), &alpha.assigned()), - (first_chunk.ec_permu_alpha.assigned(), &alpha.assigned()), + // TODO remove hardcode + (first_chunk.sc_permu_alpha.assigned(), &_hardcode_alpha), + (first_chunk.ec_permu_alpha.assigned(), &_hardcode_alpha), + // (first_chunk.sc_permu_alpha.assigned(), &alpha.assigned()), + // (first_chunk.ec_permu_alpha.assigned(), &alpha.assigned()), // challenge: gamma - (first_chunk.sc_permu_gamma.assigned(), &gamma.assigned()), - (first_chunk.ec_permu_gamma.assigned(), &gamma.assigned()), + // TODO remove hardcode + (first_chunk.sc_permu_gamma.assigned(), &_hardcode_gamma), + (first_chunk.ec_permu_gamma.assigned(), &_hardcode_gamma), + // (first_chunk.sc_permu_gamma.assigned(), &gamma.assigned()), + // (first_chunk.ec_permu_gamma.assigned(), &gamma.assigned()), // fingerprint ( first_chunk.ec_rwtable_prev_fingerprint.assigned(), diff --git a/zkevm-circuits/src/super_circuit/test.rs b/zkevm-circuits/src/super_circuit/test.rs index 19ce9ce797..090d09a8bb 100644 --- a/zkevm-circuits/src/super_circuit/test.rs +++ b/zkevm-circuits/src/super_circuit/test.rs @@ -1,14 +1,26 @@ +use crate::{table::rw_table::get_rwtable_cols_commitment, witness::RwMap}; + pub use super::*; +use bus_mapping::operation::OperationContainer; +use eth_types::{address, bytecode, geth_types::GethData, Word}; use ethers_signers::{LocalWallet, Signer}; -use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr}; +use halo2_proofs::{ + dev::MockProver, + halo2curves::{ + bn256::{Bn256, Fr}, + ff::WithSmallOrderMulGroup, + }, + poly::{ + commitment::CommitmentScheme, + kzg::commitment::{KZGCommitmentScheme, ParamsKZG}, + }, +}; use log::error; use mock::{TestContext, MOCK_CHAIN_ID}; use rand::SeedableRng; -use rand_chacha::ChaCha20Rng; +use rand_chacha::{rand_core::OsRng, ChaCha20Rng}; use std::collections::HashMap; -use eth_types::{address, bytecode, geth_types::GethData, Word}; - #[test] fn super_circuit_degree() { let mut cs = ConstraintSystem::::default(); @@ -180,3 +192,54 @@ fn serial_test_super_circuit_2tx_2max_tx() { }; test_super_circuit(block, circuits_params, Fr::from(TEST_MOCK_RANDOMNESS)); } + +#[ignore] +#[test] +fn test_rw_table_commitment() { + let k = 18; + let params = ParamsKZG::::setup(k, OsRng); + rw_table_commitment::>(¶ms); +} + +fn rw_table_commitment<'params, Scheme: CommitmentScheme>(params: &'params Scheme::ParamsProver) +where + ::Scalar: WithSmallOrderMulGroup<3> + eth_types::Field, +{ + let circuits_params = FixedCParams { + max_txs: 1, + max_calldata: 32, + max_rws: 256, + max_copy_rows: 256, + max_exp_steps: 256, + max_bytecode: 512, + max_evm_rows: 0, + max_keccak_rows: 0, + }; + let rw_map = RwMap::from(&OperationContainer { + ..Default::default() + }); + let rows = rw_map.table_assignments(false); + + const TEST_MOCK_RANDOMNESS: u64 = 0x100; + + // synthesize to get degree + let mut cs = ConstraintSystem::<::Scalar>::default(); + let config = SuperCircuit::configure_with_params( + &mut cs, + SuperCircuitParams { + max_txs: circuits_params.max_txs, + max_calldata: circuits_params.max_calldata, + mock_randomness: TEST_MOCK_RANDOMNESS.into(), + }, + ); + let degree = cs.degree(); + + let advice_commitments = get_rwtable_cols_commitment::( + degree, + &rows, + circuits_params.max_rws, + params, + false, + ); + println!("advice_commitments {:?}", advice_commitments[0]); +} diff --git a/zkevm-circuits/src/table/rw_table.rs b/zkevm-circuits/src/table/rw_table.rs index b233b2c559..9a638570b5 100644 --- a/zkevm-circuits/src/table/rw_table.rs +++ b/zkevm-circuits/src/table/rw_table.rs @@ -1,4 +1,19 @@ -use halo2_proofs::circuit::AssignedCell; +use bus_mapping::operation::OperationContainer; +use halo2_proofs::{ + self, + circuit::{AssignedCell, SimpleFloorPlanner}, + halo2curves::ff::{BatchInvert, WithSmallOrderMulGroup}, +}; + +use halo2_proofs::{ + halo2curves::{bn256::Fr, group::Curve, CurveAffine}, + plonk::{Advice, Assigned, Assignment, Challenge, Fixed, FloorPlanner, Instance, Selector}, + poly::{ + commitment::{Blind, CommitmentScheme, Params}, + EvaluationDomain, LagrangeCoeff, Polynomial, + }, +}; +use snark_verifier::util::arithmetic::PrimeCurveAffine; use super::*; @@ -72,6 +87,20 @@ impl RwTable { /// Construct a new RwTable pub fn construct(meta: &mut ConstraintSystem) -> Self { Self { + // TODO upgrade halo2 to use `unblinded_advice_column` + // https://github.com/privacy-scaling-explorations/halo2/blob/main/halo2_proofs/examples/vector-ops-unblinded.rs + // rw_counter: meta.unblinded_advice_column(), + // is_write: meta.unblinded_advice_column(), + // tag: meta.unblinded_advice_column(), + // id: meta.unblinded_advice_column(), + // address: meta.unblinded_advice_column(), + // field_tag: meta.unblinded_advice_column(), + // storage_key: word::Word::new([meta.unblinded_advice_column(), + // meta.unblinded_advice_column()]), value: word::Word::new([meta. + // unblinded_advice_column(), meta.unblinded_advice_column()]), value_prev: + // word::Word::new([meta.unblinded_advice_column(), meta.unblinded_advice_column()]), + // init_val: word::Word::new([meta.unblinded_advice_column(), + // meta.unblinded_advice_column()]), rw_counter: meta.advice_column(), is_write: meta.advice_column(), tag: meta.advice_column(), @@ -155,3 +184,289 @@ impl RwTable { Ok(rows) } } + +/// get rw table column commitment +/// implementation snippet from halo2 `create_proof` https://github.com/privacy-scaling-explorations/halo2/blob/9b33f9ce524dbb9133fc8b9638b2afd0571659a8/halo2_proofs/src/plonk/prover.rs#L37 +pub fn get_rwtable_cols_commitment<'params, Scheme: CommitmentScheme>( + degree: usize, + rws: &[Rw], + n_rows: usize, + params_prover: &'params Scheme::ParamsProver, + is_first_row_padding: bool, +) -> Vec<::Curve> +where + ::Scalar: WithSmallOrderMulGroup<3> + Field, +{ + struct WitnessCollection { + k: u32, + advice: Vec, LagrangeCoeff>>, + _marker: std::marker::PhantomData, + } + + impl<'a, F: Field> Assignment for WitnessCollection { + fn enter_region(&mut self, _: N) + where + NR: Into, + N: FnOnce() -> NR, + { + // Do nothing; we don't care about regions in this context. + } + + fn exit_region(&mut self) { + // Do nothing; we don't care about regions in this context. + } + + fn enable_selector(&mut self, _: A, _: &Selector, _: usize) -> Result<(), Error> + where + A: FnOnce() -> AR, + AR: Into, + { + // We only care about advice columns here + + Ok(()) + } + + fn annotate_column(&mut self, _annotation: A, _column: Column) + where + A: FnOnce() -> AR, + AR: Into, + { + // Do nothing + } + + fn query_instance(&self, column: Column, row: usize) -> Result, Error> { + Err(Error::BoundsFailure) + } + + fn assign_advice( + &mut self, + _: A, + column: Column, + row: usize, + to: V, + ) -> Result<(), Error> + where + V: FnOnce() -> Value, + VR: Into>, + A: FnOnce() -> AR, + AR: Into, + { + to().into_field().map(|v| { + *self + .advice + .get_mut(column.index()) + .and_then(|v| v.get_mut(row)) + .ok_or(Error::BoundsFailure) + .unwrap() = v; + }); + Ok(()) + } + + fn assign_fixed( + &mut self, + _: A, + _: Column, + _: usize, + _: V, + ) -> Result<(), Error> + where + V: FnOnce() -> Value, + VR: Into>, + A: FnOnce() -> AR, + AR: Into, + { + // We only care about advice columns here + + Ok(()) + } + + fn copy( + &mut self, + _: Column, + _: usize, + _: Column, + _: usize, + ) -> Result<(), Error> { + // We only care about advice columns here + + Ok(()) + } + + fn fill_from_row( + &mut self, + _: Column, + _: usize, + _: Value>, + ) -> Result<(), Error> { + Ok(()) + } + + fn get_challenge(&self, challenge: Challenge) -> Value { + Value::unknown() + } + + fn push_namespace(&mut self, _: N) + where + NR: Into, + N: FnOnce() -> NR, + { + // Do nothing; we don't care about namespaces in this context. + } + + fn pop_namespace(&mut self, _: Option) { + // Do nothing; we don't care about namespaces in this context. + } + } + + let rw_map = RwMap::from(&OperationContainer { + ..Default::default() + }); + let rows = rw_map.table_assignments(false); + let rwtable_circuit = RwTableCircuit::new(&rows, 1, false); + + let domain = EvaluationDomain::<::Scalar>::new( + degree as u32, + params_prover.k(), + ); + + let mut cs = ConstraintSystem::default(); + let rwtable_circuit_config = RwTableCircuit::configure(&mut cs); + // TODO adjust domain.empty_lagrange_assigned() visibility in halo2 library to public + let mut witness = WitnessCollection { + k: params_prover.k(), + advice: vec![ + domain.empty_lagrange_assigned(); + rwtable_circuit_config.rw_table.advice_columns().len() + ], + _marker: std::marker::PhantomData, + }; + + // Synthesize the circuit to obtain the witness and other information. + as Circuit>::FloorPlanner::synthesize( + &mut witness, + &rwtable_circuit, + rwtable_circuit_config, + cs.constants().clone(), + ) + .unwrap(); + + let mut advice_values = + batch_invert_assigned::(domain, witness.advice.into_iter().collect()); + + // Compute commitments to advice column polynomials + let blinds = vec![Blind::default(); witness.advice.len()]; + let advice_commitments_projective: Vec<_> = advice_values + .iter() + .zip(blinds.iter()) + .map(|(poly, blind)| params_prover.commit_lagrange(poly, *blind)) + .collect(); + let mut advice_commitments = + vec![Scheme::Curve::identity(); advice_commitments_projective.len()]; + + ::CurveExt::batch_normalize( + &advice_commitments_projective, + &mut advice_commitments, + ); + + advice_commitments +} + +struct RwTableCircuit<'a> { + rws: &'a [Rw], + n_rows: usize, + is_first_row_padding: bool, +} + +impl<'a> RwTableCircuit<'a> { + #[allow(dead_code)] + pub(crate) fn new(rws: &'a [Rw], n_rows: usize, is_first_row_padding: bool) -> Self { + Self { + rws, + n_rows, + is_first_row_padding, + } + } +} + +#[derive(Clone)] +struct RwTableCircuitConfig { + pub rw_table: RwTable, +} + +impl<'a> RwTableCircuitConfig {} + +impl<'a, F: Field> Circuit for RwTableCircuit<'a> { + type Config = RwTableCircuitConfig; + + type FloorPlanner = SimpleFloorPlanner; + + type Params = (); + + fn without_witnesses(&self) -> Self { + todo!() + } + + fn configure(meta: &mut ConstraintSystem) -> Self::Config { + RwTableCircuitConfig { + rw_table: RwTable::construct(meta), + } + } + + fn synthesize( + &self, + config: Self::Config, + mut layouter: impl Layouter, + ) -> Result<(), Error> { + layouter.assign_region( + || "XXXX", + |mut region| { + config.rw_table.load_with_region( + &mut region, + self.rws, + self.n_rows, + self.is_first_row_padding, + ) + }, + )?; + Ok(()) + } +} + +// migrate from halo2 library +fn batch_invert_assigned>( + domain: EvaluationDomain, + assigned: Vec, LagrangeCoeff>>, +) -> Vec> { + let mut assigned_denominators: Vec<_> = assigned + .iter() + .map(|f| { + f.iter() + .map(|value| value.denominator()) + .collect::>() + }) + .collect(); + + assigned_denominators + .iter_mut() + .flat_map(|f| { + f.iter_mut() + // If the denominator is trivial, we can skip it, reducing the + // size of the batch inversion. + .filter_map(|d| d.as_mut()) + }) + .batch_invert(); + + assigned + .iter() + .zip(assigned_denominators.into_iter()) + .map(|(poly, inv_denoms)| { + let inv_denoms = inv_denoms.into_iter().map(|d| d.unwrap_or(F::ONE)); + domain.lagrange_from_vec( + poly.iter() + .zip(inv_denoms.into_iter()) + .map(|(a, inv_den)| a.numerator() * inv_den) + .collect(), + ) + }) + .collect() +} From b98a4cb01eb7186742a50ab976304ba4d40ab392 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Mon, 5 Feb 2024 17:58:07 +0800 Subject: [PATCH 03/16] fix chunk first row padding logic --- zkevm-circuits/src/root_circuit.rs | 4 +-- zkevm-circuits/src/super_circuit/test.rs | 17 +++++------ zkevm-circuits/src/table/rw_table.rs | 39 ++++++++++++------------ zkevm-circuits/src/witness/chunk.rs | 3 +- zkevm-circuits/src/witness/rw.rs | 19 +++--------- 5 files changed, 36 insertions(+), 46 deletions(-) diff --git a/zkevm-circuits/src/root_circuit.rs b/zkevm-circuits/src/root_circuit.rs index 337a7ddd79..18e254dc63 100644 --- a/zkevm-circuits/src/root_circuit.rs +++ b/zkevm-circuits/src/root_circuit.rs @@ -334,11 +334,11 @@ where ( loader .scalar_chip() - .assign_constant(&mut loader.ctx_mut(), M::Scalar::from(101)) + .assign_constant(&mut loader.ctx_mut(), M::Fr::from(101)) .unwrap(), loader .scalar_chip() - .assign_constant(&mut loader.ctx_mut(), M::Scalar::from(103)) + .assign_constant(&mut loader.ctx_mut(), M::Fr::from(103)) .unwrap(), ) }; diff --git a/zkevm-circuits/src/super_circuit/test.rs b/zkevm-circuits/src/super_circuit/test.rs index 090d09a8bb..2ff959e47a 100644 --- a/zkevm-circuits/src/super_circuit/test.rs +++ b/zkevm-circuits/src/super_circuit/test.rs @@ -207,6 +207,7 @@ where { let circuits_params = FixedCParams { max_txs: 1, + max_withdrawals: 5, max_calldata: 32, max_rws: 256, max_copy_rows: 256, @@ -214,6 +215,7 @@ where max_bytecode: 512, max_evm_rows: 0, max_keccak_rows: 0, + total_chunks: 1, }; let rw_map = RwMap::from(&OperationContainer { ..Default::default() @@ -224,22 +226,19 @@ where // synthesize to get degree let mut cs = ConstraintSystem::<::Scalar>::default(); - let config = SuperCircuit::configure_with_params( + let _config = SuperCircuit::configure_with_params( &mut cs, SuperCircuitParams { max_txs: circuits_params.max_txs, + max_withdrawals: circuits_params.max_withdrawals, max_calldata: circuits_params.max_calldata, mock_randomness: TEST_MOCK_RANDOMNESS.into(), + feature_config: FeatureConfig::default(), }, ); let degree = cs.degree(); - let advice_commitments = get_rwtable_cols_commitment::( - degree, - &rows, - circuits_params.max_rws, - params, - false, - ); - println!("advice_commitments {:?}", advice_commitments[0]); + let advice_commitments = + get_rwtable_cols_commitment::(degree, &rows, circuits_params.max_rws, params); + println!("advice_commitments len() {:?}", advice_commitments.len()); } diff --git a/zkevm-circuits/src/table/rw_table.rs b/zkevm-circuits/src/table/rw_table.rs index 9a638570b5..5535d8e42e 100644 --- a/zkevm-circuits/src/table/rw_table.rs +++ b/zkevm-circuits/src/table/rw_table.rs @@ -1,4 +1,3 @@ -use bus_mapping::operation::OperationContainer; use halo2_proofs::{ self, circuit::{AssignedCell, SimpleFloorPlanner}, @@ -192,13 +191,11 @@ pub fn get_rwtable_cols_commitment<'params, Scheme: CommitmentScheme>( rws: &[Rw], n_rows: usize, params_prover: &'params Scheme::ParamsProver, - is_first_row_padding: bool, ) -> Vec<::Curve> where ::Scalar: WithSmallOrderMulGroup<3> + Field, { struct WitnessCollection { - k: u32, advice: Vec, LagrangeCoeff>>, _marker: std::marker::PhantomData, } @@ -234,7 +231,11 @@ where // Do nothing } - fn query_instance(&self, column: Column, row: usize) -> Result, Error> { + fn query_instance( + &self, + _column: Column, + _row: usize, + ) -> Result, Error> { Err(Error::BoundsFailure) } @@ -301,7 +302,7 @@ where Ok(()) } - fn get_challenge(&self, challenge: Challenge) -> Value { + fn get_challenge(&self, _challenge: Challenge) -> Value { Value::unknown() } @@ -318,25 +319,22 @@ where } } - let rw_map = RwMap::from(&OperationContainer { - ..Default::default() - }); - let rows = rw_map.table_assignments(false); - let rwtable_circuit = RwTableCircuit::new(&rows, 1, false); + let rwtable_circuit = RwTableCircuit::new(&rws, n_rows, None); let domain = EvaluationDomain::<::Scalar>::new( degree as u32, params_prover.k(), ); - let mut cs = ConstraintSystem::default(); + let mut cs = ConstraintSystem::<::Scalar>::default(); let rwtable_circuit_config = RwTableCircuit::configure(&mut cs); - // TODO adjust domain.empty_lagrange_assigned() visibility in halo2 library to public let mut witness = WitnessCollection { - k: params_prover.k(), advice: vec![ domain.empty_lagrange_assigned(); - rwtable_circuit_config.rw_table.advice_columns().len() + ::Scalar>>::advice_columns( + &rwtable_circuit_config.rw_table + ) + .len() ], _marker: std::marker::PhantomData, }; @@ -350,11 +348,12 @@ where ) .unwrap(); - let mut advice_values = + let len = witness.advice.len(); + let advice_values = batch_invert_assigned::(domain, witness.advice.into_iter().collect()); // Compute commitments to advice column polynomials - let blinds = vec![Blind::default(); witness.advice.len()]; + let blinds = vec![Blind::default(); len]; let advice_commitments_projective: Vec<_> = advice_values .iter() .zip(blinds.iter()) @@ -374,16 +373,16 @@ where struct RwTableCircuit<'a> { rws: &'a [Rw], n_rows: usize, - is_first_row_padding: bool, + prev_chunk_last_rw: Option, } impl<'a> RwTableCircuit<'a> { #[allow(dead_code)] - pub(crate) fn new(rws: &'a [Rw], n_rows: usize, is_first_row_padding: bool) -> Self { + pub(crate) fn new(rws: &'a [Rw], n_rows: usize, prev_chunk_last_rw: Option) -> Self { Self { rws, n_rows, - is_first_row_padding, + prev_chunk_last_rw, } } } @@ -424,7 +423,7 @@ impl<'a, F: Field> Circuit for RwTableCircuit<'a> { &mut region, self.rws, self.n_rows, - self.is_first_row_padding, + self.prev_chunk_last_rw, ) }, )?; diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index c954f5cd44..b71c9cdf50 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -103,7 +103,8 @@ pub fn chunk_convert( let chunk = builder.get_chunk(idx); let mut rws = RwMap::default(); let prev_chunk_last_rw = builder.prev_chunk().map(|chunk| { - RwMap::get_rw(&block.container, chunk.ctx.end_rwc).expect("Rw does not exist") + // rws range [chunk.ctx.initial_rwc, chunk.ctx.end_rwc) + RwMap::get_rw(&block.container, chunk.ctx.end_rwc - 1).expect("Rw does not exist") }); // FIXME(Cecilia): debug diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index a03bf64a8c..741258ebd0 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -143,11 +143,7 @@ impl RwMap { .collect(); let padding_length = { let length = Self::padding_len(rows_trimmed.len(), target_len); - if prev_chunk_last_rw.is_none() { - length.saturating_sub(1) - } else { - length - } + length.saturating_sub(1) }; // option 1: need to provide padding starting rw_counter at function parameters @@ -436,7 +432,7 @@ impl RwRow> { pub fn padding( rows: &[RwRow>], target_len: usize, - is_first_row_padding: bool, + padding_start_rwrow: Option>>, ) -> (Vec>>, usize) { // Remove Start/Padding rows as we will add them from scratch. let rows_trimmed = rows @@ -450,11 +446,7 @@ impl RwRow> { .collect::>>>(); let padding_length = { let length = RwMap::padding_len(rows_trimmed.len(), target_len); - if is_first_row_padding { - length.saturating_sub(1) - } else { - length - } + length.saturating_sub(1) // first row always got padding }; let start_padding_rw_counter = { let start_padding_rw_counter = rows_trimmed @@ -487,12 +479,11 @@ impl RwRow> { }, ); ( - iter::once(RwRow { + iter::once(padding_start_rwrow.unwrap_or(RwRow { rw_counter: Value::known(F::ONE), tag: Value::known(F::from(Target::Start as u64)), ..Default::default() - }) - .take(if is_first_row_padding { 1 } else { 0 }) + })) .chain(rows_trimmed.into_iter()) .chain(padding.into_iter()) .collect(), From f714c7e4f7bd76917a18aeb3e015fe2185ecb430 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Mon, 5 Feb 2024 18:02:37 +0800 Subject: [PATCH 04/16] cleanup unused code --- zkevm-circuits/src/evm_circuit.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index fec2936dea..189ad8d7e3 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -390,12 +390,8 @@ impl SubCircuit for EvmCircuit { /// Compute the public inputs for this circuit. fn instance(&self) -> Vec> { - let _block = self.block.as_ref().unwrap(); let chunk = self.chunk.as_ref().unwrap(); - let (_rw_table_chunked_index, _rw_table_total_chunks) = - (chunk.chunk_context.idx, chunk.chunk_context.total_chunks); - vec![vec![ chunk.permu_alpha, chunk.permu_gamma, From 31bf03054718178668f6c71fe029f8814e8f5bc1 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Mon, 5 Feb 2024 20:11:02 +0800 Subject: [PATCH 05/16] fix various logic error in index and padding --- bus-mapping/src/circuit_input_builder.rs | 11 +++--- .../src/circuit_input_builder/chunk.rs | 36 +++++++++---------- zkevm-circuits/src/copy_circuit.rs | 2 +- zkevm-circuits/src/copy_circuit/test.rs | 2 +- zkevm-circuits/src/evm_circuit/execution.rs | 2 +- zkevm-circuits/src/state_circuit/test.rs | 32 ++++++++++------- zkevm-circuits/src/witness/chunk.rs | 24 ++++++++++--- zkevm-circuits/src/witness/rw.rs | 4 +-- 8 files changed, 68 insertions(+), 45 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 512f580172..a699c56393 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -556,16 +556,17 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { pub fn commit_chunk( &mut self, to_next: bool, - end_tx: usize, - end_copy: usize, + next_tx_index: usize, + next_copy_index: usize, last_call: Option, ) { self.chunk_ctx.end_rwc = self.block_ctx.rwc.0; - self.chunk_ctx.end_tx = end_tx; - self.chunk_ctx.end_copy = end_copy; + self.chunk_ctx.end_tx_index = next_tx_index; + self.chunk_ctx.end_copy_index = next_copy_index; self.chunks[self.chunk_ctx.idx].ctx = self.chunk_ctx.clone(); if to_next { - self.chunk_ctx.bump(self.block_ctx.rwc.0, end_tx, end_copy); + self.chunk_ctx + .bump(self.block_ctx.rwc.0, next_tx_index, next_copy_index); self.cur_chunk_mut().prev_last_call = last_call; } } diff --git a/bus-mapping/src/circuit_input_builder/chunk.rs b/bus-mapping/src/circuit_input_builder/chunk.rs index 2558eded47..17859e9447 100644 --- a/bus-mapping/src/circuit_input_builder/chunk.rs +++ b/bus-mapping/src/circuit_input_builder/chunk.rs @@ -33,14 +33,14 @@ pub struct ChunkContext { pub initial_rwc: usize, /// End global rw counter pub end_rwc: usize, + /// tx range in block: [initial_tx_index, end_tx_index) + pub initial_tx_index: usize, /// - pub initial_tx: usize, + pub end_tx_index: usize, + /// copy range in block: [initial_copy_index, end_copy_index) + pub initial_copy_index: usize, /// - pub end_tx: usize, - /// - pub initial_copy: usize, - /// - pub end_copy: usize, + pub end_copy_index: usize, /// Druing dry run, chuncking is desabled pub enable: bool, } @@ -60,10 +60,10 @@ impl ChunkContext { total_chunks, initial_rwc: 1, // rw counter start from 1 end_rwc: 0, // end_rwc should be set in later phase - initial_tx: 1, - end_tx: 0, - initial_copy: 0, - end_copy: 0, + initial_tx_index: 0, + end_tx_index: 0, + initial_copy_index: 0, + end_copy_index: 0, enable: true, } } @@ -76,10 +76,10 @@ impl ChunkContext { total_chunks: 1, initial_rwc: 1, // rw counter start from 1 end_rwc: 0, // end_rwc should be set in later phase - initial_tx: 1, - end_tx: 0, - initial_copy: 0, - end_copy: 0, + initial_tx_index: 0, + end_tx_index: 0, + initial_copy_index: 0, + end_copy_index: 0, enable: true, } } @@ -91,11 +91,11 @@ impl ChunkContext { self.idx += 1; self.rwc = RWCounter::new(); self.initial_rwc = initial_rwc; - self.initial_tx = initial_tx; - self.initial_copy = initial_copy; + self.initial_tx_index = initial_tx; + self.initial_copy_index = initial_copy; self.end_rwc = 0; - self.end_tx = 0; - self.end_copy = 0; + self.end_tx_index = 0; + self.end_copy_index = 0; } /// Is first chunk diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 7a75109197..178202d150 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -849,7 +849,7 @@ impl SubCircuit for CopyCircuit { fn new_from_block(block: &witness::Block, chunk: &Chunk) -> Self { let chunked_copy_events = block .copy_events - .get(chunk.chunk_context.initial_copy..chunk.chunk_context.end_copy) + .get(chunk.chunk_context.initial_copy_index..chunk.chunk_context.end_copy_index) .unwrap_or_default(); Self::new_with_external_data( chunked_copy_events.to_owned(), diff --git a/zkevm-circuits/src/copy_circuit/test.rs b/zkevm-circuits/src/copy_circuit/test.rs index 4e4257fc43..d392aa0137 100644 --- a/zkevm-circuits/src/copy_circuit/test.rs +++ b/zkevm-circuits/src/copy_circuit/test.rs @@ -47,7 +47,7 @@ pub fn test_copy_circuit_from_block( ) -> Result<(), Vec> { let chunked_copy_events = block .copy_events - .get(chunk.chunk_context.initial_copy..chunk.chunk_context.end_copy) + .get(chunk.chunk_context.initial_copy_index..chunk.chunk_context.end_copy_index) .unwrap_or_default(); test_copy_circuit::( k, diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index f9648b0466..fa80d03e2e 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -1127,7 +1127,7 @@ impl ExecutionConfig { let dummy_tx = Transaction::default(); let chunk_txs = block .txs - .get(chunk.chunk_context.initial_tx - 1..chunk.chunk_context.end_tx) + .get(chunk.chunk_context.initial_tx_index..chunk.chunk_context.end_tx_index) .unwrap_or_default(); let last_call = chunk_txs diff --git a/zkevm-circuits/src/state_circuit/test.rs b/zkevm-circuits/src/state_circuit/test.rs index f61d6a78f4..ffe06afabe 100644 --- a/zkevm-circuits/src/state_circuit/test.rs +++ b/zkevm-circuits/src/state_circuit/test.rs @@ -45,7 +45,7 @@ fn test_state_circuit_ok( storage: storage_ops, ..Default::default() }); - let chunk = Chunk::new_from_rw_map(&rw_map); + let chunk = Chunk::new_from_rw_map(&rw_map, None, None); let circuit = StateCircuit::::new(&chunk); let instance = circuit.instance(); @@ -69,15 +69,19 @@ fn verifying_key_independent_of_rw_length() { let no_rows = StateCircuit::::new(&chunk); - chunk = Chunk::new_from_rw_map(&RwMap::from(&OperationContainer { - memory: vec![Operation::new( - RWCounter::from(1), - RWCounter::from(1), - RW::WRITE, - MemoryOp::new(1, MemoryAddress::from(0), 32), - )], - ..Default::default() - })); + chunk = Chunk::new_from_rw_map( + &RwMap::from(&OperationContainer { + memory: vec![Operation::new( + RWCounter::from(1), + RWCounter::from(1), + RW::WRITE, + MemoryOp::new(1, MemoryAddress::from(0), 32), + )], + ..Default::default() + }), + None, + None, + ); let one_row = StateCircuit::::new(&chunk); let vk_no_rows = keygen_vk(¶ms, &no_rows).unwrap(); @@ -944,7 +948,11 @@ fn variadic_size_check() { }, ]; // let rw_map: RwMap = rows.clone().into(); - let circuit = StateCircuit::new(&Chunk::new_from_rw_map(&RwMap::from(rows.clone()))); + let circuit = StateCircuit::new(&Chunk::new_from_rw_map( + &RwMap::from(rows.clone()), + None, + None, + )); let power_of_randomness = circuit.instance(); let prover1 = MockProver::::run(17, &circuit, power_of_randomness).unwrap(); @@ -965,7 +973,7 @@ fn variadic_size_check() { }, ]); - let circuit = StateCircuit::new(&Chunk::new_from_rw_map(&rows.into())); + let circuit = StateCircuit::new(&Chunk::new_from_rw_map(&rows.into(), None, None)); let power_of_randomness = circuit.instance(); let prover2 = MockProver::::run(17, &circuit, power_of_randomness).unwrap(); diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index b71c9cdf50..f92c0b0abc 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -68,16 +68,21 @@ impl Default for Chunk { impl Chunk { #[allow(dead_code)] - pub(crate) fn new_from_rw_map(rws: &RwMap) -> Self { + pub(crate) fn new_from_rw_map( + rws: &RwMap, + padding_start_rw: Option, + chrono_padding_start_rw: Option, + ) -> Self { let (alpha, gamma) = get_permutation_randomness(); let mut chunk = Chunk::default(); let rw_fingerprints = get_permutation_fingerprint_of_rwmap( rws, chunk.fixed_param.max_rws, - alpha, // TODO + alpha, gamma, F::from(1), false, + padding_start_rw, ); let chrono_rw_fingerprints = get_permutation_fingerprint_of_rwmap( rws, @@ -86,6 +91,7 @@ impl Chunk { gamma, F::from(1), true, + chrono_padding_start_rw, ); chunk.rws = rws.clone(); chunk.rw_fingerprints = rw_fingerprints; @@ -121,6 +127,9 @@ pub fn chunk_convert( for (i, chunk) in builder.chunks.iter().enumerate() { // Get the Rws in the i-th chunk + + // TODO this is just chronological rws, not by address sorted rws + // need another sorted method let cur_rws = RwMap::from_chunked(&block.container, chunk.ctx.initial_rwc, chunk.ctx.end_rwc); cur_rws.check_value(); @@ -164,7 +173,7 @@ pub fn chunk_convert( } // TODO(Cecilia): if we chunk across blocks then need to store the prev_block - let chunck = Chunk { + let chunk = Chunk { permu_alpha: challenges[idx][0], permu_gamma: challenges[idx][1], rw_fingerprints: rw_fingerprints[idx].clone(), @@ -179,7 +188,7 @@ pub fn chunk_convert( prev_chunk_last_rw, }; - Ok(chunck) + Ok(chunk) } /// @@ -219,6 +228,7 @@ pub fn get_permutation_fingerprint_of_rwmap( gamma: F, prev_continuous_fingerprint: F, is_chrono: bool, + padding_start_rw: Option, ) -> RwFingerprints { get_permutation_fingerprint_of_rwvec( &rwmap.table_assignments(is_chrono), @@ -226,6 +236,7 @@ pub fn get_permutation_fingerprint_of_rwmap( alpha, gamma, prev_continuous_fingerprint, + padding_start_rw, ) } @@ -236,6 +247,7 @@ pub fn get_permutation_fingerprint_of_rwvec( alpha: F, gamma: F, prev_continuous_fingerprint: F, + padding_start_rw: Option, ) -> RwFingerprints { get_permutation_fingerprint_of_rwrowvec( &rwvec @@ -246,6 +258,7 @@ pub fn get_permutation_fingerprint_of_rwvec( alpha, gamma, prev_continuous_fingerprint, + padding_start_rw.map(|r| r.table_assignment()), ) } @@ -256,8 +269,9 @@ pub fn get_permutation_fingerprint_of_rwrowvec( alpha: F, gamma: F, prev_continuous_fingerprint: F, + padding_start_rwrow: Option>>, ) -> RwFingerprints { - let (rows, _) = RwRow::padding(rwrowvec, max_row, true); + let (rows, _) = RwRow::padding(rwrowvec, max_row, padding_start_rwrow); let x = rows.to2dvec(); let fingerprints = get_permutation_fingerprints( &x, diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index 741258ebd0..2076e56a26 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -133,7 +133,7 @@ impl RwMap { pub fn table_assignments_padding( rows: &[Rw], target_len: usize, - prev_chunk_last_rw: Option, + padding_start_rw: Option, ) -> (Vec, usize) { // Remove Start/Padding rows as we will add them from scratch. let rows_trimmed: Vec = rows @@ -162,7 +162,7 @@ impl RwMap { .map(|rw_counter| Rw::Padding { rw_counter }); ( iter::empty() - .chain([prev_chunk_last_rw.unwrap_or(Rw::Start { rw_counter: 1 })]) + .chain([padding_start_rw.unwrap_or(Rw::Start { rw_counter: 1 })]) .chain(rows_trimmed.into_iter()) .chain(padding.into_iter()) .collect(), From ab43142a7207cba3d163410b777c1d14b25f6b45 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Wed, 21 Feb 2024 15:45:39 +0800 Subject: [PATCH 06/16] rewrite logic to support both chronological/by address rw_table padding --- .../src/circuit_input_builder/chunk.rs | 3 +- circuit-benchmarks/src/copy_circuit.rs | 2 +- circuit-benchmarks/src/evm_circuit.rs | 2 +- circuit-benchmarks/src/exp_circuit.rs | 7 +- .../src/integration_test_circuits.rs | 11 +- testool/src/statetest/executor.rs | 17 +- zkevm-circuits/src/copy_circuit.rs | 4 +- zkevm-circuits/src/copy_circuit/test.rs | 24 +-- zkevm-circuits/src/evm_circuit.rs | 16 +- .../src/evm_circuit/execution/end_chunk.rs | 5 +- zkevm-circuits/src/exp_circuit/test.rs | 8 +- zkevm-circuits/src/pi_circuit/test.rs | 4 +- zkevm-circuits/src/state_circuit.rs | 8 +- zkevm-circuits/src/state_circuit/test.rs | 2 +- zkevm-circuits/src/super_circuit.rs | 2 +- zkevm-circuits/src/super_circuit/test.rs | 2 +- zkevm-circuits/src/table/rw_table.rs | 19 +- zkevm-circuits/src/test_util.rs | 17 +- zkevm-circuits/src/witness/block.rs | 24 +++ zkevm-circuits/src/witness/chunk.rs | 196 ++++++++++++------ zkevm-circuits/src/witness/rw.rs | 14 +- zkevm-circuits/tests/prover_error.rs | 4 +- 22 files changed, 238 insertions(+), 153 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/chunk.rs b/bus-mapping/src/circuit_input_builder/chunk.rs index 17859e9447..6fbb9f4db1 100644 --- a/bus-mapping/src/circuit_input_builder/chunk.rs +++ b/bus-mapping/src/circuit_input_builder/chunk.rs @@ -25,6 +25,7 @@ pub struct ChunkContext { /// Index of current chunk, start from 0 pub idx: usize, /// Used to track the inner chunk counter in every operation in the chunk. + /// it will be reset for every new chunk. /// Contains the next available value. pub rwc: RWCounter, /// Number of chunks @@ -41,7 +42,7 @@ pub struct ChunkContext { pub initial_copy_index: usize, /// pub end_copy_index: usize, - /// Druing dry run, chuncking is desabled + /// Druing dry run, chuncking is disabled pub enable: bool, } diff --git a/circuit-benchmarks/src/copy_circuit.rs b/circuit-benchmarks/src/copy_circuit.rs index e35464ed7f..8638b1a1e1 100644 --- a/circuit-benchmarks/src/copy_circuit.rs +++ b/circuit-benchmarks/src/copy_circuit.rs @@ -156,7 +156,7 @@ mod tests { .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); let block = block_convert(&builder).unwrap(); - let chunk = chunk_convert(&builder, 0).unwrap(); + let chunk = chunk_convert(&block, &builder).unwrap().remove(0); assert_eq!(block.copy_events.len(), copy_event_num); (block, chunk) } diff --git a/circuit-benchmarks/src/evm_circuit.rs b/circuit-benchmarks/src/evm_circuit.rs index 44e42c0aa7..10af1a3e10 100644 --- a/circuit-benchmarks/src/evm_circuit.rs +++ b/circuit-benchmarks/src/evm_circuit.rs @@ -54,7 +54,7 @@ mod evm_circ_benches { .unwrap(); let block = block_convert(&builder).unwrap(); - let chunk = chunk_convert(&builder, 0).unwrap(); + let chunk = chunk_convert(&block, &builder).unwrap().remove(0); let circuit = TestEvmCircuit::::new(block, chunk); let mut rng = XorShiftRng::from_seed([ diff --git a/circuit-benchmarks/src/exp_circuit.rs b/circuit-benchmarks/src/exp_circuit.rs index 610da7d910..5468a4fa3f 100644 --- a/circuit-benchmarks/src/exp_circuit.rs +++ b/circuit-benchmarks/src/exp_circuit.rs @@ -149,9 +149,8 @@ mod tests { .new_circuit_input_builder() .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); - ( - block_convert(&builder).unwrap(), - chunk_convert(&builder, 0).unwrap(), - ) + let block = block_convert(&builder).unwrap(); + let chunk = chunk_convert(&block, &builder).unwrap().remove(0); + (block, chunk) } } diff --git a/integration-tests/src/integration_test_circuits.rs b/integration-tests/src/integration_test_circuits.rs index 62d8858507..3cab71fda2 100644 --- a/integration-tests/src/integration_test_circuits.rs +++ b/integration-tests/src/integration_test_circuits.rs @@ -424,7 +424,7 @@ impl + Circuit> IntegrationTest { block_tag, ); let mut block = block_convert(&builder).unwrap(); - let chunk = chunk_convert(&builder, 0).unwrap(); + let chunk = chunk_convert(&block, &builder).unwrap().remove(0); block.randomness = Fr::from(TEST_MOCK_RANDOMNESS); let circuit = C::new_from_block(&block, &chunk); let instance = circuit.instance(); @@ -441,7 +441,7 @@ impl + Circuit> IntegrationTest { ); // get chronological_rwtable and byaddr_rwtable columns index - let mut cs = ConstraintSystem::<::Scalar>::default(); + let mut cs = ConstraintSystem::<::Fr>::default(); let config = SuperCircuit::configure(&mut cs); let rwtable_columns = config.get_rwtable_columns(); @@ -515,10 +515,9 @@ fn new_empty_block_chunk() -> (Block, Chunk) { .new_circuit_input_builder() .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); - ( - block_convert(&builder).unwrap(), - chunk_convert(&builder, 0).unwrap(), - ) + let block = block_convert(&builder).unwrap(); + let chunk = chunk_convert(&block, &builder).unwrap().remove(0); + (block, chunk) } fn get_general_params(degree: u32) -> ParamsKZG { diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs index 9c60dff085..824370510e 100644 --- a/testool/src/statetest/executor.rs +++ b/testool/src/statetest/executor.rs @@ -16,13 +16,8 @@ use std::{collections::HashMap, str::FromStr}; use thiserror::Error; use zkevm_circuits::{ super_circuit::SuperCircuit, -<<<<<<< HEAD - test_util::CircuitTestBuilder, - witness::{Block, Chunk}, -======= test_util::{CircuitTestBuilder, CircuitTestError}, - witness::Block, ->>>>>>> main + witness::{Block, Chunk}, }; #[derive(PartialEq, Eq, Error, Debug)] @@ -354,12 +349,11 @@ pub fn run_test( let block: Block = zkevm_circuits::evm_circuit::witness::block_convert(&builder).unwrap(); let chunk: Chunk = - zkevm_circuits::evm_circuit::witness::chunk_convert(&builder, 0).unwrap(); + zkevm_circuits::evm_circuit::witness::chunk_convert(&block, &builder) + .unwrap() + .remove(0); -<<<<<<< HEAD - CircuitTestBuilder::<1, 1>::new_from_block(block, chunk).run(); -======= - CircuitTestBuilder::<1, 1>::new_from_block(block) + CircuitTestBuilder::<1, 1>::new_from_block(block, chunk) .run_with_result() .map_err(|err| match err { CircuitTestError::VerificationFailed { reasons, .. } => { @@ -373,7 +367,6 @@ pub fn run_test( found: err.to_string(), }, })?; ->>>>>>> main } else { geth_data.sign(&wallets); diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 178202d150..dd65fbb85c 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -859,8 +859,8 @@ impl SubCircuit for CopyCircuit { max_calldata: chunk.fixed_param.max_calldata, txs: block.txs.clone(), max_rws: chunk.fixed_param.max_rws, - rws: chunk.rws.clone(), - prev_chunk_last_rw: chunk.prev_chunk_last_rw, + rws: chunk.chrono_rws.clone(), + prev_chunk_last_rw: chunk.prev_chunk_last_chrono_rw, bytecodes: block.bytecodes.clone(), }, ) diff --git a/zkevm-circuits/src/copy_circuit/test.rs b/zkevm-circuits/src/copy_circuit/test.rs index d392aa0137..7f2056847b 100644 --- a/zkevm-circuits/src/copy_circuit/test.rs +++ b/zkevm-circuits/src/copy_circuit/test.rs @@ -58,8 +58,8 @@ pub fn test_copy_circuit_from_block( max_calldata: chunk.fixed_param.max_calldata, txs: block.txs, max_rws: chunk.fixed_param.max_rws, - rws: chunk.rws, - prev_chunk_last_rw: chunk.prev_chunk_last_rw, + rws: chunk.chrono_rws, + prev_chunk_last_rw: chunk.prev_chunk_last_chrono_rw, bytecodes: block.bytecodes, }, ) @@ -180,7 +180,7 @@ fn gen_tx_log_data() -> CircuitInputBuilder { fn copy_circuit_valid_calldatacopy() { let builder = gen_calldatacopy_data(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_eq!(test_copy_circuit_from_block(14, block, chunk), Ok(())); } @@ -188,7 +188,7 @@ fn copy_circuit_valid_calldatacopy() { fn copy_circuit_valid_codecopy() { let builder = gen_codecopy_data(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_eq!(test_copy_circuit_from_block(10, block, chunk), Ok(())); } @@ -196,7 +196,7 @@ fn copy_circuit_valid_codecopy() { fn copy_circuit_valid_extcodecopy() { let builder = gen_extcodecopy_data(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_eq!(test_copy_circuit_from_block(14, block, chunk), Ok(())); } @@ -204,7 +204,7 @@ fn copy_circuit_valid_extcodecopy() { fn copy_circuit_valid_sha3() { let builder = gen_sha3_data(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_eq!(test_copy_circuit_from_block(14, block, chunk), Ok(())); } @@ -212,7 +212,7 @@ fn copy_circuit_valid_sha3() { fn copy_circuit_valid_tx_log() { let builder = gen_tx_log_data(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_eq!(test_copy_circuit_from_block(10, block, chunk), Ok(())); } @@ -225,7 +225,7 @@ fn copy_circuit_invalid_calldatacopy() { builder.block.copy_events[0].bytes[0].0.wrapping_add(1); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_error_matches( test_copy_circuit_from_block(14, block, chunk), @@ -242,7 +242,7 @@ fn copy_circuit_invalid_codecopy() { builder.block.copy_events[0].bytes[0].0.wrapping_add(1); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_error_matches( test_copy_circuit_from_block(10, block, chunk), @@ -259,7 +259,7 @@ fn copy_circuit_invalid_extcodecopy() { builder.block.copy_events[0].bytes[0].0.wrapping_add(1); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_error_matches( test_copy_circuit_from_block(14, block, chunk), @@ -276,7 +276,7 @@ fn copy_circuit_invalid_sha3() { builder.block.copy_events[0].bytes[0].0.wrapping_add(1); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_error_matches( test_copy_circuit_from_block(14, block, chunk), @@ -293,7 +293,7 @@ fn copy_circuit_invalid_tx_log() { builder.block.copy_events[0].bytes[0].0.wrapping_add(1); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); assert_error_matches( test_copy_circuit_from_block(10, block, chunk), diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index 189ad8d7e3..d8037ae570 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -334,9 +334,9 @@ impl SubCircuit for EvmCircuit { .assign_block(layouter, block, chunk, challenges)?; let (rw_rows_padding, _) = RwMap::table_assignments_padding( - &chunk.rws.table_assignments(true), + &chunk.chrono_rws.table_assignments(true), chunk.fixed_param.max_rws, - chunk.prev_chunk_last_rw, + chunk.prev_chunk_last_chrono_rw, ); let ( alpha_cell, @@ -353,10 +353,10 @@ impl SubCircuit for EvmCircuit { &mut region, // pass non-padding rws to `load_with_region` since it will be padding // inside - &chunk.rws.table_assignments(true), + &chunk.chrono_rws.table_assignments(true), // align with state circuit to padding to same max_rws chunk.fixed_param.max_rws, - chunk.prev_chunk_last_rw, + chunk.prev_chunk_last_chrono_rw, )?; let permutation_cells = config.rw_permutation_config.assign( &mut region, @@ -565,7 +565,7 @@ impl Circuit for EvmCircuit { chunk.fixed_param.max_txs, chunk.fixed_param.max_calldata, )?; - chunk.rws.check_rw_counter_sanity(); + chunk.chrono_rws.check_rw_counter_sanity(); config .bytecode_table .load(&mut layouter, block.bytecodes.clone())?; @@ -698,7 +698,7 @@ mod evm_circuit_stats { .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); let k = block.get_test_degree(&chunk); let circuit = EvmCircuit::::get_test_circuit_from_block(block, chunk); let instance = circuit.instance_extend_chunk_ctx(); @@ -723,7 +723,7 @@ mod evm_circuit_stats { .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); let k = block.get_test_degree(&chunk); let circuit = EvmCircuit::::get_test_circuit_from_block(block, chunk); @@ -746,7 +746,7 @@ mod evm_circuit_stats { .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); let k = block.get_test_degree(&chunk); let circuit = EvmCircuit::::get_test_circuit_from_block(block, chunk); let instance = circuit.instance_extend_chunk_ctx(); diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 2b964c6607..08c89791dd 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -29,7 +29,8 @@ impl ExecutionGadget for EndChunkGadget { const EXECUTION_STATE: ExecutionState = ExecutionState::EndChunk; fn configure(cb: &mut EVMConstraintBuilder) -> Self { - // State transition + // State transition on non-last evm step + // TODO/FIXME make EndChunk must be in last evm step and remove below constraint cb.not_step_last(|cb| { // Propagate all the way down. cb.require_step_state_transition(StepStateTransition::same()); @@ -102,7 +103,7 @@ mod test { // } println!( "=> FIXME is fixed? {:?}", - chunk.rws.0.get_mut(&Target::Start) + chunk.chrono_rws.0.get_mut(&Target::Start) ); })) .run_dynamic_chunk(4, 2); diff --git a/zkevm-circuits/src/exp_circuit/test.rs b/zkevm-circuits/src/exp_circuit/test.rs index 5d3f1f5394..6f2129bda6 100644 --- a/zkevm-circuits/src/exp_circuit/test.rs +++ b/zkevm-circuits/src/exp_circuit/test.rs @@ -67,7 +67,7 @@ fn test_ok(base: Word, exponent: Word, k: Option) { let code = gen_code_single(base, exponent); let builder = gen_data(code, false); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); test_exp_circuit(k.unwrap_or(18), block, chunk); } @@ -75,7 +75,7 @@ fn test_ok_multiple(args: Vec<(Word, Word)>) { let code = gen_code_multiple(args); let builder = gen_data(code, false); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); test_exp_circuit(20, block, chunk); } @@ -125,7 +125,7 @@ fn variadic_size_check() { .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); let circuit = ExpCircuit::::new(block.exp_events, chunk.fixed_param.max_exp_steps); let prover1 = MockProver::::run(k, &circuit, vec![]).unwrap(); @@ -141,7 +141,7 @@ fn variadic_size_check() { }; let builder = gen_data(code, true); let block = block_convert::(&builder).unwrap(); - let chunk = chunk_convert::(&builder, 0).unwrap(); + let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); let circuit = ExpCircuit::::new(block.exp_events, chunk.fixed_param.max_exp_steps); let prover2 = MockProver::::run(k, &circuit, vec![]).unwrap(); diff --git a/zkevm-circuits/src/pi_circuit/test.rs b/zkevm-circuits/src/pi_circuit/test.rs index 927acb4332..e8f39ed19e 100644 --- a/zkevm-circuits/src/pi_circuit/test.rs +++ b/zkevm-circuits/src/pi_circuit/test.rs @@ -149,7 +149,7 @@ fn test_1tx_1maxtx() { block.sign(&wallets); let block = block_convert(&builder).unwrap(); - let chunk = chunk_convert(&builder, 0).unwrap(); + let chunk = chunk_convert(&block, &builder).unwrap().remove(0); // MAX_TXS, MAX_TXS align with `CircuitsParams` let circuit = PiCircuit::::new_from_block(&block, &chunk); let public_inputs = circuit.instance(); @@ -230,7 +230,7 @@ fn test_1wd_1wdmax() { .unwrap(); let block = block_convert(&builder).unwrap(); - let chunk = chunk_convert(&builder, 0).unwrap(); + let chunk = chunk_convert(&block, &builder).unwrap().remove(0); // MAX_TXS, MAX_TXS align with `CircuitsParams` let circuit = PiCircuit::::new_from_block(&block, &chunk); let public_inputs = circuit.instance(); diff --git a/zkevm-circuits/src/state_circuit.rs b/zkevm-circuits/src/state_circuit.rs index 9b5c6258fe..31657cdfe7 100644 --- a/zkevm-circuits/src/state_circuit.rs +++ b/zkevm-circuits/src/state_circuit.rs @@ -471,7 +471,7 @@ pub struct StateCircuit { impl StateCircuit { /// make a new state circuit from an RwMap pub fn new(chunk: &Chunk) -> Self { - let rows = chunk.rws.table_assignments(false); // address sorted + let rows = chunk.by_address_rws.table_assignments(false); // address sorted let updates = MptUpdates::mock_from(&rows); Self { rows, @@ -483,8 +483,8 @@ impl StateCircuit { overrides: HashMap::new(), permu_alpha: chunk.permu_alpha, permu_gamma: chunk.permu_gamma, - rw_fingerprints: chunk.rw_fingerprints.clone(), - prev_chunk_last_rw: chunk.prev_chunk_last_rw, + rw_fingerprints: chunk.by_address_rw_fingerprints.clone(), + prev_chunk_last_rw: chunk.prev_chunk_last_chrono_rw, _marker: PhantomData::default(), } } @@ -506,7 +506,7 @@ impl SubCircuit for StateCircuit { /// Return the minimum number of rows required to prove the block fn min_num_rows_block(_block: &witness::Block, chunk: &Chunk) -> (usize, usize) { ( - chunk.rws.0.values().flatten().count() + 1, + chunk.chrono_rws.0.values().flatten().count() + 1, chunk.fixed_param.max_rws, ) } diff --git a/zkevm-circuits/src/state_circuit/test.rs b/zkevm-circuits/src/state_circuit/test.rs index ffe06afabe..a00b45c798 100644 --- a/zkevm-circuits/src/state_circuit/test.rs +++ b/zkevm-circuits/src/state_circuit/test.rs @@ -1012,7 +1012,7 @@ fn prover(rows: Vec, overrides: HashMap<(AdviceColumn, isize), Fr>) -> MockP let rw_rows: Vec>> = rw_overrides_skip_first_padding(&rw_rows, &overrides); let rwtable_fingerprints = - get_permutation_fingerprint_of_rwrowvec(&rw_rows, N_ROWS, Fr::ONE, Fr::ONE, Fr::ONE); + get_permutation_fingerprint_of_rwrowvec(&rw_rows, N_ROWS, Fr::ONE, Fr::ONE, Fr::ONE, None); let row_padding_and_overridess = rw_rows.to2dvec(); let updates = MptUpdates::mock_from(&rows); diff --git a/zkevm-circuits/src/super_circuit.rs b/zkevm-circuits/src/super_circuit.rs index df39cbdc42..c19a9247d7 100644 --- a/zkevm-circuits/src/super_circuit.rs +++ b/zkevm-circuits/src/super_circuit.rs @@ -581,7 +581,7 @@ impl SuperCircuit { mock_randomness: F, ) -> Result<(u32, Self, Vec>), bus_mapping::Error> { let mut block = block_convert(builder).unwrap(); - let chunk = chunk_convert(builder, 0).unwrap(); + let chunk = chunk_convert(&block, builder).unwrap().remove(0); block.randomness = mock_randomness; let (_, rows_needed) = Self::min_num_rows_block(&block, &chunk); diff --git a/zkevm-circuits/src/super_circuit/test.rs b/zkevm-circuits/src/super_circuit/test.rs index 2ff959e47a..0acb919ec7 100644 --- a/zkevm-circuits/src/super_circuit/test.rs +++ b/zkevm-circuits/src/super_circuit/test.rs @@ -201,7 +201,7 @@ fn test_rw_table_commitment() { rw_table_commitment::>(¶ms); } -fn rw_table_commitment<'params, Scheme: CommitmentScheme>(params: &'params Scheme::ParamsProver) +fn rw_table_commitment(params: &Scheme::ParamsProver) where ::Scalar: WithSmallOrderMulGroup<3> + eth_types::Field, { diff --git a/zkevm-circuits/src/table/rw_table.rs b/zkevm-circuits/src/table/rw_table.rs index 5535d8e42e..c20da18c60 100644 --- a/zkevm-circuits/src/table/rw_table.rs +++ b/zkevm-circuits/src/table/rw_table.rs @@ -5,14 +5,17 @@ use halo2_proofs::{ }; use halo2_proofs::{ - halo2curves::{bn256::Fr, group::Curve, CurveAffine}, + halo2curves::{ + bn256::Fr, + group::{prime::PrimeCurveAffine, Curve}, + CurveAffine, + }, plonk::{Advice, Assigned, Assignment, Challenge, Fixed, FloorPlanner, Instance, Selector}, poly::{ commitment::{Blind, CommitmentScheme, Params}, EvaluationDomain, LagrangeCoeff, Polynomial, }, }; -use snark_verifier::util::arithmetic::PrimeCurveAffine; use super::*; @@ -186,11 +189,12 @@ impl RwTable { /// get rw table column commitment /// implementation snippet from halo2 `create_proof` https://github.com/privacy-scaling-explorations/halo2/blob/9b33f9ce524dbb9133fc8b9638b2afd0571659a8/halo2_proofs/src/plonk/prover.rs#L37 -pub fn get_rwtable_cols_commitment<'params, Scheme: CommitmentScheme>( +#[allow(unused)] +pub fn get_rwtable_cols_commitment( degree: usize, rws: &[Rw], n_rows: usize, - params_prover: &'params Scheme::ParamsProver, + params_prover: &Scheme::ParamsProver, ) -> Vec<::Curve> where ::Scalar: WithSmallOrderMulGroup<3> + Field, @@ -200,7 +204,7 @@ where _marker: std::marker::PhantomData, } - impl<'a, F: Field> Assignment for WitnessCollection { + impl Assignment for WitnessCollection { fn enter_region(&mut self, _: N) where NR: Into, @@ -319,7 +323,7 @@ where } } - let rwtable_circuit = RwTableCircuit::new(&rws, n_rows, None); + let rwtable_circuit = RwTableCircuit::new(rws, n_rows, None); let domain = EvaluationDomain::<::Scalar>::new( degree as u32, @@ -392,7 +396,7 @@ struct RwTableCircuitConfig { pub rw_table: RwTable, } -impl<'a> RwTableCircuitConfig {} +impl RwTableCircuitConfig {} impl<'a, F: Field> Circuit for RwTableCircuit<'a> { type Config = RwTableCircuitConfig; @@ -432,6 +436,7 @@ impl<'a, F: Field> Circuit for RwTableCircuit<'a> { } // migrate from halo2 library +#[allow(unused)] fn batch_invert_assigned>( domain: EvaluationDomain, assigned: Vec, LagrangeCoeff>>, diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index b8e901f19d..9617ad38ec 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -195,7 +195,9 @@ impl CircuitTestBuilder { // Build a witness block from trace result. let mut block = crate::witness::block_convert(&builder) .map_err(|err| CircuitTestError::CannotConvertBlock(err.to_string()))?; - let mut chunk = crate::witness::chunk_convert(&builder, chunk_index).unwrap(); + let mut chunk = crate::witness::chunk_convert(&block, &builder) + .unwrap() + .remove(chunk_index); for modifier_fn in &self.block_modifiers { modifier_fn.as_ref()(&mut block, &mut chunk); @@ -217,10 +219,12 @@ impl CircuitTestBuilder { // No cache for EVM circuit with customized features let prover = if block.feature_config.is_mainnet() { let circuit = EvmCircuitCached::get_test_circuit_from_block(block, chunk); - MockProver::::run(k, &circuit, vec![]) + let instance = circuit.instance(); + MockProver::::run(k, &circuit, instance) } else { let circuit = EvmCircuit::get_test_circuit_from_block(block, chunk); - MockProver::::run(k, &circuit, vec![]) + let instance = circuit.instance(); + MockProver::::run(k, &circuit, instance) }; let prover = prover.map_err(|err| CircuitTestError::SynthesisFailure { @@ -285,6 +289,9 @@ impl CircuitTestBuilder { } } + /// Triggers the `CircuitTestBuilder` to convert the [`TestContext`] if any, + /// into a [`Block`] and apply the default or provided block_modifiers or + /// circuit checks to the provers generated for the State and EVM circuits. pub fn run_with_result(self) -> Result<(), CircuitTestError> { let (block, chunk) = self.build_block(0, 1)?; @@ -345,7 +352,9 @@ impl CircuitTestBuilder { // Build a witness block from trace result. let mut block = crate::witness::block_convert(&builder).unwrap(); - let mut chunk = crate::witness::chunk_convert(&builder, chunk_index).unwrap(); + let mut chunk = crate::witness::chunk_convert(&block, &builder) + .unwrap() + .remove(chunk_index); println!("fingerprints = {:?}", chunk.chrono_rw_fingerprints); diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index 4032ad416b..d62a0700ec 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeMap; + use super::{ rw::{RwFingerprints, ToVec}, ExecStep, Rw, RwMap, Transaction, @@ -35,6 +37,8 @@ pub struct Block { pub end_block: ExecStep, /// Read write events in the RwTable pub rws: RwMap, + /// Read write events in the RwTable, sorted by address + pub by_address_rws: Vec, /// Bytecode used in the block pub bytecodes: CodeDB, /// The block context @@ -57,6 +61,8 @@ pub struct Block { pub keccak_inputs: Vec>, /// Original Block from geth pub eth_block: eth_types::Block, + /// rw_table padding meta data + pub rw_padding_meta: BTreeMap, } impl Block { @@ -280,12 +286,29 @@ pub fn block_convert( let block = &builder.block; let code_db = &builder.code_db; let rws = RwMap::from(&block.container); + let by_address_rws = rws.table_assignments(false); rws.check_value(); + + // get padding statistics data via BtreeMap + // TODO we can implement it in more efficient version via range sum + let rw_padding_meta = builder + .chunks + .iter() + .fold(BTreeMap::new(), |mut map, chunk| { + assert!(chunk.ctx.rwc.0.saturating_sub(1) <= builder.circuits_params.max_rws); + // [chunk.ctx.rwc.0, builder.circuits_params.max_rws + 1) + (chunk.ctx.rwc.0..builder.circuits_params.max_rws + 1).for_each(|padding_rw_counter| { + *map.entry(padding_rw_counter).or_insert(0) += 1; + }); + map + }); + let mut block = Block { // randomness: F::from(0x100), // Special value to reveal elements after RLC randomness: F::from(0xcafeu64), context: block.into(), rws, + by_address_rws, txs: block.txs().to_vec(), bytecodes: code_db.clone(), copy_events: block.copy_events.clone(), @@ -298,6 +321,7 @@ pub fn block_convert( keccak_inputs: circuit_input_builder::keccak_inputs(block, code_db)?, eth_block: block.eth_block.clone(), end_block: block.end_block.clone(), + rw_padding_meta, }; let public_data = public_data_convert(&block); diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index f92c0b0abc..0ad3907310 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -1,7 +1,9 @@ +use std::{cmp::min, iter}; + /// use super::{ rw::{RwFingerprints, ToVec}, - ExecStep, Rw, RwMap, RwRow, + Block, ExecStep, Rw, RwMap, RwRow, }; use crate::util::unwrap_value; use bus_mapping::{ @@ -11,6 +13,7 @@ use bus_mapping::{ use eth_types::Field; use gadgets::permutation::get_permutation_fingerprints; use halo2_proofs::circuit::Value; +use itertools::Itertools; /// [`Chunk`]` is the struct used by all circuits, which contains chunkwise /// data for witness generation. Used with [`Block`] for blockwise witness. @@ -24,15 +27,17 @@ pub struct Chunk { pub padding: Option, /// Chunk context pub chunk_context: ChunkContext, - /// Read write events in the RwTable - pub rws: RwMap, + /// Read write events in the chronological sorted RwTable + pub chrono_rws: RwMap, + /// Read write events in the by address sorted RwTable + pub by_address_rws: RwMap, /// Permutation challenge alpha pub permu_alpha: F, /// Permutation challenge gamma pub permu_gamma: F, /// Current rw_table permutation fingerprint - pub rw_fingerprints: RwFingerprints, + pub by_address_rw_fingerprints: RwFingerprints, /// Current chronological rw_table permutation fingerprint pub chrono_rw_fingerprints: RwFingerprints, @@ -42,7 +47,9 @@ pub struct Chunk { /// The last call of previous chunk if any, used for assigning continuation pub prev_last_call: Option, /// - pub prev_chunk_last_rw: Option, + pub prev_chunk_last_chrono_rw: Option, + /// + pub prev_chunk_last_by_address_rw: Option, } impl Default for Chunk { @@ -54,20 +61,22 @@ impl Default for Chunk { end_chunk: None, padding: None, chunk_context: ChunkContext::default(), - rws: RwMap::default(), + chrono_rws: RwMap::default(), + by_address_rws: RwMap::default(), permu_alpha: F::from(1), permu_gamma: F::from(1), - rw_fingerprints: RwFingerprints::default(), + by_address_rw_fingerprints: RwFingerprints::default(), chrono_rw_fingerprints: RwFingerprints::default(), fixed_param: FixedCParams::default(), prev_last_call: None, - prev_chunk_last_rw: None, + prev_chunk_last_chrono_rw: None, + prev_chunk_last_by_address_rw: None, } } } impl Chunk { - #[allow(dead_code)] + #[cfg(test)] pub(crate) fn new_from_rw_map( rws: &RwMap, padding_start_rw: Option, @@ -93,8 +102,8 @@ impl Chunk { true, chrono_padding_start_rw, ); - chunk.rws = rws.clone(); - chunk.rw_fingerprints = rw_fingerprints; + chunk.chrono_rws = rws.clone(); + chunk.by_address_rw_fingerprints = rw_fingerprints; chunk.chrono_rw_fingerprints = chrono_rw_fingerprints; chunk } @@ -102,93 +111,142 @@ impl Chunk { /// Convert the idx-th chunk struct in bus-mapping to a witness chunk used in circuits pub fn chunk_convert( + block: &Block, builder: &circuit_input_builder::CircuitInputBuilder, - idx: usize, -) -> Result, Error> { - let block = &builder.block; - let chunk = builder.get_chunk(idx); - let mut rws = RwMap::default(); - let prev_chunk_last_rw = builder.prev_chunk().map(|chunk| { - // rws range [chunk.ctx.initial_rwc, chunk.ctx.end_rwc) - RwMap::get_rw(&block.container, chunk.ctx.end_rwc - 1).expect("Rw does not exist") - }); +) -> Result>, Error> { + let (by_address_rws, padding_meta) = (&block.by_address_rws, &block.rw_padding_meta); - // FIXME(Cecilia): debug - println!( - "| {:?} ... {:?} | @chunk_convert", - chunk.ctx.initial_rwc, chunk.ctx.end_rwc - ); + // Todo: poseidon hash to compute alpha/gamma + let alpha = F::from(103); + let gamma = F::from(101); - // Compute fingerprints of all chunks - let mut challenges = Vec::with_capacity(builder.chunks.len()); - let mut rw_fingerprints: Vec> = Vec::with_capacity(builder.chunks.len()); - let mut chrono_rw_fingerprints: Vec> = - Vec::with_capacity(builder.chunks.len()); + let mut chunks: Vec> = Vec::with_capacity(builder.chunks.len()); + for (i, (prev_chunk, chunk)) in iter::once(None) // left append `None` to make iteration easier + .chain(builder.chunks.iter().map(Some)) + .tuple_windows() + .enumerate() + { + let chunk = chunk.unwrap(); // current chunk always there + let (prev_chunk_last_chrono_rw, prev_chunk_last_by_address_rw) = prev_chunk + .map(|prev_chunk| { + let prev_chunk_last_chrono_rw = { + assert!(builder.circuits_params.max_rws > 0); + let chunk_inner_rwc = prev_chunk.ctx.rwc.0; + if chunk_inner_rwc.saturating_sub(1) == builder.circuits_params.max_rws { + // if prev chunk rws are full, then get the last rwc + RwMap::get_rw(&builder.block.container, prev_chunk.ctx.end_rwc - 1) + .expect("Rw does not exist") + } else { + // last is the padding row + Rw::Padding { + rw_counter: builder.circuits_params.max_rws, + } + } + }; + // get prev_chunk last by_address sorted rw + let prev_chunk_by_address_last_rw = { + let by_address_last_rwc_index = + (prev_chunk.ctx.idx + 1) * builder.circuits_params.max_rws; + let prev_chunk_by_address_last_rwc = by_address_rws + .get(by_address_last_rwc_index - 1) + .cloned() + .or_else(|| { + let target_padding_count = + (by_address_last_rwc_index + 1) - by_address_rws.len(); + let mut accu_count = 0; + padding_meta + .iter() + .find(|(_, count)| { + accu_count += **count; + target_padding_count <= accu_count.try_into().unwrap() + }) + .map(|(padding_rwc, _)| Rw::Padding { + rw_counter: *padding_rwc, + }) + }); - for (i, chunk) in builder.chunks.iter().enumerate() { - // Get the Rws in the i-th chunk + prev_chunk_by_address_last_rwc + }; + (prev_chunk_last_chrono_rw, prev_chunk_by_address_last_rw) + }) + .map(|(prev_chunk_last_rwc, prev_chunk_by_address_last_rwc)| { + (Some(prev_chunk_last_rwc), prev_chunk_by_address_last_rwc) + }) + .unwrap_or_else(|| (None, None)); - // TODO this is just chronological rws, not by address sorted rws - // need another sorted method - let cur_rws = - RwMap::from_chunked(&block.container, chunk.ctx.initial_rwc, chunk.ctx.end_rwc); - cur_rws.check_value(); + println!( + "| {:?} ... {:?} | @chunk_convert", + chunk.ctx.initial_rwc, chunk.ctx.end_rwc + ); + + // Get the rws in the i-th chunk + let chrono_rws = RwMap::from(&builder.block.container) + .take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc); - // Todo: poseidon hash - let alpha = F::from(103); - let gamma = F::from(101); + chrono_rws.check_value(); + + let by_address_rws = RwMap::from({ + // by_address_rws + let start = min( + chunk.ctx.idx * builder.circuits_params.max_rws, + by_address_rws.len() - 1, + ); + let end = min( + (chunk.ctx.idx + 1) * builder.circuits_params.max_rws, + by_address_rws.len(), + ); + by_address_rws[start..end].to_vec() + }) + .take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc); // Comupute cur fingerprints from last fingerprints and current Rw rows - let cur_fingerprints = get_permutation_fingerprint_of_rwmap( - &cur_rws, + let by_address_rw_fingerprints = get_permutation_fingerprint_of_rwmap( + &by_address_rws, chunk.fixed_param.max_rws, alpha, gamma, if i == 0 { F::from(1) } else { - rw_fingerprints[i - 1].mul_acc + chunks[i - 1].by_address_rw_fingerprints.mul_acc }, false, + prev_chunk_last_by_address_rw, ); - let cur_chrono_fingerprints = get_permutation_fingerprint_of_rwmap( - &cur_rws, + + let chrono_rw_fingerprints = get_permutation_fingerprint_of_rwmap( + &chrono_rws, chunk.fixed_param.max_rws, alpha, gamma, if i == 0 { F::from(1) } else { - chrono_rw_fingerprints[i - 1].mul_acc + chunks[i - 1].chrono_rw_fingerprints.mul_acc }, true, + prev_chunk_last_chrono_rw, ); - challenges.push(vec![alpha, gamma]); - rw_fingerprints.push(cur_fingerprints); - chrono_rw_fingerprints.push(cur_chrono_fingerprints); - if i == idx { - rws = cur_rws; - } + chunks.push(Chunk { + permu_alpha: alpha, + permu_gamma: gamma, + by_address_rw_fingerprints, + chrono_rw_fingerprints, + begin_chunk: chunk.begin_chunk.clone(), + end_chunk: chunk.end_chunk.clone(), + padding: chunk.padding.clone(), + chunk_context: chunk.ctx.clone(), + chrono_rws, + by_address_rws, + fixed_param: chunk.fixed_param, + prev_last_call: chunk.prev_last_call.clone(), + prev_chunk_last_chrono_rw, + prev_chunk_last_by_address_rw, + }); } - // TODO(Cecilia): if we chunk across blocks then need to store the prev_block - let chunk = Chunk { - permu_alpha: challenges[idx][0], - permu_gamma: challenges[idx][1], - rw_fingerprints: rw_fingerprints[idx].clone(), - chrono_rw_fingerprints: chrono_rw_fingerprints[idx].clone(), - begin_chunk: chunk.begin_chunk.clone(), - end_chunk: chunk.end_chunk.clone(), - padding: chunk.padding.clone(), - chunk_context: chunk.ctx.clone(), - rws, - fixed_param: chunk.fixed_param, - prev_last_call: chunk.prev_last_call, - prev_chunk_last_rw, - }; - - Ok(chunk) + Ok(chunks) } /// diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index 2076e56a26..907472b900 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -190,19 +190,13 @@ impl RwMap { rows } - /// Get RwMap for a chunk specified by start and end - pub fn from_chunked( - container: &operation::OperationContainer, - start: usize, - end: usize, - ) -> Self { - let mut rws: Self = container.into(); - for rw in rws.0.values_mut() { + /// take only rw_counter within range + pub fn take_rw_counter_range(mut self, start: usize, end: usize) -> Self { + for rw in self.0.values_mut() { rw.retain(|r| r.rw_counter() >= start && r.rw_counter() < end) } - rws + self } - /// Get one Rw for a chunk specified by index pub fn get_rw(container: &operation::OperationContainer, counter: usize) -> Option { let rws: Self = container.into(); diff --git a/zkevm-circuits/tests/prover_error.rs b/zkevm-circuits/tests/prover_error.rs index bd8cb386c1..3b6ef967df 100644 --- a/zkevm-circuits/tests/prover_error.rs +++ b/zkevm-circuits/tests/prover_error.rs @@ -97,7 +97,9 @@ fn prover_error() { .expect("handle_block"); let (block, chunk) = { let mut block = block_convert(&builder).expect("block_convert"); - let chunk = chunk_convert(&builder, 0).expect("chunk_convert"); + let chunk = chunk_convert(&block, &builder) + .expect("chunk_convert") + .remove(0); block.randomness = Fr::from(MOCK_RANDOMNESS); (block, chunk) From 46d5437bf81cd331f71997b01c2ef04187fc0452 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Thu, 22 Feb 2024 15:53:04 +0800 Subject: [PATCH 07/16] refactor: simplify chunking logic in bus-mapping --- bus-mapping/src/circuit_input_builder.rs | 263 ++++++++++-------- .../src/circuit_input_builder/chunk.rs | 4 - .../circuit_input_builder/input_state_ref.rs | 8 +- .../src/evm_circuit/execution/end_chunk.rs | 74 ++++- zkevm-circuits/src/witness/chunk.rs | 2 +- 5 files changed, 215 insertions(+), 136 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index a699c56393..62cb9704a3 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -84,7 +84,9 @@ impl FeatureConfig { } } -const RW_BUFFER: usize = 30; +// RW_BUFFER_SIZE need to set to cover max rwc row contributed by a ExecStep +const RW_BUFFER_SIZE: usize = 30; + /// Circuit Setup Parameters #[derive(Debug, Clone, Copy)] pub struct FixedCParams { @@ -337,60 +339,40 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { geth_trace: &GethExecTrace, tx: Transaction, tx_ctx: TransactionContext, - geth_steps: Option<(usize, &GethExecStep)>, + next_geth_step: Option<(usize, &GethExecStep)>, last_call: Option, ) -> Result<(), Error> { - if !self.chunk_ctx.enable { + // we dont chunk if + // 1. on last chunk + // 2. still got some buffer room before max_rws + let Some(max_rws) = self.circuits_params.max_rws() else { + // terminiate earlier due to no max_rws return Ok(()); - } - let is_last_tx = tx_ctx.is_last_tx(); - let is_dynamic_max_row = self.circuits_params.max_rws().is_none(); - let mut gen_chunk = - // No lookahead, if chunk_rws exceed max just chunk then update param - (is_dynamic_max_row && self.chunk_rws() > self.circuits_params.max_rws().unwrap_or_default().saturating_sub(self.rws_reserve())) - // Lookahead, chunk_rws should never exceed, never update param - || (!is_dynamic_max_row && self.chunk_rws() + RW_BUFFER >= self.circuits_params.max_rws().unwrap_or_default().saturating_sub(self.rws_reserve())); - - if gen_chunk { - // Optain the first op of the next GethExecStep, for fixed case also lookahead - let (mut cib, mut tx, mut tx_ctx_) = (self.clone(), tx, tx_ctx); - let mut cib_ref = cib.state_ref(&mut tx, &mut tx_ctx_); - let ops = if let Some((i, step)) = geth_steps { - log::trace!("chunk at {}th opcode {:?} ", i, step.op); - gen_associated_ops(&step.op, &mut cib_ref, &geth_trace.struct_logs[i..])? - } else { - log::trace!("chunk at EndTx"); - let end_tx_step = gen_associated_steps(&mut cib_ref, ExecState::EndTx)?; - // When there's next Tx lined up, also peek BeginTx - // because we don't check btw EndTx & BeginTx - if !is_last_tx { - gen_associated_steps(&mut cib_ref, ExecState::BeginTx)?; - } - vec![end_tx_step] - }; - - // Check again, 1) if dynamic keep chunking 2) if fixed chunk when lookahead exceed - // 3) gen chunk steps there're more chunks after - gen_chunk = !self.chunk_ctx.is_last_chunk() - && (is_dynamic_max_row - || cib.chunk_rws() - > self - .circuits_params - .max_rws() - .unwrap_or_default() - .saturating_sub(cib.rws_reserve())); - if is_dynamic_max_row { - self.cur_chunk_mut().fixed_param = self.compute_param(&self.block.eth_block); - } - if gen_chunk { - let last_copy = self.block.copy_events.len(); - // Generate EndChunk and proceed to the next if it's not the last chunk - // Set next step pre-state as end_chunk state - self.set_end_chunk(&ops[0]); - self.commit_chunk(true, tx.id as usize, last_copy, last_call); - self.set_begin_chunk(&ops[0]); - } - } + }; + + if self.chunk_ctx.is_last_chunk() || self.chunk_rws() + RW_BUFFER_SIZE < max_rws { + return Ok(()); + }; + + // Optain the first op of the next GethExecStep, for fixed case also lookahead + let (mut cib, mut tx, mut tx_ctx) = (self.clone(), tx, tx_ctx); + let mut cib_ref = cib.state_ref(&mut tx, &mut tx_ctx); + let next_ops = if let Some((i, step)) = next_geth_step { + log::trace!("chunk at {}th opcode {:?} ", i, step.op); + gen_associated_ops(&step.op, &mut cib_ref, &geth_trace.struct_logs[i..])?.remove(0) + } else { + log::trace!("chunk at EndTx"); + gen_associated_steps(&mut cib_ref, ExecState::EndTx)? + }; + + let last_copy = self.block.copy_events.len(); + // Generate EndChunk and proceed to the next if it's not the last chunk + // Set next step pre-state as end_chunk state + self.set_end_chunk(&next_ops); + // tx.id start from 1, so it's equivalent to `next_tx_index` + self.commit_chunk_ctx(true, tx.id as usize, last_copy, last_call); + self.set_begin_chunk(&next_ops); + Ok(()) } @@ -407,13 +389,13 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { geth_trace: &GethExecTrace, is_last_tx: bool, tx_index: u64, - ) -> Result, Error> { + ) -> Result<(ExecStep, Option), Error> { let mut tx = self.new_tx(tx_index, eth_tx, !geth_trace.failed)?; let mut tx_ctx = TransactionContext::new(eth_tx, geth_trace, is_last_tx)?; - // Prev chunk last call - let mut last_call = None; - if !geth_trace.invalid { + let res = if !geth_trace.invalid { + let mut last_call = None; + // Generate BeginTx step let begin_tx_step = gen_associated_steps( &mut self.state_ref(&mut tx, &mut tx_ctx), @@ -423,7 +405,7 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { let mut trace = geth_trace.struct_logs.iter().enumerate().peekable(); while let Some((peek_i, peek_step)) = trace.peek() { - // Check the peek_sted and chunk if needed + // Check the peek step and chunk if needed self.check_and_chunk( geth_trace, tx.clone(), @@ -462,22 +444,26 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { // Generate EndTx step let end_tx_step = gen_associated_steps(&mut self.state_ref(&mut tx, &mut tx_ctx), ExecState::EndTx)?; - tx.steps_mut().push(end_tx_step); + tx.steps_mut().push(end_tx_step.clone()); + (end_tx_step, last_call) } else if self.feature_config.invalid_tx { + // chunk before hands to avoid chunk between [InvalidTx, BeginTx) + self.check_and_chunk(geth_trace, tx.clone(), tx_ctx.clone(), None, None)?; // Generate InvalidTx step let invalid_tx_step = gen_associated_steps( &mut self.state_ref(&mut tx, &mut tx_ctx), ExecState::InvalidTx, )?; - tx.steps_mut().push(invalid_tx_step); + tx.steps_mut().push(invalid_tx_step.clone()); + (invalid_tx_step, None) } else { panic!("invalid tx support not enabled") - } + }; self.sdb.commit_tx(); self.block.txs.push(tx); - Ok(last_call) + Ok(res) } // TODO Fix this, for current logic on processing `call` is incorrect @@ -553,17 +539,18 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { /// Set the end status of a chunk including the current globle rwc /// and commit the current chunk context, proceed to the next chunk /// if needed - pub fn commit_chunk( + pub fn commit_chunk_ctx( &mut self, to_next: bool, next_tx_index: usize, next_copy_index: usize, last_call: Option, ) { + println!("commit_chunk_ctx"); self.chunk_ctx.end_rwc = self.block_ctx.rwc.0; self.chunk_ctx.end_tx_index = next_tx_index; self.chunk_ctx.end_copy_index = next_copy_index; - self.chunks[self.chunk_ctx.idx].ctx = self.chunk_ctx.clone(); + self.cur_chunk_mut().ctx = self.chunk_ctx.clone(); if to_next { self.chunk_ctx .bump(self.block_ctx.rwc.0, next_tx_index, next_copy_index); @@ -571,18 +558,16 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { } } - fn set_begin_chunk(&mut self, last_step: &ExecStep) { - let mut begin_chunk = last_step.clone(); + fn set_begin_chunk(&mut self, first_step: &ExecStep) { + let mut begin_chunk = first_step.clone(); begin_chunk.exec_state = ExecState::BeginChunk; self.gen_chunk_associated_steps(&mut begin_chunk, RW::READ); self.chunks[self.chunk_ctx.idx].begin_chunk = Some(begin_chunk); } - fn set_end_chunk(&mut self, last_step: &ExecStep) { - let mut end_chunk = last_step.clone(); + fn set_end_chunk(&mut self, next_step: &ExecStep) { + let mut end_chunk = next_step.clone(); end_chunk.exec_state = ExecState::EndChunk; - end_chunk.rwc = self.block_ctx.rwc; - end_chunk.rwc_inner_chunk = self.chunk_ctx.rwc; self.gen_chunk_associated_steps(&mut end_chunk, RW::WRITE); self.gen_chunk_padding(&mut end_chunk); self.chunks[self.chunk_ctx.idx].end_chunk = Some(end_chunk); @@ -672,6 +657,41 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { } impl CircuitInputBuilder { + /// First part of handle_block, only called by fixed Builder + pub fn begin_handle_block( + &mut self, + eth_block: &EthBlock, + geth_traces: &[eth_types::GethExecTrace], + ) -> Result<(ExecStep, Option), Error> { + assert!( + self.circuits_params.max_rws().unwrap_or_default() > self.rws_reserve(), + "Fixed max_rws not enough for rws reserve" + ); + + // accumulates gas across all txs in the block + let mut res = eth_block + .transactions + .iter() + .enumerate() + .map(|(idx, tx)| { + let geth_trace = &geth_traces[idx]; + // Transaction index starts from 1 + let tx_id = idx + 1; + self.handle_tx( + tx, + geth_trace, + tx_id == eth_block.transactions.len(), + tx_id as u64, + ) + }) + .collect::)>, _>>()?; + let res = res.remove(res.len() - 1); + // set eth_block + self.block.eth_block = eth_block.clone(); + self.set_value_ops_call_context_rwc_eor(); + Ok(res) + } + /// Handle a block by handling each transaction to generate all the /// associated operations. pub fn handle_block( @@ -681,17 +701,41 @@ impl CircuitInputBuilder { ) -> Result, Error> { println!("--------------{:?}", self.circuits_params); // accumulates gas across all txs in the block - let last_call = self.begin_handle_block(eth_block, geth_traces)?; + let (last_step, last_call) = self.begin_handle_block(eth_block, geth_traces)?; + + assert!(self.circuits_params.max_rws().is_some()); - // At the last chunk fixed param also need to be updated - if self.circuits_params.max_rws().is_none() { - self.cur_chunk_mut().fixed_param = self.compute_param(&self.block.eth_block); - } else { - self.cur_chunk_mut().fixed_param = self.circuits_params; - } - self.set_end_block()?; let last_copy = self.block.copy_events.len(); - self.commit_chunk(false, eth_block.transactions.len(), last_copy, last_call); + + // We fill dummy virtual steps: BeginChunk,EndChunk for all lefted empty chunks + let last_process_chunk_id = self.chunk_ctx.idx; + (last_process_chunk_id..self.circuits_params.total_chunks()).try_for_each(|idx| { + // TODO figure out and resolve generic param type and move fixed_param set inside + // commit_chunk_ctx After fixed, then we can set fixed_param on all chunks + self.cur_chunk_mut().fixed_param = self.circuits_params; + + if idx == self.circuits_params.total_chunks() - 1 { + self.commit_chunk_ctx( + false, + eth_block.transactions.len(), + last_copy, + last_call.clone(), + ); + self.set_end_block()?; + } else { + // it doent matter what step was put to set_end_chunk/set_begin_chunk on no-used + // chunks before end_block. Just need to make sure it's step lookup is consistency. + self.set_end_chunk(&last_step); + self.commit_chunk_ctx( + true, + eth_block.transactions.len(), + last_copy, + last_call.clone(), + ); + self.set_begin_chunk(&last_step); + } + Ok::<(), Error>(()) + })?; let used_chunks = self.chunk_ctx.idx + 1; assert!( @@ -699,6 +743,16 @@ impl CircuitInputBuilder { "Used more chunks than given total_chunks" ); + self.chunks.iter().enumerate().for_each(|(id, chunk)| { + println!("chunk {}th ctx {:?}", id, chunk.ctx); + }); + assert!( + self.chunks.len() == self.chunk_ctx.idx + 1, + "number of chunks {} mis-match with chunk_ctx id {}", + self.chunks.len(), + self.chunk_ctx.idx + 1, + ); + // Truncate chunks to the actual used amount & correct ctx.total_chunks // Set length to the actual used amount of chunks self.chunks.truncate(self.chunk_ctx.idx + 1); @@ -747,47 +801,6 @@ fn push_op( } impl CircuitInputBuilder { - /// First part of handle_block, only called by fixed Builder - pub fn begin_handle_block( - &mut self, - eth_block: &EthBlock, - geth_traces: &[eth_types::GethExecTrace], - ) -> Result, Error> { - assert!( - self.circuits_params.max_rws().unwrap_or_default() > self.rws_reserve(), - "Fixed max_rws not enough for rws reserve" - ); - self.chunk_ctx.enable = true; - if !self.chunk_ctx.is_first_chunk() { - // Last step of previous chunk contains the same transition witness - // needed for current begin_chunk step - let last_step = &self - .prev_chunk() - .unwrap() - .end_chunk - .expect("Last chunk is incomplete"); - self.set_begin_chunk(last_step); - } - - // accumulates gas across all txs in the block - let mut last_call = None; - for (idx, tx) in eth_block.transactions.iter().enumerate() { - let geth_trace = &geth_traces[idx]; - // Transaction index starts from 1 - let tx_id = idx + 1; - last_call = self.handle_tx( - tx, - geth_trace, - tx_id == eth_block.transactions.len(), - tx_id as u64, - )?; - } - // set eth_block - self.block.eth_block = eth_block.clone(); - self.set_value_ops_call_context_rwc_eor(); - Ok(last_call) - } - /// pub fn rws_reserve(&self) -> usize { // This is the last chunk of a block, reserve for EndBlock, not EndChunk @@ -831,7 +844,7 @@ impl CircuitInputBuilder { * 2 + 4; // disabled and unused rows. - let max_rws = self.chunk_rws() + self.rws_reserve(); + let max_rws = >::into(self.block_ctx.rwc) - 1 + self.rws_reserve(); // Computing the number of rows for the EVM circuit requires the size of ExecStep, // which is determined in the code of zkevm-circuits and cannot be imported here. @@ -867,7 +880,6 @@ impl CircuitInputBuilder { let mut cib = self.clone(); cib.circuits_params.total_chunks = 1; cib.chunk_ctx.total_chunks = 1; - cib.chunk_ctx.enable = false; // accumulates gas across all txs in the block for (idx, tx) in eth_block.transactions.iter().enumerate() { let geth_trace = &geth_traces[idx]; @@ -884,6 +896,12 @@ impl CircuitInputBuilder { cib.block.eth_block = eth_block.clone(); cib.set_value_ops_call_context_rwc_eor(); + debug_assert!( + cib.chunk_ctx.idx == 0, + "processing {} > 1 chunk", + cib.chunk_ctx.idx + ); // dry run mode only one chunk + Ok(cib) } @@ -904,7 +922,8 @@ impl CircuitInputBuilder { // Calculate the chunkwise params from total number of chunks let total_chunks = self.circuits_params.total_chunks; target_params.total_chunks = total_chunks; - target_params.max_rws = (target_params.max_rws + 1) / total_chunks; + // count rws buffer here to left some space for extra virtual steps + target_params.max_rws = (target_params.max_rws + 1) / total_chunks + RW_BUFFER_SIZE; // Use a new builder with targeted params to handle the block // chunking context is set to dynamic so for the actual param is update per chunk diff --git a/bus-mapping/src/circuit_input_builder/chunk.rs b/bus-mapping/src/circuit_input_builder/chunk.rs index 6fbb9f4db1..30ace0d91b 100644 --- a/bus-mapping/src/circuit_input_builder/chunk.rs +++ b/bus-mapping/src/circuit_input_builder/chunk.rs @@ -42,8 +42,6 @@ pub struct ChunkContext { pub initial_copy_index: usize, /// pub end_copy_index: usize, - /// Druing dry run, chuncking is disabled - pub enable: bool, } impl Default for ChunkContext { @@ -65,7 +63,6 @@ impl ChunkContext { end_tx_index: 0, initial_copy_index: 0, end_copy_index: 0, - enable: true, } } @@ -81,7 +78,6 @@ impl ChunkContext { end_tx_index: 0, initial_copy_index: 0, end_copy_index: 0, - enable: true, } } diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index 89f1cbf1b9..63f6ba04a5 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -148,9 +148,13 @@ impl<'a> CircuitInputStateRef<'a> { /// Check whether rws will overflow circuit limit. pub fn check_rw_num_limit(&self) -> Result<(), Error> { if let Some(max_rws) = self.max_rws { - let rwc = self.block_ctx.rwc.0; + let rwc = self.chunk_ctx.rwc.0; if rwc > max_rws { - log::error!("rwc > max_rws, rwc={}, max_rws={}", rwc, max_rws); + log::error!( + "chunk inner rwc > max_rws, rwc={}, max_rws={}", + rwc, + max_rws + ); return Err(Error::RwsNotEnough(max_rws, rwc)); }; } diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 08c89791dd..608f360fbf 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -74,11 +74,69 @@ impl ExecutionGadget for EndChunkGadget { #[cfg(test)] mod test { - use crate::test_util::CircuitTestBuilder; - use bus_mapping::{circuit_input_builder::FixedCParams, operation::Target}; - use eth_types::bytecode; + use crate::{ + test_util::CircuitTestBuilder, + witness::{block_convert, chunk_convert}, + }; + use bus_mapping::{circuit_input_builder::FixedCParams, mock::BlockData, operation::Target}; + use eth_types::{address, bytecode, geth_types::GethData, Word}; + use halo2_proofs::halo2curves::bn256::Fr; use mock::TestContext; + #[test] + fn test_chunking_rwmap_logic() { + let bytecode = bytecode! { + GAS + STOP + }; + let addr_a = address!("0x000000000000000000000000000000000000AAAA"); + let addr_b = address!("0x000000000000000000000000000000000000BBBB"); + let test_ctx = TestContext::<2, 2>::new( + None, + |accs| { + accs[0] + .address(addr_b) + .balance(Word::from(1u64 << 20)) + .code(bytecode); + accs[1].address(addr_a).balance(Word::from(1u64 << 20)); + }, + |mut txs, accs| { + txs[0] + .from(accs[1].address) + .to(accs[0].address) + .gas(Word::from(1_000_000u64)); + txs[1] + .from(accs[1].address) + .to(accs[0].address) + .gas(Word::from(1_000_000u64)); + }, + |block, _tx| block.number(0xcafeu64), + ) + .unwrap(); + let block: GethData = test_ctx.into(); + let builder = BlockData::new_from_geth_data_with_params( + block.clone(), + FixedCParams { + total_chunks: 6, + max_rws: 64, + max_txs: 2, + ..Default::default() + }, + ) + .new_circuit_input_builder() + .handle_block(&block.eth_block, &block.geth_traces) + .unwrap(); + let block = block_convert::(&builder).unwrap(); + let chunks = chunk_convert(&block, &builder).unwrap(); + println!("num of chunk {:?}", chunks.len()); + chunks.iter().enumerate().for_each(|(idx, chunk)| { + println!( + "{}th chunk by_address_rw_fingerprints {:?}, chrono_rw_fingerprints {:?} ", + idx, chunk.by_address_rw_fingerprints, chunk.chrono_rw_fingerprints, + ); + }); + } + #[test] fn test_intermediate_single_chunk() { let bytecode = bytecode! { @@ -125,10 +183,12 @@ mod test { CircuitTestBuilder::new_from_test_ctx( TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(), ) - .params(FixedCParams { - total_chunks: 2, - max_rws: 60, - ..Default::default() + .params({ + FixedCParams { + total_chunks: 2, + max_rws: 60, + ..Default::default() + } }) .run_chunk(1); } diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index 0ad3907310..3a0681b23d 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -189,7 +189,7 @@ pub fn chunk_convert( // by_address_rws let start = min( chunk.ctx.idx * builder.circuits_params.max_rws, - by_address_rws.len() - 1, + by_address_rws.len(), ); let end = min( (chunk.ctx.idx + 1) * builder.circuits_params.max_rws, From 03d00ec2ec6f8c130fa94f3329a2608080f5c608 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Thu, 22 Feb 2024 20:47:52 +0800 Subject: [PATCH 08/16] more test: verify all chunks individually correctness correctly --- bus-mapping/src/circuit_input_builder.rs | 20 ++++--- .../src/evm_circuit/execution/end_chunk.rs | 60 ++++++++++++------- 2 files changed, 52 insertions(+), 28 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 62cb9704a3..ea49454a94 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -546,7 +546,6 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { next_copy_index: usize, last_call: Option, ) { - println!("commit_chunk_ctx"); self.chunk_ctx.end_rwc = self.block_ctx.rwc.0; self.chunk_ctx.end_tx_index = next_tx_index; self.chunk_ctx.end_copy_index = next_copy_index; @@ -627,6 +626,11 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { self.chunks[self.chunk_ctx.idx].padding = Some(padding); } + /// Get the i-th mutable chunk + pub fn get_chunk_mut(&mut self, i: usize) -> &mut Chunk { + self.chunks.get_mut(i).expect("Chunk does not exist") + } + /// Get the i-th chunk pub fn get_chunk(&self, i: usize) -> Chunk { self.chunks.get(i).expect("Chunk does not exist").clone() @@ -707,21 +711,23 @@ impl CircuitInputBuilder { let last_copy = self.block.copy_events.len(); - // We fill dummy virtual steps: BeginChunk,EndChunk for all lefted empty chunks + // TODO figure out and resolve generic param type and move fixed_param set inside + // commit_chunk_ctx After fixed, then we can set fixed_param on all chunks + (0..self.circuits_params.total_chunks()).for_each(|idx| { + self.get_chunk_mut(idx).fixed_param = self.circuits_params; + }); + + // We fill dummy virtual steps: BeginChunk,EndChunk for redundant chunks let last_process_chunk_id = self.chunk_ctx.idx; (last_process_chunk_id..self.circuits_params.total_chunks()).try_for_each(|idx| { - // TODO figure out and resolve generic param type and move fixed_param set inside - // commit_chunk_ctx After fixed, then we can set fixed_param on all chunks - self.cur_chunk_mut().fixed_param = self.circuits_params; - if idx == self.circuits_params.total_chunks() - 1 { + self.set_end_block()?; self.commit_chunk_ctx( false, eth_block.transactions.len(), last_copy, last_call.clone(), ); - self.set_end_block()?; } else { // it doent matter what step was put to set_end_chunk/set_begin_chunk on no-used // chunks before end_block. Just need to make sure it's step lookup is consistency. diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 608f360fbf..91acd9ed50 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -138,7 +138,7 @@ mod test { } #[test] - fn test_intermediate_single_chunk() { + fn test_all_chunks_ok() { let bytecode = bytecode! { PUSH1(0x0) // retLength PUSH1(0x0) // retOffset @@ -168,29 +168,47 @@ mod test { } #[test] - fn test_intermediate_single_chunk_fixed() { + fn test_all_chunks_fixed() { let bytecode = bytecode! { - PUSH1(0x0) // retLength - PUSH1(0x0) // retOffset - PUSH1(0x0) // argsLength - PUSH1(0x0) // argsOffset - PUSH1(0x0) // value - PUSH32(0x10_0000) // addr - PUSH32(0x10_0000) // gas - CALL - PUSH2(0xaa) + GAS + STOP }; - CircuitTestBuilder::new_from_test_ctx( - TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(), + let addr_a = address!("0x000000000000000000000000000000000000AAAA"); + let addr_b = address!("0x000000000000000000000000000000000000BBBB"); + let test_ctx = TestContext::<2, 2>::new( + None, + |accs| { + accs[0] + .address(addr_b) + .balance(Word::from(1u64 << 20)) + .code(bytecode); + accs[1].address(addr_a).balance(Word::from(1u64 << 20)); + }, + |mut txs, accs| { + txs[0] + .from(accs[1].address) + .to(accs[0].address) + .gas(Word::from(1_000_000u64)); + txs[1] + .from(accs[1].address) + .to(accs[0].address) + .gas(Word::from(1_000_000u64)); + }, + |block, _tx| block.number(0xcafeu64), ) - .params({ - FixedCParams { - total_chunks: 2, - max_rws: 60, - ..Default::default() - } - }) - .run_chunk(1); + .unwrap(); + (0..6).for_each(|chunk_id| { + CircuitTestBuilder::new_from_test_ctx(test_ctx.clone()) + .params({ + FixedCParams { + total_chunks: 6, + max_rws: 64, + max_txs: 2, + ..Default::default() + } + }) + .run_chunk(chunk_id); + }); } #[test] From a7802e079c4af001372896f4675d6b8046376b72 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Fri, 23 Feb 2024 16:30:43 +0800 Subject: [PATCH 09/16] allow zero limb diff in state_circuit lexicoordering --- bus-mapping/src/circuit_input_builder.rs | 17 +- zkevm-circuits/src/evm_circuit.rs | 2 +- .../src/evm_circuit/execution/end_chunk.rs | 48 ++---- zkevm-circuits/src/state_circuit.rs | 2 +- .../state_circuit/lexicographic_ordering.rs | 35 ++-- zkevm-circuits/src/state_circuit/test.rs | 33 ++-- zkevm-circuits/src/test_util.rs | 24 +-- zkevm-circuits/src/witness/block.rs | 11 +- zkevm-circuits/src/witness/chunk.rs | 152 +++++++----------- zkevm-circuits/src/witness/rw.rs | 84 ++++++---- 10 files changed, 186 insertions(+), 222 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index ea49454a94..95f7ae1ca6 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -666,7 +666,7 @@ impl CircuitInputBuilder { &mut self, eth_block: &EthBlock, geth_traces: &[eth_types::GethExecTrace], - ) -> Result<(ExecStep, Option), Error> { + ) -> Result<(Option, Option), Error> { assert!( self.circuits_params.max_rws().unwrap_or_default() > self.rws_reserve(), "Fixed max_rws not enough for rws reserve" @@ -687,13 +687,17 @@ impl CircuitInputBuilder { tx_id == eth_block.transactions.len(), tx_id as u64, ) + .map(|(exec_step, last_call)| (Some(exec_step), last_call)) }) - .collect::)>, _>>()?; - let res = res.remove(res.len() - 1); + .collect::, Option)>, _>>()?; // set eth_block self.block.eth_block = eth_block.clone(); self.set_value_ops_call_context_rwc_eor(); - Ok(res) + if !res.is_empty() { + Ok(res.remove(res.len() - 1)) + } else { + Ok((None, None)) + } } /// Handle a block by handling each transaction to generate all the @@ -706,6 +710,7 @@ impl CircuitInputBuilder { println!("--------------{:?}", self.circuits_params); // accumulates gas across all txs in the block let (last_step, last_call) = self.begin_handle_block(eth_block, geth_traces)?; + let last_step = last_step.unwrap_or_default(); assert!(self.circuits_params.max_rws().is_some()); @@ -748,10 +753,6 @@ impl CircuitInputBuilder { used_chunks <= self.circuits_params.total_chunks(), "Used more chunks than given total_chunks" ); - - self.chunks.iter().enumerate().for_each(|(id, chunk)| { - println!("chunk {}th ctx {:?}", id, chunk.ctx); - }); assert!( self.chunks.len() == self.chunk_ctx.idx + 1, "number of chunks {} mis-match with chunk_ctx id {}", diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index d8037ae570..9ee996fa8d 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -267,7 +267,7 @@ impl EvmCircuit { } /// Compute the public inputs for this circuit. - fn instance_extend_chunk_ctx(&self) -> Vec> { + pub fn instance_extend_chunk_ctx(&self) -> Vec> { let chunk = self.chunk.as_ref().unwrap(); let (rw_table_chunked_index, rw_table_total_chunks) = diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 91acd9ed50..57132acead 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -117,7 +117,7 @@ mod test { let builder = BlockData::new_from_geth_data_with_params( block.clone(), FixedCParams { - total_chunks: 6, + total_chunks: 4, max_rws: 64, max_txs: 2, ..Default::default() @@ -128,43 +128,13 @@ mod test { .unwrap(); let block = block_convert::(&builder).unwrap(); let chunks = chunk_convert(&block, &builder).unwrap(); - println!("num of chunk {:?}", chunks.len()); - chunks.iter().enumerate().for_each(|(idx, chunk)| { - println!( - "{}th chunk by_address_rw_fingerprints {:?}, chrono_rw_fingerprints {:?} ", - idx, chunk.by_address_rw_fingerprints, chunk.chrono_rw_fingerprints, - ); - }); - } - - #[test] - fn test_all_chunks_ok() { - let bytecode = bytecode! { - PUSH1(0x0) // retLength - PUSH1(0x0) // retOffset - PUSH1(0x0) // argsLength - PUSH1(0x0) // argsOffset - PUSH1(0x0) // value - PUSH32(0x10_0000) // addr - PUSH32(0x10_0000) // gas - CALL - PUSH2(0xaa) - }; - CircuitTestBuilder::new_from_test_ctx( - TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(), - ) - .block_modifier(Box::new(move |_block, chunk| { - // TODO FIXME padding start as a workaround. The practical should be last chunk last row - // rws - // if let Some(a) = chunk.rws.0.get_mut(&Target::Start) { - // a.push(Rw::Start { rw_counter: 1 }); - // } - println!( - "=> FIXME is fixed? {:?}", - chunk.chrono_rws.0.get_mut(&Target::Start) - ); - })) - .run_dynamic_chunk(4, 2); + // assert last fingerprint acc are equal + if let Some(last_chunk) = chunks.last() { + assert_eq!( + last_chunk.by_address_rw_fingerprints.mul_acc, + last_chunk.chrono_rw_fingerprints.mul_acc + ) + } } #[test] @@ -202,7 +172,7 @@ mod test { .params({ FixedCParams { total_chunks: 6, - max_rws: 64, + max_rws: 90, max_txs: 2, ..Default::default() } diff --git a/zkevm-circuits/src/state_circuit.rs b/zkevm-circuits/src/state_circuit.rs index 31657cdfe7..7f302a6c61 100644 --- a/zkevm-circuits/src/state_circuit.rs +++ b/zkevm-circuits/src/state_circuit.rs @@ -484,7 +484,7 @@ impl StateCircuit { permu_alpha: chunk.permu_alpha, permu_gamma: chunk.permu_gamma, rw_fingerprints: chunk.by_address_rw_fingerprints.clone(), - prev_chunk_last_rw: chunk.prev_chunk_last_chrono_rw, + prev_chunk_last_rw: chunk.prev_chunk_last_by_address_rw, _marker: PhantomData::default(), } } diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index 062a85cb00..cd81f6752e 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -99,7 +99,7 @@ pub struct Config { pub(crate) selector: Column, pub first_different_limb: BinaryNumberConfig, limb_difference: Column, - limb_difference_inverse: Column, + // limb_difference_inverse: Column, } impl Config { @@ -112,26 +112,26 @@ impl Config { let selector = meta.fixed_column(); let first_different_limb = BinaryNumberChip::configure(meta, selector, None); let limb_difference = meta.advice_column(); - let limb_difference_inverse = meta.advice_column(); + // let limb_difference_inverse = meta.advice_column(); let config = Config { selector, first_different_limb, limb_difference, - limb_difference_inverse, + // limb_difference_inverse, }; lookup.range_check_u16(meta, "limb_difference fits into u16", |meta| { meta.query_advice(limb_difference, Rotation::cur()) }); - meta.create_gate("limb_difference is not zero", |meta| { - let selector = meta.query_fixed(selector, Rotation::cur()); - let limb_difference = meta.query_advice(limb_difference, Rotation::cur()); - let limb_difference_inverse = - meta.query_advice(limb_difference_inverse, Rotation::cur()); - vec![selector * (1.expr() - limb_difference * limb_difference_inverse)] - }); + // meta.create_gate("limb_difference is not zero", |meta| { + // let selector = meta.query_fixed(selector, Rotation::cur()); + // let limb_difference = meta.query_advice(limb_difference, Rotation::cur()); + // let limb_difference_inverse = + // meta.query_advice(limb_difference_inverse, Rotation::cur()); + // vec![selector * (1.expr() - limb_difference * limb_difference_inverse)] + // }); meta.create_gate( "limb differences before first_different_limb are all 0", @@ -221,24 +221,15 @@ impl Config { offset, || Value::known(limb_difference), )?; - region.assign_advice( - || "limb_difference_inverse", - self.limb_difference_inverse, - offset, - || Value::known(limb_difference.invert().unwrap()), - )?; Ok(index) } /// Annotates columns of this gadget embedded within a circuit region. pub fn annotate_columns_in_region(&self, region: &mut Region, prefix: &str) { - [ - (self.limb_difference, "LO_limb_difference"), - (self.limb_difference_inverse, "LO_limb_difference_inverse"), - ] - .iter() - .for_each(|(col, ann)| region.name_column(|| format!("{}_{}", prefix, ann), *col)); + [(self.limb_difference, "LO_limb_difference")] + .iter() + .for_each(|(col, ann)| region.name_column(|| format!("{}_{}", prefix, ann), *col)); // fixed column region.name_column( || format!("{}_LO_upper_limb_difference", prefix), diff --git a/zkevm-circuits/src/state_circuit/test.rs b/zkevm-circuits/src/state_circuit/test.rs index a00b45c798..3cf36b1ed3 100644 --- a/zkevm-circuits/src/state_circuit/test.rs +++ b/zkevm-circuits/src/state_circuit/test.rs @@ -34,6 +34,26 @@ fn state_circuit_unusable_rows() { ) } +fn new_chunk_from_rw_map(rws: &RwMap, padding_start_rw: Option) -> Chunk { + let (alpha, gamma) = get_permutation_randomness(); + let mut chunk = Chunk { + by_address_rws: rws.clone(), + ..Default::default() + }; + + let rw_fingerprints = get_permutation_fingerprint_of_rwmap( + &chunk.by_address_rws, + chunk.fixed_param.max_rws, + alpha, + gamma, + F::from(1), + false, + padding_start_rw, + ); + chunk.by_address_rw_fingerprints = rw_fingerprints; + chunk +} + fn test_state_circuit_ok( memory_ops: Vec>, stack_ops: Vec>, @@ -45,7 +65,7 @@ fn test_state_circuit_ok( storage: storage_ops, ..Default::default() }); - let chunk = Chunk::new_from_rw_map(&rw_map, None, None); + let chunk = new_chunk_from_rw_map(&rw_map, None); let circuit = StateCircuit::::new(&chunk); let instance = circuit.instance(); @@ -69,7 +89,7 @@ fn verifying_key_independent_of_rw_length() { let no_rows = StateCircuit::::new(&chunk); - chunk = Chunk::new_from_rw_map( + chunk = new_chunk_from_rw_map( &RwMap::from(&OperationContainer { memory: vec![Operation::new( RWCounter::from(1), @@ -80,7 +100,6 @@ fn verifying_key_independent_of_rw_length() { ..Default::default() }), None, - None, ); let one_row = StateCircuit::::new(&chunk); @@ -948,11 +967,7 @@ fn variadic_size_check() { }, ]; // let rw_map: RwMap = rows.clone().into(); - let circuit = StateCircuit::new(&Chunk::new_from_rw_map( - &RwMap::from(rows.clone()), - None, - None, - )); + let circuit = StateCircuit::new(&new_chunk_from_rw_map(&RwMap::from(rows.clone()), None)); let power_of_randomness = circuit.instance(); let prover1 = MockProver::::run(17, &circuit, power_of_randomness).unwrap(); @@ -973,7 +988,7 @@ fn variadic_size_check() { }, ]); - let circuit = StateCircuit::new(&Chunk::new_from_rw_map(&rows.into(), None, None)); + let circuit = StateCircuit::new(&new_chunk_from_rw_map(&rows.into(), None)); let power_of_randomness = circuit.instance(); let prover2 = MockProver::::run(17, &circuit, power_of_randomness).unwrap(); diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 9617ad38ec..e26999c7c7 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -79,7 +79,6 @@ const NUM_BLINDING_ROWS: usize = 64; /// /// CircuitTestBuilder::new_from_test_ctx(ctx) /// .block_modifier(Box::new(|block, chunk| chunk.fixed_param.max_evm_rows = (1 << 18) - 100)) -/// .state_checks(Box::new(|prover, evm_rows, lookup_rows| assert!(prover.verify_at_rows_par(evm_rows.iter().cloned(), lookup_rows.iter().cloned()).is_err()))) /// .run(); /// ``` pub struct CircuitTestBuilder { @@ -327,12 +326,14 @@ impl CircuitTestBuilder { "Total chunks unmatched with fixed param" ); BlockData::new_from_geth_data_with_params(block.clone(), fixed_param) - .new_circuit_input_builder() + .new_circuit_input_builder_with_feature( + self.feature_config.unwrap_or_default(), + ) .handle_block(&block.eth_block, &block.geth_traces) .unwrap() } None => BlockData::new_from_geth_data_chunked(block.clone(), total_chunk) - .new_circuit_input_builder() + .new_circuit_input_builder_with_feature(self.feature_config.unwrap_or_default()) .handle_block(&block.eth_block, &block.geth_traces) .unwrap(), }; @@ -352,12 +353,11 @@ impl CircuitTestBuilder { // Build a witness block from trace result. let mut block = crate::witness::block_convert(&builder).unwrap(); + let mut chunk = crate::witness::chunk_convert(&block, &builder) .unwrap() .remove(chunk_index); - println!("fingerprints = {:?}", chunk.chrono_rw_fingerprints); - for modifier_fn in self.block_modifiers { modifier_fn.as_ref()(&mut block, &mut chunk); } @@ -374,10 +374,16 @@ impl CircuitTestBuilder { let (_active_gate_rows, _active_lookup_rows) = EvmCircuit::::get_active_rows(&block, &chunk); - let circuit = - EvmCircuitCached::get_test_circuit_from_block(block.clone(), chunk.clone()); - let instance = circuit.instance(); - let _prover = MockProver::::run(k, &circuit, instance).unwrap(); + let _prover = if block.feature_config.is_mainnet() { + let circuit = + EvmCircuitCached::get_test_circuit_from_block(block.clone(), chunk.clone()); + let instance = circuit.instance(); + MockProver::::run(k, &circuit, instance) + } else { + let circuit = EvmCircuit::get_test_circuit_from_block(block.clone(), chunk.clone()); + let instance = circuit.instance_extend_chunk_ctx(); + MockProver::::run(k, &circuit, instance) + }; // self.evm_checks.as_ref()(prover, &active_gate_rows, &active_lookup_rows) } diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index d62a0700ec..d79641cba9 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -295,9 +295,14 @@ pub fn block_convert( .chunks .iter() .fold(BTreeMap::new(), |mut map, chunk| { - assert!(chunk.ctx.rwc.0.saturating_sub(1) <= builder.circuits_params.max_rws); - // [chunk.ctx.rwc.0, builder.circuits_params.max_rws + 1) - (chunk.ctx.rwc.0..builder.circuits_params.max_rws + 1).for_each(|padding_rw_counter| { + assert!( + chunk.ctx.rwc.0.saturating_sub(1) <= builder.circuits_params.max_rws, + "max_rws size {} must larger than chunk rws size {}", + builder.circuits_params.max_rws, + chunk.ctx.rwc.0.saturating_sub(1), + ); + // [chunk.ctx.rwc.0, builder.circuits_params.max_rws) + (chunk.ctx.rwc.0..builder.circuits_params.max_rws).for_each(|padding_rw_counter| { *map.entry(padding_rw_counter).or_insert(0) += 1; }); map diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index 3a0681b23d..15127e172a 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -1,4 +1,4 @@ -use std::{cmp::min, iter}; +use std::iter; /// use super::{ @@ -8,6 +8,7 @@ use super::{ use crate::util::unwrap_value; use bus_mapping::{ circuit_input_builder::{self, Call, ChunkContext, FixedCParams}, + operation::Target, Error, }; use eth_types::Field; @@ -75,40 +76,6 @@ impl Default for Chunk { } } -impl Chunk { - #[cfg(test)] - pub(crate) fn new_from_rw_map( - rws: &RwMap, - padding_start_rw: Option, - chrono_padding_start_rw: Option, - ) -> Self { - let (alpha, gamma) = get_permutation_randomness(); - let mut chunk = Chunk::default(); - let rw_fingerprints = get_permutation_fingerprint_of_rwmap( - rws, - chunk.fixed_param.max_rws, - alpha, - gamma, - F::from(1), - false, - padding_start_rw, - ); - let chrono_rw_fingerprints = get_permutation_fingerprint_of_rwmap( - rws, - chunk.fixed_param.max_rws, - alpha, - gamma, - F::from(1), - true, - chrono_padding_start_rw, - ); - chunk.chrono_rws = rws.clone(); - chunk.by_address_rw_fingerprints = rw_fingerprints; - chunk.chrono_rw_fingerprints = chrono_rw_fingerprints; - chunk - } -} - /// Convert the idx-th chunk struct in bus-mapping to a witness chunk used in circuits pub fn chunk_convert( block: &Block, @@ -127,52 +94,20 @@ pub fn chunk_convert( .enumerate() { let chunk = chunk.unwrap(); // current chunk always there - let (prev_chunk_last_chrono_rw, prev_chunk_last_by_address_rw) = prev_chunk - .map(|prev_chunk| { - let prev_chunk_last_chrono_rw = { - assert!(builder.circuits_params.max_rws > 0); - let chunk_inner_rwc = prev_chunk.ctx.rwc.0; - if chunk_inner_rwc.saturating_sub(1) == builder.circuits_params.max_rws { - // if prev chunk rws are full, then get the last rwc - RwMap::get_rw(&builder.block.container, prev_chunk.ctx.end_rwc - 1) - .expect("Rw does not exist") - } else { - // last is the padding row - Rw::Padding { - rw_counter: builder.circuits_params.max_rws, - } - } - }; - // get prev_chunk last by_address sorted rw - let prev_chunk_by_address_last_rw = { - let by_address_last_rwc_index = - (prev_chunk.ctx.idx + 1) * builder.circuits_params.max_rws; - let prev_chunk_by_address_last_rwc = by_address_rws - .get(by_address_last_rwc_index - 1) - .cloned() - .or_else(|| { - let target_padding_count = - (by_address_last_rwc_index + 1) - by_address_rws.len(); - let mut accu_count = 0; - padding_meta - .iter() - .find(|(_, count)| { - accu_count += **count; - target_padding_count <= accu_count.try_into().unwrap() - }) - .map(|(padding_rwc, _)| Rw::Padding { - rw_counter: *padding_rwc, - }) - }); - - prev_chunk_by_address_last_rwc - }; - (prev_chunk_last_chrono_rw, prev_chunk_by_address_last_rw) - }) - .map(|(prev_chunk_last_rwc, prev_chunk_by_address_last_rwc)| { - (Some(prev_chunk_last_rwc), prev_chunk_by_address_last_rwc) - }) - .unwrap_or_else(|| (None, None)); + let prev_chunk_last_chrono_rw = prev_chunk.map(|prev_chunk| { + assert!(builder.circuits_params.max_rws > 0); + let chunk_inner_rwc = prev_chunk.ctx.rwc.0; + if chunk_inner_rwc.saturating_sub(1) == builder.circuits_params.max_rws { + // if prev chunk rws are full, then get the last rwc + RwMap::get_rw(&builder.block.container, prev_chunk.ctx.end_rwc - 1) + .expect("Rw does not exist") + } else { + // last is the padding row + Rw::Padding { + rw_counter: builder.circuits_params.max_rws - 1, + } + } + }); println!( "| {:?} ... {:?} | @chunk_convert", @@ -180,24 +115,47 @@ pub fn chunk_convert( ); // Get the rws in the i-th chunk - let chrono_rws = RwMap::from(&builder.block.container) - .take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc); - - chrono_rws.check_value(); + let chrono_rws = { + let mut chrono_rws = RwMap::from(&builder.block.container); + // remove paading here since it will be attached later + if let Some(padding_vec) = chrono_rws.0.get_mut(&Target::Padding) { + padding_vec.clear() + } + chrono_rws.take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc) + }; - let by_address_rws = RwMap::from({ + let (prev_chunk_last_by_address_rw, by_address_rws) = { // by_address_rws - let start = min( - chunk.ctx.idx * builder.circuits_params.max_rws, - by_address_rws.len(), - ); - let end = min( - (chunk.ctx.idx + 1) * builder.circuits_params.max_rws, - by_address_rws.len(), - ); - by_address_rws[start..end].to_vec() - }) - .take_rw_counter_range(chunk.ctx.initial_rwc, chunk.ctx.end_rwc); + let start = chunk.ctx.idx * builder.circuits_params.max_rws; + let size = builder.circuits_params.max_rws; + // by_address_rws[start..end].to_vec() + + let skipped = by_address_rws + .iter() + // remove paading here since it will be attached later + .filter(|rw| rw.tag() != Target::Padding) + .cloned() // TODO avoid clone here + .chain(padding_meta.iter().flat_map(|(k, v)| { + vec![ + Rw::Padding { rw_counter: *k }; + >::try_into(*v).unwrap() + ] + })); + // there is no previous chunk + if start == 0 { + (None, RwMap::from(skipped.take(size).collect::>())) + } else { + // here have `chunk.ctx.idx - 1` because each chunk first row are probagated from + // prev chunk. giving idx>0 th chunk, there will be (idx-1) placeholders cant' count + // in real order + let mut skipped = skipped.skip(start - 1 - (chunk.ctx.idx - 1)); + let prev_chunk_last_by_address_rw = skipped.next(); + ( + prev_chunk_last_by_address_rw, + RwMap::from(skipped.take(size).collect::>()), + ) + } + }; // Comupute cur fingerprints from last fingerprints and current Rw rows let by_address_rw_fingerprints = get_permutation_fingerprint_of_rwmap( diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index 907472b900..06120f95a3 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -1,5 +1,8 @@ //! The Read-Write table related structs -use std::{collections::HashMap, iter}; +use std::{ + collections::{HashMap, HashSet}, + iter, +}; use bus_mapping::{ exec_trace::OperationRef, @@ -116,8 +119,9 @@ impl RwMap { } /// Calculates the number of Rw::Padding rows needed. /// `target_len` is allowed to be 0 as an "auto" mode, + /// return padding size also allow to be 0, means no padding pub(crate) fn padding_len(rows_len: usize, target_len: usize) -> usize { - if target_len > rows_len { + if target_len >= rows_len { target_len - rows_len } else { if target_len != 0 { @@ -135,10 +139,17 @@ impl RwMap { target_len: usize, padding_start_rw: Option, ) -> (Vec, usize) { + let mut padding_exist = HashSet::new(); // Remove Start/Padding rows as we will add them from scratch. let rows_trimmed: Vec = rows .iter() - .filter(|rw| !matches!(rw, Rw::Start { .. } | Rw::Padding { .. })) + .filter(|rw| { + if let Rw::Padding { rw_counter } = rw { + padding_exist.insert(*rw_counter); + } + + !matches!(rw, Rw::Start { .. }) + }) .cloned() .collect(); let padding_length = { @@ -146,25 +157,23 @@ impl RwMap { length.saturating_sub(1) }; - // option 1: need to provide padding starting rw_counter at function parameters - // option 2: just padding after local max rw_counter + 1 - // We adapt option 2 for now - // the side effect is it introduce malleable proof when append `Rw::Padding` rw_counter, - // because `Rw::Padding` is not global unique - let start_padding_rw_counter = rows_trimmed - .iter() - .map(|rw| rw.rw_counter()) - .max() - .unwrap_or(1) - + 1; + // padding rw_counter starting from + // +1 for to including padding_start row + let start_padding_rw_counter = rows_trimmed.len() + 1; - let padding = (start_padding_rw_counter..start_padding_rw_counter + padding_length) - .map(|rw_counter| Rw::Padding { rw_counter }); + let padding = (start_padding_rw_counter..).flat_map(|rw_counter| { + if padding_exist.contains(&rw_counter) { + None + } else { + Some(Rw::Padding { rw_counter }) + } + }); ( iter::empty() .chain([padding_start_rw.unwrap_or(Rw::Start { rw_counter: 1 })]) .chain(rows_trimmed.into_iter()) .chain(padding.into_iter()) + .take(target_len) .collect(), padding_length, ) @@ -191,9 +200,9 @@ impl RwMap { } /// take only rw_counter within range - pub fn take_rw_counter_range(mut self, start: usize, end: usize) -> Self { + pub fn take_rw_counter_range(mut self, start_rwc: usize, end_rwc: usize) -> Self { for rw in self.0.values_mut() { - rw.retain(|r| r.rw_counter() >= start && r.rw_counter() < end) + rw.retain(|r| r.rw_counter() >= start_rwc && r.rw_counter() < end_rwc) } self } @@ -428,13 +437,22 @@ impl RwRow> { target_len: usize, padding_start_rwrow: Option>>, ) -> (Vec>>, usize) { + let mut padding_exist = HashSet::new(); // Remove Start/Padding rows as we will add them from scratch. let rows_trimmed = rows .iter() .filter(|rw| { let tag = unwrap_value(rw.tag); - !(tag == F::from(Target::Start as u64) || tag == F::from(Target::Padding as u64)) - && tag != F::ZERO // 0 is invalid tag + + if tag == F::from(Target::Padding as u64) { + let rw_counter = u64::from_le_bytes( + unwrap_value(rw.rw_counter).to_repr()[..U64_BYTES] + .try_into() + .unwrap(), + ); + padding_exist.insert(rw_counter); + } + tag != F::from(Target::Start as u64) && tag != F::ZERO // 0 is invalid tag }) .cloned() .collect::>>>(); @@ -443,12 +461,7 @@ impl RwRow> { length.saturating_sub(1) // first row always got padding }; let start_padding_rw_counter = { - let start_padding_rw_counter = rows_trimmed - .iter() - .map(|rw| unwrap_value(rw.rw_counter)) - .max() - .unwrap_or(F::from(1u64)) - + F::ONE; + let start_padding_rw_counter = F::from(rows_trimmed.len() as u64) + F::ONE; // Assume root of unity < 2**64 assert!( start_padding_rw_counter.to_repr()[U64_BYTES..] @@ -465,13 +478,17 @@ impl RwRow> { ) } as usize; - let padding = (start_padding_rw_counter..start_padding_rw_counter + padding_length).map( - |rw_counter| RwRow { - rw_counter: Value::known(F::from(rw_counter as u64)), - tag: Value::known(F::from(Target::Padding as u64)), - ..Default::default() - }, - ); + let padding = (start_padding_rw_counter..).flat_map(|rw_counter| { + if padding_exist.contains(&rw_counter.try_into().unwrap()) { + None + } else { + Some(RwRow { + rw_counter: Value::known(F::from(rw_counter as u64)), + tag: Value::known(F::from(Target::Padding as u64)), + ..Default::default() + }) + } + }); ( iter::once(padding_start_rwrow.unwrap_or(RwRow { rw_counter: Value::known(F::ONE), @@ -480,6 +497,7 @@ impl RwRow> { })) .chain(rows_trimmed.into_iter()) .chain(padding.into_iter()) + .take(target_len) .collect(), padding_length, ) From 594e7a5cd97afd54406082319f0e2962728a4636 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Sat, 24 Feb 2024 23:00:02 +0800 Subject: [PATCH 10/16] revert instance_extend_chunk_ctx from evm_circuit and unify to instance instead --- zkevm-circuits/src/evm_circuit.rs | 56 ++++++++----------- .../src/evm_circuit/execution/end_chunk.rs | 2 +- zkevm-circuits/src/super_circuit.rs | 4 +- zkevm-circuits/src/test_util.rs | 2 +- 4 files changed, 29 insertions(+), 35 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index 9ee996fa8d..e50b619fe3 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -265,26 +265,6 @@ impl EvmCircuit { // It must have one row for EndBlock/EndChunk and at least one unused one num_rows + 2 } - - /// Compute the public inputs for this circuit. - pub fn instance_extend_chunk_ctx(&self) -> Vec> { - let chunk = self.chunk.as_ref().unwrap(); - - let (rw_table_chunked_index, rw_table_total_chunks) = - (chunk.chunk_context.idx, chunk.chunk_context.total_chunks); - - let mut instance = vec![vec![ - F::from(rw_table_chunked_index as u64), - F::from(rw_table_chunked_index as u64) + F::ONE, - F::from(rw_table_total_chunks as u64), - F::from(chunk.chunk_context.initial_rwc as u64), - F::from(chunk.chunk_context.end_rwc as u64), - ]]; - - instance.extend(self.instance()); - - instance - } } impl SubCircuit for EvmCircuit { @@ -392,14 +372,26 @@ impl SubCircuit for EvmCircuit { fn instance(&self) -> Vec> { let chunk = self.chunk.as_ref().unwrap(); - vec![vec![ - chunk.permu_alpha, - chunk.permu_gamma, - chunk.chrono_rw_fingerprints.prev_ending_row, - chunk.chrono_rw_fingerprints.ending_row, - chunk.chrono_rw_fingerprints.prev_mul_acc, - chunk.chrono_rw_fingerprints.mul_acc, - ]] + let (rw_table_chunked_index, rw_table_total_chunks) = + (chunk.chunk_context.idx, chunk.chunk_context.total_chunks); + + vec![ + vec![ + F::from(rw_table_chunked_index as u64), + F::from(rw_table_chunked_index as u64) + F::ONE, + F::from(rw_table_total_chunks as u64), + F::from(chunk.chunk_context.initial_rwc as u64), + F::from(chunk.chunk_context.end_rwc as u64), + ], + vec![ + chunk.permu_alpha, + chunk.permu_gamma, + chunk.chrono_rw_fingerprints.prev_ending_row, + chunk.chrono_rw_fingerprints.ending_row, + chunk.chrono_rw_fingerprints.prev_mul_acc, + chunk.chrono_rw_fingerprints.mul_acc, + ], + ] } } @@ -484,7 +476,7 @@ pub(crate) mod cached { } pub(crate) fn instance(&self) -> Vec> { - self.0.instance_extend_chunk_ctx() + self.0.instance() } } } @@ -701,7 +693,7 @@ mod evm_circuit_stats { let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); let k = block.get_test_degree(&chunk); let circuit = EvmCircuit::::get_test_circuit_from_block(block, chunk); - let instance = circuit.instance_extend_chunk_ctx(); + let instance = circuit.instance(); let prover1 = MockProver::::run(k, &circuit, instance).unwrap(); let res = prover1.verify(); if let Err(err) = res { @@ -727,7 +719,7 @@ mod evm_circuit_stats { let k = block.get_test_degree(&chunk); let circuit = EvmCircuit::::get_test_circuit_from_block(block, chunk); - let instance = circuit.instance_extend_chunk_ctx(); + let instance = circuit.instance(); let prover1 = MockProver::::run(k, &circuit, instance).unwrap(); let code = bytecode! { @@ -749,7 +741,7 @@ mod evm_circuit_stats { let chunk = chunk_convert::(&block, &builder).unwrap().remove(0); let k = block.get_test_degree(&chunk); let circuit = EvmCircuit::::get_test_circuit_from_block(block, chunk); - let instance = circuit.instance_extend_chunk_ctx(); + let instance = circuit.instance(); let prover2 = MockProver::::run(k, &circuit, instance).unwrap(); assert_eq!(prover1.fixed().len(), prover2.fixed().len()); diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 57132acead..4231a06cd3 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -78,7 +78,7 @@ mod test { test_util::CircuitTestBuilder, witness::{block_convert, chunk_convert}, }; - use bus_mapping::{circuit_input_builder::FixedCParams, mock::BlockData, operation::Target}; + use bus_mapping::{circuit_input_builder::FixedCParams, mock::BlockData}; use eth_types::{address, bytecode, geth_types::GethData, Word}; use halo2_proofs::halo2curves::bn256::Fr; use mock::TestContext; diff --git a/zkevm-circuits/src/super_circuit.rs b/zkevm-circuits/src/super_circuit.rs index c19a9247d7..09635c80ed 100644 --- a/zkevm-circuits/src/super_circuit.rs +++ b/zkevm-circuits/src/super_circuit.rs @@ -426,7 +426,9 @@ impl SubCircuit for SuperCircuit { instance.extend_from_slice(&self.copy_circuit.instance()); instance.extend_from_slice(&self.state_circuit.instance()); instance.extend_from_slice(&self.exp_circuit.instance()); - instance.extend_from_slice(&self.evm_circuit.instance()); + // remove first vector which is chunk_ctx + // which supercircuit already supply globally on top + instance.extend_from_slice(&self.evm_circuit.instance()[1..]); instance } diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index e26999c7c7..b594fadeb4 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -381,7 +381,7 @@ impl CircuitTestBuilder { MockProver::::run(k, &circuit, instance) } else { let circuit = EvmCircuit::get_test_circuit_from_block(block.clone(), chunk.clone()); - let instance = circuit.instance_extend_chunk_ctx(); + let instance = circuit.instance(); MockProver::::run(k, &circuit, instance) }; From 8aaaa0d60d3c4bf7b8943a24e61cce3c5d2338c8 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Sun, 25 Feb 2024 00:05:41 +0800 Subject: [PATCH 11/16] debug evm fixed column variadic issue --- Cargo.lock | 1 + integration-tests/Cargo.toml | 1 + .../src/integration_test_circuits.rs | 43 +++++++++++++++++-- 3 files changed, 41 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index df4c71e864..f8eaffb421 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2179,6 +2179,7 @@ dependencies = [ "ethers-contract-abigen", "glob", "halo2_proofs", + "itertools 0.10.5", "lazy_static", "log", "mock", diff --git a/integration-tests/Cargo.toml b/integration-tests/Cargo.toml index 068341dcdf..d9acb79d01 100644 --- a/integration-tests/Cargo.toml +++ b/integration-tests/Cargo.toml @@ -24,6 +24,7 @@ rand_chacha = "0.3" paste = "1.0" rand_xorshift = "0.3.0" rand_core = "0.6.4" +itertools = "0.10" mock = { path = "../mock" } [dev-dependencies] diff --git a/integration-tests/src/integration_test_circuits.rs b/integration-tests/src/integration_test_circuits.rs index 3cab71fda2..49349f434f 100644 --- a/integration-tests/src/integration_test_circuits.rs +++ b/integration-tests/src/integration_test_circuits.rs @@ -25,6 +25,7 @@ use halo2_proofs::{ }, }, }; +use itertools::Itertools; use lazy_static::lazy_static; use mock::TestContext; use rand_chacha::rand_core::SeedableRng; @@ -358,10 +359,26 @@ impl + Circuit> IntegrationTest { let fixed = mock_prover.fixed(); if let Some(prev_fixed) = self.fixed.clone() { - assert!( - fixed.eq(&prev_fixed), - "circuit fixed columns are not constant for different witnesses" - ); + fixed + .iter() + .enumerate() + .zip_eq(prev_fixed.iter()) + .for_each(|((index, col1), col2)| { + if !col1.eq(col2) { + println!("on column index {} not equal", index); + col1.iter().enumerate().zip_eq(col2.iter()).for_each( + |((index, cellv1), cellv2)| { + assert!( + cellv1.eq(&cellv2), + "cellv1 {:?} != cellv2 {:?} on index {}", + cellv1, + cellv2, + index + ); + }, + ); + } + }); } else { self.fixed = Some(fixed.clone()); } @@ -383,6 +400,24 @@ impl + Circuit> IntegrationTest { match self.root_fixed.clone() { Some(prev_fixed) => { + fixed.iter().enumerate().zip_eq(prev_fixed.iter()).for_each( + |((index, col1), col2)| { + if !col1.eq(col2) { + println!("on column index {} not equal", index); + col1.iter().enumerate().zip_eq(col2.iter()).for_each( + |((index, cellv1), cellv2)| { + assert!( + cellv1.eq(&cellv2), + "cellv1 {:?} != cellv2 {:?} on index {}", + cellv1, + cellv2, + index + ); + }, + ); + } + }, + ); assert!( fixed.eq(&prev_fixed), "root circuit fixed columns are not constant for different witnesses" From 1ca62a0d31bc98218f1ab2f3b81cfa85f3ca976b Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Thu, 29 Feb 2024 10:17:29 +0800 Subject: [PATCH 12/16] add more tests related intermediate chunk and fixed bugs --- bus-mapping/src/circuit_input_builder.rs | 113 ++++-- .../src/integration_test_circuits.rs | 4 +- testool/src/statetest/executor.rs | 9 +- zkevm-circuits/src/evm_circuit.rs | 4 +- zkevm-circuits/src/evm_circuit/execution.rs | 56 ++- .../src/evm_circuit/execution/begin_chunk.rs | 7 +- .../src/evm_circuit/execution/callop.rs | 9 +- .../src/evm_circuit/execution/end_block.rs | 4 +- .../src/evm_circuit/execution/end_chunk.rs | 68 ++-- .../src/evm_circuit/execution/end_tx.rs | 2 +- zkevm-circuits/src/evm_circuit/step.rs | 5 + .../src/evm_circuit/util/common_gadget.rs | 2 + zkevm-circuits/src/test_util.rs | 348 +++++++++--------- zkevm-circuits/src/witness/chunk.rs | 13 +- 14 files changed, 370 insertions(+), 274 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 95f7ae1ca6..da87f78811 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -357,7 +357,7 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { // Optain the first op of the next GethExecStep, for fixed case also lookahead let (mut cib, mut tx, mut tx_ctx) = (self.clone(), tx, tx_ctx); let mut cib_ref = cib.state_ref(&mut tx, &mut tx_ctx); - let next_ops = if let Some((i, step)) = next_geth_step { + let mut next_ops = if let Some((i, step)) = next_geth_step { log::trace!("chunk at {}th opcode {:?} ", i, step.op); gen_associated_ops(&step.op, &mut cib_ref, &geth_trace.struct_logs[i..])?.remove(0) } else { @@ -368,10 +368,14 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { let last_copy = self.block.copy_events.len(); // Generate EndChunk and proceed to the next if it's not the last chunk // Set next step pre-state as end_chunk state - self.set_end_chunk(&next_ops); + self.set_end_chunk(&next_ops, Some(&tx)); + + // need to update next_ops.rwc to catch block_ctx.rwc in `set_end_chunk` + next_ops.rwc = self.block_ctx.rwc; + // tx.id start from 1, so it's equivalent to `next_tx_index` self.commit_chunk_ctx(true, tx.id as usize, last_copy, last_call); - self.set_begin_chunk(&next_ops); + self.set_begin_chunk(&next_ops, Some(&tx)); Ok(()) } @@ -394,13 +398,12 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { let mut tx_ctx = TransactionContext::new(eth_tx, geth_trace, is_last_tx)?; let res = if !geth_trace.invalid { - let mut last_call = None; - // Generate BeginTx step let begin_tx_step = gen_associated_steps( &mut self.state_ref(&mut tx, &mut tx_ctx), ExecState::BeginTx, )?; + let mut last_call = Some(tx.calls().get(begin_tx_step.call_index).unwrap().clone()); tx.steps_mut().push(begin_tx_step); let mut trace = geth_trace.struct_logs.iter().enumerate().peekable(); @@ -416,9 +419,10 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { // Proceed to the next step let (i, step) = trace.next().expect("Peeked step should exist"); log::trace!( - "handle {}th opcode {:?} rws = {:?}", + "handle {}th opcode {:?} {:?} rws = {:?}", i, step.op, + step, self.chunk_rws() ); let exec_steps = gen_associated_ops( @@ -466,9 +470,13 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { Ok(res) } - // TODO Fix this, for current logic on processing `call` is incorrect - // TODO re-design `gen_chunk_associated_steps` to separate RW - fn gen_chunk_associated_steps(&mut self, step: &mut ExecStep, rw: RW) { + // generate chunk related steps + fn gen_chunk_associated_steps( + &mut self, + step: &mut ExecStep, + rw: RW, + tx: Option<&Transaction>, + ) { let STEP_STATE_LEN = 10; let mut dummy_tx = Transaction::default(); let mut dummy_tx_ctx = TransactionContext::default(); @@ -483,12 +491,10 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { let tags = { let state = self.state_ref(&mut dummy_tx, &mut dummy_tx_ctx); - let last_call = state - .block - .txs - .last() - .map(|tx| tx.calls[0].clone()) - .unwrap_or_else(Call::default); + let last_call = tx + .map(|tx| tx.calls()[step.call_index].clone()) + .or_else(|| state.block.txs.last().map(|tx| tx.calls[0].clone())) + .unwrap(); [ (StepStateField::CodeHash, last_call.code_hash.to_word()), (StepStateField::CallID, Word::from(last_call.call_id)), @@ -551,24 +557,44 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { self.chunk_ctx.end_copy_index = next_copy_index; self.cur_chunk_mut().ctx = self.chunk_ctx.clone(); if to_next { + // here use `-1` to include previous set self.chunk_ctx - .bump(self.block_ctx.rwc.0, next_tx_index, next_copy_index); + .bump(self.block_ctx.rwc.0, next_tx_index - 1, next_copy_index); + // println!("bump last_call {:?}", last_call); + if last_call.is_none() { + panic!("??") + } self.cur_chunk_mut().prev_last_call = last_call; } } - fn set_begin_chunk(&mut self, first_step: &ExecStep) { - let mut begin_chunk = first_step.clone(); - begin_chunk.exec_state = ExecState::BeginChunk; - self.gen_chunk_associated_steps(&mut begin_chunk, RW::READ); + fn set_begin_chunk(&mut self, first_step: &ExecStep, tx: Option<&Transaction>) { + let mut begin_chunk = ExecStep { + exec_state: ExecState::BeginChunk, + rwc: first_step.rwc, + gas_left: first_step.gas_left, + call_index: first_step.call_index, + ..ExecStep::default() + }; + self.gen_chunk_associated_steps(&mut begin_chunk, RW::READ, tx); + println!("in set begin chunk {:?}", begin_chunk); self.chunks[self.chunk_ctx.idx].begin_chunk = Some(begin_chunk); } - fn set_end_chunk(&mut self, next_step: &ExecStep) { - let mut end_chunk = next_step.clone(); - end_chunk.exec_state = ExecState::EndChunk; - self.gen_chunk_associated_steps(&mut end_chunk, RW::WRITE); + fn set_end_chunk(&mut self, next_step: &ExecStep, tx: Option<&Transaction>) { + println!("before self.block_ctx.rwc.0 {}", self.block_ctx.rwc.0); + // println!("next step {:?}", next_step); + let mut end_chunk = ExecStep { + exec_state: ExecState::EndChunk, + rwc: next_step.rwc, + rwc_inner_chunk: next_step.rwc_inner_chunk, + gas_left: next_step.gas_left, + call_index: next_step.call_index, + ..ExecStep::default() + }; + self.gen_chunk_associated_steps(&mut end_chunk, RW::WRITE, tx); self.gen_chunk_padding(&mut end_chunk); + println!("after self.block_ctx.rwc.0 {}", self.block_ctx.rwc.0); self.chunks[self.chunk_ctx.idx].end_chunk = Some(end_chunk); } @@ -578,8 +604,6 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { let total_rws = end_rwc - 1; let max_rws = self.cur_chunk().fixed_param.max_rws; - // We need at least 1 extra row at offset 0 for chunk continuous - // FIXME(Cecilia): adding + 1 fail some tests assert!( total_rws < max_rws, "total_rws <= max_rws, total_rws={}, max_rws={}", @@ -707,17 +731,26 @@ impl CircuitInputBuilder { eth_block: &EthBlock, geth_traces: &[eth_types::GethExecTrace], ) -> Result, Error> { - println!("--------------{:?}", self.circuits_params); // accumulates gas across all txs in the block let (last_step, last_call) = self.begin_handle_block(eth_block, geth_traces)?; - let last_step = last_step.unwrap_or_default(); + // since there is no next step, we cook dummy next step from last step to reuse + // existing field while update its `rwc`. + let mut dummy_next_step = { + let mut dummy_next_step = last_step.unwrap_or_default(); + // raise last step rwc to match with next step + (0..dummy_next_step.rw_indices_len()).for_each(|_| { + dummy_next_step.rwc.inc_pre(); + dummy_next_step.rwc_inner_chunk.inc_pre(); + }); + dummy_next_step + }; assert!(self.circuits_params.max_rws().is_some()); let last_copy = self.block.copy_events.len(); // TODO figure out and resolve generic param type and move fixed_param set inside - // commit_chunk_ctx After fixed, then we can set fixed_param on all chunks + // commit_chunk_ctx. After fixed, then we can set fixed_param on all chunks (0..self.circuits_params.total_chunks()).for_each(|idx| { self.get_chunk_mut(idx).fixed_param = self.circuits_params; }); @@ -734,16 +767,29 @@ impl CircuitInputBuilder { last_call.clone(), ); } else { - // it doent matter what step was put to set_end_chunk/set_begin_chunk on no-used - // chunks before end_block. Just need to make sure it's step lookup is consistency. - self.set_end_chunk(&last_step); + self.set_end_chunk(&dummy_next_step, None); + self.commit_chunk_ctx( true, eth_block.transactions.len(), last_copy, last_call.clone(), ); - self.set_begin_chunk(&last_step); + // update dummy_next_step rwc to be used for next + dummy_next_step.rwc = self.block_ctx.rwc; + dummy_next_step.rwc_inner_chunk = self.chunk_ctx.rwc; + self.set_begin_chunk(&dummy_next_step, None); + dummy_next_step.rwc = self.block_ctx.rwc; + dummy_next_step.rwc_inner_chunk = self.chunk_ctx.rwc; + // update virtual step: end_block/padding so it can carry state context correctly + // TODO: enhance virtual step updating mechanism by having `running_next_step` + // defined in circuit_input_builder, so we dont need to + self.block.end_block = dummy_next_step.clone(); + self.cur_chunk_mut().padding = { + let mut padding = dummy_next_step.clone(); + padding.exec_state = ExecState::Padding; + Some(padding) + }; } Ok::<(), Error>(()) })?; @@ -773,6 +819,7 @@ impl CircuitInputBuilder { fn set_end_block(&mut self) -> Result<(), Error> { let mut end_block = self.block.end_block.clone(); end_block.rwc = self.block_ctx.rwc; + end_block.exec_state = ExecState::EndBlock; end_block.rwc_inner_chunk = self.chunk_ctx.rwc; let mut dummy_tx = Transaction::default(); diff --git a/integration-tests/src/integration_test_circuits.rs b/integration-tests/src/integration_test_circuits.rs index 49349f434f..91d1bb8039 100644 --- a/integration-tests/src/integration_test_circuits.rs +++ b/integration-tests/src/integration_test_circuits.rs @@ -369,7 +369,7 @@ impl + Circuit> IntegrationTest { col1.iter().enumerate().zip_eq(col2.iter()).for_each( |((index, cellv1), cellv2)| { assert!( - cellv1.eq(&cellv2), + cellv1.eq(cellv2), "cellv1 {:?} != cellv2 {:?} on index {}", cellv1, cellv2, @@ -407,7 +407,7 @@ impl + Circuit> IntegrationTest { col1.iter().enumerate().zip_eq(col2.iter()).for_each( |((index, cellv1), cellv2)| { assert!( - cellv1.eq(&cellv2), + cellv1.eq(cellv2), "cellv1 {:?} != cellv2 {:?} on index {}", cellv1, cellv2, diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs index 824370510e..a731188ca7 100644 --- a/testool/src/statetest/executor.rs +++ b/testool/src/statetest/executor.rs @@ -348,12 +348,9 @@ pub fn run_test( let block: Block = zkevm_circuits::evm_circuit::witness::block_convert(&builder).unwrap(); - let chunk: Chunk = - zkevm_circuits::evm_circuit::witness::chunk_convert(&block, &builder) - .unwrap() - .remove(0); - - CircuitTestBuilder::<1, 1>::new_from_block(block, chunk) + let chunks: Vec> = + zkevm_circuits::evm_circuit::witness::chunk_convert(&block, &builder).unwrap(); + CircuitTestBuilder::<1, 1>::new_from_block(block, chunks) .run_with_result() .map_err(|err| match err { CircuitTestError::VerificationFailed { reasons, .. } => { diff --git a/zkevm-circuits/src/evm_circuit.rs b/zkevm-circuits/src/evm_circuit.rs index e50b619fe3..53e8cdbde1 100644 --- a/zkevm-circuits/src/evm_circuit.rs +++ b/zkevm-circuits/src/evm_circuit.rs @@ -638,7 +638,9 @@ mod evm_circuit_stats { TestContext::<0, 0>::new(None, |_| {}, |_, _| {}, |b, _| b).unwrap(), ) .block_modifier(Box::new(|_block, chunk| { - chunk.fixed_param.max_evm_rows = (1 << 18) - 100 + chunk + .iter_mut() + .for_each(|chunk| chunk.fixed_param.max_evm_rows = (1 << 18) - 100); })) .run(); } diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index fa80d03e2e..a3e68d799c 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -815,6 +815,10 @@ impl ExecutionConfig { let step_curr_rw_counter = cb.curr.state.rw_counter.clone(); let step_curr_rw_counter_offset = cb.rw_counter_offset(); + if execution_state == ExecutionState::BeginChunk { + cb.debug_expression("step_curr_rw_counter.expr()", step_curr_rw_counter.expr()); + } + let debug_expressions = cb.debug_expressions.clone(); // Extract feature config here before cb is built. @@ -979,9 +983,10 @@ impl ExecutionConfig { .collect(), ), ( - "Only EndTx or InvalidTx or EndBlock or Padding can transit to EndBlock", + "Only BeginChunk or EndTx or InvalidTx or EndBlock or Padding can transit to EndBlock", ExecutionState::EndBlock, vec![ + ExecutionState::BeginChunk, ExecutionState::EndTx, ExecutionState::EndBlock, ExecutionState::Padding, @@ -1125,24 +1130,26 @@ impl ExecutionConfig { self.q_step_first.enable(&mut region, offset)?; let dummy_tx = Transaction::default(); - let chunk_txs = block + // chunk_txs is just a super set of execstep including both belong to this chunk and + // outside of this chunk + let chunk_txs: &[Transaction] = block .txs .get(chunk.chunk_context.initial_tx_index..chunk.chunk_context.end_tx_index) .unwrap_or_default(); - let last_call = chunk_txs + // If it's the very first chunk in a block set last call & begin_chunk to default + let prev_chunk_last_call = chunk.prev_last_call.clone().unwrap_or_default(); + let cur_chunk_last_call = chunk_txs .last() .map(|tx| tx.calls()[0].clone()) - .unwrap_or_else(Call::default); - // If it's the very first chunk in a block set last call & begin_chunk to default - let prev_last_call = chunk.prev_last_call.clone().unwrap_or_default(); + .unwrap_or_else(|| prev_chunk_last_call.clone()); let padding = chunk.padding.as_ref().expect("padding can't be None"); // conditionally adding first step as begin chunk let maybe_begin_chunk = { if let Some(begin_chunk) = &chunk.begin_chunk { - vec![(&dummy_tx, &prev_last_call, begin_chunk)] + vec![(&dummy_tx, &prev_chunk_last_call, begin_chunk)] } else { vec![] } @@ -1153,10 +1160,16 @@ impl ExecutionConfig { .chain(chunk_txs.iter().flat_map(|tx| { tx.steps() .iter() + // chunk_txs is just a super set of execstep. To filter targetting + // execstep we need to further filter by [initial_rwc, end_rwc) + .filter(|step| { + step.rwc.0 >= chunk.chunk_context.initial_rwc + && step.rwc.0 < chunk.chunk_context.end_rwc + }) .map(move |step| (tx, &tx.calls()[step.call_index], step)) })) // this dummy step is just for real step assignment proceed to `second last` - .chain(std::iter::once((&dummy_tx, &last_call, padding))) + .chain(std::iter::once((&dummy_tx, &cur_chunk_last_call, padding))) .peekable(); let evm_rows = chunk.fixed_param.max_evm_rows; @@ -1228,6 +1241,7 @@ impl ExecutionConfig { if next.is_none() { break; } + second_last_real_step = Some(cur); // record offset of current step before assignment second_last_real_step_offset = offset; @@ -1243,7 +1257,7 @@ impl ExecutionConfig { next_step_after_real_step = Some(padding.clone()); } offset = assign_padding_or_step( - (&dummy_tx, &last_call, padding), + (&dummy_tx, &cur_chunk_last_call, padding), offset, None, Some(evm_rows - 1), @@ -1254,7 +1268,7 @@ impl ExecutionConfig { if let Some(end_chunk) = &chunk.end_chunk { debug_assert_eq!(ExecutionState::EndChunk.get_step_height(), 1); offset = assign_padding_or_step( - (&dummy_tx, &last_call, end_chunk), + (&dummy_tx, &cur_chunk_last_call, end_chunk), offset, None, None, @@ -1269,7 +1283,7 @@ impl ExecutionConfig { ); debug_assert_eq!(ExecutionState::EndBlock.get_step_height(), 1); offset = assign_padding_or_step( - (&dummy_tx, &last_call, &block.end_block), + (&dummy_tx, &cur_chunk_last_call, &block.end_block), offset, None, None, @@ -1286,7 +1300,11 @@ impl ExecutionConfig { _ = assign_padding_or_step( last_real_step, second_last_real_step_offset, - Some((&dummy_tx, &last_call, &next_step_after_real_step.unwrap())), + Some(( + &dummy_tx, + &cur_chunk_last_call, + &next_step_after_real_step.unwrap(), + )), None, )?; } @@ -1421,6 +1439,13 @@ impl ExecutionConfig { step, call ); + // println!( + // "assign_exec_step offset: {} state {:?} step: {:?} call: {:?}", + // offset, + // step.execution_state(), + // step, + // call + // ); } // Make the region large enough for the current step and the next step. // The next step's next step may also be accessed, so make the region large @@ -1438,6 +1463,13 @@ impl ExecutionConfig { // so their witness values need to be known to be able // to correctly calculate the intermediate value. if let Some(next_step) = next_step { + // println!( + // "assign_exec_step nextstep offset: {} state {:?} step: {:?} call: {:?}", + // offset, + // next_step.2.execution_state(), + // next_step.2, + // next_step.1 + // ); self.assign_exec_step_int( region, offset + height, diff --git a/zkevm-circuits/src/evm_circuit/execution/begin_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/begin_chunk.rs index 9687b72091..149dfe050c 100644 --- a/zkevm-circuits/src/evm_circuit/execution/begin_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/begin_chunk.rs @@ -4,7 +4,7 @@ use crate::{ evm_circuit::{ step::ExecutionState, util::{ - constraint_builder::{EVMConstraintBuilder, StepStateTransition}, + constraint_builder::{EVMConstraintBuilder, StepStateTransition, Transition::Delta}, CachedRegion, }, witness::{Block, Call, Chunk, ExecStep, Transaction}, @@ -29,7 +29,10 @@ impl ExecutionGadget for BeginChunkGadget { fn configure(cb: &mut EVMConstraintBuilder) -> Self { // state lookup cb.step_state_lookup(0.expr()); - let step_state_transition = StepStateTransition::same(); + let step_state_transition = StepStateTransition { + rw_counter: Delta(cb.rw_counter_offset()), + ..StepStateTransition::same() + }; cb.require_step_state_transition(step_state_transition); Self { _marker: PhantomData {}, diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs index d5221e6a78..cf416dfe8e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/callop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/callop.rs @@ -1382,12 +1382,11 @@ mod test { ) .unwrap(); - // FIXME (Cecilia): Somehow needs way more than 500 CircuitTestBuilder::new_from_test_ctx(ctx) - // .params(FixedCParams { - // max_rws: 500, - // ..Default::default() - // }) + .params(FixedCParams { + max_rws: 500, + ..Default::default() + }) .run(); } diff --git a/zkevm-circuits/src/evm_circuit/execution/end_block.rs b/zkevm-circuits/src/evm_circuit/execution/end_block.rs index bbe2b38ad8..9854d01368 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_block.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_block.rs @@ -174,7 +174,9 @@ mod test { // finish required tests using this witness block CircuitTestBuilder::<2, 1>::new_from_test_ctx(ctx) .block_modifier(Box::new(move |_block, chunk| { - chunk.fixed_param.max_evm_rows = evm_circuit_pad_to + chunk + .iter_mut() + .for_each(|chunk| chunk.fixed_param.max_evm_rows = evm_circuit_pad_to); })) .run(); } diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 4231a06cd3..4e7a8f6583 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -12,6 +12,7 @@ use crate::{ }, util::Expr, }; +use bus_mapping::{exec_trace::OperationRef, operation::Target}; use eth_types::Field; use halo2_proofs::plonk::Error; @@ -60,12 +61,20 @@ impl ExecutionGadget for EndChunkGadget { _: &Call, step: &ExecStep, ) -> Result<(), Error> { + let rwc_before_padding = step + .bus_mapping_instance + .iter() + .filter(|x| { + let OperationRef(c, _) = x; + *c != Target::Start && *c != Target::Padding + }) + .count(); self.rw_table_padding_gadget.assign_exec_step( region, offset, block, chunk, - (step.rwc_inner_chunk.0 - 1 + step.bus_mapping_instance.len()) as u64, + (step.rwc_inner_chunk.0 - 1 + rwc_before_padding) as u64, step, )?; Ok(()) @@ -83,6 +92,17 @@ mod test { use halo2_proofs::halo2curves::bn256::Fr; use mock::TestContext; + macro_rules! test_2_txs_with_various_chunk_size { + ($($name:ident: $value:expr,)*) => { + $( + #[test] + fn $name() { + let (total_chunks, total_rws) = $value; + test_2_txs_with_chunk_size(total_chunks, total_rws); + } + )* + } + } #[test] fn test_chunking_rwmap_logic() { let bytecode = bytecode! { @@ -137,8 +157,7 @@ mod test { } } - #[test] - fn test_all_chunks_fixed() { + fn test_2_txs_with_chunk_size(total_chunks: usize, total_rws: usize) { let bytecode = bytecode! { GAS STOP @@ -167,28 +186,29 @@ mod test { |block, _tx| block.number(0xcafeu64), ) .unwrap(); - (0..6).for_each(|chunk_id| { - CircuitTestBuilder::new_from_test_ctx(test_ctx.clone()) - .params({ - FixedCParams { - total_chunks: 6, - max_rws: 90, - max_txs: 2, - ..Default::default() - } - }) - .run_chunk(chunk_id); - }); + CircuitTestBuilder::new_from_test_ctx(test_ctx) + .params({ + FixedCParams { + total_chunks, + max_rws: total_rws / total_chunks, + max_txs: 2, + ..Default::default() + } + }) + .run_multiple_chunks_with_result(Some(total_chunks)) + .unwrap(); } - #[test] - fn test_single_chunk() { - let bytecode = bytecode! { - STOP - }; - CircuitTestBuilder::new_from_test_ctx( - TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(), - ) - .run(); + test_2_txs_with_various_chunk_size! { + test_2_txs_with_1_400: (1, 400), + test_2_txs_with_2_400: (2, 400), + test_2_txs_with_3_400: (3, 400), + test_2_txs_with_4_400: (4, 400), + test_2_txs_with_1_600: (1, 600), + test_2_txs_with_2_600: (2, 600), + test_2_txs_with_3_600: (3, 600), + test_2_txs_with_4_600: (4, 600), + test_2_txs_with_5_600: (5, 600), + test_2_txs_with_6_600: (6, 600), } } diff --git a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs index 422d79e97a..1450954358 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs @@ -453,7 +453,7 @@ mod test { max_txs: 5, ..Default::default() }) - .build_block(0, 1) + .build_block(None) .unwrap(); block.rws.0[&Target::CallContext] diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index 9d90677b42..181f6d1b4f 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -850,6 +850,11 @@ impl Step { offset, Value::known(F::from(step.rwc_inner_chunk.into())), )?; + // println!( + // "execstate {:?} self.state.call_id {:?}", + // step.execution_state(), + // F::from(call.call_id as u64) + // ); self.state .call_id .assign(region, offset, Value::known(F::from(call.call_id as u64)))?; diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs index 76ccd31967..6cfbb365e8 100644 --- a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs @@ -1336,10 +1336,12 @@ impl RwTablePaddingGadget { 1.expr(), max_rws.expr() - inner_rws_before_padding.expr(), ); + cb.condition(is_end_padding_exist.expr(), |cb| { cb.rw_table_padding_lookup(inner_rws_before_padding.expr() + 1.expr()); cb.rw_table_padding_lookup(max_rws.expr() - 1.expr()); }); + // Since every lookup done in the EVM circuit must succeed and uses // a unique rw_counter, we know that at least there are // total_rws meaningful entries in the rw_table. diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index b594fadeb4..2d05ece1e4 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -11,7 +11,7 @@ use bus_mapping::{ mock::BlockData, }; use eth_types::geth_types::GethData; -use itertools::all; +use itertools::{all, Itertools}; use std::cmp; use thiserror::Error; @@ -86,8 +86,8 @@ pub struct CircuitTestBuilder { circuits_params: Option, feature_config: Option, block: Option>, - chunk: Option>, - block_modifiers: Vec, &mut Chunk)>>, + chunks: Option>>, + block_modifiers: Vec, &mut Vec>)>>, } impl CircuitTestBuilder { @@ -98,7 +98,7 @@ impl CircuitTestBuilder { circuits_params: None, feature_config: None, block: None, - chunk: None, + chunks: None, block_modifiers: vec![], } } @@ -111,8 +111,8 @@ impl CircuitTestBuilder { /// Generates a CTBC from a [`Block`] passed with all the other fields /// set to [`Default`]. - pub fn new_from_block(block: Block, chunk: Chunk) -> Self { - Self::empty().block_chunk(block, chunk) + pub fn new_from_block(block: Block, chunks: Vec>) -> Self { + Self::empty().set_block_chunk(block, chunks) } /// Allows to produce a [`TestContext`] which will serve as the generator of @@ -140,10 +140,10 @@ impl CircuitTestBuilder { self } - /// Allows to pass a [`Block`] already built to the constructor. - pub fn block_chunk(mut self, block: Block, chunk: Chunk) -> Self { + /// Allows to pass a [`Block`], [`Chunk`] vectors already built to the constructor. + pub fn set_block_chunk(mut self, block: Block, chunks: Vec>) -> Self { self.block = Some(block); - self.chunk = Some(chunk); + self.chunks = Some(chunks); self } @@ -153,7 +153,10 @@ impl CircuitTestBuilder { /// /// That removes the need in a lot of tests to build the block outside of /// the builder because they need to modify something particular. - pub fn block_modifier(mut self, modifier: Box, &mut Chunk)>) -> Self { + pub fn block_modifier( + mut self, + modifier: Box, &mut Vec>)>, + ) -> Self { self.block_modifiers.push(modifier); self } @@ -163,12 +166,11 @@ impl CircuitTestBuilder { /// build block pub fn build_block( &self, - chunk_index: usize, - total_chunk: usize, - ) -> Result<(Block, Chunk), CircuitTestError> { - if let (Some(block), Some(chunk)) = (&self.block, &self.chunk) { + total_chunks: Option, + ) -> Result<(Block, Vec>), CircuitTestError> { + if let (Some(block), Some(chunks)) = (&self.block, &self.chunks) { // If a block is specified, no need to modify the block - return Ok((block.clone(), chunk.clone())); + return Ok((block.clone(), chunks.clone())); } let block = self .test_ctx @@ -177,16 +179,19 @@ impl CircuitTestBuilder { let block: GethData = block.clone().into(); let builder = match self.circuits_params { Some(fixed_param) => { - assert!( - fixed_param.total_chunks == total_chunk, - "Total chunks unmatched with fixed param" - ); + if let Some(total_chunks) = total_chunks { + assert!( + fixed_param.total_chunks == total_chunks, + "Total chunks unmatched with fixed param" + ); + } + BlockData::new_from_geth_data_with_params(block.clone(), fixed_param) .new_circuit_input_builder_with_feature(self.feature_config.unwrap_or_default()) .handle_block(&block.eth_block, &block.geth_traces) .map_err(|err| CircuitTestError::CannotHandleBlock(err.to_string()))? } - None => BlockData::new_from_geth_data_chunked(block.clone(), total_chunk) + None => BlockData::new_from_geth_data_chunked(block.clone(), total_chunks.unwrap_or(1)) .new_circuit_input_builder_with_feature(self.feature_config.unwrap_or_default()) .handle_block(&block.eth_block, &block.geth_traces) .map_err(|err| CircuitTestError::CannotHandleBlock(err.to_string()))?, @@ -194,64 +199,152 @@ impl CircuitTestBuilder { // Build a witness block from trace result. let mut block = crate::witness::block_convert(&builder) .map_err(|err| CircuitTestError::CannotConvertBlock(err.to_string()))?; - let mut chunk = crate::witness::chunk_convert(&block, &builder) - .unwrap() - .remove(chunk_index); + let mut chunks = crate::witness::chunk_convert(&block, &builder).unwrap(); for modifier_fn in &self.block_modifiers { - modifier_fn.as_ref()(&mut block, &mut chunk); + modifier_fn.as_ref()(&mut block, &mut chunks); } - Ok((block, chunk)) + Ok((block, chunks)) } fn run_evm_circuit_test( &self, block: Block, - chunk: Chunk, + chunks: Vec>, ) -> Result<(), CircuitTestError> { - let k = block.get_test_degree(&chunk); + if chunks.is_empty() { + return Err(CircuitTestError::SanityCheckChunks( + "empty chunks vector".to_string(), + )); + } + + let k = block.get_test_degree(&chunks[0]); let (active_gate_rows, active_lookup_rows) = - EvmCircuit::::get_active_rows(&block, &chunk); - - // Mainnet EVM circuit constraints can be cached for test performance. - // No cache for EVM circuit with customized features - let prover = if block.feature_config.is_mainnet() { - let circuit = EvmCircuitCached::get_test_circuit_from_block(block, chunk); - let instance = circuit.instance(); - MockProver::::run(k, &circuit, instance) - } else { - let circuit = EvmCircuit::get_test_circuit_from_block(block, chunk); - let instance = circuit.instance(); - MockProver::::run(k, &circuit, instance) - }; + EvmCircuit::::get_active_rows(&block, &chunks[0]); - let prover = prover.map_err(|err| CircuitTestError::SynthesisFailure { - circuit: Circuit::EVM, - reason: err, - })?; + // check consistency between chunk + chunks + .iter() + .tuple_windows() + .find_map(|(prev_chunk, chunk)| { + // global consistent + if prev_chunk.permu_alpha != chunk.permu_alpha { + return Some(Err(CircuitTestError::SanityCheckChunks( + "mismatch challenge alpha".to_string(), + ))); + } + if prev_chunk.permu_gamma != chunk.permu_gamma { + return Some(Err(CircuitTestError::SanityCheckChunks( + "mismatch challenge gamma".to_string(), + ))); + } - prover - .verify_at_rows( - active_gate_rows.iter().cloned(), - active_lookup_rows.iter().cloned(), - ) - .map_err(|err| CircuitTestError::VerificationFailed { - circuit: Circuit::EVM, - reasons: err, + if prev_chunk.by_address_rw_fingerprints.ending_row + != chunk.by_address_rw_fingerprints.prev_ending_row + { + return Some(Err(CircuitTestError::SanityCheckChunks( + "mismatch by_address_rw_fingerprints ending_row".to_string(), + ))); + } + if prev_chunk.by_address_rw_fingerprints.mul_acc + != chunk.by_address_rw_fingerprints.prev_mul_acc + { + return Some(Err(CircuitTestError::SanityCheckChunks( + "mismatch by_address_rw_fingerprints mul_acc".to_string(), + ))); + } + + if prev_chunk.chrono_rw_fingerprints.ending_row + != chunk.chrono_rw_fingerprints.prev_ending_row + { + return Some(Err(CircuitTestError::SanityCheckChunks( + "mismatch chrono_rw_fingerprints ending_row".to_string(), + ))); + } + if prev_chunk.chrono_rw_fingerprints.mul_acc + != chunk.chrono_rw_fingerprints.prev_mul_acc + { + return Some(Err(CircuitTestError::SanityCheckChunks( + "mismatch chrono_rw_fingerprints mul_acc".to_string(), + ))); + } + None + }) + .unwrap_or_else(|| Ok(()))?; + + // check last chunk fingerprints + chunks + .last() + .map(|last_chunk| { + if last_chunk.by_address_rw_fingerprints.mul_acc + != last_chunk.chrono_rw_fingerprints.mul_acc + { + Err(CircuitTestError::SanityCheckChunks( + "mismatch last rw_fingerprint mul_acc".to_string(), + )) + } else { + Ok(()) + } + }) + .unwrap_or_else(|| Ok(()))?; + + // stop on first chunk validation error + chunks + .into_iter() + .enumerate() + .find_map(|(i, chunk)| { + // Mainnet EVM circuit constraints can be cached for test performance. + // No cache for EVM circuit with customized features + let prover = if block.feature_config.is_mainnet() { + let circuit = + EvmCircuitCached::get_test_circuit_from_block(block.clone(), chunk); + let instance = circuit.instance(); + MockProver::::run(k, &circuit, instance) + } else { + let circuit = EvmCircuit::get_test_circuit_from_block(block.clone(), chunk); + let instance = circuit.instance(); + MockProver::::run(k, &circuit, instance) + }; + + if let Err(err) = prover { + return Some(Err(CircuitTestError::SynthesisFailure { + circuit: Circuit::EVM, + reason: err, + })); + } + + let prover = prover.unwrap(); + + let res = prover + .verify_at_rows( + active_gate_rows.iter().cloned(), + active_lookup_rows.iter().cloned(), + ) + .map_err(|err| CircuitTestError::VerificationFailed { + circuit: Circuit::EVM, + reasons: err, + }); + if res.is_err() { + println!("failed on chunk index {}", i); + Some(res) + } else { + None + } }) + .unwrap_or_else(|| Ok(())) } // TODO: use randomness as one of the circuit public input, since randomness in // state circuit and evm circuit must be same fn run_state_circuit_test( &self, block: Block, - chunk: Chunk, + chunks: Vec>, ) -> Result<(), CircuitTestError> { - let rows_needed = StateCircuit::::min_num_rows_block(&block, &chunk).1; + let rows_needed = StateCircuit::::min_num_rows_block(&block, &chunks[0]).1; let k = cmp::max(log2_ceil(rows_needed + NUM_BLINDING_ROWS), 18); - let state_circuit = StateCircuit::::new(&chunk); - let max_rws = chunk.fixed_param.max_rws; + let state_circuit = StateCircuit::::new(&chunks[0]); + let max_rws = &chunks[0].fixed_param.max_rws; let instance = state_circuit.instance(); let prover = MockProver::::run(k, &state_circuit, instance).map_err(|err| { CircuitTestError::SynthesisFailure { @@ -265,7 +358,7 @@ impl CircuitTestBuilder { .iter() .filter(|rw| !matches!(rw, Rw::Start { .. })) .count(); - let rows = max_rws - non_start_rows_len..max_rws; + let rows = max_rws - non_start_rows_len..*max_rws; prover.verify_at_rows(rows.clone(), rows).map_err(|err| { CircuitTestError::VerificationFailed { circuit: Circuit::EVM, @@ -273,140 +366,30 @@ impl CircuitTestBuilder { } }) } - /// Triggers the `CircuitTestBuilder` to convert the [`TestContext`] if any, - /// into a [`Block`] and specified numbers of [`Chunk`]s - /// and apply the default or provided modifiers or - /// circuit checks to the provers generated for the State and EVM circuits. - /// One [`Chunk`] is generated by default and the first one is run unless - /// [`FixedCParams`] is set. - pub fn run(self) { - println!("--------------{:?}", self.circuits_params); - if let Some(fixed_params) = self.circuits_params { - self.run_dynamic_chunk(fixed_params.total_chunks, 0); - } else { - self.run_dynamic_chunk(1, 0); - } - } /// Triggers the `CircuitTestBuilder` to convert the [`TestContext`] if any, /// into a [`Block`] and apply the default or provided block_modifiers or /// circuit checks to the provers generated for the State and EVM circuits. pub fn run_with_result(self) -> Result<(), CircuitTestError> { - let (block, chunk) = self.build_block(0, 1)?; - - self.run_evm_circuit_test(block.clone(), chunk.clone())?; - self.run_state_circuit_test(block, chunk) + self.run_multiple_chunks_with_result(None) } /// Triggers the `CircuitTestBuilder` to convert the [`TestContext`] if any, - /// into a [`Block`] and specified numbers of [`Chunk`]s. - /// [`FixedCParams`] must be set to build the amount of chunks - /// and run the indexed [`Chunk`]. - pub fn run_chunk(self, chunk_index: usize) { - let total_chunk = self - .circuits_params - .expect("Fixed param not specified") - .total_chunks; - self.run_dynamic_chunk(total_chunk, chunk_index); - } + /// into a [`Block`] and apply the default or provided block_modifiers or + /// circuit checks to the provers generated for the State and EVM circuits. + pub fn run_multiple_chunks_with_result( + self, + total_chunks: Option, + ) -> Result<(), CircuitTestError> { + let (block, chunks) = self.build_block(total_chunks)?; - /// Triggers the `CircuitTestBuilder` to convert the [`TestContext`] if any, - /// into a [`Block`] and specified numbers of [`Chunk`]s with dynamic chunk size - /// if [`FixedCParams`] is not set, otherwise the `total_chunk` must matched. - pub fn run_dynamic_chunk(self, total_chunk: usize, chunk_index: usize) { - assert!(chunk_index < total_chunk, "Chunk index exceed total chunks"); - let (block, chunk) = if self.block.is_some() && self.chunk.is_some() { - (self.block.unwrap(), self.chunk.unwrap()) - } else if self.test_ctx.is_some() { - let block: GethData = self.test_ctx.unwrap().into(); - let builder = match self.circuits_params { - Some(fixed_param) => { - assert_eq!( - fixed_param.total_chunks, total_chunk, - "Total chunks unmatched with fixed param" - ); - BlockData::new_from_geth_data_with_params(block.clone(), fixed_param) - .new_circuit_input_builder_with_feature( - self.feature_config.unwrap_or_default(), - ) - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap() - } - None => BlockData::new_from_geth_data_chunked(block.clone(), total_chunk) - .new_circuit_input_builder_with_feature(self.feature_config.unwrap_or_default()) - .handle_block(&block.eth_block, &block.geth_traces) - .unwrap(), - }; - // FIXME(Cecilia): debug - println!("----after-handle-block-----"); - builder.chunks.iter().for_each(|c| { - println!( - "{:?}\n{:?}\nbegin {:?}\nend {:?}\n", - c.ctx, - c.fixed_param, - c.begin_chunk.is_some(), - c.end_chunk.is_some() - ); - println!("----------"); - }); - println!("block rwc = {:?}", builder.block_ctx.rwc); - - // Build a witness block from trace result. - let mut block = crate::witness::block_convert(&builder).unwrap(); - - let mut chunk = crate::witness::chunk_convert(&block, &builder) - .unwrap() - .remove(chunk_index); - - for modifier_fn in self.block_modifiers { - modifier_fn.as_ref()(&mut block, &mut chunk); - } - (block, chunk) - } else { - panic!("No attribute to build a block was passed to the CircuitTestBuilder") - }; - let params = chunk.fixed_param; - - // Run evm circuit test - { - let k = block.get_test_degree(&chunk); - - let (_active_gate_rows, _active_lookup_rows) = - EvmCircuit::::get_active_rows(&block, &chunk); - - let _prover = if block.feature_config.is_mainnet() { - let circuit = - EvmCircuitCached::get_test_circuit_from_block(block.clone(), chunk.clone()); - let instance = circuit.instance(); - MockProver::::run(k, &circuit, instance) - } else { - let circuit = EvmCircuit::get_test_circuit_from_block(block.clone(), chunk.clone()); - let instance = circuit.instance(); - MockProver::::run(k, &circuit, instance) - }; - - // self.evm_checks.as_ref()(prover, &active_gate_rows, &active_lookup_rows) - } + self.run_evm_circuit_test(block.clone(), chunks.clone())?; + self.run_state_circuit_test(block, chunks) + } - // Run state circuit test - // TODO: use randomness as one of the circuit public input, since randomness in - // state circuit and evm circuit must be same - { - let rows_needed = StateCircuit::::min_num_rows_block(&block, &chunk).1; - let k = cmp::max(log2_ceil(rows_needed + NUM_BLINDING_ROWS), 18); - let state_circuit = StateCircuit::::new(&chunk); - let instance = state_circuit.instance(); - let _prover = MockProver::::run(k, &state_circuit, instance).unwrap(); - // Skip verification of Start rows to accelerate testing - let non_start_rows_len = state_circuit - .rows - .iter() - .filter(|rw| !matches!(rw, Rw::Padding { .. })) - .count(); - let _rows: Vec = (params.max_rws - non_start_rows_len..params.max_rws).collect(); - - // self.state_checks.as_ref()(prover, &rows, &rows); - } + /// Convenient method to run in test cases that error handling is not required. + pub fn run(self) { + self.run_with_result().unwrap() } } @@ -431,6 +414,9 @@ pub enum CircuitTestError { /// Something worng in the block_convert #[error("CannotConvertBlock({0})")] CannotConvertBlock(String), + /// Something worng in the chunk_convert + #[error("SanityCheckChunks({0})")] + SanityCheckChunks(String), /// Problem constructing MockProver #[error("SynthesisFailure({circuit:?}, reason: {reason:?})")] SynthesisFailure { diff --git a/zkevm-circuits/src/witness/chunk.rs b/zkevm-circuits/src/witness/chunk.rs index 15127e172a..136d92644d 100755 --- a/zkevm-circuits/src/witness/chunk.rs +++ b/zkevm-circuits/src/witness/chunk.rs @@ -109,11 +109,6 @@ pub fn chunk_convert( } }); - println!( - "| {:?} ... {:?} | @chunk_convert", - chunk.ctx.initial_rwc, chunk.ctx.end_rwc - ); - // Get the rws in the i-th chunk let chrono_rws = { let mut chrono_rws = RwMap::from(&builder.block.container); @@ -185,7 +180,6 @@ pub fn chunk_convert( true, prev_chunk_last_chrono_rw, ); - chunks.push(Chunk { permu_alpha: alpha, permu_gamma: gamma, @@ -204,6 +198,13 @@ pub fn chunk_convert( }); } + if log::log_enabled!(log::Level::Debug) { + chunks + .iter() + .enumerate() + .for_each(|(i, chunk)| log::debug!("{}th chunk context {:?}", i, chunk,)); + } + Ok(chunks) } From 418f37be21d5288a30243474a49f9bde123dc822 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Thu, 29 Feb 2024 15:39:45 +0800 Subject: [PATCH 13/16] root/super circuit multiple chunk mock test --- .github/workflows/main-tests.yml | 6 +- bus-mapping/src/circuit_input_builder.rs | 12 +-- .../circuit_input_builder/input_state_ref.rs | 1 + circuit-benchmarks/src/super_circuit.rs | 3 +- testool/src/statetest/executor.rs | 5 +- zkevm-circuits/src/evm_circuit/execution.rs | 6 +- .../src/evm_circuit/execution/callop.rs | 2 +- .../src/evm_circuit/execution/end_chunk.rs | 1 + zkevm-circuits/src/root_circuit/test.rs | 77 +++++++++++-------- zkevm-circuits/src/state_circuit.rs | 2 +- zkevm-circuits/src/super_circuit.rs | 50 +++++++++--- zkevm-circuits/src/super_circuit/test.rs | 39 ++++++++-- zkevm-circuits/src/test_util.rs | 72 ++++++++++++----- 13 files changed, 192 insertions(+), 84 deletions(-) diff --git a/.github/workflows/main-tests.yml b/.github/workflows/main-tests.yml index 5a4826347a..4af43069fb 100644 --- a/.github/workflows/main-tests.yml +++ b/.github/workflows/main-tests.yml @@ -133,12 +133,16 @@ jobs: sudo chmod +x /usr/bin/solc - name: Update ERC20 git submodule run: git submodule update --init --recursive --checkout integration-tests/contracts/vendor/openzeppelin-contracts + - name: Run root test + uses: actions-rs/cargo@v1 + with: + command: test + args: --package zkevm-circuits --lib -- root_circuit::test::test_root_circuit_multiple_chunk --exact --ignored - name: Run heavy tests # heavy tests are run serially to avoid OOM uses: actions-rs/cargo@v1 with: command: test args: --verbose --release --all --all-features --exclude integration-tests --exclude circuit-benchmarks serial_ -- --ignored --test-threads 1 - build: needs: [skip_check] if: | diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index da87f78811..26cb003798 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -88,7 +88,7 @@ impl FeatureConfig { const RW_BUFFER_SIZE: usize = 30; /// Circuit Setup Parameters -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Copy, PartialEq)] pub struct FixedCParams { /// pub total_chunks: usize, @@ -557,13 +557,9 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { self.chunk_ctx.end_copy_index = next_copy_index; self.cur_chunk_mut().ctx = self.chunk_ctx.clone(); if to_next { - // here use `-1` to include previous set + // add `-1` to include previous set and deal with transaction cross-chunk case self.chunk_ctx .bump(self.block_ctx.rwc.0, next_tx_index - 1, next_copy_index); - // println!("bump last_call {:?}", last_call); - if last_call.is_none() { - panic!("??") - } self.cur_chunk_mut().prev_last_call = last_call; } } @@ -577,13 +573,10 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { ..ExecStep::default() }; self.gen_chunk_associated_steps(&mut begin_chunk, RW::READ, tx); - println!("in set begin chunk {:?}", begin_chunk); self.chunks[self.chunk_ctx.idx].begin_chunk = Some(begin_chunk); } fn set_end_chunk(&mut self, next_step: &ExecStep, tx: Option<&Transaction>) { - println!("before self.block_ctx.rwc.0 {}", self.block_ctx.rwc.0); - // println!("next step {:?}", next_step); let mut end_chunk = ExecStep { exec_state: ExecState::EndChunk, rwc: next_step.rwc, @@ -594,7 +587,6 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { }; self.gen_chunk_associated_steps(&mut end_chunk, RW::WRITE, tx); self.gen_chunk_padding(&mut end_chunk); - println!("after self.block_ctx.rwc.0 {}", self.block_ctx.rwc.0); self.chunks[self.chunk_ctx.idx].end_chunk = Some(end_chunk); } diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index 63f6ba04a5..2f1fff9d92 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -67,6 +67,7 @@ impl<'a> CircuitInputStateRef<'a> { exec_state: ExecState::InvalidTx, gas_left: self.tx.gas(), rwc: self.block_ctx.rwc, + rwc_inner_chunk: self.chunk_ctx.rwc, ..Default::default() } } diff --git a/circuit-benchmarks/src/super_circuit.rs b/circuit-benchmarks/src/super_circuit.rs index f7458b6f4a..603ce8929b 100644 --- a/circuit-benchmarks/src/super_circuit.rs +++ b/circuit-benchmarks/src/super_circuit.rs @@ -91,8 +91,9 @@ mod tests { max_evm_rows: 0, max_keccak_rows: 0, }; - let (_, circuit, instance, _) = + let (_, mut circuits, mut instances, _) = SuperCircuit::build(block, circuits_params, Fr::from(0x100)).unwrap(); + let (circuit, instance) = (circuits.remove(0), instances.remove(0)); let instance_refs: Vec<&[Fr]> = instance.iter().map(|v| &v[..]).collect(); // Bench setup generation diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs index a731188ca7..316e3e38fd 100644 --- a/testool/src/statetest/executor.rs +++ b/testool/src/statetest/executor.rs @@ -379,10 +379,13 @@ pub fn run_test( max_evm_rows: 0, max_keccak_rows: 0, }; - let (k, circuit, instance, _builder) = + let (k, mut circuits, mut instances, _builder) = SuperCircuit::::build(geth_data, circuits_params, Fr::from(0x100)).unwrap(); builder = _builder; + let circuit = circuits.remove(0); + let instance = instances.remove(0); + let prover = MockProver::run(k, &circuit, instance).unwrap(); prover .verify() diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index a3e68d799c..7fa128bbd8 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -930,7 +930,7 @@ impl ExecutionConfig { .chain( [ ( - "EndTx can only transit to BeginTx or Padding or EndBlock or EndChunk", + "EndTx can only transit to BeginTx or Padding or EndBlock or EndChunk or InvalidTx", ExecutionState::EndTx, vec![ ExecutionState::BeginTx, @@ -1172,6 +1172,10 @@ impl ExecutionConfig { .chain(std::iter::once((&dummy_tx, &cur_chunk_last_call, padding))) .peekable(); + tx_call_steps + .clone() + .for_each(|step| println!("assigned_step step {:?}", step.2)); + let evm_rows = chunk.fixed_param.max_evm_rows; let mut assign_padding_or_step = |cur_tx_call_step: TxCallStep, diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs index cf416dfe8e..e04fbfe16e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/callop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/callop.rs @@ -1384,7 +1384,7 @@ mod test { CircuitTestBuilder::new_from_test_ctx(ctx) .params(FixedCParams { - max_rws: 500, + max_rws: 1 << 12, ..Default::default() }) .run(); diff --git a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs index 4e7a8f6583..b10e4e1e34 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_chunk.rs @@ -190,6 +190,7 @@ mod test { .params({ FixedCParams { total_chunks, + max_evm_rows: 1 << 12, max_rws: total_rws / total_chunks, max_txs: 2, ..Default::default() diff --git a/zkevm-circuits/src/root_circuit/test.rs b/zkevm-circuits/src/root_circuit/test.rs index bada3372a2..071d4f129f 100644 --- a/zkevm-circuits/src/root_circuit/test.rs +++ b/zkevm-circuits/src/root_circuit/test.rs @@ -20,70 +20,87 @@ use rand::rngs::OsRng; #[ignore = "Due to high memory requirement"] #[test] -fn test_root_circuit() { - let (params, protocol, proof, instance, rwtable_columns) = { +fn test_root_circuit_multiple_chunk() { + let (params, protocol, proofs, instances, rwtable_columns) = { // Preprocess const TEST_MOCK_RANDOMNESS: u64 = 0x100; let circuits_params = FixedCParams { - total_chunks: 1, + total_chunks: 3, max_txs: 1, max_withdrawals: 5, max_calldata: 32, - max_rws: 256, + max_rws: 100, max_copy_rows: 256, max_exp_steps: 256, max_bytecode: 512, - max_evm_rows: 0, + max_evm_rows: 1 << 12, max_keccak_rows: 0, }; - let (k, circuit, instance, _) = + let (k, circuits, instances, _) = SuperCircuit::<_>::build(block_1tx(), circuits_params, TEST_MOCK_RANDOMNESS.into()) .unwrap(); + assert!(!circuits.is_empty()); + assert!(circuits.len() == instances.len()); // get chronological_rwtable and byaddr_rwtable columns index let mut cs = ConstraintSystem::default(); - let config = SuperCircuit::configure_with_params(&mut cs, circuit.params()); + let config = SuperCircuit::configure_with_params(&mut cs, circuits[0].params()); let rwtable_columns = config.get_rwtable_columns(); let params = ParamsKZG::::setup(k, OsRng); - let pk = keygen_pk(¶ms, keygen_vk(¶ms, &circuit).unwrap(), &circuit).unwrap(); + let pk = keygen_pk( + ¶ms, + keygen_vk(¶ms, &circuits[0]).unwrap(), + &circuits[0], + ) + .unwrap(); let protocol = compile( ¶ms, pk.get_vk(), Config::kzg() - .with_num_instance(instance.iter().map(|instance| instance.len()).collect()), + .with_num_instance(instances[0].iter().map(|instance| instance.len()).collect()), ); - // Create proof - let proof = { - let mut transcript = PoseidonTranscript::new(Vec::new()); - create_proof::, ProverGWC<_>, _, _, _, _>( - ¶ms, - &pk, - &[circuit], - &[&instance.iter().map(Vec::as_slice).collect_vec()], - OsRng, - &mut transcript, - ) - .unwrap(); - transcript.finalize() - }; - - (params, protocol, proof, instance, rwtable_columns) + let proofs: Vec> = circuits + .into_iter() + .zip(instances.iter()) + .map(|(circuit, instance)| { + // Create proof + let proof = { + let mut transcript = PoseidonTranscript::new(Vec::new()); + create_proof::, ProverGWC<_>, _, _, _, _>( + ¶ms, + &pk, + &[circuit], + &[&instance.iter().map(Vec::as_slice).collect_vec()], + OsRng, + &mut transcript, + ) + .unwrap(); + transcript.finalize() + }; + proof + }) + .collect(); + (params, protocol, proofs, instances, rwtable_columns) }; let user_challenge = UserChallenge { column_indexes: rwtable_columns, num_challenges: 2, // alpha, gamma }; + let snark_witnesses: Vec<_> = proofs + .iter() + .zip(instances.iter()) + .map(|(proof, instance)| { + SnarkWitness::new(&protocol, Value::known(instance), Value::known(proof)) + }) + .collect(); + let root_circuit = RootCircuit::>::new( ¶ms, &protocol, - vec![SnarkWitness::new( - &protocol, - Value::known(&instance), - Value::known(&proof), - )], + snark_witnesses, Some(&user_challenge), ) .unwrap(); diff --git a/zkevm-circuits/src/state_circuit.rs b/zkevm-circuits/src/state_circuit.rs index 7f302a6c61..769819d336 100644 --- a/zkevm-circuits/src/state_circuit.rs +++ b/zkevm-circuits/src/state_circuit.rs @@ -506,7 +506,7 @@ impl SubCircuit for StateCircuit { /// Return the minimum number of rows required to prove the block fn min_num_rows_block(_block: &witness::Block, chunk: &Chunk) -> (usize, usize) { ( - chunk.chrono_rws.0.values().flatten().count() + 1, + chunk.by_address_rws.0.values().flatten().count() + 1, chunk.fixed_param.max_rws, ) } diff --git a/zkevm-circuits/src/super_circuit.rs b/zkevm-circuits/src/super_circuit.rs index 09635c80ed..a768514325 100644 --- a/zkevm-circuits/src/super_circuit.rs +++ b/zkevm-circuits/src/super_circuit.rs @@ -79,6 +79,7 @@ use halo2_proofs::{ circuit::{Layouter, SimpleFloorPlanner, Value}, plonk::{Any, Circuit, Column, ConstraintSystem, Error, Expression}, }; +use itertools::Itertools; use std::array; @@ -560,8 +561,15 @@ impl SuperCircuit { geth_data: GethData, circuits_params: FixedCParams, mock_randomness: F, - ) -> Result<(u32, Self, Vec>, CircuitInputBuilder), bus_mapping::Error> - { + ) -> Result< + ( + u32, + Vec, + Vec>>, + CircuitInputBuilder, + ), + bus_mapping::Error, + > { let block_data = BlockData::new_from_geth_data_with_params(geth_data.clone(), circuits_params); let builder = block_data @@ -578,21 +586,43 @@ impl SuperCircuit { /// /// Also, return with it the minimum required SRS degree for the circuit and /// the Public Inputs needed. + #[allow(clippy::type_complexity)] pub fn build_from_circuit_input_builder( builder: &CircuitInputBuilder, mock_randomness: F, - ) -> Result<(u32, Self, Vec>), bus_mapping::Error> { + ) -> Result<(u32, Vec, Vec>>), bus_mapping::Error> { let mut block = block_convert(builder).unwrap(); - let chunk = chunk_convert(&block, builder).unwrap().remove(0); + let chunks = chunk_convert(&block, builder).unwrap(); block.randomness = mock_randomness; - let (_, rows_needed) = Self::min_num_rows_block(&block, &chunk); - let k = log2_ceil(Self::unusable_rows() + rows_needed); + let (rows_needed, circuit_instance_pairs): (Vec, Vec<(_, _)>) = chunks + .iter() + .map(|chunk| { + let (_, rows_needed) = Self::min_num_rows_block(&block, chunk); + + let circuit = SuperCircuit::new_from_block(&block, chunk); + let instance = circuit.instance(); + (rows_needed, (circuit, instance)) + }) + .unzip(); + + // assert all rows needed are equal + rows_needed + .iter() + .tuple_windows() + .for_each(|rows_needed: (&usize, &usize)| { + assert!( + rows_needed.0 == rows_needed.1, + "mismatched super_circuit rows_needed {:?} != {:?}", + rows_needed.0, + rows_needed.1 + ) + }); + + let k = log2_ceil(Self::unusable_rows() + rows_needed[0]); log::debug!("super circuit uses k = {}", k); - let circuit = SuperCircuit::new_from_block(&block, &chunk); - - let instance = circuit.instance(); - Ok((k, circuit, instance)) + let (circuits, instances) = circuit_instance_pairs.into_iter().unzip(); + Ok((k, circuits, instances)) } } diff --git a/zkevm-circuits/src/super_circuit/test.rs b/zkevm-circuits/src/super_circuit/test.rs index 0acb919ec7..f6891026c3 100644 --- a/zkevm-circuits/src/super_circuit/test.rs +++ b/zkevm-circuits/src/super_circuit/test.rs @@ -38,14 +38,20 @@ fn super_circuit_degree() { } fn test_super_circuit(block: GethData, circuits_params: FixedCParams, mock_randomness: Fr) { - let (k, circuit, instance, _) = + let (k, circuits, instances, _) = SuperCircuit::::build(block, circuits_params, mock_randomness).unwrap(); - let prover = MockProver::run(k, &circuit, instance).unwrap(); - let res = prover.verify(); - if let Err(err) = res { - error!("Verification failures: {:#?}", err); - panic!("Failed verification"); - } + circuits + .into_iter() + .zip(instances.into_iter()) + .enumerate() + .for_each(|(i, (circuit, instance))| { + let prover = MockProver::run(k, &circuit, instance).unwrap(); + let res = prover.verify(); + if let Err(err) = res { + error!("{}th supercircuit Verification failures: {:#?}", i, err); + panic!("Failed verification"); + } + }); } pub(crate) fn block_1tx() -> GethData { @@ -193,6 +199,25 @@ fn serial_test_super_circuit_2tx_2max_tx() { test_super_circuit(block, circuits_params, Fr::from(TEST_MOCK_RANDOMNESS)); } +#[ignore] +#[test] +fn serial_test_multi_chunk_super_circuit_2tx_2max_tx() { + let block = block_2tx(); + let circuits_params = FixedCParams { + total_chunks: 4, + max_txs: 2, + max_withdrawals: 5, + max_calldata: 32, + max_rws: 90, + max_copy_rows: 256, + max_exp_steps: 256, + max_bytecode: 512, + max_evm_rows: 0, + max_keccak_rows: 0, + }; + test_super_circuit(block, circuits_params, Fr::from(TEST_MOCK_RANDOMNESS)); +} + #[ignore] #[test] fn test_rw_table_commitment() { diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 2d05ece1e4..14c860c704 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -293,6 +293,7 @@ impl CircuitTestBuilder { chunks .into_iter() .enumerate() + // terminate on first error .find_map(|(i, chunk)| { // Mainnet EVM circuit constraints can be cached for test performance. // No cache for EVM circuit with customized features @@ -341,30 +342,59 @@ impl CircuitTestBuilder { block: Block, chunks: Vec>, ) -> Result<(), CircuitTestError> { + // sanity check + assert!(!chunks.is_empty()); + chunks.iter().tuple_windows().for_each(|(chunk1, chunk2)| { + let (rows_needed_1, rows_needed_2) = ( + StateCircuit::::min_num_rows_block(&block, chunk1).1, + StateCircuit::::min_num_rows_block(&block, chunk2).1, + ); + assert!(rows_needed_1 == rows_needed_2); + + assert!(chunk1.fixed_param == chunk2.fixed_param); + }); + let rows_needed = StateCircuit::::min_num_rows_block(&block, &chunks[0]).1; let k = cmp::max(log2_ceil(rows_needed + NUM_BLINDING_ROWS), 18); - let state_circuit = StateCircuit::::new(&chunks[0]); - let max_rws = &chunks[0].fixed_param.max_rws; - let instance = state_circuit.instance(); - let prover = MockProver::::run(k, &state_circuit, instance).map_err(|err| { - CircuitTestError::SynthesisFailure { - circuit: Circuit::State, - reason: err, - } - })?; - // Skip verification of Start rows to accelerate testing - let non_start_rows_len = state_circuit - .rows + + chunks .iter() - .filter(|rw| !matches!(rw, Rw::Start { .. })) - .count(); - let rows = max_rws - non_start_rows_len..*max_rws; - prover.verify_at_rows(rows.clone(), rows).map_err(|err| { - CircuitTestError::VerificationFailed { - circuit: Circuit::EVM, - reasons: err, - } - }) + // terminate on first error + .find_map(|chunk| { + let state_circuit = StateCircuit::::new(chunk); + let instance = state_circuit.instance(); + let prover = MockProver::::run(k, &state_circuit, instance).map_err(|err| { + CircuitTestError::SynthesisFailure { + circuit: Circuit::State, + reason: err, + } + }); + if let Err(err) = prover { + return Some(Err(err)); + } + let prover = prover.unwrap(); + // Skip verification of Start and Padding rows accelerate testing + let non_padding_rows_len = state_circuit + .rows + .iter() + .filter(|rw| { + !matches!(rw, Rw::Start { .. }) && !matches!(rw, Rw::Padding { .. }) + }) + .count(); + let rows = 1..1 + non_padding_rows_len; + let result: Result<(), CircuitTestError> = prover + .verify_at_rows(rows.clone(), rows) + .map_err(|err| CircuitTestError::VerificationFailed { + circuit: Circuit::EVM, + reasons: err, + }); + if result.is_ok() { + None + } else { + Some(result) + } + }) + .unwrap_or_else(|| Ok(())) } /// Triggers the `CircuitTestBuilder` to convert the [`TestContext`] if any, From d04a971c2b0039954f89a49ca058501ce19eedd8 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Thu, 29 Feb 2024 17:45:55 +0800 Subject: [PATCH 14/16] unimplemented when chunk right after invalid_tx --- bus-mapping/src/circuit_input_builder.rs | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 26cb003798..7cd93c0900 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -341,17 +341,17 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { tx_ctx: TransactionContext, next_geth_step: Option<(usize, &GethExecStep)>, last_call: Option, - ) -> Result<(), Error> { + ) -> Result { // we dont chunk if // 1. on last chunk // 2. still got some buffer room before max_rws let Some(max_rws) = self.circuits_params.max_rws() else { // terminiate earlier due to no max_rws - return Ok(()); + return Ok(false); }; if self.chunk_ctx.is_last_chunk() || self.chunk_rws() + RW_BUFFER_SIZE < max_rws { - return Ok(()); + return Ok(false); }; // Optain the first op of the next GethExecStep, for fixed case also lookahead @@ -377,7 +377,7 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { self.commit_chunk_ctx(true, tx.id as usize, last_copy, last_call); self.set_begin_chunk(&next_ops, Some(&tx)); - Ok(()) + Ok(true) } /// Handle a transaction with its corresponding execution trace to generate @@ -451,14 +451,23 @@ impl<'a, C: CircuitsParams> CircuitInputBuilder { tx.steps_mut().push(end_tx_step.clone()); (end_tx_step, last_call) } else if self.feature_config.invalid_tx { - // chunk before hands to avoid chunk between [InvalidTx, BeginTx) - self.check_and_chunk(geth_trace, tx.clone(), tx_ctx.clone(), None, None)?; // Generate InvalidTx step let invalid_tx_step = gen_associated_steps( &mut self.state_ref(&mut tx, &mut tx_ctx), ExecState::InvalidTx, )?; tx.steps_mut().push(invalid_tx_step.clone()); + // Peek the end_tx_step + let is_chunk = + self.check_and_chunk(geth_trace, tx.clone(), tx_ctx.clone(), None, None)?; + if is_chunk { + // TODO we dont support chunk after invalid_tx + // becasuse begin_chunk will constraints what next step execution state. + // And for next step either begin_tx or invalid_tx will both failed because + // begin_tx/invalid_tx define new execution state. + unimplemented!("dont support invalid_tx with multiple chunks") + } + (invalid_tx_step, None) } else { panic!("invalid tx support not enabled") From a7e3610b37dd1ff50fea6d997d64792b0b72f8c6 Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Thu, 29 Feb 2024 17:58:29 +0800 Subject: [PATCH 15/16] fix doc test --- zkevm-circuits/src/test_util.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 14c860c704..6d32d3090b 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -78,8 +78,12 @@ const NUM_BLINDING_ROWS: usize = 64; /// .unwrap(); /// /// CircuitTestBuilder::new_from_test_ctx(ctx) -/// .block_modifier(Box::new(|block, chunk| chunk.fixed_param.max_evm_rows = (1 << 18) - 100)) -/// .run(); +/// .block_modifier(Box::new(|_block, chunk| { +/// chunk +/// .iter_mut() +/// .for_each(|chunk| chunk.fixed_param.max_evm_rows = (1 << 18) - 100); +/// })) +/// .run() /// ``` pub struct CircuitTestBuilder { test_ctx: Option>, From 6a39447608778bb6a77089b18fab196876ba9a7d Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Fri, 1 Mar 2024 10:03:27 +0800 Subject: [PATCH 16/16] chores: cleaup log --- .github/workflows/main-tests.yml | 6 +-- zkevm-circuits/src/evm_circuit/execution.rs | 14 ------ zkevm-circuits/src/evm_circuit/step.rs | 5 -- .../state_circuit/lexicographic_ordering.rs | 11 ----- zkevm-circuits/src/table/rw_table.rs | 46 +++++++++---------- 5 files changed, 23 insertions(+), 59 deletions(-) diff --git a/.github/workflows/main-tests.yml b/.github/workflows/main-tests.yml index 4af43069fb..5a4826347a 100644 --- a/.github/workflows/main-tests.yml +++ b/.github/workflows/main-tests.yml @@ -133,16 +133,12 @@ jobs: sudo chmod +x /usr/bin/solc - name: Update ERC20 git submodule run: git submodule update --init --recursive --checkout integration-tests/contracts/vendor/openzeppelin-contracts - - name: Run root test - uses: actions-rs/cargo@v1 - with: - command: test - args: --package zkevm-circuits --lib -- root_circuit::test::test_root_circuit_multiple_chunk --exact --ignored - name: Run heavy tests # heavy tests are run serially to avoid OOM uses: actions-rs/cargo@v1 with: command: test args: --verbose --release --all --all-features --exclude integration-tests --exclude circuit-benchmarks serial_ -- --ignored --test-threads 1 + build: needs: [skip_check] if: | diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 7fa128bbd8..35d348a359 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -1443,13 +1443,6 @@ impl ExecutionConfig { step, call ); - // println!( - // "assign_exec_step offset: {} state {:?} step: {:?} call: {:?}", - // offset, - // step.execution_state(), - // step, - // call - // ); } // Make the region large enough for the current step and the next step. // The next step's next step may also be accessed, so make the region large @@ -1467,13 +1460,6 @@ impl ExecutionConfig { // so their witness values need to be known to be able // to correctly calculate the intermediate value. if let Some(next_step) = next_step { - // println!( - // "assign_exec_step nextstep offset: {} state {:?} step: {:?} call: {:?}", - // offset, - // next_step.2.execution_state(), - // next_step.2, - // next_step.1 - // ); self.assign_exec_step_int( region, offset + height, diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index 181f6d1b4f..9d90677b42 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -850,11 +850,6 @@ impl Step { offset, Value::known(F::from(step.rwc_inner_chunk.into())), )?; - // println!( - // "execstate {:?} self.state.call_id {:?}", - // step.execution_state(), - // F::from(call.call_id as u64) - // ); self.state .call_id .assign(region, offset, Value::known(F::from(call.call_id as u64)))?; diff --git a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs index cd81f6752e..67953ea37c 100644 --- a/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs +++ b/zkevm-circuits/src/state_circuit/lexicographic_ordering.rs @@ -99,7 +99,6 @@ pub struct Config { pub(crate) selector: Column, pub first_different_limb: BinaryNumberConfig, limb_difference: Column, - // limb_difference_inverse: Column, } impl Config { @@ -112,27 +111,17 @@ impl Config { let selector = meta.fixed_column(); let first_different_limb = BinaryNumberChip::configure(meta, selector, None); let limb_difference = meta.advice_column(); - // let limb_difference_inverse = meta.advice_column(); let config = Config { selector, first_different_limb, limb_difference, - // limb_difference_inverse, }; lookup.range_check_u16(meta, "limb_difference fits into u16", |meta| { meta.query_advice(limb_difference, Rotation::cur()) }); - // meta.create_gate("limb_difference is not zero", |meta| { - // let selector = meta.query_fixed(selector, Rotation::cur()); - // let limb_difference = meta.query_advice(limb_difference, Rotation::cur()); - // let limb_difference_inverse = - // meta.query_advice(limb_difference_inverse, Rotation::cur()); - // vec![selector * (1.expr() - limb_difference * limb_difference_inverse)] - // }); - meta.create_gate( "limb differences before first_different_limb are all 0", |meta| { diff --git a/zkevm-circuits/src/table/rw_table.rs b/zkevm-circuits/src/table/rw_table.rs index c20da18c60..94a10852e5 100644 --- a/zkevm-circuits/src/table/rw_table.rs +++ b/zkevm-circuits/src/table/rw_table.rs @@ -89,30 +89,28 @@ impl RwTable { /// Construct a new RwTable pub fn construct(meta: &mut ConstraintSystem) -> Self { Self { - // TODO upgrade halo2 to use `unblinded_advice_column` - // https://github.com/privacy-scaling-explorations/halo2/blob/main/halo2_proofs/examples/vector-ops-unblinded.rs - // rw_counter: meta.unblinded_advice_column(), - // is_write: meta.unblinded_advice_column(), - // tag: meta.unblinded_advice_column(), - // id: meta.unblinded_advice_column(), - // address: meta.unblinded_advice_column(), - // field_tag: meta.unblinded_advice_column(), - // storage_key: word::Word::new([meta.unblinded_advice_column(), - // meta.unblinded_advice_column()]), value: word::Word::new([meta. - // unblinded_advice_column(), meta.unblinded_advice_column()]), value_prev: - // word::Word::new([meta.unblinded_advice_column(), meta.unblinded_advice_column()]), - // init_val: word::Word::new([meta.unblinded_advice_column(), - // meta.unblinded_advice_column()]), - rw_counter: meta.advice_column(), - is_write: meta.advice_column(), - tag: meta.advice_column(), - id: meta.advice_column(), - address: meta.advice_column(), - field_tag: meta.advice_column(), - storage_key: word::Word::new([meta.advice_column(), meta.advice_column()]), - value: word::Word::new([meta.advice_column(), meta.advice_column()]), - value_prev: word::Word::new([meta.advice_column(), meta.advice_column()]), - init_val: word::Word::new([meta.advice_column(), meta.advice_column()]), + rw_counter: meta.unblinded_advice_column(), + is_write: meta.unblinded_advice_column(), + tag: meta.unblinded_advice_column(), + id: meta.unblinded_advice_column(), + address: meta.unblinded_advice_column(), + field_tag: meta.unblinded_advice_column(), + storage_key: word::Word::new([ + meta.unblinded_advice_column(), + meta.unblinded_advice_column(), + ]), + value: word::Word::new([ + meta.unblinded_advice_column(), + meta.unblinded_advice_column(), + ]), + value_prev: word::Word::new([ + meta.unblinded_advice_column(), + meta.unblinded_advice_column(), + ]), + init_val: word::Word::new([ + meta.unblinded_advice_column(), + meta.unblinded_advice_column(), + ]), } } fn assign(