From 43817d3354394dddd400c848521f719c252b708e Mon Sep 17 00:00:00 2001 From: Akase Cho Date: Mon, 9 Oct 2023 13:29:14 +0800 Subject: [PATCH] ccc performance optimization (#970) * construct Address direct * aggressive inline hotspot * cache key * don't sort for from_rws_with_mock_state_roots * mark possible optimization * wrap error string with arc * Revert "wrap error string with arc" This reverts commit ad05aff272b1ec093e053fa684a32565d5bf2be5. * exclude original_rws for release build * [wip] reduce clone in ccc path (#973) * estimate_circuit_capacity take BlockTrace ownership * Convert ExecutionResult with explicit clone * add block_trace_to_witness_block * construct from ref * sync default mod * fix order * clippy * assertion * dirty fix * extract common * clippy && fmt * fix clippy * async drop (#981) * skip small drop * add comments * fix typo --------- Co-authored-by: Zhang Zhuo --- bus-mapping/src/circuit_input_builder/l2.rs | 24 +- eth-types/src/l2_types.rs | 46 +-- external-tracer/src/lib.rs | 2 +- mock/src/test_ctx.rs | 3 +- prover/src/inner/prover/mock.rs | 6 +- prover/src/utils.rs | 2 +- prover/src/zkevm/capacity_checker.rs | 23 +- prover/src/zkevm/circuit.rs | 24 +- prover/src/zkevm/circuit/l1_builder.rs | 8 +- prover/src/zkevm/circuit/l2_builder.rs | 75 +++-- testool/src/statetest/executor.rs | 7 +- zkevm-circuits/src/super_circuit/test.rs | 2 +- zkevm-circuits/src/test_util.rs | 2 +- zkevm-circuits/src/witness/block.rs | 4 +- zkevm-circuits/src/witness/mpt.rs | 301 ++++++++------------ zkevm-circuits/src/witness/rw.rs | 23 +- 16 files changed, 287 insertions(+), 265 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/l2.rs b/bus-mapping/src/circuit_input_builder/l2.rs index 30780bb5ac..117cea4084 100644 --- a/bus-mapping/src/circuit_input_builder/l2.rs +++ b/bus-mapping/src/circuit_input_builder/l2.rs @@ -204,7 +204,7 @@ fn dump_code_db(cdb: &CodeDB) { } impl CircuitInputBuilder { - fn apply_l2_trace(&mut self, block_trace: &BlockTrace, is_last: bool) -> Result<(), Error> { + fn apply_l2_trace(&mut self, block_trace: BlockTrace, is_last: bool) -> Result<(), Error> { log::trace!( "apply_l2_trace start, block num {:?}, is_last {is_last}", block_trace.header.number @@ -214,12 +214,12 @@ impl CircuitInputBuilder { dump_code_db(&self.code_db); } + let eth_block = EthBlock::from(&block_trace); let geth_trace: Vec = block_trace .execution_results - .iter() + .into_iter() .map(From::from) .collect(); - let eth_block: EthBlock = block_trace.clone().into(); assert_eq!( self.block.chain_id, block_trace.chain_id, "unexpected chain id in new block_trace" @@ -241,6 +241,16 @@ impl CircuitInputBuilder { // note the actions when `handle_rwc_reversion` argument (the 4th one) // is true is executing outside this closure self.handle_block_inner(ð_block, &geth_trace, false, is_last)?; + // TODO: remove this when GethExecStep don't contains heap data + // send to another thread to drop the heap data + // here we use a magic number from benchmark to decide whether to + // spawn-drop or not + if !geth_trace.is_empty() && geth_trace[0].struct_logs.len() > 2000 { + std::thread::spawn(move || { + std::mem::drop(eth_block); + std::mem::drop(geth_trace); + }); + } log::debug!("apply_l2_trace done for block {:?}", block_num); //self.sdb.list_accounts(); Ok(()) @@ -286,7 +296,7 @@ impl CircuitInputBuilder { /// Create a new CircuitInputBuilder from the given `l2_trace` and `circuits_params` pub fn new_from_l2_trace( circuits_params: CircuitsParams, - l2_trace: &BlockTrace, + l2_trace: BlockTrace, more: bool, light_mode: bool, ) -> Result { @@ -346,7 +356,7 @@ impl CircuitInputBuilder { let mut code_db = CodeDB::new(); code_db.insert(Vec::new()); - update_codedb(&mut code_db, &sdb, l2_trace)?; + update_codedb(&mut code_db, &sdb, &l2_trace)?; let mut builder_block = circuit_input_builder::Block::from_headers(&[], circuits_params); builder_block.chain_id = chain_id; @@ -365,7 +375,7 @@ impl CircuitInputBuilder { } /// ... - pub fn add_more_l2_trace(&mut self, l2_trace: &BlockTrace, more: bool) -> Result<(), Error> { + pub fn add_more_l2_trace(&mut self, l2_trace: BlockTrace, more: bool) -> Result<(), Error> { // update init state new data from storage if let Some(mpt_init_state) = &mut self.mpt_init_state { mpt_init_state.update_from_trace( @@ -419,7 +429,7 @@ impl CircuitInputBuilder { *self.sdb.get_storage_mut(&addr, &key).1 = val; } - update_codedb(&mut self.code_db, &self.sdb, l2_trace)?; + update_codedb(&mut self.code_db, &self.sdb, &l2_trace)?; self.apply_l2_trace(l2_trace, !more)?; Ok(()) diff --git a/eth-types/src/l2_types.rs b/eth-types/src/l2_types.rs index d7b41c669e..c3ab407354 100644 --- a/eth-types/src/l2_types.rs +++ b/eth-types/src/l2_types.rs @@ -35,9 +35,9 @@ pub struct BlockTrace { } impl From for EthBlock { - fn from(mut b: BlockTrace) -> Self { + fn from(b: BlockTrace) -> Self { let mut txs = Vec::new(); - for (idx, tx_data) in b.transactions.iter_mut().enumerate() { + for (idx, tx_data) in b.transactions.iter().enumerate() { let tx_idx = Some(U64::from(idx)); let tx = tx_data.to_eth_tx(b.header.hash, b.header.number, tx_idx); txs.push(tx) @@ -50,6 +50,22 @@ impl From for EthBlock { } } +impl From<&BlockTrace> for EthBlock { + fn from(b: &BlockTrace) -> Self { + let mut txs = Vec::new(); + for (idx, tx_data) in b.transactions.iter().enumerate() { + let tx_idx = Some(U64::from(idx)); + let tx = tx_data.to_eth_tx(b.header.hash, b.header.number, tx_idx); + txs.push(tx) + } + EthBlock { + transactions: txs, + difficulty: 0.into(), + ..b.header.clone() + } + } +} + /// l2 tx trace #[derive(Deserialize, Serialize, Debug, Clone)] pub struct TransactionTrace { @@ -183,20 +199,16 @@ pub struct ExecutionResult { pub exec_steps: Vec, } -impl From<&ExecutionResult> for GethExecTrace { - fn from(e: &ExecutionResult) -> Self { - let mut struct_logs = Vec::new(); - for exec_step in &e.exec_steps { - let step = exec_step.into(); - struct_logs.push(step) - } +impl From for GethExecTrace { + fn from(e: ExecutionResult) -> Self { + let struct_logs = e.exec_steps.into_iter().map(GethExecStep::from).collect(); GethExecTrace { l1_fee: e.l1_fee.as_u64(), gas: Gas(e.gas), failed: e.failed, - return_value: e.return_value.clone(), + return_value: e.return_value, struct_logs, - account_after: e.account_after.clone(), + account_after: e.account_after, } } } @@ -221,11 +233,11 @@ pub struct ExecStep { pub extra_data: Option, } -impl From<&ExecStep> for GethExecStep { - fn from(e: &ExecStep) -> Self { - let stack = e.stack.clone().map_or_else(Stack::new, Stack::from); - let storage = e.storage.clone().map_or_else(Storage::empty, Storage::from); - let memory = e.memory.clone().map_or_else(Memory::default, Memory::from); +impl From for GethExecStep { + fn from(e: ExecStep) -> Self { + let stack = e.stack.map_or_else(Stack::new, Stack::from); + let storage = e.storage.map_or_else(Storage::empty, Storage::from); + let memory = e.memory.map_or_else(Memory::default, Memory::from); GethExecStep { pc: ProgramCounter(e.pc as usize), @@ -235,7 +247,7 @@ impl From<&ExecStep> for GethExecStep { gas_cost: GasCost(e.gas_cost), refund: Gas(e.refund), depth: e.depth as u16, - error: e.error.clone(), + error: e.error, stack, memory, storage, diff --git a/external-tracer/src/lib.rs b/external-tracer/src/lib.rs index 5575b5561c..a90b099db7 100644 --- a/external-tracer/src/lib.rs +++ b/external-tracer/src/lib.rs @@ -144,7 +144,7 @@ pub fn trace(config: &TraceConfig) -> Result, Error> { Ok(block_trace .execution_results - .iter() + .into_iter() .map(From::from) .collect::>()) } diff --git a/mock/src/test_ctx.rs b/mock/src/test_ctx.rs index 7cc3b52b0b..472e983e60 100644 --- a/mock/src/test_ctx.rs +++ b/mock/src/test_ctx.rs @@ -196,7 +196,8 @@ impl TestContext { #[cfg(feature = "scroll")] let geth_traces = block_trace .execution_results - .iter() + .clone() + .into_iter() .map(From::from) .collect::>(); diff --git a/prover/src/inner/prover/mock.rs b/prover/src/inner/prover/mock.rs index 2d1c8976b6..af1afa4b61 100644 --- a/prover/src/inner/prover/mock.rs +++ b/prover/src/inner/prover/mock.rs @@ -10,11 +10,11 @@ use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr}; use zkevm_circuits::witness::Block; impl Prover { - pub fn mock_prove_target_circuit(block_trace: &BlockTrace) -> anyhow::Result<()> { - Self::mock_prove_target_circuit_batch(&[block_trace.clone()]) + pub fn mock_prove_target_circuit(block_trace: BlockTrace) -> anyhow::Result<()> { + Self::mock_prove_target_circuit_batch(vec![block_trace]) } - pub fn mock_prove_target_circuit_batch(block_traces: &[BlockTrace]) -> anyhow::Result<()> { + pub fn mock_prove_target_circuit_batch(block_traces: Vec) -> anyhow::Result<()> { let witness_block = block_traces_to_witness_block(block_traces)?; Self::mock_prove_witness_block(&witness_block) } diff --git a/prover/src/utils.rs b/prover/src/utils.rs index bef28a9f3f..cc1df32096 100644 --- a/prover/src/utils.rs +++ b/prover/src/utils.rs @@ -162,7 +162,7 @@ pub fn chunk_trace_to_witness_block(mut chunk_trace: Vec) -> Result< // Check if the trace exceeds the circuit capacity. check_batch_capacity(&mut chunk_trace)?; - block_traces_to_witness_block(&chunk_trace) + block_traces_to_witness_block(chunk_trace) } // Return the output dir. diff --git a/prover/src/zkevm/capacity_checker.rs b/prover/src/zkevm/capacity_checker.rs index c747ae0317..73eba4fe55 100644 --- a/prover/src/zkevm/capacity_checker.rs +++ b/prover/src/zkevm/capacity_checker.rs @@ -122,10 +122,6 @@ pub struct CircuitCapacityChecker { pub builder_ctx: Option<(CodeDB, StateDB, Option)>, } -// Currently TxTrace is same as BlockTrace, with "transactions" and "executionResults" should be of -// len 1, "storageProofs" should contain "slot touched" during when executing this tx. -pub type TxTrace = BlockTrace; - impl Default for CircuitCapacityChecker { fn default() -> Self { Self::new() @@ -162,10 +158,8 @@ impl CircuitCapacityChecker { } pub fn estimate_circuit_capacity( &mut self, - txs: &[TxTrace], + trace: BlockTrace, ) -> Result { - log::debug!("estimate_circuit_capacity with txs num {}", txs.len()); - assert!(!txs.is_empty()); let (mut estimate_builder, codedb_prev) = if let Some((code_db, sdb, mpt_state)) = self.builder_ctx.take() { // here we create a new builder for another (sealed) witness block @@ -175,13 +169,13 @@ impl CircuitCapacityChecker { // changed but we may not update it in light mode) let mut builder_block = circuit_input_builder::Block::from_headers(&[], get_super_circuit_params()); - builder_block.chain_id = txs[0].chain_id; - builder_block.start_l1_queue_index = txs[0].start_l1_queue_index; + builder_block.chain_id = trace.chain_id; + builder_block.start_l1_queue_index = trace.start_l1_queue_index; builder_block.prev_state_root = mpt_state .as_ref() .map(|state| state.root()) .map(|root| H256(*root)) - .unwrap_or(txs[0].header.state_root) + .unwrap_or(trace.header.state_root) .to_word(); // notice the trace has included all code required for builidng witness block, // so we do not need to pick them from previous one, but we still keep the @@ -196,22 +190,21 @@ impl CircuitCapacityChecker { } else { CircuitInputBuilder::new(sdb, CodeDB::new(), &builder_block) }; - builder.add_more_l2_trace(&txs[0], txs.len() > 1)?; + builder.add_more_l2_trace(trace, false)?; (builder, Some(code_db)) } else { ( CircuitInputBuilder::new_from_l2_trace( get_super_circuit_params(), - &txs[0], - txs.len() > 1, + trace, + false, self.light_mode, )?, None, ) }; - let traces = &txs[1..]; let witness_block = - block_traces_to_witness_block_with_updated_state(traces, &mut estimate_builder)?; + block_traces_to_witness_block_with_updated_state(vec![], &mut estimate_builder)?; let mut rows = calculate_row_usage_of_witness_block(&witness_block)?; let mut code_db = codedb_prev.unwrap_or_else(CodeDB::new); diff --git a/prover/src/zkevm/circuit.rs b/prover/src/zkevm/circuit.rs index 283c688455..95152ee3b8 100644 --- a/prover/src/zkevm/circuit.rs +++ b/prover/src/zkevm/circuit.rs @@ -13,9 +13,10 @@ mod l1_builder; use l1_builder as builder; mod super_circuit; pub use self::builder::{ - block_traces_to_witness_block, block_traces_to_witness_block_with_updated_state, - calculate_row_usage_of_trace, calculate_row_usage_of_witness_block, check_batch_capacity, - get_super_circuit_params, validite_block_traces, + block_trace_to_witness_block, block_traces_to_witness_block, + block_traces_to_witness_block_with_updated_state, calculate_row_usage_of_trace, + calculate_row_usage_of_witness_block, check_batch_capacity, get_super_circuit_params, + validite_block_traces, }; pub use super_circuit::SuperCircuit; @@ -52,19 +53,21 @@ pub trait TargetCircuit { where Self: Sized, { - Self::from_block_traces(&[]).unwrap().0 + Self::from_block_traces(vec![]).unwrap().0 } /// Build the inner circuit and the instances from a traces - fn from_block_trace(block_trace: &BlockTrace) -> anyhow::Result<(Self::Inner, Vec>)> + fn from_block_trace(block_trace: BlockTrace) -> anyhow::Result<(Self::Inner, Vec>)> where Self: Sized, { - Self::from_block_traces(std::slice::from_ref(block_trace)) + Self::from_block_traces(vec![block_trace]) } /// Build the inner circuit and the instances from a list of traces - fn from_block_traces(block_traces: &[BlockTrace]) -> anyhow::Result<(Self::Inner, Vec>)> + fn from_block_traces( + block_traces: Vec, + ) -> anyhow::Result<(Self::Inner, Vec>)> where Self: Sized, { @@ -79,7 +82,12 @@ pub trait TargetCircuit { where Self: Sized; - fn estimate_rows(block_traces: &[BlockTrace]) -> anyhow::Result { + fn estimate_block_rows(block_trace: BlockTrace) -> anyhow::Result { + let witness_block = block_trace_to_witness_block(block_trace)?; + Ok(Self::estimate_rows_from_witness_block(&witness_block)) + } + + fn estimate_rows(block_traces: Vec) -> anyhow::Result { let witness_block = block_traces_to_witness_block(block_traces)?; Ok(Self::estimate_rows_from_witness_block(&witness_block)) } diff --git a/prover/src/zkevm/circuit/l1_builder.rs b/prover/src/zkevm/circuit/l1_builder.rs index 06bb0714db..ffd0dec610 100644 --- a/prover/src/zkevm/circuit/l1_builder.rs +++ b/prover/src/zkevm/circuit/l1_builder.rs @@ -28,12 +28,16 @@ pub fn check_batch_capacity(_block_traces: &mut Vec) -> Result<()> { unimplemented!("Must build with feature scroll") } -pub fn block_traces_to_witness_block(_block_traces: &[BlockTrace]) -> Result> { +pub fn block_trace_to_witness_block(_block_traces: BlockTrace) -> Result> { + unimplemented!("Must build with feature scroll") +} + +pub fn block_traces_to_witness_block(_block_traces: Vec) -> Result> { unimplemented!("Must build with feature scroll") } pub fn block_traces_to_witness_block_with_updated_state( - _block_traces: &[BlockTrace], + _block_traces: Vec, _builder: &mut CircuitInputBuilder, _light_mode: bool, ) -> Result> { diff --git a/prover/src/zkevm/circuit/l2_builder.rs b/prover/src/zkevm/circuit/l2_builder.rs index 2dea455481..6b8793489a 100644 --- a/prover/src/zkevm/circuit/l2_builder.rs +++ b/prover/src/zkevm/circuit/l2_builder.rs @@ -12,8 +12,9 @@ use mpt_zktrie::state::{ZkTrieHash, ZktrieState}; use once_cell::sync::Lazy; use std::time::Instant; use zkevm_circuits::{ - evm_circuit::witness::{block_apply_mpt_state, block_convert_with_l1_queue_index, Block}, + evm_circuit::witness::{block_apply_mpt_state, Block}, util::SubCircuit, + witness::block_convert, }; static CHAIN_ID: Lazy = Lazy::new(|| read_env_var("CHAIN_ID", 53077)); @@ -61,9 +62,9 @@ pub fn get_super_circuit_params() -> CircuitsParams { // TODO: optimize it later pub fn calculate_row_usage_of_trace( - block_trace: &BlockTrace, + block_trace: BlockTrace, ) -> Result> { - let witness_block = block_traces_to_witness_block(std::slice::from_ref(block_trace))?; + let witness_block = block_traces_to_witness_block(vec![block_trace])?; calculate_row_usage_of_witness_block(&witness_block) } @@ -136,7 +137,7 @@ pub fn check_batch_capacity(block_traces: &mut Vec) -> Result<()> { let mut n_txs = 0; let mut truncate_idx = block_traces.len(); for (idx, block) in block_traces.iter().enumerate() { - let usage = calculate_row_usage_of_trace(block)? + let usage = calculate_row_usage_of_trace(block.clone())? .into_iter() .map(|x| crate::zkevm::SubCircuitRowUsage { name: x.name, @@ -227,8 +228,34 @@ pub fn validite_block_traces(block_traces: &[BlockTrace]) -> Result<()> { Ok(()) } -pub fn block_traces_to_witness_block(block_traces: &[BlockTrace]) -> Result> { - validite_block_traces(block_traces)?; +pub fn block_trace_to_witness_block(block_trace: BlockTrace) -> Result> { + let chain_id = block_trace.chain_id; + if *CHAIN_ID != chain_id { + bail!( + "CHAIN_ID env var is wrong. chain id in trace {chain_id}, CHAIN_ID {}", + *CHAIN_ID + ); + } + let total_tx_num = block_trace.transactions.len(); + if total_tx_num > MAX_TXS { + bail!( + "block {}tx num overflow {total_tx_num}", + block_trace.header.number.unwrap() + ); + } + log::info!("block_trace_to_witness_block, tx num {total_tx_num}"); + log::debug!("start_l1_queue_index: {}", block_trace.start_l1_queue_index); + let mut builder = CircuitInputBuilder::new_from_l2_trace( + get_super_circuit_params(), + block_trace, + false, + false, + )?; + block_traces_to_witness_block_with_updated_state(vec![], &mut builder) +} + +pub fn block_traces_to_witness_block(block_traces: Vec) -> Result> { + validite_block_traces(&block_traces)?; let block_num = block_traces.len(); let total_tx_num = block_traces .iter() @@ -247,7 +274,7 @@ pub fn block_traces_to_witness_block(block_traces: &[BlockTrace]) -> Result Result 1, + traces.next().unwrap(), + block_traces_len > 1, false, )?; - block_traces_to_witness_block_with_updated_state(&block_traces[1..], &mut builder) + let witness = block_traces_to_witness_block_with_updated_state( + traces.collect(), // this is a cold path + &mut builder, + ); + // send to other thread to drop + std::thread::spawn(move || drop(builder.block)); + witness } } @@ -272,16 +307,12 @@ pub fn block_traces_to_witness_block(block_traces: &[BlockTrace]) -> Result, builder: &mut CircuitInputBuilder, ) -> Result> { let metric = |builder: &CircuitInputBuilder, idx: usize| -> Result<(), bus_mapping::Error> { let t = Instant::now(); - let block = block_convert_with_l1_queue_index::( - &builder.block, - &builder.code_db, - builder.block.start_l1_queue_index, - )?; + let block = block_convert(&builder.block, &builder.code_db)?; log::debug!("block convert time {:?}", t.elapsed()); let rows = ::Inner::min_num_rows_block(&block); log::debug!( @@ -312,8 +343,9 @@ pub fn block_traces_to_witness_block_with_updated_state( 1 }; - for (idx, block_trace) in block_traces.iter().enumerate() { - let is_last = idx == block_traces.len() - 1; + let block_traces_len = block_traces.len(); + for (idx, block_trace) in block_traces.into_iter().enumerate() { + let is_last = idx == block_traces_len - 1; log::debug!( "add_more_l2_trace idx {idx}, block num {:?}", block_trace.header.number @@ -325,11 +357,10 @@ pub fn block_traces_to_witness_block_with_updated_state( } builder.finalize_building()?; - let start_l1_queue_index = builder.block.start_l1_queue_index; log::debug!("converting builder.block to witness block"); - let mut witness_block = - block_convert_with_l1_queue_index(&builder.block, &builder.code_db, start_l1_queue_index)?; + + let mut witness_block = block_convert(&builder.block, &builder.code_db)?; log::debug!( "witness_block built with circuits_params {:?}", witness_block.circuits_params diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs index c12d9779c9..8339941b4a 100644 --- a/testool/src/statetest/executor.rs +++ b/testool/src/statetest/executor.rs @@ -291,7 +291,8 @@ fn trace_config_to_witness_block_l2( let geth_traces = block_trace .execution_results - .iter() + .clone() + .into_iter() .map(From::from) .collect::>(); // if the trace exceed max steps, we cannot fit it into circuit @@ -307,7 +308,7 @@ fn trace_config_to_witness_block_l2( let difficulty_be_bytes = [0u8; 32]; env::set_var("DIFFICULTY", hex::encode(difficulty_be_bytes)); let mut builder = - CircuitInputBuilder::new_from_l2_trace(circuits_params, &block_trace, false, false) + CircuitInputBuilder::new_from_l2_trace(circuits_params, block_trace, false, false) .expect("could not handle block tx"); builder .finalize_building() @@ -749,7 +750,7 @@ fn mock_prove(test_id: &str, witness_block: &Block) { // TODO: do we need to automatically adjust this k? let k = 20; // TODO: remove this MOCK_RANDOMNESS? - let circuit = ScrollSuperCircuit::new_from_block(&witness_block); + let circuit = ScrollSuperCircuit::new_from_block(witness_block); let instance = circuit.instance(); let prover = MockProver::run(k, &circuit, instance).unwrap(); prover.assert_satisfied_par(); diff --git a/zkevm-circuits/src/super_circuit/test.rs b/zkevm-circuits/src/super_circuit/test.rs index b9dc1314ca..f368bd44f6 100644 --- a/zkevm-circuits/src/super_circuit/test.rs +++ b/zkevm-circuits/src/super_circuit/test.rs @@ -48,7 +48,7 @@ fn test_super_circuit< set_var("DIFFICULTY", hex::encode(difficulty_be_bytes)); let mut builder = - CircuitInputBuilder::new_from_l2_trace(circuits_params, &l2_trace, false, false) + CircuitInputBuilder::new_from_l2_trace(circuits_params, l2_trace, false, false) .expect("could not handle block tx"); builder diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 2b82c5665f..39358f91cd 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -221,7 +221,7 @@ impl CircuitTestBuilder { { let mut builder = CircuitInputBuilder::new_from_l2_trace( params, - self.test_ctx.unwrap().l2_trace(), + self.test_ctx.unwrap().l2_trace().clone(), false, false, ) diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index b2ede6153e..5e587e4aaa 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -482,8 +482,8 @@ pub fn block_convert( block.circuits_params.max_rws }; - let mpt_updates = MptUpdates::from_rws_with_mock_state_roots( - &rws.table_assignments(), + let mpt_updates = MptUpdates::from_unsorted_rws_with_mock_state_roots( + &rws.table_assignments_unsorted(), block.prev_state_root, block.end_state_root(), ); diff --git a/zkevm-circuits/src/witness/mpt.rs b/zkevm-circuits/src/witness/mpt.rs index 55ee429c7b..479811b863 100644 --- a/zkevm-circuits/src/witness/mpt.rs +++ b/zkevm-circuits/src/witness/mpt.rs @@ -12,11 +12,9 @@ use mpt_zktrie::{ state, state::witness::WitnessGenerator, }; - use serde::{Deserialize, Serialize}; -use std::collections::BTreeMap; - pub use state::ZktrieState; +use std::collections::BTreeMap; /// Used to store withdraw proof #[derive(Default, Debug, Clone, Serialize, Deserialize)] @@ -30,7 +28,7 @@ pub struct WithdrawProof { } /// An MPT update whose validity is proved by the MptCircuit -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] pub struct MptUpdate { key: Key, old_value: Word, @@ -38,9 +36,60 @@ pub struct MptUpdate { old_root: Word, new_root: Word, // for debugging + #[cfg(debug_assertions)] original_rws: Vec, } +impl MptUpdate { + fn from_rows( + key: Key, + rows: Vec, + offset: usize, + total_lens: usize, + old_root: Word, + new_root: Word, + ) -> (Key, Self) { + let first = &rows[0]; + let last = rows.iter().last().unwrap_or(first); + let key_exists = key; + let key = key.set_non_exists(value_prev(first), value(last)); + ( + key_exists, + MptUpdate { + key, + old_root: Word::from(offset as u64) + old_root, + new_root: if offset + 1 == total_lens { + new_root + } else { + Word::from(offset as u64 + 1) + old_root + }, + old_value: value_prev(first), + new_value: value(last), + #[cfg(debug_assertions)] + original_rws: rows, + }, + ) + } +} + +// just for convenience +impl Default for MptUpdate { + fn default() -> Self { + Self { + key: Key::Account { + address: Address::zero(), + field_tag: AccountFieldTag::Nonce, + }, + old_value: Word::zero(), + new_value: Word::zero(), + old_root: Word::zero(), + new_root: Word::zero(), + #[cfg(debug_assertions)] + original_rws: Vec::new(), + } + } +} + /// All the MPT updates in the MptCircuit, accessible by their key #[derive(Default, Clone, Debug)] pub struct MptUpdates { @@ -211,6 +260,41 @@ impl MptUpdates { Self::from_rws_with_mock_state_roots(rows, 0xcafeu64.into(), 0xdeadbeefu64.into()) } + pub(crate) fn from_unsorted_rws_with_mock_state_roots( + rows: &[Rw], + old_root: U256, + new_root: U256, + ) -> Self { + log::debug!("mpt update roots (mocking) {:?} {:?}", old_root, new_root); + let rows_len = rows.len(); + let mut updates = BTreeMap::new(); // TODO: preallocate + for (key, row) in rows.iter().filter_map(|row| key(row).map(|key| (key, row))) { + updates.entry(key).or_insert_with(Vec::new).push(*row); // TODO: preallocate + } + let updates: BTreeMap<_, _> = updates + .into_iter() + .enumerate() + .map(|(i, (key, rows))| { + MptUpdate::from_rows(key, rows, i, rows_len, old_root, new_root) + }) + .collect(); + let mpt_updates = MptUpdates { + updates, + old_root, + new_root, + ..Default::default() + }; + // FIXME: we can remove this assert after the code runs a while and everything is ok? + #[cfg(debug_assertions)] + { + let mut rows = rows.to_vec(); + rows.sort_by_key(Rw::as_key); + let old_updates = Self::from_rws_with_mock_state_roots(&rows, old_root, new_root); + assert_eq!(old_updates.updates, mpt_updates.updates); + } + mpt_updates + } + pub(crate) fn from_rws_with_mock_state_roots( rows: &[Rw], old_root: U256, @@ -226,25 +310,7 @@ impl MptUpdates { .enumerate() .map(|(i, (key, rows))| { let rows: Vec = rows.copied().collect_vec(); - let first = &rows[0]; - let last = rows.iter().last().unwrap_or(first); - let key_exists = key; - let key = key.set_non_exists(value_prev(first), value(last)); - ( - key_exists, - MptUpdate { - key, - old_root: Word::from(i as u64) + old_root, - new_root: if i + 1 == rows_len { - new_root - } else { - Word::from(i as u64 + 1) + old_root - }, - old_value: value_prev(first), - new_value: value(last), - original_rws: rows, - }, - ) + MptUpdate::from_rows(key, rows, i, rows_len, old_root, new_root) }) .collect(); MptUpdates { @@ -489,11 +555,8 @@ mod test { }; let update = MptUpdate { key, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), new_root: Word::one(), - original_rws: Default::default(), + ..Default::default() }; let mut updates = MptUpdates::default(); @@ -512,11 +575,8 @@ mod test { }; let update = MptUpdate { key, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), new_root: Word::one(), - original_rws: Default::default(), + ..Default::default() }; let mut updates = MptUpdates::default(); @@ -535,11 +595,8 @@ mod test { address, field_tag: AccountFieldTag::Nonce, }, - old_value: Word::zero(), new_value: Word::one(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() } } @@ -593,11 +650,7 @@ mod test { address, field_tag: AccountFieldTag::NonExisting, }, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots(&ZktrieState::default()); @@ -661,11 +714,8 @@ mod test { address, field_tag: AccountFieldTag::Balance, }, - old_value: Word::zero(), new_value: Word::from(u64::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() } } @@ -770,11 +820,8 @@ mod test { address, field_tag: AccountFieldTag::CodeSize, }, - old_value: Word::zero(), new_value: Word::from(23412341231u64), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }; updates.insert(update); @@ -809,11 +856,8 @@ mod test { address, field_tag: AccountFieldTag::CodeHash, }, - old_value: Word::zero(), new_value: Word::from(234123124231231u64), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }; updates.insert(update); @@ -848,11 +892,8 @@ mod test { address, field_tag: AccountFieldTag::KeccakCodeHash, }, - old_value: Word::zero(), new_value: U256([u64::MAX; 4]), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }; updates.insert(update); @@ -886,11 +927,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::from(i), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); } updates.insert(MptUpdate { @@ -900,11 +938,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::from(2431230), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -915,9 +950,7 @@ mod test { }, old_value: Word::from(u32::MAX), new_value: Word::MAX - Word::from(u64::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -948,11 +981,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::from(2431230), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -963,9 +993,7 @@ mod test { }, old_value: Word::from(u32::MAX), new_value: Word::from(24), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -974,11 +1002,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::from(2431230), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -989,9 +1014,7 @@ mod test { }, old_value: Word::from(u32::MAX), new_value: Word::from(24), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1056,11 +1079,7 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::from(u64::MAX), }, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }; updates.insert(update); @@ -1091,11 +1110,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::MAX, }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }; updates.insert(update); @@ -1126,11 +1142,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::MAX - Word::from(43), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -1139,11 +1152,7 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::MAX, }, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1173,11 +1182,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::MAX - Word::from(43), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -1186,11 +1192,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::MAX, }, - old_value: Word::zero(), new_value: Word::one(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1220,11 +1223,8 @@ mod test { exists: false, storage_key: Word::MAX - Word::from(43), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -1234,10 +1234,7 @@ mod test { storage_key: Word::MAX - Word::from(43), }, old_value: Word::from(u32::MAX), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1267,11 +1264,8 @@ mod test { exists: false, storage_key: Word::MAX - Word::from(43), }, - old_value: Word::zero(), new_value: Word::MAX - Word::from(43), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -1280,11 +1274,8 @@ mod test { exists: false, storage_key: Word::MAX - Word::from(12312), }, - old_value: Word::zero(), new_value: Word::from(u64::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -1294,10 +1285,7 @@ mod test { storage_key: Word::MAX - Word::from(43), }, old_value: Word::MAX - Word::from(43), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1328,11 +1316,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::from(1000 * i), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); } updates.insert(MptUpdate { @@ -1343,10 +1328,7 @@ mod test { storage_key: Word::from(2000), }, old_value: Word::from(u32::MAX), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1376,11 +1358,7 @@ mod test { exists: false, storage_key: Word::from(2000), }, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1410,11 +1388,8 @@ mod test { exists: false, storage_key: Word::from(1000), }, - old_value: Word::zero(), new_value: Word::one(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.insert(MptUpdate { key: Key::AccountStorage { @@ -1423,11 +1398,7 @@ mod test { exists: false, storage_key: Word::from(2000), }, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1458,11 +1429,8 @@ mod test { exists: false, // true causes an unwraprror? nope.... storage_key: Word::from(1000 * i), }, - old_value: Word::zero(), new_value: Word::from(u32::MAX), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); } updates.insert(MptUpdate { @@ -1472,11 +1440,7 @@ mod test { exists: false, storage_key: Word::from(3), }, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1503,11 +1467,8 @@ mod test { exists: false, storage_key: Word::from(3), }, - old_value: Word::zero(), new_value: Word::one(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); } @@ -1519,11 +1480,7 @@ mod test { exists: false, storage_key: Word::from(3), }, - old_value: Word::zero(), - new_value: Word::zero(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); @@ -1550,11 +1507,8 @@ mod test { exists: false, storage_key: Word::from(3), }, - old_value: Word::zero(), new_value: Word::one(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); } @@ -1566,11 +1520,8 @@ mod test { exists: false, storage_key: Word::from(3), }, - old_value: Word::zero(), new_value: Word::one(), - old_root: Word::zero(), - new_root: Word::zero(), - original_rws: Default::default(), + ..Default::default() }); updates.fill_state_roots_from_generator(WitnessGenerator::from(&ZktrieState::default())); diff --git a/zkevm-circuits/src/witness/rw.rs b/zkevm-circuits/src/witness/rw.rs index 1f805a1f34..1ab5c1707b 100644 --- a/zkevm-circuits/src/witness/rw.rs +++ b/zkevm-circuits/src/witness/rw.rs @@ -5,7 +5,7 @@ use bus_mapping::{ operation::{self, AccountField, CallContextField, TxLogField, TxReceiptField}, Error, }; -use eth_types::{Address, Field, ToAddress, ToLittleEndian, ToScalar, Word, U256}; +use eth_types::{Address, Field, ToLittleEndian, ToScalar, Word, U256}; use halo2_proofs::{circuit::Value, halo2curves::bn256::Fr}; use itertools::Itertools; @@ -182,10 +182,16 @@ impl RwMap { let padding = (1..=padding_length).map(|rw_counter| Rw::Start { rw_counter }); (padding.chain(rows.into_iter()).collect(), padding_length) } + /// Build Rws for assignment + #[inline(always)] + pub fn table_assignments_unsorted(&self) -> Vec { + self.0.values().flatten().cloned().collect() + } + /// Build Rws for assignment pub fn table_assignments(&self) -> Vec { - let mut rows: Vec = self.0.values().flatten().cloned().collect(); - rows.sort_by_key(Rw::as_key); + let mut rows = self.table_assignments_unsorted(); + rows.sort_by_cached_key(Rw::as_key); rows } @@ -215,7 +221,7 @@ pub type RwKey = (u64, usize, Address, u64, Word); /// Read-write records in execution. Rws are used for connecting evm circuit and /// state circuits. -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum Rw { /// Start Start { rw_counter: usize }, @@ -585,6 +591,7 @@ impl Rw { } } + #[inline(always)] pub fn id(&self) -> Option { match self { Self::AccountStorage { tx_id, .. } @@ -600,6 +607,7 @@ impl Rw { } } + #[inline(always)] pub fn address(&self) -> Option
{ match self { Self::TxAccessListAccount { @@ -614,9 +622,9 @@ impl Rw { | Self::AccountStorage { account_address, .. } => Some(*account_address), - Self::Memory { memory_address, .. } => Some(U256::from(*memory_address).to_address()), + Self::Memory { memory_address, .. } => Some(Address::from_low_u64_be(*memory_address)), Self::Stack { stack_pointer, .. } => { - Some(U256::from(*stack_pointer as u64).to_address()) + Some(Address::from_low_u64_be(*stack_pointer as u64)) } Self::TxLog { log_id, @@ -634,6 +642,7 @@ impl Rw { } } + #[inline(always)] pub fn field_tag(&self) -> Option { match self { Self::Account { field_tag, .. } => Some(*field_tag as u64), @@ -652,6 +661,7 @@ impl Rw { } } + #[inline(always)] pub fn storage_key(&self) -> Option { match self { Self::AccountStorage { storage_key, .. } @@ -788,6 +798,7 @@ impl Rw { } } + #[inline(always)] pub(crate) fn as_key(&self) -> RwKey { ( self.tag() as u64,