diff --git a/Cargo.lock b/Cargo.lock index 16f089b930..417733ae61 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -148,6 +148,16 @@ version = "1.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" +[[package]] +name = "bitflags" +version = "2.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24a6904aef64d73cf10ab17ebace7befb918b82164785cb89907993be7f83813" +dependencies = [ + "arbitrary", + "serde", +] + [[package]] name = "bitvec" version = "1.0.1" @@ -227,7 +237,7 @@ checksum = "a0610544180c38b88101fecf2dd634b174a62eef6946f84dfc6a7127512b381c" dependencies = [ "ansi_term", "atty", - "bitflags", + "bitflags 1.3.2", "strsim", "textwrap", "unicode-width", @@ -1482,7 +1492,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "29f1b898011ce9595050a68e60f90bad083ff2987a695a42357134c8381fba70" dependencies = [ "bit-set", - "bitflags", + "bitflags 1.3.2", "byteorder", "lazy_static", "num-traits", @@ -1588,7 +1598,7 @@ version = "0.3.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "567664f262709473930a4bf9e51bf2ebf3348f2e748ccc50dea20646858f8f29" dependencies = [ - "bitflags", + "bitflags 1.3.2", ] [[package]] @@ -1703,6 +1713,7 @@ version = "1.1.2" dependencies = [ "arbitrary", "auto_impl", + "bitflags 2.2.1", "bitvec", "bytes", "derive_more", @@ -1854,7 +1865,7 @@ version = "0.37.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2aae838e49b3d63e9274e1c01833cc8139d3fec468c3b84688c628f44b1ae11d" dependencies = [ - "bitflags", + "bitflags 1.3.2", "errno", "io-lifetimes", "libc", diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index 1b1d6037e6..b9ba3953e8 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -14,11 +14,13 @@ bytes = { version = "1.4", default-features = false } hashbrown = { version = "0.13" } hex = { version = "0.4", default-features = false } primitive-types = { version = "0.12", default-features = false } -rlp = { version = "0.5", default-features = false } # used for create2 address calculation +rlp = { version = "0.5", default-features = false } # used for create2 address calculation ruint = { version = "1.8.0", features = ["primitive-types", "rlp"] } auto_impl = "1.0" bitvec = { version = "1", default-features = false, features = ["alloc"] } +bitflags = { version = "2.2.1", default-features = false} + # bits B256 B160 crate fixed-hash = { version = "0.8", default-features = false, features = [ "rustc-hex", @@ -66,14 +68,15 @@ optional_block_gas_limit = [] optional_eip3607 = [] optional_gas_refund = [] optional_no_base_fee = [] -std = ["bytes/std", "rlp/std", "hex/std", "bitvec/std"] +std = ["bytes/std", "rlp/std", "hex/std", "bitvec/std", "bitflags/std"] serde = [ "dep:serde", "hex/serde", "hashbrown/serde", "ruint/serde", "bytes/serde", - "bitvec/serde" + "bitvec/serde", + "bitflags/serde", ] arbitrary = [ "std", @@ -82,4 +85,5 @@ arbitrary = [ "dep:arbitrary", "dep:proptest", "dep:proptest-derive", -] \ No newline at end of file + "bitflags/arbitrary", +] diff --git a/crates/primitives/src/state.rs b/crates/primitives/src/state.rs index c05b8f1c98..6a69b93044 100644 --- a/crates/primitives/src/state.rs +++ b/crates/primitives/src/state.rs @@ -1,39 +1,107 @@ use crate::{Bytecode, B160, B256, KECCAK_EMPTY, U256}; +use bitflags::bitflags; use hashbrown::HashMap; -#[derive(Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Clone, Eq, PartialEq, Default)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Account { /// Balance of the account. pub info: AccountInfo, /// storage cache pub storage: HashMap, - /// If account is newly created, we will not ask database for storage values - pub storage_cleared: bool, - /// if account is destroyed it will be scheduled for removal. - pub is_destroyed: bool, - /// if account is touched - pub is_touched: bool, - /// used only for pre spurious dragon hardforks where existing and empty were two separate states. - /// it became same state after EIP-161: State trie clearing - pub is_not_existing: bool, + // Account status flags. + pub status: AccountStatus, +} + +// The `bitflags!` macro generates `struct`s that manage a set of flags. +bitflags! { + #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] + #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] + #[cfg_attr(feature = "serde", serde(transparent))] + pub struct AccountStatus: u8 { + /// When account is loaded but not touched or interacted with. + const Loaded = 0b00000000; + /// When account is newly created we will not access database + /// to fetch storage values + const Created = 0b00000001; + /// If account is marked for self destruct. + const SelfDestructed = 0b00000010; + /// Only when account is marked as touched we will save it to database. + const Touched = 0b00000100; + /// used only for pre spurious dragon hardforks where existing and empty were two separate states. + /// it became same state after EIP-161: State trie clearing + const LoadedAsNotExisting = 0b0001000; + } +} + +impl Default for AccountStatus { + fn default() -> Self { + Self::Loaded + } } pub type State = HashMap; pub type Storage = HashMap; impl Account { + /// Mark account as self destructed. + pub fn mark_selfdestruct(&mut self) { + self.status |= AccountStatus::SelfDestructed; + } + + /// Unmark account as self destructed. + pub fn unmark_selfdestruct(&mut self) { + self.status -= AccountStatus::SelfDestructed; + } + + /// Is account marked for self destruct. + pub fn is_selfdestructed(&self) -> bool { + self.status.contains(AccountStatus::SelfDestructed) + } + + /// Mark account as touched + pub fn mark_touch(&mut self) { + self.status |= AccountStatus::Touched; + } + + /// Unmark the touch flag. + pub fn unmark_touch(&mut self) { + self.status -= AccountStatus::Touched; + } + + /// If account status is marked as touched. + pub fn is_touched(&self) -> bool { + self.status.contains(AccountStatus::Touched) + } + + /// Mark account as newly created. + pub fn mark_created(&mut self) { + self.status |= AccountStatus::Created; + } + + /// Is account loaded as not existing from database + /// This is needed for pre spurious dragon hardforks where + /// existing and empty were two separate states. + pub fn is_loaded_as_not_existing(&self) -> bool { + self.status.contains(AccountStatus::LoadedAsNotExisting) + } + + /// Is account newly created in this transaction. + pub fn is_newly_created(&self) -> bool { + self.status.contains(AccountStatus::Created) + } + + /// Is account empty, check if nonce and balance are zero and code is empty. pub fn is_empty(&self) -> bool { self.info.is_empty() } + + /// Create new account and mark it as non existing. pub fn new_not_existing() -> Self { Self { info: AccountInfo::default(), storage: HashMap::new(), - storage_cleared: false, - is_destroyed: false, - is_touched: false, - is_not_existing: true, + status: AccountStatus::LoadedAsNotExisting, } } } @@ -43,10 +111,7 @@ impl From for Account { Self { info, storage: HashMap::new(), - storage_cleared: false, - is_destroyed: false, - is_touched: false, - is_not_existing: false, + status: AccountStatus::Loaded, } } } @@ -142,3 +207,28 @@ impl AccountInfo { } } } + +#[cfg(test)] +mod tests { + use crate::Account; + + #[test] + pub fn account_state() { + let mut account = Account::default(); + + assert!(!account.is_touched()); + assert!(!account.is_selfdestructed()); + + account.mark_touch(); + assert!(account.is_touched()); + assert!(!account.is_selfdestructed()); + + account.mark_selfdestruct(); + assert!(account.is_touched()); + assert!(account.is_selfdestructed()); + + account.unmark_selfdestruct(); + assert!(account.is_touched()); + assert!(!account.is_selfdestructed()); + } +} diff --git a/crates/revm/src/db/in_memory_db.rs b/crates/revm/src/db/in_memory_db.rs index 4aada7dc7a..047055e6e6 100644 --- a/crates/revm/src/db/in_memory_db.rs +++ b/crates/revm/src/db/in_memory_db.rs @@ -175,19 +175,20 @@ impl CacheDB { impl DatabaseCommit for CacheDB { fn commit(&mut self, changes: HashMap) { for (address, mut account) in changes { - if account.is_destroyed { + if account.is_selfdestructed() { let db_account = self.accounts.entry(address).or_default(); db_account.storage.clear(); db_account.account_state = AccountState::NotExisting; db_account.info = AccountInfo::default(); continue; } + let is_newly_created = account.is_newly_created(); self.insert_contract(&mut account.info); let db_account = self.accounts.entry(address).or_default(); db_account.info = account.info; - db_account.account_state = if account.storage_cleared { + db_account.account_state = if is_newly_created { db_account.storage.clear(); AccountState::StorageCleared } else if db_account.account_state.is_storage_cleared() { diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index a1417ef225..88d443a734 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -544,32 +544,15 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, let checkpoint = self.data.journaled_state.checkpoint(); // Create contract account and check for collision - match self.data.journaled_state.create_account( - created_address, - self.precompiles.contains(&created_address), - self.data.db, - ) { - Ok(false) => { - self.data.journaled_state.checkpoint_revert(checkpoint); - return self.create_end( - inputs, - InstructionResult::CreateCollision, - ret, - gas, - Bytes::new(), - ); - } - Err(err) => { - self.data.error = Some(err); - return self.create_end( - inputs, - InstructionResult::FatalExternalError, - ret, - gas, - Bytes::new(), - ); - } - Ok(true) => (), + if !self.data.journaled_state.create_account(created_address) { + self.data.journaled_state.checkpoint_revert(checkpoint); + return self.create_end( + inputs, + InstructionResult::CreateCollision, + ret, + gas, + Bytes::new(), + ); } // Transfer value to contract address @@ -692,7 +675,6 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, AnalysisKind::Check => Bytecode::new_raw(bytes.clone()).to_checked(), AnalysisKind::Analyse => to_analysed(Bytecode::new_raw(bytes.clone())), }; - self.data .journaled_state .set_code(created_address, bytecode); diff --git a/crates/revm/src/journaled_state.rs b/crates/revm/src/journaled_state.rs index e396f4164c..cebd3bd4eb 100644 --- a/crates/revm/src/journaled_state.rs +++ b/crates/revm/src/journaled_state.rs @@ -79,6 +79,13 @@ pub struct JournalCheckpoint { } impl JournaledState { + /// Create new JournaledState. + /// + /// num_of_precompiles is used to determine how many precompiles are there. + /// Assumption is that number of N first addresses are precompiles (exclusing 0x00..00) + /// + /// Note: This function will journal state after Spurious Dragon fork. + /// And will not take into account if account is not existing or empty. pub fn new(num_of_precompiles: usize) -> JournaledState { Self { state: HashMap::new(), @@ -90,16 +97,23 @@ impl JournaledState { } } + /// Same as [`Self::new`] but will journal state before Spurious Dragon fork. + /// + /// Note: Before Spurious Dragon fork empty and not existing accounts were treated differently. pub fn new_legacy(num_of_precompiles: usize) -> JournaledState { let mut journal = Self::new(num_of_precompiles); journal.is_before_spurious_dragon = true; journal } + /// Return reference to state. pub fn state(&mut self) -> &mut State { &mut self.state } + /// Mark account as touched as only touched accounts will be added to state. + /// This is expecially important for state clear where touched empty accounts needs to + /// be removed from state. pub fn touch(&mut self, address: &B160) { if let Some(account) = self.state.get_mut(address) { Self::touch_account(self.journal.last_mut().unwrap(), address, account); @@ -107,9 +121,9 @@ impl JournaledState { } fn touch_account(journal: &mut Vec, address: &B160, account: &mut Account) { - if !account.is_touched { + if !account.is_touched() { journal.push(JournalEntry::AccountTouched { address: *address }); - account.is_touched = true; + account.mark_touch(); } } @@ -119,7 +133,7 @@ impl JournaledState { let state = state .into_iter() - .filter(|(_, account)| account.is_touched) + .filter(|(_, account)| account.is_touched()) .collect(); let logs = mem::take(&mut self.logs); @@ -217,51 +231,48 @@ impl JournaledState { Ok((from_is_cold, to_is_cold)) } - /// return if it has collision of addresses - pub fn create_account( - &mut self, - address: B160, - is_precompile: bool, - db: &mut DB, - ) -> Result { - let (acc, _) = self.load_code(address, db)?; + /// Check if new account has collision with existing account. + /// Return false if collision is detected. + pub fn create_account(&mut self, address: B160) -> bool { + // Safety: this function is called only from REVM::crate_inner + // and it load account before calling this function. + + let account = self.state.get_mut(&address).unwrap(); // Check collision. Bytecode needs to be empty. - if let Some(ref code) = acc.info.code { - if !code.is_empty() { - return Ok(false); - } + if account.info.code_hash != KECCAK_EMPTY { + return false; } // Check collision. Nonce is not zero - if acc.info.nonce != 0 { - return Ok(false); + if account.info.nonce != 0 { + return false; } // Check collision. New account address is precompile. - if is_precompile { - return Ok(false); + if is_precompile(address, self.num_of_precompiles) { + return false; } - acc.storage_cleared = true; + // set account status to created. + account.mark_created(); + + // Bytecode is initially empty and after constructor is executed and all checks pass + // the bytecode will be set. + account.info.code_hash = KECCAK_EMPTY; + account.info.code = None; // Set all storages to default value. They need to be present to act as accessed slots in access list. // it shouldn't be possible for them to have different values then zero as code is not existing for this account // , but because tests can change that assumption we are doing it. let empty = StorageSlot::default(); - acc.storage + account + .storage .iter_mut() .for_each(|(_, slot)| *slot = empty.clone()); - acc.info.code_hash = KECCAK_EMPTY; - acc.info.code = None; + // touch account. This is important as for pre SpuriousDragon there is possibility + Self::touch_account(self.journal.last_mut().unwrap(), &address, account); - if !acc.is_touched { - acc.is_touched = true; - self.journal - .last_mut() - .unwrap() - .push(JournalEntry::AccountTouched { address }); - } - Ok(true) + true } fn journal_revert( @@ -283,7 +294,8 @@ impl JournaledState { if is_spurious_dragon_enabled && address == PRECOMPILE3 { continue; } - state.get_mut(&address).unwrap().is_touched = false; + // remove touched status + state.get_mut(&address).unwrap().unmark_touch(); } JournalEntry::AccountDestroyed { address, @@ -292,7 +304,15 @@ impl JournaledState { had_balance, } => { let account = state.get_mut(&address).unwrap(); - account.is_destroyed = was_destroyed; + // set previous ste of selfdestructed flag. as there could be multiple + // selfdestructs in one transaction. + if was_destroyed { + // flag is still selfdestructed + account.mark_selfdestruct(); + } else { + // flag that is not selfdestructed + account.unmark_selfdestruct(); + } account.info.balance += had_balance; if address != target { @@ -372,13 +392,15 @@ impl JournaledState { // transfer all the balance let acc = self.state.get_mut(&address).unwrap(); let balance = mem::take(&mut acc.info.balance); - let previously_destroyed = acc.is_destroyed; - acc.is_destroyed = true; - // In case that target and destroyed addresses are same, balance will be lost. + let previously_destroyed = acc.is_selfdestructed(); + acc.mark_selfdestruct(); + + // NOTE: In case that target and destroyed addresses are same, balance will be lost. // ref: https://github.com/ethereum/go-ethereum/blob/141cd425310b503c5678e674a8c3872cf46b7086/core/vm/instructions.go#L832-L833 // https://github.com/ethereum/go-ethereum/blob/141cd425310b503c5678e674a8c3872cf46b7086/core/state/statedb.go#L449 if address != target { let target_account = self.state.get_mut(&target).unwrap(); + // touch target account Self::touch_account(self.journal.last_mut().unwrap(), &target, target_account); target_account.info.balance += balance; } @@ -437,10 +459,12 @@ impl JournaledState { db: &mut DB, ) -> Result<(bool, bool), DB::Error> { let is_before_spurious_dragon = self.is_before_spurious_dragon; - let (acc, is_cold) = self.load_code(address, db)?; + let (acc, is_cold) = self.load_account(address, db)?; let exist = if is_before_spurious_dragon { - !acc.is_not_existing || acc.is_touched + let is_existing = !acc.is_loaded_as_not_existing(); + let is_touched = acc.is_touched(); + is_existing || is_touched } else { !acc.is_empty() }; @@ -473,11 +497,12 @@ impl JournaledState { db: &mut DB, ) -> Result<(U256, bool), DB::Error> { let account = self.state.get_mut(&address).unwrap(); // asume acc is hot + let is_newly_created = account.is_newly_created(); let load = match account.storage.entry(key) { Entry::Occupied(occ) => (occ.get().present_value, false), Entry::Vacant(vac) => { // if storage was cleared, we dont need to ping db. - let value = if account.storage_cleared { + let value = if is_newly_created { U256::ZERO } else { db.storage(address, key)? @@ -540,6 +565,8 @@ impl JournaledState { } } +/// Check if address is precompile by having assumption +/// that precompiles are in range of 1 to N. fn is_precompile(address: B160, num_of_precompiles: usize) -> bool { if !address[..18].iter().all(|i| *i == 0) { return false;