chore: add ReentrancySentryOOG for SSTORE (#1795)

* feat: add ReentrancySentryOOG for SSTORE Signed-off-by: jsvisa <[email protected]> * sstore_cost return u64 instead of Option Signed-off-by: jsvisa <[email protected]> * fix testcase Signed-off-by: jsvisa <[email protected]> --------- Signed-off-by: jsvisa <[email protected]>
bluealloy · Sep 24, 2024 · 5b24f61 · 5b24f61
1 parent 3562d2e
commit 5b24f61
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 22 deletions.
diff --git a/crates/interpreter/src/gas/calc.rs b/crates/interpreter/src/gas/calc.rs
@@ -185,28 +185,21 @@ pub const fn sload_cost(spec_id: SpecId, is_cold: bool) -> u64 {
 
 /// `SSTORE` opcode cost calculation.
 #[inline]
-pub fn sstore_cost(spec_id: SpecId, vals: &SStoreResult, gas: u64, is_cold: bool) -> Option<u64> {
-    // EIP-1706 Disable SSTORE with gasleft lower than call stipend
-    if spec_id.is_enabled_in(SpecId::ISTANBUL) && gas <= CALL_STIPEND {
-        return None;
-    }
-
+pub fn sstore_cost(spec_id: SpecId, vals: &SStoreResult, is_cold: bool) -> u64 {
     if spec_id.is_enabled_in(SpecId::BERLIN) {
         // Berlin specification logic
         let mut gas_cost = istanbul_sstore_cost::<WARM_STORAGE_READ_COST, WARM_SSTORE_RESET>(vals);
 
         if is_cold {
             gas_cost += COLD_SLOAD_COST;
         }
-        Some(gas_cost)
+        gas_cost
     } else if spec_id.is_enabled_in(SpecId::ISTANBUL) {
         // Istanbul logic
-        Some(istanbul_sstore_cost::<INSTANBUL_SLOAD_GAS, SSTORE_RESET>(
-            vals,
-        ))
+        istanbul_sstore_cost::<INSTANBUL_SLOAD_GAS, SSTORE_RESET>(vals)
     } else {
         // Frontier logic
-        Some(frontier_sstore_cost(vals))
+        frontier_sstore_cost(vals)
     }
 }
 

diff --git a/crates/interpreter/src/instruction_result.rs b/crates/interpreter/src/instruction_result.rs
@@ -50,6 +50,8 @@ pub enum InstructionResult {
     PrecompileOOG,
     /// Out of gas error encountered while calling an invalid operand.
     InvalidOperandOOG,
+    /// Out of gas error encountered while checking for reentrancy sentry.
+    ReentrancySentryOOG,
     /// Unknown or invalid opcode.
     OpcodeNotFound,
     /// Invalid `CALL` with value transfer in static context.
@@ -118,6 +120,7 @@ impl From<HaltReason> for InstructionResult {
                 OutOfGasError::Memory => Self::MemoryOOG,
                 OutOfGasError::MemoryLimit => Self::MemoryLimitOOG,
                 OutOfGasError::Precompile => Self::PrecompileOOG,
+                OutOfGasError::ReentrancySentry => Self::ReentrancySentryOOG,
             },
             HaltReason::OpcodeNotFound => Self::OpcodeNotFound,
             HaltReason::InvalidFEOpcode => Self::InvalidFEOpcode,
@@ -176,6 +179,7 @@ macro_rules! return_error {
             | InstructionResult::MemoryLimitOOG
             | InstructionResult::PrecompileOOG
             | InstructionResult::InvalidOperandOOG
+            | InstructionResult::ReentrancySentryOOG
             | InstructionResult::OpcodeNotFound
             | InstructionResult::CallNotAllowedInsideStatic
             | InstructionResult::StateChangeDuringStaticCall
@@ -309,6 +313,9 @@ impl<HaltReasonT: HaltReasonTrait> From<InstructionResult> for SuccessOrHalt<Hal
             InstructionResult::InvalidOperandOOG => {
                 Self::Halt(HaltReason::OutOfGas(OutOfGasError::InvalidOperand).into())
             }
+            InstructionResult::ReentrancySentryOOG => {
+                Self::Halt(HaltReason::OutOfGas(OutOfGasError::ReentrancySentry).into())
+            }
             InstructionResult::OpcodeNotFound | InstructionResult::ReturnContractInNotInitEOF => {
                 Self::Halt(HaltReason::OpcodeNotFound.into())
             }

diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs
@@ -1,5 +1,5 @@
 use crate::{
-    gas::{self, warm_cold_cost, warm_cold_cost_with_delegation},
+    gas::{self, warm_cold_cost, warm_cold_cost_with_delegation, CALL_STIPEND},
     interpreter::Interpreter,
     Host, InstructionResult,
 };
@@ -136,15 +136,16 @@ pub fn sstore<H: Host + ?Sized, SPEC: Spec>(interpreter: &mut Interpreter, host:
         interpreter.instruction_result = InstructionResult::FatalExternalError;
         return;
     };
-    gas_or_fail!(interpreter, {
-        let remaining_gas = interpreter.gas.remaining();
-        gas::sstore_cost(
-            SPEC::SPEC_ID,
-            &state_load.data,
-            remaining_gas,
-            state_load.is_cold,
-        )
-    });
+
+    // EIP-1706 Disable SSTORE with gasleft lower than call stipend
+    if SPEC::SPEC_ID.is_enabled_in(ISTANBUL) && interpreter.gas.remaining() <= CALL_STIPEND {
+        interpreter.instruction_result = InstructionResult::ReentrancySentryOOG;
+        return;
+    }
+    gas!(
+        interpreter,
+        gas::sstore_cost(SPEC::SPEC_ID, &state_load.data, state_load.is_cold)
+    );
     refund!(
         interpreter,
         gas::sstore_refund(SPEC::SPEC_ID, &state_load.data)

diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs
@@ -250,7 +250,7 @@ macro_rules! pop_top {
 /// Pushes `B256` values onto the stack. Fails the instruction if the stack is full.
 #[macro_export]
 macro_rules! push_b256 {
-	($interp:expr, $($x:expr),* $(,)?) => ($(
+    ($interp:expr, $($x:expr),* $(,)?) => ($(
         match $interp.stack.push_b256($x) {
             Ok(()) => {},
             Err(e) => {

diff --git a/crates/wiring/src/result.rs b/crates/wiring/src/result.rs
@@ -463,4 +463,6 @@ pub enum OutOfGasError {
     // When performing something that takes a U256 and casts down to a u64, if its too large this would fire
     // i.e. in `as_usize_or_fail`
     InvalidOperand,
+    // When performing SSTORE the gasleft is less than or equal to 2300
+    ReentrancySentry,
 }