refactor common error gadget (privacy-scaling-explorations#1177)

### Description fix issue privacy-scaling-explorations#1059 This PR contains: - [x] add a new common error gadget - [x] update existing error gadget to use new common error gadget - [x] related tests pass NOTE: better to wait for previous error gadget PRs merge privacy-scaling-explorations#1001 privacy-scaling-explorations#1147 , then sync with this refactoring.
ed255 · Feb 23, 2023 · 0120c7f · 0120c7f
1 parent a8318ea
commit 0120c7f
Show file tree

Hide file tree

Showing 9 changed files with 181 additions and 562 deletions.
diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_jump.rs
@@ -4,18 +4,14 @@ use crate::{
         param::N_BYTES_PROGRAM_COUNTER,
         step::ExecutionState,
         util::{
-            common_gadget::RestoreContextGadget,
-            constraint_builder::{
-                ConstraintBuilder, StepStateTransition,
-                Transition::{Delta, Same},
-            },
+            common_gadget::CommonErrorGadget,
+            constraint_builder::ConstraintBuilder,
             from_bytes,
             math_gadget::{IsEqualGadget, IsZeroGadget, LtGadget},
             CachedRegion, Cell, RandomLinearCombination,
         },
         witness::{Block, Call, ExecStep, Transaction},
     },
-    table::CallContextFieldTag,
     util::Expr,
 };
 use eth_types::{evm_types::OpcodeId, Field, ToLittleEndian, Word};
@@ -34,8 +30,7 @@ pub(crate) struct ErrorInvalidJumpGadget<F> {
     is_jumpi: IsEqualGadget<F>,
     phase2_condition: Cell<F>,
     is_condition_zero: IsZeroGadget<F>,
-    rw_counter_end_of_reversion: Cell<F>,
-    restore_context: RestoreContextGadget<F>,
+    common_error_gadget: CommonErrorGadget<F>,
 }
 
 impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
@@ -48,7 +43,6 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
         let opcode = cb.query_cell();
         let value = cb.query_cell();
         let is_code = cb.query_cell();
-        let rw_counter_end_of_reversion = cb.query_cell();
         let phase2_condition = cb.query_cell_phase2();
 
         cb.require_in_set(
@@ -96,59 +90,8 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
             );
         });
 
-        cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr());
-
-        cb.call_context_lookup(
-            false.expr(),
-            None,
-            CallContextFieldTag::RwCounterEndOfReversion,
-            rw_counter_end_of_reversion.expr(),
-        );
-
-        // Go to EndTx only when is_root
-        let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]);
-        cb.require_equal(
-            "Go to EndTx only when is_root",
-            cb.curr.state.is_root.expr(),
-            is_to_end_tx,
-        );
-
-        // When it's a root call
-        cb.condition(cb.curr.state.is_root.expr(), |cb| {
-            // Do step state transition
-            cb.require_step_state_transition(StepStateTransition {
-                call_id: Same,
-                rw_counter: Delta(
-                    3.expr() + is_jumpi.expr() + cb.curr.state.reversible_write_counter.expr(),
-                ),
-
-                ..StepStateTransition::any()
-            });
-        });
-
-        // When it's an internal call, need to restore caller's state as finishing this
-        // call. Restore caller state to next StepState
-        let restore_context = cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| {
-            RestoreContextGadget::construct(
-                cb,
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-            )
-        });
-
-        // constrain RwCounterEndOfReversion
-        let rw_counter_end_of_step =
-            cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr();
-        cb.require_equal(
-            "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter",
-            rw_counter_end_of_reversion.expr(),
-            rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(),
-        );
-
+        let common_error_gadget =
+            CommonErrorGadget::construct(cb, opcode.expr(), 3.expr() + is_jumpi.expr());
         Self {
             opcode,
             destination,
@@ -160,8 +103,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
             is_jumpi,
             phase2_condition,
             is_condition_zero,
-            rw_counter_end_of_reversion,
-            restore_context,
+            common_error_gadget,
         }
     }
 
@@ -241,13 +183,15 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidJumpGadget<F> {
         self.is_condition_zero
             .assign_value(region, offset, condition_rlc)?;
 
-        self.rw_counter_end_of_reversion.assign(
+        self.common_error_gadget.assign(
             region,
             offset,
-            Value::known(F::from(call.rw_counter_end_of_reversion as u64)),
+            block,
+            call,
+            step,
+            3 + is_jumpi as usize,
         )?;
-        self.restore_context
-            .assign(region, offset, block, call, step, 3 + is_jumpi as usize)?;
+
         Ok(())
     }
 }

diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs
@@ -1,12 +1,10 @@
 use crate::evm_circuit::execution::ExecutionGadget;
 use crate::evm_circuit::step::ExecutionState;
 use crate::evm_circuit::table::{FixedTableTag, Lookup};
-use crate::evm_circuit::util::common_gadget::RestoreContextGadget;
-use crate::evm_circuit::util::constraint_builder::Transition::{Delta, Same};
-use crate::evm_circuit::util::constraint_builder::{ConstraintBuilder, StepStateTransition};
-use crate::evm_circuit::util::{not, CachedRegion, Cell};
+use crate::evm_circuit::util::common_gadget::CommonErrorGadget;
+use crate::evm_circuit::util::constraint_builder::ConstraintBuilder;
+use crate::evm_circuit::util::{CachedRegion, Cell};
 use crate::evm_circuit::witness::{Block, Call, ExecStep, Transaction};
-use crate::table::CallContextFieldTag;
 use eth_types::Field;
 use gadgets::util::Expr;
 use halo2_proofs::circuit::Value;
@@ -17,8 +15,7 @@ use halo2_proofs::plonk::Error;
 #[derive(Clone, Debug)]
 pub(crate) struct ErrorInvalidOpcodeGadget<F> {
     opcode: Cell<F>,
-    rw_counter_end_of_reversion: Cell<F>,
-    restore_context: RestoreContextGadget<F>,
+    common_error_gadget: CommonErrorGadget<F>,
 }
 
 impl<F: Field> ExecutionGadget<F> for ErrorInvalidOpcodeGadget<F> {
@@ -28,7 +25,6 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidOpcodeGadget<F> {
 
     fn configure(cb: &mut ConstraintBuilder<F>) -> Self {
         let opcode = cb.query_cell();
-        cb.opcode_lookup(opcode.expr(), 1.expr());
         cb.add_lookup(
             "Responsible opcode lookup",
             Lookup::Fixed {
@@ -41,61 +37,11 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidOpcodeGadget<F> {
             },
         );
 
-        // Current call must be failed.
-        let rw_counter_end_of_reversion = cb.query_cell();
-        cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr());
-        cb.call_context_lookup(
-            false.expr(),
-            None,
-            CallContextFieldTag::RwCounterEndOfReversion,
-            rw_counter_end_of_reversion.expr(),
-        );
-
-        // Go to EndTx only when is_root.
-        let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]);
-        cb.require_equal(
-            "Go to EndTx only when is_root",
-            cb.curr.state.is_root.expr(),
-            is_to_end_tx,
-        );
-
-        // When it's a root call.
-        cb.condition(cb.curr.state.is_root.expr(), |cb| {
-            // Do step state transition
-            cb.require_step_state_transition(StepStateTransition {
-                call_id: Same,
-                rw_counter: Delta(2.expr() + cb.curr.state.reversible_write_counter.expr()),
-                ..StepStateTransition::any()
-            });
-        });
-
-        // When it is an internal call, need to restore caller's state as finishing this
-        // call. Restore caller state to next StepState.
-        let restore_context = cb.condition(not::expr(cb.curr.state.is_root.expr()), |cb| {
-            RestoreContextGadget::construct(
-                cb,
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-            )
-        });
-
-        // Constrain `RwCounterEndOfReversion`.
-        let rw_counter_end_of_step =
-            cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr();
-        cb.require_equal(
-            "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter",
-            rw_counter_end_of_reversion.expr(),
-            rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(),
-        );
+        let common_error_gadget = CommonErrorGadget::construct(cb, opcode.expr(), 2.expr());
 
         Self {
             opcode,
-            rw_counter_end_of_reversion,
-            restore_context,
+            common_error_gadget,
         }
     }
 
@@ -111,14 +57,9 @@ impl<F: Field> ExecutionGadget<F> for ErrorInvalidOpcodeGadget<F> {
         let opcode = F::from(step.opcode.unwrap().as_u64());
         self.opcode.assign(region, offset, Value::known(opcode))?;
 
-        self.rw_counter_end_of_reversion.assign(
-            region,
-            offset,
-            Value::known(F::from(call.rw_counter_end_of_reversion as u64)),
-        )?;
-
-        self.restore_context
-            .assign(region, offset, block, call, step, 2)
+        self.common_error_gadget
+            .assign(region, offset, block, call, step, 2)?;
+        Ok(())
     }
 }
 

diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs
@@ -6,11 +6,8 @@ use crate::{
         param::N_BYTES_GAS,
         step::ExecutionState,
         util::{
-            common_gadget::{CommonCallGadget, RestoreContextGadget},
-            constraint_builder::{
-                ConstraintBuilder, StepStateTransition,
-                Transition::{Delta, Same},
-            },
+            common_gadget::{CommonCallGadget, CommonErrorGadget},
+            constraint_builder::ConstraintBuilder,
             math_gadget::{IsZeroGadget, LtGadget},
             CachedRegion, Cell,
         },
@@ -36,8 +33,7 @@ pub(crate) struct ErrorOOGCallGadget<F> {
     call: CommonCallGadget<F, false>,
     is_warm: Cell<F>,
     insufficient_gas: LtGadget<F, N_BYTES_GAS>,
-    rw_counter_end_of_reversion: Cell<F>,
-    restore_context: RestoreContextGadget<F>,
+    common_error_gadget: CommonErrorGadget<F>,
 }
 
 impl<F: Field> ExecutionGadget<F> for ErrorOOGCallGadget<F> {
@@ -47,15 +43,14 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGCallGadget<F> {
 
     fn configure(cb: &mut ConstraintBuilder<F>) -> Self {
         let opcode = cb.query_cell();
-        cb.opcode_lookup(opcode.expr(), 1.expr());
+
         let is_call = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALL.expr());
         let is_callcode = IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::CALLCODE.expr());
         let is_delegatecall =
             IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::DELEGATECALL.expr());
         let is_staticcall =
             IsZeroGadget::construct(cb, opcode.expr() - OpcodeId::STATICCALL.expr());
 
-        let rw_counter_end_of_reversion = cb.query_cell();
         let tx_id = cb.call_context(None, CallContextFieldTag::TxId);
         let is_static = cb.call_context(None, CallContextFieldTag::IsStatic);
 
@@ -93,62 +88,12 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGCallGadget<F> {
             1.expr(),
         );
 
-        // current call must be failed.
-        cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr());
-
-        cb.call_context_lookup(
-            false.expr(),
-            None,
-            CallContextFieldTag::RwCounterEndOfReversion,
-            rw_counter_end_of_reversion.expr(),
-        );
-
-        // Go to EndTx only when is_root
-        let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]);
-        cb.require_equal(
-            "Go to EndTx only when is_root",
-            cb.curr.state.is_root.expr(),
-            is_to_end_tx,
-        );
-
-        // When it's a root call
-        cb.condition(cb.curr.state.is_root.expr(), |cb| {
-            // Do step state transition
-            cb.require_step_state_transition(StepStateTransition {
-                call_id: Same,
-                // Both CALL and CALLCODE opcodes have an extra stack pop `value` relative to
-                // DELEGATECALL and STATICCALL.
-                rw_counter: Delta(
-                    13.expr()
-                        + is_call.expr()
-                        + is_callcode.expr()
-                        + cb.curr.state.reversible_write_counter.expr(),
-                ),
-                ..StepStateTransition::any()
-            });
-        });
-
-        // When it's an internal call, need to restore caller's state as finishing this
-        // call. Restore caller state to next StepState
-        let restore_context = cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| {
-            RestoreContextGadget::construct(
-                cb,
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-                0.expr(),
-            )
-        });
-
-        // constrain RwCounterEndOfReversion
-        let rw_counter_end_of_step =
-            cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr();
-        cb.require_equal(
-            "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter",
-            rw_counter_end_of_reversion.expr(),
-            rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(),
+        // Both CALL and CALLCODE opcodes have an extra stack pop `value` relative to
+        // DELEGATECALL and STATICCALL.
+        let common_error_gadget = CommonErrorGadget::construct(
+            cb,
+            opcode.expr(),
+            13.expr() + is_call.expr() + is_callcode.expr(),
         );
 
         Self {
@@ -162,8 +107,7 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGCallGadget<F> {
             call: call_gadget,
             is_warm,
             insufficient_gas,
-            rw_counter_end_of_reversion,
-            restore_context,
+            common_error_gadget,
         }
     }
 
@@ -272,16 +216,17 @@ impl<F: Field> ExecutionGadget<F> for ErrorOOGCallGadget<F> {
             Value::known(F::from(gas_cost)),
         )?;
 
-        self.rw_counter_end_of_reversion.assign(
+        // Both CALL and CALLCODE opcodes have an extra stack pop `value` relative to
+        // DELEGATECALL and STATICCALL.
+        self.common_error_gadget.assign(
             region,
             offset,
-            Value::known(F::from(call.rw_counter_end_of_reversion as u64)),
+            block,
+            call,
+            step,
+            13 + is_call_or_callcode,
         )?;
-
-        // Both CALL and CALLCODE opcodes have an extra stack pop `value` relative to
-        // DELEGATECALL and STATICCALL.
-        self.restore_context
-            .assign(region, offset, block, call, step, 13 + is_call_or_callcode)
+        Ok(())
     }
 }