Fix codehash phase; fix l1 tx nonce (#741)

lispc · web-flow · commit 106ccd5c86c9 · 2023-08-08T18:59:09.000+08:00
* fix sender_nonce vs tx_nonce handling inside begin tx * v0.5.8: fix codehash phase * fix code_hash * Revert "Update create.rs" This reverts commit fb86b74. * disable copy test for testool * fix doc
diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs
@@ -349,7 +349,9 @@ pub fn run_test(
             zkevm_circuits::evm_circuit::witness::block_convert(&builder.block, &builder.code_db)
                 .unwrap();
 
-        CircuitTestBuilder::<1, 1>::new_from_block(block).run();
+        CircuitTestBuilder::<1, 1>::new_from_block(block)
+            .copy_checks(None)
+            .run();
     } else {
         geth_data.sign(&wallets);
 
diff --git a/zkevm-circuits/src/evm_circuit/execution/addmod.rs b/zkevm-circuits/src/evm_circuit/execution/addmod.rs
@@ -251,11 +251,11 @@ mod test {
         }
         let mut ctb = CircuitTestBuilder::new_from_test_ctx(ctx);
         if !ok {
-            ctb = ctb.evm_checks(Box::new(|prover, gate_rows, lookup_rows| {
+            ctb = ctb.evm_checks(Some(Box::new(|prover, gate_rows, lookup_rows| {
                 assert!(prover
                     .verify_at_rows_par(gate_rows.iter().cloned(), lookup_rows.iter().cloned())
                     .is_err())
-            }));
+            })));
         };
         ctb.run()
     }
diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs
@@ -856,7 +856,7 @@ impl<F: Field> ExecutionGadget<F> for CallOpGadget<F> {
                     self.code_hash_previous.assign(
                         region,
                         offset,
-                        region.word_rlc(code_hash_previous),
+                        region.code_hash(code_hash_previous),
                     )?;
                     rw_offset += 2; // codehash read and write
                     #[cfg(feature = "scroll")]
diff --git a/zkevm-circuits/src/evm_circuit/execution/create.rs b/zkevm-circuits/src/evm_circuit/execution/create.rs
@@ -84,7 +84,7 @@ impl<F: Field, const IS_CREATE2: bool, const S: ExecutionState> ExecutionGadget<
     fn configure(cb: &mut EVMConstraintBuilder<F>) -> Self {
         // Use rw_counter of the step which triggers next call as its call_id.
         let callee_call_id = cb.curr.state.rw_counter.clone();
-        let code_hash_previous = cb.query_cell_phase2();
+        let code_hash_previous = cb.query_cell();
         #[cfg(feature = "scroll")]
         let keccak_code_hash_previous = cb.query_cell_phase2();
         let opcode = cb.query_cell();
diff --git a/zkevm-circuits/src/evm_circuit/execution/gas.rs b/zkevm-circuits/src/evm_circuit/execution/gas.rs
@@ -149,11 +149,11 @@ mod test {
                 assert_eq!(block.txs[0].steps.len(), 5);
                 block.txs[0].steps[2].gas_left -= 1;
             }))
-            .evm_checks(Box::new(|prover, gate_rows, lookup_rows| {
+            .evm_checks(Some(Box::new(|prover, gate_rows, lookup_rows| {
                 assert!(prover
                     .verify_at_rows_par(gate_rows.iter().cloned(), lookup_rows.iter().cloned())
                     .is_err())
-            }))
+            })))
             .run();
     }
 }
diff --git a/zkevm-circuits/src/evm_circuit/execution/mulmod.rs b/zkevm-circuits/src/evm_circuit/execution/mulmod.rs
@@ -193,11 +193,11 @@ mod test {
 
         let mut ctb = CircuitTestBuilder::new_from_test_ctx(ctx);
         if !ok {
-            ctb = ctb.evm_checks(Box::new(|prover, gate_rows, lookup_rows| {
+            ctb = ctb.evm_checks(Some(Box::new(|prover, gate_rows, lookup_rows| {
                 assert!(prover
                     .verify_at_rows_par(gate_rows.iter().cloned(), lookup_rows.iter().cloned())
                     .is_err())
-            }));
+            })));
         };
         ctb.run()
     }
diff --git a/zkevm-circuits/src/evm_circuit/util.rs b/zkevm-circuits/src/evm_circuit/util.rs
@@ -199,14 +199,14 @@ impl<'r, 'b, F: FieldExt> CachedRegion<'r, 'b, F> {
     }
 
     pub fn code_hash(&self, n: U256) -> Value<F> {
-        self.challenges.evm_word().map(|r| {
-            if cfg!(feature = "poseidon-codehash") {
-                // only FieldExt is not enough for ToScalar trait so we have to make workaround
-                rlc::value(&n.to_le_bytes(), F::from(256u64))
-            } else {
-                rlc::value(&n.to_le_bytes(), r)
-            }
-        })
+        if cfg!(feature = "poseidon-codehash") {
+            // only FieldExt is not enough for ToScalar trait so we have to make workaround
+            Value::known(rlc::value(&n.to_le_bytes(), F::from(256u64)))
+        } else {
+            self.challenges
+                .evm_word()
+                .map(|r| rlc::value(&n.to_le_bytes(), r))
+        }
     }
 
     pub fn keccak_rlc(&self, le_bytes: &[u8]) -> Value<F> {
diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs
@@ -72,16 +72,16 @@ fn init_env_logger() {
 ///
 /// CircuitTestBuilder::new_from_test_ctx(ctx)
 ///     .block_modifier(Box::new(|block| block.circuits_params.max_evm_rows = (1 << 18) - 100))
-///     .state_checks(Box::new(|prover, evm_rows, lookup_rows| assert!(prover.verify_at_rows_par(evm_rows.iter().cloned(), lookup_rows.iter().cloned()).is_err())))
+///     .state_checks(Some(Box::new(|prover, evm_rows, lookup_rows| assert!(prover.verify_at_rows_par(evm_rows.iter().cloned(), lookup_rows.iter().cloned()).is_err()))))
 ///     .run();
 /// ```
 pub struct CircuitTestBuilder<const NACC: usize, const NTX: usize> {
     test_ctx: Option<TestContext<NACC, NTX>>,
     circuits_params: Option<CircuitsParams>,
     block: Option<Block<Fr>>,
-    evm_checks: Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>,
-    state_checks: Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>,
-    copy_checks: Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>,
+    evm_checks: Option<Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>>,
+    state_checks: Option<Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>>,
+    copy_checks: Option<Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>>,
     block_modifiers: Vec<Box<dyn Fn(&mut Block<Fr>)>>,
     geth_data_modifiers: Vec<Box<dyn Fn(&mut GethData)>>,
 }
@@ -93,24 +93,24 @@ impl<const NACC: usize, const NTX: usize> CircuitTestBuilder<NACC, NTX> {
             test_ctx: None,
             circuits_params: None,
             block: None,
-            evm_checks: Box::new(|prover, gate_rows, lookup_rows| {
+            evm_checks: Some(Box::new(|prover, gate_rows, lookup_rows| {
                 assert_eq!(prover.verify_at_rows_par(
                     gate_rows.iter().cloned(),
                     lookup_rows.iter().cloned(),
                 ), Ok(()));
-            }),
-            state_checks: Box::new(|prover, gate_rows, lookup_rows| {
+            })),
+            state_checks: Some(Box::new(|prover, gate_rows, lookup_rows| {
                 assert_eq!(prover.verify_at_rows_par(
                     gate_rows.iter().cloned(),
                     lookup_rows.iter().cloned(),
                 ), Ok(()));
-            }),
-            copy_checks: Box::new(|prover, gate_rows, lookup_rows| {
+            })),
+            copy_checks: Some(Box::new(|prover, gate_rows, lookup_rows| {
                 assert_eq!(prover.verify_at_rows_par(
                     gate_rows.iter().cloned(),
                     lookup_rows.iter().cloned(),
                 ), Ok(()));
-            }),
+            })),
             block_modifiers: vec![],
             geth_data_modifiers: vec![],
         }
@@ -157,7 +157,7 @@ impl<const NACC: usize, const NTX: usize> CircuitTestBuilder<NACC, NTX> {
     /// Circuit verification.
     pub fn state_checks(
         mut self,
-        state_checks: Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>,
+        state_checks: Option<Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>>,
     ) -> Self {
         self.state_checks = state_checks;
         self
@@ -168,12 +168,23 @@ impl<const NACC: usize, const NTX: usize> CircuitTestBuilder<NACC, NTX> {
     /// Circuit verification.
     pub fn evm_checks(
         mut self,
-        evm_checks: Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>,
+        evm_checks: Option<Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>>,
     ) -> Self {
         self.evm_checks = evm_checks;
         self
     }
 
+    #[allow(clippy::type_complexity)]
+    /// Allows to provide checks different than the default ones for the Copy
+    /// Circuit verification.
+    pub fn copy_checks(
+        mut self,
+        copy_checks: Option<Box<dyn Fn(MockProver<Fr>, &Vec<usize>, &Vec<usize>)>>,
+    ) -> Self {
+        self.copy_checks = copy_checks;
+        self
+    }
+
     #[allow(clippy::type_complexity)]
     /// Allows to provide modifier functions for the [`Block`] that will be
     /// generated within this builder.
@@ -235,18 +246,18 @@ impl<const NACC: usize, const NTX: usize> CircuitTestBuilder<NACC, NTX> {
 
         const NUM_BLINDING_ROWS: usize = 64;
         // Run evm circuit test
-        {
+        if let Some(evm_checks) = &self.evm_checks {
             let k = block.get_test_degree();
             let (active_gate_rows, active_lookup_rows) = EvmCircuit::<Fr>::get_active_rows(&block);
 
             let circuit = EvmCircuit::get_test_cicuit_from_block(block.clone());
             let prover = MockProver::<Fr>::run(k, &circuit, vec![]).unwrap();
 
-            self.evm_checks.as_ref()(prover, &active_gate_rows, &active_lookup_rows)
+            evm_checks(prover, &active_gate_rows, &active_lookup_rows)
         }
 
         // Run state circuit test
-        {
+        if let Some(state_checks) = &self.state_checks {
             let rows_needed = StateCircuit::<Fr>::min_num_rows_block(&block).1;
             let k = log2_ceil(rows_needed + NUM_BLINDING_ROWS);
             let state_circuit = StateCircuit::<Fr>::new(block.rws.clone(), params.max_rws);
@@ -260,11 +271,11 @@ impl<const NACC: usize, const NTX: usize> CircuitTestBuilder<NACC, NTX> {
                 .count();
             let rows = (params.max_rws - non_start_rows_len..params.max_rws).collect();
 
-            self.state_checks.as_ref()(prover, &rows, &rows);
+            state_checks(prover, &rows, &rows);
         }
 
         // Run copy circuit test
-        {
+        if let Some(copy_checks) = &self.copy_checks {
             let (active_rows, max_rows) = CopyCircuit::<Fr>::min_num_rows_block(&block);
             let k1 = block.get_test_degree();
             let k2 = log2_ceil(max_rows + NUM_BLINDING_ROWS);
@@ -274,7 +285,7 @@ impl<const NACC: usize, const NTX: usize> CircuitTestBuilder<NACC, NTX> {
             let prover = MockProver::<Fr>::run(k, &copy_circuit, instance).unwrap();
             let rows = (0..active_rows).collect();
 
-            self.copy_checks.as_ref()(prover, &rows, &rows);
+            copy_checks(prover, &rows, &rows);
         }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -251,11 +251,11 @@ mod test {`
`251`	`251`	`}`
`252`	`252`	`let mut ctb = CircuitTestBuilder::new_from_test_ctx(ctx);`
`253`	`253`	`if !ok {`
`254`		`- ctb = ctb.evm_checks(Box::new(\|prover, gate_rows, lookup_rows\| {`
	`254`	`+ ctb = ctb.evm_checks(Some(Box::new(\|prover, gate_rows, lookup_rows\| {`
`255`	`255`	`assert!(prover`
`256`	`256`	`.verify_at_rows_par(gate_rows.iter().cloned(), lookup_rows.iter().cloned())`
`257`	`257`	`.is_err())`
`258`		`- }));`
	`258`	`+ })));`
`259`	`259`	`};`
`260`	`260`	`ctb.run()`
`261`	`261`	`}`
Original file line number	Diff line number	Diff line change
`@@ -149,11 +149,11 @@ mod test {`
`149`	`149`	`assert_eq!(block.txs[0].steps.len(), 5);`
`150`	`150`	`block.txs[0].steps[2].gas_left -= 1;`
`151`	`151`	`}))`
`152`		`- .evm_checks(Box::new(\|prover, gate_rows, lookup_rows\| {`
	`152`	`+ .evm_checks(Some(Box::new(\|prover, gate_rows, lookup_rows\| {`
`153`	`153`	`assert!(prover`
`154`	`154`	`.verify_at_rows_par(gate_rows.iter().cloned(), lookup_rows.iter().cloned())`
`155`	`155`	`.is_err())`
`156`		`- }))`
	`156`	`+ })))`
`157`	`157`	`.run();`
`158`	`158`	`}`
`159`	`159`	`}`