From d2d8ca78601d3537d16a4366eff3dd1f00da2500 Mon Sep 17 00:00:00 2001 From: Arkadiy Paronyan Date: Thu, 20 Oct 2016 16:49:27 +0200 Subject: [PATCH] Don't add empty accounts to bloom (#2753) --- ethcore/src/account.rs | 9 +++++++++ ethcore/src/snapshot/mod.rs | 9 +++++++-- ethcore/src/state.rs | 25 +++++++++++++++---------- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index fa6fedae69e..8511a5b4b55 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -288,6 +288,15 @@ impl Account { /// Determine whether there are any un-`commit()`-ed storage-setting operations. pub fn storage_is_clean(&self) -> bool { self.storage_changes.is_empty() } + /// Check if account has zero nonce, balance, no code and no storage. + pub fn is_empty(&self) -> bool { + self.storage_changes.is_empty() && + self.balance.is_zero() && + self.nonce.is_zero() && + self.storage_root == SHA3_NULL_RLP && + self.code_hash == SHA3_EMPTY + } + #[cfg(test)] /// return the storage root associated with this account or None if it has been altered via the overlay. pub fn storage_root(&self) -> Option<&H256> { if self.storage_is_clean() {Some(&self.storage_root)} else {None} } diff --git a/ethcore/src/snapshot/mod.rs b/ethcore/src/snapshot/mod.rs index 21d03e51a4a..51cc82045cc 100644 --- a/ethcore/src/snapshot/mod.rs +++ b/ethcore/src/snapshot/mod.rs @@ -27,7 +27,7 @@ use ids::BlockID; use views::BlockView; use super::state_db::StateDB; -use util::{Bytes, Hashable, HashDB, snappy, TrieDB, TrieDBMut, TrieMut, BytesConvertable}; +use util::{Bytes, Hashable, HashDB, snappy, TrieDB, TrieDBMut, TrieMut, BytesConvertable, U256, Uint}; use util::Mutex; use util::hash::{FixedHash, H256}; use util::journaldb::{self, Algorithm, JournalDB}; @@ -39,6 +39,8 @@ use self::account::Account; use self::block::AbridgedBlock; use self::io::SnapshotWriter; +use super::account::Account as StateAccount; + use crossbeam::{scope, ScopedJoinHandle}; use rand::{Rng, OsRng}; @@ -417,6 +419,7 @@ impl StateRebuilder { /// Feed an uncompressed state chunk into the rebuilder. pub fn feed(&mut self, chunk: &[u8]) -> Result<(), ::error::Error> { let rlp = UntrustedRlp::new(chunk); + let empty_rlp = StateAccount::new_basic(U256::zero(), U256::zero()).rlp(); let account_fat_rlps: Vec<_> = rlp.iter().map(|r| r.as_raw()).collect(); let mut pairs = Vec::with_capacity(rlp.item_count()); let backing = self.db.backing().clone(); @@ -464,7 +467,9 @@ impl StateRebuilder { }; for (hash, thin_rlp) in pairs { - bloom.set(hash.as_slice()); + if &thin_rlp[..] != &empty_rlp[..] { + bloom.set(hash.as_slice()); + } try!(account_trie.insert(&hash, &thin_rlp)); } } diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index f77ba023f28..d9f6e540ccb 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -335,18 +335,20 @@ impl State { /// Determine whether an account exists. pub fn exists(&self, a: &Address) -> bool { - self.ensure_cached(a, RequireCache::None, |a| a.is_some()) + // Bloom filter does not contain empty accounts, so it is important here to + // check if account exists in the database directly before EIP-158 is in effect. + self.ensure_cached(a, RequireCache::None, false, |a| a.is_some()) } /// Get the balance of account `a`. pub fn balance(&self, a: &Address) -> U256 { - self.ensure_cached(a, RequireCache::None, + self.ensure_cached(a, RequireCache::None, true, |a| a.as_ref().map_or(U256::zero(), |account| *account.balance())) } /// Get the nonce of account `a`. pub fn nonce(&self, a: &Address) -> U256 { - self.ensure_cached(a, RequireCache::None, + self.ensure_cached(a, RequireCache::None, true, |a| a.as_ref().map_or(self.account_start_nonce, |account| *account.nonce())) } @@ -403,18 +405,18 @@ impl State { /// Get accounts' code. pub fn code(&self, a: &Address) -> Option> { - self.ensure_cached(a, RequireCache::Code, + self.ensure_cached(a, RequireCache::Code, true, |a| a.as_ref().map_or(None, |a| a.code().clone())) } pub fn code_hash(&self, a: &Address) -> H256 { - self.ensure_cached(a, RequireCache::None, + self.ensure_cached(a, RequireCache::None, true, |a| a.as_ref().map_or(SHA3_EMPTY, |a| a.code_hash())) } /// Get accounts' code size. pub fn code_size(&self, a: &Address) -> Option { - self.ensure_cached(a, RequireCache::CodeSize, + self.ensure_cached(a, RequireCache::CodeSize, true, |a| a.as_ref().and_then(|a| a.code_size())) } @@ -492,7 +494,9 @@ impl State { for (address, ref mut a) in accounts.iter_mut().filter(|&(_, ref a)| a.is_dirty()) { match a.account { Some(ref mut account) => { - db.note_account_bloom(&address); + if !account.is_empty() { + db.note_account_bloom(&address); + } let mut account_db = AccountDBMut::from_hash(db.as_hashdb_mut(), account.address_hash(address)); account.commit_storage(trie_factory, &mut account_db); account.commit_code(&mut account_db); @@ -545,7 +549,6 @@ impl State { pub fn populate_from(&mut self, accounts: PodState) { assert!(self.snapshots.borrow().is_empty()); for (add, acc) in accounts.drain().into_iter() { - self.db.note_account_bloom(&add); self.cache.borrow_mut().insert(add, AccountEntry::new_dirty(Some(Account::from_pod(acc)))); } } @@ -565,7 +568,7 @@ impl State { fn query_pod(&mut self, query: &PodState) { for (address, pod_account) in query.get().into_iter() - .filter(|&(ref a, _)| self.ensure_cached(a, RequireCache::Code, |a| a.is_some())) + .filter(|&(ref a, _)| self.ensure_cached(a, RequireCache::Code, true, |a| a.is_some())) { // needs to be split into two parts for the refcell code here // to work. @@ -601,7 +604,7 @@ impl State { /// Check caches for required data /// First searches for account in the local, then the shared cache. /// Populates local cache if nothing found. - fn ensure_cached(&self, a: &Address, require: RequireCache, f: F) -> U + fn ensure_cached(&self, a: &Address, require: RequireCache, check_bloom: bool, f: F) -> U where F: Fn(Option<&Account>) -> U { // check local cache first if let Some(ref mut maybe_acc) = self.cache.borrow_mut().get_mut(a) { @@ -621,6 +624,8 @@ impl State { match result { Some(r) => r, None => { + // first check bloom if it is not in database for sure + if check_bloom && !self.db.check_account_bloom(a) { return f(None); } // not found in the global cache, get from the DB and insert into local if !self.db.check_account_bloom(a) { return f(None); } let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR);