Skip to content

chore(levm): improve storage_original_values hashmap and gen_db field names #3261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/ef_tests/state/runner/levm_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ pub async fn ensure_post_state(
fork: &Fork,
db: &mut GeneralizedDatabase,
) -> Result<(), EFTestRunnerError> {
let cache = db.cache.clone();
let cache = db.current_accounts_state.clone();
match levm_execution_result {
Ok(execution_report) => {
match test.post.vector_post_value(vector, *fork).expect_exception {
Expand Down
2 changes: 1 addition & 1 deletion cmd/ef_tests/state/runner/revm_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ pub async fn ensure_post_state(
// We only want to compare account updates when no exception is expected.
None => {
let mut db = load_initial_state_levm(test).await;
db.cache = levm_cache;
db.current_accounts_state = levm_cache;
let levm_account_updates = backends::levm::LEVM::get_state_transitions(&mut db)
.map_err(|_| {
InternalError::Custom("Error at LEVM::get_state_transitions()".to_owned())
Expand Down
18 changes: 6 additions & 12 deletions crates/l2/sequencer/block_producer/payload_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,12 +276,9 @@ fn get_account_diffs_in_tx(
})?;
// First we add the account info
for (address, original_account) in transaction_backup.original_accounts_info.iter() {
let new_account =
db.cache
.get(address)
.ok_or(BlockProducerError::FailedToGetDataFrom(
"DB Cache".to_owned(),
))?;
let new_account = db.current_accounts_state.get(address).ok_or(
BlockProducerError::FailedToGetDataFrom("DB Cache".to_owned()),
)?;

let nonce_diff: u16 = (new_account.info.nonce - original_account.info.nonce)
.try_into()
Expand Down Expand Up @@ -314,12 +311,9 @@ fn get_account_diffs_in_tx(
for (address, original_storage_slots) in
transaction_backup.original_account_storage_slots.iter()
{
let account_info =
db.cache
.get(address)
.ok_or(BlockProducerError::FailedToGetDataFrom(
"DB Cache".to_owned(),
))?;
let account_info = db.current_accounts_state.get(address).ok_or(
BlockProducerError::FailedToGetDataFrom("DB Cache".to_owned()),
)?;

let mut added_storage = BTreeMap::new();
for key in original_storage_slots.keys() {
Expand Down
27 changes: 16 additions & 11 deletions crates/vm/backends/levm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,10 @@ impl LEVM {
db: &mut GeneralizedDatabase,
) -> Result<Vec<AccountUpdate>, EvmError> {
let mut account_updates: Vec<AccountUpdate> = vec![];
for (address, new_state_account) in db.cache.iter() {
for (address, new_state_account) in db.current_accounts_state.iter() {
// In case the account is not in immutable_cache (rare) we search for it in the actual database.
let initial_state_account =
db.immutable_cache
db.initial_accounts_state
.get(address)
.ok_or(EvmError::Custom(format!(
"Failed to get account {address} from immutable cache",
Expand Down Expand Up @@ -258,8 +258,8 @@ impl LEVM {

account_updates.push(account_update);
}
db.cache.clear();
db.immutable_cache.clear();
db.current_accounts_state.clear();
db.initial_accounts_state.clear();
Ok(account_updates)
}

Expand All @@ -279,7 +279,7 @@ impl LEVM {
.clone(); // Not a big deal cloning here because it's an EOA.

account.info.balance += increment.into();
db.cache.insert(address, account);
db.current_accounts_state.insert(address, account);
}
Ok(())
}
Expand Down Expand Up @@ -435,8 +435,11 @@ pub fn generic_system_contract_levm(
) -> Result<ExecutionReport, EvmError> {
let chain_config = db.store.get_chain_config()?;
let config = EVMConfig::new_from_chain_config(&chain_config, block_header);
let system_account_backup = db.cache.get(&system_address).cloned();
let coinbase_backup = db.cache.get(&block_header.coinbase).cloned();
let system_account_backup = db.current_accounts_state.get(&system_address).cloned();
let coinbase_backup = db
.current_accounts_state
.get(&block_header.coinbase)
.cloned();
let env = Environment {
origin: system_address,
// EIPs 2935, 4788, 7002 and 7251 dictate that the system calls have a gas limit of 30 million and they do not use intrinsic gas.
Expand Down Expand Up @@ -466,17 +469,19 @@ pub fn generic_system_contract_levm(
let report = vm.execute().map_err(EvmError::from)?;

if let Some(system_account) = system_account_backup {
db.cache.insert(system_address, system_account);
db.current_accounts_state
.insert(system_address, system_account);
} else {
// If the system account was not in the cache, we need to remove it
db.cache.remove(&system_address);
db.current_accounts_state.remove(&system_address);
}

if let Some(coinbase_account) = coinbase_backup {
db.cache.insert(block_header.coinbase, coinbase_account);
db.current_accounts_state
.insert(block_header.coinbase, coinbase_account);
} else {
// If the coinbase account was not in the cache, we need to remove it
db.cache.remove(&block_header.coinbase);
db.current_accounts_state.remove(&block_header.coinbase);
}

Ok(report)
Expand Down
52 changes: 23 additions & 29 deletions crates/vm/levm/src/db/gen_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,17 @@ use super::cache;
#[derive(Clone)]
pub struct GeneralizedDatabase {
pub store: Arc<dyn Database>,
pub cache: CacheDB,
pub immutable_cache: HashMap<Address, Account>,
pub current_accounts_state: CacheDB,
pub initial_accounts_state: HashMap<Address, Account>,
pub tx_backup: Option<CallFrameBackup>,
}

impl GeneralizedDatabase {
pub fn new(store: Arc<dyn Database>, cache: CacheDB) -> Self {
pub fn new(store: Arc<dyn Database>, current_accounts_state: CacheDB) -> Self {
Self {
store,
cache: cache.clone(),
immutable_cache: cache,
current_accounts_state: current_accounts_state.clone(),
initial_accounts_state: current_accounts_state,
tx_backup: None,
}
}
Expand All @@ -40,11 +40,12 @@ impl GeneralizedDatabase {
/// Gets account, first checking the cache and then the database
/// (caching in the second case)
pub fn get_account(&mut self, address: Address) -> Result<&Account, InternalError> {
if !cache::account_is_cached(&self.cache, &address) {
if !cache::account_is_cached(&self.current_accounts_state, &address) {
let account = self.get_account_from_database(address)?;
cache::insert_account(&mut self.cache, address, account);
cache::insert_account(&mut self.current_accounts_state, address, account);
}
cache::get_account(&self.cache, &address).ok_or(InternalError::AccountNotFound)
cache::get_account(&self.current_accounts_state, &address)
.ok_or(InternalError::AccountNotFound)
}

/// **Accesses to an account's information.**
Expand All @@ -62,25 +63,25 @@ impl GeneralizedDatabase {
Ok((account, address_was_cold))
}

/// Gets account from storage, storing in Immutable Cache for efficiency when getting AccountUpdates.
/// Gets account from storage, storing in initial_accounts_state for efficiency when getting AccountUpdates.
pub fn get_account_from_database(
&mut self,
address: Address,
) -> Result<Account, InternalError> {
let account = self.store.get_account(address)?;
self.immutable_cache.insert(address, account.clone());
self.initial_accounts_state.insert(address, account.clone());
Ok(account)
}

/// Gets storage slot from Database, storing in Immutable Cache for efficiency when getting AccountUpdates.
/// Gets storage slot from Database, storing in initial_accounts_state for efficiency when getting AccountUpdates.
pub fn get_value_from_database(
&mut self,
address: Address,
key: H256,
) -> Result<U256, InternalError> {
let value = self.store.get_storage_value(address, key)?;
// Account must already be in immutable_cache
match self.immutable_cache.get_mut(&address) {
// Account must already be in initial_accounts_state
match self.initial_accounts_state.get_mut(&address) {
Some(account) => {
account.storage.insert(key, value);
}
Expand Down Expand Up @@ -133,15 +134,15 @@ impl<'a> VM<'a> {

*/
pub fn get_account_mut(&mut self, address: Address) -> Result<&mut Account, InternalError> {
if cache::is_account_cached(&self.db.cache, &address) {
if cache::is_account_cached(&self.db.current_accounts_state, &address) {
self.backup_account_info(address)?;
cache::get_account_mut(&mut self.db.cache, &address)
cache::get_account_mut(&mut self.db.current_accounts_state, &address)
.ok_or(InternalError::AccountNotFound)
} else {
let acc = self.db.get_account_from_database(address)?;
cache::insert_account(&mut self.db.cache, address, acc);
cache::insert_account(&mut self.db.current_accounts_state, address, acc);
self.backup_account_info(address)?;
cache::get_account_mut(&mut self.db.cache, &address)
cache::get_account_mut(&mut self.db.current_accounts_state, &address)
.ok_or(InternalError::AccountNotFound)
}
}
Expand Down Expand Up @@ -214,7 +215,7 @@ impl<'a> VM<'a> {
account: Account,
) -> Result<(), InternalError> {
self.backup_account_info(address)?;
let _ = cache::insert_account(&mut self.db.cache, address, account);
let _ = cache::insert_account(&mut self.db.current_accounts_state, address, account);

Ok(())
}
Expand All @@ -226,19 +227,12 @@ impl<'a> VM<'a> {
address: Address,
key: H256,
) -> Result<U256, InternalError> {
if let Some(value) = self
.storage_original_values
.get(&address)
.and_then(|account_storage| account_storage.get(&key))
{
if let Some(value) = self.storage_original_values.get(&(address, key)) {
return Ok(*value);
}

let value = self.get_storage_value(address, key)?;
self.storage_original_values
.entry(address)
.or_default()
.insert(key, value);
self.storage_original_values.insert((address, key), value);
Ok(value)
}

Expand Down Expand Up @@ -270,7 +264,7 @@ impl<'a> VM<'a> {
address: Address,
key: H256,
) -> Result<U256, InternalError> {
if let Some(account) = cache::get_account(&self.db.cache, &address) {
if let Some(account) = cache::get_account(&self.db.current_accounts_state, &address) {
if let Some(value) = account.storage.get(&key) {
return Ok(*value);
}
Expand Down Expand Up @@ -333,7 +327,7 @@ impl<'a> VM<'a> {
.contains_key(&address);

if is_not_backed_up {
let account = cache::get_account(&self.db.cache, &address)
let account = cache::get_account(&self.db.current_accounts_state, &address)
.ok_or(InternalError::AccountNotFound)?;
let info = account.info.clone();
let code = account.code.clone();
Expand Down
6 changes: 4 additions & 2 deletions crates/vm/levm/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,9 @@ pub fn restore_cache_state(
callframe_backup: CallFrameBackup,
) -> Result<(), VMError> {
for (address, account) in callframe_backup.original_accounts_info {
if let Some(current_account) = cache::get_account_mut(&mut db.cache, &address) {
if let Some(current_account) =
cache::get_account_mut(&mut db.current_accounts_state, &address)
{
current_account.info = account.info;
current_account.code = account.code;
}
Expand All @@ -140,7 +142,7 @@ pub fn restore_cache_state(
// This call to `get_account_mut` should never return None, because we are looking up accounts
// that had their storage modified, which means they should be in the cache. That's why
// we return an internal error in case we haven't found it.
let account = cache::get_account_mut(&mut db.cache, &address)
let account = cache::get_account_mut(&mut db.current_accounts_state, &address)
.ok_or(InternalError::AccountNotFound)?;

for (key, value) in storage {
Expand Down
2 changes: 1 addition & 1 deletion crates/vm/levm/src/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub struct VM<'a> {
pub hooks: Vec<Rc<RefCell<dyn Hook>>>,
pub substate_backups: Vec<Substate>,
/// Original storage values before the transaction. Used for gas calculations in SSTORE.
pub storage_original_values: HashMap<Address, HashMap<H256, U256>>,
pub storage_original_values: HashMap<(Address, H256), U256>,
/// When enabled, it "logs" relevant information during execution
pub tracer: LevmCallTracer,
/// Mode for printing some useful stuff, only used in development!
Expand Down
Loading