From b5b839ad69b989e7903bd1a07e40facf135657f9 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 17 Sep 2024 13:58:58 -0400 Subject: [PATCH] feat: extend to reverts --- crates/primitives/src/state.rs | 26 ++++++- crates/revm/src/db/states/bundle_state.rs | 85 ++++++----------------- crates/revm/src/db/states/reverts.rs | 34 ++++++--- 3 files changed, 71 insertions(+), 74 deletions(-) diff --git a/crates/primitives/src/state.rs b/crates/primitives/src/state.rs index dd93763608..d814450621 100644 --- a/crates/primitives/src/state.rs +++ b/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. diff --git a/crates/revm/src/db/states/bundle_state.rs b/crates/revm/src/db/states/bundle_state.rs index f37c94ac72..c2d27a3d58 100644 --- a/crates/revm/src/db/states/bundle_state.rs +++ b/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::>(); 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::>(); - 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 diff --git a/crates/revm/src/db/states/reverts.rs b/crates/revm/src/db/states/reverts.rs index 4d8d3f402d..b6aa9dfff7 100644 --- a/crates/revm/src/db/states/reverts.rs +++ b/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::>(), + storage_revert: revert_account + .storage + .iter() + .map(|(k, v)| (*k, *v)) + .collect::>(), }); } } @@ -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.