Don't add empty accounts to bloom (#2753)

ethcore/src/snapshot/mod.rs

@@ -29,7 +29,7 @@ use engines::Engine;
 use ids::BlockID;
 use views::BlockView;
 
-use util::{Bytes, Hashable, HashDB, snappy};
+use util::{Bytes, Hashable, HashDB, snappy, U256, Uint};
 use util::memorydb::MemoryDB;
 use util::Mutex;
 use util::hash::{FixedHash, H256};
@@ -44,6 +44,7 @@ use self::block::AbridgedBlock;
 use self::io::SnapshotWriter;
 
 use super::state_db::StateDB;
+use super::state::Account as StateAccount;
 
 use crossbeam::{scope, ScopedJoinHandle};
 use rand::{Rng, OsRng};
@@ -409,6 +410,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());
 
@@ -476,7 +478,9 @@ impl StateRebuilder {
 			};
 
 			for (hash, thin_rlp) in pairs {
+				if &thin_rlp[..] != &empty_rlp[..] {
 					bloom.set(&*hash);
+				}
 				try!(account_trie.insert(&hash, &thin_rlp));
 			}
 		}

ethcore/src/state/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} }

ethcore/src/state/mod.rs

@@ -340,18 +340,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()))
 	}
 
@@ -415,18 +417,18 @@ impl State {
 
 	/// Get accounts' code.
 	pub fn code(&self, a: &Address) -> Option<Arc<Bytes>> {
-		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<usize> {
-		self.ensure_cached(a, RequireCache::CodeSize,
+		self.ensure_cached(a, RequireCache::CodeSize, true,
 			|a| a.as_ref().and_then(|a| a.code_size()))
 	}
 
@@ -505,7 +507,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) => {
+					if !account.is_empty() {
 						db.note_account_bloom(&address);
+					}
 					let addr_hash = account.address_hash(address);
 					let mut account_db = factories.accountdb.create(db.as_hashdb_mut(), addr_hash);
 					account.commit_storage(&factories.trie, account_db.as_hashdb_mut());
@@ -559,7 +563,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))));
 		}
 	}
@@ -579,7 +582,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.
@@ -613,7 +616,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<F, U>(&self, a: &Address, require: RequireCache, f: F) -> U
+	fn ensure_cached<F, U>(&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) {
@@ -636,7 +639,7 @@ impl State {
 			Some(r) => r,
 			None => {
 				// first check bloom if it is not in database for sure
-				if !self.db.check_account_bloom(a) { return f(None); }
+				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
 				let db = self.factories.trie.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR);