From 5ad03df46f35f0ecbc764ba5f2f545b0fa8b713e Mon Sep 17 00:00:00 2001 From: "sm.wu" Date: Fri, 30 Jun 2023 15:47:59 +0800 Subject: [PATCH] fix conflicts --- .../src/circuit_input_builder/tracer_tests.rs | 32 +++--- bus-mapping/src/evm/opcodes/create.rs | 1 - zkevm-circuits/src/evm_circuit/execution.rs | 48 +++++++-- zkevm-circuits/src/evm_circuit/util.rs | 102 +++++++++--------- 4 files changed, 105 insertions(+), 78 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/tracer_tests.rs b/bus-mapping/src/circuit_input_builder/tracer_tests.rs index 227aeaf6a24..7d99e2e25bf 100644 --- a/bus-mapping/src/circuit_input_builder/tracer_tests.rs +++ b/bus-mapping/src/circuit_input_builder/tracer_tests.rs @@ -250,7 +250,7 @@ fn tracer_err_insufficient_balance() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -405,7 +405,7 @@ fn tracer_err_address_collision() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -525,7 +525,7 @@ fn tracer_create_collision_free() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -655,7 +655,7 @@ fn tracer_err_code_store_out_of_gas() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -804,7 +804,7 @@ fn tracer_err_invalid_code() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -901,7 +901,7 @@ fn tracer_err_max_code_size_exceeded() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1042,7 +1042,7 @@ fn tracer_create_stop() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1160,7 +1160,7 @@ fn tracer_err_invalid_jump() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1260,7 +1260,7 @@ fn tracer_err_execution_reverted() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1319,7 +1319,7 @@ fn tracer_stop() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1387,7 +1387,7 @@ fn tracer_err_return_data_out_of_bounds() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1540,7 +1540,7 @@ fn tracer_err_write_protection(is_call: bool) { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1742,7 +1742,7 @@ fn create2_address() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1840,7 +1840,7 @@ fn create_address() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -1924,7 +1924,7 @@ fn test_gen_access_trace() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), @@ -2146,7 +2146,7 @@ fn test_gen_access_trace_create_push_call_stack() { }, |mut txs, accs| { txs[0].to(accs[0].address).from(accs[2].address); - txs[1].to(accs[1].address).from(accs[2].address).nonce(1); + txs[1].to(accs[1].address).from(accs[2].address); }, |block, _tx| block.number(0xcafeu64), LoggerConfig::enable_memory(), diff --git a/bus-mapping/src/evm/opcodes/create.rs b/bus-mapping/src/evm/opcodes/create.rs index 277da2431f6..eb89b8af4ff 100644 --- a/bus-mapping/src/evm/opcodes/create.rs +++ b/bus-mapping/src/evm/opcodes/create.rs @@ -251,7 +251,6 @@ impl Opcode for Create { (CallContextField::IsStatic, false.to_word()), (CallContextField::IsCreate, true.to_word()), (CallContextField::CodeHash, code_hash.to_word()), - (CallContextField::Value, callee.value), ] { state.call_context_write(&mut exec_step, callee.call_id, field, value); } diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 71289a8a28b..db294b9b534 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -1394,6 +1394,15 @@ impl ExecutionConfig { } } + // TODO: We should find a better way to avoid this kind of special case. + // #1489 is the issue for this refactor. + if copy_lookup_count > 1 { + log::warn!("The number of copy events is more than 1, it's not supported by current design. Stop checking this step: {:?}", + step + ); + return; + } + let rlc_assignments: BTreeSet<_> = (0..step.rw_indices_len()) .map(|index| block.get_rws(step, index)) .map(|rw| rw.table_assignment().unwrap().rlc(lookup_randomness)) @@ -1420,20 +1429,24 @@ impl ExecutionConfig { // Check that the number of rw operations generated from the bus-mapping // correspond to the number of assigned rw lookups by the EVM Circuit - // plus the number of rw lookups done by the copy circuit. - if step.rw_indices_len() != assigned_rw_values.len() + step.copy_rw_counter_delta as usize { + // plus the number of rw lookups done by the copy circuit + // minus the number of copy lookup event. + if step.rw_indices_len() + != assigned_rw_values.len() + step.copy_rw_counter_delta as usize - copy_lookup_count + { log::error!( - "step.rw_indices.len: {} != assigned_rw_values.len: {} + step.copy_rw_counter_delta: {} in step: {:?}", + "step.rw_indices.len: {} != assigned_rw_values.len: {} + step.copy_rw_counter_delta: {} - copy_lookup_count: {} in step: {:?}", step.rw_indices_len(), assigned_rw_values.len(), step.copy_rw_counter_delta, copy_lookup_count, + step ); } let mut rev_count = 0; - let offset = 0; - let copy_lookup_processed = false; + let mut offset = 0; + let mut copy_lookup_processed = false; for (idx, assigned_rw_value) in assigned_rw_values.iter().enumerate() { let is_rev = if assigned_rw_value.0.contains(" with reversion") { rev_count += 1; @@ -1451,11 +1464,26 @@ impl ExecutionConfig { // In the EVM Circuit, reversion rw lookups are assigned after their // corresponding rw lookup, but in the bus-mapping they are // generated at the end of the step. - let idx = if is_rev { - step.rw_indices_len() - rev_count - } else { - idx - rev_count + offset - copy_lookup_processed as usize - }; + // If assigned_rw_value is a `copy lookup` event, the following + // `step.copy_rw_counter_delta` rw lookups must be memory operations. + if assigned_rw_value.0.starts_with("copy lookup") { + for i in 0..step.copy_rw_counter_delta as usize { + let index = idx + i; + let rw = block.get_rws(step, index); + if rw.tag() != Target::Memory { + log::error!( + "incorrect rw memory witness from copy lookup.\n lookup name: \"{}\"\n {}th rw of step {:?}, rw: {:?}", + assigned_rw_value.0, + index, + step.execution_state(), + rw); + } + } + + offset = step.copy_rw_counter_delta as usize; + copy_lookup_processed = true; + continue; + } let rw = block.get_rws(step, idx); let table_assignments = rw.table_assignment(); diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index d80666d4c72..198c4d528f6 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -167,6 +167,57 @@ impl<'r, 'b, F: Field> CachedRegion<'r, 'b, F> { } } +#[derive(Debug, Clone)] +pub struct StoredExpression { + pub(crate) name: String, + cell: Cell, + cell_type: CellType, + expr: Expression, + expr_id: String, +} + +impl Hash for StoredExpression { + fn hash(&self, state: &mut H) { + self.expr_id.hash(state); + self.cell_type.hash(state); + } +} + +impl StoredExpression { + pub fn assign( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + ) -> Result, Error> { + let value = self.expr.evaluate( + &|scalar| Value::known(scalar), + &|_| unimplemented!("selector column"), + &|fixed_query| { + Value::known(region.get_fixed( + offset, + fixed_query.column_index(), + fixed_query.rotation(), + )) + }, + &|advice_query| { + Value::known(region.get_advice( + offset, + advice_query.column_index(), + advice_query.rotation(), + )) + }, + &|_| unimplemented!("instance column"), + &|challenge| *region.challenges().indexed()[challenge.index()], + &|a| -a, + &|a, b| a + b, + &|a, b| a * b, + &|a, scalar| a * Value::known(scalar), + ); + self.cell.assign(region, offset, value)?; + Ok(value) + } +} + #[allow(clippy::mut_range_bound)] pub(crate) fn evm_cm_distribute_advice( meta: &mut ConstraintSystem, @@ -221,57 +272,6 @@ pub(crate) fn evm_cm_distribute_advice( dist } -#[derive(Debug, Clone)] -pub struct StoredExpression { - pub(crate) name: String, - cell: Cell, - cell_type: CellType, - expr: Expression, - expr_id: String, -} - -impl Hash for StoredExpression { - fn hash(&self, state: &mut H) { - self.expr_id.hash(state); - self.cell_type.hash(state); - } -} - -impl StoredExpression { - pub fn assign( - &self, - region: &mut CachedRegion<'_, '_, F>, - offset: usize, - ) -> Result, Error> { - let value = self.expr.evaluate( - &|scalar| Value::known(scalar), - &|_| unimplemented!("selector column"), - &|fixed_query| { - Value::known(region.get_fixed( - offset, - fixed_query.column_index(), - fixed_query.rotation(), - )) - }, - &|advice_query| { - Value::known(region.get_advice( - offset, - advice_query.column_index(), - advice_query.rotation(), - )) - }, - &|_| unimplemented!("instance column"), - &|challenge| *region.challenges().indexed()[challenge.index()], - &|a| -a, - &|a, b| a + b, - &|a, b| a * b, - &|a, scalar| a * Value::known(scalar), - ); - self.cell.assign(region, offset, value)?; - Ok(value) - } -} - #[derive(Clone, Debug)] pub struct RandomLinearCombination { // random linear combination expression of cells