From bbaf5ed4f5991cd4b0c2d7c49ee33817d3334d22 Mon Sep 17 00:00:00 2001 From: Arkadiy Paronyan Date: Fri, 7 Oct 2016 11:49:48 +0200 Subject: [PATCH] Backports into beta (#2512) * RocksDB version bump * Preserve cache on reverting the snapshot (#2488) * Preserve cache on reverting the snapshot * Renamed merge_with into replace_with * Renamed and documented snapshotting methods * Track dirty accounts in the state (#2461) * State to track dirty accounts * Removed clone_for_snapshot * Renaming stuff * Documentation and other minor fixes * Replaced MaybeAccount with Option * Adjustable stack size for EVM (#2483) * stack size for io workers & evm threshold * rust way to remember stack size * right value * 24kb size * some stack reduction * Fixed overflow panic in handshake_panic (#2495) --- Cargo.lock | 21 +-- ethcore/src/account.rs | 63 ++------- ethcore/src/executive.rs | 28 ++-- ethcore/src/state.rs | 278 +++++++++++++++++++++++++-------------- ethcore/src/state_db.rs | 2 +- util/io/src/lib.rs | 2 + util/io/src/worker.rs | 13 +- util/network/src/host.rs | 3 +- 8 files changed, 237 insertions(+), 173 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ac1dee3c4..5f96450a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -237,7 +237,7 @@ version = "0.5.4" source = "git+https://github.com/ethcore/rust-secp256k1#a9a0b1be1f39560ca86e8fc8e55e205a753ff25c" dependencies = [ "arrayvec 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)", - "gcc 0.3.28 (registry+https://github.com/rust-lang/crates.io-index)", + "gcc 0.3.35 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", @@ -572,8 +572,11 @@ dependencies = [ [[package]] name = "gcc" -version = "0.3.28" +version = "0.3.35" source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "rayon 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", +] [[package]] name = "glob" @@ -886,7 +889,7 @@ name = "nanomsg-sys" version = "0.5.0" source = "git+https://github.com/ethcore/nanomsg.rs.git#c40fe442c9afaea5b38009a3d992ca044dcceb00" dependencies = [ - "gcc 0.3.28 (registry+https://github.com/rust-lang/crates.io-index)", + "gcc 0.3.35 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1222,7 +1225,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "rocksdb" version = "0.4.5" -source = "git+https://github.com/ethcore/rust-rocksdb#485dd747a2c9a9f910fc8ac696fc9edf5fa22aa3" +source = "git+https://github.com/ethcore/rust-rocksdb#ffc7c82380fe8569f85ae6743f7f620af2d4a679" dependencies = [ "libc 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", "rocksdb-sys 0.3.0 (git+https://github.com/ethcore/rust-rocksdb)", @@ -1231,9 +1234,9 @@ dependencies = [ [[package]] name = "rocksdb-sys" version = "0.3.0" -source = "git+https://github.com/ethcore/rust-rocksdb#485dd747a2c9a9f910fc8ac696fc9edf5fa22aa3" +source = "git+https://github.com/ethcore/rust-rocksdb#ffc7c82380fe8569f85ae6743f7f620af2d4a679" dependencies = [ - "gcc 0.3.28 (registry+https://github.com/rust-lang/crates.io-index)", + "gcc 0.3.35 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -1264,7 +1267,7 @@ name = "rust-crypto" version = "0.2.36" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "gcc 0.3.28 (registry+https://github.com/rust-lang/crates.io-index)", + "gcc 0.3.35 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1332,7 +1335,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" name = "sha3" version = "0.1.0" dependencies = [ - "gcc 0.3.28 (registry+https://github.com/rust-lang/crates.io-index)", + "gcc 0.3.35 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -1669,7 +1672,7 @@ dependencies = [ "checksum elastic-array 0.4.0 (git+https://github.com/ethcore/elastic-array)" = "" "checksum env_logger 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "aba65b63ffcc17ffacd6cf5aa843da7c5a25e3bd4bbe0b7def8b214e411250e5" "checksum eth-secp256k1 0.5.4 (git+https://github.com/ethcore/rust-secp256k1)" = "" -"checksum gcc 0.3.28 (registry+https://github.com/rust-lang/crates.io-index)" = "3da3a2cbaeb01363c8e3704fd9fd0eb2ceb17c6f27abd4c1ef040fb57d20dc79" +"checksum gcc 0.3.35 (registry+https://github.com/rust-lang/crates.io-index)" = "91ecd03771effb0c968fd6950b37e89476a578aaf1c70297d8e92b6516ec3312" "checksum glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "8be18de09a56b60ed0edf84bc9df007e30040691af7acd1c41874faac5895bfb" "checksum hamming 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "65043da274378d68241eb9a8f8f8aa54e349136f7b8e12f63e3ef44043cc30e1" "checksum heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)" = "abb306abb8d398e053cfb1b3e7b72c2f580be048b85745c52652954f8ad1439c" diff --git a/ethcore/src/account.rs b/ethcore/src/account.rs index 845fff8e3..fa6fedae6 100644 --- a/ethcore/src/account.rs +++ b/ethcore/src/account.rs @@ -16,7 +16,6 @@ //! Single account in the system. -use std::collections::hash_map::Entry; use util::*; use pod_account::*; use account_db::*; @@ -24,9 +23,11 @@ use lru_cache::LruCache; use std::cell::{RefCell, Cell}; -const STORAGE_CACHE_ITEMS: usize = 4096; +const STORAGE_CACHE_ITEMS: usize = 8192; /// Single account in the system. +/// Keeps track of changes to the code and storage. +/// The changes are applied in `commit_storage` and `commit_code` pub struct Account { // Balance of the account. balance: U256, @@ -46,8 +47,6 @@ pub struct Account { code_size: Option, // Code cache of the account. code_cache: Arc, - // Account is new or has been modified. - filth: Filth, // Account code new or has been modified. code_filth: Filth, // Cached address hash. @@ -67,7 +66,6 @@ impl Account { code_hash: code.sha3(), code_size: Some(code.len() as u64), code_cache: Arc::new(code), - filth: Filth::Dirty, code_filth: Filth::Dirty, address_hash: Cell::new(None), } @@ -89,7 +87,6 @@ impl Account { code_filth: Filth::Dirty, code_size: Some(pod.code.as_ref().map_or(0, |c| c.len() as u64)), code_cache: Arc::new(pod.code.map_or_else(|| { warn!("POD account with unknown code is being created! Assuming no code."); vec![] }, |c| c)), - filth: Filth::Dirty, address_hash: Cell::new(None), } } @@ -105,7 +102,6 @@ impl Account { code_hash: SHA3_EMPTY, code_cache: Arc::new(vec![]), code_size: Some(0), - filth: Filth::Dirty, code_filth: Filth::Clean, address_hash: Cell::new(None), } @@ -123,7 +119,6 @@ impl Account { code_hash: r.val_at(3), code_cache: Arc::new(vec![]), code_size: None, - filth: Filth::Clean, code_filth: Filth::Clean, address_hash: Cell::new(None), } @@ -141,7 +136,6 @@ impl Account { code_hash: SHA3_EMPTY, code_cache: Arc::new(vec![]), code_size: None, - filth: Filth::Dirty, code_filth: Filth::Clean, address_hash: Cell::new(None), } @@ -153,7 +147,6 @@ impl Account { self.code_hash = code.sha3(); self.code_cache = Arc::new(code); self.code_size = Some(self.code_cache.len() as u64); - self.filth = Filth::Dirty; self.code_filth = Filth::Dirty; } @@ -164,17 +157,7 @@ impl Account { /// Set (and cache) the contents of the trie's storage at `key` to `value`. pub fn set_storage(&mut self, key: H256, value: H256) { - match self.storage_changes.entry(key) { - Entry::Occupied(ref mut entry) if entry.get() != &value => { - entry.insert(value); - self.filth = Filth::Dirty; - }, - Entry::Vacant(entry) => { - entry.insert(value); - self.filth = Filth::Dirty; - }, - _ => {}, - } + self.storage_changes.insert(key, value); } /// Get (and cache) the contents of the trie's storage at `key`. @@ -263,17 +246,6 @@ impl Account { !self.code_cache.is_empty() || (self.code_cache.is_empty() && self.code_hash == SHA3_EMPTY) } - /// Is this a new or modified account? - pub fn is_dirty(&self) -> bool { - self.filth == Filth::Dirty || self.code_filth == Filth::Dirty || !self.storage_is_clean() - } - - /// Mark account as clean. - pub fn set_clean(&mut self) { - assert!(self.storage_is_clean()); - self.filth = Filth::Clean - } - /// Provide a database to get `code_hash`. Should not be called if it is a contract without code. pub fn cache_code(&mut self, db: &AccountDB) -> bool { // TODO: fill out self.code_cache; @@ -326,25 +298,18 @@ impl Account { /// Increment the nonce of the account by one. pub fn inc_nonce(&mut self) { self.nonce = self.nonce + U256::from(1u8); - self.filth = Filth::Dirty; } - /// Increment the nonce of the account by one. + /// Increase account balance. pub fn add_balance(&mut self, x: &U256) { - if !x.is_zero() { - self.balance = self.balance + *x; - self.filth = Filth::Dirty; - } + self.balance = self.balance + *x; } - /// Increment the nonce of the account by one. + /// Decrease account balance. /// Panics if balance is less than `x` pub fn sub_balance(&mut self, x: &U256) { - if !x.is_zero() { - assert!(self.balance >= *x); - self.balance = self.balance - *x; - self.filth = Filth::Dirty; - } + assert!(self.balance >= *x); + self.balance = self.balance - *x; } /// Commit the `storage_changes` to the backing DB and update `storage_root`. @@ -406,7 +371,6 @@ impl Account { code_hash: self.code_hash.clone(), code_size: self.code_size.clone(), code_cache: self.code_cache.clone(), - filth: self.filth, code_filth: self.code_filth, address_hash: self.address_hash.clone(), } @@ -427,10 +391,10 @@ impl Account { account } - /// Replace self with the data from other account merging storage cache - pub fn merge_with(&mut self, other: Account) { - assert!(self.storage_is_clean()); - assert!(other.storage_is_clean()); + /// Replace self with the data from other account merging storage cache. + /// Basic account data and all modifications are overwritten + /// with new values. + pub fn overwrite_with(&mut self, other: Account) { self.balance = other.balance; self.nonce = other.nonce; self.storage_root = other.storage_root; @@ -443,6 +407,7 @@ impl Account { for (k, v) in other.storage_cache.into_inner().into_iter() { cache.insert(k.clone() , v.clone()); //TODO: cloning should not be required here } + self.storage_changes = other.storage_changes; } } diff --git a/ethcore/src/executive.rs b/ethcore/src/executive.rs index 37f038329..de75908b3 100644 --- a/ethcore/src/executive.rs +++ b/ethcore/src/executive.rs @@ -26,10 +26,10 @@ use trace::{FlatTrace, Tracer, NoopTracer, ExecutiveTracer, VMTrace, VMTracer, E use crossbeam; pub use types::executed::{Executed, ExecutionResult}; -/// Max depth to avoid stack overflow (when it's reached we start a new thread with VM) +/// Roughly estimate what stack size each level of evm depth will use /// TODO [todr] We probably need some more sophisticated calculations here (limit on my machine 132) /// Maybe something like here: `https://github.com/ethereum/libethereum/blob/4db169b8504f2b87f7d5a481819cfb959fc65f6c/libethereum/ExtVM.cpp` -const MAX_VM_DEPTH_FOR_THREAD: usize = 64; +const STACK_SIZE_PER_DEPTH: usize = 24*1024; /// Returns new address created from address and given nonce. pub fn contract_address(address: &Address, nonce: &U256) -> Address { @@ -148,12 +148,13 @@ impl<'a> Executive<'a> { // TODO: we might need bigints here, or at least check overflows. let balance = self.state.balance(&sender); - let gas_cost = U512::from(t.gas) * U512::from(t.gas_price); + let gas_cost = t.gas.full_mul(t.gas_price); let total_cost = U512::from(t.value) + gas_cost; // avoid unaffordable transactions - if U512::from(balance) < total_cost { - return Err(From::from(ExecutionError::NotEnoughCash { required: total_cost, got: U512::from(balance) })); + let balance512 = U512::from(balance); + if balance512 < total_cost { + return Err(From::from(ExecutionError::NotEnoughCash { required: total_cost, got: balance512 })); } // NOTE: there can be no invalid transactions from this point. @@ -212,8 +213,11 @@ impl<'a> Executive<'a> { tracer: &mut T, vm_tracer: &mut V ) -> evm::Result where T: Tracer, V: VMTracer { + + let depth_threshold = ::io::LOCAL_STACK_SIZE.with(|sz| sz.get() / STACK_SIZE_PER_DEPTH); + // Ordinary execution - keep VM in same thread - if (self.depth + 1) % MAX_VM_DEPTH_FOR_THREAD != 0 { + if (self.depth + 1) % depth_threshold != 0 { let vm_factory = self.vm_factory; let mut ext = self.as_externalities(OriginInfo::from(¶ms), unconfirmed_substate, output_policy, tracer, vm_tracer); trace!(target: "executive", "ext.schedule.have_delegate_call: {}", ext.schedule().have_delegate_call); @@ -265,7 +269,7 @@ impl<'a> Executive<'a> { let cost = self.engine.cost_of_builtin(¶ms.code_address, data); if cost <= params.gas { self.engine.execute_builtin(¶ms.code_address, data, &mut output); - self.state.clear_snapshot(); + self.state.discard_snapshot(); // trace only top level calls to builtins to avoid DDoS attacks if self.depth == 0 { @@ -285,7 +289,7 @@ impl<'a> Executive<'a> { Ok(params.gas - cost) } else { // just drain the whole gas - self.state.revert_snapshot(); + self.state.revert_to_snapshot(); tracer.trace_failed_call(trace_info, vec![]); @@ -331,7 +335,7 @@ impl<'a> Executive<'a> { res } else { // otherwise it's just a basic transaction, only do tracing, if necessary. - self.state.clear_snapshot(); + self.state.discard_snapshot(); tracer.trace_call(trace_info, U256::zero(), trace_output, vec![]); Ok(params.gas) @@ -413,7 +417,7 @@ impl<'a> Executive<'a> { // real ammount to refund let gas_left_prerefund = match result { Ok(x) => x, _ => 0.into() }; - let refunded = cmp::min(refunds_bound, (t.gas - gas_left_prerefund) / U256::from(2)); + let refunded = cmp::min(refunds_bound, (t.gas - gas_left_prerefund) >> 1); let gas_left = gas_left_prerefund + refunded; let gas_used = t.gas - gas_left; @@ -473,10 +477,10 @@ impl<'a> Executive<'a> { | Err(evm::Error::BadInstruction {.. }) | Err(evm::Error::StackUnderflow {..}) | Err(evm::Error::OutOfStack {..}) => { - self.state.revert_snapshot(); + self.state.revert_to_snapshot(); }, Ok(_) | Err(evm::Error::Internal) => { - self.state.clear_snapshot(); + self.state.discard_snapshot(); substate.accrue(un_substate); } } diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index 08235213b..0fe5931d3 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -15,7 +15,7 @@ // along with Parity. If not, see . use std::cell::{RefCell, RefMut}; - +use std::collections::hash_map::Entry; use common::*; use engines::Engine; use executive::{Executive, TransactOptions}; @@ -38,42 +38,83 @@ pub struct ApplyOutcome { /// Result type for the execution ("application") of a transaction. pub type ApplyResult = Result; -#[derive(Debug)] -enum AccountEntry { - /// Contains account data. - Cached(Account), - /// Account has been deleted. - Killed, - /// Account does not exist. - Missing, +#[derive(Eq, PartialEq, Clone, Copy, Debug)] +/// Account modification state. Used to check if the account was +/// Modified in between commits and overall. +enum AccountState { + /// Account was never modified in this state object. + Clean, + /// Account has been modified and is not committed to the trie yet. + /// This is set than any of the account data is changed, including + /// storage and code. + Dirty, + /// Account was modified and committed to the trie. + Commited, } +#[derive(Debug)] +/// In-memory copy of the account data. Holds the optional account +/// and the modification status. +/// Account entry can contain existing (`Some`) or non-existing +/// account (`None`) +struct AccountEntry { + account: Option, + state: AccountState, +} + +// Account cache item. Contains account data and +// modification state impl AccountEntry { fn is_dirty(&self) -> bool { - match *self { - AccountEntry::Cached(ref a) => a.is_dirty(), - AccountEntry::Killed => true, - AccountEntry::Missing => false, - } + self.state == AccountState::Dirty } - /// Clone dirty data into new `AccountEntry`. + /// Clone dirty data into new `AccountEntry`. This includes + /// basic account data and modified storage keys. /// Returns None if clean. - fn clone_dirty(&self) -> Option { - match *self { - AccountEntry::Cached(ref acc) if acc.is_dirty() => Some(AccountEntry::Cached(acc.clone_dirty())), - AccountEntry::Killed => Some(AccountEntry::Killed), - _ => None, + fn clone_if_dirty(&self) -> Option { + match self.is_dirty() { + true => Some(self.clone_dirty()), + false => None, } } - /// Clone account entry data that needs to be saved in the snapshot. - /// This includes basic account information and all locally cached storage keys - fn clone_for_snapshot(&self) -> AccountEntry { - match *self { - AccountEntry::Cached(ref acc) => AccountEntry::Cached(acc.clone_all()), - AccountEntry::Killed => AccountEntry::Killed, - AccountEntry::Missing => AccountEntry::Missing, + /// Clone dirty data into new `AccountEntry`. This includes + /// basic account data and modified storage keys. + fn clone_dirty(&self) -> AccountEntry { + AccountEntry { + account: self.account.as_ref().map(Account::clone_dirty), + state: self.state, + } + } + + // Create a new account entry and mark it as dirty. + fn new_dirty(account: Option) -> AccountEntry { + AccountEntry { + account: account, + state: AccountState::Dirty, + } + } + + // Create a new account entry and mark it as clean. + fn new_clean(account: Option) -> AccountEntry { + AccountEntry { + account: account, + state: AccountState::Clean, + } + } + + // Replace data with another entry but preserve storage cache. + fn overwrite_with(&mut self, other: AccountEntry) { + self.state = other.state; + match other.account { + Some(acc) => match self.account { + Some(ref mut ours) => { + ours.overwrite_with(acc); + }, + None => {}, + }, + None => self.account = None, } } } @@ -86,6 +127,9 @@ impl AccountEntry { /// locally from previous commits. Global cache reflects the database /// state and never contains any changes. /// +/// Cache items contains account data, or the flag that account does not exist +/// and modification state (see `AccountState`) +/// /// Account data can be in the following cache states: /// * In global but not local - something that was queried from the database, /// but never modified @@ -99,12 +143,32 @@ impl AccountEntry { /// then global state cache. If data is not found in any of the caches /// it is loaded from the DB to the local cache. /// +/// **** IMPORTANT ************************************************************* +/// All the modifications to the account data must set the `Dirty` state in the +/// `AccountEntry`. This is done in `require` and `require_or_from`. So just +/// use that. +/// **************************************************************************** +/// /// Upon destruction all the local cache data merged into the global cache. /// The merge might be rejected if current state is non-canonical. +/// +/// State snapshotting. +/// +/// A new snapshot can be created with `snapshot()`. Snapshots can be +/// created in a hierarchy. +/// When a snapshot is active all changes are applied directly into +/// `cache` and the original value is copied into an active snapshot. +/// Reverting a snapshot with `revert_to_snapshot` involves copying +/// original values from the latest snapshot back into `cache`. The code +/// takes care not to overwrite cached storage while doing that. +/// Snapshot can be discateded with `discard_snapshot`. All of the orignal +/// backed-up values are moved into a parent snapshot (if any). +/// pub struct State { db: StateDB, root: H256, cache: RefCell>, + // The original account is preserved in snapshots: RefCell>>>, account_start_nonce: U256, trie_factory: TrieFactory, @@ -158,35 +222,48 @@ impl State { Ok(state) } - /// Create a recoverable snaphot of this state + /// Create a recoverable snaphot of this state. pub fn snapshot(&mut self) { self.snapshots.borrow_mut().push(HashMap::new()); } - /// Merge last snapshot with previous - pub fn clear_snapshot(&mut self) { + /// Merge last snapshot with previous. + pub fn discard_snapshot(&mut self) { // merge with previous snapshot let last = self.snapshots.borrow_mut().pop(); if let Some(mut snapshot) = last { if let Some(ref mut prev) = self.snapshots.borrow_mut().last_mut() { - for (k, v) in snapshot.drain() { - prev.entry(k).or_insert(v); + if prev.is_empty() { + **prev = snapshot; + } else { + for (k, v) in snapshot.drain() { + prev.entry(k).or_insert(v); + } } } } } - /// Revert to snapshot - pub fn revert_snapshot(&mut self) { + /// Revert to the last snapshot and discard it. + pub fn revert_to_snapshot(&mut self) { if let Some(mut snapshot) = self.snapshots.borrow_mut().pop() { for (k, v) in snapshot.drain() { match v { Some(v) => { - self.cache.borrow_mut().insert(k, v); + match self.cache.borrow_mut().entry(k) { + Entry::Occupied(mut e) => { + // Merge snapshotted changes back into the main account + // storage preserving the cache. + e.get_mut().overwrite_with(v); + }, + Entry::Vacant(e) => { + e.insert(v); + } + } }, None => { match self.cache.borrow_mut().entry(k) { - ::std::collections::hash_map::Entry::Occupied(e) => { + Entry::Occupied(e) => { if e.get().is_dirty() { e.remove(); } @@ -200,10 +277,17 @@ impl State { } fn insert_cache(&self, address: &Address, account: AccountEntry) { - if let Some(ref mut snapshot) = self.snapshots.borrow_mut().last_mut() { - if !snapshot.contains_key(address) { - snapshot.insert(address.clone(), self.cache.borrow_mut().insert(address.clone(), account)); - return; + // Dirty account which is not in the cache means this is a new account. + // It goes directly into the snapshot as there's nothing to rever to. + // + // In all other cases account is read as clean first, and after that made + // dirty in and added to the snapshot with `note_cache`. + if account.is_dirty() { + if let Some(ref mut snapshot) = self.snapshots.borrow_mut().last_mut() { + if !snapshot.contains_key(address) { + snapshot.insert(address.clone(), self.cache.borrow_mut().insert(address.clone(), account)); + return; + } } } self.cache.borrow_mut().insert(address.clone(), account); @@ -212,7 +296,7 @@ impl State { fn note_cache(&self, address: &Address) { if let Some(ref mut snapshot) = self.snapshots.borrow_mut().last_mut() { if !snapshot.contains_key(address) { - snapshot.insert(address.clone(), self.cache.borrow().get(address).map(AccountEntry::clone_for_snapshot)); + snapshot.insert(address.clone(), self.cache.borrow().get(address).map(AccountEntry::clone_dirty)); } } } @@ -231,12 +315,12 @@ impl State { /// Create a new contract at address `contract`. If there is already an account at the address /// it will have its code reset, ready for `init_code()`. pub fn new_contract(&mut self, contract: &Address, balance: U256) { - self.insert_cache(contract, AccountEntry::Cached(Account::new_contract(balance, self.account_start_nonce))); + self.insert_cache(contract, AccountEntry::new_dirty(Some(Account::new_contract(balance, self.account_start_nonce)))); } /// Remove an existing account. pub fn kill_account(&mut self, account: &Address) { - self.insert_cache(account, AccountEntry::Killed); + self.insert_cache(account, AccountEntry::new_dirty(None)); } /// Determine whether an account exists. @@ -271,8 +355,8 @@ impl State { let local_cache = self.cache.borrow_mut(); let mut local_account = None; if let Some(maybe_acc) = local_cache.get(address) { - match *maybe_acc { - AccountEntry::Cached(ref account) => { + match maybe_acc.account { + Some(ref account) => { if let Some(value) = account.cached_storage_at(key) { return value; } else { @@ -288,7 +372,7 @@ impl State { return result; } if let Some(ref mut acc) = local_account { - if let AccountEntry::Cached(ref account) = **acc { + if let Some(ref account) = acc.account { return account.storage_at(&AccountDB::from_hash(self.db.as_hashdb(), account.address_hash(address)), key) } else { return H256::new() @@ -303,10 +387,7 @@ impl State { Err(e) => panic!("Potential DB corruption encountered: {}", e), }; let r = maybe_acc.as_ref().map_or(H256::new(), |a| a.storage_at(&AccountDB::from_hash(self.db.as_hashdb(), a.address_hash(address)), key)); - match maybe_acc { - Some(account) => self.insert_cache(address, AccountEntry::Cached(account)), - None => self.insert_cache(address, AccountEntry::Missing), - } + self.insert_cache(address, AccountEntry::new_clean(maybe_acc)); r } @@ -330,13 +411,17 @@ impl State { /// Add `incr` to the balance of account `a`. pub fn add_balance(&mut self, a: &Address, incr: &U256) { trace!(target: "state", "add_balance({}, {}): {}", a, incr, self.balance(a)); - self.require(a, false).add_balance(incr); + if !incr.is_zero() || !self.exists(a) { + self.require(a, false).add_balance(incr); + } } /// Subtract `decr` from the balance of account `a`. pub fn sub_balance(&mut self, a: &Address, decr: &U256) { trace!(target: "state", "sub_balance({}, {}): {}", a, decr, self.balance(a)); - self.require(a, false).sub_balance(decr); + if !decr.is_zero() || !self.exists(a) { + self.require(a, false).sub_balance(decr); + } } /// Subtracts `by` from the balance of `from` and adds it to that of `to`. @@ -352,7 +437,9 @@ impl State { /// Mutate storage of account `a` so that it is `value` for `key`. pub fn set_storage(&mut self, a: &Address, key: H256, value: H256) { - self.require(a, false).set_storage(key, value) + if self.storage_at(a, &key) != value { + self.require(a, false).set_storage(key, value) + } } /// Initialise the code of account `a` so that it is `code`. @@ -392,10 +479,9 @@ impl State { accounts: &mut HashMap ) -> Result<(), Error> { // first, commit the sub trees. - // TODO: is this necessary or can we dispense with the `ref mut a` for just `a`? - for (address, ref mut a) in accounts.iter_mut() { - match a { - &mut&mut AccountEntry::Cached(ref mut account) if account.is_dirty() => { + 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); let mut account_db = AccountDBMut::from_hash(db.as_hashdb_mut(), account.address_hash(address)); account.commit_storage(trie_factory, &mut account_db); @@ -407,17 +493,15 @@ impl State { { let mut trie = trie_factory.from_existing(db.as_hashdb_mut(), root).unwrap(); - for (address, ref mut a) in accounts.iter_mut() { - match **a { - AccountEntry::Cached(ref mut account) if account.is_dirty() => { - account.set_clean(); + for (address, ref mut a) in accounts.iter_mut().filter(|&(_, ref a)| a.is_dirty()) { + a.state = AccountState::Commited; + match a.account { + Some(ref mut account) => { try!(trie.insert(address, &account.rlp())); }, - AccountEntry::Killed => { + None => { try!(trie.remove(address)); - **a = AccountEntry::Missing; }, - _ => {}, } } } @@ -427,18 +511,9 @@ impl State { fn commit_cache(&mut self) { let mut addresses = self.cache.borrow_mut(); - for (address, a) in addresses.drain() { - match a { - AccountEntry::Cached(account) => { - if !account.is_dirty() { - self.db.cache_account(address, Some(account)); - } - }, - AccountEntry::Missing => { - self.db.cache_account(address, None); - }, - _ => {}, - } + trace!("Committing cache {:?} entries", addresses.len()); + for (address, a) in addresses.drain().filter(|&(_, ref a)| !a.is_dirty()) { + self.db.cache_account(address, a.account); } } @@ -460,7 +535,7 @@ impl State { 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::Cached(Account::from_pod(acc))); + self.cache.borrow_mut().insert(add, AccountEntry::new_dirty(Some(Account::from_pod(acc)))); } } @@ -470,7 +545,7 @@ impl State { // TODO: handle database rather than just the cache. // will need fat db. PodState::from(self.cache.borrow().iter().fold(BTreeMap::new(), |mut m, (add, opt)| { - if let AccountEntry::Cached(ref acc) = *opt { + if let Some(ref acc) = opt.account { m.insert(add.clone(), PodAccount::from_account(acc)); } m @@ -519,7 +594,7 @@ impl State { where F: Fn(Option<&Account>) -> U { // check local cache first if let Some(ref mut maybe_acc) = self.cache.borrow_mut().get_mut(a) { - if let AccountEntry::Cached(ref mut account) = **maybe_acc { + if let Some(ref mut account) = maybe_acc.account { Self::update_account_cache(require, account, a, self.db.as_hashdb()); return f(Some(account)); } @@ -547,8 +622,8 @@ impl State { } let r = f(maybe_acc.as_ref()); match maybe_acc { - Some(account) => self.insert_cache(a, AccountEntry::Cached(account)), - None => self.insert_cache(a, AccountEntry::Missing), + Some(account) => self.insert_cache(a, AccountEntry::new_clean(Some(account))), + None => self.insert_cache(a, AccountEntry::new_clean(None)), } r } @@ -568,36 +643,39 @@ impl State { let contains_key = self.cache.borrow().contains_key(a); if !contains_key { match self.db.get_cached_account(a) { - Some(Some(acc)) => self.insert_cache(a, AccountEntry::Cached(acc)), - Some(None) => self.insert_cache(a, AccountEntry::Missing), + Some(Some(acc)) => self.insert_cache(a, AccountEntry::new_clean(Some(acc))), + Some(None) => self.insert_cache(a, AccountEntry::new_clean(None)), None => { let maybe_acc = if self.db.check_account_bloom(a) { let db = self.trie_factory.readonly(self.db.as_hashdb(), &self.root).expect(SEC_TRIE_DB_UNWRAP_STR); let maybe_acc = match db.get(a) { - Ok(Some(acc)) => AccountEntry::Cached(Account::from_rlp(acc)), - Ok(None) => AccountEntry::Missing, + Ok(Some(acc)) => AccountEntry::new_clean(Some(Account::from_rlp(acc))), + Ok(None) => AccountEntry::new_clean(None), Err(e) => panic!("Potential DB corruption encountered: {}", e), }; maybe_acc } else { - AccountEntry::Missing + AccountEntry::new_clean(None) }; self.insert_cache(a, maybe_acc); } } - } else { - self.note_cache(a); - } - - match self.cache.borrow_mut().get_mut(a).unwrap() { - &mut AccountEntry::Cached(ref mut acc) => not_default(acc), - slot => *slot = AccountEntry::Cached(default()), + } + self.note_cache(a); + + match &mut self.cache.borrow_mut().get_mut(a).unwrap().account { + &mut Some(ref mut acc) => not_default(acc), + slot => *slot = Some(default()), } + // at this point the account is guaranteed to be in the cache. RefMut::map(self.cache.borrow_mut(), |c| { - match c.get_mut(a).unwrap() { - &mut AccountEntry::Cached(ref mut account) => { + let mut entry = c.get_mut(a).unwrap(); + // set the dirty flag after changing account data. + entry.state = AccountState::Dirty; + match entry.account { + Some(ref mut account) => { if require_code { let addr_hash = account.address_hash(a); account.cache_code(&AccountDB::from_hash(self.db.as_hashdb(), addr_hash)); @@ -621,7 +699,7 @@ impl Clone for State { let cache = { let mut cache: HashMap = HashMap::new(); for (key, val) in self.cache.borrow().iter() { - if let Some(entry) = val.clone_dirty() { + if let Some(entry) = val.clone_if_dirty() { cache.insert(key.clone(), entry); } } @@ -1677,12 +1755,12 @@ fn snapshot_basic() { state.snapshot(); state.add_balance(&a, &U256::from(69u64)); assert_eq!(state.balance(&a), U256::from(69u64)); - state.clear_snapshot(); + state.discard_snapshot(); assert_eq!(state.balance(&a), U256::from(69u64)); state.snapshot(); state.add_balance(&a, &U256::from(1u64)); assert_eq!(state.balance(&a), U256::from(70u64)); - state.revert_snapshot(); + state.revert_to_snapshot(); assert_eq!(state.balance(&a), U256::from(69u64)); } @@ -1695,9 +1773,9 @@ fn snapshot_nested() { state.snapshot(); state.add_balance(&a, &U256::from(69u64)); assert_eq!(state.balance(&a), U256::from(69u64)); - state.clear_snapshot(); + state.discard_snapshot(); assert_eq!(state.balance(&a), U256::from(69u64)); - state.revert_snapshot(); + state.revert_to_snapshot(); assert_eq!(state.balance(&a), U256::from(0)); } diff --git a/ethcore/src/state_db.rs b/ethcore/src/state_db.rs index 0714202fe..bae461af5 100644 --- a/ethcore/src/state_db.rs +++ b/ethcore/src/state_db.rs @@ -190,7 +190,7 @@ impl StateDB { for (address, account) in self.cache_overlay.drain(..) { if let Some(&mut Some(ref mut existing)) = cache.accounts.get_mut(&address) { if let Some(new) = account { - existing.merge_with(new); + existing.overwrite_with(new); continue; } } diff --git a/util/io/src/lib.rs b/util/io/src/lib.rs index b2a16e19b..082192dfa 100644 --- a/util/io/src/lib.rs +++ b/util/io/src/lib.rs @@ -68,6 +68,8 @@ mod panics; use mio::{EventLoop, Token}; use std::fmt; +pub use worker::LOCAL_STACK_SIZE; + #[derive(Debug)] /// IO Error pub enum IoError { diff --git a/util/io/src/worker.rs b/util/io/src/worker.rs index 0176c467c..f4f63919f 100644 --- a/util/io/src/worker.rs +++ b/util/io/src/worker.rs @@ -22,9 +22,19 @@ use crossbeam::sync::chase_lev; use service::{HandlerId, IoChannel, IoContext}; use IoHandler; use panics::*; +use std::cell::Cell; use std::sync::{Condvar as SCondvar, Mutex as SMutex}; +const STACK_SIZE: usize = 16*1024*1024; + +thread_local! { + /// Stack size + /// Should be modified if it is changed in Rust since it is no way + /// to know or get it + pub static LOCAL_STACK_SIZE: Cell = Cell::new(::std::env::var("RUST_MIN_STACK").ok().and_then(|s| s.parse().ok()).unwrap_or(2 * 1024 * 1024)); +} + pub enum WorkType { Readable, Writable, @@ -66,8 +76,9 @@ impl Worker { deleting: deleting.clone(), wait_mutex: wait_mutex.clone(), }; - worker.thread = Some(thread::Builder::new().name(format!("IO Worker #{}", index)).spawn( + worker.thread = Some(thread::Builder::new().stack_size(STACK_SIZE).name(format!("IO Worker #{}", index)).spawn( move || { + LOCAL_STACK_SIZE.with(|val| val.set(STACK_SIZE)); panic_handler.catch_panic(move || { Worker::work_loop(stealer, channel.clone(), wait, wait_mutex.clone(), deleting) }).unwrap() diff --git a/util/network/src/host.rs b/util/network/src/host.rs index dfadd12d8..e15d1d568 100644 --- a/util/network/src/host.rs +++ b/util/network/src/host.rs @@ -580,7 +580,8 @@ impl Host { } fn handshake_count(&self) -> usize { - self.sessions.read().count() - self.session_count() + // session_count < total_count is possible because of the data race. + self.sessions.read().count().saturating_sub(self.session_count()) } fn keep_alive(&self, io: &IoContext) {