better error msgs for rw lookup error (privacy-scaling-explorations#676)

zkevm-circuits/src/evm_circuit/execution.rs

@@ -1042,14 +1042,66 @@ impl<F: Field> ExecutionConfig<F> {
         }
 
         // Fill in the witness values for stored expressions
+        let assigned_stored_expressions = self.assign_stored_expressions(region, offset, step)?;
+
+        Self::check_rw_lookup(&assigned_stored_expressions, step, block);
+        Ok(())
+    }
+
+    fn assign_stored_expressions(
+        &self,
+        region: &mut CachedRegion<'_, '_, F>,
+        offset: usize,
+        step: &ExecStep,
+    ) -> Result<Vec<(String, F)>, Error> {
+        let mut assigned_stored_expressions = Vec::new();
         for stored_expression in self
             .stored_expressions_map
             .get(&step.execution_state)
             .unwrap_or_else(|| panic!("Execution state unknown: {:?}", step.execution_state))
         {
-            stored_expression.assign(region, offset)?;
+            let assigned = stored_expression.assign(region, offset)?;
+            if let Some(v) = assigned.value() {
+                let name = stored_expression.name.clone();
+                assigned_stored_expressions.push((name, *v))
+            }
         }
+        Ok(assigned_stored_expressions)
+    }
 
-        Ok(())
+    fn check_rw_lookup(
+        assigned_stored_expressions: &[(String, F)],
+        step: &ExecStep,
+        block: &Block<F>,
+    ) {
+        let mut assigned_rw_values = Vec::new();
+        // Reversion lookup expressions have different ordering compared to rw table,
+        // making it a bit complex to check,
+        // so we skip checking reversion lookups.
+        for (name, v) in assigned_stored_expressions {
+            if name.starts_with("rw lookup ")
+                && !name.contains(" with reversion")
+                && !v.is_zero_vartime()
+                && !assigned_rw_values.contains(&(name.clone(), *v))
+            {
+                assigned_rw_values.push((name.clone(), *v));
+            }
+        }
+
+        for (idx, assigned_rw_value) in assigned_rw_values.iter().enumerate() {
+            let rw_idx = step.rw_indices[idx];
+            let rw = block.rws[rw_idx];
+            let table_assignments = rw.table_assignment(block.randomness);
+            let rlc = table_assignments.rlc(block.randomness);
+            if rlc != assigned_rw_value.1 {
+                log::error!(
+                    "incorrect rw witness. lookup input name: \"{}\". rw: {:?}, rw index: {:?}, {}th rw of step {:?}",
+                    assigned_rw_value.0,
+                    rw,
+                    rw_idx,
+                    idx,
+                    step.execution_state);
+            }
+        }
     }
 }

zkevm-circuits/src/evm_circuit/table.rs

@@ -108,6 +108,43 @@ pub(crate) enum Table {
     Keccak,
 }
 
+#[derive(Clone, Debug)]
+pub struct RwValues<F> {
+    pub id: Expression<F>,
+    pub address: Expression<F>,
+    pub field_tag: Expression<F>,
+    pub storage_key: Expression<F>,
+    pub value: Expression<F>,
+    pub value_prev: Expression<F>,
+    pub aux1: Expression<F>,
+    pub aux2: Expression<F>,
+}
+
+impl<F: Field> RwValues<F> {
+    #[allow(clippy::too_many_arguments)]
+    pub fn new(
+        id: Expression<F>,
+        address: Expression<F>,
+        field_tag: Expression<F>,
+        storage_key: Expression<F>,
+        value: Expression<F>,
+        value_prev: Expression<F>,
+        aux1: Expression<F>,
+        aux2: Expression<F>,
+    ) -> Self {
+        Self {
+            id,
+            address,
+            field_tag,
+            storage_key,
+            value,
+            value_prev,
+            aux1,
+            aux2,
+        }
+    }
+}
+
 #[derive(Clone, Debug)]
 pub(crate) enum Lookup<F> {
     /// Lookup to fixed table, which contains serveral pre-built tables such as
@@ -142,7 +179,7 @@ pub(crate) enum Lookup<F> {
         /// all tags.
         tag: Expression<F>,
         /// Values corresponding to the tag.
-        values: [Expression<F>; 8],
+        values: RwValues<F>,
     },
     /// Lookup to bytecode table, which contains all used creation code and
     /// contract code.
@@ -252,11 +289,21 @@ impl<F: Field> Lookup<F> {
                 is_write,
                 tag,
                 values,
-            } => [
-                vec![counter.clone(), is_write.clone(), tag.clone()],
-                values.to_vec(),
-            ]
-            .concat(),
+            } => {
+                vec![
+                    counter.clone(),
+                    is_write.clone(),
+                    tag.clone(),
+                    values.id.clone(),
+                    values.address.clone(),
+                    values.field_tag.clone(),
+                    values.storage_key.clone(),
+                    values.value.clone(),
+                    values.value_prev.clone(),
+                    values.aux1.clone(),
+                    values.aux2.clone(),
+                ]
+            }
             Self::Bytecode {
                 hash,
                 tag,

zkevm-circuits/src/evm_circuit/util.rs

@@ -141,7 +141,7 @@ impl<'r, 'b, F: FieldExt> CachedRegion<'r, 'b, F> {
 
 #[derive(Debug, Clone)]
 pub struct StoredExpression<F> {
-    name: String,
+    pub(crate) name: String,
     cell: Cell<F>,
     cell_type: CellType,
     expr: Expression<F>,