Skip to content

Commit

Permalink
Merge pull request #2677 from OffchainLabs/stylus_gas_ink_fixes
Browse files Browse the repository at this point in the history
Stylus gas ink fixes
  • Loading branch information
tsahee authored Sep 20, 2024
2 parents 12ac2e4 + f6fcf75 commit 8fd0ff4
Show file tree
Hide file tree
Showing 13 changed files with 50 additions and 12 deletions.
2 changes: 1 addition & 1 deletion arbitrator/arbutil/src/evm/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
1 change: 1 addition & 0 deletions arbitrator/arbutil/src/evm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ pub const ARBOS_VERSION_STYLUS_CHARGING_FIXES: u64 = 32;
#[derive(Clone, Copy, Debug, Default)]
#[repr(C)]
pub struct EvmData {
pub arbos_version: u64,
pub block_basefee: Bytes32,
pub chainid: u64,
pub block_coinbase: Bytes20,
Expand Down
5 changes: 2 additions & 3 deletions arbitrator/arbutil/src/evm/req.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::{
storage::{StorageCache, StorageWord},
user::UserOutcomeKind,
},
pricing::EVM_API_INK,
Bytes20, Bytes32,
};
use eyre::{bail, eyre, Result};
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions arbitrator/jit/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ pub fn create_stylus_config(
/// Creates an `EvmData` handler from its component parts.
pub fn create_evm_data(
mut env: WasmEnvMut,
arbos_version: u64,
block_basefee_ptr: GuestPtr,
chainid: u64,
block_coinbase_ptr: GuestPtr,
Expand All @@ -243,6 +244,7 @@ pub fn create_evm_data(
let (mut mem, _) = env.jit_env();

let evm_data = EvmData {
arbos_version,
block_basefee: mem.read_bytes32(block_basefee_ptr),
cached: cached != 0,
chainid,
Expand Down
2 changes: 1 addition & 1 deletion arbitrator/stylus/src/test/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions arbitrator/stylus/src/test/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;
Expand Down Expand Up @@ -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(())
}
Expand Down
15 changes: 12 additions & 3 deletions arbitrator/wasm-libraries/user-host-trait/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions arbitrator/wasm-libraries/user-host/src/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ pub unsafe extern "C" fn programs__create_stylus_config(
///
#[no_mangle]
pub unsafe extern "C" fn programs__create_evm_data(
arbos_version: u64,
block_basefee_ptr: GuestPtr,
chainid: u64,
block_coinbase_ptr: GuestPtr,
Expand All @@ -259,6 +260,7 @@ pub unsafe extern "C" fn programs__create_evm_data(
reentrant: u32,
) -> u64 {
let evm_data = EvmData {
arbos_version,
block_basefee: read_bytes32(block_basefee_ptr),
cached: cached != 0,
chainid,
Expand Down
2 changes: 1 addition & 1 deletion arbitrator/wasm-libraries/user-test/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl Program {
pub struct MockEvmApi;

impl EvmApi<VecReader> for MockEvmApi {
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 value = KEYS.lock().get(&key).cloned().unwrap_or_default();
(value, 2100) // pretend worst case
}
Expand Down
1 change: 1 addition & 0 deletions arbos/programs/native.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ func (params *ProgParams) encode() C.StylusConfig {

func (data *EvmData) encode() C.EvmData {
return C.EvmData{
arbos_version: u64(data.arbosVersion),
block_basefee: hashToBytes32(data.blockBasefee),
chainid: u64(data.chainId),
block_coinbase: addressToBytes20(data.blockCoinbase),
Expand Down
22 changes: 22 additions & 0 deletions arbos/programs/programs.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ func (p Programs) CallProgram(
}

evmData := &EvmData{
arbosVersion: evm.Context.ArbOSVersion,
blockBasefee: common.BigToHash(evm.Context.BaseFee),
chainId: evm.ChainConfig().ChainID.Uint64(),
blockCoinbase: evm.Context.Coinbase,
Expand Down Expand Up @@ -516,7 +517,9 @@ func (p Programs) progParams(version uint16, debug bool, params *StylusParams) *
}
}

// lint:require-exhaustive-initialization
type EvmData struct {
arbosVersion uint64
blockBasefee common.Hash
chainId uint64
blockCoinbase common.Address
Expand All @@ -534,6 +537,25 @@ type EvmData struct {
tracing bool
}

var EmptyEvmData EvmData = EvmData{
arbosVersion: 0,
blockBasefee: common.Hash{},
chainId: 0,
blockCoinbase: common.Address{},
blockGasLimit: 0,
blockNumber: 0,
blockTimestamp: 0,
contractAddress: common.Address{},
moduleHash: common.Hash{},
msgSender: common.Address{},
msgValue: common.Hash{},
txGasPrice: common.Hash{},
txOrigin: common.Address{},
reentrant: 0,
cached: false,
tracing: false,
}

type activationInfo struct {
moduleHash common.Hash
initGas uint16
Expand Down
2 changes: 1 addition & 1 deletion arbos/programs/testcompile.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ func testCompileLoad() error {

calldata := []byte{}

evmData := EvmData{}
evmData := EmptyEvmData
progParams := ProgParams{
MaxDepth: 10000,
InkPrice: 1,
Expand Down
2 changes: 2 additions & 0 deletions arbos/programs/wasm_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ type evmDataHandler uint64

//go:wasmimport programs create_evm_data
func createEvmData(
arbosVersion uint64,
blockBaseFee unsafe.Pointer,
chainid uint64,
blockCoinbase unsafe.Pointer,
Expand All @@ -45,6 +46,7 @@ func (params *ProgParams) createHandler() stylusConfigHandler {

func (data *EvmData) createHandler() evmDataHandler {
return createEvmData(
data.arbosVersion,
arbutil.SliceToUnsafePointer(data.blockBasefee[:]),
data.chainId,
arbutil.SliceToUnsafePointer(data.blockCoinbase[:]),
Expand Down

0 comments on commit 8fd0ff4

Please sign in to comment.