From 935e8dcf56a492831be85cc8c8e1b23d5064dceb Mon Sep 17 00:00:00 2001 From: Arkadiy Paronyan Date: Fri, 14 Oct 2016 17:26:35 +0200 Subject: [PATCH] Backports to beta (#2628) * v1.3.8 * mitigate refcell conflict in state diffing (#2601) * mitigate refcell conflict in state diffing Also uses RefCell::get_mut in a few places. * Add test case * Fixed stalled sync * Fixed tx queue limit for local transactions (#2616) * Fixed tx queue limit for local tx * Fixing test * Increas gas limit to 20x * Additional logs when transactions is removed from queue (#2617) * Database performance tweaks (#2619) --- Cargo.lock | 30 +++++++++--------- Cargo.toml | 2 +- ethcore/src/miner/miner.rs | 4 +-- ethcore/src/miner/transaction_queue.rs | 15 +++++++-- ethcore/src/state.rs | 44 ++++++++++++++++++-------- ethcore/src/state_db.rs | 2 +- nsis/installer.nsi | 2 +- parity/cli.rs | 2 +- sync/src/chain.rs | 7 +--- util/Cargo.toml | 2 +- util/src/kvdb.rs | 21 +++++++----- 11 files changed, 78 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0ad729658..64316e12e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ [root] name = "parity" -version = "1.3.7" +version = "1.3.8" dependencies = [ "ansi_term 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)", "clippy 0.0.80 (registry+https://github.com/rust-lang/crates.io-index)", @@ -20,7 +20,7 @@ dependencies = [ "ethcore-logger 1.3.0", "ethcore-rpc 1.3.0", "ethcore-signer 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "ethsync 1.3.0", "fdlimit 0.1.0", "hyper 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)", @@ -270,7 +270,7 @@ dependencies = [ "ethcore-ipc 1.3.0", "ethcore-ipc-codegen 1.3.0", "ethcore-ipc-nano 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "ethjson 0.1.0", "ethstore 0.1.0", "evmjit 1.3.0", @@ -294,7 +294,7 @@ version = "1.3.0" dependencies = [ "clippy 0.0.80 (registry+https://github.com/rust-lang/crates.io-index)", "ethcore-rpc 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "hyper 0.9.4 (git+https://github.com/ethcore/hyper)", "jsonrpc-core 2.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "jsonrpc-http-server 6.1.0 (git+https://github.com/ethcore/jsonrpc-http-server.git)", @@ -336,7 +336,7 @@ name = "ethcore-ipc" version = "1.3.0" dependencies = [ "ethcore-devtools 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "nanomsg 0.5.1 (git+https://github.com/ethcore/nanomsg.rs.git)", "semver 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -381,7 +381,7 @@ dependencies = [ "ethcore-ipc 1.3.0", "ethcore-ipc-codegen 1.3.0", "ethcore-ipc-nano 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "nanomsg 0.5.1 (git+https://github.com/ethcore/nanomsg.rs.git)", "semver 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -393,7 +393,7 @@ name = "ethcore-logger" version = "1.3.0" dependencies = [ "env_logger 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "isatty 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "lazy_static 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -408,7 +408,7 @@ dependencies = [ "ansi_term 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)", "ethcore-devtools 1.3.0", "ethcore-io 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "igd 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)", "libc 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -432,7 +432,7 @@ dependencies = [ "ethcore-devtools 1.3.0", "ethcore-io 1.3.0", "ethcore-ipc 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "ethjson 0.1.0", "ethsync 1.3.0", "json-ipc-server 0.2.4 (git+https://github.com/ethcore/json-ipc-server.git?branch=beta)", @@ -455,7 +455,7 @@ dependencies = [ "env_logger 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "ethcore-io 1.3.0", "ethcore-rpc 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "jsonrpc-core 2.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "parity-dapps-signer 1.4.0 (git+https://github.com/ethcore/parity-ui.git)", @@ -466,7 +466,7 @@ dependencies = [ [[package]] name = "ethcore-util" -version = "1.3.7" +version = "1.3.8" dependencies = [ "ansi_term 0.7.2 (registry+https://github.com/rust-lang/crates.io-index)", "arrayvec 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)", @@ -499,7 +499,7 @@ dependencies = [ name = "ethjson" version = "0.1.0" dependencies = [ - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "serde 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", "serde_codegen 0.7.9 (registry+https://github.com/rust-lang/crates.io-index)", @@ -547,7 +547,7 @@ dependencies = [ "ethcore-ipc-codegen 1.3.0", "ethcore-ipc-nano 1.3.0", "ethcore-network 1.3.0", - "ethcore-util 1.3.7", + "ethcore-util 1.3.8", "heapsize 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.2.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1225,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#ffc7c82380fe8569f85ae6743f7f620af2d4a679" +source = "git+https://github.com/ethcore/rust-rocksdb#64c63ccbe1f62c2e2b39262486f9ba813793af58" 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)", @@ -1234,7 +1234,7 @@ dependencies = [ [[package]] name = "rocksdb-sys" version = "0.3.0" -source = "git+https://github.com/ethcore/rust-rocksdb#ffc7c82380fe8569f85ae6743f7f620af2d4a679" +source = "git+https://github.com/ethcore/rust-rocksdb#64c63ccbe1f62c2e2b39262486f9ba813793af58" dependencies = [ "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)", diff --git a/Cargo.toml b/Cargo.toml index 63f7b2e9a..1a127bb3d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] description = "Ethcore client." name = "parity" -version = "1.3.7" +version = "1.3.8" license = "GPL-3.0" authors = ["Ethcore "] build = "build.rs" diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index ff68f4291..a6bf13637 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -421,8 +421,8 @@ impl Miner { let mut queue = self.transaction_queue.lock(); queue.set_gas_limit(gas_limit); if let GasLimit::Auto = self.options.tx_queue_gas_limit { - // Set total tx queue gas limit to be 2x the block gas limit. - queue.set_total_gas_limit(gas_limit << 1); + // Set total tx queue gas limit to be 20x the block gas limit. + queue.set_total_gas_limit(gas_limit * 20.into()); } } diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs index bfbe2c645..c48f1d02c 100644 --- a/ethcore/src/miner/transaction_queue.rs +++ b/ethcore/src/miner/transaction_queue.rs @@ -305,14 +305,14 @@ impl TransactionSet { let to_drop : Vec<(Address, U256)> = { self.by_priority .iter() - .skip_while(|order| { + .filter(|order| { count = count + 1; let r = gas.overflowing_add(order.gas); if r.1 { return false } gas = r.0; // Own and retracted transactions are allowed to go above the gas limit, bot not above the count limit. - (gas <= self.gas_limit || order.origin == TransactionOrigin::Local || order.origin == TransactionOrigin::RetractedBlock) && - count <= self.limit + (gas > self.gas_limit && order.origin != TransactionOrigin::Local && order.origin != TransactionOrigin::RetractedBlock) || + count > self.limit }) .map(|order| by_hash.get(&order.hash) .expect("All transactions in `self.by_priority` and `self.by_address` are kept in sync with `by_hash`.")) @@ -324,6 +324,7 @@ impl TransactionSet { .fold(HashMap::new(), |mut removed, (sender, nonce)| { let order = self.drop(&sender, &nonce) .expect("Transaction has just been found in `by_priority`; so it is in `by_address` also."); + trace!(target: "txqueue", "Dropped out of limit transaction: {:?}", order.hash); by_hash.remove(&order.hash) .expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`"); @@ -647,6 +648,8 @@ impl TransactionQueue { let nonce = transaction.nonce(); let current_nonce = fetch_account(&sender).nonce; + trace!(target: "txqueue", "Removing invalid transaction: {:?}", transaction.hash()); + // Remove from future let order = self.future.drop(&sender, &nonce); if order.is_some() { @@ -920,12 +923,14 @@ impl TransactionQueue { let old_fee = old.gas_price; let new_fee = order.gas_price; if old_fee.cmp(&new_fee) == Ordering::Greater { + trace!(target: "txqueue", "Didn't insert transaction because gas price was too low: {:?} ({:?} stays in the queue)", order.hash, old.hash); // Put back old transaction since it has greater priority (higher gas_price) set.insert(address, nonce, old); // and remove new one by_hash.remove(&order.hash).expect("The hash has been just inserted and no other line is altering `by_hash`."); false } else { + trace!(target: "txqueue", "Replaced transaction: {:?} with transaction with higher gas price: {:?}", old.hash, order.hash); // Make sure we remove old transaction entirely by_hash.remove(&old.hash).expect("The hash is coming from `future` so it has to be in `by_hash`."); true @@ -1773,8 +1778,12 @@ mod test { let mut txq = TransactionQueue::with_limits(PrioritizationStrategy::GasPriceOnly, 100, default_gas_val() * U256::from(2), !U256::zero()); let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(1), U256::from(1)); let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(1), U256::from(2)); + let (tx5, tx6) = new_txs_with_gas_price_diff(U256::from(1), U256::from(2)); txq.add(tx1.clone(), &default_nonce, TransactionOrigin::Local).unwrap(); txq.add(tx2.clone(), &default_nonce, TransactionOrigin::Local).unwrap(); + txq.add(tx5.clone(), &default_nonce, TransactionOrigin::External).unwrap(); + // Not accepted because of limit + txq.add(tx6.clone(), &default_nonce, TransactionOrigin::External).unwrap_err(); txq.add(tx3.clone(), &default_nonce, TransactionOrigin::Local).unwrap(); txq.add(tx4.clone(), &default_nonce, TransactionOrigin::Local).unwrap(); assert_eq!(txq.status().pending, 4); diff --git a/ethcore/src/state.rs b/ethcore/src/state.rs index b2d370512..f77ba023f 100644 --- a/ethcore/src/state.rs +++ b/ethcore/src/state.rs @@ -234,15 +234,15 @@ impl State { /// Create a recoverable snaphot of this state. pub fn snapshot(&mut self) { - self.snapshots.borrow_mut().push(HashMap::new()); + self.snapshots.get_mut().push(HashMap::new()); } /// Merge last snapshot with previous. pub fn discard_snapshot(&mut self) { // merge with previous snapshot - let last = self.snapshots.borrow_mut().pop(); + let last = self.snapshots.get_mut().pop(); if let Some(mut snapshot) = last { - if let Some(ref mut prev) = self.snapshots.borrow_mut().last_mut() { + if let Some(ref mut prev) = self.snapshots.get_mut().last_mut() { if prev.is_empty() { **prev = snapshot; } else { @@ -256,11 +256,11 @@ impl State { /// 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() { + if let Some(mut snapshot) = self.snapshots.get_mut().pop() { for (k, v) in snapshot.drain() { match v { Some(v) => { - match self.cache.borrow_mut().entry(k) { + match self.cache.get_mut().entry(k) { Entry::Occupied(mut e) => { // Merge snapshotted changes back into the main account // storage preserving the cache. @@ -272,7 +272,7 @@ impl State { } }, None => { - match self.cache.borrow_mut().entry(k) { + match self.cache.get_mut().entry(k) { Entry::Occupied(e) => { if e.get().is_dirty() { e.remove(); @@ -564,14 +564,14 @@ impl State { } fn query_pod(&mut self, query: &PodState) { - for (ref address, ref pod_account) in query.get() { - self.ensure_cached(address, RequireCache::Code, |a| { - if a.is_some() { - for key in pod_account.storage.keys() { - self.storage_at(address, key); - } - } - }); + for (address, pod_account) in query.get().into_iter() + .filter(|&(ref a, _)| self.ensure_cached(a, RequireCache::Code, |a| a.is_some())) + { + // needs to be split into two parts for the refcell code here + // to work. + for key in pod_account.storage.keys() { + self.storage_at(address, key); + } } } @@ -1794,4 +1794,20 @@ fn create_empty() { assert_eq!(state.root().hex(), "56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"); } +#[test] +fn should_not_panic_on_state_diff_with_storage() { + let state = get_temp_state(); + let mut state = state.reference().clone(); + + let a: Address = 0xa.into(); + state.init_code(&a, b"abcdefg".to_vec()); + state.add_balance(&a, &256.into()); + state.set_storage(&a, 0xb.into(), 0xc.into()); + + let mut new_state = state.clone(); + new_state.set_storage(&a, 0xb.into(), 0xd.into()); + + new_state.diff_from(state); +} + } diff --git a/ethcore/src/state_db.rs b/ethcore/src/state_db.rs index 6f779e968..345b424b5 100644 --- a/ethcore/src/state_db.rs +++ b/ethcore/src/state_db.rs @@ -27,7 +27,7 @@ use client::DB_COL_ACCOUNT_BLOOM; use byteorder::{LittleEndian, ByteOrder}; const STATE_CACHE_ITEMS: usize = 256000; -const STATE_CACHE_BLOCKS: usize = 8; +const STATE_CACHE_BLOCKS: usize = 12; /// Shared canonical state cache. struct AccountCache { diff --git a/nsis/installer.nsi b/nsis/installer.nsi index 80a558883..8682424ca 100644 --- a/nsis/installer.nsi +++ b/nsis/installer.nsi @@ -4,7 +4,7 @@ !define DESCRIPTION "Fast, light, robust Ethereum implementation" !define VERSIONMAJOR 1 !define VERSIONMINOR 3 -!define VERSIONBUILD 7 +!define VERSIONBUILD 8 !addplugindir .\ diff --git a/parity/cli.rs b/parity/cli.rs index e6c2b74f9..62deafeb4 100644 --- a/parity/cli.rs +++ b/parity/cli.rs @@ -202,7 +202,7 @@ Sealing/Mining Options: and gas limit ratio [default: gas_factor]. --tx-queue-gas LIMIT Maximum amount of total gas for external transactions in the queue. LIMIT can be either an amount of gas or - 'auto' or 'off'. 'auto' sets the limit to be 2x + 'auto' or 'off'. 'auto' sets the limit to be 20x the current block gas limit. [default: auto]. --remove-solved Move solved blocks from the work package queue instead of cloning them. This gives a slightly diff --git a/sync/src/chain.rs b/sync/src/chain.rs index 750d4528e..dd45e1d80 100644 --- a/sync/src/chain.rs +++ b/sync/src/chain.rs @@ -562,12 +562,7 @@ impl ChainSync { } match self.state { SyncState::ChainHead => { - if headers.is_empty() { - // peer is not on our chain - // track back and try again - self.imported_this_round = Some(0); - self.start_sync_round(io); - } else { + if !headers.is_empty() { // TODO: validate heads better. E.g. check that there is enough distance between blocks. trace!(target: "sync", "Received {} subchain heads, proceeding to download", headers.len()); self.blocks.reset_to(hashes); diff --git a/util/Cargo.toml b/util/Cargo.toml index fd84feb53..3ff359da2 100644 --- a/util/Cargo.toml +++ b/util/Cargo.toml @@ -3,7 +3,7 @@ description = "Ethcore utility library" homepage = "http://ethcore.io" license = "GPL-3.0" name = "ethcore-util" -version = "1.3.7" +version = "1.3.8" authors = ["Ethcore "] build = "build.rs" diff --git a/util/src/kvdb.rs b/util/src/kvdb.rs index 253b98d61..6eb26d6a7 100644 --- a/util/src/kvdb.rs +++ b/util/src/kvdb.rs @@ -21,7 +21,7 @@ use elastic_array::*; use std::default::Default; use rlp::{UntrustedRlp, RlpType, View, Compressible}; use rocksdb::{DB, Writable, WriteBatch, WriteOptions, IteratorMode, DBIterator, - Options, DBCompactionStyle, BlockBasedOptions, Direction, Cache, Column}; + Options, DBCompactionStyle, BlockBasedOptions, Direction, Cache, Column, ReadOptions}; const DB_BACKGROUND_FLUSHES: i32 = 2; const DB_BACKGROUND_COMPACTIONS: i32 = 2; @@ -198,6 +198,7 @@ pub struct Database { db: DB, write_opts: WriteOptions, cfs: Vec, + read_opts: ReadOptions, overlay: RwLock, KeyState>>>, } @@ -213,7 +214,8 @@ impl Database { if let Some(rate_limit) = config.compaction.write_rate_limit { try!(opts.set_parsed_options(&format!("rate_limiter_bytes_per_sec={}", rate_limit))); } - try!(opts.set_parsed_options(&format!("max_total_wal_size={}", 256 * 1024 * 1024))); + try!(opts.set_parsed_options(&format!("max_total_wal_size={}", 64 * 1024 * 1024))); + try!(opts.set_parsed_options("verify_checksums_in_compaction=0")); opts.set_max_open_files(config.max_open_files); opts.create_if_missing(true); opts.set_use_fsync(false); @@ -246,6 +248,8 @@ impl Database { if !config.wal { write_opts.disable_wal(true); } + let mut read_opts = ReadOptions::new(); + read_opts.set_verify_checksums(false); let mut cfs: Vec = Vec::new(); let db = match config.columns { @@ -287,6 +291,7 @@ impl Database { write_opts: write_opts, overlay: RwLock::new((0..(cfs.len() + 1)).map(|_| HashMap::new()).collect()), cfs: cfs, + read_opts: read_opts, }) } @@ -390,8 +395,8 @@ impl Database { Some(&KeyState::Delete) => Ok(None), None => { col.map_or_else( - || self.db.get(key).map(|r| r.map(|v| v.to_vec())), - |c| self.db.get_cf(self.cfs[c as usize], key).map(|r| r.map(|v| v.to_vec()))) + || self.db.get_opt(key, &self.read_opts).map(|r| r.map(|v| v.to_vec())), + |c| self.db.get_cf_opt(self.cfs[c as usize], key, &self.read_opts).map(|r| r.map(|v| v.to_vec()))) }, } } @@ -399,8 +404,8 @@ impl Database { /// Get value by partial key. Prefix size should match configured prefix size. Only searches flushed values. // TODO: support prefix seek for unflushed ata pub fn get_by_prefix(&self, col: Option, prefix: &[u8]) -> Option> { - let mut iter = col.map_or_else(|| self.db.iterator(IteratorMode::From(prefix, Direction::Forward)), - |c| self.db.iterator_cf(self.cfs[c as usize], IteratorMode::From(prefix, Direction::Forward)).unwrap()); + let mut iter = col.map_or_else(|| self.db.iterator_opt(IteratorMode::From(prefix, Direction::Forward), &self.read_opts), + |c| self.db.iterator_cf_opt(self.cfs[c as usize], IteratorMode::From(prefix, Direction::Forward), &self.read_opts).unwrap()); match iter.next() { // TODO: use prefix_same_as_start read option (not availabele in C API currently) Some((k, v)) => if k[0 .. prefix.len()] == prefix[..] { Some(v) } else { None }, @@ -411,8 +416,8 @@ impl Database { /// Get database iterator for flushed data. pub fn iter(&self, col: Option) -> DatabaseIterator { //TODO: iterate over overlay - col.map_or_else(|| DatabaseIterator { iter: self.db.iterator(IteratorMode::Start) }, - |c| DatabaseIterator { iter: self.db.iterator_cf(self.cfs[c as usize], IteratorMode::Start).unwrap() }) + col.map_or_else(|| DatabaseIterator { iter: self.db.iterator_opt(IteratorMode::Start, &self.read_opts) }, + |c| DatabaseIterator { iter: self.db.iterator_cf_opt(self.cfs[c as usize], IteratorMode::Start, &self.read_opts).unwrap() }) } }