FIX constraints+tests for copy circuit and SHA3 gadget

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<u8>) {
         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);

zkevm-circuits/src/copy_circuit.rs

@@ -226,11 +226,25 @@ impl<F: Field> CopyCircuit<F> {
                     );
                 },
             );
-            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<F: Field> CopyCircuit<F> {
         &self,
         layouter: &mut impl Layouter<F>,
         block: &Block<F>,
+        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<F: Field> CopyCircuit<F> {
                             .filter(|s| s.rw.is_write())
                             .map(|s| s.value)
                             .collect::<Vec<u8>>();
-                        rlc::value(values.iter().rev(), block.randomness)
+                        rlc::value(values.iter().rev(), randomness)
                     } else {
                         F::zero()
                     };
@@ -392,7 +407,7 @@ impl<F: Field> CopyCircuit<F> {
                                 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<F: Field> CopyCircuit<F> {
                         self.assign_step(
                             &mut region,
                             offset,
-                            block.randomness,
+                            randomness,
                             copy_event,
                             step_idx,
                             copy_step,
@@ -686,28 +701,30 @@ mod tests {
     use crate::{
         evm_circuit::witness::{block_convert, Block},
         table::{BytecodeTable, RwTable, TxTable},
+        util::power_of_randomness_from_instance,
     };
 
     #[derive(Clone)]
     struct MyConfig<F> {
         tx_table: TxTable,
         rw_table: RwTable,
         bytecode_table: BytecodeTable,
-        copy_table: CopyCircuit<F>,
+        copy_circuit: CopyCircuit<F>,
     }
 
     #[derive(Default)]
     struct MyCircuit<F> {
         block: Block<F>,
+        randomness: F,
     }
 
-    impl<F: Field> MyCircuit<F> {
-        pub fn r() -> Expression<F> {
-            123456u64.expr()
-        }
+    fn get_randomness<F: Field>() -> F {
+        F::random(rand::thread_rng())
+    }
 
-        pub fn new(block: Block<F>) -> Self {
-            Self { block }
+    impl<F: Field> MyCircuit<F> {
+        pub fn new(block: Block<F>, 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<F: Field>(k: u32, block: Block<F>) -> Result<(), Vec<VerifyFailure>> {
-        let circuit = MyCircuit::<F>::new(block);
-        let prover = MockProver::<F>::run(k, &circuit, vec![]).unwrap();
+    fn run_circuit<F: Field>(
+        k: u32,
+        block: Block<F>,
+        randomness: F,
+    ) -> Result<(), Vec<VerifyFailure>> {
+        let circuit = MyCircuit::<F>::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::<F>::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());
     }
 }

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<F: Field> ExecutionGadget<F> for Sha3Gadget<F> {
 
         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);

zkevm-circuits/src/super_circuit.rs

@@ -222,7 +222,7 @@ impl<F: Field, const MAX_TXS: usize, const MAX_CALLDATA: usize> Circuit<F>
         // --- Copy Circuit ---
         config
             .copy_circuit
-            .assign_block(&mut layouter, &self.block)?;
+            .assign_block(&mut layouter, &self.block, self.block.randomness)?;
         Ok(())
     }
 }