diff --git a/mock/src/test_ctx.rs b/mock/src/test_ctx.rs index befff9f3e0..4c70d309c4 100644 --- a/mock/src/test_ctx.rs +++ b/mock/src/test_ctx.rs @@ -76,7 +76,7 @@ pub use external_tracer::LoggerConfig; /// // Now we can start generating the traces and items we need to inspect /// // the behaviour of the generated env. /// ``` -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct TestContext { /// chain id pub chain_id: Word, diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index 8d283abb8c..cb9de0dc47 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -50,6 +50,8 @@ pub struct CopyCircuitConfig { pub is_last: Column, /// The value copied in this copy step. pub value: Column, + /// Random linear combination accumulator value. + pub value_acc_rlc: Column, /// Whether the row is padding. pub is_pad: Column, /// In case of a bytecode tag, this denotes whether or not the copied byte @@ -107,7 +109,8 @@ impl SubCircuitConfig for CopyCircuitConfig { ) -> Self { let q_step = meta.complex_selector(); let is_last = meta.advice_column(); - let value = meta.advice_column_in(SecondPhase); + let value = meta.advice_column(); + let value_acc_rlc = meta.advice_column_in(SecondPhase); let is_code = meta.advice_column(); let is_pad = meta.advice_column(); let is_first = copy_table.is_first; @@ -225,23 +228,36 @@ impl SubCircuitConfig for CopyCircuitConfig { rw_diff, ); }); - cb.condition( - and::expr([ - meta.query_advice(is_last, Rotation::cur()), - tag.value_equals(CopyDataType::RlcAcc, Rotation::cur())(meta), - ]), - |cb| { - cb.require_equal( - "value == rlc_acc at the last row for RlcAcc", - meta.query_advice(value, Rotation::cur()), - meta.query_advice(rlc_acc, Rotation::cur()), - ); - }, - ); cb.gate(meta.query_fixed(q_enable, Rotation::cur())) }); + meta.create_gate( + "Last Step (check value accumulator) Memory => Bytecode or RlcAcc", + |meta: &mut halo2_proofs::plonk::VirtualCells| { + let mut cb = BaseConstraintBuilder::default(); + + cb.require_equal( + "value_acc_rlc == rlc_acc on the last row", + meta.query_advice(value_acc_rlc, Rotation::next()), + meta.query_advice(rlc_acc, Rotation::next()), + ); + + cb.gate(and::expr([ + meta.query_fixed(q_enable, Rotation::cur()), + meta.query_advice(is_last, Rotation::next()), + // To build a selector expression just having 0 when false and != 0 when true + // is enough, so we could replace the `or` by a `+`. This + // would give 2 when both expressions are true + // but it's fine. + and::expr([ + tag.value_equals(CopyDataType::Memory, Rotation::cur())(meta), + tag.value_equals(CopyDataType::Bytecode, Rotation::next())(meta), + ]) + tag.value_equals(CopyDataType::RlcAcc, Rotation::next())(meta), + ])) + }, + ); + meta.create_gate("verify step (q_step == 1)", |meta| { let mut cb = BaseConstraintBuilder::default(); @@ -253,10 +269,10 @@ impl SubCircuitConfig for CopyCircuitConfig { ]), ); cb.condition( - not::expr(meta.query_advice(is_last, Rotation::next())) - * (not::expr(tag.value_equals(CopyDataType::Padding, Rotation::cur())( - meta, - ))), + not::expr(or::expr([ + meta.query_advice(is_last, Rotation::next()), + tag.value_equals(CopyDataType::Padding, Rotation::cur())(meta), + ])), |cb| { cb.require_equal( "bytes_left == bytes_left_next + 1 for non-last step", @@ -265,25 +281,38 @@ impl SubCircuitConfig for CopyCircuitConfig { ); }, ); + cb.condition(meta.query_advice(is_first, Rotation::cur()), |cb| { + cb.require_equal( + "value == value_acc_rlc at every first copy event", + meta.query_advice(value, Rotation::cur()), + meta.query_advice(value_acc_rlc, Rotation::cur()), + ); + }); + cb.require_equal( + "write value == read value", + meta.query_advice(value, Rotation::cur()), + meta.query_advice(value, Rotation::next()), + ); + cb.require_equal( + "value_acc_rlc is same for read-write rows", + meta.query_advice(value_acc_rlc, Rotation::cur()), + meta.query_advice(value_acc_rlc, Rotation::next()), + ); cb.condition( - not::expr(tag.value_equals(CopyDataType::RlcAcc, Rotation::next())( - meta, - )), + and::expr([ + not::expr(meta.query_advice(is_last, Rotation::next())), + not::expr(meta.query_advice(is_pad, Rotation::cur())), + ]), |cb| { cb.require_equal( - "write value == read value (if not rlc acc)", - meta.query_advice(value, Rotation::cur()), - meta.query_advice(value, Rotation::next()), + "value_acc_rlc(2) == value_acc_rlc(0) * r + value(2)", + meta.query_advice(value_acc_rlc, Rotation(2)), + meta.query_advice(value_acc_rlc, Rotation::cur()) + * challenges.keccak_input() + + meta.query_advice(value, Rotation(2)), ); }, ); - cb.condition(meta.query_advice(is_first, Rotation::cur()), |cb| { - cb.require_equal( - "write value == read value (is_first == 1)", - meta.query_advice(value, Rotation::cur()), - meta.query_advice(value, Rotation::next()), - ); - }); cb.require_zero( "value == 0 when is_pad == 1 for read", and::expr([ @@ -301,26 +330,7 @@ impl SubCircuitConfig for CopyCircuitConfig { meta.query_advice(is_pad, Rotation::next()), ); - cb.gate(meta.query_selector(q_step)) - }); - - meta.create_gate("verify_step (q_step == 0)", |meta| { - let mut cb = BaseConstraintBuilder::default(); - - cb.require_equal( - "rows[2].value == rows[0].value * r + rows[1].value", - meta.query_advice(value, Rotation(2)), - meta.query_advice(value, Rotation::cur()) * challenges.keccak_input() - + meta.query_advice(value, Rotation::next()), - ); - - cb.gate(and::expr([ - meta.query_fixed(q_enable, Rotation::cur()), - not::expr(meta.query_selector(q_step)), - not::expr(meta.query_advice(is_last, Rotation::cur())), - tag.value_equals(CopyDataType::RlcAcc, Rotation::cur())(meta), - not::expr(meta.query_advice(is_pad, Rotation::cur())), - ])) + cb.gate(and::expr([meta.query_selector(q_step)])) }); meta.lookup_any("Memory lookup", |meta| { @@ -405,6 +415,7 @@ impl SubCircuitConfig for CopyCircuitConfig { q_step, is_last, value, + value_acc_rlc, is_pad, is_code, q_enable, @@ -466,9 +477,15 @@ impl CopyCircuitConfig { )?; // is_last, value, is_pad, is_code - for (column, &(value, label)) in [self.is_last, self.value, self.is_pad, self.is_code] - .iter() - .zip_eq(circuit_row) + for (column, &(value, label)) in [ + self.is_last, + self.value, + self.value_acc_rlc, + self.is_pad, + self.is_code, + ] + .iter() + .zip_eq(circuit_row) { region.assign_advice( || format!("{} at row: {}", label, *offset), @@ -621,6 +638,13 @@ impl CopyCircuitConfig { *offset, || Value::known(F::ZERO), )?; + // value_acc_rlc + region.assign_advice( + || format!("assign value_acc_rlc {}", *offset), + self.value_acc_rlc, + *offset, + || Value::known(F::ZERO), + )?; // rlc_acc region.assign_advice( || format!("assign rlc_acc {}", *offset), diff --git a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs index cad355c2bc..f5dbefa402 100644 --- a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs +++ b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs @@ -27,6 +27,7 @@ pub(crate) struct ReturnRevertGadget { opcode: Cell, range: MemoryAddressGadget, + deployed_code_rlc: Cell, is_success: Cell, restore_context: RestoreContextGadget, @@ -94,11 +95,12 @@ impl ExecutionGadget for ReturnRevertGadget { let is_contract_deployment = is_create.clone() * is_success.expr() * not::expr(copy_rw_increase_is_zero.expr()); - let (caller_id, address, reversion_info, code_hash) = + let (caller_id, address, reversion_info, code_hash, deployed_code_rlc) = cb.condition(is_contract_deployment.clone(), |cb| { // We don't need to place any additional constraints on code_hash because the // copy circuit enforces that it is the hash of the bytes in the copy lookup. let code_hash = cb.query_cell_phase2(); + let deployed_code_rlc = cb.query_cell_phase2(); cb.copy_table_lookup( cb.curr.state.call_id.expr(), CopyDataType::Memory.expr(), @@ -108,7 +110,7 @@ impl ExecutionGadget for ReturnRevertGadget { range.address(), 0.expr(), range.length(), - 0.expr(), + deployed_code_rlc.expr(), copy_rw_increase.expr(), ); @@ -127,7 +129,13 @@ impl ExecutionGadget for ReturnRevertGadget { Some(&mut reversion_info), ); - (caller_id, address, reversion_info, code_hash) + ( + caller_id, + address, + reversion_info, + code_hash, + deployed_code_rlc, + ) }); // Case B in the specs. @@ -218,6 +226,7 @@ impl ExecutionGadget for ReturnRevertGadget { Self { opcode, range, + deployed_code_rlc, is_success, copy_length, copy_rw_increase, @@ -279,6 +288,11 @@ impl ExecutionGadget for ReturnRevertGadget { let values: Vec<_> = (3..3 + length.as_usize()) .map(|index| block.get_rws(step, index).memory_value()) .collect(); + self.deployed_code_rlc.assign( + region, + offset, + region.keccak_rlc(&values.iter().rev().cloned().collect::>()), + )?; let mut code_hash = CodeDB::hash(&values).to_fixed_bytes(); code_hash.reverse(); self.code_hash.assign( @@ -614,10 +628,8 @@ mod test { PUSH1(0) // dest offset RETURNDATACOPY }); - - let block: GethData = TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode.clone()) - .unwrap() - .into(); + let test_ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode.clone()).unwrap(); + let block: GethData = test_ctx.clone().into(); // collect return opcode, retrieve next step, assure both contract create // successfully @@ -648,8 +660,7 @@ mod test { .iter() .for_each(|size| assert_eq!(size, &Word::zero())); - let text_ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(); - CircuitTestBuilder::new_from_test_ctx(text_ctx).run(); + CircuitTestBuilder::new_from_test_ctx(test_ctx).run(); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs index de2a71bad5..f8136f55db 100644 --- a/zkevm-circuits/src/evm_circuit/util.rs +++ b/zkevm-circuits/src/evm_circuit/util.rs @@ -193,10 +193,23 @@ impl<'r, 'b, F: Field> CachedRegion<'r, 'b, F> { .evm_word() .map(|r| rlc::value(&n.to_le_bytes(), r)) } + + pub fn keccak_rlc(&self, le_bytes: &[u8]) -> Value { + self.challenges + .keccak_input() + .map(|r| rlc::value(le_bytes, r)) + } + pub fn empty_code_hash_rlc(&self) -> Value { self.word_rlc(CodeDB::empty_code_hash().to_word()) } + pub fn code_hash(&self, n: U256) -> Value { + self.challenges + .evm_word() + .map(|r| rlc::value(&n.to_le_bytes(), r)) + } + /// Constrains a cell to have a constant value. /// /// Returns an error if the cell is in a column where equality has not been diff --git a/zkevm-circuits/src/table/copy_table.rs b/zkevm-circuits/src/table/copy_table.rs index 9db69d7c27..a48761229d 100644 --- a/zkevm-circuits/src/table/copy_table.rs +++ b/zkevm-circuits/src/table/copy_table.rs @@ -1,7 +1,7 @@ use super::*; type CopyTableRow = [(Value, &'static str); 8]; -type CopyCircuitRow = [(Value, &'static str); 4]; +type CopyCircuitRow = [(Value, &'static str); 5]; /// Copy Table, used to verify copies of byte chunks between Memory, Bytecode, /// TxLogs and TxCallData. @@ -61,7 +61,7 @@ impl CopyTable { ) -> Vec<(CopyDataType, CopyTableRow, CopyCircuitRow)> { let mut assignments = Vec::new(); // rlc_acc - let rlc_acc = if copy_event.dst_type == CopyDataType::RlcAcc { + let rlc_acc = { let values = copy_event .bytes .iter() @@ -70,8 +70,6 @@ impl CopyTable { challenges .keccak_input() .map(|keccak_input| rlc::value(values.iter().rev(), keccak_input)) - } else { - Value::known(F::ZERO) }; let mut value_acc = Value::known(F::ZERO); for (step_idx, (is_read_step, copy_step)) in copy_event @@ -146,17 +144,11 @@ impl CopyTable { // bytes_left let bytes_left = u64::try_from(copy_event.bytes.len() * 2 - step_idx).unwrap() / 2; // value - let value = if copy_event.dst_type == CopyDataType::RlcAcc { - if is_read_step { - Value::known(F::from(copy_step.value as u64)) - } else { - value_acc = value_acc * challenges.keccak_input() - + Value::known(F::from(copy_step.value as u64)); - value_acc - } - } else { - Value::known(F::from(copy_step.value as u64)) - }; + let value = Value::known(F::from(copy_step.value as u64)); + // value_acc + if is_read_step { + value_acc = value_acc * challenges.keccak_input() + value; + } // is_pad let is_pad = Value::known(F::from( (is_read_step && copy_step_addr >= copy_event.src_addr_end) as u64, @@ -176,7 +168,14 @@ impl CopyTable { "src_addr_end", ), (Value::known(F::from(bytes_left)), "bytes_left"), - (rlc_acc, "rlc_acc"), + ( + match (copy_event.src_type, copy_event.dst_type) { + (CopyDataType::Memory, CopyDataType::Bytecode) => rlc_acc, + (_, CopyDataType::RlcAcc) => rlc_acc, + _ => Value::known(F::ZERO), + }, + "rlc_acc", + ), ( Value::known(F::from(copy_event.rw_counter(step_idx))), "rw_counter", @@ -189,6 +188,7 @@ impl CopyTable { [ (is_last, "is_last"), (value, "value"), + (value_acc, "value_acc"), (is_pad, "is_pad"), (is_code, "is_code"), ],