OOG error circuit for SLOAD and SSTORE (privacy-scaling-explorati…

…ons#1147)

Close
privacy-scaling-explorations#1119

Spec markdown PR
privacy-scaling-explorations/zkevm-specs#381

### Summary

1. Merge OOG errors for `SLOAD` and `SSTORE` into
`OogError::SloadSstore`, and error states into
`ExecutionState::ErrorOutOfGasSloadSstore`.

2. Move both `SloadGasGadget` and `SstoreGasGadget` to common gadget
which could be reused by `SloadGadget`, `SstoreGadget` and
`ErrorOOGSloadSstoreGadget`.

3. Implement `ErrorOutOfGasSloadSstore` state in both bus-mapping and
zkevm-circuit.

4. Constrain `SSTORE` OOG error for both `gas_cost` and `gas_sentry`.
Gas reentrancy sentry could be referenced in [this go-ethereum
code](https://github.com/ethereum/go-ethereum/blob/master/core/vm/operations_acl.go#L30).

---------

Co-authored-by: Han <[email protected]>

bus-mapping/src/error.rs

@@ -90,10 +90,8 @@ pub enum OogError {
     Sha3,
     /// Out of Gas for EXTCODECOPY
     ExtCodeCopy,
-    /// Out of Gas for SLOAD
-    Sload,
-    /// Out of Gas for SSTORE
-    Sstore,
+    /// Out of Gas for SLOAD and SSTORE
+    SloadSstore,
     /// Out of Gas for CALL, CALLCODE, DELEGATECALL and STATICCALL
     Call,
     /// Out of Gas for CREATE2
@@ -158,11 +156,10 @@ pub(crate) fn get_step_reported_error(op: &OpcodeId, error: &str) -> ExecError {
             OpcodeId::EXP => OogError::Exp,
             OpcodeId::SHA3 => OogError::Sha3,
             OpcodeId::EXTCODECOPY => OogError::ExtCodeCopy,
-            OpcodeId::SLOAD => OogError::Sload,
-            OpcodeId::SSTORE => OogError::Sstore,
             OpcodeId::CALL | OpcodeId::CALLCODE | OpcodeId::DELEGATECALL | OpcodeId::STATICCALL => {
                 OogError::Call
             }
+            OpcodeId::SLOAD | OpcodeId::SSTORE => OogError::SloadSstore,
             OpcodeId::CREATE2 => OogError::Create2,
             OpcodeId::SELFDESTRUCT => OogError::SelfDestruct,
             _ => OogError::Constant,

bus-mapping/src/evm/opcodes.rs

@@ -56,6 +56,7 @@ mod error_invalid_jump;
 mod error_invalid_opcode;
 mod error_oog_call;
 mod error_oog_log;
+mod error_oog_sload_sstore;
 mod error_return_data_outofbound;
 mod error_stack_oog_constant;
 
@@ -79,6 +80,7 @@ use error_invalid_jump::InvalidJump;
 use error_invalid_opcode::InvalidOpcode;
 use error_oog_call::OOGCall;
 use error_oog_log::ErrorOOGLog;
+use error_oog_sload_sstore::OOGSloadSstore;
 use error_return_data_outofbound::ErrorReturnDataOutOfBound;
 use error_stack_oog_constant::ErrorStackOogConstant;
 use exp::Exponentiation;
@@ -267,6 +269,7 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option<FnGenAssociate
         ExecError::InvalidOpcode => Some(InvalidOpcode::gen_associated_ops),
         ExecError::OutOfGas(OogError::Call) => Some(OOGCall::gen_associated_ops),
         ExecError::OutOfGas(OogError::Constant) => Some(ErrorStackOogConstant::gen_associated_ops),
+        ExecError::OutOfGas(OogError::SloadSstore) => Some(OOGSloadSstore::gen_associated_ops),
         ExecError::StackOverflow => Some(ErrorStackOogConstant::gen_associated_ops),
         ExecError::StackUnderflow => Some(ErrorStackOogConstant::gen_associated_ops),
         // call & callcode can encounter InsufficientBalance error, Use pop-7 generic CallOpcode

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

@@ -0,0 +1,96 @@
+use super::{Opcode, OpcodeId};
+use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep};
+use crate::error::{ExecError, OogError};
+use crate::operation::{CallContextField, RW};
+use crate::operation::{StorageOp, TxAccessListAccountStorageOp};
+use crate::Error;
+use eth_types::{GethExecStep, ToWord};
+
+/// Placeholder structure used to implement [`Opcode`] trait over it
+/// corresponding to the
+/// [`OogError::SloadSstore`](crate::error::OogError::SloadSstore).
+#[derive(Clone, Copy, Debug)]
+pub(crate) struct OOGSloadSstore;
+
+impl Opcode for OOGSloadSstore {
+    fn gen_associated_ops(
+        state: &mut CircuitInputStateRef,
+        geth_steps: &[GethExecStep],
+    ) -> Result<Vec<ExecStep>, Error> {
+        let geth_step = &geth_steps[0];
+        debug_assert!([OpcodeId::SLOAD, OpcodeId::SSTORE].contains(&geth_step.op));
+
+        let mut exec_step = state.new_step(geth_step)?;
+        exec_step.error = Some(ExecError::OutOfGas(OogError::SloadSstore));
+
+        let call_id = state.call()?.call_id;
+        let callee_address = state.call()?.address;
+        let tx_id = state.tx_ctx.id();
+
+        state.call_context_read(
+            &mut exec_step,
+            call_id,
+            CallContextField::TxId,
+            tx_id.into(),
+        );
+
+        state.call_context_read(
+            &mut exec_step,
+            call_id,
+            CallContextField::IsStatic,
+            (state.call()?.is_static as u8).into(),
+        );
+
+        state.call_context_read(
+            &mut exec_step,
+            call_id,
+            CallContextField::CalleeAddress,
+            callee_address.to_word(),
+        );
+
+        let key = geth_step.stack.last()?;
+        state.stack_read(&mut exec_step, geth_step.stack.last_filled(), key)?;
+
+        let is_warm = state
+            .sdb
+            .check_account_storage_in_access_list(&(callee_address, key));
+        state.push_op(
+            &mut exec_step,
+            RW::READ,
+            TxAccessListAccountStorageOp {
+                tx_id,
+                address: callee_address,
+                key,
+                is_warm,
+                is_warm_prev: is_warm,
+            },
+        );
+
+        // Special operations are only used for SSTORE.
+        if geth_step.op == OpcodeId::SSTORE {
+            let value = geth_step.stack.nth_last(1)?;
+            state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), value)?;
+
+            let (_, value_prev) = state.sdb.get_storage(&callee_address, &key);
+            let (_, original_value) = state.sdb.get_committed_storage(&callee_address, &key);
+
+            state.push_op(
+                &mut exec_step,
+                RW::READ,
+                StorageOp::new(
+                    callee_address,
+                    key,
+                    *value_prev,
+                    *value_prev,
+                    tx_id,
+                    *original_value,
+                ),
+            );
+        }
+
+        state.gen_restore_context_ops(&mut exec_step, geth_steps)?;
+        state.handle_return(geth_step)?;
+
+        Ok(vec![exec_step])
+    }
+}

eth-types/src/evm_types.rs

@@ -111,6 +111,8 @@ impl GasCost {
     pub const COLD_SLOAD: Self = Self(2100);
     /// Constant cost for a cold account access
     pub const COLD_ACCOUNT_ACCESS: Self = Self(2600);
+    /// SSTORE reentrancy sentry
+    pub const SSTORE_SENTRY: Self = Self(2300);
     /// Constant cost for a storage set
     pub const SSTORE_SET: Self = Self(20000);
     /// Constant cost for a storage reset

zkevm-circuits/src/evm_circuit/execution.rs

@@ -65,6 +65,7 @@ mod error_invalid_opcode;
 mod error_oog_call;
 mod error_oog_constant;
 mod error_oog_log;
+mod error_oog_sload_sstore;
 mod error_oog_static_memory;
 mod error_return_data_oo_bound;
 mod error_stack;
@@ -133,6 +134,7 @@ use error_invalid_opcode::ErrorInvalidOpcodeGadget;
 use error_oog_call::ErrorOOGCallGadget;
 use error_oog_constant::ErrorOOGConstantGadget;
 use error_oog_log::ErrorOOGLogGadget;
+use error_oog_sload_sstore::ErrorOOGSloadSstoreGadget;
 use error_return_data_oo_bound::ErrorReturnDataOutOfBoundGadget;
 use error_stack::ErrorStackGadget;
 use exp::ExponentiationGadget;
@@ -274,14 +276,13 @@ pub(crate) struct ExecutionConfig<F> {
     // error gadgets
     error_oog_call: ErrorOOGCallGadget<F>,
     error_oog_constant: ErrorOOGConstantGadget<F>,
+    error_oog_sload_sstore: ErrorOOGSloadSstoreGadget<F>,
     error_oog_static_memory_gadget:
         DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasStaticMemoryExpansion }>,
     error_stack: ErrorStackGadget<F>,
     error_oog_dynamic_memory_gadget:
         DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasDynamicMemoryExpansion }>,
     error_oog_log: ErrorOOGLogGadget<F>,
-    error_oog_sload: DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasSLOAD }>,
-    error_oog_sstore: DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasSSTORE }>,
     error_oog_memory_copy: DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasMemoryCopy }>,
     error_oog_account_access: DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasAccountAccess }>,
     error_oog_sha3: DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasSHA3 }>,
@@ -527,8 +528,7 @@ impl<F: Field> ExecutionConfig<F> {
             error_stack: configure_gadget!(),
             error_oog_dynamic_memory_gadget: configure_gadget!(),
             error_oog_log: configure_gadget!(),
-            error_oog_sload: configure_gadget!(),
-            error_oog_sstore: configure_gadget!(),
+            error_oog_sload_sstore: configure_gadget!(),
             error_oog_call: configure_gadget!(),
             error_oog_memory_copy: configure_gadget!(),
             error_oog_account_access: configure_gadget!(),
@@ -1177,11 +1177,8 @@ impl<F: Field> ExecutionConfig<F> {
             ExecutionState::ErrorOutOfGasLOG => {
                 assign_exec_step!(self.error_oog_log)
             }
-            ExecutionState::ErrorOutOfGasSLOAD => {
-                assign_exec_step!(self.error_oog_sload)
-            }
-            ExecutionState::ErrorOutOfGasSSTORE => {
-                assign_exec_step!(self.error_oog_sstore)
+            ExecutionState::ErrorOutOfGasSloadSstore => {
+                assign_exec_step!(self.error_oog_sload_sstore)
             }
             ExecutionState::ErrorOutOfGasMemoryCopy => {
                 assign_exec_step!(self.error_oog_memory_copy)