Fix RLC values in Copy/Keccak table lookup (SHA3)

bus-mapping/Cargo.toml

@@ -9,21 +9,25 @@ license = "MIT OR Apache-2.0"
 eth-types = { path = "../eth-types" }
 gadgets = { path = "../gadgets" }
 keccak256 = { path = "../keccak256" }
+mock = { path = "../mock", optional = true }
+
 ethers-core = "0.6"
 ethers-providers = "0.6"
 halo2_proofs = { version = "0.1.0-beta.1" }
 itertools = "0.10"
 lazy_static = "1.4"
 log = "0.4.14"
+rand = { version = "0.8", optional = true }
 serde = {version = "1.0.130", features = ["derive"] }
 serde_json = "1.0.66"
 strum = "0.24"
 strum_macros = "0.24"
 
 [dev-dependencies]
 hex = "0.4.3"
-mock = { path = "../mock" }
 pretty_assertions = "1.0.0"
-rand = "0.8"
 tokio = { version = "1.13", features = ["macros"] }
 url = "2.2.2"
+
+[features]
+test = ["mock", "rand"]

bus-mapping/src/evm.rs

@@ -4,3 +4,6 @@ pub(crate) mod opcodes;
 
 pub use eth_types::evm_types::opcode_ids::OpcodeId;
 pub use opcodes::Opcode;
+
+#[cfg(any(feature = "test", test))]
+pub use opcodes::{gen_sha3_code, MemoryKind};

bus-mapping/src/evm/opcodes.rs

@@ -15,6 +15,9 @@ use eth_types::{
 use keccak256::EMPTY_HASH;
 use log::warn;
 
+#[cfg(any(feature = "test", test))]
+pub use sha3::sha3_tests::{gen_sha3_code, MemoryKind};
+
 mod call;
 mod calldatacopy;
 mod calldataload;

bus-mapping/src/evm/opcodes/sha3.rs

@@ -104,8 +104,8 @@ impl Opcode for Sha3 {
     }
 }
 
-#[cfg(test)]
-mod sha3_tests {
+#[cfg(any(feature = "test", test))]
+pub mod sha3_tests {
     use eth_types::{bytecode, evm_types::OpcodeId, geth_types::GethData, Bytecode, Word};
     use ethers_core::utils::keccak256;
     use mock::{
@@ -120,18 +120,9 @@ mod sha3_tests {
         operation::{MemoryOp, RWCounter, StackOp, RW},
     };
 
-    enum MemoryKind {
-        Empty,
-        LessThanSize,
-        EqualToSize,
-        MoreThanSize,
-    }
-
-    fn rand_bytes(size: usize) -> Vec<u8> {
-        (0..size).map(|_| random()).collect::<Vec<u8>>()
-    }
-
-    fn test_ok(offset: usize, size: usize, mem_kind: MemoryKind) {
+    /// Generate bytecode for SHA3 opcode after having populated sufficient
+    /// memory given the offset and size arguments for SHA3.
+    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),
@@ -158,8 +149,6 @@ mod sha3_tests {
             code.push(32, (32 * i).into());
             code.write_op(OpcodeId::MSTORE);
         }
-        let memory_len = memory.len();
-
         // append SHA3 related opcodes at the tail end.
         let code_tail = bytecode! {
             PUSH32(size)
@@ -168,6 +157,28 @@ mod sha3_tests {
             STOP
         };
         code.append(&code_tail);
+        (code, memory)
+    }
+
+    /// Memory of a context with respect to the input size to SHA3.
+    pub enum MemoryKind {
+        /// Variant defining empty memory.
+        Empty,
+        /// Variant defining memory length being less than size.
+        LessThanSize,
+        /// Variant defining memory length being equal to size.
+        EqualToSize,
+        /// Variant defining memory length being more than size.
+        MoreThanSize,
+    }
+
+    fn rand_bytes(size: usize) -> Vec<u8> {
+        (0..size).map(|_| random()).collect::<Vec<u8>>()
+    }
+
+    fn test_ok(offset: usize, size: usize, mem_kind: MemoryKind) {
+        let (code, memory) = gen_sha3_code(offset, size, mem_kind);
+        let memory_len = memory.len();
 
         // The memory that is hashed.
         let mut memory_view = memory

zkevm-circuits/Cargo.toml

@@ -42,6 +42,7 @@ num-bigint = { version = "0.4" }
 subtle = "2.4"
 
 [dev-dependencies]
+bus-mapping = { path = "../bus-mapping", features = ["test"] }
 criterion = "0.3"
 ctor = "0.1.22"
 env_logger = "0.9.0"

zkevm-circuits/src/copy_circuit.rs

@@ -381,7 +381,7 @@ impl<F: Field> CopyCircuit<F> {
                             .filter(|s| s.rw.is_write())
                             .map(|s| s.value)
                             .collect::<Vec<u8>>();
-                        rlc::value(&values, block.randomness)
+                        rlc::value(values.iter().rev(), block.randomness)
                     } else {
                         F::zero()
                     };
@@ -670,6 +670,7 @@ mod tests {
     use super::*;
     use bus_mapping::{
         circuit_input_builder::{CircuitInputBuilder, CopyDataType},
+        evm::{gen_sha3_code, MemoryKind},
         mock::BlockData,
         operation::RWCounter,
     };
@@ -802,6 +803,17 @@ mod tests {
         builder
     }
 
+    fn gen_sha3_data() -> CircuitInputBuilder {
+        let (code, _) = gen_sha3_code(0x20, 0x200, MemoryKind::EqualToSize);
+        let test_ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(code).unwrap();
+        let block: GethData = test_ctx.into();
+        let mut builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder();
+        builder
+            .handle_block(&block.eth_block, &block.geth_traces)
+            .unwrap();
+        builder
+    }
+
     #[test]
     fn copy_circuit_valid_calldatacopy() {
         let builder = gen_calldatacopy_data();
@@ -816,6 +828,13 @@ mod tests {
         assert!(run_circuit(10, block).is_ok());
     }
 
+    #[test]
+    fn copy_circuit_valid_sha3() {
+        let builder = gen_codecopy_data();
+        let block = block_convert(&builder.block, &builder.code_db);
+        assert!(run_circuit(20, block).is_ok());
+    }
+
     fn perturb_tag(block: &mut bus_mapping::circuit_input_builder::Block, tag: CopyDataType) {
         debug_assert!(!block.copy_events.is_empty());
         debug_assert!(!block.copy_events[0].steps.is_empty());

zkevm-circuits/src/evm_circuit/execution/sha3.rs

@@ -122,7 +122,7 @@ impl<F: Field> ExecutionGadget<F> for Sha3Gadget<F> {
         let values: Vec<u8> = (3..3 + (size.low_u64() as usize))
             .map(|i| block.rws[step.rw_indices[i]].memory_value())
             .collect();
-        let rlc_acc = rlc::value(&values, block.randomness);
+        let rlc_acc = rlc::value(values.iter().rev(), block.randomness);
         self.rlc_acc.assign(region, offset, Some(rlc_acc))?;
 
         // Memory expansion and dynamic gas cost for reading it.
@@ -145,19 +145,13 @@ impl<F: Field> ExecutionGadget<F> for Sha3Gadget<F> {
 
 #[cfg(test)]
 mod tests {
-    use eth_types::bytecode;
+    use bus_mapping::evm::{gen_sha3_code, MemoryKind};
     use mock::TestContext;
 
     use crate::test_util::run_test_circuits;
 
-    #[test]
-    fn sha3_gadget_simple() {
-        let code = bytecode! {
-            PUSH32(0x08) // size
-            PUSH32(0x00) // offset
-            SHA3
-            STOP
-        };
+    fn test_ok(offset: usize, size: usize, mem_kind: MemoryKind) {
+        let (code, _) = gen_sha3_code(offset, size, mem_kind);
         assert_eq!(
             run_test_circuits(
                 TestContext::<2, 1>::simple_ctx_with_bytecode(code).unwrap(),
@@ -167,20 +161,19 @@ mod tests {
         );
     }
 
+    #[test]
+    fn sha3_gadget_simple() {
+        test_ok(0x00, 0x08, MemoryKind::Empty);
+        test_ok(0x10, 0x10, MemoryKind::LessThanSize);
+        test_ok(0x24, 0x16, MemoryKind::EqualToSize);
+        test_ok(0x32, 0x78, MemoryKind::MoreThanSize);
+    }
+
     #[test]
     fn sha3_gadget_large() {
-        let code = bytecode! {
-            PUSH32(0x101)
-            PUSH32(0x202)
-            SHA3
-            STOP
-        };
-        assert_eq!(
-            run_test_circuits(
-                TestContext::<2, 1>::simple_ctx_with_bytecode(code).unwrap(),
-                None
-            ),
-            Ok(())
-        );
+        test_ok(0x101, 0x202, MemoryKind::Empty);
+        test_ok(0x202, 0x303, MemoryKind::LessThanSize);
+        test_ok(0x303, 0x404, MemoryKind::EqualToSize);
+        test_ok(0x404, 0x505, MemoryKind::MoreThanSize);
     }
 }

zkevm-circuits/src/evm_circuit/witness.rs

@@ -1207,6 +1207,7 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState {
                     OpcodeId::DIFFICULTY | OpcodeId::BASEFEE => ExecutionState::BLOCKCTXU256,
                     OpcodeId::GAS => ExecutionState::GAS,
                     OpcodeId::SELFBALANCE => ExecutionState::SELFBALANCE,
+                    OpcodeId::SHA3 => ExecutionState::SHA3,
                     OpcodeId::SHR => ExecutionState::SHR,
                     OpcodeId::SLOAD => ExecutionState::SLOAD,
                     OpcodeId::SSTORE => ExecutionState::SSTORE,
@@ -1221,7 +1222,6 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState {
                     OpcodeId::CODESIZE => ExecutionState::CODESIZE,
                     OpcodeId::RETURN | OpcodeId::REVERT => ExecutionState::RETURN,
                     // dummy ops
-                    OpcodeId::SHA3 => dummy!(ExecutionState::SHA3),
                     OpcodeId::ADDRESS => dummy!(ExecutionState::ADDRESS),
                     OpcodeId::BALANCE => dummy!(ExecutionState::BALANCE),
                     OpcodeId::BLOCKHASH => dummy!(ExecutionState::BLOCKHASH),

zkevm-circuits/src/table.rs

@@ -771,7 +771,7 @@ impl CopyTable {
                 .filter(|s| s.rw.is_write())
                 .map(|s| s.value)
                 .collect::<Vec<u8>>();
-            rlc::value(&values, randomness)
+            rlc::value(values.iter().rev(), randomness)
         } else {
             F::zero()
         };