diff --git a/crates/primitives/src/state.rs b/crates/primitives/src/state.rs index 57c08a444d..86bd5d3020 100644 --- a/crates/primitives/src/state.rs +++ b/crates/primitives/src/state.rs @@ -242,10 +242,16 @@ impl AccountInfo { self.balance == U256::ZERO && self.nonce == 0 && code_empty } + /// Returns `true` if the account is not empty. pub fn exists(&self) -> bool { !self.is_empty() } + /// Returns `true` if account has no nonce and code. + pub fn has_no_code_and_nonce(&self) -> bool { + self.is_empty_code_hash() && self.nonce == 0 + } + /// Return bytecode hash associated with this account. /// If account does not have code, it return's `KECCAK_EMPTY` hash. pub fn code_hash(&self) -> B256 { diff --git a/crates/revm/src/db/states/account_status.rs b/crates/revm/src/db/states/account_status.rs index 5003ecaf19..d077caf0d0 100644 --- a/crates/revm/src/db/states/account_status.rs +++ b/crates/revm/src/db/states/account_status.rs @@ -15,27 +15,8 @@ pub enum AccountStatus { } impl AccountStatus { - /// Transition to other state while preserving invariance of this state. - /// - /// It this account was Destroyed and other account is not: - /// we should mark extended account as destroyed too. - /// and as other account had some changes, extended account - /// should be marked as DestroyedChanged. - /// - /// If both account are not destroyed and if this account is in memory: - /// this means that extended account is in memory too. - /// - /// Otherwise, if both are destroyed or other is destroyed: - /// set other status to extended account. - pub fn transition(&mut self, other: Self) { - *self = match (self.was_destroyed(), other.was_destroyed()) { - (true, false) => Self::DestroyedChanged, - (false, false) if *self == Self::InMemoryChange => Self::InMemoryChange, - _ => other, - }; - } /// Account is not modified and just loaded from database. - pub fn not_modified(&self) -> bool { + pub fn is_not_modified(&self) -> bool { matches!( self, AccountStatus::LoadedNotExisting @@ -56,7 +37,7 @@ impl AccountStatus { } /// This means storage is known, it can be newly created or storage got destroyed. - pub fn storage_known(&self) -> bool { + pub fn is_storage_known(&self) -> bool { matches!( self, AccountStatus::LoadedNotExisting @@ -70,9 +51,153 @@ impl AccountStatus { /// Account is modified but not destroyed. /// This means that some storage values can be found in both /// memory and database. - pub fn modified_but_not_destroyed(&self) -> bool { + pub fn is_modified_and_not_destroyed(&self) -> bool { matches!(self, AccountStatus::Changed | AccountStatus::InMemoryChange) } + + /// Returns the next account status on creation. + pub fn on_created(&self) -> AccountStatus { + match self { + // if account was destroyed previously just copy new info to it. + AccountStatus::DestroyedAgain + | AccountStatus::Destroyed + | AccountStatus::DestroyedChanged => AccountStatus::DestroyedChanged, + // if account is loaded from db. + AccountStatus::LoadedNotExisting + // Loaded empty eip161 to creates is not possible as CREATE2 was added after EIP-161 + | AccountStatus::LoadedEmptyEIP161 + | AccountStatus::Loaded + | AccountStatus::Changed + | AccountStatus::InMemoryChange => { + // If account is loaded and not empty this means that account has some balance. + // This means that account cannot be created. + // We are assuming that EVM did necessary checks before allowing account to be created. + AccountStatus::InMemoryChange + } + } + } + + /// Returns the next account status on touched empty account post state clear EIP (EIP-161). + /// + /// # Panics + /// + /// If current status is [AccountStatus::Loaded] or [AccountStatus::Changed]. + pub fn on_touched_empty_post_eip161(&self) -> AccountStatus { + match self { + // Account can be touched but not existing. The status should remain the same. + AccountStatus::LoadedNotExisting => AccountStatus::LoadedNotExisting, + // Account can be created empty and only then touched. + AccountStatus::InMemoryChange + | AccountStatus::Destroyed + | AccountStatus::LoadedEmptyEIP161 => AccountStatus::Destroyed, + // Transition to destroy the account. + AccountStatus::DestroyedAgain | AccountStatus::DestroyedChanged => { + AccountStatus::DestroyedAgain + } + // Account statuses considered unreachable. + AccountStatus::Loaded | AccountStatus::Changed => { + unreachable!("Wrong state transition, touch empty is not possible from {self:?}"); + } + } + } + + /// Returns the next account status on touched or created account pre state clear EIP (EIP-161). + /// Returns `None` if the account status didn't change. + /// + /// # Panics + /// + /// If current status is [AccountStatus::Loaded] or [AccountStatus::Changed]. + pub fn on_touched_created_pre_eip161(&self, had_no_info: bool) -> Option { + match self { + AccountStatus::LoadedEmptyEIP161 => None, + AccountStatus::DestroyedChanged => { + if had_no_info { + None + } else { + Some(AccountStatus::DestroyedChanged) + } + } + AccountStatus::Destroyed | AccountStatus::DestroyedAgain => { + Some(AccountStatus::DestroyedChanged) + } + AccountStatus::InMemoryChange | AccountStatus::LoadedNotExisting => { + Some(AccountStatus::InMemoryChange) + } + AccountStatus::Loaded | AccountStatus::Changed => { + unreachable!("Wrong state transition, touch crate is not possible from {self:?}") + } + } + } + + /// Returns the next account status on change. + pub fn on_changed(&self, had_no_nonce_and_code: bool) -> AccountStatus { + match self { + // If the account was loaded as not existing, promote it to changed. + // This account was likely created by a balance transfer. + AccountStatus::LoadedNotExisting => AccountStatus::InMemoryChange, + // Change on empty account, should transfer storage if there is any. + // There is possibility that there are storage entries inside db. + // That storage is used in merkle tree calculation before state clear EIP. + AccountStatus::LoadedEmptyEIP161 => AccountStatus::InMemoryChange, + // The account was loaded as existing. + AccountStatus::Loaded => { + if had_no_nonce_and_code { + // account is fully in memory + AccountStatus::InMemoryChange + } else { + // can be contract and some of storage slots can be present inside db. + AccountStatus::Changed + } + } + + // On change, the "changed" type account statuses are preserved. + // Any checks for empty accounts are done outside of this fn. + AccountStatus::Changed => AccountStatus::Changed, + AccountStatus::InMemoryChange => AccountStatus::InMemoryChange, + AccountStatus::DestroyedChanged => AccountStatus::DestroyedChanged, + + // If account is destroyed and then changed this means this is + // balance transfer. + AccountStatus::Destroyed | AccountStatus::DestroyedAgain => { + AccountStatus::DestroyedChanged + } + } + } + + /// Returns the next account status on selfdestruct. + pub fn on_selfdestructed(&self) -> AccountStatus { + match self { + // If account is created and selfdestructed in the same block, mark it as destroyed again. + // Note: there is no big difference between Destroyed and DestroyedAgain in this case, + // but was added for clarity. + AccountStatus::DestroyedChanged + | AccountStatus::DestroyedAgain + | AccountStatus::Destroyed => AccountStatus::DestroyedAgain, + + // Transition to destroyed status. + _ => AccountStatus::Destroyed, + } + } + + /// Transition to other state while preserving invariance of this state. + /// + /// It this account was Destroyed and other account is not: + /// we should mark extended account as destroyed too. + /// and as other account had some changes, extended account + /// should be marked as DestroyedChanged. + /// + /// If both account are not destroyed and if this account is in memory: + /// this means that extended account is in memory too. + /// + /// Otherwise, if both are destroyed or other is destroyed: + /// set other status to extended account. + pub fn transition(&mut self, other: Self) { + *self = match (self.was_destroyed(), other.was_destroyed()) { + (true, false) => Self::DestroyedChanged, + (false, false) if *self == Self::InMemoryChange => Self::InMemoryChange, + _ => other, + }; + } } #[cfg(test)] @@ -83,24 +208,24 @@ mod test { #[test] fn test_account_status() { // account not modified - assert!(AccountStatus::Loaded.not_modified()); - assert!(AccountStatus::LoadedEmptyEIP161.not_modified()); - assert!(AccountStatus::LoadedNotExisting.not_modified()); - assert!(!AccountStatus::Changed.not_modified()); - assert!(!AccountStatus::InMemoryChange.not_modified()); - assert!(!AccountStatus::Destroyed.not_modified()); - assert!(!AccountStatus::DestroyedChanged.not_modified()); - assert!(!AccountStatus::DestroyedAgain.not_modified()); + assert!(AccountStatus::Loaded.is_not_modified()); + assert!(AccountStatus::LoadedEmptyEIP161.is_not_modified()); + assert!(AccountStatus::LoadedNotExisting.is_not_modified()); + assert!(!AccountStatus::Changed.is_not_modified()); + assert!(!AccountStatus::InMemoryChange.is_not_modified()); + assert!(!AccountStatus::Destroyed.is_not_modified()); + assert!(!AccountStatus::DestroyedChanged.is_not_modified()); + assert!(!AccountStatus::DestroyedAgain.is_not_modified()); // we know full storage - assert!(!AccountStatus::LoadedEmptyEIP161.storage_known()); - assert!(AccountStatus::LoadedNotExisting.storage_known()); - assert!(AccountStatus::InMemoryChange.storage_known()); - assert!(AccountStatus::Destroyed.storage_known()); - assert!(AccountStatus::DestroyedChanged.storage_known()); - assert!(AccountStatus::DestroyedAgain.storage_known()); - assert!(!AccountStatus::Loaded.storage_known()); - assert!(!AccountStatus::Changed.storage_known()); + assert!(!AccountStatus::LoadedEmptyEIP161.is_storage_known()); + assert!(AccountStatus::LoadedNotExisting.is_storage_known()); + assert!(AccountStatus::InMemoryChange.is_storage_known()); + assert!(AccountStatus::Destroyed.is_storage_known()); + assert!(AccountStatus::DestroyedChanged.is_storage_known()); + assert!(AccountStatus::DestroyedAgain.is_storage_known()); + assert!(!AccountStatus::Loaded.is_storage_known()); + assert!(!AccountStatus::Changed.is_storage_known()); // account was destroyed assert!(!AccountStatus::LoadedEmptyEIP161.was_destroyed()); @@ -113,13 +238,13 @@ mod test { assert!(!AccountStatus::Changed.was_destroyed()); // account modified but not destroyed - assert!(AccountStatus::Changed.modified_but_not_destroyed()); - assert!(AccountStatus::InMemoryChange.modified_but_not_destroyed()); - assert!(!AccountStatus::Loaded.modified_but_not_destroyed()); - assert!(!AccountStatus::LoadedEmptyEIP161.modified_but_not_destroyed()); - assert!(!AccountStatus::LoadedNotExisting.modified_but_not_destroyed()); - assert!(!AccountStatus::Destroyed.modified_but_not_destroyed()); - assert!(!AccountStatus::DestroyedChanged.modified_but_not_destroyed()); - assert!(!AccountStatus::DestroyedAgain.modified_but_not_destroyed()); + assert!(AccountStatus::Changed.is_modified_and_not_destroyed()); + assert!(AccountStatus::InMemoryChange.is_modified_and_not_destroyed()); + assert!(!AccountStatus::Loaded.is_modified_and_not_destroyed()); + assert!(!AccountStatus::LoadedEmptyEIP161.is_modified_and_not_destroyed()); + assert!(!AccountStatus::LoadedNotExisting.is_modified_and_not_destroyed()); + assert!(!AccountStatus::Destroyed.is_modified_and_not_destroyed()); + assert!(!AccountStatus::DestroyedChanged.is_modified_and_not_destroyed()); + assert!(!AccountStatus::DestroyedAgain.is_modified_and_not_destroyed()); } } diff --git a/crates/revm/src/db/states/bundle_account.rs b/crates/revm/src/db/states/bundle_account.rs index 38c0831c76..1cc873e384 100644 --- a/crates/revm/src/db/states/bundle_account.rs +++ b/crates/revm/src/db/states/bundle_account.rs @@ -57,7 +57,7 @@ impl BundleAccount { let slot = self.storage.get(&slot).map(|s| s.present_value); if slot.is_some() { slot - } else if self.status.storage_known() { + } else if self.status.is_storage_known() { Some(U256::ZERO) } else { None diff --git a/crates/revm/src/db/states/cache_account.rs b/crates/revm/src/db/states/cache_account.rs index c6769eba99..2d6232766c 100644 --- a/crates/revm/src/db/states/cache_account.rs +++ b/crates/revm/src/db/states/cache_account.rs @@ -2,7 +2,7 @@ use super::{ plain_account::PlainStorage, AccountStatus, BundleAccount, PlainAccount, StorageWithOriginalValues, TransitionAccount, }; -use revm_interpreter::primitives::{AccountInfo, KECCAK_EMPTY, U256}; +use revm_interpreter::primitives::{AccountInfo, U256}; use revm_precompile::HashMap; /// Cache account contains plain state that gets updated @@ -115,31 +115,13 @@ impl CacheAccount { ) -> Option { let previous_status = self.status; - self.status = match self.status { - AccountStatus::DestroyedChanged => { - if self - .account - .as_ref() - .map(|a| a.info.is_empty()) - .unwrap_or_default() - { - return None; - } - AccountStatus::DestroyedChanged - } - AccountStatus::Destroyed | AccountStatus::DestroyedAgain => { - AccountStatus::DestroyedChanged - } - AccountStatus::LoadedEmptyEIP161 => { - return None; - } - AccountStatus::InMemoryChange | AccountStatus::LoadedNotExisting => { - AccountStatus::InMemoryChange - } - AccountStatus::Loaded | AccountStatus::Changed => { - unreachable!("Wrong state transition, touch crate is not possible from {self:?}") - } - }; + let had_no_info = self + .account + .as_ref() + .map(|a| a.info.is_empty()) + .unwrap_or_default(); + self.status = self.status.on_touched_created_pre_eip161(had_no_info)?; + let plain_storage = storage.iter().map(|(k, v)| (*k, v.present_value)).collect(); let previous_info = self.account.take().map(|a| a.info); @@ -165,27 +147,8 @@ impl CacheAccount { let previous_info = self.account.take().map(|acc| acc.info); // Set account state to Destroyed as we need to clear the storage if it exist. - self.status = match self.status { - AccountStatus::InMemoryChange - | AccountStatus::Destroyed - | AccountStatus::LoadedEmptyEIP161 => { - // account can be created empty then touched. - AccountStatus::Destroyed - } - AccountStatus::LoadedNotExisting => { - // account can be touched but not existing. - // This is a noop. - AccountStatus::LoadedNotExisting - } - AccountStatus::DestroyedAgain | AccountStatus::DestroyedChanged => { - // do nothing - AccountStatus::DestroyedAgain - } - _ => { - // do nothing - unreachable!("Wrong state transition, touch empty is not possible from {self:?}"); - } - }; + self.status = self.status.on_touched_empty_post_eip161(); + if matches!( previous_status, AccountStatus::LoadedNotExisting @@ -213,19 +176,7 @@ impl CacheAccount { let previous_info = self.account.take().map(|a| a.info); let previous_status = self.status; - self.status = match self.status { - AccountStatus::DestroyedChanged - | AccountStatus::DestroyedAgain - | AccountStatus::Destroyed => { - // mark as destroyed again, this can happen if account is created and - // then selfdestructed in same block. - // Note: there is no big difference between Destroyed and DestroyedAgain - // in this case, but was added for clarity. - AccountStatus::DestroyedAgain - } - - _ => AccountStatus::Destroyed, - }; + self.status = self.status.on_selfdestructed(); if previous_status == AccountStatus::LoadedNotExisting { // transitions for account loaded as not existing. @@ -257,24 +208,7 @@ impl CacheAccount { .map(|(k, s)| (*k, s.present_value)) .collect(); - self.status = match self.status { - // if account was destroyed previously just copy new info to it. - AccountStatus::DestroyedAgain - | AccountStatus::Destroyed - | AccountStatus::DestroyedChanged => AccountStatus::DestroyedChanged, - // if account is loaded from db. - AccountStatus::LoadedNotExisting - // Loaded empty eip161 to creates is not possible as CREATE2 was added after EIP-161 - | AccountStatus::LoadedEmptyEIP161 - | AccountStatus::Loaded - | AccountStatus::Changed - | AccountStatus::InMemoryChange => { - // If account is loaded and not empty this means that account has some balance. - // This means that account cannot be created. - // We are assuming that EVM did necessary checks before allowing account to be created. - AccountStatus::InMemoryChange - } - }; + self.status = self.status.on_created(); let transition_account = TransitionAccount { info: Some(new_info.clone()), status: self.status, @@ -314,24 +248,11 @@ impl CacheAccount { let output = change(&mut account.info); self.account = Some(account); - self.status = match self.status { - AccountStatus::Loaded => { - // Account that have nonce zero and empty code hash is considered to be fully in memory. - if previous_info.as_ref().map(|a| (a.code_hash, a.nonce)) == Some((KECCAK_EMPTY, 0)) - { - AccountStatus::InMemoryChange - } else { - AccountStatus::Changed - } - } - AccountStatus::LoadedNotExisting - | AccountStatus::LoadedEmptyEIP161 - | AccountStatus::InMemoryChange => AccountStatus::InMemoryChange, - AccountStatus::Changed => AccountStatus::Changed, - AccountStatus::Destroyed - | AccountStatus::DestroyedAgain - | AccountStatus::DestroyedChanged => AccountStatus::DestroyedChanged, - }; + let had_no_nonce_and_code = previous_info + .as_ref() + .map(AccountInfo::has_no_code_and_nonce) + .unwrap_or_default(); + self.status = self.status.on_changed(had_no_nonce_and_code); ( output, @@ -376,47 +297,11 @@ impl CacheAccount { storage: this_storage, }; - self.status = match self.status { - AccountStatus::Loaded => { - if previous_info.as_ref().map(|a| (a.code_hash, a.nonce)) == Some((KECCAK_EMPTY, 0)) - { - // account is fully in memory - AccountStatus::InMemoryChange - } else { - // can be contract and some of storage slots can be present inside db. - AccountStatus::Changed - } - } - AccountStatus::Changed => { - // Update to new changed state. - AccountStatus::Changed - } - AccountStatus::InMemoryChange => { - // promote to NewChanged. - // Check if account is empty is done outside of this fn. - AccountStatus::InMemoryChange - } - AccountStatus::DestroyedChanged => { - // have same state - AccountStatus::DestroyedChanged - } - AccountStatus::LoadedEmptyEIP161 => { - // Change on empty account, should transfer storage if there is any. - // There is possibility that there are storage inside db. - // That storage is used in merkle tree calculation before state clear EIP. - AccountStatus::InMemoryChange - } - AccountStatus::LoadedNotExisting => { - // if it is loaded not existing and then changed - // This means this is balance transfer that created the account. - AccountStatus::InMemoryChange - } - AccountStatus::Destroyed | AccountStatus::DestroyedAgain => { - // If account is destroyed and then changed this means this is - // balance transfer. - AccountStatus::DestroyedChanged - } - }; + let had_no_nonce_and_code = previous_info + .as_ref() + .map(AccountInfo::has_no_code_and_nonce) + .unwrap_or_default(); + self.status = self.status.on_changed(had_no_nonce_and_code); self.account = Some(changed_account); TransitionAccount { diff --git a/crates/revm/src/db/states/state.rs b/crates/revm/src/db/states/state.rs index 750725dc4c..cbfc53f201 100644 --- a/crates/revm/src/db/states/state.rs +++ b/crates/revm/src/db/states/state.rs @@ -243,7 +243,7 @@ impl Database for State { // Note that storage from bundle is already loaded with account. if let Some(account) = self.cache.accounts.get_mut(&address) { // account will always be some, but if it is not, U256::ZERO will be returned. - let is_storage_known = account.status.storage_known(); + let is_storage_known = account.status.is_storage_known(); Ok(account .account .as_mut()