From 86d25c6d5b7bb7fc4ecc43774d49f8175c2fb178 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Fri, 1 Sep 2023 16:00:06 -0700 Subject: [PATCH 01/14] chore: remove unused new_raw_with_hash (#676) --- crates/primitives/src/bytecode.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/crates/primitives/src/bytecode.rs b/crates/primitives/src/bytecode.rs index ea28c9137c..f8a0a70601 100644 --- a/crates/primitives/src/bytecode.rs +++ b/crates/primitives/src/bytecode.rs @@ -101,19 +101,6 @@ impl Bytecode { } } - /// Creates a new raw Bytecode with the given hash. - /// - /// # Safety - /// - /// The given `hash` has to be equal to the [`keccak256`] hash of the given `bytecode`. - #[inline] - pub unsafe fn new_raw_with_hash(bytecode: Bytes) -> Self { - Self { - bytecode, - state: BytecodeState::Raw, - } - } - /// Create new checked bytecode /// /// # Safety From 6bd05c90864bbf20f5c58f94e7d1103e6d140e3a Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Fri, 1 Sep 2023 16:00:17 -0700 Subject: [PATCH 02/14] chore: impl Eq, PartialEq for TransitionState (#677) * chore: impl Eq, PartialEq for TransitionState * re-add default impl with comment * remove conflicting Default impls --- crates/revm/src/db/states/transition_account.rs | 2 +- crates/revm/src/db/states/transition_state.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/revm/src/db/states/transition_account.rs b/crates/revm/src/db/states/transition_account.rs index 7c4232552f..fc9e4eed80 100644 --- a/crates/revm/src/db/states/transition_account.rs +++ b/crates/revm/src/db/states/transition_account.rs @@ -7,7 +7,7 @@ use revm_interpreter::primitives::{hash_map, AccountInfo, Bytecode, B256}; /// /// It is used when block state gets merged to bundle state to /// create needed Reverts. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Eq, PartialEq, Default)] pub struct TransitionAccount { pub info: Option, pub status: AccountStatus, diff --git a/crates/revm/src/db/states/transition_state.rs b/crates/revm/src/db/states/transition_state.rs index b6a5ce9235..a22c34af2d 100644 --- a/crates/revm/src/db/states/transition_state.rs +++ b/crates/revm/src/db/states/transition_state.rs @@ -1,7 +1,7 @@ use super::TransitionAccount; use revm_interpreter::primitives::{hash_map::Entry, HashMap, B160}; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct TransitionState { /// Block state account with account state pub transitions: HashMap, From 59db24225d867ff530567259e03ddd25f9ae5d11 Mon Sep 17 00:00:00 2001 From: rakita Date: Sat, 2 Sep 2023 13:38:45 +0200 Subject: [PATCH 03/14] fix: build documentation book (#678) --- .github/workflows/book.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/book.yml b/.github/workflows/book.yml index 088b01b5f0..3f3d90367a 100644 --- a/.github/workflows/book.yml +++ b/.github/workflows/book.yml @@ -73,7 +73,7 @@ jobs: echo `pwd`/mdbook-template >> $GITHUB_PATH - name: Build book - run: mdbook build + run: mdbook build documentation - name: Build docs run: RUSTDOCFLAGS="--enable-index-page -Zunstable-options" cargo +nightly doc --all --no-deps @@ -81,18 +81,18 @@ jobs: - name: Move docs to book folder run: | mkdir -p target/book/docs - mv target/doc target/book/docs + mv target/doc documentation/book/docs - name: Archive artifact shell: sh run: | - chmod -c -R +rX "target/book" | + chmod -c -R +rX "documentation/book" | while read line; do echo "::warning title=Invalid file permissions automatically fixed::$line" done tar \ --dereference --hard-dereference \ - --directory "target/book" \ + --directory "documentation/book" \ -cvf "$RUNNER_TEMP/artifact.tar" \ --exclude=.git \ --exclude=.github \ From 28976553c1110e943c6adfb65a22fd878c957ef4 Mon Sep 17 00:00:00 2001 From: rakita Date: Mon, 4 Sep 2023 11:19:39 +0200 Subject: [PATCH 04/14] fix(state): Extend now properly transfers wiped storage (#675) * fix: Fix extend functionality * added test --- crates/revm/src/db/states/bundle_state.rs | 96 ++++++++++++++--------- 1 file changed, 61 insertions(+), 35 deletions(-) diff --git a/crates/revm/src/db/states/bundle_state.rs b/crates/revm/src/db/states/bundle_state.rs index 4c13ba56fe..0e21abffe7 100644 --- a/crates/revm/src/db/states/bundle_state.rs +++ b/crates/revm/src/db/states/bundle_state.rs @@ -313,44 +313,44 @@ impl BundleState { /// Extend the state with state that is build on top of it. /// - /// For other state, if there a wipe storage flag set inside Revert copy the state - /// from `this` to `other` revert (if there is no duplicates of course). + /// If storage was wiped in `other` state, copy `this` plain state + /// and put it inside `other` revert (if there is no duplicates of course). /// - /// Check if `this` bundle was selfdestroyed if it is and if `other` - /// was selfdestroyed too we need to invalidate second (`other`) wipe flag - /// as wiping from database is done only once and we already transferred - /// all potentially missing storages to the `other` revert. + /// If `this` and `other` accounts were both destroyed invalidate second + /// wipe flag (from `other`). As wiping from database should be done only once + /// and we already transferred all potentially missing storages to the `other` revert. /// - /// Additionally update the `other` state only if there is no selfdestruct inside - /// `other` revert. + /// Additionally update the `other` state only if `other` is not flagged as destroyed. pub fn extend(&mut self, mut other: Self) { - // Extend the state. + // iterate over reverts and if its storage is wiped try to add previous bundle + // state as there is potential missing slots. + for (address, revert) in other.reverts.iter_mut().flatten() { + if revert.wipe_storage { + // If there is wipe storage in `other` revert + // we need to move storage from present state. + if let Some(this_account) = self.state.get_mut(address) { + // As this account was destroyed inside `other` bundle. + // we are fine to wipe/drain this storage and put it inside revert. + for (key, value) in this_account.storage.drain() { + revert + .storage + .entry(key) + .or_insert(RevertToSlot::Some(value.present_value)); + } + + // nullify `other` wipe as primary database wipe is done in `this`. + if this_account.was_destroyed() { + revert.wipe_storage = false; + } + } + } + } for (address, other_account) in other.state { match self.state.entry(address) { hash_map::Entry::Occupied(mut entry) => { - // iterate over reverts and if its storage is wiped try to add previous bundle - // state as there is potential missing slots. - let this = entry.get_mut(); - for (_, revert_account) in other.reverts.iter_mut().flatten() { - if revert_account.wipe_storage { - // If there is wipe storage in other revert - // we need to copy the storage from this revert - // to other revert. - for (key, value) in &this.storage { - revert_account - .storage - .entry(*key) - .or_insert(RevertToSlot::Some(value.present_value)); - } - - // nullify this wipe as database wipe is done in `this`. - if this.was_destroyed() { - revert_account.wipe_storage = false; - } - } - } + // if other was destroyed. replace `this` storage with // the `other one. if other_account.was_destroyed() { @@ -489,10 +489,14 @@ mod tests { B160([0x61; 20]) } - fn slot() -> U256 { + fn slot1() -> U256 { U256::from(5) } + fn slot2() -> U256 { + U256::from(7) + } + /// Test bundle one fn test_bundle1() -> BundleState { // block changes @@ -507,7 +511,10 @@ mod tests { code_hash: KECCAK_EMPTY, code: None, }), - HashMap::from([(slot(), (U256::from(0), U256::from(10)))]), + HashMap::from([ + (slot1(), (U256::from(0), U256::from(10))), + (slot2(), (U256::from(0), U256::from(15))), + ]), ), ( account2(), @@ -522,7 +529,11 @@ mod tests { ), ], vec![vec![ - (account1(), Some(None), vec![(slot(), U256::from(0))]), + ( + account1(), + Some(None), + vec![(slot1(), U256::from(0)), (slot2(), U256::from(0))], + ), (account2(), Some(None), vec![]), ]], vec![], @@ -542,7 +553,7 @@ mod tests { code_hash: KECCAK_EMPTY, code: None, }), - HashMap::from([(slot(), (U256::from(0), U256::from(15)))]), + HashMap::from([(slot1(), (U256::from(0), U256::from(15)))]), )], vec![vec![( account1(), @@ -552,7 +563,7 @@ mod tests { code_hash: KECCAK_EMPTY, code: None, })), - vec![(slot(), U256::from(10))], + vec![(slot1(), U256::from(10))], )]], vec![], ) @@ -613,12 +624,27 @@ mod tests { let mut b2 = base_bundle2.clone(); b1.state.get_mut(&account1()).unwrap().status = AccountStatus::Changed; b2.state.get_mut(&account1()).unwrap().status = AccountStatus::Destroyed; + b2.reverts[0][0].1.wipe_storage = true; b1.extend(b2); assert_eq!( b1.state.get_mut(&account1()).unwrap().status, AccountStatus::Destroyed ); + // test2 extension + // revert of b2 should contains plain state of b1. + let mut revert1 = base_bundle2.reverts[0][0].clone(); + revert1.1.wipe_storage = true; + revert1 + .1 + .storage + .insert(slot2(), RevertToSlot::Some(U256::from(15))); + + assert_eq!( + b1.reverts, + vec![base_bundle1.reverts[0].clone(), vec![revert1]], + ); + // test3 // bundle1 has InMemoryChange // bundle2 has Change From fde6df13ed5db5ff66c75483cff62fc65f1797c7 Mon Sep 17 00:00:00 2001 From: Eric Date: Mon, 4 Sep 2023 20:22:59 +0800 Subject: [PATCH 05/14] apply builder pattern for BundleState initialization (#649) --- crates/revm/src/db/states/bundle_state.rs | 294 +++++++++++++++++++++- 1 file changed, 290 insertions(+), 4 deletions(-) diff --git a/crates/revm/src/db/states/bundle_state.rs b/crates/revm/src/db/states/bundle_state.rs index 0e21abffe7..12b7e2729d 100644 --- a/crates/revm/src/db/states/bundle_state.rs +++ b/crates/revm/src/db/states/bundle_state.rs @@ -7,6 +7,196 @@ use revm_interpreter::primitives::{ hash_map::{self, Entry}, AccountInfo, Bytecode, HashMap, StorageSlot, B160, B256, KECCAK_EMPTY, U256, }; +use std::collections::{BTreeMap, BTreeSet, HashSet}; +use std::ops::RangeInclusive; + +/// This builder is used to help to facilitate the initialization of `BundleState` struct +#[derive(Debug)] +pub struct BundleBuilder { + states: HashSet, + state_original: HashMap, + state_present: HashMap, + state_storage: HashMap>, + + reverts: BTreeSet<(u64, B160)>, + revert_range: RangeInclusive, + revert_account: HashMap<(u64, B160), Option>>, + revert_storage: HashMap<(u64, B160), Vec<(U256, U256)>>, + + contracts: HashMap, +} + +impl Default for BundleBuilder { + fn default() -> Self { + BundleBuilder { + states: HashSet::new(), + state_original: HashMap::new(), + state_present: HashMap::new(), + state_storage: HashMap::new(), + reverts: BTreeSet::new(), + revert_range: 0..=0, + revert_account: HashMap::new(), + revert_storage: HashMap::new(), + contracts: HashMap::new(), + } + } +} + +impl BundleBuilder { + /// Create builder instance + /// + /// `revert_range` indicates the size of BundleState `reverts` field + pub fn new(revert_range: RangeInclusive) -> Self { + BundleBuilder { + revert_range, + ..Default::default() + } + } + + /// Collect address info of BundleState state + pub fn state_address(mut self, address: B160) -> Self { + self.states.insert(address); + self + } + + /// Collect account info of BundleState state + pub fn state_original_account_info(mut self, address: B160, original: AccountInfo) -> Self { + self.states.insert(address); + self.state_original.insert(address, original); + self + } + + /// Collect account info of BundleState state + pub fn state_present_account_info(mut self, address: B160, present: AccountInfo) -> Self { + self.states.insert(address); + self.state_present.insert(address, present); + self + } + + /// Collect storage info of BundleState state + pub fn state_storage(mut self, address: B160, storage: HashMap) -> Self { + self.states.insert(address); + self.state_storage.insert(address, storage); + self + } + + /// Collect address info of BundleState reverts + /// + /// `block_number` must respect `revert_range`, or the input + /// will be ignored during the final build process + pub fn revert_address(mut self, block_number: u64, address: B160) -> Self { + self.reverts.insert((block_number, address)); + self + } + + /// Collect account info of BundleState reverts + /// + /// `block_number` must respect `revert_range`, or the input + /// will be ignored during the final build process + pub fn revert_account_info( + mut self, + block_number: u64, + address: B160, + account: Option>, + ) -> Self { + self.reverts.insert((block_number, address)); + self.revert_account.insert((block_number, address), account); + self + } + + /// Collect storage info of BundleState reverts + /// + /// `block_number` must respect `revert_range`, or the input + /// will be ignored during the final build process + pub fn revert_storage( + mut self, + block_number: u64, + address: B160, + storage: Vec<(U256, U256)>, + ) -> Self { + self.reverts.insert((block_number, address)); + self.revert_storage.insert((block_number, address), storage); + self + } + + /// Collect contracts info + pub fn contract(mut self, address: B256, bytecode: Bytecode) -> Self { + self.contracts.insert(address, bytecode); + self + } + + /// Create `BundleState` instance based on collected information + pub fn build(mut self) -> BundleState { + let state = self + .states + .into_iter() + .map(|address| { + let storage = self + .state_storage + .remove(&address) + .map(|s| { + s.into_iter() + .map(|(k, (o_val, p_val))| (k, StorageSlot::new_changed(o_val, p_val))) + .collect() + }) + .unwrap_or_default(); + let bundle_account = BundleAccount::new( + self.state_original.remove(&address), + self.state_present.remove(&address), + storage, + AccountStatus::Changed, + ); + (address, bundle_account) + }) + .collect(); + + let mut reverts_map = BTreeMap::new(); + for block_number in self.revert_range { + reverts_map.insert(block_number, Vec::new()); + } + self.reverts + .into_iter() + .for_each(|(block_number, address)| { + let account = match self + .revert_account + .remove(&(block_number, address)) + .unwrap_or_default() + { + Some(Some(account)) => AccountInfoRevert::RevertTo(account), + Some(None) => AccountInfoRevert::DeleteIt, + None => AccountInfoRevert::DoNothing, + }; + let storage = self + .revert_storage + .remove(&(block_number, address)) + .map(|s| { + s.into_iter() + .map(|(k, v)| (k, RevertToSlot::Some(v))) + .collect() + }) + .unwrap_or_default(); + let account_revert = AccountRevert { + account, + storage, + previous_status: AccountStatus::Changed, + wipe_storage: false, + }; + + if reverts_map.contains_key(&block_number) { + reverts_map + .entry(block_number) + .or_insert(Vec::new()) + .push((address, account_revert)); + } + }); + + BundleState { + state, + contracts: self.contracts, + reverts: reverts_map.into_values().collect(), + } + } +} /// Bundle retention policy for applying substate to the bundle. #[derive(Debug)] @@ -57,6 +247,11 @@ impl Default for BundleState { } impl BundleState { + /// Return builder instance for further manipulation + pub fn builder(revert_range: RangeInclusive) -> BundleBuilder { + BundleBuilder::new(revert_range) + } + /// Create it with new and old values of both Storage and AccountInfo. pub fn new( state: impl IntoIterator< @@ -569,11 +764,71 @@ mod tests { ) } - #[test] - fn sanity_path() { - let bundle1 = test_bundle1(); - let bundle2 = test_bundle2(); + /// Test bundle three + fn test_bundle3() -> BundleState { + BundleState::builder(0..=0) + .state_present_account_info( + account1(), + AccountInfo { + nonce: 1, + balance: U256::from(10), + code_hash: KECCAK_EMPTY, + code: None, + }, + ) + .state_storage( + account1(), + HashMap::from([(slot(), (U256::from(0), U256::from(10)))]), + ) + .state_address(account2()) + .state_present_account_info( + account2(), + AccountInfo { + nonce: 1, + balance: U256::from(10), + code_hash: KECCAK_EMPTY, + code: None, + }, + ) + .revert_address(0, account1()) + .revert_account_info(0, account1(), Some(None)) + .revert_storage(0, account1(), vec![(slot(), U256::from(0))]) + .revert_account_info(0, account2(), Some(None)) + .build() + } + + /// Test bundle four + fn test_bundle4() -> BundleState { + BundleState::builder(0..=0) + .state_present_account_info( + account1(), + AccountInfo { + nonce: 3, + balance: U256::from(20), + code_hash: KECCAK_EMPTY, + code: None, + }, + ) + .state_storage( + account1(), + HashMap::from([(slot(), (U256::from(0), U256::from(15)))]), + ) + .revert_address(0, account1()) + .revert_account_info( + 0, + account1(), + Some(Some(AccountInfo { + nonce: 1, + balance: U256::from(10), + code_hash: KECCAK_EMPTY, + code: None, + })), + ) + .revert_storage(0, account1(), vec![(slot(), U256::from(10))]) + .build() + } + fn sanity_path(bundle1: BundleState, bundle2: BundleState) { let mut extended = bundle1.clone(); extended.extend(bundle2.clone()); @@ -660,4 +915,35 @@ mod tests { AccountStatus::InMemoryChange ); } + + #[test] + fn test_sanity_path() { + sanity_path(test_bundle1(), test_bundle2()); + sanity_path(test_bundle3(), test_bundle4()); + } + + #[test] + fn test_revert_capacity() { + let state = BundleState::builder(0..=3) + .revert_address(0, account1()) + .revert_address(2, account2()) + .revert_account_info(0, account1(), Some(None)) + .revert_account_info(2, account2(), None) + .revert_storage(0, account1(), vec![(slot(), U256::from(10))]) + .build(); + + assert_eq!(state.reverts.len(), 4); + assert_eq!(state.reverts[1], vec![]); + assert_eq!(state.reverts[3], vec![]); + assert_eq!(state.reverts[0].len(), 1); + assert_eq!(state.reverts[2].len(), 1); + + let (addr1, revert1) = &state.reverts[0][0]; + assert_eq!(addr1, &account1()); + assert_eq!(revert1.account, AccountInfoRevert::DeleteIt); + + let (addr2, revert2) = &state.reverts[2][0]; + assert_eq!(addr2, &account2()); + assert_eq!(revert2.account, AccountInfoRevert::DoNothing); + } } From b50071808ce3b8d76a80e5185350a3733ce0194c Mon Sep 17 00:00:00 2001 From: rakita Date: Mon, 4 Sep 2023 15:02:21 +0200 Subject: [PATCH 06/14] feat(state): take N reverts from BundleState, struct refactor (#681) * feat(state): take N reverts from BundleState * new line * nits, export and PartialEq,Eq * ftm --- crates/revm/Cargo.toml | 2 +- crates/revm/src/db/states.rs | 2 +- crates/revm/src/db/states/bundle_state.rs | 136 +++++++++++----------- crates/revm/src/db/states/changes.rs | 37 ++++-- crates/revm/src/db/states/reverts.rs | 78 ++++++++++++- crates/revm/src/db/states/state.rs | 10 +- 6 files changed, 180 insertions(+), 85 deletions(-) diff --git a/crates/revm/Cargo.toml b/crates/revm/Cargo.toml index 41ff5cb49d..2f7d7574e4 100644 --- a/crates/revm/Cargo.toml +++ b/crates/revm/Cargo.toml @@ -10,8 +10,8 @@ version = "3.3.0" readme = "../../README.md" [dependencies] -revm-precompile = { path = "../precompile", version = "2.0.2", default-features = false } revm-interpreter = { path = "../interpreter", version = "1.1.2", default-features = false } +revm-precompile = { path = "../precompile", version = "2.0.2", default-features = false } #misc auto_impl = { version = "1.1", default-features = false } diff --git a/crates/revm/src/db/states.rs b/crates/revm/src/db/states.rs index 20f4d565f7..f62d72c101 100644 --- a/crates/revm/src/db/states.rs +++ b/crates/revm/src/db/states.rs @@ -17,7 +17,7 @@ pub use bundle_account::BundleAccount; pub use bundle_state::BundleState; pub use cache::CacheState; pub use cache_account::CacheAccount; -pub use changes::{StateChangeset, StateReverts}; +pub use changes::{PlainStateReverts, PlainStorageChangeset, PlainStorageRevert, StateChangeset}; pub use plain_account::{PlainAccount, StorageWithOriginalValues}; pub use reverts::{AccountRevert, RevertToSlot}; pub use state::State; diff --git a/crates/revm/src/db/states/bundle_state.rs b/crates/revm/src/db/states/bundle_state.rs index 12b7e2729d..8c4a86d5d1 100644 --- a/crates/revm/src/db/states/bundle_state.rs +++ b/crates/revm/src/db/states/bundle_state.rs @@ -1,6 +1,7 @@ use super::{ - changes::StateChangeset, reverts::AccountInfoRevert, AccountRevert, AccountStatus, - BundleAccount, RevertToSlot, StateReverts, TransitionState, + changes::{PlainStorageChangeset, StateChangeset}, + reverts::{AccountInfoRevert, Reverts}, + AccountRevert, AccountStatus, BundleAccount, PlainStateReverts, RevertToSlot, TransitionState, }; use rayon::slice::ParallelSliceMut; use revm_interpreter::primitives::{ @@ -193,7 +194,7 @@ impl BundleBuilder { BundleState { state, contracts: self.contracts, - reverts: reverts_map.into_values().collect(), + reverts: Reverts::new(reverts_map.into_values().collect()), } } } @@ -233,7 +234,7 @@ pub struct BundleState { /// /// Note: Inside vector is *not* sorted by address. /// But it is unique by address. - pub reverts: Vec>, + pub reverts: Reverts, } impl Default for BundleState { @@ -241,7 +242,7 @@ impl Default for BundleState { Self { state: HashMap::new(), contracts: HashMap::new(), - reverts: Vec::new(), + reverts: Reverts::default(), } } } @@ -324,7 +325,7 @@ impl BundleState { Self { state, contracts: contracts.into_iter().collect(), - reverts, + reverts: Reverts::new(reverts), } } @@ -401,39 +402,11 @@ impl BundleState { self.reverts.push(reverts); } - /// Return and clear all reverts from [BundleState], sort them before returning. - pub fn take_reverts(&mut self) -> StateReverts { - let mut state_reverts = StateReverts::with_capacity(self.reverts.len()); - for reverts in self.reverts.drain(..) { - // 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)), - AccountInfoRevert::DoNothing => (), - } - if revert_account.wipe_storage || !revert_account.storage.is_empty() { - let mut account_storage = - revert_account.storage.into_iter().collect::>(); - account_storage.par_sort_unstable_by(|a, b| a.0.cmp(&b.0)); - storage.push((address, revert_account.wipe_storage, account_storage)); - } - } - accounts.par_sort_unstable_by(|a, b| a.0.cmp(&b.0)); - state_reverts.accounts.push(accounts); - state_reverts.storage.push(storage); - } - - state_reverts - } - /// Consume the bundle state and return sorted plain state. /// - /// `omit_changed_check` does not check If account is same as + /// `omit_changed_check` does not check if account is same as /// original state, this assumption can't be made in cases when - /// we split the bundle state and commit part of it. + /// we split the bundle state and commit parts of it. pub fn into_plain_state_sorted(self, omit_changed_check: bool) -> StateChangeset { // pessimistically pre-allocate assuming _all_ accounts changed. let state_len = self.state.len(); @@ -471,15 +444,15 @@ impl BundleState { if !account_storage_changed.is_empty() { account_storage_changed.sort_by(|a, b| a.0.cmp(&b.0)); // append storage changes to account. - storage.push(( + storage.push(PlainStorageChangeset { address, - (account.status.was_destroyed(), account_storage_changed), - )); + storage: account_storage_changed, + }); } } accounts.par_sort_unstable_by(|a, b| a.0.cmp(&b.0)); - storage.par_sort_unstable_by(|a, b| a.0.cmp(&b.0)); + storage.par_sort_unstable_by(|a, b| a.address.cmp(&b.address)); let mut contracts = self .contracts @@ -500,10 +473,10 @@ impl BundleState { pub fn into_sorted_plain_state_and_reverts( mut self, omit_changed_check: bool, - ) -> (StateChangeset, StateReverts) { - let reverts = self.take_reverts(); + ) -> (StateChangeset, PlainStateReverts) { + let reverts = self.take_all_reverts(); let plain_state = self.into_plain_state_sorted(omit_changed_check); - (plain_state, reverts) + (plain_state, reverts.into_plain_state_reverts()) } /// Extend the state with state that is build on top of it. @@ -575,27 +548,21 @@ impl BundleState { self.reverts.extend(other.reverts); } - /// This will return detached lower part of reverts - /// - /// Note that plain state will stay the same and returned BundleState - /// will contain only reverts and will be considered broken. - /// - /// If given number is greater then number of reverts then None is returned. - /// Same if given transition number is zero. - pub fn detach_lower_part_reverts(&mut self, num_of_detachments: usize) -> Option { - if num_of_detachments == 0 || num_of_detachments > self.reverts.len() { - return None; - } - + /// Take first N raw reverts from the [BundleState]. + pub fn take_n_reverts(&mut self, reverts_to_take: usize) -> Reverts { // split is done as [0, num) and [num, len]. - let (detach, this) = self.reverts.split_at(num_of_detachments); + if reverts_to_take > self.reverts.len() { + return self.take_all_reverts(); + } + let (detach, this) = self.reverts.split_at(reverts_to_take); + let ret = Reverts::new(detach.to_vec()); + self.reverts = Reverts::new(this.to_vec()); + ret + } - let detached_reverts = detach.to_vec(); - self.reverts = this.to_vec(); - Some(Self { - reverts: detached_reverts, - ..Default::default() - }) + /// Return and clear all reverts from [BundleState] + pub fn take_all_reverts(&mut self) -> Reverts { + core::mem::take(&mut self.reverts) } /// Reverts the state changes of the latest transition @@ -778,7 +745,7 @@ mod tests { ) .state_storage( account1(), - HashMap::from([(slot(), (U256::from(0), U256::from(10)))]), + HashMap::from([(slot1(), (U256::from(0), U256::from(10)))]), ) .state_address(account2()) .state_present_account_info( @@ -792,7 +759,7 @@ mod tests { ) .revert_address(0, account1()) .revert_account_info(0, account1(), Some(None)) - .revert_storage(0, account1(), vec![(slot(), U256::from(0))]) + .revert_storage(0, account1(), vec![(slot1(), U256::from(0))]) .revert_account_info(0, account2(), Some(None)) .build() } @@ -811,7 +778,7 @@ mod tests { ) .state_storage( account1(), - HashMap::from([(slot(), (U256::from(0), U256::from(15)))]), + HashMap::from([(slot1(), (U256::from(0), U256::from(15)))]), ) .revert_address(0, account1()) .revert_account_info( @@ -824,7 +791,7 @@ mod tests { code: None, })), ) - .revert_storage(0, account1(), vec![(slot(), U256::from(10))]) + .revert_storage(0, account1(), vec![(slot1(), U256::from(10))]) .build() } @@ -896,7 +863,7 @@ mod tests { .insert(slot2(), RevertToSlot::Some(U256::from(15))); assert_eq!( - b1.reverts, + b1.reverts.as_ref(), vec![base_bundle1.reverts[0].clone(), vec![revert1]], ); @@ -929,7 +896,7 @@ mod tests { .revert_address(2, account2()) .revert_account_info(0, account1(), Some(None)) .revert_account_info(2, account2(), None) - .revert_storage(0, account1(), vec![(slot(), U256::from(10))]) + .revert_storage(0, account1(), vec![(slot1(), U256::from(10))]) .build(); assert_eq!(state.reverts.len(), 4); @@ -946,4 +913,37 @@ mod tests { assert_eq!(addr2, &account2()); assert_eq!(revert2.account, AccountInfoRevert::DoNothing); } + + #[test] + fn take_reverts() { + let bundle1 = test_bundle1(); + let bundle2 = test_bundle2(); + + let mut extended = bundle1.clone(); + extended.extend(bundle2.clone()); + + // check that we have two reverts + assert_eq!(extended.reverts.len(), 2); + + // take all by big N + let mut extended2 = extended.clone(); + assert_eq!(extended2.take_n_reverts(100), extended.reverts); + + // take all reverts + let mut extended2 = extended.clone(); + assert_eq!(extended2.take_all_reverts(), extended.reverts); + + // take zero revert + let taken_reverts = extended.take_n_reverts(0); + assert_eq!(taken_reverts, Reverts::default()); + assert_eq!(extended.reverts.len(), 2); + + // take one revert + let taken_reverts = extended.take_n_reverts(1); + assert_eq!(taken_reverts, bundle1.reverts); + + // take last revert + let taken_reverts = extended.take_n_reverts(1); + assert_eq!(taken_reverts, bundle2.reverts); + } } diff --git a/crates/revm/src/db/states/changes.rs b/crates/revm/src/db/states/changes.rs index 657668be0d..99411b7b8c 100644 --- a/crates/revm/src/db/states/changes.rs +++ b/crates/revm/src/db/states/changes.rs @@ -10,26 +10,49 @@ pub struct StateChangeset { pub accounts: Vec<(B160, Option)>, /// Vector of storage presorted by address /// First bool is indicator if storage needs to be dropped. - pub storage: StorageChangeset, + pub storage: Vec, /// Vector of contracts presorted by bytecode hash pub contracts: Vec<(B256, Bytecode)>, } -/// Storage changeset -pub type StorageChangeset = Vec<(B160, (bool, Vec<(U256, U256)>))>; +/// Plain storage changeset. Used to apply storage changes of plain state to +/// the database. +#[derive(Clone, Debug, PartialEq, Eq, Default)] +pub struct PlainStorageChangeset { + /// Address of account + pub address: B160, + /// Storage key value pairs. + pub storage: Vec<(U256, U256)>, +} + +/// Plain Storage Revert. Containing old values of changed storage. +#[derive(Clone, Debug, PartialEq, Eq, Default)] +pub struct PlainStorageRevert { + /// Address of account + pub address: B160, + /// Is storage wiped in this revert. Wiped flag is set on + /// first known selfdestruct and would require clearing the + /// state of this storage from database (And moving it to revert). + pub wiped: bool, + /// Contains the storage key and old values of that storage. + /// Assume they are sorted by the key. + pub storage_revert: Vec<(U256, RevertToSlot)>, +} +/// Plain state reverts are used to easily store reverts into database. +/// +/// Note that accounts are assumed sorted by address. #[derive(Clone, Debug, Default)] -pub struct StateReverts { +pub struct PlainStateReverts { /// Vector of account presorted by address, with removed contracts bytecode /// /// Note: AccountInfo None means that account needs to be removed. pub accounts: Vec)>>, /// Vector of storage presorted by address - /// U256::ZERO means that storage needs to be removed. - pub storage: StorageRevert, + pub storage: Vec>, } -impl StateReverts { +impl PlainStateReverts { /// Constructs new [StateReverts] with pre-allocated capacity. pub fn with_capacity(capacity: usize) -> Self { Self { diff --git a/crates/revm/src/db/states/reverts.rs b/crates/revm/src/db/states/reverts.rs index b3c293a8ce..fe67f33e3d 100644 --- a/crates/revm/src/db/states/reverts.rs +++ b/crates/revm/src/db/states/reverts.rs @@ -1,6 +1,80 @@ -use revm_interpreter::primitives::{AccountInfo, HashMap, U256}; +use super::{ + changes::PlainStorageRevert, AccountStatus, BundleAccount, PlainStateReverts, + StorageWithOriginalValues, +}; +use core::ops::{Deref, DerefMut}; +use rayon::slice::ParallelSliceMut; +use revm_interpreter::primitives::{AccountInfo, HashMap, B160, U256}; -use super::{AccountStatus, BundleAccount, StorageWithOriginalValues}; +/// Contains reverts of multiple account in multiple transitions (Transitions as a block). +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct Reverts(Vec>); + +impl Deref for Reverts { + type Target = Vec>; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for Reverts { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + +impl Reverts { + /// Create new reverts + pub fn new(reverts: Vec>) -> Self { + Self(reverts) + } + + /// Sort account inside transition by their address. + pub fn sort(&mut self) { + for revert in &mut self.0 { + revert.sort_by_key(|(address, _)| *address); + } + } + + /// Extend reverts with other reverts. + pub fn extend(&mut self, other: Reverts) { + self.0.extend(other.0); + } + + /// Consume reverts and create plain state reverts. + /// + /// Note that account are sorted by address. + pub fn into_plain_state_reverts(mut self) -> PlainStateReverts { + let mut state_reverts = PlainStateReverts::with_capacity(self.0.len()); + for reverts in self.0.drain(..) { + // 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)), + AccountInfoRevert::DoNothing => (), + } + if revert_account.wipe_storage || !revert_account.storage.is_empty() { + let mut account_storage = + revert_account.storage.into_iter().collect::>(); + account_storage.par_sort_unstable_by(|a, b| a.0.cmp(&b.0)); + storage.push(PlainStorageRevert { + address, + wiped: revert_account.wipe_storage, + storage_revert: account_storage, + }); + } + } + accounts.par_sort_unstable_by(|a, b| a.0.cmp(&b.0)); + state_reverts.accounts.push(accounts); + state_reverts.storage.push(storage); + } + state_reverts + } +} /// Assumption is that Revert can return full state from any future state to any past state. /// diff --git a/crates/revm/src/db/states/state.rs b/crates/revm/src/db/states/state.rs index 6ec5daba2c..da19bc8f04 100644 --- a/crates/revm/src/db/states/state.rs +++ b/crates/revm/src/db/states/state.rs @@ -449,7 +449,7 @@ mod tests { // The new account revert should be `DeleteIt` since this was an account creation. // The existing account revert should be reverted to its previous state. assert_eq!( - bundle_state.reverts, + bundle_state.reverts.as_ref(), Vec::from([Vec::from([ ( new_account_address, @@ -682,12 +682,10 @@ mod tests { state.merge_transitions(BundleRetention::Reverts); let mut bundle_state = state.take_bundle(); - for revert in &mut bundle_state.reverts { - revert.sort_unstable_by_key(|(address, _)| *address); - } + bundle_state.reverts.sort(); assert_eq!( - bundle_state.reverts, + bundle_state.reverts.as_ref(), Vec::from([Vec::from([ // new account is destroyed as if it never existed. // ( ... ) @@ -818,7 +816,7 @@ mod tests { ); assert_eq!( - bundle_state.reverts, + bundle_state.reverts.as_ref(), Vec::from([Vec::from([( existing_account_address, AccountRevert { From a9dce302f7df04a838b4a365ad92dc9dbae88fc4 Mon Sep 17 00:00:00 2001 From: rakita Date: Tue, 5 Sep 2023 15:20:53 +0200 Subject: [PATCH 07/14] chore: nits and renamings (#684) --- crates/revm/src/db/states/bundle_state.rs | 4 ++-- crates/revm/src/db/states/cache_account.rs | 5 ++--- crates/revm/src/db/states/reverts.rs | 2 ++ crates/revm/src/db/states/state.rs | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/revm/src/db/states/bundle_state.rs b/crates/revm/src/db/states/bundle_state.rs index 8c4a86d5d1..adeddc3efc 100644 --- a/crates/revm/src/db/states/bundle_state.rs +++ b/crates/revm/src/db/states/bundle_state.rs @@ -357,7 +357,7 @@ impl BundleState { /// Consume `TransitionState` by applying the changes and creating the reverts /// /// If [BundleRetention::includes_reverts] is `true`, then the reverts will be retained. - pub fn apply_block_substate_and_create_reverts( + pub fn apply_transitions_and_create_reverts( &mut self, transitions: TransitionState, retention: BundleRetention, @@ -637,7 +637,7 @@ mod tests { }; // apply first transition - bundle_state.apply_block_substate_and_create_reverts( + bundle_state.apply_transitions_and_create_reverts( TransitionState::single(address, transition.clone()), BundleRetention::Reverts, ); diff --git a/crates/revm/src/db/states/cache_account.rs b/crates/revm/src/db/states/cache_account.rs index 5341fb398f..29d7b81a2a 100644 --- a/crates/revm/src/db/states/cache_account.rs +++ b/crates/revm/src/db/states/cache_account.rs @@ -5,9 +5,8 @@ use super::{ use revm_interpreter::primitives::{AccountInfo, KECCAK_EMPTY, U256}; use revm_precompile::HashMap; -/// Cache account is to store account from database be able -/// to be updated from output of revm and while doing that -/// create TransitionAccount needed for BundleState. +/// Cache account contains plain state that gets updated +/// at every transaction when evm output is applied to CacheState. #[derive(Clone, Debug)] pub struct CacheAccount { pub account: Option, diff --git a/crates/revm/src/db/states/reverts.rs b/crates/revm/src/db/states/reverts.rs index fe67f33e3d..fe17d76875 100644 --- a/crates/revm/src/db/states/reverts.rs +++ b/crates/revm/src/db/states/reverts.rs @@ -167,6 +167,8 @@ impl AccountRevert { } } +/// Depending on previous state of account info this +/// will tell us what to do on revert. #[derive(Clone, Default, Debug, PartialEq, Eq)] pub enum AccountInfoRevert { #[default] diff --git a/crates/revm/src/db/states/state.rs b/crates/revm/src/db/states/state.rs index da19bc8f04..35b6b0583e 100644 --- a/crates/revm/src/db/states/state.rs +++ b/crates/revm/src/db/states/state.rs @@ -130,7 +130,7 @@ impl<'a, DBError> State<'a, DBError> { if let Some(transition_state) = self.transition_state.as_mut().map(TransitionState::take) { self.bundle_state .get_or_insert(BundleState::default()) - .apply_block_substate_and_create_reverts(transition_state, retention); + .apply_transitions_and_create_reverts(transition_state, retention); } } From 99432e3725d317f57de8bf54a149b10795c02196 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 17:22:40 +0200 Subject: [PATCH 08/14] chore(deps): bump enumn from 0.1.11 to 0.1.12 (#679) Bumps [enumn](https://github.com/dtolnay/enumn) from 0.1.11 to 0.1.12. - [Release notes](https://github.com/dtolnay/enumn/releases) - [Commits](https://github.com/dtolnay/enumn/compare/0.1.11...0.1.12) --- updated-dependencies: - dependency-name: enumn dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ae802d0394..e031cbe82a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -809,9 +809,9 @@ dependencies = [ [[package]] name = "enumn" -version = "0.1.11" +version = "0.1.12" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b893c4eb2dc092c811165f84dc7447fae16fb66521717968c34c509b39b1a5c5" +checksum = "c2ad8cef1d801a4686bfd8919f0b30eac4c8e48968c437a6405ded4fb5272d2b" dependencies = [ "proc-macro2", "quote", From c0767fa3e3b49497fa67cef37a9519fa6e55dd12 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 5 Sep 2023 17:22:46 +0200 Subject: [PATCH 09/14] chore(deps): bump thiserror from 1.0.47 to 1.0.48 (#680) Bumps [thiserror](https://github.com/dtolnay/thiserror) from 1.0.47 to 1.0.48. - [Release notes](https://github.com/dtolnay/thiserror/releases) - [Commits](https://github.com/dtolnay/thiserror/compare/1.0.47...1.0.48) --- updated-dependencies: - dependency-name: thiserror dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e031cbe82a..25650cda91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2913,18 +2913,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.47" +version = "1.0.48" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97a802ec30afc17eee47b2855fc72e0c4cd62be9b4efe6591edde0ec5bd68d8f" +checksum = "9d6d7a740b8a666a7e828dd00da9c0dc290dff53154ea77ac109281de90589b7" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.47" +version = "1.0.48" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6bb623b56e39ab7dcd4b1b98bb6c8f8d907ed255b18de254088016b27a8ee19b" +checksum = "49922ecae66cc8a249b77e68d1d0623c1b2c514f0060c27cdc68bd62a1219d35" dependencies = [ "proc-macro2", "quote", From 7d7f63ff61753edfcc132f2a2c6405fd3ec5eabb Mon Sep 17 00:00:00 2001 From: rakita Date: Tue, 5 Sep 2023 19:41:09 +0200 Subject: [PATCH 10/14] chore(state): Make Database more generic. (#687) * chore(state): Make database more generic * doc --- crates/revm/src/db/states/state.rs | 23 +++++++++++++--------- crates/revm/src/db/states/state_builder.rs | 19 +++++++++--------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/crates/revm/src/db/states/state.rs b/crates/revm/src/db/states/state.rs index 35b6b0583e..71cef87fed 100644 --- a/crates/revm/src/db/states/state.rs +++ b/crates/revm/src/db/states/state.rs @@ -9,11 +9,16 @@ use revm_interpreter::primitives::{ hash_map, Account, AccountInfo, Bytecode, HashMap, B160, B256, BLOCK_HASH_HISTORY, U256, }; +/// More constrained version of State that uses Boxed database with a lifetime. +/// +/// This is used to make it easier to use State. +pub type StateDBBox<'a, DBError> = State + Send + 'a>>; + /// State of blockchain. /// /// State clear flag is set inside CacheState and by default it is enabled. /// If you want to disable it use `set_state_clear_flag` function. -pub struct State<'a, DBError> { +pub struct State { /// Cached state contains both changed from evm execution and cached/loaded account/storages /// from database. This allows us to have only one layer of cache where we can fetch data. /// Additionaly we can introduce some preloading of data from database. @@ -22,7 +27,7 @@ pub struct State<'a, DBError> { /// return not existing account and storage. /// /// Note: It is marked as Send so database can be shared between threads. - pub database: Box + Send + 'a>, + pub database: DB, //Box + Send + 'a>, /// Block state, it aggregates transactions transitions into one state. /// /// Build reverts and state that gets applied to the state. @@ -46,7 +51,7 @@ pub struct State<'a, DBError> { pub block_hashes: BTreeMap, } -impl<'a, DBError> State<'a, DBError> { +impl State { /// Iterate over received balances and increment all account balances. /// If account is not found inside cache state it will be loaded from database. /// @@ -54,7 +59,7 @@ impl<'a, DBError> State<'a, DBError> { pub fn increment_balances( &mut self, balances: impl IntoIterator, - ) -> Result<(), DBError> { + ) -> Result<(), DB::Error> { // make transition and update cache state let mut transitions = Vec::new(); for (address, balance) in balances { @@ -74,7 +79,7 @@ impl<'a, DBError> State<'a, DBError> { pub fn drain_balances( &mut self, addresses: impl IntoIterator, - ) -> Result, DBError> { + ) -> Result, DB::Error> { // make transition and update cache state let mut transitions = Vec::new(); let mut balances = Vec::new(); @@ -134,7 +139,7 @@ impl<'a, DBError> State<'a, DBError> { } } - pub fn load_cache_account(&mut self, address: B160) -> Result<&mut CacheAccount, DBError> { + pub fn load_cache_account(&mut self, address: B160) -> Result<&mut CacheAccount, DB::Error> { match self.cache.accounts.entry(address) { hash_map::Entry::Vacant(entry) => { if self.use_preloaded_bundle { @@ -171,8 +176,8 @@ impl<'a, DBError> State<'a, DBError> { } } -impl<'a, DBError> Database for State<'a, DBError> { - type Error = DBError; +impl Database for State { + type Error = DB::Error; fn basic(&mut self, address: B160) -> Result, Self::Error> { self.load_cache_account(address).map(|a| a.account_info()) @@ -254,7 +259,7 @@ impl<'a, DBError> Database for State<'a, DBError> { } } -impl<'a, DBError> DatabaseCommit for State<'a, DBError> { +impl DatabaseCommit for State { fn commit(&mut self, evm_state: HashMap) { let transitions = self.cache.apply_evm_state(evm_state); self.apply_transition(transitions); diff --git a/crates/revm/src/db/states/state_builder.rs b/crates/revm/src/db/states/state_builder.rs index d3d65d2080..fb943815a7 100644 --- a/crates/revm/src/db/states/state_builder.rs +++ b/crates/revm/src/db/states/state_builder.rs @@ -1,17 +1,16 @@ use super::{cache::CacheState, BundleState, State, TransitionState}; use crate::db::EmptyDB; use alloc::collections::BTreeMap; -use core::convert::Infallible; use revm_interpreter::primitives::{db::Database, B256}; /// Allows building of State and initializing it with different options. -pub struct StateBuilder<'a, DBError> { +pub struct StateBuilder { pub with_state_clear: bool, /// Optional database that we use to fetch data from. If database is not present, we will /// return not existing account and storage. /// /// Note: It is marked as Send so database can be shared between threads. - pub database: Box + Send + 'a>, + pub database: DB, //Box + Send + 'a>, /// if there is prestate that we want to use. /// This would mean that we have additional state layer between evm and disk/database. pub with_bundle_prestate: Option, @@ -28,7 +27,7 @@ pub struct StateBuilder<'a, DBError> { pub with_block_hashes: BTreeMap, } -impl Default for StateBuilder<'_, Infallible> { +impl Default for StateBuilder> { fn default() -> Self { Self { with_state_clear: true, @@ -42,16 +41,16 @@ impl Default for StateBuilder<'_, Infallible> { } } -impl<'a, DBError> StateBuilder<'a, DBError> { +impl StateBuilder { /// Create default instance of builder. - pub fn new() -> StateBuilder<'a, Infallible> { - StateBuilder::<'a, Infallible>::default() + pub fn new() -> StateBuilder> { + StateBuilder::>::default() } - pub fn with_database( + pub fn with_database_boxed<'a, NewDBError>( self, database: Box + Send + 'a>, - ) -> StateBuilder<'a, NewDBError> { + ) -> StateBuilder + Send + 'a>> { // cast to the different database, // Note that we return different type depending of the database NewDBError. StateBuilder { @@ -124,7 +123,7 @@ impl<'a, DBError> StateBuilder<'a, DBError> { } } - pub fn build(mut self) -> State<'a, DBError> { + pub fn build(mut self) -> State { let use_preloaded_bundle = if self.with_cache_prestate.is_some() { self.with_bundle_prestate = None; false From b0ee6d4ce20b7cdf9314511625eedb11e8f8fda6 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Wed, 6 Sep 2023 05:25:00 -0400 Subject: [PATCH 11/14] feat: derive PartialEq, Eq for Env (#689) --- crates/primitives/src/env.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/primitives/src/env.rs b/crates/primitives/src/env.rs index d9e41ba308..f6a774c2a7 100644 --- a/crates/primitives/src/env.rs +++ b/crates/primitives/src/env.rs @@ -5,14 +5,14 @@ use crate::{ use bytes::Bytes; use core::cmp::{min, Ordering}; -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Env { pub cfg: CfgEnv, pub block: BlockEnv, pub tx: TxEnv, } -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct BlockEnv { pub number: U256, @@ -30,7 +30,7 @@ pub struct BlockEnv { pub gas_limit: U256, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct TxEnv { /// Caller or Author or tx signer @@ -47,7 +47,7 @@ pub struct TxEnv { pub access_list: Vec<(B160, Vec)>, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub enum TransactTo { Call(B160), From ee13aacaf0edebfe0ef2d770ca593bcb1cdd3aee Mon Sep 17 00:00:00 2001 From: rakita Date: Wed, 6 Sep 2023 13:38:29 +0200 Subject: [PATCH 12/14] feat(StateBuilder): switch builder option from without_bundle to with_bundle (#688) * feat(StateBuilder): make builder option become * make private field, nit rename * fix tests --- bins/revme/src/statetest/runner.rs | 2 ++ crates/revm/src/db/states/state.rs | 6 ++-- crates/revm/src/db/states/state_builder.rs | 38 ++++++++++++---------- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/bins/revme/src/statetest/runner.rs b/bins/revme/src/statetest/runner.rs index 55c388796e..ca92b1f299 100644 --- a/bins/revme/src/statetest/runner.rs +++ b/bins/revme/src/statetest/runner.rs @@ -257,6 +257,7 @@ pub fn execute_test_suit( )); let mut state = revm::db::StateBuilder::default() .with_cached_prestate(cache) + .with_bundle_update() .build(); let mut evm = revm::new(); evm.database(&mut state); @@ -294,6 +295,7 @@ pub fn execute_test_suit( )); let mut state = revm::db::StateBuilder::default() .with_cached_prestate(cache) + .with_bundle_update() .build(); evm.database(&mut state); let _ = diff --git a/crates/revm/src/db/states/state.rs b/crates/revm/src/db/states/state.rs index 71cef87fed..4a35c89890 100644 --- a/crates/revm/src/db/states/state.rs +++ b/crates/revm/src/db/states/state.rs @@ -310,7 +310,7 @@ mod tests { /// state of the account before the block. #[test] fn reverts_preserve_old_values() { - let mut state = StateBuilder::default().build(); + let mut state = StateBuilder::default().with_bundle_update().build(); let (slot1, slot2, slot3) = (U256::from(1), U256::from(2), U256::from(3)); @@ -555,7 +555,7 @@ mod tests { /// Checks that the accounts and storages that are changed within the block and reverted to their previous state do not appear in the reverts. #[test] fn bundle_scoped_reverts_collapse() { - let mut state = StateBuilder::default().build(); + let mut state = StateBuilder::default().with_bundle_update().build(); // Non-existing account. let new_account_address = B160::from_slice(&[0x1; 20]); @@ -721,7 +721,7 @@ mod tests { /// Checks that the behavior of selfdestruct within the block is correct. #[test] fn selfdestruct_state_and_reverts() { - let mut state = StateBuilder::default().build(); + let mut state = StateBuilder::default().with_bundle_update().build(); // Existing account. let existing_account_address = B160::from_slice(&[0x1; 20]); diff --git a/crates/revm/src/db/states/state_builder.rs b/crates/revm/src/db/states/state_builder.rs index fb943815a7..d4b7914066 100644 --- a/crates/revm/src/db/states/state_builder.rs +++ b/crates/revm/src/db/states/state_builder.rs @@ -5,26 +5,28 @@ use revm_interpreter::primitives::{db::Database, B256}; /// Allows building of State and initializing it with different options. pub struct StateBuilder { - pub with_state_clear: bool, + /// Enabled state clear flag that is introduced in Spurious Dragon hardfork. + /// Default is true as spurious dragon happened long time ago. + with_state_clear: bool, /// Optional database that we use to fetch data from. If database is not present, we will /// return not existing account and storage. /// /// Note: It is marked as Send so database can be shared between threads. - pub database: DB, //Box + Send + 'a>, + database: DB, //Box + Send + 'a>, /// if there is prestate that we want to use. /// This would mean that we have additional state layer between evm and disk/database. - pub with_bundle_prestate: Option, + with_bundle_prestate: Option, /// This will initialize cache to this state. - pub with_cache_prestate: Option, + with_cache_prestate: Option, /// Do we want to create reverts and update bundle state. - /// Default is true. - pub without_bundle_update: bool, + /// Default is false. + with_bundle_update: bool, /// Do we want to merge transitions in background. /// This will allows evm to continue executing. /// Default is false. - pub with_background_transition_merge: bool, + with_background_transition_merge: bool, /// If we want to set different block hashes - pub with_block_hashes: BTreeMap, + with_block_hashes: BTreeMap, } impl Default for StateBuilder> { @@ -34,7 +36,7 @@ impl Default for StateBuilder> { database: Box::::default(), with_cache_prestate: None, with_bundle_prestate: None, - without_bundle_update: false, + with_bundle_update: false, with_background_transition_merge: false, with_block_hashes: BTreeMap::new(), } @@ -58,7 +60,7 @@ impl StateBuilder { database, with_cache_prestate: self.with_cache_prestate, with_bundle_prestate: self.with_bundle_prestate, - without_bundle_update: self.without_bundle_update, + with_bundle_update: self.with_bundle_update, with_background_transition_merge: self.with_background_transition_merge, with_block_hashes: self.with_block_hashes, } @@ -85,13 +87,13 @@ impl StateBuilder { } } - /// Don't make transitions and don't update bundle state. + /// Make transitions and update bundle state. /// - /// This is good option if we don't care about creating reverts - /// or getting output of changed states. - pub fn without_bundle_update(self) -> Self { + /// This is needed option if we want to create reverts + /// and getting output of changed states. + pub fn with_bundle_update(self) -> Self { Self { - without_bundle_update: true, + with_bundle_update: true, ..self } } @@ -135,10 +137,10 @@ impl StateBuilder { .with_cache_prestate .unwrap_or(CacheState::new(self.with_state_clear)), database: self.database, - transition_state: if self.without_bundle_update { - None - } else { + transition_state: if self.with_bundle_update { Some(TransitionState::default()) + } else { + None }, bundle_state: self.with_bundle_prestate, use_preloaded_bundle, From d04aad30144d0e55d04c0b8ef7630b418f875869 Mon Sep 17 00:00:00 2001 From: rakita Date: Wed, 6 Sep 2023 16:31:49 +0200 Subject: [PATCH 13/14] chore: expose StateDBBox (#694) --- crates/revm/src/db.rs | 2 +- crates/revm/src/db/states.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/revm/src/db.rs b/crates/revm/src/db.rs index 076a6eef5f..7f9af7a93b 100644 --- a/crates/revm/src/db.rs +++ b/crates/revm/src/db.rs @@ -12,7 +12,7 @@ pub mod states; #[cfg(feature = "std")] pub use states::{ AccountRevert, AccountStatus, BundleAccount, BundleState, CacheState, PlainAccount, - RevertToSlot, State, StateBuilder, StorageWithOriginalValues, TransitionAccount, + RevertToSlot, State, StateBuilder, StateDBBox, StorageWithOriginalValues, TransitionAccount, TransitionState, }; diff --git a/crates/revm/src/db/states.rs b/crates/revm/src/db/states.rs index f62d72c101..860f04742b 100644 --- a/crates/revm/src/db/states.rs +++ b/crates/revm/src/db/states.rs @@ -20,7 +20,7 @@ pub use cache_account::CacheAccount; pub use changes::{PlainStateReverts, PlainStorageChangeset, PlainStorageRevert, StateChangeset}; pub use plain_account::{PlainAccount, StorageWithOriginalValues}; pub use reverts::{AccountRevert, RevertToSlot}; -pub use state::State; +pub use state::{State, StateDBBox}; pub use state_builder::StateBuilder; pub use transition_account::TransitionAccount; pub use transition_state::TransitionState; From f6c9c7fe92199339ff937bbedd9bce287e0c2276 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Wed, 6 Sep 2023 18:13:17 +0200 Subject: [PATCH 14/14] chore: deprecate `RefDBWrapper` (#696) --- crates/interpreter/src/gas.rs | 2 + crates/primitives/src/db.rs | 29 +++++--- crates/revm/src/evm.rs | 120 ++++++++++++++---------------- crates/revm/src/evm_impl.rs | 10 +-- crates/revm/src/inspector/noop.rs | 2 +- 5 files changed, 82 insertions(+), 81 deletions(-) diff --git a/crates/interpreter/src/gas.rs b/crates/interpreter/src/gas.rs index 0cd45e1171..a7797186c8 100644 --- a/crates/interpreter/src/gas.rs +++ b/crates/interpreter/src/gas.rs @@ -79,6 +79,8 @@ impl Gas { /// Records an explicit cost. /// + /// Returns `false` if the gas limit is exceeded. + /// /// This function is called on every instruction in the interpreter if the feature /// `no_gas_measuring` is not enabled. #[inline(always)] diff --git a/crates/primitives/src/db.rs b/crates/primitives/src/db.rs index 15770907e0..ce3334d9b4 100644 --- a/crates/primitives/src/db.rs +++ b/crates/primitives/src/db.rs @@ -74,32 +74,41 @@ impl Database for WrapDatabaseRef { } } -pub struct RefDBWrapper<'a, Error> { - pub db: &'a dyn DatabaseRef, +/// Wraps a `dyn DatabaseRef` to provide a [`Database`] implementation. +#[doc(hidden)] +#[deprecated = "use `WrapDatabaseRef` instead"] +pub struct RefDBWrapper<'a, E> { + pub db: &'a dyn DatabaseRef, } -impl<'a, Error> RefDBWrapper<'a, Error> { - pub fn new(db: &'a dyn DatabaseRef) -> Self { +#[allow(deprecated)] +impl<'a, E> RefDBWrapper<'a, E> { + #[inline] + pub fn new(db: &'a dyn DatabaseRef) -> Self { Self { db } } } -impl<'a, Error> Database for RefDBWrapper<'a, Error> { - type Error = Error; - /// Get basic account information. +#[allow(deprecated)] +impl<'a, E> Database for RefDBWrapper<'a, E> { + type Error = E; + + #[inline] fn basic(&mut self, address: B160) -> Result, Self::Error> { self.db.basic(address) } - /// Get account code by its hash + + #[inline] fn code_by_hash(&mut self, code_hash: B256) -> Result { self.db.code_by_hash(code_hash) } - /// Get storage value of address at index. + + #[inline] fn storage(&mut self, address: B160, index: U256) -> Result { self.db.storage(address, index) } - // History related + #[inline] fn block_hash(&mut self, number: U256) -> Result { self.db.block_hash(number) } diff --git a/crates/revm/src/evm.rs b/crates/revm/src/evm.rs index 526b73fb2a..8fcd4eabbc 100644 --- a/crates/revm/src/evm.rs +++ b/crates/revm/src/evm.rs @@ -1,11 +1,12 @@ use crate::primitives::{specification, EVMError, EVMResult, Env, ExecutionResult, SpecId}; use crate::{ - db::{Database, DatabaseCommit, DatabaseRef, RefDBWrapper}, + db::{Database, DatabaseCommit, DatabaseRef}, evm_impl::{EVMImpl, Transact}, inspectors::NoOpInspector, Inspector, }; use alloc::boxed::Box; +use revm_interpreter::primitives::db::WrapDatabaseRef; use revm_interpreter::primitives::ResultAndState; use revm_precompile::Precompiles; @@ -60,6 +61,7 @@ impl EVM { self.db.as_mut().unwrap().commit(state); Ok(result) } + /// Inspect transaction and commit changes to database. pub fn inspect_commit>( &mut self, @@ -75,21 +77,17 @@ impl EVM { /// Do checks that could make transaction fail before call/create pub fn preverify_transaction(&mut self) -> Result<(), EVMError> { if let Some(db) = self.db.as_mut() { - let mut noop = NoOpInspector {}; - let out = evm_inner::(&mut self.env, db, &mut noop).preverify_transaction(); - out + evm_inner::(&mut self.env, db, &mut NoOpInspector).preverify_transaction() } else { panic!("Database needs to be set"); } } - /// Skip preverification steps and execute transaction - /// without writing to DB, return change state. + /// Skip preverification steps and execute transaction without writing to DB, return change + /// state. pub fn transact_preverified(&mut self) -> EVMResult { if let Some(db) = self.db.as_mut() { - let mut noop = NoOpInspector {}; - let out = evm_inner::(&mut self.env, db, &mut noop).transact_preverified(); - out + evm_inner::(&mut self.env, db, &mut NoOpInspector).transact_preverified() } else { panic!("Database needs to be set"); } @@ -98,9 +96,7 @@ impl EVM { /// Execute transaction without writing to DB, return change state. pub fn transact(&mut self) -> EVMResult { if let Some(db) = self.db.as_mut() { - let mut noop = NoOpInspector {}; - let out = evm_inner::(&mut self.env, db, &mut noop).transact(); - out + evm_inner::(&mut self.env, db, &mut NoOpInspector).transact() } else { panic!("Database needs to be set"); } @@ -120,13 +116,12 @@ impl<'a, DB: DatabaseRef> EVM { /// Do checks that could make transaction fail before call/create pub fn preverify_transaction_ref(&self) -> Result<(), EVMError> { if let Some(db) = self.db.as_ref() { - let mut noop = NoOpInspector {}; - let mut db = RefDBWrapper::new(db); - let db = &mut db; - let out = - evm_inner::, false>(&mut self.env.clone(), db, &mut noop) - .preverify_transaction(); - out + evm_inner::<_, false>( + &mut self.env.clone(), + &mut WrapDatabaseRef(db), + &mut NoOpInspector, + ) + .preverify_transaction() } else { panic!("Database needs to be set"); } @@ -136,13 +131,12 @@ impl<'a, DB: DatabaseRef> EVM { /// without writing to DB, return change state. pub fn transact_preverified_ref(&self) -> EVMResult { if let Some(db) = self.db.as_ref() { - let mut noop = NoOpInspector {}; - let mut db = RefDBWrapper::new(db); - let db = &mut db; - let out = - evm_inner::, false>(&mut self.env.clone(), db, &mut noop) - .transact_preverified(); - out + evm_inner::<_, false>( + &mut self.env.clone(), + &mut WrapDatabaseRef(db), + &mut NoOpInspector, + ) + .transact_preverified() } else { panic!("Database needs to be set"); } @@ -151,33 +145,29 @@ impl<'a, DB: DatabaseRef> EVM { /// Execute transaction without writing to DB, return change state. pub fn transact_ref(&self) -> EVMResult { if let Some(db) = self.db.as_ref() { - let mut noop = NoOpInspector {}; - let mut db = RefDBWrapper::new(db); - let db = &mut db; - let out = - evm_inner::, false>(&mut self.env.clone(), db, &mut noop) - .transact(); - out + evm_inner::<_, false>( + &mut self.env.clone(), + &mut WrapDatabaseRef(db), + &mut NoOpInspector, + ) + .transact() } else { panic!("Database needs to be set"); } } /// Execute transaction with given inspector, without wring to DB. Return change state. - pub fn inspect_ref>>( + pub fn inspect_ref>>( &'a self, - mut inspector: INSP, + mut inspector: I, ) -> EVMResult { if let Some(db) = self.db.as_ref() { - let mut db = RefDBWrapper::new(db); - let db = &mut db; - let out = evm_inner::, true>( + evm_inner::<_, true>( &mut self.env.clone(), - db, + &mut WrapDatabaseRef(db), &mut inspector, ) - .transact(); - out + .transact() } else { panic!("Database needs to be set"); } @@ -208,17 +198,6 @@ impl EVM { } } -macro_rules! create_evm { - ($spec:ident, $db:ident, $env:ident, $inspector:ident) => { - Box::new(EVMImpl::<'a, $spec, DB, INSPECT>::new( - $db, - $env, - $inspector, - Precompiles::new(to_precompile_id($spec::SPEC_ID)).clone(), - )) as Box + 'a> - }; -} - pub fn to_precompile_id(spec_id: SpecId) -> revm_precompile::SpecId { match spec_id { SpecId::FRONTIER @@ -247,22 +226,33 @@ pub fn evm_inner<'a, DB: Database, const INSPECT: bool>( db: &'a mut DB, insp: &'a mut dyn Inspector, ) -> Box + 'a> { + macro_rules! create_evm { + ($spec:ident) => { + Box::new(EVMImpl::<'a, $spec, DB, INSPECT>::new( + db, + env, + insp, + Precompiles::new(to_precompile_id($spec::SPEC_ID)).clone(), + )) as Box + 'a> + }; + } + use specification::*; match env.cfg.spec_id { - SpecId::FRONTIER | SpecId::FRONTIER_THAWING => create_evm!(FrontierSpec, db, env, insp), - SpecId::HOMESTEAD | SpecId::DAO_FORK => create_evm!(HomesteadSpec, db, env, insp), - SpecId::TANGERINE => create_evm!(TangerineSpec, db, env, insp), - SpecId::SPURIOUS_DRAGON => create_evm!(SpuriousDragonSpec, db, env, insp), - SpecId::BYZANTIUM => create_evm!(ByzantiumSpec, db, env, insp), - SpecId::PETERSBURG | SpecId::CONSTANTINOPLE => create_evm!(PetersburgSpec, db, env, insp), - SpecId::ISTANBUL | SpecId::MUIR_GLACIER => create_evm!(IstanbulSpec, db, env, insp), - SpecId::BERLIN => create_evm!(BerlinSpec, db, env, insp), + SpecId::FRONTIER | SpecId::FRONTIER_THAWING => create_evm!(FrontierSpec), + SpecId::HOMESTEAD | SpecId::DAO_FORK => create_evm!(HomesteadSpec), + SpecId::TANGERINE => create_evm!(TangerineSpec), + SpecId::SPURIOUS_DRAGON => create_evm!(SpuriousDragonSpec), + SpecId::BYZANTIUM => create_evm!(ByzantiumSpec), + SpecId::PETERSBURG | SpecId::CONSTANTINOPLE => create_evm!(PetersburgSpec), + SpecId::ISTANBUL | SpecId::MUIR_GLACIER => create_evm!(IstanbulSpec), + SpecId::BERLIN => create_evm!(BerlinSpec), SpecId::LONDON | SpecId::ARROW_GLACIER | SpecId::GRAY_GLACIER => { - create_evm!(LondonSpec, db, env, insp) + create_evm!(LondonSpec) } - SpecId::MERGE => create_evm!(MergeSpec, db, env, insp), - SpecId::SHANGHAI => create_evm!(ShanghaiSpec, db, env, insp), - SpecId::CANCUN => create_evm!(CancunSpec, db, env, insp), - SpecId::LATEST => create_evm!(LatestSpec, db, env, insp), + SpecId::MERGE => create_evm!(MergeSpec), + SpecId::SHANGHAI => create_evm!(ShanghaiSpec), + SpecId::CANCUN => create_evm!(CancunSpec), + SpecId::LATEST => create_evm!(LatestSpec), } } diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index c3da184cf0..da83fd3b89 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -586,7 +586,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, /// Call precompile contract fn call_precompile(&mut self, inputs: &CallInputs, mut gas: Gas) -> CallResult { - let input_data = inputs.input.clone(); + let input_data = &inputs.input; let contract = inputs.contract; let precompile = self @@ -595,8 +595,8 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, .get(&contract) .expect("Check for precompile should be already done"); let out = match precompile { - Precompile::Standard(fun) => fun(&input_data, gas.limit()), - Precompile::Custom(fun) => fun(&input_data, gas.limit()), + Precompile::Standard(fun) => fun(input_data, gas.limit()), + Precompile::Custom(fun) => fun(input_data, gas.limit()), }; match out { Ok((gas_used, data)) => { @@ -615,13 +615,13 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, } } Err(e) => { - let ret = if precompile::Error::OutOfGas == e { + let result = if precompile::Error::OutOfGas == e { InstructionResult::PrecompileOOG } else { InstructionResult::PrecompileError }; CallResult { - result: ret, + result, gas, return_value: Bytes::new(), } diff --git a/crates/revm/src/inspector/noop.rs b/crates/revm/src/inspector/noop.rs index 63a6014eb1..12123b47f3 100644 --- a/crates/revm/src/inspector/noop.rs +++ b/crates/revm/src/inspector/noop.rs @@ -3,6 +3,6 @@ use crate::{Database, Inspector}; #[derive(Clone, Copy)] -pub struct NoOpInspector(); +pub struct NoOpInspector; impl Inspector for NoOpInspector {}