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.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 <admin@ethcore.io>"]
 build = "build.rs"

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());
 		}
 	}
 

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);

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);
+}
+
 }

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 {

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 .\
 

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

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);

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 <admin@ethcore.io>"]
 build = "build.rs"