feat: extend to reverts

crates/primitives/src/state.rs

@@ -260,8 +260,16 @@ impl AccountInfo {
         }
     }
 
-    /// Returns account info without the code.
-    pub fn without_code(&self) -> Self {
+    /// Returns a copy of this account with the [`Bytecode`] removed. This is
+    /// useful when creating journals or snapshots of the state, where it is
+    /// desirable to store the code blobs elsewhere.
+    ///
+    /// ## Note
+    ///
+    /// This is distinct from [`AccountInfo::without_code`] in that it returns
+    /// a new `AccountInfo` instance with the code removed.
+    /// [`AccountInfo::without_code`] will modify and return the same instance.
+    pub fn copy_without_code(&self) -> Self {
         Self {
             balance: self.balance,
             nonce: self.nonce,
@@ -270,6 +278,20 @@ impl AccountInfo {
         }
     }
 
+    /// Strip the [`Bytecode`] from this account and drop it. This is
+    /// useful when creating journals or snapshots of the state, where it is
+    /// desirable to store the code blobs elsewhere.
+    ///
+    /// ## Note
+    ///
+    /// This is distinct from [`AccountInfo::copy_without_code`] in that it
+    /// modifies the account in place. [`AccountInfo::copy_without_code`]
+    /// will copy the non-code fields and return a new `AccountInfo` instance.
+    pub fn without_code(mut self) -> Self {
+        self.take_bytecode();
+        self
+    }
+
     /// Returns if an account is empty.
     ///
     /// An account is empty if the following conditions are met.

crates/revm/src/db/states/bundle_state.rs

@@ -593,7 +593,7 @@ impl BundleState {
             // append account info if it is changed.
             let was_destroyed = account.was_destroyed();
             if is_value_known.is_not_known() || account.is_info_changed() {
-                let info = account.info.as_ref().map(AccountInfo::without_code);
+                let info = account.info.as_ref().map(AccountInfo::copy_without_code);
                 accounts.push((*address, info));
             }
 
@@ -634,7 +634,8 @@ impl BundleState {
             .contracts
             .iter()
             // remove empty bytecodes
-            .filter_map(|(b, code)| (*b != KECCAK_EMPTY).then_some((*b, code.clone())))
+            .filter(|(b, _)| **b != KECCAK_EMPTY)
+            .map(|(b, code)| (*b, code.clone()))
             .collect::<Vec<_>>();
         StateChangeset {
             accounts,
@@ -643,74 +644,32 @@ impl BundleState {
         }
     }
 
-    /// Consume the bundle state and return plain state.
+    /// Convert the bundle state into a [`StateChangeset`].
+    #[deprecated = "Use `to_plain_state` instead"]
     pub fn into_plain_state(self, is_value_known: OriginalValuesKnown) -> StateChangeset {
-        // pessimistically pre-allocate assuming _all_ accounts changed.
-        let state_len = self.state.len();
-        let mut accounts = Vec::with_capacity(state_len);
-        let mut storage = Vec::with_capacity(state_len);
-
-        for (address, account) in self.state {
-            // append account info if it is changed.
-            let was_destroyed = account.was_destroyed();
-            if is_value_known.is_not_known() || account.is_info_changed() {
-                let info = account.info.as_ref().map(AccountInfo::without_code);
-                accounts.push((address, info));
-            }
-
-            // append storage changes
-
-            // NOTE: Assumption is that revert is going to remove whole plain storage from
-            // database so we can check if plain state was wiped or not.
-            let mut account_storage_changed = Vec::with_capacity(account.storage.len());
-
-            for (key, slot) in account.storage {
-                // If storage was destroyed that means that storage was wiped.
-                // In that case we need to check if present storage value is different then ZERO.
-                let destroyed_and_not_zero = was_destroyed && !slot.present_value.is_zero();
-
-                // If account is not destroyed check if original values was changed,
-                // so we can update it.
-                let not_destroyed_and_changed = !was_destroyed && slot.is_changed();
-
-                if is_value_known.is_not_known()
-                    || destroyed_and_not_zero
-                    || not_destroyed_and_changed
-                {
-                    account_storage_changed.push((key, slot.present_value));
-                }
-            }
+        self.to_plain_state(is_value_known)
+    }
 
-            if !account_storage_changed.is_empty() || was_destroyed {
-                // append storage changes to account.
-                storage.push(PlainStorageChangeset {
-                    address,
-                    wipe_storage: was_destroyed,
-                    storage: account_storage_changed,
-                });
-            }
-        }
-        let contracts = self
-            .contracts
-            .into_iter()
-            // remove empty bytecodes
-            .filter(|(b, _)| *b != KECCAK_EMPTY)
-            .collect::<Vec<_>>();
-        StateChangeset {
-            accounts,
-            storage,
-            contracts,
-        }
+    /// Generate a [`StateChangeset`] and [`PlainStateReverts`] from the bundle
+    /// state.
+    pub fn to_plain_state_and_reverts(
+        &self,
+        is_value_known: OriginalValuesKnown,
+    ) -> (StateChangeset, PlainStateReverts) {
+        (
+            self.to_plain_state(is_value_known),
+            self.reverts.to_plain_state_reverts(),
+        )
     }
 
-    /// Consume the bundle state and split it into reverts and plain state.
+    /// Consume the bundle state and split it into a [`StateChangeset`] and a
+    /// [`PlainStateReverts`].
+    #[deprecated = "Use `to_plain_state_and_reverts` instead"]
     pub fn into_plain_state_and_reverts(
-        mut self,
+        self,
         is_value_known: OriginalValuesKnown,
     ) -> (StateChangeset, PlainStateReverts) {
-        let reverts = self.take_all_reverts();
-        let plain_state = self.into_plain_state(is_value_known);
-        (plain_state, reverts.into_plain_state_reverts())
+        self.to_plain_state_and_reverts(is_value_known)
     }
 
     /// Extend the bundle with other state

crates/revm/src/db/states/reverts.rs

@@ -43,26 +43,34 @@ impl Reverts {
         self.0.extend(other.0);
     }
 
-    /// Consume reverts and create plain state reverts.
+    /// Generate a [`PlainStateReverts`].
     ///
     /// Note that account are sorted by address.
-    pub fn into_plain_state_reverts(mut self) -> PlainStateReverts {
+    pub fn to_plain_state_reverts(&self) -> PlainStateReverts {
         let mut state_reverts = PlainStateReverts::with_capacity(self.0.len());
-        for reverts in self.0.drain(..) {
+        for reverts in &self.0 {
             // pessimistically pre-allocate assuming _all_ accounts changed.
             let mut accounts = Vec::with_capacity(reverts.len());
             let mut storage = Vec::with_capacity(reverts.len());
-            for (address, revert_account) in reverts.into_iter() {
-                match revert_account.account {
-                    AccountInfoRevert::RevertTo(acc) => accounts.push((address, Some(acc))),
-                    AccountInfoRevert::DeleteIt => accounts.push((address, None)),
+            for (address, revert_account) in reverts {
+                match &revert_account.account {
+                    AccountInfoRevert::RevertTo(acc) => {
+                        // cloning is cheap, because account info has 3 small
+                        // fields and a Bytes
+                        accounts.push((*address, Some(acc.clone())))
+                    }
+                    AccountInfoRevert::DeleteIt => accounts.push((*address, None)),
                     AccountInfoRevert::DoNothing => (),
                 }
                 if revert_account.wipe_storage || !revert_account.storage.is_empty() {
                     storage.push(PlainStorageRevert {
-                        address,
+                        address: *address,
                         wiped: revert_account.wipe_storage,
-                        storage_revert: revert_account.storage.into_iter().collect::<Vec<_>>(),
+                        storage_revert: revert_account
+                            .storage
+                            .iter()
+                            .map(|(k, v)| (*k, *v))
+                            .collect::<Vec<_>>(),
                     });
                 }
             }
@@ -71,6 +79,14 @@ impl Reverts {
         }
         state_reverts
     }
+
+    /// Consume reverts and create [`PlainStateReverts`].
+    ///
+    /// Note that account are sorted by address.
+    #[deprecated = "Use `to_plain_state_reverts` instead"]
+    pub fn into_plain_state_reverts(self) -> PlainStateReverts {
+        self.to_plain_state_reverts()
+    }
 }
 
 /// Assumption is that Revert can return full state from any future state to any past state.