feat: Introduce account status as bitflag inside JournalState (#477)

* feat: Introduce bytefield inside JournalState

* Add more functions and do some cleanup

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",
-]
+    "bitflags/arbitrary",
+]

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<U256, StorageSlot>,
-    /// 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<B160, Account>;
 pub type Storage = HashMap<U256, StorageSlot>;
 
 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<AccountInfo> 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());
+    }
+}

crates/revm/src/db/in_memory_db.rs

@@ -175,19 +175,20 @@ impl<ExtDB: DatabaseRef> CacheDB<ExtDB> {
 impl<ExtDB: DatabaseRef> DatabaseCommit for CacheDB<ExtDB> {
     fn commit(&mut self, changes: HashMap<B160, Account>) {
         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() {

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);