From fa92da627dc70438414a55910bbb18f5f5818fa2 Mon Sep 17 00:00:00 2001 From: James Prestwich Date: Wed, 18 Sep 2024 06:07:33 -0400 Subject: [PATCH] feat: to_plain_state (#1778) * feat: to_plain_state * feat: extend to reverts --- crates/primitives/src/state.rs | 28 ++++++++++++- crates/revm/src/db/states/bundle_state.rs | 49 +++++++++++++++------- crates/revm/src/db/states/plain_account.rs | 2 +- crates/revm/src/db/states/reverts.rs | 34 +++++++++++---- 4 files changed, 88 insertions(+), 25 deletions(-) diff --git a/crates/primitives/src/state.rs b/crates/primitives/src/state.rs index 35a3aab603..c42f5588aa 100644 --- a/crates/primitives/src/state.rs +++ b/crates/primitives/src/state.rs @@ -272,7 +272,33 @@ impl AccountInfo { } } - /// Returns account info without the code. + /// 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, + code_hash: self.code_hash, + code: None, + } + } + + /// 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 diff --git a/crates/revm/src/db/states/bundle_state.rs b/crates/revm/src/db/states/bundle_state.rs index d1db92e021..f478bfefa7 100644 --- a/crates/revm/src/db/states/bundle_state.rs +++ b/crates/revm/src/db/states/bundle_state.rs @@ -583,19 +583,20 @@ impl BundleState { self.reverts.push(reverts); } - /// Consume the bundle state and return plain state. - pub fn into_plain_state(self, is_value_known: OriginalValuesKnown) -> StateChangeset { + /// Generate a [`StateChangeset`] from the bundle state without consuming + /// it. + pub fn to_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 { + for (address, account) in self.state.iter() { // 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.map(AccountInfo::without_code); - accounts.push((address, info)); + let info = account.info.as_ref().map(AccountInfo::copy_without_code); + accounts.push((*address, info)); } // append storage changes @@ -604,7 +605,7 @@ impl BundleState { // 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 { + for (key, slot) in account.storage.iter().map(|(k, v)| (*k, *v)) { // 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(); @@ -624,17 +625,19 @@ impl BundleState { if !account_storage_changed.is_empty() || was_destroyed { // append storage changes to account. storage.push(PlainStorageChangeset { - address, + address: *address, wipe_storage: was_destroyed, storage: account_storage_changed, }); } } + let contracts = self .contracts - .into_iter() + .iter() // remove empty bytecodes - .filter(|(b, _)| *b != KECCAK_EMPTY) + .filter(|(b, _)| **b != KECCAK_EMPTY) + .map(|(b, code)| (*b, code.clone())) .collect::>(); StateChangeset { accounts, @@ -643,14 +646,32 @@ impl BundleState { } } - /// Consume the bundle state and split it into reverts and 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 { + self.to_plain_state(is_value_known) + } + + /// 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 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/plain_account.rs b/crates/revm/src/db/states/plain_account.rs index 6bf5d6dfe8..c02e4edb23 100644 --- a/crates/revm/src/db/states/plain_account.rs +++ b/crates/revm/src/db/states/plain_account.rs @@ -21,7 +21,7 @@ impl PlainAccount { } /// This type keeps track of the current value of a storage slot. -#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] +#[derive(Debug, Copy, Clone, Default, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct StorageSlot { /// The value of the storage slot before it was changed. 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.