From 9d017e5840835643b84e3041a2dc4e27de79dc81 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Wed, 4 Dec 2024 14:50:11 +0100 Subject: [PATCH 1/9] Etherlink/Sputnik: rust-fmt --- etherlink/sputnikvm/runtime/src/eval/system.rs | 9 +-------- etherlink/sputnikvm/runtime/src/handler.rs | 4 +++- etherlink/sputnikvm/src/executor/stack/executor.rs | 8 ++++---- 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/etherlink/sputnikvm/runtime/src/eval/system.rs b/etherlink/sputnikvm/runtime/src/eval/system.rs index 3487b9221435..c0dcee5b9d49 100644 --- a/etherlink/sputnikvm/runtime/src/eval/system.rs +++ b/etherlink/sputnikvm/runtime/src/eval/system.rs @@ -387,14 +387,7 @@ pub fn call(runtime: &mut Runtime, scheme: CallScheme, handler: &mut None }; - match handler.call( - to.into(), - transfer, - input, - gas, - scheme, - context, - ) { + match handler.call(to.into(), transfer, input, gas, scheme, context) { Capture::Exit((reason, return_data)) => { match super::finish_call(runtime, out_len, out_offset, reason, return_data) { Ok(()) => Control::Continue, diff --git a/etherlink/sputnikvm/runtime/src/handler.rs b/etherlink/sputnikvm/runtime/src/handler.rs index 1714157fcacf..356661c248fd 100644 --- a/etherlink/sputnikvm/runtime/src/handler.rs +++ b/etherlink/sputnikvm/runtime/src/handler.rs @@ -1,4 +1,6 @@ -use crate::{CallScheme, Capture, Context, CreateScheme, ExitError, ExitReason, Machine, Opcode, Stack}; +use crate::{ + CallScheme, Capture, Context, CreateScheme, ExitError, ExitReason, Machine, Opcode, Stack, +}; use alloc::vec::Vec; use primitive_types::{H160, H256, U256}; diff --git a/etherlink/sputnikvm/src/executor/stack/executor.rs b/etherlink/sputnikvm/src/executor/stack/executor.rs index c1b6cbd7db8d..5e9c937b10bf 100644 --- a/etherlink/sputnikvm/src/executor/stack/executor.rs +++ b/etherlink/sputnikvm/src/executor/stack/executor.rs @@ -6,8 +6,8 @@ use crate::executor::stack::tagged_runtime::{RuntimeKind, TaggedRuntime}; use crate::gasometer::{self, Gasometer, StorageTarget}; use crate::maybe_borrowed::MaybeBorrowed; use crate::{ - CallScheme, Capture, Config, Context, CreateScheme, ExitError, ExitReason, Handler, Opcode, Runtime, Stack, - Transfer, + CallScheme, Capture, Config, Context, CreateScheme, ExitError, ExitReason, Handler, Opcode, + Runtime, Stack, Transfer, }; use alloc::{collections::BTreeSet, rc::Rc, vec::Vec}; use core::{cmp::min, convert::Infallible}; @@ -802,7 +802,7 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> fn l64(gas: u64) -> u64 { gas - gas / 64 } - + let is_static = call_scheme == CallScheme::StaticCall; event!(Call { @@ -1347,7 +1347,7 @@ impl<'inner, 'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Pr { return (ExitReason::Error(error), Vec::new()); } - + event!(PrecompileSubcall { code_address, transfer: &transfer, -- GitLab From 8545dfb6134a3102877c9195a72c336172cc25a0 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Wed, 4 Dec 2024 14:50:44 +0100 Subject: [PATCH 2/9] Etherlink/Sputnik: make the handler mutable for storage access --- etherlink/kernel_bifrost/evm_execution/src/handler.rs | 4 ++-- etherlink/kernel_evm/evm_execution/src/handler.rs | 4 ++-- etherlink/sputnikvm/runtime/src/eval/system.rs | 2 +- etherlink/sputnikvm/runtime/src/handler.rs | 4 ++-- etherlink/sputnikvm/src/executor/stack/executor.rs | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/etherlink/kernel_bifrost/evm_execution/src/handler.rs b/etherlink/kernel_bifrost/evm_execution/src/handler.rs index 3258a339ebf4..4888df0a3145 100644 --- a/etherlink/kernel_bifrost/evm_execution/src/handler.rs +++ b/etherlink/kernel_bifrost/evm_execution/src/handler.rs @@ -2078,7 +2078,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { .unwrap_or_default() } - fn storage(&self, address: H160, index: H256) -> H256 { + fn storage(&mut self, address: H160, index: H256) -> H256 { self.get_account(address) .ok() .flatten() @@ -2086,7 +2086,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { .unwrap_or_default() } - fn original_storage(&self, address: H160, index: H256) -> H256 { + fn original_storage(&mut self, address: H160, index: H256) -> H256 { self.get_original_account(address) .ok() .flatten() diff --git a/etherlink/kernel_evm/evm_execution/src/handler.rs b/etherlink/kernel_evm/evm_execution/src/handler.rs index 3258a339ebf4..4888df0a3145 100644 --- a/etherlink/kernel_evm/evm_execution/src/handler.rs +++ b/etherlink/kernel_evm/evm_execution/src/handler.rs @@ -2078,7 +2078,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { .unwrap_or_default() } - fn storage(&self, address: H160, index: H256) -> H256 { + fn storage(&mut self, address: H160, index: H256) -> H256 { self.get_account(address) .ok() .flatten() @@ -2086,7 +2086,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { .unwrap_or_default() } - fn original_storage(&self, address: H160, index: H256) -> H256 { + fn original_storage(&mut self, address: H160, index: H256) -> H256 { self.get_original_account(address) .ok() .flatten() diff --git a/etherlink/sputnikvm/runtime/src/eval/system.rs b/etherlink/sputnikvm/runtime/src/eval/system.rs index c0dcee5b9d49..5b5353751035 100644 --- a/etherlink/sputnikvm/runtime/src/eval/system.rs +++ b/etherlink/sputnikvm/runtime/src/eval/system.rs @@ -199,7 +199,7 @@ pub fn gaslimit(runtime: &mut Runtime, handler: &H) -> Control { Control::Continue } -pub fn sload(runtime: &mut Runtime, handler: &H) -> Control { +pub fn sload(runtime: &mut Runtime, handler: &mut H) -> Control { pop!(runtime, index); let value = handler.storage(runtime.context.address, index); push!(runtime, value); diff --git a/etherlink/sputnikvm/runtime/src/handler.rs b/etherlink/sputnikvm/runtime/src/handler.rs index 356661c248fd..e87b88d53c66 100644 --- a/etherlink/sputnikvm/runtime/src/handler.rs +++ b/etherlink/sputnikvm/runtime/src/handler.rs @@ -36,9 +36,9 @@ pub trait Handler { /// Get code of address. fn code(&self, address: H160) -> Vec; /// Get storage value of address at index. - fn storage(&self, address: H160, index: H256) -> H256; + fn storage(&mut self, address: H160, index: H256) -> H256; /// Get original storage value of address at index. - fn original_storage(&self, address: H160, index: H256) -> H256; + fn original_storage(&mut self, address: H160, index: H256) -> H256; /// Get the gas left value. fn gas_left(&self) -> U256; diff --git a/etherlink/sputnikvm/src/executor/stack/executor.rs b/etherlink/sputnikvm/src/executor/stack/executor.rs index 5e9c937b10bf..480ea3bd43fb 100644 --- a/etherlink/sputnikvm/src/executor/stack/executor.rs +++ b/etherlink/sputnikvm/src/executor/stack/executor.rs @@ -1045,11 +1045,11 @@ impl<'config, 'precompiles, S: StackState<'config>, P: PrecompileSet> Handler self.state.code(address) } - fn storage(&self, address: H160, index: H256) -> H256 { + fn storage(&mut self, address: H160, index: H256) -> H256 { self.state.storage(address, index) } - fn original_storage(&self, address: H160, index: H256) -> H256 { + fn original_storage(&mut self, address: H160, index: H256) -> H256 { self.state .original_storage(address, index) .unwrap_or_default() -- GitLab From 13e76c9bc7111086fb6d4d0570a18a1e403a014d Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Mon, 2 Dec 2024 16:27:26 +0100 Subject: [PATCH 3/9] Etherlink/Execution: rely on a cache for storage access This is done in order to avoid having to read/write every time a global value is used in a smart contract. We rely on a local cache for r/w access and only flush in the durable storage once the whole execution is over. --- .../evm_execution/src/access_record.rs | 2 - .../kernel_evm/evm_execution/src/handler.rs | 234 ++++++++++++++---- 2 files changed, 189 insertions(+), 47 deletions(-) diff --git a/etherlink/kernel_evm/evm_execution/src/access_record.rs b/etherlink/kernel_evm/evm_execution/src/access_record.rs index c207063fe283..50a6942f9650 100644 --- a/etherlink/kernel_evm/evm_execution/src/access_record.rs +++ b/etherlink/kernel_evm/evm_execution/src/access_record.rs @@ -4,8 +4,6 @@ use primitive_types::{H160, H256}; #[derive(Eq, Clone, PartialOrd, PartialEq, Ord, Debug)] struct AddressIndex(H160, H256); -// TODO: Implement caching for accessed storage value -// issue: https://gitlab.com/tezos/tezos/-/issues/6879 #[derive(Clone, Debug)] pub struct AccessRecord { accessed_storage_keys: BTreeSet, diff --git a/etherlink/kernel_evm/evm_execution/src/handler.rs b/etherlink/kernel_evm/evm_execution/src/handler.rs index 4888df0a3145..bc51c4d88bd9 100644 --- a/etherlink/kernel_evm/evm_execution/src/handler.rs +++ b/etherlink/kernel_evm/evm_execution/src/handler.rs @@ -1,5 +1,5 @@ // SPDX-FileCopyrightText: 2022-2024 TriliTech -// SPDX-FileCopyrightText: 2023-2024 Functori +// SPDX-FileCopyrightText: 2023-2025 Functori // SPDX-FileCopyrightText: 2023-2024 PK Lab // // SPDX-License-Identifier: MIT @@ -41,6 +41,7 @@ use evm::{ use primitive_types::{H160, H256, U256}; use sha3::{Digest, Keccak256}; use std::cmp::min; +use std::collections::HashMap; use std::fmt::Debug; use tezos_ethereum::block::BlockConstants; use tezos_evm_logging::{log, Level::*}; @@ -291,6 +292,27 @@ mod benchmarks { } } +#[derive(Eq, Hash, PartialEq, Clone, Copy, Debug)] +pub struct StorageKey { + pub address: H160, + pub index: H256, +} + +/// The layer cache is associating a storage slot which is an +/// address and an index (StorageKey) to a value (H256). +pub type LayerCache = HashMap; + +/// The storage cache is associating at each layer (usize) its +/// own cache (LayerCache). For each slot that is modified or +/// read during a call it will be added to the cache in its own +/// layer. +// NB: The cache is implicitly bounded in memory thanks to the +// gas limit, because at most we can do 300_000 different SSTORE +// which costs 100 gas (300_000 × 100 = 30M gas). +// In memory it means we take at most: +// 300_000 × 32B = 9_600_000B = 9.6MB +pub type StorageCache = HashMap; + /// The implementation of the SputnikVM [Handler] trait pub struct EvmHandler<'a, Host: Runtime> { /// The host @@ -320,14 +342,14 @@ pub struct EvmHandler<'a, Host: Runtime> { pub enable_warm_cold_access: bool, /// Tracer configuration for debugging. tracer: Option, - /// State of the storage during a given execution. - /// NB: For now, only used for tracing. - /// - /// TODO: https://gitlab.com/tezos/tezos/-/issues/6879 - /// With a better representation (map) this could easily be - /// used for caching and optimise the VM's execution in terms - /// of speed. - storage_state: Vec, + /// Storage cache during a given execution. + /// NB: See `StorageCache`'s documentation for more information. + storage_cache: StorageCache, + /// The original storage cache has its own cache because its unrelated + /// to the VM, its used by an internal mechanism of Sputnik to quickly + /// access storage slots before any transaction happens. + /// See: `fn original_storage`. + original_storage_cache: LayerCache, /// Reentrancy guard prevents circular calls to impure precompiles reentrancy_guard: ReentrancyGuard, } @@ -360,7 +382,8 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { effective_gas_price, enable_warm_cold_access, tracer, - storage_state: vec![], + storage_cache: HashMap::with_capacity(10), + original_storage_cache: HashMap::with_capacity(10), reentrancy_guard: ReentrancyGuard::new(vec![ WITHDRAWAL_ADDRESS, FA_BRIDGE_PRECOMPILE_ADDRESS, @@ -703,7 +726,7 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { /// Prepare trace info if needed fn prepare_trace( - &self, + &mut self, runtime: &evm::Runtime, opcode: &Option, ) -> Option { @@ -732,7 +755,21 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { let memory = config .enable_memory .then(|| runtime.machine().memory().data()[..].to_vec()); - let storage = (!config.disable_storage).then(|| self.storage_state.clone()); + let storage = (!config.disable_storage).then(|| { + let mut flat_storage = vec![]; + self.storage_cache + .clone() + .into_iter() + .flat_map(|(_, layer_cache)| layer_cache) + .for_each(|(StorageKey { address, index }, value)| { + flat_storage.push(StorageMapItem { + address, + index, + value, + }); + }); + flat_storage + }); Some(StructLog::prepare( pc, @@ -1554,6 +1591,8 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { } } + commit_storage_cache(self, number_of_tx_layer); + self.evm_account_storage .commit_transaction(self.host) .map_err(EthereumError::from)?; @@ -1613,6 +1652,8 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { .map_err(EthereumError::from)?; let _ = self.transaction_data.pop(); + self.storage_cache.clear(); + self.original_storage_cache.clear(); Ok(ExecutionOutcome { gas_used, @@ -1623,6 +1664,32 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { }) } + fn flush_storage_cache(&mut self) -> Result<(), EthereumError> { + let storage_cache_size = self.storage_cache.len(); + if storage_cache_size > 1 { + log!( + self.host, + Fatal, + "The storage cache size is {storage_cache_size} when \ + flushing. It is inconsistent with the transaction stack. \ + The EVM is most likely broken." + ); + return Err(EthereumError::InconsistentTransactionStack( + storage_cache_size, + false, + false, + )); + } + + if let Some(cache) = self.storage_cache.get(&0) { + for (StorageKey { address, index }, value) in cache.iter() { + let mut account = self.get_or_create_account(*address)?; + account.set_storage(self.host, index, value)?; + } + } + Ok(()) + } + /// End the initial transaction with either a commit or a rollback. The /// outcome depends on the execution result given. pub(crate) fn end_initial_transaction( @@ -1638,11 +1705,19 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { r ); - self.commit_initial_transaction(if let Some(new_address) = new_address { - ExecutionResult::ContractDeployed(new_address, result) - } else { - ExecutionResult::CallSucceeded(r, result) - }) + let commit_result = self.commit_initial_transaction( + if let Some(new_address) = new_address { + ExecutionResult::ContractDeployed(new_address, result) + } else { + ExecutionResult::CallSucceeded(r, result) + }, + ); + + // We flush the storage's cache into the durable storage + // by actually writing in it. + self.flush_storage_cache()?; + + commit_result } Ok((ExitReason::Revert(ExitRevert::Reverted), _, result)) => { self.rollback_initial_transaction(ExecutionResult::CallReverted(result)) @@ -1758,6 +1833,8 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { let gas_remaining = self.gas_remaining(); + commit_storage_cache(self, number_of_tx_layer); + self.evm_account_storage .commit_transaction(self.host) .map_err(EthereumError::from)?; @@ -1826,6 +1903,8 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { let _ = self.transaction_data.pop(); } + self.storage_cache.remove(&number_of_tx_layer); + self.evm_account_storage .rollback_transaction(self.host) .map_err(EthereumError::from) @@ -2033,6 +2112,79 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { } } +fn update_cache( + handler: &mut EvmHandler<'_, Host>, + address: H160, + index: H256, + value: H256, + layer: usize, +) { + if let Some(cache) = handler.storage_cache.get_mut(&layer) { + cache.insert(StorageKey { address, index }, value); + } else { + let mut cache = HashMap::new(); + cache.insert(StorageKey { address, index }, value); + handler.storage_cache.insert(layer, cache); + } +} + +fn find_storage_key( + handler: &mut EvmHandler<'_, Host>, + address: H160, + index: H256, + current_layer: usize, +) -> Option { + for layer in (0..=current_layer).rev() { + if let Some(cache) = handler.storage_cache.get(&layer) { + if let Some(value) = cache.get(&StorageKey { address, index }) { + return Some(*value); + } + } + } + None +} + +/// Committing the storage cache means that the current layer changes +/// are propagated to the previous one and the current layer is popped. +fn commit_storage_cache( + handler: &mut EvmHandler<'_, Host>, + current_layer: usize, +) { + let commit_layer = current_layer - 1; + if let Some(cache) = handler.storage_cache.remove(¤t_layer) { + if let Some(prev_layer_cache) = handler.storage_cache.get_mut(&commit_layer) { + prev_layer_cache.extend(cache); + } else { + handler.storage_cache.insert(commit_layer, cache); + } + } +} + +fn cached_storage_access( + handler: &mut EvmHandler<'_, Host>, + address: H160, + index: H256, + layer: usize, +) -> H256 { + if let Some(value) = find_storage_key(handler, address, index, layer) { + value + } else { + let value = handler + .get_account(address) + .ok() + .flatten() + .and_then(|a| a.get_storage(handler.host, &index).ok()); + + // This condition will help avoiding unecessary write access + // in the durable storage at the end of the transaction. + if let Some(value) = value { + update_cache(handler, address, index, value, layer); + } + + value.unwrap_or_default() + } +} + #[allow(unused_variables)] impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { type CreateInterrupt = Infallible; @@ -2079,19 +2231,26 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { } fn storage(&mut self, address: H160, index: H256) -> H256 { - self.get_account(address) - .ok() - .flatten() - .and_then(|a| a.get_storage(self.host, &index).ok()) - .unwrap_or_default() + let layer = self.evm_account_storage.stack_depth(); + cached_storage_access(self, address, index, layer) } fn original_storage(&mut self, address: H160, index: H256) -> H256 { - self.get_original_account(address) - .ok() - .flatten() - .and_then(|a| a.get_storage(self.host, &index).ok()) - .unwrap_or_default() + let key = StorageKey { address, index }; + if let Some(value) = self.original_storage_cache.get(&key) { + *value + } else { + let value = self + .get_original_account(address) + .ok() + .flatten() + .and_then(|a| a.get_storage(self.host, &index).ok()) + .unwrap_or_default(); + + self.original_storage_cache.insert(key, value); + + value + } } fn gas_left(&self) -> U256 { @@ -2204,24 +2363,9 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { index: H256, value: H256, ) -> Result<(), ExitError> { - let mut account = self.get_or_create_account(address).map_err(|_| { - ExitError::Other(Cow::from("Could not get account for set_storage")) - })?; - - let storage_result = account - .set_storage(self.host, &index, &value) - .map_err(|_| ExitError::Other(Cow::from("Could not set_storage in handler"))); - - // Tracing - if self.tracer.is_some() { - self.storage_state.push(StorageMapItem { - address, - index, - value, - }); - } - - storage_result + let layer = self.evm_account_storage.stack_depth(); + update_cache(self, address, index, value, layer); + Ok(()) } fn log( -- GitLab From 72fd913cc1f59dd128be23392bf64bcc745138f1 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Tue, 10 Dec 2024 11:55:28 +0100 Subject: [PATCH 4/9] EVM/Tests: readapt test helper so that it also rely on the cache --- etherlink/kernel_evm/evm_execution/src/handler.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/etherlink/kernel_evm/evm_execution/src/handler.rs b/etherlink/kernel_evm/evm_execution/src/handler.rs index bc51c4d88bd9..420770070505 100644 --- a/etherlink/kernel_evm/evm_execution/src/handler.rs +++ b/etherlink/kernel_evm/evm_execution/src/handler.rs @@ -2850,8 +2850,8 @@ mod test { address: &H160, index: &H256, ) -> H256 { - let account = handler.get_or_create_account(*address).unwrap(); - account.get_storage(handler.borrow_host(), index).unwrap() + let layer = handler.evm_account_storage.stack_depth(); + cached_storage_access(handler, *address, *index, layer) } fn dummy_first_block() -> BlockConstants { -- GitLab From 49d35742c2552ab59a99b00c8da8c64cd5fab17f Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Fri, 17 Jan 2025 15:03:51 +0100 Subject: [PATCH 5/9] Etherlink/Tezt: fix error output from eth_cli Co-authored-by: Valentin Chaboche --- etherlink/tezt/lib/eth_cli.ml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/etherlink/tezt/lib/eth_cli.ml b/etherlink/tezt/lib/eth_cli.ml index de5ccc18fe56..503e0456994b 100644 --- a/etherlink/tezt/lib/eth_cli.ml +++ b/etherlink/tezt/lib/eth_cli.ml @@ -33,7 +33,9 @@ let spawn ?runner command = let spawn_command_and_read_string ?runner ?expect_failure command = let process = spawn ?runner command in - Process.check_and_read_stdout ?expect_failure process + if expect_failure = Some true then + Process.check_and_read_stderr ?expect_failure process + else Process.check_and_read_stdout ?expect_failure process let spawn_command ?runner ?expect_failure command = let process = spawn ?runner command in -- GitLab From a52b5f2d6fe6354a01542674746dbbeb87ff495c Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Fri, 17 Jan 2025 12:40:47 +0100 Subject: [PATCH 6/9] Etherlink/Tezt: make sure that we OOG when filling max slots --- .../solidity_examples/slot_filler.sol | 22 ++++++++ etherlink/tezt/lib/solidity_contracts.ml | 7 +++ etherlink/tezt/tests/evm_sequencer.ml | 50 ++++++++++++++++++- 3 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 etherlink/kernel_evm/solidity_examples/slot_filler.sol diff --git a/etherlink/kernel_evm/solidity_examples/slot_filler.sol b/etherlink/kernel_evm/solidity_examples/slot_filler.sol new file mode 100644 index 000000000000..aa7e21d6727c --- /dev/null +++ b/etherlink/kernel_evm/solidity_examples/slot_filler.sol @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: 2025 Functori +// +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.0; + +contract SlotFiller { + uint256 public counter; + + // Mapping to fill storage slots dynamically + mapping(uint256 => uint256) public storageMap; + + // Function that keeps filling storage slots until it runs out of gas + function fillSlots() external { + while (true) { + storageMap[counter] = counter; + unchecked { + counter++; + } + } + } +} diff --git a/etherlink/tezt/lib/solidity_contracts.ml b/etherlink/tezt/lib/solidity_contracts.ml index b11d15106b0f..911a060f545e 100644 --- a/etherlink/tezt/lib/solidity_contracts.ml +++ b/etherlink/tezt/lib/solidity_contracts.ml @@ -405,3 +405,10 @@ let reentrancy_test () = ~label:"reentrancy_tester" ~contract:"ReentrancyTester" ~evm_version:"shanghai" + +let slot_filler () = + compile_contract + ~source:(solidity_contracts_path ^ "/slot_filler.sol") + ~label:"slot_filler" + ~contract:"SlotFiller" + ~evm_version:"shanghai" diff --git a/etherlink/tezt/tests/evm_sequencer.ml b/etherlink/tezt/tests/evm_sequencer.ml index 81ba7c62fdde..2e11cd01be09 100644 --- a/etherlink/tezt/tests/evm_sequencer.ml +++ b/etherlink/tezt/tests/evm_sequencer.ml @@ -3,7 +3,7 @@ (* SPDX-License-Identifier: MIT *) (* Copyright (c) 2023 Nomadic Labs *) (* Copyright (c) 2024 Trilitech *) -(* Copyright (c) 2024 Functori *) +(* Copyright (c) 2024-2025 Functori *) (* *) (*****************************************************************************) @@ -9593,6 +9593,51 @@ let test_init_config_mainnet network = Regression.capture config ; unit +let test_filling_max_slots_cant_lead_to_out_of_memory = + register_all + ~kernels:[Latest] + ~tags:["evm"; "slots"; "out_of_memory"] + ~title: + "Calling a contract that fills the maximum amount of slots leads to \ + OutOfGas not an out of memory error" + ~da_fee:Wei.zero + ~time_between_blocks:Nothing + @@ fun {sequencer; _} _protocol -> + let endpoint = Evm_node.endpoint sequencer in + let sender = Eth_account.bootstrap_accounts.(0) in + let* slot_filler = Solidity_contracts.slot_filler () in + let* () = Eth_cli.add_abi ~label:slot_filler.label ~abi:slot_filler.abi () in + let* contract_address, _ = + send_transaction_to_sequencer + (Eth_cli.deploy + ~source_private_key:sender.Eth_account.private_key + ~endpoint + ~abi:slot_filler.label + ~bin:slot_filler.bin) + sequencer + in + let* _ = produce_block sequencer in + let* failure = + Eth_cli.contract_send + ~source_private_key:sender.private_key + ~endpoint + ~abi_label:slot_filler.label + ~expect_failure:true + ~address:contract_address + ~method_call:"fillSlots()" + () + in + let contains s sub = + let re = Str.regexp_string sub in + try + ignore (Str.search_forward re s 0) ; + true + with Not_found -> false + in + let error_out_of_gas = "Error(OutOfGas)" in + if contains failure error_out_of_gas then unit + else Test.fail "Test should fail with error: %s" error_out_of_gas + let protocols = Protocol.all let () = @@ -9723,4 +9768,5 @@ let () = test_websocket_logs_event [Protocol.Alpha] ; test_node_correctly_uses_batcher_heap [Protocol.Alpha] ; test_init_config_mainnet "mainnet" ; - test_init_config_mainnet "testnet" + test_init_config_mainnet "testnet" ; + test_filling_max_slots_cant_lead_to_out_of_memory protocols -- GitLab From 3f4a97a3ff7b8ec205f7e38b0c586292a3ae6a87 Mon Sep 17 00:00:00 2001 From: Rodi-Can Bozman Date: Tue, 3 Dec 2024 15:40:02 +0100 Subject: [PATCH 7/9] Etherlink/Logs: add an entry in kernel features --- etherlink/CHANGES_KERNEL.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/etherlink/CHANGES_KERNEL.md b/etherlink/CHANGES_KERNEL.md index c9153694db81..78fbdee90295 100644 --- a/etherlink/CHANGES_KERNEL.md +++ b/etherlink/CHANGES_KERNEL.md @@ -10,6 +10,8 @@ with previous version. This should sensibly reduces the disk consumption of full Rollup Node. The full history will remain available via Octez EVM Node deployments. (!15490) +- The VM is now relying on cache to make r/w access to storage indexes + which improves time performances on the execution side. (!15812) ### Internal -- GitLab From cf1399da231560ab6838dc38c327fe2dbe128493 Mon Sep 17 00:00:00 2001 From: Thomas Letan Date: Wed, 22 Jan 2025 22:08:56 +0100 Subject: [PATCH 8/9] EVM Kernel: Do not create indexes on storage miss --- .../evm_execution/src/account_storage.rs | 30 ++++++++++++++++++- .../kernel_evm/evm_execution/src/handler.rs | 8 ++--- etherlink/kernel_evm/storage/src/lib.rs | 21 ++++++++++--- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/etherlink/kernel_evm/evm_execution/src/account_storage.rs b/etherlink/kernel_evm/evm_execution/src/account_storage.rs index 77946172a3a7..2ddd7871568e 100644 --- a/etherlink/kernel_evm/evm_execution/src/account_storage.rs +++ b/etherlink/kernel_evm/evm_execution/src/account_storage.rs @@ -15,7 +15,7 @@ use tezos_smart_rollup_storage::storage::Storage; use tezos_storage::{ error::Error as GenStorageError, read_u256_le_default, read_u64_le_default, }; -use tezos_storage::{path_from_h256, read_h256_be_default, WORD_SIZE}; +use tezos_storage::{path_from_h256, read_h256_be_default, read_h256_be_opt, WORD_SIZE}; use thiserror::Error; use crate::{code_storage, DurableStorageError}; @@ -187,6 +187,20 @@ pub fn account_path(address: &H160) -> Result { OwnedPath::try_from(path_string).map_err(DurableStorageError::from) } +pub enum StorageValue { + Hit(H256), + Default, +} + +impl StorageValue { + pub fn h256(self) -> H256 { + match self { + StorageValue::Hit(v) => v, + StorageValue::Default => STORAGE_DEFAULT_VALUE, + } + } +} + impl EthereumAccount { pub fn from_address(address: &H160) -> Result { let path = concat(&EVM_ACCOUNTS_PATH, &account_path(address)?)?; @@ -359,6 +373,20 @@ impl EthereumAccount { .map_err(AccountStorageError::from) } + pub fn read_storage( + &self, + host: &impl Runtime, + index: &H256, + ) -> Result { + let path = self.storage_path(index)?; + let res = read_h256_be_opt(host, &path).map_err(AccountStorageError::from)?; + + match res { + Some(v) => Ok(StorageValue::Hit(v)), + None => Ok(StorageValue::Default), + } + } + /// Set the value associated with an index in durable storage. The result depends on the /// values being stored. It tracks whether the update went from default to non-default, /// non-default to default, et.c. This is for the purpose of calculating gas cost. diff --git a/etherlink/kernel_evm/evm_execution/src/handler.rs b/etherlink/kernel_evm/evm_execution/src/handler.rs index 420770070505..dec8d0f87db8 100644 --- a/etherlink/kernel_evm/evm_execution/src/handler.rs +++ b/etherlink/kernel_evm/evm_execution/src/handler.rs @@ -12,7 +12,7 @@ use crate::access_record::AccessRecord; use crate::account_storage::{ account_path, AccountStorageError, EthereumAccount, EthereumAccountStorage, - CODE_HASH_DEFAULT, + StorageValue, CODE_HASH_DEFAULT, }; use crate::precompiles::reentrancy_guard::ReentrancyGuard; use crate::precompiles::{FA_BRIDGE_PRECOMPILE_ADDRESS, WITHDRAWAL_ADDRESS}; @@ -2173,15 +2173,15 @@ fn cached_storage_access( .get_account(address) .ok() .flatten() - .and_then(|a| a.get_storage(handler.host, &index).ok()); + .and_then(|a| a.read_storage(handler.host, &index).ok()); // This condition will help avoiding unecessary write access // in the durable storage at the end of the transaction. - if let Some(value) = value { + if let Some(StorageValue::Hit(value)) = value { update_cache(handler, address, index, value, layer); } - value.unwrap_or_default() + value.map(|x| x.h256()).unwrap_or_default() } } diff --git a/etherlink/kernel_evm/storage/src/lib.rs b/etherlink/kernel_evm/storage/src/lib.rs index ef7e793cd879..a4834e10f972 100644 --- a/etherlink/kernel_evm/storage/src/lib.rs +++ b/etherlink/kernel_evm/storage/src/lib.rs @@ -58,6 +58,20 @@ pub fn read_h256_be(host: &impl Runtime, path: &impl Path) -> anyhow::Result Result, Error> { + match host.store_read_all(path) { + Ok(bytes) if bytes.len() == WORD_SIZE => Ok(Some(H256::from_slice(&bytes))), + Ok(_) | Err(RuntimeError::PathNotFound) => Ok(None), + Err(err) => Err(err.into()), + } +} + /// Return a 32 bytes hash from storage at the given `path`. /// If the path is not found, `default` is returned. /// @@ -67,10 +81,9 @@ pub fn read_h256_be_default( path: &impl Path, default: H256, ) -> Result { - match host.store_read_all(path) { - Ok(bytes) if bytes.len() == WORD_SIZE => Ok(H256::from_slice(&bytes)), - Ok(_) | Err(RuntimeError::PathNotFound) => Ok(default), - Err(err) => Err(err.into()), + match read_h256_be_opt(host, path)? { + Some(v) => Ok(v), + None => Ok(default), } } -- GitLab From ac969283eec7691490a8e8c9dfc8548a9bf918c0 Mon Sep 17 00:00:00 2001 From: Thomas Letan Date: Wed, 22 Jan 2025 22:09:55 +0100 Subject: [PATCH 9/9] EVM Kernel: Cache storage miss --- .../evm_execution/src/account_storage.rs | 1 + .../kernel_evm/evm_execution/src/handler.rs | 25 +++++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/etherlink/kernel_evm/evm_execution/src/account_storage.rs b/etherlink/kernel_evm/evm_execution/src/account_storage.rs index 2ddd7871568e..edd710855a0c 100644 --- a/etherlink/kernel_evm/evm_execution/src/account_storage.rs +++ b/etherlink/kernel_evm/evm_execution/src/account_storage.rs @@ -187,6 +187,7 @@ pub fn account_path(address: &H160) -> Result { OwnedPath::try_from(path_string).map_err(DurableStorageError::from) } +#[derive(Clone, Copy)] pub enum StorageValue { Hit(H256), Default, diff --git a/etherlink/kernel_evm/evm_execution/src/handler.rs b/etherlink/kernel_evm/evm_execution/src/handler.rs index dec8d0f87db8..18b6e930a8da 100644 --- a/etherlink/kernel_evm/evm_execution/src/handler.rs +++ b/etherlink/kernel_evm/evm_execution/src/handler.rs @@ -300,7 +300,7 @@ pub struct StorageKey { /// The layer cache is associating a storage slot which is an /// address and an index (StorageKey) to a value (H256). -pub type LayerCache = HashMap; +pub type LayerCache = HashMap; /// The storage cache is associating at each layer (usize) its /// own cache (LayerCache). For each slot that is modified or @@ -765,7 +765,7 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { flat_storage.push(StorageMapItem { address, index, - value, + value: value.h256(), }); }); flat_storage @@ -1683,8 +1683,10 @@ impl<'a, Host: Runtime> EvmHandler<'a, Host> { if let Some(cache) = self.storage_cache.get(&0) { for (StorageKey { address, index }, value) in cache.iter() { - let mut account = self.get_or_create_account(*address)?; - account.set_storage(self.host, index, value)?; + if let StorageValue::Hit(value) = value { + let mut account = self.get_or_create_account(*address)?; + account.set_storage(self.host, index, value)?; + } } } Ok(()) @@ -2116,7 +2118,7 @@ fn update_cache( handler: &mut EvmHandler<'_, Host>, address: H160, index: H256, - value: H256, + value: StorageValue, layer: usize, ) { if let Some(cache) = handler.storage_cache.get_mut(&layer) { @@ -2133,7 +2135,7 @@ fn find_storage_key( address: H160, index: H256, current_layer: usize, -) -> Option { +) -> Option { for layer in (0..=current_layer).rev() { if let Some(cache) = handler.storage_cache.get(&layer) { if let Some(value) = cache.get(&StorageKey { address, index }) { @@ -2167,7 +2169,7 @@ fn cached_storage_access( layer: usize, ) -> H256 { if let Some(value) = find_storage_key(handler, address, index, layer) { - value + value.h256() } else { let value = handler .get_account(address) @@ -2177,7 +2179,7 @@ fn cached_storage_access( // This condition will help avoiding unecessary write access // in the durable storage at the end of the transaction. - if let Some(StorageValue::Hit(value)) = value { + if let Some(value) = value { update_cache(handler, address, index, value, layer); } @@ -2238,7 +2240,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { fn original_storage(&mut self, address: H160, index: H256) -> H256 { let key = StorageKey { address, index }; if let Some(value) = self.original_storage_cache.get(&key) { - *value + value.h256() } else { let value = self .get_original_account(address) @@ -2247,7 +2249,8 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { .and_then(|a| a.get_storage(self.host, &index).ok()) .unwrap_or_default(); - self.original_storage_cache.insert(key, value); + self.original_storage_cache + .insert(key, StorageValue::Hit(value)); value } @@ -2364,7 +2367,7 @@ impl<'a, Host: Runtime> Handler for EvmHandler<'a, Host> { value: H256, ) -> Result<(), ExitError> { let layer = self.evm_account_storage.stack_depth(); - update_cache(self, address, index, value, layer); + update_cache(self, address, index, StorageValue::Hit(value), layer); Ok(()) } -- GitLab