stylus: fix gas overcharging in get_bytes32 for new arbos

OffchainLabs · Sep 13, 2024 · 0ca2aeb · 0ca2aeb
1 parent a219e3e
commit 0ca2aeb
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 10 deletions.
diff --git a/arbitrator/arbutil/src/evm/api.rs b/arbitrator/arbutil/src/evm/api.rs
@@ -77,7 +77,7 @@ pub trait EvmApi<D: DataReader>: Send + 'static {
     /// Reads the 32-byte value in the EVM state trie at offset `key`.
     /// Returns the value and the access cost in gas.
     /// Analogous to `vm.SLOAD`.
-    fn get_bytes32(&mut self, key: Bytes32) -> (Bytes32, u64);
+    fn get_bytes32(&mut self, key: Bytes32, evm_api_gas_to_use: u64) -> (Bytes32, u64);
 
     /// Stores the given value at the given key in Stylus VM's cache of the EVM state trie.
     /// Note that the actual values only get written after calls to `set_trie_slots`.

diff --git a/arbitrator/arbutil/src/evm/req.rs b/arbitrator/arbutil/src/evm/req.rs
@@ -7,7 +7,6 @@ use crate::{
         storage::{StorageCache, StorageWord},
         user::UserOutcomeKind,
     },
-    pricing::EVM_API_INK,
     Bytes20, Bytes32,
 };
 use eyre::{bail, eyre, Result};
@@ -99,13 +98,13 @@ impl<D: DataReader, H: RequestHandler<D>> EvmApiRequestor<D, H> {
 }
 
 impl<D: DataReader, H: RequestHandler<D>> EvmApi<D> for EvmApiRequestor<D, H> {
-    fn get_bytes32(&mut self, key: Bytes32) -> (Bytes32, u64) {
+    fn get_bytes32(&mut self, key: Bytes32, evm_api_gas_to_use: u64) -> (Bytes32, u64) {
         let cache = &mut self.storage_cache;
         let mut cost = cache.read_gas();
 
         let value = cache.entry(key).or_insert_with(|| {
             let (res, _, gas) = self.handler.request(EvmApiMethod::GetBytes32, key);
-            cost = cost.saturating_add(gas).saturating_add(EVM_API_INK);
+            cost = cost.saturating_add(gas).saturating_add(evm_api_gas_to_use);
             StorageWord::known(res.try_into().unwrap())
         });
         (value.value, cost)

diff --git a/arbitrator/stylus/src/test/api.rs b/arbitrator/stylus/src/test/api.rs
@@ -68,7 +68,7 @@ impl TestEvmApi {
 }
 
 impl EvmApi<VecReader> for TestEvmApi {
-    fn get_bytes32(&mut self, key: Bytes32) -> (Bytes32, u64) {
+    fn get_bytes32(&mut self, key: Bytes32, _evm_api_gas_to_use: u64) -> (Bytes32, u64) {
         let storage = &mut self.storage.lock();
         let storage = storage.get_mut(&self.program).unwrap();
         let value = storage.get(&key).cloned().unwrap_or_default();

diff --git a/arbitrator/stylus/src/test/native.rs b/arbitrator/stylus/src/test/native.rs
@@ -381,7 +381,7 @@ fn test_storage() -> Result<()> {
 
     let (mut native, mut evm) = TestInstance::new_with_evm(filename, &compile, config)?;
     run_native(&mut native, &store_args, ink)?;
-    assert_eq!(evm.get_bytes32(key.into()).0, Bytes32(value));
+    assert_eq!(evm.get_bytes32(key.into(), 0).0, Bytes32(value));
     assert_eq!(run_native(&mut native, &load_args, ink)?, value);
 
     let mut machine = Machine::from_user_path(Path::new(filename), &compile)?;
@@ -465,7 +465,7 @@ fn test_calls() -> Result<()> {
     run_native(&mut native, &args, ink)?;
 
     for (key, value) in slots {
-        assert_eq!(evm.get_bytes32(key).0, value);
+        assert_eq!(evm.get_bytes32(key, 0).0, value);
     }
     Ok(())
 }

diff --git a/arbitrator/wasm-libraries/user-host-trait/src/lib.rs b/arbitrator/wasm-libraries/user-host-trait/src/lib.rs
@@ -8,7 +8,7 @@ use arbutil::{
         api::{DataReader, EvmApi},
         storage::StorageCache,
         user::UserOutcomeKind,
-        EvmData,
+        EvmData, ARBOS_VERSION_STYLUS_CHARGING_FIXES,
     },
     pricing::{self, EVM_API_INK, HOSTIO_INK, PTR_INK},
     Bytes20, Bytes32,
@@ -143,11 +143,20 @@ pub trait UserHost<DR: DataReader>: GasMeteredMachine {
     /// [`SLOAD`]: https://www.evm.codes/#54
     fn storage_load_bytes32(&mut self, key: GuestPtr, dest: GuestPtr) -> Result<(), Self::Err> {
         self.buy_ink(HOSTIO_INK + 2 * PTR_INK)?;
-        self.require_gas(evm::COLD_SLOAD_GAS + EVM_API_INK + StorageCache::REQUIRED_ACCESS_GAS)?; // cache-miss case
+        let arbos_version = self.evm_data().arbos_version;
 
+        // require for cache-miss case, preserve wrong behavior for old arbos
+        let evm_api_gas_to_use = if arbos_version < ARBOS_VERSION_STYLUS_CHARGING_FIXES {
+            EVM_API_INK
+        } else {
+            self.pricing().ink_to_gas(EVM_API_INK)
+        };
+        self.require_gas(
+            evm::COLD_SLOAD_GAS + StorageCache::REQUIRED_ACCESS_GAS + evm_api_gas_to_use,
+        )?;
         let key = self.read_bytes32(key)?;
 
-        let (value, gas_cost) = self.evm_api().get_bytes32(key);
+        let (value, gas_cost) = self.evm_api().get_bytes32(key, evm_api_gas_to_use);
         self.buy_gas(gas_cost)?;
         self.write_bytes32(dest, value)?;
         trace!("storage_load_bytes32", self, key, value)