diff --git a/bus-mapping/src/evm/opcodes/sha3.rs b/bus-mapping/src/evm/opcodes/sha3.rs index 8c1b7eb11a..6783844fef 100644 --- a/bus-mapping/src/evm/opcodes/sha3.rs +++ b/bus-mapping/src/evm/opcodes/sha3.rs @@ -122,9 +122,24 @@ pub mod sha3_tests { pub fn gen_sha3_code(offset: usize, size: usize, mem_kind: MemoryKind) -> (Bytecode, Vec) { let mut rng = rand::thread_rng(); let data_len = match mem_kind { - MemoryKind::LessThanSize => offset + rng.gen_range(0..size), + MemoryKind::LessThanSize => { + offset + + if size.gt(&0) { + rng.gen_range(0..size) + } else { + 0 + } + } MemoryKind::EqualToSize => offset + size, - MemoryKind::MoreThanSize => offset + size + rng.gen_range(0..size), + MemoryKind::MoreThanSize => { + offset + + size + + if size.gt(&0) { + rng.gen_range(0..size) + } else { + 0 + } + } MemoryKind::Empty => 0, }; let data = rand_bytes(data_len); diff --git a/zkevm-circuits/src/copy_circuit.rs b/zkevm-circuits/src/copy_circuit.rs index bc3931bf62..7e6a3b1845 100644 --- a/zkevm-circuits/src/copy_circuit.rs +++ b/zkevm-circuits/src/copy_circuit.rs @@ -226,11 +226,25 @@ impl CopyCircuit { ); }, ); - cb.require_equal( - "write value == read value", - meta.query_advice(value, Rotation::cur()), - meta.query_advice(value, Rotation::next()), + cb.condition( + not::expr(tag.value_equals(CopyDataType::RlcAcc, Rotation::next())( + meta, + )), + |cb| { + cb.require_equal( + "write value == read value (if not rlc acc)", + meta.query_advice(value, Rotation::cur()), + meta.query_advice(value, Rotation::next()), + ); + }, ); + 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([ @@ -365,6 +379,7 @@ impl CopyCircuit { &self, layouter: &mut impl Layouter, block: &Block, + randomness: F, ) -> Result<(), Error> { let tag_chip = BinaryNumberChip::construct(self.copy_table.tag); let lt_chip = LtChip::construct(self.addr_lt_addr_end); @@ -381,7 +396,7 @@ impl CopyCircuit { .filter(|s| s.rw.is_write()) .map(|s| s.value) .collect::>(); - rlc::value(values.iter().rev(), block.randomness) + rlc::value(values.iter().rev(), randomness) } else { F::zero() }; @@ -392,7 +407,7 @@ impl CopyCircuit { F::from(copy_step.value as u64) } else { value_acc = - value_acc * block.randomness + F::from(copy_step.value as u64); + value_acc * randomness + F::from(copy_step.value as u64); value_acc } } else { @@ -401,7 +416,7 @@ impl CopyCircuit { self.assign_step( &mut region, offset, - block.randomness, + randomness, copy_event, step_idx, copy_step, @@ -686,6 +701,7 @@ mod tests { use crate::{ evm_circuit::witness::{block_convert, Block}, table::{BytecodeTable, RwTable, TxTable}, + util::power_of_randomness_from_instance, }; #[derive(Clone)] @@ -693,21 +709,22 @@ mod tests { tx_table: TxTable, rw_table: RwTable, bytecode_table: BytecodeTable, - copy_table: CopyCircuit, + copy_circuit: CopyCircuit, } #[derive(Default)] struct MyCircuit { block: Block, + randomness: F, } - impl MyCircuit { - pub fn r() -> Expression { - 123456u64.expr() - } + fn get_randomness() -> F { + F::random(rand::thread_rng()) + } - pub fn new(block: Block) -> Self { - Self { block } + impl MyCircuit { + pub fn new(block: Block, randomness: F) -> Self { + Self { block, randomness } } } @@ -724,22 +741,24 @@ mod tests { let rw_table = RwTable::construct(meta); let bytecode_table = BytecodeTable::construct(meta); let q_enable = meta.fixed_column(); + + let randomness = power_of_randomness_from_instance::<_, 1>(meta); let copy_table = CopyTable::construct(meta, q_enable); - let copy_table = CopyCircuit::configure( + let copy_circuit = CopyCircuit::configure( meta, &tx_table, &rw_table, &bytecode_table, copy_table, q_enable, - Self::r().expr(), + randomness[0].clone(), ); MyConfig { tx_table, rw_table, bytecode_table, - copy_table, + copy_circuit, } } @@ -750,22 +769,31 @@ mod tests { ) -> Result<(), halo2_proofs::plonk::Error> { config .tx_table - .load(&mut layouter, &self.block.txs, self.block.randomness)?; + .load(&mut layouter, &self.block.txs, self.randomness)?; config .rw_table - .load(&mut layouter, &self.block.rws, self.block.randomness)?; + .load(&mut layouter, &self.block.rws, self.randomness)?; config.bytecode_table.load( &mut layouter, self.block.bytecodes.values(), - self.block.randomness, + self.randomness, )?; - config.copy_table.assign_block(&mut layouter, &self.block) + config + .copy_circuit + .assign_block(&mut layouter, &self.block, self.randomness) } } - fn run_circuit(k: u32, block: Block) -> Result<(), Vec> { - let circuit = MyCircuit::::new(block); - let prover = MockProver::::run(k, &circuit, vec![]).unwrap(); + fn run_circuit( + k: u32, + block: Block, + randomness: F, + ) -> Result<(), Vec> { + let circuit = MyCircuit::::new(block, randomness); + let num_rows = 1 << k; + const NUM_BLINDING_ROWS: usize = 7 - 1; + let instance = vec![vec![randomness; num_rows - NUM_BLINDING_ROWS]]; + let prover = MockProver::::run(k, &circuit, instance).unwrap(); prover.verify() } @@ -818,21 +846,21 @@ mod tests { fn copy_circuit_valid_calldatacopy() { let builder = gen_calldatacopy_data(); let block = block_convert(&builder.block, &builder.code_db); - assert!(run_circuit(10, block).is_ok()); + assert!(run_circuit(10, block, get_randomness()).is_ok()); } #[test] fn copy_circuit_valid_codecopy() { let builder = gen_codecopy_data(); let block = block_convert(&builder.block, &builder.code_db); - assert!(run_circuit(10, block).is_ok()); + assert!(run_circuit(10, block, get_randomness()).is_ok()); } #[test] fn copy_circuit_valid_sha3() { - let builder = gen_codecopy_data(); + let builder = gen_sha3_data(); let block = block_convert(&builder.block, &builder.code_db); - assert!(run_circuit(20, block).is_ok()); + assert!(run_circuit(20, block, get_randomness()).is_ok()); } fn perturb_tag(block: &mut bus_mapping::circuit_input_builder::Block, tag: CopyDataType) { @@ -864,7 +892,7 @@ mod tests { false => perturb_tag(&mut builder.block, CopyDataType::TxCalldata), } let block = block_convert(&builder.block, &builder.code_db); - assert!(run_circuit(10, block).is_err()); + assert!(run_circuit(10, block, get_randomness()).is_err()); } #[test] @@ -875,6 +903,17 @@ mod tests { false => perturb_tag(&mut builder.block, CopyDataType::Bytecode), } let block = block_convert(&builder.block, &builder.code_db); - assert!(run_circuit(10, block).is_err()); + assert!(run_circuit(10, block, get_randomness()).is_err()); + } + + #[test] + fn copy_circuit_invalid_sha3() { + let mut builder = gen_sha3_data(); + match rand::thread_rng().gen_bool(0.5) { + true => perturb_tag(&mut builder.block, CopyDataType::Memory), + false => perturb_tag(&mut builder.block, CopyDataType::RlcAcc), + } + let block = block_convert(&builder.block, &builder.code_db); + assert!(run_circuit(20, block, get_randomness()).is_err()); } } diff --git a/zkevm-circuits/src/evm_circuit/execution/sha3.rs b/zkevm-circuits/src/evm_circuit/execution/sha3.rs index 06587238ad..74e6cac130 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sha3.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sha3.rs @@ -1,6 +1,6 @@ use bus_mapping::{circuit_input_builder::CopyDataType, evm::OpcodeId}; use eth_types::{evm_types::GasCost, Field, ToLittleEndian, ToScalar}; -use gadgets::util::Expr; +use gadgets::util::{not, Expr}; use halo2_proofs::plonk::Error; use crate::evm_circuit::{ @@ -48,19 +48,25 @@ impl ExecutionGadget for Sha3Gadget { let copy_rwc_inc = cb.query_cell(); let rlc_acc = cb.query_cell(); - cb.copy_table_lookup( - cb.curr.state.call_id.expr(), - CopyDataType::Memory.expr(), - cb.curr.state.call_id.expr(), - CopyDataType::RlcAcc.expr(), - memory_address.offset(), - memory_address.address(), - 0.expr(), // dst_addr for CopyDataType::RlcAcc is 0. - memory_address.length(), - rlc_acc.expr(), - cb.curr.state.rw_counter.expr() + cb.rw_counter_offset(), - copy_rwc_inc.expr(), - ); + cb.condition(memory_address.has_length(), |cb| { + cb.copy_table_lookup( + cb.curr.state.call_id.expr(), + CopyDataType::Memory.expr(), + cb.curr.state.call_id.expr(), + CopyDataType::RlcAcc.expr(), + memory_address.offset(), + memory_address.address(), + 0.expr(), // dst_addr for CopyDataType::RlcAcc is 0. + memory_address.length(), + rlc_acc.expr(), + cb.curr.state.rw_counter.expr() + cb.rw_counter_offset(), + copy_rwc_inc.expr(), + ); + }); + cb.condition(not::expr(memory_address.has_length()), |cb| { + cb.require_zero("copy_rwc_inc == 0 for size = 0", copy_rwc_inc.expr()); + cb.require_zero("rlc_acc == 0 for size = 0", rlc_acc.expr()); + }); cb.keccak_table_lookup(rlc_acc.expr(), memory_address.length(), sha3_rlc.expr()); let memory_expansion = MemoryExpansionGadget::construct( @@ -161,6 +167,11 @@ mod tests { ); } + #[test] + fn sha3_gadget_zero_length() { + test_ok(0x20, 0x00, MemoryKind::MoreThanSize); + } + #[test] fn sha3_gadget_simple() { test_ok(0x00, 0x08, MemoryKind::Empty); diff --git a/zkevm-circuits/src/super_circuit.rs b/zkevm-circuits/src/super_circuit.rs index 07a85e3f44..b7457f7029 100644 --- a/zkevm-circuits/src/super_circuit.rs +++ b/zkevm-circuits/src/super_circuit.rs @@ -222,7 +222,7 @@ impl Circuit // --- Copy Circuit --- config .copy_circuit - .assign_block(&mut layouter, &self.block)?; + .assign_block(&mut layouter, &self.block, self.block.randomness)?; Ok(()) } }