Merge branch 'master' into refactor/hashdb-generic

* master:
  Minimal effective gas price in the queue (#8934)
  parity: fix db path when migrating to blooms db (#8975)
  Preserve the current abort behavior (#8995)
  Improve should_replace on NonceAndGasPrice (#8980)
  Tentative fix for missing dependency error (#8973)

ethcore/Cargo.toml

@@ -80,6 +80,7 @@ fake-hardware-wallet = { path = "../util/fake-hardware-wallet" }
 [dev-dependencies]
 tempdir = "0.3"
 trie-standardmap = { path = "../util/trie-standardmap" }
+tempdir = "0.3"
 
 [features]
 # Display EVM debug traces.

miner/src/pool/mod.rs

@@ -83,20 +83,20 @@ impl PendingSettings {
 }
 
 /// Transaction priority.
-#[derive(Debug, PartialEq, Eq, Clone, Copy)]
+#[derive(Debug, PartialEq, Eq, PartialOrd,  Clone, Copy)]
 pub(crate) enum Priority {
-	/// Local transactions (high priority)
-	///
-	/// Transactions either from a local account or
-	/// submitted over local RPC connection via `eth_sendRawTransaction`
-	Local,
+	/// Regular transactions received over the network. (no priority boost)
+	Regular,
 	/// Transactions from retracted blocks (medium priority)
 	///
 	/// When block becomes non-canonical we re-import the transactions it contains
 	/// to the queue and boost their priority.
 	Retracted,
-	/// Regular transactions received over the network. (no priority boost)
-	Regular,
+	/// Local transactions (high priority)
+	///
+	/// Transactions either from a local account or
+	/// submitted over local RPC connection via `eth_sendRawTransaction`
+	Local,
 }
 
 impl Priority {

miner/src/pool/queue.rs

@@ -187,7 +187,20 @@ impl TransactionQueue {
 		trace_time!("pool::verify_and_import");
 		let options = self.options.read().clone();
 
-		let verifier = verifier::Verifier::new(client, options, self.insertion_id.clone());
+		let transaction_to_replace = {
+			let pool = self.pool.read();
+			if pool.is_full() {
+				pool.worst_transaction().map(|worst| (pool.scoring().clone(), worst))
+			} else {
+				None
+			}
+		};
+		let verifier = verifier::Verifier::new(
+			client,
+			options,
+			self.insertion_id.clone(),
+			transaction_to_replace,
+		);
 		let results = transactions
 			.into_iter()
 			.map(|transaction| {

miner/src/pool/scoring.rs

@@ -31,19 +31,42 @@ use std::cmp;
 
 use ethereum_types::U256;
 use txpool;
-use super::{PrioritizationStrategy, VerifiedTransaction};
+use super::{verifier, PrioritizationStrategy, VerifiedTransaction};
 
 /// Transaction with the same (sender, nonce) can be replaced only if
 /// `new_gas_price > old_gas_price + old_gas_price >> SHIFT`
 const GAS_PRICE_BUMP_SHIFT: usize = 3; // 2 = 25%, 3 = 12.5%, 4 = 6.25%
 
+/// Calculate minimal gas price requirement.
+#[inline]
+fn bump_gas_price(old_gp: U256) -> U256 {
+	old_gp.saturating_add(old_gp >> GAS_PRICE_BUMP_SHIFT)
+}
+
 /// Simple, gas-price based scoring for transactions.
 ///
 /// NOTE: Currently penalization does not apply to new transactions that enter the pool.
 /// We might want to store penalization status in some persistent state.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct NonceAndGasPrice(pub PrioritizationStrategy);
 
+impl NonceAndGasPrice {
+	/// Decide if the transaction should even be considered into the pool (if the pool is full).
+	///
+	/// Used by Verifier to quickly reject transactions that don't have any chance to get into the pool later on,
+	/// and save time on more expensive checks like sender recovery, etc.
+	///
+	/// NOTE The method is never called for zero-gas-price transactions or local transactions
+	/// (such transactions are always considered to the pool and potentially rejected later on)
+	pub fn should_reject_early(&self, old: &VerifiedTransaction, new: &verifier::Transaction) -> bool {
+		if old.priority().is_local() {
+			return true
+		}
+
+		&old.transaction.gas_price > new.gas_price()
+	}
+}
+
 impl txpool::Scoring<VerifiedTransaction> for NonceAndGasPrice {
 	type Score = U256;
 	type Event = ();
@@ -60,7 +83,7 @@ impl txpool::Scoring<VerifiedTransaction> for NonceAndGasPrice {
 		let old_gp = old.transaction.gas_price;
 		let new_gp = new.transaction.gas_price;
 
-		let min_required_gp = old_gp + (old_gp >> GAS_PRICE_BUMP_SHIFT);
+		let min_required_gp = bump_gas_price(old_gp);
 
 		match min_required_gp.cmp(&new_gp) {
 			cmp::Ordering::Greater => txpool::scoring::Choice::RejectNew,
@@ -102,21 +125,16 @@ impl txpool::Scoring<VerifiedTransaction> for NonceAndGasPrice {
 	fn should_replace(&self, old: &VerifiedTransaction, new: &VerifiedTransaction) -> bool {
 		if old.sender == new.sender {
 			// prefer earliest transaction
-			if new.transaction.nonce < old.transaction.nonce {
-				return true
+			match new.transaction.nonce.cmp(&old.transaction.nonce) {
+				cmp::Ordering::Less => true,
+				cmp::Ordering::Greater => false,
+				cmp::Ordering::Equal => self.choose(old, new) == txpool::scoring::Choice::ReplaceOld,
 			}
+		} else {
+			let old_score = (old.priority(), old.transaction.gas_price);
+			let new_score = (new.priority(), new.transaction.gas_price);
+			new_score > old_score
 		}
-
-		// Always kick out non-local transactions in favour of local ones.
-		if new.priority().is_local() && !old.priority().is_local() {
-			return true;
-		}
-		// And never kick out local transactions in favour of external ones.
-		if !new.priority().is_local() && old.priority.is_local() {
-			return false;
-		}
-
-		self.choose(old, new) == txpool::scoring::Choice::ReplaceOld
 	}
 }
 
@@ -125,31 +143,117 @@ mod tests {
 	use super::*;
 
 	use std::sync::Arc;
+	use ethkey::{Random, Generator};
 	use pool::tests::tx::{Tx, TxExt};
 	use txpool::Scoring;
 
 	#[test]
-	fn should_replace_non_local_transaction_with_local_one() {
+	fn should_replace_same_sender_by_nonce() {
+		let scoring = NonceAndGasPrice(PrioritizationStrategy::GasPriceOnly);
+
+		let tx1 = Tx {
+			nonce: 1,
+			gas_price: 1,
+			..Default::default()
+		};
+		let tx2 = Tx {
+			nonce: 2,
+			gas_price: 100,
+			..Default::default()
+		};
+		let tx3 = Tx {
+			nonce: 2,
+			gas_price: 110,
+			..Default::default()
+		};
+		let tx4 = Tx {
+			nonce: 2,
+			gas_price: 130,
+			..Default::default()
+		};
+
+		let keypair = Random.generate().unwrap();
+		let txs = vec![tx1, tx2, tx3, tx4].into_iter().enumerate().map(|(i, tx)| {
+			let verified = tx.unsigned().sign(keypair.secret(), None).verified();
+			txpool::Transaction {
+				insertion_id: i as u64,
+				transaction: Arc::new(verified),
+			}
+		}).collect::<Vec<_>>();
+
+		assert!(!scoring.should_replace(&txs[0], &txs[1]));
+		assert!(scoring.should_replace(&txs[1], &txs[0]));
+
+		assert!(!scoring.should_replace(&txs[1], &txs[2]));
+		assert!(!scoring.should_replace(&txs[2], &txs[1]));
+
+		assert!(scoring.should_replace(&txs[1], &txs[3]));
+		assert!(!scoring.should_replace(&txs[3], &txs[1]));
+	}
+
+	#[test]
+	fn should_replace_different_sender_by_priority_and_gas_price() {
 		// given
 		let scoring = NonceAndGasPrice(PrioritizationStrategy::GasPriceOnly);
-		let tx1 = {
-			let tx = Tx::default().signed().verified();
+		let tx_regular_low_gas = {
+			let tx = Tx {
+				nonce: 1,
+				gas_price: 1,
+				..Default::default()
+			};
+			let verified_tx = tx.signed().verified();
 			txpool::Transaction {
 				insertion_id: 0,
-				transaction: Arc::new(tx),
+				transaction: Arc::new(verified_tx),
 			}
 		};
-		let tx2 = {
-			let mut tx = Tx::default().signed().verified();
-			tx.priority = ::pool::Priority::Local;
+		let tx_regular_high_gas = {
+			let tx = Tx {
+				nonce: 2,
+				gas_price: 10,
+				..Default::default()
+			};
+			let verified_tx = tx.signed().verified();
 			txpool::Transaction {
-				insertion_id: 0,
-				transaction: Arc::new(tx),
+				insertion_id: 1,
+				transaction: Arc::new(verified_tx),
 			}
 		};
+		let tx_local_low_gas = {
+			let tx = Tx {
+				nonce: 2,
+				gas_price: 1,
+				..Default::default()
+			};
+			let mut verified_tx = tx.signed().verified();
+			verified_tx.priority = ::pool::Priority::Local;
+			txpool::Transaction {
+				insertion_id: 2,
+				transaction: Arc::new(verified_tx),
+			}
+		};
+		let tx_local_high_gas = {
+			let tx = Tx {
+				nonce: 1,
+				gas_price: 10,
+				..Default::default()
+			};
+			let mut verified_tx = tx.signed().verified();
+			verified_tx.priority = ::pool::Priority::Local;
+			txpool::Transaction {
+				insertion_id: 3,
+				transaction: Arc::new(verified_tx),
+			}
+		};
+
+		assert!(scoring.should_replace(&tx_regular_low_gas, &tx_regular_high_gas));
+		assert!(!scoring.should_replace(&tx_regular_high_gas, &tx_regular_low_gas));
+
+		assert!(scoring.should_replace(&tx_regular_high_gas, &tx_local_low_gas));
+		assert!(!scoring.should_replace(&tx_local_low_gas, &tx_regular_high_gas));
 
-		assert!(scoring.should_replace(&tx1, &tx2));
-		assert!(!scoring.should_replace(&tx2, &tx1));
+		assert!(scoring.should_replace(&tx_local_low_gas, &tx_local_high_gas));
+		assert!(!scoring.should_replace(&tx_local_high_gas, &tx_regular_low_gas));
 	}
 
 	#[test]

miner/src/pool/tests/mod.rs

@@ -875,8 +875,8 @@ fn should_avoid_verifying_transaction_already_in_pool() {
 		},
 		PrioritizationStrategy::GasPriceOnly,
 	);
-	let client = TestClient::new();
-	let tx1 = Tx::default().signed().unverified();
+	let client = TestClient::new().with_balance(1_000_000_000);
+	let tx1 = Tx::gas_price(2).signed().unverified();
 
 	let res = txq.import(client.clone(), vec![tx1.clone()]);
 	assert_eq!(res, vec![Ok(())]);
@@ -892,3 +892,41 @@ fn should_avoid_verifying_transaction_already_in_pool() {
 	// then
 	assert_eq!(txq.status().status.transaction_count, 1);
 }
+
+#[test]
+fn should_reject_early_in_case_gas_price_is_less_than_min_effective() {
+	// given
+	let txq = TransactionQueue::new(
+		txpool::Options {
+			max_count: 1,
+			max_per_sender: 2,
+			max_mem_usage: 50
+		},
+		verifier::Options {
+			minimal_gas_price: 1.into(),
+			block_gas_limit: 1_000_000.into(),
+			tx_gas_limit: 1_000_000.into(),
+		},
+		PrioritizationStrategy::GasPriceOnly,
+	);
+	let client = TestClient::new().with_balance(1_000_000_000);
+	let tx1 = Tx::gas_price(2).signed().unverified();
+
+	let res = txq.import(client.clone(), vec![tx1]);
+	assert_eq!(res, vec![Ok(())]);
+	assert_eq!(txq.status().status.transaction_count, 1);
+	assert!(client.was_verification_triggered());
+
+	// when
+	let client = TestClient::new();
+	let tx1 = Tx::default().signed().unverified();
+	let res = txq.import(client.clone(), vec![tx1]);
+	assert_eq!(res, vec![Err(transaction::Error::InsufficientGasPrice {
+		minimal: 2.into(),
+		got: 1.into(),
+	})]);
+	assert!(!client.was_verification_triggered());
+
+	// then
+	assert_eq!(txq.status().status.transaction_count, 1);
+}

miner/src/pool/tests/tx.rs

@@ -23,9 +23,9 @@ use pool::{verifier, VerifiedTransaction};
 
 #[derive(Clone)]
 pub struct Tx {
-	nonce: u64,
-	gas: u64,
-	gas_price: u64,
+	pub nonce: u64,
+	pub gas: u64,
+	pub gas_price: u64,
 }
 
 impl Default for Tx {