diff --git a/src/lib.rs b/src/lib.rs index d166dc0..80f3c55 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,5 @@ use lazy_static::lazy_static; -use revm::primitives::{Address, EVMError, EVMResultGeneric, ExecutionResult, U256}; -use revm::TransitionAccount; +use revm::primitives::{Address, EVMError, EVMResultGeneric, EvmState, ExecutionResult, U256}; use std::cmp::min; use std::fmt::{Display, Formatter}; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -13,13 +12,14 @@ mod storage; mod tx_dependency; lazy_static! { - static ref CPU_CORES: usize = thread::available_parallelism().map(|n| n.get()).unwrap_or(8); + static ref CPU_CORES: usize = + std::thread::available_parallelism().map(|n| n.get()).unwrap_or(8); } lazy_static! { static ref GREVM_RUNTIME: Runtime = Builder::new_multi_thread() // .worker_threads(1) // for debug - .worker_threads(thread::available_parallelism().map(|n| n.get() * 2).unwrap_or(8)) + .worker_threads(std::thread::available_parallelism().map(|n| n.get() * 2).unwrap_or(8)) .thread_name("grevm-tokio-runtime") .enable_all() .build() @@ -75,7 +75,7 @@ pub struct ResultAndTransition { /// Status of execution pub result: Option, /// State that got updated - pub transition: Vec<(Address, TransitionAccount)>, + pub transition: EvmState, /// Rewards to miner pub rewards: u128, } diff --git a/src/partition.rs b/src/partition.rs index b331547..8cb3b57 100644 --- a/src/partition.rs +++ b/src/partition.rs @@ -156,7 +156,12 @@ where if read_set.is_disjoint(&update_write_set) { self.read_set.push(read_set); self.write_set.push(write_set); - evm.db_mut().temporary_commit_transition(&execute_state.transition); + // All accounts should be present inside cache. + for address in execute_state.transition.keys() { + // FIXME(gravity): error handling + let _ = evm.db_mut().load_cache_account(*address); + } + evm.db_mut().temporary_commit(&execute_state.transition); self.execute_results.push(Ok(execute_state)); should_rerun = false; self.metrics.reusable_tx_cnt += 1; @@ -182,10 +187,10 @@ where result_and_state.state.remove(&self.coinbase); } // temporary commit to cache_db, to make use the remaining txs can read the updated data - let transition = evm.db_mut().temporary_commit(result_and_state.state); + evm.db_mut().temporary_commit(&result_and_state.state); self.execute_results.push(Ok(ResultAndTransition { result: Some(result_and_state.result), - transition, + transition: result_and_state.state, rewards: rewards.unwrap_or(0), })); } diff --git a/src/scheduler.rs b/src/scheduler.rs index 7b8190a..e5ed0fa 100644 --- a/src/scheduler.rs +++ b/src/scheduler.rs @@ -9,20 +9,19 @@ use crate::hint::ParallelExecutionHints; use crate::partition::{ OrderedVectorExt, PartitionExecutor, PreRoundContext, PreUnconfirmedContext, }; -use crate::storage::SchedulerDB; +use crate::storage::{SchedulerDB, StateCache}; use crate::tx_dependency::{DependentTxsVec, TxDependency}; use crate::{ - fork_join_util, GrevmError, LocationAndType, PartitionId, TxId, CPU_CORES, GREVM_RUNTIME, - MAX_NUM_ROUND, + fork_join_util, GrevmError, LocationAndType, TxId, CPU_CORES, GREVM_RUNTIME, MAX_NUM_ROUND, }; -use metrics::{counter, gauge, histogram}; +use metrics::{counter, gauge}; use revm::db::states::bundle_state::BundleRetention; use revm::db::BundleState; use revm::primitives::{ AccountInfo, Address, Bytecode, EVMError, Env, ExecutionResult, SpecId, TxEnv, B256, U256, }; -use revm::{CacheState, DatabaseRef, EvmBuilder}; +use revm::{DatabaseRef, EvmBuilder}; pub struct ExecuteMetrics { /// Number of times parallel execution is called. @@ -460,12 +459,20 @@ where // update and pruning tx dependencies self.update_and_pruning_dependency(); - let partition_state: Vec = self + let partition_state: Vec = self .partition_executors .iter() .map(|executor| { let mut executor = executor.write().unwrap(); - std::mem::take(&mut executor.partition_db.cache) + let mut cache = std::mem::take(&mut executor.partition_db.cache); + if !executor.partition_db.miner_involved { + // Remove miner account in partitions that only treat it as a reward account. + // We have preload it into scheduler state cache, and will handle rewards + // separately. Only when miner account participates in the transactions should + // it be merged into the scheduler state cache. + cache.accounts.remove(&self.coinbase); + } + cache }) .collect(); @@ -475,14 +482,40 @@ where drop(span); let database = Arc::get_mut(&mut self.database).unwrap(); - Self::merge_not_modified_state(&mut database.cache, partition_state); + if self.num_finality_txs == self.txs.len() { + // Merge partition state to scheduler directly if there are no conflicts in the entire + // partition. + let span = Span::enter_with_local_parent("directly merge accounts"); + for state in partition_state { + database.cache.merge_accounts(state); + } + + let mut rewards: u128 = 0; + for ctx in finality_txs.into_values() { + rewards += ctx.execute_state.rewards; + self.results.push(ctx.execute_state.result.unwrap()); + } + database + .increment_balances(vec![(self.coinbase, rewards)]) + .map_err(|err| GrevmError::EvmError(EVMError::Database(err)))?; + + self.metrics.validate_time.increment(start.elapsed().as_nanos() as u64); + return Ok(()); + } + + // Must merge loaded state to database cache before commit changes + let span = Span::enter_with_local_parent("merge loaded state"); + for state in partition_state { + database.cache.merge_loaded_state(state); + } + std::mem::drop(span); let span = Span::enter_with_local_parent("commit transition to scheduler db"); let mut rewards: u128 = 0; for ctx in finality_txs.into_values() { rewards += ctx.execute_state.rewards; self.results.push(ctx.execute_state.result.unwrap()); - database.commit_transition(ctx.execute_state.transition); + database.commit(&ctx.execute_state.transition); } // Each transaction updates three accounts: from, to, and coinbase. // If every tx updates the coinbase account, it will cause conflicts across all txs. @@ -498,27 +531,6 @@ where Ok(()) } - /// Merge not modified state from partition to scheduler. These data are just loaded from - /// database, so we can merge them to state as original value for next round. - #[fastrace::trace] - fn merge_not_modified_state(state: &mut CacheState, partition_state: Vec) { - for partition in partition_state { - // merge account state that is not modified - for (address, account) in partition.accounts { - if account.status.is_not_modified() && state.accounts.get(&address).is_none() { - state.accounts.insert(address, account); - } - } - - // merge contract code - for (hash, code) in partition.contracts { - if state.contracts.get(&hash).is_none() { - state.contracts.insert(hash, code); - } - } - } - } - #[fastrace::trace] fn execute_remaining_sequential(&mut self) -> Result<(), GrevmError> { self.metrics.sequential_execute_calls.increment(1); @@ -539,7 +551,7 @@ where } match evm.transact() { Ok(result_and_state) => { - evm.db_mut().commit(result_and_state.state); + evm.db_mut().commit(&result_and_state.state); self.results.push(result_and_state.result); } Err(err) => return Err(GrevmError::EvmError(err)), @@ -562,13 +574,18 @@ where // MUST drop the `PartitionExecutor::scheduler_db` before get mut this.partition_executors.clear(); let database = Arc::get_mut(&mut this.database).unwrap(); - database.merge_transitions(BundleRetention::Reverts); - ExecuteOutput { - state: std::mem::take(&mut database.bundle_state), - results: std::mem::take(&mut this.results), - } + let state = database.create_bundle_state(BundleRetention::Reverts); + ExecuteOutput { state, results: std::mem::take(&mut this.results) } }; + { + // Preload coinbase account + let db = Arc::get_mut(&mut self.database).unwrap(); + let _ = db + .load_cache_account(self.coinbase) + .map_err(|err| GrevmError::EvmError(EVMError::Database(err)))?; + } + if self.txs.len() < self.num_partitions && !force_parallel { self.execute_remaining_sequential()?; return Ok(build_execute_output(self)); diff --git a/src/storage.rs b/src/storage.rs index 2e7ce3d..0c2a261 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -1,30 +1,235 @@ use crate::LocationAndType; use revm::db::states::bundle_state::BundleRetention; -use revm::db::states::CacheAccount; -use revm::db::{BundleState, PlainAccount}; +use revm::db::states::StorageSlot; +use revm::db::{AccountStatus, BundleState, StorageWithOriginalValues}; use revm::precompile::Address; use revm::primitives::{Account, AccountInfo, Bytecode, EvmState, B256, BLOCK_HASH_HISTORY, U256}; -use revm::{CacheState, Database, DatabaseRef, TransitionAccount, TransitionState}; +use revm::{Database, DatabaseRef, TransitionAccount}; use std::collections::{btree_map, hash_map, BTreeMap, HashMap, HashSet}; use std::sync::Arc; +#[derive(Default)] +pub(crate) struct StateCache { + /// Account state. + pub accounts: HashMap, + /// Loaded contracts. + pub contracts: HashMap, +} + +impl StateCache { + fn apply_evm_state(&mut self, changes: &EvmState) { + for (address, account) in changes { + self.apply_account_state(address, account); + } + } + + /// Refer to `CacheState::apply_evm_state` + fn apply_account_state(&mut self, address: &Address, account: &Account) { + // not touched account are never changed. + if !account.is_touched() { + return; + } + + let this_account = + self.accounts.get_mut(address).expect("All accounts should be present inside cache"); + + // If it is marked as selfdestructed inside revm + // we need to changed state to destroyed. + if account.is_selfdestructed() { + return this_account.selfdestruct(); + } + + let is_created = account.is_created(); + let is_empty = account.is_empty(); + + let changed_storage = + account.storage.iter().filter(|(_, slot)| slot.is_changed()).map(|(k, slot)| { + ( + *k, + StorageSlot { + previous_or_original_value: slot.original_value, + present_value: slot.present_value, + }, + ) + }); + + if is_created { + this_account.newly_created(account.info.clone(), changed_storage.collect()); + return; + } + + if is_empty { + // TODO(gravity): has_state_clear + + // if account is empty and state clear is not enabled we should save + // empty account. + this_account.touch_create_pre_eip161(changed_storage.collect()); + return; + } + + this_account.change(account.info.clone(), changed_storage.collect()); + return; + } + + /// Merge loaded state from other as initial state. + pub(crate) fn merge_loaded_state(&mut self, other: Self) { + for (address, account) in other.accounts { + if self.accounts.contains_key(&address) { + continue; + } + + let (info, status) = if account.status.is_not_modified() { + (account.info, account.status) + } else { + (account.original_info, account.original_status) + }; + // TODO(gravity_nekomoto): Also merge storage? However modified storage will be + // filled by call `apply_account_state`. + self.accounts.insert(address, CachedAccount { info, status, ..Default::default() }); + } + + for (code_hash, code) in other.contracts { + if !self.contracts.contains_key(&code_hash) { + self.contracts.insert(code_hash, code); + } + } + } + + pub(crate) fn merge_accounts(&mut self, other: Self) { + for (address, account) in other.accounts { + match self.accounts.entry(address) { + hash_map::Entry::Occupied(mut entry) => { + let this_account = entry.get_mut(); + if this_account.status.is_not_modified() { + this_account.original_info = this_account.info.take(); + this_account.original_status = this_account.status; + } + this_account.info = account.info; + this_account.status = account.status; + if account.storage_was_destroyed { + this_account.storage = account.storage; + } else { + this_account.storage.extend( + account.storage.into_iter().filter(|(_, slot)| slot.is_changed()), + ); + } + this_account.storage_was_destroyed = account.storage_was_destroyed; + } + hash_map::Entry::Vacant(entry) => { + entry.insert(account); + } + } + } + } +} + +#[derive(Debug, Clone, Default)] +pub(crate) struct CachedAccount { + info: Option, + status: AccountStatus, + /// Only present if account is modified. + original_info: Option, + original_status: AccountStatus, + storage: StorageWithOriginalValues, + storage_was_destroyed: bool, +} + +impl CachedAccount { + /// Refer to `CacheAccount::increment_balance` + fn increment_balance(&mut self, balance: u128) { + let had_no_nonce_and_code = + self.info.as_ref().map(AccountInfo::has_no_code_and_nonce).unwrap_or_default(); + + if self.status.is_not_modified() { + self.original_info = self.info.clone(); + self.original_status = self.status; + } + + if self.info.is_none() { + self.info = Some(AccountInfo::default()); + } + + self.status = self.status.on_changed(had_no_nonce_and_code); + let info = self.info.as_mut().unwrap(); + info.balance = info.balance.saturating_add(U256::from(balance)); + } + + /// Refer to `CacheAccount::selfdestruct` + fn selfdestruct(&mut self) { + self.status = self.status.on_selfdestructed(); + if self.status.is_not_modified() { + self.original_info = self.info.take(); + self.original_status = self.status; + } + self.info = None; + self.storage = HashMap::default(); + self.storage_was_destroyed = true; + } + + /// Refer to `CacheAccount::newly_created` + fn newly_created(&mut self, new_info: AccountInfo, new_storage: StorageWithOriginalValues) { + self.status = self.status.on_created(); + if self.status.is_not_modified() { + self.original_info = self.info.take(); + self.original_status = self.status; + } + self.info = Some(new_info); + self.storage = new_storage; + } + + /// Refer to `CacheAccount::touch_create_pre_eip161` + fn touch_create_pre_eip161(&mut self, storage: StorageWithOriginalValues) { + let previous_status = self.status; + + let had_no_info = + self.info.as_ref().map(|a: &AccountInfo| a.is_empty()).unwrap_or_default(); + if let Some(status) = previous_status.on_touched_created_pre_eip161(had_no_info) { + self.status = status; + } else { + // account status didn't change + return; + } + + if previous_status.is_not_modified() { + self.original_info = self.info.take(); + self.original_status = previous_status; + } + self.info = Some(AccountInfo::default()); + self.storage = storage; + } + + /// Refer to `CacheAccount::change` + fn change(&mut self, new: AccountInfo, storage: Vec<(U256, StorageSlot)>) { + let had_no_nonce_and_code = + self.info.as_ref().map(AccountInfo::has_no_code_and_nonce).unwrap_or_default(); + + if self.status.is_not_modified() { + self.original_info = self.info.take(); + self.original_status = self.status; + } + + self.status = self.status.on_changed(had_no_nonce_and_code); + self.info = Some(new); + for (key, slot) in storage { + match self.storage.entry(key) { + hash_map::Entry::Occupied(mut entry) => { + entry.get_mut().present_value = slot.present_value; + } + hash_map::Entry::Vacant(entry) => { + entry.insert(slot); + } + } + } + } +} + pub(crate) struct SchedulerDB { /// Cache the committed data of finality txns and the read-only data during execution after each /// round of execution. Used as the initial state for the next round of partition executors. /// When fall back to sequential execution, used as cached state contains both changed from evm /// execution and cached/loaded account/storages. - pub cache: CacheState, + pub cache: StateCache, pub database: DB, - /// Block state, it aggregates transactions transitions into one state. - /// - /// Build reverts and state that gets applied to the state. - // TODO(gravity_nekomoto): Try to directly generate bundle state from cache, rather than - // transitions. - pub transition_state: Option, - /// After block is finishes we merge those changes inside bundle. - /// Bundle is used to update database and create changesets. - /// Bundle state can be set on initialization if we want to use preloaded bundle. - pub bundle_state: BundleState, /// If EVM asks for block hash we will first check if they are found here. /// and then ask the database. /// @@ -35,43 +240,60 @@ pub(crate) struct SchedulerDB { impl SchedulerDB { pub(crate) fn new(database: DB) -> Self { - Self { - cache: CacheState::default(), - database, - transition_state: Some(TransitionState::default()), - bundle_state: BundleState::default(), - block_hashes: BTreeMap::new(), - } + Self { cache: StateCache::default(), database, block_hashes: BTreeMap::new() } } - /// Commit transitions to the cache state and transition state after one round of execution. - pub(crate) fn commit_transition(&mut self, transitions: Vec<(Address, TransitionAccount)>) { - apply_transition_to_cache(&mut self.cache, &transitions); - self.apply_transition(transitions); + pub(crate) fn commit(&mut self, changes: &EvmState) { + self.cache.apply_evm_state(changes); } - /// Fall back to sequential execute. - pub(crate) fn commit(&mut self, changes: HashMap) { - let transitions = self.cache.apply_evm_state(changes); - self.apply_transition(transitions); - } + /// Refer to `BundleState::apply_transitions_and_create_reverts` + #[fastrace::trace] + pub(crate) fn create_bundle_state(&mut self, retention: BundleRetention) -> BundleState { + let mut state = BundleState::default(); - fn apply_transition(&mut self, transitions: Vec<(Address, TransitionAccount)>) { - // add transition to transition state. - if let Some(s) = self.transition_state.as_mut() { - s.add_transitions(transitions) - } - } + let include_reverts = retention.includes_reverts(); + // pessimistically pre-allocate assuming _all_ accounts changed. + let reverts_capacity = if include_reverts { self.cache.accounts.len() } else { 0 }; + let mut reverts = Vec::with_capacity(reverts_capacity); - /// Take all transitions and merge them inside bundle state. - /// This action will create final post state and all reverts so that - /// we at any time revert state of bundle to the state before transition - /// is applied. - #[fastrace::trace] - pub(crate) fn merge_transitions(&mut self, retention: BundleRetention) { - if let Some(transition_state) = self.transition_state.as_mut().map(TransitionState::take) { - self.bundle_state.apply_transitions_and_create_reverts(transition_state, retention); + for (address, account) in std::mem::take(&mut self.cache.accounts) { + if account.status.is_not_modified() { + continue; + } + + let transition = TransitionAccount { + info: account.info, + status: account.status, + previous_info: account.original_info, + previous_status: account.original_status, + storage: account + .storage + .into_iter() + .filter(|(_, slot)| slot.is_changed()) + .collect(), + storage_was_destroyed: account.storage_was_destroyed, + }; + + // add new contract if it was created/changed. + if let Some((hash, new_bytecode)) = transition.has_new_contract() { + state.contracts.insert(hash, new_bytecode.clone()); + } + + let present_bundle = transition.present_bundle_account(); + let revert = transition.create_revert(); + if let Some(revert) = revert { + state.state_size += present_bundle.size_hint(); + state.state.insert(address, present_bundle); + if include_reverts { + reverts.push((address, revert)); + } + } } + + state.reverts.push(reverts); + + state } } @@ -79,11 +301,14 @@ impl SchedulerDB where DB: DatabaseRef, { - fn load_cache_account(&mut self, address: Address) -> Result<&mut CacheAccount, DB::Error> { + pub(crate) fn load_cache_account( + &mut self, + address: Address, + ) -> Result<&mut CachedAccount, DB::Error> { match self.cache.accounts.entry(address) { hash_map::Entry::Vacant(entry) => { let info = self.database.basic_ref(address)?; - Ok(entry.insert(into_cache_account(info))) + Ok(entry.insert(into_cached_account(info))) } hash_map::Entry::Occupied(entry) => Ok(entry.into_mut()), } @@ -96,37 +321,42 @@ where balances: impl IntoIterator, ) -> Result<(), DB::Error> { // make transition and update cache state - let mut transitions = Vec::new(); for (address, balance) in balances { if balance == 0 { continue; } let cache_account = self.load_cache_account(address)?; - transitions.push(( - address, - cache_account.increment_balance(balance).expect("Balance is not zero"), - )) - } - // append transition - if let Some(s) = self.transition_state.as_mut() { - s.add_transitions(transitions) + cache_account.increment_balance(balance); } Ok(()) } } -fn into_cache_account(account: Option) -> CacheAccount { +fn into_cached_account(account: Option) -> CachedAccount { match account { - None => CacheAccount::new_loaded_not_existing(), - Some(acc) if acc.is_empty() => CacheAccount::new_loaded_empty_eip161(HashMap::new()), - Some(acc) => CacheAccount::new_loaded(acc, HashMap::new()), + // refer to `CacheAccount::new_loaded_not_existing` + None => CachedAccount { + info: None, + status: AccountStatus::LoadedNotExisting, + ..Default::default() + }, + // refer to `CacheAccount::new_loaded_empty_eip161` + Some(acc) if acc.is_empty() => CachedAccount { + info: Some(AccountInfo::default()), + status: AccountStatus::LoadedEmptyEIP161, + ..Default::default() + }, + // refer to `CacheAccount::new_loaded` + Some(acc) => { + CachedAccount { info: Some(acc), status: AccountStatus::Loaded, ..Default::default() } + } } } /// Get storage value of address at index. fn load_storage( - cache: &mut CacheState, + cache: &mut StateCache, database: &DB, address: Address, index: U256, @@ -136,67 +366,22 @@ fn load_storage( if let Some(account) = cache.accounts.get_mut(&address) { // account will always be some, but if it is not, U256::ZERO will be returned. let is_storage_known = account.status.is_storage_known(); - Ok(account - .account - .as_mut() - .map(|account| match account.storage.entry(index) { - hash_map::Entry::Occupied(entry) => Ok(*entry.get()), - hash_map::Entry::Vacant(entry) => { - // if account was destroyed or account is newly built - // we return zero and don't ask database. - let value = if is_storage_known { - U256::ZERO - } else { - tokio::task::block_in_place(|| database.storage_ref(address, index))? - }; - entry.insert(value); - Ok(value) - } - }) - .transpose()? - .unwrap_or_default()) - } else { - unreachable!("For accessing any storage account is guaranteed to be loaded beforehand") - } -} - -fn apply_transition_to_cache( - cache: &mut CacheState, - transitions: &Vec<(Address, TransitionAccount)>, -) { - for (address, account) in transitions { - let new_storage = account.storage.iter().map(|(k, s)| (*k, s.present_value)); - if let Some(entry) = cache.accounts.get_mut(address) { - if let Some(new_info) = &account.info { - assert!(!account.storage_was_destroyed); - if let Some(read_account) = entry.account.as_mut() { - // account is loaded - read_account.info = new_info.clone(); - read_account.storage.extend(new_storage); + match account.storage.entry(index) { + hash_map::Entry::Occupied(entry) => Ok(entry.get().present_value()), + hash_map::Entry::Vacant(entry) => { + // if account was destroyed or account is newly built + // we return zero and don't ask database. + let value = if is_storage_known { + U256::ZERO } else { - // account is loaded not existing - entry.account = Some(PlainAccount { - info: new_info.clone(), - storage: new_storage.collect(), - }); - } - } else { - assert!(account.storage_was_destroyed); - entry.account = None; + tokio::task::block_in_place(|| database.storage_ref(address, index))? + }; + entry.insert(StorageSlot::new(value)); + Ok(value) } - entry.status = account.status; - } else { - cache.accounts.insert( - *address, - CacheAccount { - account: account.info.as_ref().map(|info| PlainAccount { - info: info.clone(), - storage: new_storage.collect(), - }), - status: account.status, - }, - ); } + } else { + unreachable!("For accessing any storage account is guaranteed to be loaded beforehand") } } @@ -208,7 +393,7 @@ where type Error = DB::Error; fn basic(&mut self, address: Address) -> Result, Self::Error> { - self.load_cache_account(address).map(|account| account.account_info()) + self.load_cache_account(address).map(|account| account.info.clone()) } fn code_by_hash(&mut self, code_hash: B256) -> Result { @@ -253,7 +438,7 @@ pub(crate) struct PartitionDB { pub coinbase: Address, // partition internal cache - pub cache: CacheState, + pub cache: StateCache, pub scheduler_db: Arc>, pub block_hashes: BTreeMap, @@ -263,11 +448,11 @@ pub(crate) struct PartitionDB { tx_read_set: HashSet, } -impl PartitionDB { +impl PartitionDB { pub(crate) fn new(coinbase: Address, scheduler_db: Arc>) -> Self { Self { coinbase, - cache: CacheState::default(), + cache: StateCache::default(), scheduler_db, block_hashes: BTreeMap::new(), miner_involved: false, @@ -304,9 +489,9 @@ impl PartitionDB { // we should set rewards = 0 if self.coinbase == *address && !self.miner_involved { match self.cache.accounts.get(address) { - Some(miner) => match miner.account.as_ref() { + Some(miner) => match miner.info.as_ref() { Some(miner) => { - rewards = Some((account.info.balance - miner.info.balance).to()); + rewards = Some((account.info.balance - miner.balance).to()); miner_updated = true; } // LoadedNotExisting @@ -325,15 +510,12 @@ impl PartitionDB { let mut new_contract_account = false; if match self.cache.accounts.get(address) { - Some(read_account) => { - read_account.account.as_ref().map_or(true, |read_account| { - new_contract_account = - has_code && read_account.info.is_empty_code_hash(); - new_contract_account - || read_account.info.nonce != account.info.nonce - || read_account.info.balance != account.info.balance - }) - } + Some(read_account) => read_account.info.as_ref().map_or(true, |read_account| { + new_contract_account = has_code && read_account.is_empty_code_hash(); + new_contract_account + || read_account.nonce != account.info.nonce + || read_account.balance != account.info.balance + }), None => { new_contract_account = has_code; true @@ -355,19 +537,36 @@ impl PartitionDB { (write_set, rewards) } - /// Temporary commit the state change after evm.transact() for each tx - pub(crate) fn temporary_commit( - &mut self, - changes: EvmState, - ) -> Vec<(Address, TransitionAccount)> { - self.cache.apply_evm_state(changes) + /// Temporary commit the state change after evm.transact() for each tx. + pub(crate) fn temporary_commit(&mut self, changes: &EvmState) { + self.cache.apply_evm_state(changes); } - pub(crate) fn temporary_commit_transition( + pub(crate) fn load_cache_account( &mut self, - transitions: &Vec<(Address, TransitionAccount)>, - ) { - apply_transition_to_cache(&mut self.cache, transitions); + address: Address, + ) -> Result<&mut CachedAccount, DB::Error> { + // 1. read from internal cache + match self.cache.accounts.entry(address) { + hash_map::Entry::Vacant(entry) => { + // 2. read initial state of this round from scheduler cache + if let Some(account) = self.scheduler_db.cache.accounts.get(&address) { + Ok(entry.insert(CachedAccount { + info: account.info.clone(), + status: account.status, + storage: account.storage.clone(), + ..Default::default() + })) + } else { + // 3. read from origin database + let info = tokio::task::block_in_place(|| { + self.scheduler_db.database.basic_ref(address) + })?; + Ok(entry.insert(into_cached_account(info))) + } + } + hash_map::Entry::Occupied(entry) => Ok(entry.into_mut()), + } } } @@ -383,20 +582,7 @@ where self.tx_read_set.insert(LocationAndType::Basic(address)); } - // 1. read from internal cache - let result = match self.cache.accounts.entry(address) { - hash_map::Entry::Vacant(entry) => { - // 2. read initial state of this round from scheduler cache - if let Some(account) = self.scheduler_db.cache.accounts.get(&address) { - Ok(entry.insert(account.clone()).account_info()) - } else { - // 3. read from origin database - tokio::task::block_in_place(|| self.scheduler_db.database.basic_ref(address)) - .map(|info| entry.insert(into_cache_account(info)).account_info()) - } - } - hash_map::Entry::Occupied(entry) => Ok(entry.get().account_info()), - }; + let result = self.load_cache_account(address).map(|account| account.info.clone()); if let Ok(account) = &result { if let Some(info) = account { if !info.is_empty_code_hash() { diff --git a/tests/common/execute.rs b/tests/common/execute.rs index 2193375..a12a981 100644 --- a/tests/common/execute.rs +++ b/tests/common/execute.rs @@ -17,8 +17,6 @@ use std::sync::Arc; use std::time::Instant; fn compare_bundle_state(left: &BundleState, right: &BundleState) { - let left = left.clone(); - let right = right.clone(); assert!( left.contracts.keys().all(|k| right.contracts.contains_key(k)), "Left contracts: {:?}, Right contracts: {:?}", @@ -33,22 +31,21 @@ fn compare_bundle_state(left: &BundleState, right: &BundleState) { right.contracts.keys() ); - let left_state: BTreeMap = left.state.into_iter().collect(); - let right_state: BTreeMap = right.state.into_iter().collect(); + let left_state: BTreeMap<&Address, &BundleAccount> = left.state.iter().collect(); + let right_state: BTreeMap<&Address, &BundleAccount> = right.state.iter().collect(); assert_eq!(left_state.len(), right_state.len()); for ((addr1, account1), (addr2, account2)) in left_state.into_iter().zip(right_state.into_iter()) { assert_eq!(addr1, addr2); - let BundleAccount { info, original_info, storage, status } = account1; - assert_eq!(info, account2.info); - assert_eq!(original_info, account2.original_info); - assert_eq!(status, account2.status); - let left_storage: BTreeMap = storage.into_iter().collect(); - let right_storage: BTreeMap = account2.storage.into_iter().collect(); + assert_eq!(account1.info, account2.info, "Address: {:?}", addr1); + assert_eq!(account1.original_info, account2.original_info, "Address: {:?}", addr1); + assert_eq!(account1.status, account2.status, "Address: {:?}", addr1); + let left_storage: BTreeMap<&U256, &StorageSlot> = account1.storage.iter().collect(); + let right_storage: BTreeMap<&U256, &StorageSlot> = account2.storage.iter().collect(); for (s1, s2) in left_storage.into_iter().zip(right_storage.into_iter()) { - assert_eq!(s1, s2); + assert_eq!(s1, s2, "Address: {:?}", addr1); } } }