From 8ca70f5cdb37a74721f07c43f3c10ae808266a0c Mon Sep 17 00:00:00 2001 From: JereSalo Date: Mon, 28 Apr 2025 15:59:14 -0300 Subject: [PATCH 01/14] squash changes from old branch --- cmd/ef_tests/state/report.rs | 16 +-- cmd/ef_tests/state/runner/levm_runner.rs | 14 +-- cmd/ef_tests/state/runner/mod.rs | 8 +- cmd/ef_tests/state/runner/revm_runner.rs | 15 +-- crates/common/types/account.rs | 38 ++++++ crates/l2/prover/bench/src/rpc/db.rs | 21 ++-- crates/vm/backends/levm/db.rs | 42 +++---- crates/vm/backends/levm/mod.rs | 43 +++---- .../vm/levm/bench/revm_comparison/src/lib.rs | 4 +- crates/vm/levm/src/account.rs | 110 ------------------ crates/vm/levm/src/call_frame.rs | 6 +- crates/vm/levm/src/db/cache.rs | 3 +- crates/vm/levm/src/db/gen_db.rs | 107 +++++++++-------- crates/vm/levm/src/db/mod.rs | 8 +- crates/vm/levm/src/gas_cost.rs | 13 ++- crates/vm/levm/src/hooks/default_hook.rs | 8 +- crates/vm/levm/src/hooks/l2_hook.rs | 6 +- crates/vm/levm/src/lib.rs | 2 - .../levm/src/opcode_handlers/environment.rs | 21 ++-- .../stack_memory_storage_flow.rs | 29 +++-- crates/vm/levm/src/opcode_handlers/system.rs | 28 ++--- crates/vm/levm/src/utils.rs | 43 +++---- crates/vm/levm/src/vm.rs | 10 +- 23 files changed, 245 insertions(+), 350 deletions(-) delete mode 100644 crates/vm/levm/src/account.rs diff --git a/cmd/ef_tests/state/report.rs b/cmd/ef_tests/state/report.rs index b9d6b92fdf..428c638fa3 100644 --- a/cmd/ef_tests/state/report.rs +++ b/cmd/ef_tests/state/report.rs @@ -1,10 +1,10 @@ use crate::runner::{EFTestRunnerError, InternalError}; use colored::Colorize; -use ethrex_common::{types::Fork, Address, H256}; -use ethrex_levm::{ - errors::{ExecutionReport, TxResult, VMError}, - Account, +use ethrex_common::{ + types::{Account, Fork}, + Address, H256, }; +use ethrex_levm::errors::{ExecutionReport, TxResult, VMError}; use ethrex_storage::{error::StoreError, AccountUpdate}; use itertools::Itertools; use revm::primitives::{EVMError, ExecutionResult as RevmExecutionResult}; @@ -664,14 +664,14 @@ impl fmt::Display for ComparisonReport { base_account.info.balance, new_info.balance )?; - if base_account.info.bytecode_hash() != new_info.code_hash { + if base_account.info.code_hash != new_info.code_hash { writeln!( f, "\t\t\t\t\tCode: {} -> {}", - if base_account.info.bytecode.is_empty() { + if base_account.code.is_empty() { "empty".to_string() } else { - hex::encode(&base_account.info.bytecode) + hex::encode(&base_account.code) }, account_update .code @@ -691,7 +691,7 @@ impl fmt::Display for ComparisonReport { writeln!( f, "\t\t\t\t\tStorage slot: {key:#x}: {} -> {}", - initial_value.original_value, value + initial_value, value )?; } } diff --git a/cmd/ef_tests/state/runner/levm_runner.rs b/cmd/ef_tests/state/runner/levm_runner.rs index 8a23b1ef73..3a0845e6f5 100644 --- a/cmd/ef_tests/state/runner/levm_runner.rs +++ b/cmd/ef_tests/state/runner/levm_runner.rs @@ -198,24 +198,24 @@ pub fn prepare_vm_for_tx<'a>( pub fn ensure_pre_state(evm: &VM, test: &EFTest) -> Result<(), EFTestRunnerError> { let world_state = &evm.db.store; for (address, pre_value) in &test.pre.0 { - let account = world_state.get_account_info(*address).map_err(|e| { + let account = world_state.get_account(*address).map_err(|e| { EFTestRunnerError::Internal(InternalError::Custom(format!( "Failed to get account info when ensuring pre state: {}", e ))) })?; ensure_pre_state_condition( - account.nonce == pre_value.nonce, + account.info.nonce == pre_value.nonce, format!( "Nonce mismatch for account {:#x}: expected {}, got {}", - address, pre_value.nonce, account.nonce + address, pre_value.nonce, account.info.nonce ), )?; ensure_pre_state_condition( - account.balance == pre_value.balance, + account.info.balance == pre_value.balance, format!( "Balance mismatch for account {:#x}: expected {}, got {}", - address, pre_value.balance, account.balance + address, pre_value.balance, account.info.balance ), )?; for (k, v) in &pre_value.storage { @@ -231,12 +231,12 @@ pub fn ensure_pre_state(evm: &VM, test: &EFTest) -> Result<(), EFTestRunnerError )?; } ensure_pre_state_condition( - keccak(account.bytecode.clone()) == keccak(pre_value.code.as_ref()), + account.info.code_hash == keccak(pre_value.code.as_ref()), format!( "Code hash mismatch for account {:#x}: expected {}, got {}", address, keccak(pre_value.code.as_ref()), - keccak(account.bytecode) + account.info.code_hash ), )?; } diff --git a/cmd/ef_tests/state/runner/mod.rs b/cmd/ef_tests/state/runner/mod.rs index c0c7e3f37c..c5fc367757 100644 --- a/cmd/ef_tests/state/runner/mod.rs +++ b/cmd/ef_tests/state/runner/mod.rs @@ -6,7 +6,7 @@ use crate::{ }; use clap::Parser; use colored::Colorize; -use ethrex_common::Address; +use ethrex_common::{types::Account, Address}; use ethrex_levm::errors::{ExecutionReport, VMError}; use ethrex_vm::SpecId; use serde::{Deserialize, Serialize}; @@ -24,11 +24,7 @@ pub enum EFTestRunnerError { #[error("Failed to ensure pre-state: {0}")] FailedToEnsurePreState(String), #[error("Failed to ensure post-state: {1}")] - FailedToEnsurePostState( - ExecutionReport, - String, - HashMap, - ), + FailedToEnsurePostState(ExecutionReport, String, HashMap), #[error("VM run mismatch: {0}")] VMExecutionMismatch(String), #[error("Exception does not match the expected: {0}")] diff --git a/cmd/ef_tests/state/runner/revm_runner.rs b/cmd/ef_tests/state/runner/revm_runner.rs index f66042c7e0..d5d518d420 100644 --- a/cmd/ef_tests/state/runner/revm_runner.rs +++ b/cmd/ef_tests/state/runner/revm_runner.rs @@ -6,13 +6,10 @@ use crate::{ }; use bytes::Bytes; use ethrex_common::{ - types::{Fork, TxKind}, + types::{Account, Fork, TxKind}, Address, H256, }; -use ethrex_levm::{ - errors::{ExecutionReport, TxResult}, - Account, StorageSlot, -}; +use ethrex_levm::errors::{ExecutionReport, TxResult}; use ethrex_storage::{error::StoreError, AccountUpdate}; use ethrex_vm::{ self, @@ -362,13 +359,7 @@ pub async fn compare_levm_revm_account_updates( let account_storage = pre_state_value .storage .iter() - .map(|(key, value)| { - let storage_slot = StorageSlot { - original_value: *value, - current_value: *value, - }; - (H256::from_slice(&key.to_big_endian()), storage_slot) - }) + .map(|(key, value)| (H256::from_slice(&key.to_big_endian()), *value)) .collect(); let account = Account::new( pre_state_value.balance, diff --git a/crates/common/types/account.rs b/crates/common/types/account.rs index 847056eedc..d814311128 100644 --- a/crates/common/types/account.rs +++ b/crates/common/types/account.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use bytes::Bytes; use ethereum_types::{H256, U256}; use ethrex_trie::Trie; +use keccak_hash::keccak; use serde::{Deserialize, Serialize}; use sha3::{Digest as _, Keccak256}; @@ -169,6 +170,43 @@ impl From<&GenesisAccount> for AccountState { } } +impl Account { + pub fn new(balance: U256, code: Bytes, nonce: u64, storage: HashMap) -> Self { + Self { + info: AccountInfo { + balance, + code_hash: keccak(code.as_ref()).0.into(), + nonce, + }, + code, + storage, + } + } + + pub fn has_nonce(&self) -> bool { + self.info.nonce != 0 + } + + pub fn has_code(&self) -> bool { + self.info.code_hash != *EMPTY_KECCACK_HASH + } + + pub fn has_code_or_nonce(&self) -> bool { + self.has_code() || self.has_nonce() + } + + pub fn is_empty(&self) -> bool { + self.info.balance.is_zero() + && self.info.nonce == 0 + && self.info.code_hash == *EMPTY_KECCACK_HASH + } + + pub fn set_code(&mut self, code: Bytes) { + self.code = code.clone(); + self.info.code_hash = keccak(code.as_ref()).0.into(); + } +} + #[cfg(test)] mod test { use std::str::FromStr; diff --git a/crates/l2/prover/bench/src/rpc/db.rs b/crates/l2/prover/bench/src/rpc/db.rs index 71ed98634d..9c7e94f6d5 100644 --- a/crates/l2/prover/bench/src/rpc/db.rs +++ b/crates/l2/prover/bench/src/rpc/db.rs @@ -1,4 +1,3 @@ -use std::cell::RefCell; use std::collections::HashMap; use crate::constants::{CANCUN_CONFIG, RPC_RATE_LIMIT}; @@ -9,8 +8,8 @@ use ethrex_common::{ types::{AccountInfo, AccountState, Block, Fork, TxKind}, Address, H256, U256, }; +use ethrex_levm::db::gen_db::GeneralizedDatabase; use ethrex_levm::db::Database as LevmDatabase; -use ethrex_levm::vm::GeneralizedDatabase; use ethrex_storage::{hash_address, hash_key}; use ethrex_trie::{Node, PathRLP, Trie}; use ethrex_vm::backends::levm::{CacheDB, LEVM}; @@ -445,10 +444,11 @@ impl LevmDatabase for RpcDB { Ok(None) // code is stored in account info } - fn get_account_info( + fn get_account( &self, address: Address, - ) -> std::result::Result { + ) -> std::result::Result + { let cache = self.cache.lock().unwrap(); let account = if let Some(account) = cache.get(&address).cloned() { account @@ -466,13 +466,14 @@ impl LevmDatabase for RpcDB { .. } = account { - Ok(ethrex_levm::AccountInfo { - bytecode: code.clone().unwrap_or_default(), - balance: account_state.balance, - nonce: account_state.nonce, - }) + Ok(ethrex_common::types::Account::new( + account_state.balance, + code.clone().unwrap_or_default(), + account_state.nonce, + HashMap::new(), + )) } else { - Ok(ethrex_levm::AccountInfo::default()) + Ok(ethrex_common::types::Account::default()) } } diff --git a/crates/vm/backends/levm/db.rs b/crates/vm/backends/levm/db.rs index 92bb518e8d..deca442f17 100644 --- a/crates/vm/backends/levm/db.rs +++ b/crates/vm/backends/levm/db.rs @@ -1,3 +1,4 @@ +use ethrex_common::types::Account; use ethrex_common::U256 as CoreU256; use ethrex_common::{Address as CoreAddress, H256 as CoreH256}; use ethrex_levm::constants::EMPTY_CODE_HASH; @@ -29,16 +30,13 @@ impl DatabaseLogger { } impl LevmDatabase for DatabaseLogger { - fn get_account_info( - &self, - address: CoreAddress, - ) -> Result { + fn get_account(&self, address: CoreAddress) -> Result { self.state_accessed .lock() .map_err(|_| DatabaseError::Custom("Could not lock mutex".to_string()))? .entry(address) .or_default(); - self.store.get_account_info(address) + self.store.get_account(address) } fn account_exists(&self, address: CoreAddress) -> bool { @@ -87,10 +85,7 @@ impl LevmDatabase for DatabaseLogger { } impl LevmDatabase for StoreWrapper { - fn get_account_info( - &self, - address: CoreAddress, - ) -> Result { + fn get_account(&self, address: CoreAddress) -> Result { let acc_info = self .store .get_account_info_by_hash(self.block_hash, address) @@ -103,11 +98,12 @@ impl LevmDatabase for StoreWrapper { .map_err(|e| DatabaseError::Custom(e.to_string()))? .unwrap_or_default(); - Ok(ethrex_levm::account::AccountInfo { - balance: acc_info.balance, - nonce: acc_info.nonce, - bytecode: acc_code, - }) + Ok(Account::new( + acc_info.balance, + acc_code, + acc_info.nonce, + HashMap::new(), + )) } fn account_exists(&self, address: CoreAddress) -> bool { @@ -151,12 +147,9 @@ impl LevmDatabase for StoreWrapper { } impl LevmDatabase for ExecutionDB { - fn get_account_info( - &self, - address: CoreAddress, - ) -> Result { + fn get_account(&self, address: CoreAddress) -> Result { let Some(acc_info) = self.accounts.get(&address) else { - return Ok(ethrex_levm::AccountInfo::default()); + return Ok(Account::default()); }; let acc_code = if acc_info.code_hash != EMPTY_CODE_HASH { @@ -170,11 +163,12 @@ impl LevmDatabase for ExecutionDB { &bytes::Bytes::new() }; - Ok(ethrex_levm::AccountInfo { - balance: acc_info.balance, - bytecode: acc_code.clone(), - nonce: acc_info.nonce, - }) + Ok(Account::new( + acc_info.balance, + acc_code.clone(), + acc_info.nonce, + HashMap::new(), + )) } fn account_exists(&self, address: CoreAddress) -> bool { diff --git a/crates/vm/backends/levm/mod.rs b/crates/vm/backends/levm/mod.rs index 79a48a09a3..00ef6fa443 100644 --- a/crates/vm/backends/levm/mod.rs +++ b/crates/vm/backends/levm/mod.rs @@ -11,9 +11,9 @@ use crate::{EvmError, ExecutionDB, ExecutionDBError, ExecutionResult, StoreWrapp use bytes::Bytes; use ethrex_common::{ types::{ - code_hash, requests::Requests, AccessList, AccountInfo, AuthorizationTuple, Block, - BlockHeader, EIP1559Transaction, EIP7702Transaction, Fork, GenericTransaction, Receipt, - Transaction, TxKind, Withdrawal, GWEI_TO_WEI, INITIAL_BASE_FEE, + requests::Requests, AccessList, AuthorizationTuple, Block, BlockHeader, EIP1559Transaction, + EIP7702Transaction, Fork, GenericTransaction, Receipt, Transaction, TxKind, Withdrawal, + GWEI_TO_WEI, INITIAL_BASE_FEE, }, Address, H256, U256, }; @@ -21,7 +21,7 @@ use ethrex_levm::db::gen_db::GeneralizedDatabase; use ethrex_levm::{ errors::{ExecutionReport, TxResult, VMError}, vm::{EVMConfig, Substate, VM}, - Account, Environment, + Environment, }; use ethrex_storage::error::StoreError; use ethrex_storage::{hash_address, hash_key, AccountUpdate, Store}; @@ -164,25 +164,24 @@ impl LEVM { ) -> Result, EvmError> { let mut account_updates: Vec = vec![]; for (address, new_state_account) in db.cache.drain() { - let initial_state_account = db.store.get_account_info(address)?; + let initial_state_account = db.store.get_account(address)?; let account_existed = db.store.account_exists(address); let mut acc_info_updated = false; let mut storage_updated = false; // 1. Account Info has been updated if balance, nonce or bytecode changed. - if initial_state_account.balance != new_state_account.info.balance { + if initial_state_account.info.balance != new_state_account.info.balance { acc_info_updated = true; } - if initial_state_account.nonce != new_state_account.info.nonce { + if initial_state_account.info.nonce != new_state_account.info.nonce { acc_info_updated = true; } - let new_state_code_hash = code_hash(&new_state_account.info.bytecode); - let code = if initial_state_account.bytecode_hash() != new_state_code_hash { + let code = if initial_state_account.info.code_hash != new_state_account.info.code_hash { acc_info_updated = true; - Some(new_state_account.info.bytecode.clone()) + Some(new_state_account.code.clone()) } else { None }; @@ -191,18 +190,14 @@ impl LEVM { let mut added_storage = HashMap::new(); for (key, storage_slot) in &new_state_account.storage { let storage_before_block = db.store.get_storage_slot(address, *key)?; - if storage_slot.current_value != storage_before_block { - added_storage.insert(*key, storage_slot.current_value); + if *storage_slot != storage_before_block { + added_storage.insert(*key, *storage_slot); storage_updated = true; } } let info = if acc_info_updated { - Some(AccountInfo { - code_hash: new_state_code_hash, - balance: new_state_account.info.balance, - nonce: new_state_account.info.nonce, - }) + Some(new_state_account.info.clone()) } else { None }; @@ -253,17 +248,9 @@ impl LEVM { { // We check if it was in block_cache, if not, we get it from DB. let mut account = db.cache.get(&address).cloned().unwrap_or({ - let info = db - .store - .get_account_info(address) - .map_err(|e| StoreError::Custom(e.to_string()))?; - - Account { - info, - // This is the added_storage for the withdrawal. - // If not involved in the TX, there won't be any updates in the storage - storage: HashMap::new(), - } + db.store + .get_account(address) + .map_err(|e| StoreError::Custom(e.to_string()))? }); account.info.balance += increment.into(); diff --git a/crates/vm/levm/bench/revm_comparison/src/lib.rs b/crates/vm/levm/bench/revm_comparison/src/lib.rs index 8e6ac95f74..14bc78cc6b 100644 --- a/crates/vm/levm/bench/revm_comparison/src/lib.rs +++ b/crates/vm/levm/bench/revm_comparison/src/lib.rs @@ -66,7 +66,7 @@ pub fn run_with_levm(program: &str, runs: u64, calldata: &str) { cache::insert_account( &mut db.cache, accounts[0].0, - ethrex_levm::Account::new( + Account::new( accounts[0].1.info.balance, accounts[0].1.code.clone(), accounts[0].1.info.nonce, @@ -76,7 +76,7 @@ pub fn run_with_levm(program: &str, runs: u64, calldata: &str) { cache::insert_account( &mut db.cache, accounts[1].0, - ethrex_levm::Account::new( + Account::new( accounts[1].1.info.balance, accounts[1].1.code.clone(), accounts[1].1.info.nonce, diff --git a/crates/vm/levm/src/account.rs b/crates/vm/levm/src/account.rs deleted file mode 100644 index 7cf497fecb..0000000000 --- a/crates/vm/levm/src/account.rs +++ /dev/null @@ -1,110 +0,0 @@ -use crate::constants::EMPTY_CODE_HASH; -use bytes::Bytes; -use ethrex_common::{H256, U256}; -use keccak_hash::keccak; -use serde::{Deserialize, Serialize}; -use std::collections::HashMap; - -#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct AccountInfo { - pub balance: U256, - pub bytecode: Bytes, - pub nonce: u64, -} - -impl AccountInfo { - pub fn is_empty(&self) -> bool { - self.balance.is_zero() && self.nonce == 0 && self.bytecode.is_empty() - } - - pub fn has_code(&self) -> bool { - !(self.bytecode.is_empty() || self.bytecode_hash() == EMPTY_CODE_HASH) - } - - pub fn bytecode_hash(&self) -> H256 { - keccak(self.bytecode.as_ref()).0.into() - } - - pub fn has_nonce(&self) -> bool { - self.nonce != 0 - } -} - -#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct Account { - pub info: AccountInfo, - pub storage: HashMap, -} - -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct StorageSlot { - pub original_value: U256, - pub current_value: U256, -} - -impl From for Account { - fn from(info: AccountInfo) -> Self { - Self { - info, - storage: HashMap::new(), - } - } -} - -impl Account { - pub fn new( - balance: U256, - bytecode: Bytes, - nonce: u64, - storage: HashMap, - ) -> Self { - Self { - info: AccountInfo { - balance, - bytecode, - nonce, - }, - storage, - } - } - - pub fn has_nonce(&self) -> bool { - self.info.has_nonce() - } - - pub fn has_code(&self) -> bool { - self.info.has_code() - } - - pub fn has_code_or_nonce(&self) -> bool { - self.has_code() || self.has_nonce() - } - - pub fn bytecode_hash(&self) -> H256 { - self.info.bytecode_hash() - } - - pub fn is_empty(&self) -> bool { - self.info.balance.is_zero() && self.info.nonce == 0 && self.info.bytecode.is_empty() - } - - pub fn with_balance(mut self, balance: U256) -> Self { - self.info.balance = balance; - self - } - - pub fn with_bytecode(mut self, bytecode: Bytes) -> Self { - self.info.bytecode = bytecode; - self - } - - pub fn with_storage(mut self, storage: HashMap) -> Self { - self.storage = storage; - self - } - - pub fn with_nonce(mut self, nonce: u64) -> Self { - self.info.nonce = nonce; - self - } -} diff --git a/crates/vm/levm/src/call_frame.rs b/crates/vm/levm/src/call_frame.rs index 14b082a1b0..c7c33dfdc6 100644 --- a/crates/vm/levm/src/call_frame.rs +++ b/crates/vm/levm/src/call_frame.rs @@ -4,10 +4,12 @@ use crate::{ memory::Memory, opcodes::Opcode, utils::get_valid_jump_destinations, - Account, }; use bytes::Bytes; -use ethrex_common::{types::Log, Address, U256}; +use ethrex_common::{ + types::{Account, Log}, + Address, U256, +}; use std::collections::{HashMap, HashSet}; #[derive(Debug, Clone, Default, PartialEq, Eq)] diff --git a/crates/vm/levm/src/db/cache.rs b/crates/vm/levm/src/db/cache.rs index 44b3c3a9d5..84c30b36a8 100644 --- a/crates/vm/levm/src/db/cache.rs +++ b/crates/vm/levm/src/db/cache.rs @@ -1,5 +1,4 @@ -use crate::Account; -use ethrex_common::Address; +use ethrex_common::{types::Account, Address}; use std::collections::HashMap; pub type CacheDB = HashMap; diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index eaa6b3cb80..e0a65c41b5 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -1,6 +1,8 @@ +use std::collections::HashMap; use std::sync::Arc; use bytes::Bytes; +use ethrex_common::types::Account; use ethrex_common::types::Fork; use ethrex_common::Address; use ethrex_common::U256; @@ -10,10 +12,6 @@ use crate::errors::InternalError; use crate::errors::VMError; use crate::vm::Substate; use crate::vm::VM; -use crate::Account; -use crate::AccountInfo; -use crate::StorageSlot; -use std::collections::HashMap; use super::cache; use super::error::DatabaseError; @@ -38,11 +36,7 @@ impl GeneralizedDatabase { match cache::get_account(&self.cache, &address) { Some(acc) => Ok(acc.clone()), None => { - let account_info = self.store.get_account_info(address)?; - let account = Account { - info: account_info, - storage: HashMap::new(), - }; + let account = self.store.get_account(address)?; cache::insert_account(&mut self.cache, address, account.clone()); Ok(account) } @@ -53,13 +47,7 @@ impl GeneralizedDatabase { pub fn get_account_no_push_cache(&self, address: Address) -> Result { match cache::get_account(&self.cache, &address) { Some(acc) => Ok(acc.clone()), - None => { - let account_info = self.store.get_account_info(address)?; - Ok(Account { - info: account_info, - storage: HashMap::new(), - }) - } + None => self.store.get_account(address), } } @@ -71,12 +59,10 @@ impl GeneralizedDatabase { &mut self, accrued_substate: &mut Substate, address: Address, - ) -> Result<(AccountInfo, bool), DatabaseError> { + ) -> Result<(Account, bool), DatabaseError> { let address_was_cold = accrued_substate.touched_accounts.insert(address); - let account = match cache::get_account(&self.cache, &address) { - Some(account) => account.info.clone(), - None => self.store.get_account_info(address)?, - }; + let account = self.get_account(address)?; + Ok((account, address_was_cold)) } } @@ -86,11 +72,7 @@ impl<'a> VM<'a> { pub fn get_account_mut(&mut self, address: Address) -> Result<&mut Account, VMError> { if !cache::is_account_cached(&self.db.cache, &address) { - let account_info = self.db.store.get_account_info(address)?; - let account = Account { - info: account_info, - storage: HashMap::new(), - }; + let account = self.db.store.get_account(address)?; cache::insert_account(&mut self.db.cache, address, account.clone()); } @@ -146,7 +128,7 @@ impl<'a> VM<'a> { new_bytecode: Bytes, ) -> Result<(), VMError> { let account = self.get_account_mut(address)?; - account.info.bytecode = new_bytecode; + account.set_code(new_bytecode); Ok(()) } @@ -189,6 +171,30 @@ impl<'a> VM<'a> { Ok(()) } + /// Gets original storage value of an account, caching it if not already cached. + /// Also saves the original value for future gas calculations. + pub fn get_original_storage(&mut self, address: Address, key: H256) -> Result { + let value_pre_tx = match self.storage_original_values.get(&address).cloned() { + Some(account_storage) => match account_storage.get(&key) { + Some(value) => *value, + None => self.get_storage_slot(address, key)?, + }, + None => self.get_storage_slot(address, key)?, + }; + + // Add it to the original values if it wasn't already there + if let Some(account_storage) = self.storage_original_values.get_mut(&address) { + account_storage.entry(key).or_insert(value_pre_tx); + } else { + let mut account_storage = HashMap::new(); + account_storage.insert(key, value_pre_tx); + self.storage_original_values + .insert(address, account_storage); + } + + Ok(value_pre_tx) + } + /// Accesses to an account's storage slot. /// /// Accessed storage slots are stored in the `touched_storage_slots` set. @@ -197,7 +203,7 @@ impl<'a> VM<'a> { &mut self, address: Address, key: H256, - ) -> Result<(StorageSlot, bool), VMError> { + ) -> Result<(U256, bool), VMError> { // [EIP-2929] - Introduced conditional tracking of accessed storage slots for Berlin and later specs. let mut storage_slot_was_cold = false; if self.env.config.fork >= Fork::Berlin { @@ -208,34 +214,31 @@ impl<'a> VM<'a> { .or_default() .insert(key); } + + let storage_slot = self.get_storage_slot(address, key)?; + + Ok((storage_slot, storage_slot_was_cold)) + } + + /// Gets storage slot of an account, caching it if not already cached. + pub fn get_storage_slot(&mut self, address: Address, key: H256) -> Result { let storage_slot = match cache::get_account(&self.db.cache, &address) { Some(account) => match account.storage.get(&key) { - Some(storage_slot) => storage_slot.clone(), - None => { - let value = self.db.store.get_storage_slot(address, key)?; - StorageSlot { - original_value: value, - current_value: value, - } - } + Some(storage_slot) => *storage_slot, + None => self.db.store.get_storage_slot(address, key)?, }, - None => { - let value = self.db.store.get_storage_slot(address, key)?; - StorageSlot { - original_value: value, - current_value: value, - } - } + None => self.db.store.get_storage_slot(address, key)?, }; - // When updating account storage of an account that's not yet cached we need to store the StorageSlot in the account + // When getting storage slot of an account that's not yet cached we need to store it in the account // Note: We end up caching the account because it is the most straightforward way of doing it. let account = self.get_account_mut(address)?; - account.storage.insert(key, storage_slot.clone()); + account.storage.entry(key).or_insert(storage_slot); - Ok((storage_slot, storage_slot_was_cold)) + Ok(storage_slot) } + //TODO: Can be more performant with .entry and .and_modify, but didn't want to add complexity. pub fn update_account_storage( &mut self, address: Address, @@ -243,15 +246,9 @@ impl<'a> VM<'a> { new_value: U256, ) -> Result<(), VMError> { let account = self.get_account_mut(address)?; - let account_original_storage_slot_value = account - .storage - .get(&key) - .map_or(U256::zero(), |slot| slot.original_value); - let slot = account.storage.entry(key).or_insert(StorageSlot { - original_value: account_original_storage_slot_value, - current_value: new_value, - }); - slot.current_value = new_value; + if account.storage.get(&key) != Some(&new_value) { + account.storage.insert(key, new_value); + } Ok(()) } } diff --git a/crates/vm/levm/src/db/mod.rs b/crates/vm/levm/src/db/mod.rs index 4833875f58..717502455e 100644 --- a/crates/vm/levm/src/db/mod.rs +++ b/crates/vm/levm/src/db/mod.rs @@ -1,7 +1,9 @@ -use crate::account::AccountInfo; use bytes::Bytes; use error::DatabaseError; -use ethrex_common::{types::ChainConfig, Address, H256, U256}; +use ethrex_common::{ + types::{Account, ChainConfig}, + Address, H256, U256, +}; pub mod cache; pub use cache::CacheDB; @@ -9,7 +11,7 @@ pub mod error; pub mod gen_db; pub trait Database: Send + Sync { - fn get_account_info(&self, address: Address) -> Result; + fn get_account(&self, address: Address) -> Result; fn get_storage_slot(&self, address: Address, key: H256) -> Result; fn get_block_hash(&self, block_number: u64) -> Result, DatabaseError>; fn account_exists(&self, address: Address) -> bool; diff --git a/crates/vm/levm/src/gas_cost.rs b/crates/vm/levm/src/gas_cost.rs index a90f39fd5e..1572c5eda3 100644 --- a/crates/vm/levm/src/gas_cost.rs +++ b/crates/vm/levm/src/gas_cost.rs @@ -2,7 +2,7 @@ use crate::{ call_frame::CallFrame, constants::{WORD_SIZE, WORD_SIZE_IN_BYTES_U64}, errors::{InternalError, OutOfGasError, PrecompileError, VMError}, - memory, StorageSlot, + memory, }; use bytes::Bytes; /// Contains the gas costs of the EVM instructions @@ -454,7 +454,8 @@ pub fn sload(storage_slot_was_cold: bool, fork: Fork) -> Result { } pub fn sstore( - storage_slot: &StorageSlot, + original_value: U256, + current_value: U256, new_value: U256, storage_slot_was_cold: bool, fork: Fork, @@ -463,7 +464,7 @@ pub fn sstore( // Petersburg removes EIP-1283 // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1716.md if fork <= Fork::Byzantium || fork == Fork::Petersburg { - if storage_slot.current_value.is_zero() && !new_value.is_zero() { + if current_value.is_zero() && !new_value.is_zero() { Ok(SSTORE_PRE_BERLIN_NON_ZERO) } else { Ok(SSTORE_PRE_BERLIN) @@ -489,10 +490,10 @@ pub fn sstore( SSTORE_STORAGE_MODIFICATION }; - let mut base_dynamic_gas = if new_value == storage_slot.current_value { + let mut base_dynamic_gas = if new_value == current_value { default_dynamic - } else if storage_slot.current_value == storage_slot.original_value { - if storage_slot.original_value.is_zero() { + } else if current_value == original_value { + if original_value.is_zero() { SSTORE_STORAGE_CREATION } else { dynamic_gas_modification diff --git a/crates/vm/levm/src/hooks/default_hook.rs b/crates/vm/levm/src/hooks/default_hook.rs index 6480d57ee0..014973a261 100644 --- a/crates/vm/levm/src/hooks/default_hook.rs +++ b/crates/vm/levm/src/hooks/default_hook.rs @@ -1,5 +1,4 @@ use crate::{ - account::Account, constants::*, errors::{ExecutionReport, InternalError, TxValidationError, VMError}, gas_cost::{self, STANDARD_TOKEN_COST, TOTAL_COST_FLOOR_PER_TOKEN}, @@ -8,7 +7,10 @@ use crate::{ vm::VM, }; -use ethrex_common::{types::Fork, U256}; +use ethrex_common::{ + types::{Account, Fork}, + U256, +}; use std::cmp::max; @@ -180,7 +182,7 @@ impl Hook for DefaultHook { } // (9) SENDER_NOT_EOA - if sender_account.has_code() && !has_delegation(&sender_account.info)? { + if sender_account.has_code() && !has_delegation(&sender_account)? { return Err(VMError::TxValidation(TxValidationError::SenderNotEOA)); } diff --git a/crates/vm/levm/src/hooks/l2_hook.rs b/crates/vm/levm/src/hooks/l2_hook.rs index 4bf417bc9e..24d5f93b80 100644 --- a/crates/vm/levm/src/hooks/l2_hook.rs +++ b/crates/vm/levm/src/hooks/l2_hook.rs @@ -1,13 +1,15 @@ use std::cmp::max; -use ethrex_common::{types::Fork, Address, U256}; +use ethrex_common::{ + types::{Account, Fork}, + Address, U256, +}; use crate::{ constants::{INIT_CODE_MAX_SIZE, TX_BASE_COST, VALID_BLOB_PREFIXES}, errors::{InternalError, TxValidationError, VMError}, gas_cost::{self, STANDARD_TOKEN_COST, TOTAL_COST_FLOOR_PER_TOKEN}, utils::{get_base_fee_per_blob_gas, get_valid_jump_destinations}, - Account, }; use super::{ diff --git a/crates/vm/levm/src/lib.rs b/crates/vm/levm/src/lib.rs index a90141598b..bf430bd6bb 100644 --- a/crates/vm/levm/src/lib.rs +++ b/crates/vm/levm/src/lib.rs @@ -1,4 +1,3 @@ -pub mod account; pub mod call_frame; pub mod constants; pub mod db; @@ -14,5 +13,4 @@ pub mod operations; pub mod precompiles; pub mod utils; pub mod vm; -pub use account::*; pub use environment::*; diff --git a/crates/vm/levm/src/opcode_handlers/environment.rs b/crates/vm/levm/src/opcode_handlers/environment.rs index fc471be863..204caf84db 100644 --- a/crates/vm/levm/src/opcode_handlers/environment.rs +++ b/crates/vm/levm/src/opcode_handlers/environment.rs @@ -6,7 +6,6 @@ use crate::{ vm::VM, }; use ethrex_common::{types::Fork, U256}; -use keccak_hash::keccak; // Environmental Information (16) // Opcodes: ADDRESS, BALANCE, ORIGIN, CALLER, CALLVALUE, CALLDATALOAD, CALLDATASIZE, CALLDATACOPY, CODESIZE, CODECOPY, GASPRICE, EXTCODESIZE, EXTCODECOPY, RETURNDATASIZE, RETURNDATACOPY, EXTCODEHASH @@ -31,7 +30,7 @@ impl<'a> VM<'a> { let fork = self.env.config.fork; let address = word_to_address(self.current_call_frame_mut()?.stack.pop()?); - let (account_info, address_was_cold) = self + let (account, address_was_cold) = self .db .access_account(&mut self.accrued_substate, address)?; @@ -39,7 +38,7 @@ impl<'a> VM<'a> { current_call_frame.increase_consumed_gas(gas_cost::balance(address_was_cold, fork)?)?; - current_call_frame.stack.push(account_info.balance)?; + current_call_frame.stack.push(account.info.balance)?; Ok(OpcodeResult::Continue { pc_increment: 1 }) } @@ -259,7 +258,7 @@ impl<'a> VM<'a> { let fork = self.env.config.fork; let address = word_to_address(self.current_call_frame_mut()?.stack.pop()?); - let (account_info, address_was_cold) = self + let (account, address_was_cold) = self .db .access_account(&mut self.accrued_substate, address)?; @@ -267,9 +266,7 @@ impl<'a> VM<'a> { current_call_frame.increase_consumed_gas(gas_cost::extcodesize(address_was_cold, fork)?)?; - current_call_frame - .stack - .push(account_info.bytecode.len().into())?; + current_call_frame.stack.push(account.code.len().into())?; Ok(OpcodeResult::Continue { pc_increment: 1 }) } @@ -288,7 +285,7 @@ impl<'a> VM<'a> { .map_err(|_| VMError::VeryLargeNumber)?; let current_memory_size = self.current_call_frame()?.memory.len(); - let (account_info, address_was_cold) = self + let (account, address_was_cold) = self .db .access_account(&mut self.accrued_substate, address)?; @@ -309,7 +306,7 @@ impl<'a> VM<'a> { // If the bytecode is a delegation designation, it will copy the marker (0xef0100) || address. // https://eips.ethereum.org/EIPS/eip-7702#delegation-designation - let bytecode = account_info.bytecode; + let bytecode = account.code; let mut data = vec![0u8; size]; if offset < bytecode.len().into() { @@ -414,7 +411,7 @@ impl<'a> VM<'a> { let fork = self.env.config.fork; let address = word_to_address(self.current_call_frame_mut()?.stack.pop()?); - let (account_info, address_was_cold) = self + let (account, address_was_cold) = self .db .access_account(&mut self.accrued_substate, address)?; @@ -423,12 +420,12 @@ impl<'a> VM<'a> { current_call_frame.increase_consumed_gas(gas_cost::extcodehash(address_was_cold, fork)?)?; // An account is considered empty when it has no code and zero nonce and zero balance. [EIP-161] - if account_info.is_empty() { + if account.is_empty() { current_call_frame.stack.push(U256::zero())?; return Ok(OpcodeResult::Continue { pc_increment: 1 }); } - let hash = U256::from_big_endian(keccak(account_info.bytecode).as_fixed_bytes()); + let hash = U256::from_big_endian(&account.info.code_hash.0); current_call_frame.stack.push(hash)?; Ok(OpcodeResult::Continue { pc_increment: 1 }) diff --git a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs index 4b255a5883..96c05394d5 100644 --- a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs +++ b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs @@ -153,7 +153,7 @@ impl<'a> VM<'a> { current_call_frame.increase_consumed_gas(gas_cost::sload(storage_slot_was_cold, fork)?)?; - current_call_frame.stack.push(storage_slot.current_value)?; + current_call_frame.stack.push(storage_slot)?; Ok(OpcodeResult::Continue { pc_increment: 1 }) } @@ -181,10 +181,10 @@ impl<'a> VM<'a> { return Err(VMError::OutOfGas(OutOfGasError::MaxGasLimitExceeded)); } - // Convert key from U256 to H256 + // Get current and original (pre-tx) values. let key = H256::from(storage_slot_key.to_big_endian()); - - let (storage_slot, storage_slot_was_cold) = self.access_storage_slot(to, key)?; + let (current_value, storage_slot_was_cold) = self.access_storage_slot(to, key)?; + let original_value = self.get_original_storage(to, key)?; // Gas Refunds // Sync gas refund with global env, ensuring consistency accross contexts. @@ -200,26 +200,24 @@ impl<'a> VM<'a> { }; if self.env.config.fork < Fork::Istanbul && self.env.config.fork != Fork::Constantinople { - if !storage_slot.current_value.is_zero() && new_storage_slot_value.is_zero() { + if !current_value.is_zero() && new_storage_slot_value.is_zero() { gas_refunds = gas_refunds .checked_add(15000) .ok_or(VMError::GasRefundsOverflow)?; } } else if (self.env.config.fork == Fork::Constantinople || self.env.config.fork >= Fork::Istanbul) - && new_storage_slot_value != storage_slot.current_value + && new_storage_slot_value != current_value { - if storage_slot.current_value == storage_slot.original_value { - if storage_slot.original_value != U256::zero() - && new_storage_slot_value == U256::zero() - { + if current_value == original_value { + if original_value != U256::zero() && new_storage_slot_value == U256::zero() { gas_refunds = gas_refunds .checked_add(remove_slot_cost) .ok_or(VMError::GasRefundsOverflow)?; } } else { - if storage_slot.original_value != U256::zero() { - if storage_slot.current_value == U256::zero() { + if original_value != U256::zero() { + if current_value == U256::zero() { gas_refunds = gas_refunds .checked_sub(remove_slot_cost) .ok_or(VMError::GasRefundsUnderflow)?; @@ -229,8 +227,8 @@ impl<'a> VM<'a> { .ok_or(VMError::GasRefundsUnderflow)?; } } - if new_storage_slot_value == storage_slot.original_value { - if storage_slot.original_value == U256::zero() { + if new_storage_slot_value == original_value { + if original_value == U256::zero() { gas_refunds = gas_refunds .checked_add(restore_empty_slot_cost) .ok_or(VMError::GasRefundsUnderflow)?; @@ -248,7 +246,8 @@ impl<'a> VM<'a> { self.current_call_frame_mut()? .increase_consumed_gas(gas_cost::sstore( - &storage_slot, + original_value, + current_value, new_storage_slot_value, storage_slot_was_cold, fork, diff --git a/crates/vm/levm/src/opcode_handlers/system.rs b/crates/vm/levm/src/opcode_handlers/system.rs index d16034f914..4ba88bf98e 100644 --- a/crates/vm/levm/src/opcode_handlers/system.rs +++ b/crates/vm/levm/src/opcode_handlers/system.rs @@ -8,10 +8,12 @@ use crate::{ precompiles::is_precompile, utils::{address_to_word, word_to_address, *}, vm::{RetData, StateBackup, VM}, - Account, }; use bytes::Bytes; -use ethrex_common::{types::Fork, Address, U256}; +use ethrex_common::{ + types::{Account, Fork}, + Address, U256, +}; // System Operations (10) // Opcodes: CREATE, CALL, CALLCODE, RETURN, DELEGATECALL, CREATE2, STATICCALL, REVERT, INVALID, SELFDESTRUCT @@ -579,16 +581,16 @@ impl<'a> VM<'a> { }; let fork = self.env.config.fork; - let (target_account_info, target_account_is_cold) = self + let (target_account, target_account_is_cold) = self .db .access_account(&mut self.accrued_substate, target_address)?; - let (current_account_info, _current_account_is_cold) = + let (current_account, _current_account_is_cold) = self.db.access_account(&mut self.accrued_substate, to)?; - let balance_to_transfer = current_account_info.balance; + let balance_to_transfer = current_account.info.balance; let account_is_empty = if self.env.config.fork >= Fork::SpuriousDragon { - target_account_info.is_empty() + target_account.is_empty() } else { !account_exists(self.db, target_address) }; @@ -630,7 +632,7 @@ impl<'a> VM<'a> { self.accrued_substate.selfdestruct_set.insert(to); } - if account_exists(self.db, target_address) && target_account_info.is_empty() { + if account_exists(self.db, target_address) && target_account.is_empty() { self.accrued_substate .touched_accounts .insert(target_address); @@ -671,7 +673,7 @@ impl<'a> VM<'a> { (deployer_address, max_message_call_gas) }; - let deployer_account_info = self + let deployer_account = self .db .access_account(&mut self.accrued_substate, deployer_address)? .0; @@ -687,7 +689,7 @@ impl<'a> VM<'a> { let new_address = match salt { Some(salt) => calculate_create2_address(deployer_address, &code, salt)?, - None => calculate_create_address(deployer_address, deployer_account_info.nonce)?, + None => calculate_create_address(deployer_address, deployer_account.info.nonce)?, }; // touch account @@ -703,9 +705,9 @@ impl<'a> VM<'a> { // 1. Sender doesn't have enough balance to send value. // 2. Depth limit has been reached // 3. Sender nonce is max. - if deployer_account_info.balance < value_in_wei_to_send + if deployer_account.info.balance < value_in_wei_to_send || new_depth > 1024 - || deployer_account_info.nonce == u64::MAX + || deployer_account.info.nonce == u64::MAX { // Return reserved gas current_call_frame.gas_used = current_call_frame @@ -808,7 +810,7 @@ impl<'a> VM<'a> { bytecode: Bytes, is_delegation: bool, ) -> Result { - let sender_account_info = self + let sender_account = self .db .access_account(&mut self.accrued_substate, msg_sender)? .0; @@ -823,7 +825,7 @@ impl<'a> VM<'a> { .to_vec(); // 1. Validate sender has enough value - if should_transfer_value && sender_account_info.balance < value { + if should_transfer_value && sender_account.info.balance < value { current_call_frame.gas_used = current_call_frame .gas_used .checked_sub(gas_limit) diff --git a/crates/vm/levm/src/utils.rs b/crates/vm/levm/src/utils.rs index 5b302f36b2..4835af78f4 100644 --- a/crates/vm/levm/src/utils.rs +++ b/crates/vm/levm/src/utils.rs @@ -12,9 +12,9 @@ use crate::{ }, opcodes::Opcode, vm::{EVMConfig, Substate, VM}, - AccountInfo, }; use bytes::Bytes; +use ethrex_common::types::Account; use ethrex_common::{ types::{tx_fields::*, Fork}, Address, H256, U256, @@ -211,15 +211,15 @@ pub fn word_to_address(word: U256) -> Address { /// Checks if account.info.bytecode has been delegated as the EIP7702 /// determines. -pub fn has_delegation(account_info: &AccountInfo) -> Result { +pub fn has_delegation(account: &Account) -> Result { let mut has_delegation = false; - if account_info.has_code() && account_info.bytecode.len() == EIP7702_DELEGATED_CODE_LEN { - let first_3_bytes = account_info - .bytecode + if account.has_code() && account.code.len() == EIP7702_DELEGATED_CODE_LEN { + let first_3_bytes = &account + .code .get(..3) .ok_or(VMError::Internal(InternalError::SlicingError))?; - if first_3_bytes == SET_CODE_DELEGATION_BYTES { + if *first_3_bytes == SET_CODE_DELEGATION_BYTES { has_delegation = true; } } @@ -228,10 +228,10 @@ pub fn has_delegation(account_info: &AccountInfo) -> Result { /// Gets the address inside the account.info.bytecode if it has been /// delegated as the EIP7702 determines. -pub fn get_authorized_address(account_info: &AccountInfo) -> Result { - if has_delegation(account_info)? { - let address_bytes = account_info - .bytecode +pub fn get_authorized_address(account: &Account) -> Result { + if has_delegation(account)? { + let address_bytes = &account + .code .get(SET_CODE_DELEGATION_BYTES.len()..) .ok_or(VMError::Internal(InternalError::SlicingError))?; // It shouldn't panic when doing Address::from_slice() @@ -337,19 +337,19 @@ pub fn eip7702_get_code( ) -> Result<(bool, u64, Address, Bytes), VMError> { // Address is the delgated address let account = db.get_account_no_push_cache(address)?; - let bytecode = account.info.bytecode.clone(); + let bytecode = account.code.clone(); // If the Address doesn't have a delegation code // return false meaning that is not a delegation // return the same address given // return the bytecode of the given address - if !has_delegation(&account.info)? { + if !has_delegation(&account)? { return Ok((false, 0, address, bytecode)); } // Here the address has a delegation code // The delegation code has the authorized address - let auth_address = get_authorized_address(&account.info)?; + let auth_address = get_authorized_address(&account)?; let access_cost = if accrued_substate.touched_accounts.contains(&auth_address) { WARM_ADDRESS_ACCESS_COST @@ -358,7 +358,7 @@ pub fn eip7702_get_code( COLD_ADDRESS_ACCESS_COST }; - let authorized_bytecode = db.get_account_no_push_cache(auth_address)?.info.bytecode; + let authorized_bytecode = db.get_account_no_push_cache(auth_address)?.code; Ok((true, access_cost, auth_address, authorized_bytecode)) } @@ -403,11 +403,11 @@ impl<'a> VM<'a> { self.accrued_substate .touched_accounts .insert(authority_address); - let authority_account_info = self.db.get_account_no_push_cache(authority_address)?.info; + let authority_account = self.db.get_account_no_push_cache(authority_address)?; // 5. Verify the code of authority is either empty or already delegated. - let empty_or_delegated = authority_account_info.bytecode.is_empty() - || has_delegation(&authority_account_info)?; + let empty_or_delegated = + authority_account.code.is_empty() || has_delegation(&authority_account)?; if !empty_or_delegated { continue; } @@ -415,7 +415,7 @@ impl<'a> VM<'a> { // 6. Verify the nonce of authority is equal to nonce. In case authority does not exist in the trie, verify that nonce is equal to 0. // If it doesn't exist, it means the nonce is zero. The access_account() function will return AccountInfo::default() // If it has nonce, the account.info.nonce should equal auth_tuple.nonce - if authority_account_info.nonce != auth_tuple.nonce { + if authority_account.info.nonce != auth_tuple.nonce { continue; } @@ -440,11 +440,12 @@ impl<'a> VM<'a> { // Clear the account’s code and reset the account’s code hash to the empty hash. let auth_account = self.get_account_mut(authority_address)?; - auth_account.info.bytecode = if auth_tuple.address != Address::zero() { + let code = if auth_tuple.address != Address::zero() { delegation_bytes.into() } else { Bytes::new() }; + auth_account.set_code(code); // 9. Increase the nonce of authority by one. self.increment_account_nonce(authority_address) @@ -464,9 +465,9 @@ impl<'a> VM<'a> { .db .access_account(&mut self.accrued_substate, code_address)?; - self.current_call_frame_mut()?.bytecode = auth_address_info.bytecode.clone(); + self.current_call_frame_mut()?.bytecode = auth_address_info.code.clone(); } else { - self.current_call_frame_mut()?.bytecode = code_address_info.bytecode.clone(); + self.current_call_frame_mut()?.bytecode = code_address_info.code.clone(); } self.current_call_frame_mut()?.valid_jump_destinations = diff --git a/crates/vm/levm/src/vm.rs b/crates/vm/levm/src/vm.rs index 757fbd02f8..9c661d761b 100644 --- a/crates/vm/levm/src/vm.rs +++ b/crates/vm/levm/src/vm.rs @@ -167,6 +167,7 @@ pub struct VM<'a> { pub hooks: Vec>, pub return_data: Vec, pub backups: Vec, + pub storage_original_values: HashMap>, } pub struct RetData { @@ -217,13 +218,6 @@ impl<'a> VM<'a> { default_touched_accounts.insert(Address::from_low_u64_be(i)); } - // When instantiating a new vm the current value of the storage slots are actually the original values because it is a new transaction - for account in db.cache.values_mut() { - for storage_slot in account.storage.values_mut() { - storage_slot.original_value = storage_slot.current_value; - } - } - let hooks: Vec> = match tx { Transaction::PrivilegedL2Transaction(privileged_tx) => vec![Arc::new(L2Hook { recipient: privileged_tx.recipient, @@ -270,6 +264,7 @@ impl<'a> VM<'a> { hooks, return_data: vec![], backups: vec![], + storage_original_values: HashMap::new(), }) } TxKind::Create => { @@ -311,6 +306,7 @@ impl<'a> VM<'a> { hooks, return_data: vec![], backups: vec![], + storage_original_values: HashMap::new(), }) } } From d75425716cc42cb876369a884e11004e3fcc3cb5 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Mon, 28 Apr 2025 16:13:38 -0300 Subject: [PATCH 02/14] improve get_original_storage a little bit --- crates/vm/levm/src/db/gen_db.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index e0a65c41b5..75afbd61c8 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -183,14 +183,11 @@ impl<'a> VM<'a> { }; // Add it to the original values if it wasn't already there - if let Some(account_storage) = self.storage_original_values.get_mut(&address) { - account_storage.entry(key).or_insert(value_pre_tx); - } else { - let mut account_storage = HashMap::new(); - account_storage.insert(key, value_pre_tx); - self.storage_original_values - .insert(address, account_storage); - } + self.storage_original_values + .entry(address) + .or_insert_with(HashMap::new) + .entry(key) + .or_insert(value_pre_tx); Ok(value_pre_tx) } From df52a5d345ff92b51334d7096c80ab95615b8cd9 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Mon, 28 Apr 2025 16:21:45 -0300 Subject: [PATCH 03/14] change or_insert_with for or_default --- crates/vm/levm/src/db/gen_db.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index 75afbd61c8..df60dd52b2 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -185,7 +185,7 @@ impl<'a> VM<'a> { // Add it to the original values if it wasn't already there self.storage_original_values .entry(address) - .or_insert_with(HashMap::new) + .or_default() .entry(key) .or_insert(value_pre_tx); From e2562d14cec73ccd2e1296cbdafeb9fe8f4dc514 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Mon, 28 Apr 2025 16:25:03 -0300 Subject: [PATCH 04/14] clippy lint --- crates/vm/levm/src/db/gen_db.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index df60dd52b2..8b4c9c2e1b 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::sync::Arc; use bytes::Bytes; From 80268a9371c4e7610981cd574722b1de473c94b1 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 09:50:27 -0300 Subject: [PATCH 05/14] add in memory db --- crates/vm/backends/levm/mod.rs | 14 +++---- crates/vm/levm/src/db/gen_db.rs | 72 ++++++++++++++++++++++++--------- crates/vm/levm/src/utils.rs | 2 +- 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/crates/vm/backends/levm/mod.rs b/crates/vm/backends/levm/mod.rs index 00ef6fa443..0081701bf5 100644 --- a/crates/vm/backends/levm/mod.rs +++ b/crates/vm/backends/levm/mod.rs @@ -164,8 +164,7 @@ impl LEVM { ) -> Result, EvmError> { let mut account_updates: Vec = vec![]; for (address, new_state_account) in db.cache.drain() { - let initial_state_account = db.store.get_account(address)?; - let account_existed = db.store.account_exists(address); + let initial_state_account = db.in_memory_db.get(&address).expect("account not found"); let mut acc_info_updated = false; let mut storage_updated = false; @@ -206,15 +205,16 @@ impl LEVM { // https://eips.ethereum.org/EIPS/eip-161 if fork >= Fork::SpuriousDragon { - // "No account may change state from non-existent to existent-but-_empty_. If an operation would do this, the account SHALL instead remain non-existent." - if !account_existed && new_state_account.is_empty() { - continue; - } - // "At the end of the transaction, any account touched by the execution of that transaction which is now empty SHALL instead become non-existent (i.e. deleted)." // Note: An account can be empty but still exist in the trie (if that's the case we remove it) if new_state_account.is_empty() { removed = true; + + // "No account may change state from non-existent to existent-but-_empty_. If an operation would do this, the account SHALL instead remain non-existent." + let account_existed = db.store.account_exists(address); + if !account_existed { + continue; + } } } diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index 8b4c9c2e1b..cf20fccac2 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::sync::Arc; use bytes::Bytes; @@ -21,11 +22,16 @@ use super::Database; pub struct GeneralizedDatabase { pub store: Arc, pub cache: CacheDB, + pub in_memory_db: HashMap, } impl GeneralizedDatabase { pub fn new(store: Arc, cache: CacheDB) -> Self { - Self { store, cache } + Self { + store, + cache, + in_memory_db: HashMap::new(), + } } // ================== Account related functions ===================== @@ -35,18 +41,46 @@ impl GeneralizedDatabase { match cache::get_account(&self.cache, &address) { Some(acc) => Ok(acc.clone()), None => { - let account = self.store.get_account(address)?; + let account = self.get_account_from_storage(address)?; cache::insert_account(&mut self.cache, address, account.clone()); Ok(account) } } } + /// Gets account from storage, storing in InMemoryDB for efficiency when getting AccountUpdates. + pub fn get_account_from_storage(&mut self, address: Address) -> Result { + let account = self.store.get_account(address)?; + self.in_memory_db.insert(address, account.clone()); + Ok(account) + } + + /// Gets storage slot from Database, storing in InMemoryDB for efficiency when getting AccountUpdates. + pub fn get_storage_slot_from_storage( + &mut self, + address: Address, + key: H256, + ) -> Result { + let value = self.store.get_storage_slot(address, key)?; + // Account must be already in in_memory_db + if let Some(account) = self.in_memory_db.get_mut(&address) { + account.storage.insert(key, value); + } else { + return Err(DatabaseError::Custom( + "Account not found in InMemoryDB".to_string(), + )); + } + Ok(value) + } + /// Gets account without pushing it to the cache - pub fn get_account_no_push_cache(&self, address: Address) -> Result { + pub fn get_account_no_push_cache( + &mut self, + address: Address, + ) -> Result { match cache::get_account(&self.cache, &address) { Some(acc) => Ok(acc.clone()), - None => self.store.get_account(address), + None => self.get_account_from_storage(address), } } @@ -71,7 +105,7 @@ impl<'a> VM<'a> { pub fn get_account_mut(&mut self, address: Address) -> Result<&mut Account, VMError> { if !cache::is_account_cached(&self.db.cache, &address) { - let account = self.db.store.get_account(address)?; + let account = self.db.get_account_from_storage(address)?; cache::insert_account(&mut self.db.cache, address, account.clone()); } @@ -216,22 +250,20 @@ impl<'a> VM<'a> { Ok((storage_slot, storage_slot_was_cold)) } - /// Gets storage slot of an account, caching it if not already cached. + /// Gets storage slot of an account, caching the account and the storage slot if not already cached. pub fn get_storage_slot(&mut self, address: Address, key: H256) -> Result { - let storage_slot = match cache::get_account(&self.db.cache, &address) { - Some(account) => match account.storage.get(&key) { - Some(storage_slot) => *storage_slot, - None => self.db.store.get_storage_slot(address, key)?, - }, - None => self.db.store.get_storage_slot(address, key)?, - }; - - // When getting storage slot of an account that's not yet cached we need to store it in the account - // Note: We end up caching the account because it is the most straightforward way of doing it. - let account = self.get_account_mut(address)?; - account.storage.entry(key).or_insert(storage_slot); - - Ok(storage_slot) + let account = self.db.get_account(address)?; + + let storage_slot = account.storage.get(&key); + if let Some(value) = storage_slot { + return Ok(*value); + } else { + self.db.get_account(address)?; // For storing and caching the account first if necessary. + let value = self.db.get_storage_slot_from_storage(address, key)?; + let account = self.get_account_mut(address)?; + account.storage.insert(key, value); + return Ok(value); + } } //TODO: Can be more performant with .entry and .and_modify, but didn't want to add complexity. diff --git a/crates/vm/levm/src/utils.rs b/crates/vm/levm/src/utils.rs index 4835af78f4..dbff02577f 100644 --- a/crates/vm/levm/src/utils.rs +++ b/crates/vm/levm/src/utils.rs @@ -331,7 +331,7 @@ pub fn eip7702_recover_address( /// The idea of this function comes from ethereum/execution-specs: /// https://github.com/ethereum/execution-specs/blob/951fc43a709b493f27418a8e57d2d6f3608cef84/src/ethereum/prague/vm/eoa_delegation.py#L115 pub fn eip7702_get_code( - db: &GeneralizedDatabase, + db: &mut GeneralizedDatabase, accrued_substate: &mut Substate, address: Address, ) -> Result<(bool, u64, Address, Bytes), VMError> { From 18b077533f17c1ceeccb646639659cb7861880c2 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 10:59:20 -0300 Subject: [PATCH 06/14] refactor get_original_storage --- crates/vm/levm/src/db/gen_db.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index 8b4c9c2e1b..5ad5d00e7d 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -173,22 +173,20 @@ impl<'a> VM<'a> { /// Gets original storage value of an account, caching it if not already cached. /// Also saves the original value for future gas calculations. pub fn get_original_storage(&mut self, address: Address, key: H256) -> Result { - let value_pre_tx = match self.storage_original_values.get(&address).cloned() { - Some(account_storage) => match account_storage.get(&key) { - Some(value) => *value, - None => self.get_storage_slot(address, key)?, - }, - None => self.get_storage_slot(address, key)?, - }; + if let Some(value) = self + .storage_original_values + .get(&address) + .and_then(|account_storage| account_storage.get(&key)) + { + return Ok(*value); + } - // Add it to the original values if it wasn't already there + let value = self.get_storage_slot(address, key)?; self.storage_original_values .entry(address) .or_default() - .entry(key) - .or_insert(value_pre_tx); - - Ok(value_pre_tx) + .insert(key, value); + Ok(value) } /// Accesses to an account's storage slot. From 4a20a4e18ea968301f7b1fa2b9762af3800f7b89 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 11:12:39 -0300 Subject: [PATCH 07/14] clear in memory db after get_state_transitions --- crates/vm/backends/levm/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/vm/backends/levm/mod.rs b/crates/vm/backends/levm/mod.rs index 0081701bf5..768353b218 100644 --- a/crates/vm/backends/levm/mod.rs +++ b/crates/vm/backends/levm/mod.rs @@ -233,6 +233,7 @@ impl LEVM { account_updates.push(account_update); } + db.in_memory_db.clear(); Ok(account_updates) } From f8bdfc89a3e30f0a5234c7130ec6b3424bc677e6 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 16:43:58 -0300 Subject: [PATCH 08/14] change set_code --- crates/common/types/account.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/common/types/account.rs b/crates/common/types/account.rs index d814311128..0552451d1c 100644 --- a/crates/common/types/account.rs +++ b/crates/common/types/account.rs @@ -202,8 +202,8 @@ impl Account { } pub fn set_code(&mut self, code: Bytes) { - self.code = code.clone(); self.info.code_hash = keccak(code.as_ref()).0.into(); + self.code = code; } } From f1ee8cec0b17c001f5a69246eb1d12921b4191f1 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 17:01:11 -0300 Subject: [PATCH 09/14] make suggested changes --- cmd/ef_tests/state/runner/levm_runner.rs | 2 +- crates/l2/prover/bench/src/rpc/db.rs | 2 +- crates/vm/backends/levm/db.rs | 8 +++--- crates/vm/backends/levm/mod.rs | 2 +- crates/vm/levm/src/db/gen_db.rs | 28 +++++++++---------- crates/vm/levm/src/db/mod.rs | 2 +- .../stack_memory_storage_flow.rs | 4 +-- 7 files changed, 23 insertions(+), 25 deletions(-) diff --git a/cmd/ef_tests/state/runner/levm_runner.rs b/cmd/ef_tests/state/runner/levm_runner.rs index 3a0845e6f5..5fa3e7d0f1 100644 --- a/cmd/ef_tests/state/runner/levm_runner.rs +++ b/cmd/ef_tests/state/runner/levm_runner.rs @@ -220,7 +220,7 @@ pub fn ensure_pre_state(evm: &VM, test: &EFTest) -> Result<(), EFTestRunnerError )?; for (k, v) in &pre_value.storage { let storage_slot = world_state - .get_storage_slot(*address, H256::from_slice(&k.to_big_endian())) + .get_storage_value(*address, H256::from_slice(&k.to_big_endian())) .unwrap(); ensure_pre_state_condition( &storage_slot == v, diff --git a/crates/l2/prover/bench/src/rpc/db.rs b/crates/l2/prover/bench/src/rpc/db.rs index 9c7e94f6d5..6fc04c9d39 100644 --- a/crates/l2/prover/bench/src/rpc/db.rs +++ b/crates/l2/prover/bench/src/rpc/db.rs @@ -477,7 +477,7 @@ impl LevmDatabase for RpcDB { } } - fn get_storage_slot(&self, address: Address, key: H256) -> Result { + fn get_storage_value(&self, address: Address, key: H256) -> Result { let account = self .fetch_accounts_blocking(&[(address, vec![key])], false) .map_err(|e| DatabaseError::Custom(format!("Failed to fetch account info: {e}")))? diff --git a/crates/vm/backends/levm/db.rs b/crates/vm/backends/levm/db.rs index deca442f17..ecd72a34a0 100644 --- a/crates/vm/backends/levm/db.rs +++ b/crates/vm/backends/levm/db.rs @@ -43,7 +43,7 @@ impl LevmDatabase for DatabaseLogger { self.store.account_exists(address) } - fn get_storage_slot( + fn get_storage_value( &self, address: CoreAddress, key: CoreH256, @@ -54,7 +54,7 @@ impl LevmDatabase for DatabaseLogger { .entry(address) .and_modify(|keys| keys.push(key)) .or_insert(vec![key]); - self.store.get_storage_slot(address, key) + self.store.get_storage_value(address, key) } fn get_block_hash(&self, block_number: u64) -> Result, DatabaseError> { @@ -115,7 +115,7 @@ impl LevmDatabase for StoreWrapper { acc_info.is_some() } - fn get_storage_slot( + fn get_storage_value( &self, address: CoreAddress, key: CoreH256, @@ -179,7 +179,7 @@ impl LevmDatabase for ExecutionDB { Ok(self.block_hashes.get(&block_number).cloned()) } - fn get_storage_slot( + fn get_storage_value( &self, address: CoreAddress, key: CoreH256, diff --git a/crates/vm/backends/levm/mod.rs b/crates/vm/backends/levm/mod.rs index 00ef6fa443..c9bbfca0b0 100644 --- a/crates/vm/backends/levm/mod.rs +++ b/crates/vm/backends/levm/mod.rs @@ -189,7 +189,7 @@ impl LEVM { // 2. Storage has been updated if the current value is different from the one before execution. let mut added_storage = HashMap::new(); for (key, storage_slot) in &new_state_account.storage { - let storage_before_block = db.store.get_storage_slot(address, *key)?; + let storage_before_block = db.store.get_storage_value(address, *key)?; if *storage_slot != storage_before_block { added_storage.insert(*key, *storage_slot); storage_updated = true; diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index 5ad5d00e7d..3338cedbd7 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -181,7 +181,7 @@ impl<'a> VM<'a> { return Ok(*value); } - let value = self.get_storage_slot(address, key)?; + let value = self.get_storage_value(address, key)?; self.storage_original_values .entry(address) .or_default() @@ -189,7 +189,7 @@ impl<'a> VM<'a> { Ok(value) } - /// Accesses to an account's storage slot. + /// Accesses to an account's storage slot and returns the value in it. /// /// Accessed storage slots are stored in the `touched_storage_slots` set. /// Accessed storage slots take place in some gas cost computation. @@ -209,30 +209,30 @@ impl<'a> VM<'a> { .insert(key); } - let storage_slot = self.get_storage_slot(address, key)?; + let storage_slot = self.get_storage_value(address, key)?; Ok((storage_slot, storage_slot_was_cold)) } /// Gets storage slot of an account, caching it if not already cached. - pub fn get_storage_slot(&mut self, address: Address, key: H256) -> Result { - let storage_slot = match cache::get_account(&self.db.cache, &address) { + pub fn get_storage_value(&mut self, address: Address, key: H256) -> Result { + let value = match cache::get_account(&self.db.cache, &address) { Some(account) => match account.storage.get(&key) { - Some(storage_slot) => *storage_slot, - None => self.db.store.get_storage_slot(address, key)?, + Some(value) => *value, + None => self.db.store.get_storage_value(address, key)?, }, - None => self.db.store.get_storage_slot(address, key)?, + None => self.db.store.get_storage_value(address, key)?, }; - // When getting storage slot of an account that's not yet cached we need to store it in the account + // When getting storage value of an account that's not yet cached we need to store it in the account // Note: We end up caching the account because it is the most straightforward way of doing it. let account = self.get_account_mut(address)?; - account.storage.entry(key).or_insert(storage_slot); + account.storage.entry(key).or_insert(value); - Ok(storage_slot) + Ok(value) } - //TODO: Can be more performant with .entry and .and_modify, but didn't want to add complexity. + /// Updates storage of an account, caching it if not already cached. pub fn update_account_storage( &mut self, address: Address, @@ -240,9 +240,7 @@ impl<'a> VM<'a> { new_value: U256, ) -> Result<(), VMError> { let account = self.get_account_mut(address)?; - if account.storage.get(&key) != Some(&new_value) { - account.storage.insert(key, new_value); - } + account.storage.insert(key, new_value); Ok(()) } } diff --git a/crates/vm/levm/src/db/mod.rs b/crates/vm/levm/src/db/mod.rs index 717502455e..97f6b383bc 100644 --- a/crates/vm/levm/src/db/mod.rs +++ b/crates/vm/levm/src/db/mod.rs @@ -12,7 +12,7 @@ pub mod gen_db; pub trait Database: Send + Sync { fn get_account(&self, address: Address) -> Result; - fn get_storage_slot(&self, address: Address, key: H256) -> Result; + fn get_storage_value(&self, address: Address, key: H256) -> Result; fn get_block_hash(&self, block_number: u64) -> Result, DatabaseError>; fn account_exists(&self, address: Address) -> bool; fn get_chain_config(&self) -> ChainConfig; diff --git a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs index 96c05394d5..1545c65087 100644 --- a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs +++ b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs @@ -146,14 +146,14 @@ impl<'a> VM<'a> { let storage_slot_key = H256::from(storage_slot_key.to_big_endian()); - let (storage_slot, storage_slot_was_cold) = + let (value, storage_slot_was_cold) = self.access_storage_slot(address, storage_slot_key)?; let current_call_frame = self.current_call_frame_mut()?; current_call_frame.increase_consumed_gas(gas_cost::sload(storage_slot_was_cold, fork)?)?; - current_call_frame.stack.push(storage_slot)?; + current_call_frame.stack.push(value)?; Ok(OpcodeResult::Continue { pc_increment: 1 }) } From e72eaf1e0e311ca15f30833240388dc49b97357c Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 17:07:18 -0300 Subject: [PATCH 10/14] implement nit --- crates/vm/levm/src/db/gen_db.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index 3338cedbd7..e60c122648 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -70,14 +70,14 @@ impl<'a> VM<'a> { // ================== Account related functions ===================== pub fn get_account_mut(&mut self, address: Address) -> Result<&mut Account, VMError> { - if !cache::is_account_cached(&self.db.cache, &address) { - let account = self.db.store.get_account(address)?; - cache::insert_account(&mut self.db.cache, address, account.clone()); - } - - let backup_account = cache::get_account(&self.db.cache, &address) - .ok_or(VMError::Internal(InternalError::AccountNotFound))? - .clone(); + let backup_account = match cache::get_account(&self.db.cache, &address) { + Some(acc) => acc.clone(), + None => { + let acc = self.db.store.get_account(address)?; + cache::insert_account(&mut self.db.cache, address, acc.clone()); + acc + } + }; if let Ok(frame) = self.current_call_frame_mut() { frame From 5af34b2ee2af6bab9b9eb2d4da068ff626643305 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 17:16:50 -0300 Subject: [PATCH 11/14] update get storage value --- crates/vm/levm/src/db/gen_db.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index e60c122648..3aa2b35a16 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -214,18 +214,18 @@ impl<'a> VM<'a> { Ok((storage_slot, storage_slot_was_cold)) } - /// Gets storage slot of an account, caching it if not already cached. + /// Gets storage value of an account, caching it if not already cached. pub fn get_storage_value(&mut self, address: Address, key: H256) -> Result { - let value = match cache::get_account(&self.db.cache, &address) { - Some(account) => match account.storage.get(&key) { - Some(value) => *value, - None => self.db.store.get_storage_value(address, key)?, - }, - None => self.db.store.get_storage_value(address, key)?, - }; + if let Some(account) = cache::get_account(&self.db.cache, &address) { + if let Some(value) = account.storage.get(&key) { + return Ok(*value); + } + } + + let value = self.db.store.get_storage_value(address, key)?; // When getting storage value of an account that's not yet cached we need to store it in the account - // Note: We end up caching the account because it is the most straightforward way of doing it. + // We don't actually know if the account is cached so we cache it anyway let account = self.get_account_mut(address)?; account.storage.entry(key).or_insert(value); From 36a7c24e198ccfd45efce1afc6efd6e0a4421ee2 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 18:26:14 -0300 Subject: [PATCH 12/14] run cargo fmt --- .../vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs index 1545c65087..f51882a879 100644 --- a/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs +++ b/crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs @@ -146,8 +146,7 @@ impl<'a> VM<'a> { let storage_slot_key = H256::from(storage_slot_key.to_big_endian()); - let (value, storage_slot_was_cold) = - self.access_storage_slot(address, storage_slot_key)?; + let (value, storage_slot_was_cold) = self.access_storage_slot(address, storage_slot_key)?; let current_call_frame = self.current_call_frame_mut()?; From c473f68b68721397245bf7c7174d530e1075a58b Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 18:42:38 -0300 Subject: [PATCH 13/14] get from database if not in in_memory_db --- crates/vm/backends/levm/mod.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/vm/backends/levm/mod.rs b/crates/vm/backends/levm/mod.rs index 768353b218..4b877d4f0c 100644 --- a/crates/vm/backends/levm/mod.rs +++ b/crates/vm/backends/levm/mod.rs @@ -164,7 +164,11 @@ impl LEVM { ) -> Result, EvmError> { let mut account_updates: Vec = vec![]; for (address, new_state_account) in db.cache.drain() { - let initial_state_account = db.in_memory_db.get(&address).expect("account not found"); + let initial_state_account = db + .in_memory_db + .get(&address) + .cloned() + .unwrap_or(db.store.get_account(address)?); let mut acc_info_updated = false; let mut storage_updated = false; From 94e3fbcac208636be908cacdeb80685e13b50112 Mon Sep 17 00:00:00 2001 From: JereSalo Date: Tue, 29 Apr 2025 19:00:39 -0300 Subject: [PATCH 14/14] change some names --- crates/vm/levm/src/db/gen_db.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/crates/vm/levm/src/db/gen_db.rs b/crates/vm/levm/src/db/gen_db.rs index 9052bf343e..d42bef3c74 100644 --- a/crates/vm/levm/src/db/gen_db.rs +++ b/crates/vm/levm/src/db/gen_db.rs @@ -41,7 +41,7 @@ impl GeneralizedDatabase { match cache::get_account(&self.cache, &address) { Some(acc) => Ok(acc.clone()), None => { - let account = self.get_account_from_storage(address)?; + let account = self.get_account_from_database(address)?; cache::insert_account(&mut self.cache, address, account.clone()); Ok(account) } @@ -49,14 +49,17 @@ impl GeneralizedDatabase { } /// Gets account from storage, storing in InMemoryDB for efficiency when getting AccountUpdates. - pub fn get_account_from_storage(&mut self, address: Address) -> Result { + pub fn get_account_from_database( + &mut self, + address: Address, + ) -> Result { let account = self.store.get_account(address)?; self.in_memory_db.insert(address, account.clone()); Ok(account) } /// Gets storage slot from Database, storing in InMemoryDB for efficiency when getting AccountUpdates. - pub fn get_storage_slot_from_storage( + pub fn get_value_from_database( &mut self, address: Address, key: H256, @@ -80,7 +83,7 @@ impl GeneralizedDatabase { ) -> Result { match cache::get_account(&self.cache, &address) { Some(acc) => Ok(acc.clone()), - None => self.get_account_from_storage(address), + None => self.get_account_from_database(address), } }