New Transaction Queue implementation (#8074)

* Implementation of Verifier, Scoring and Ready.

* Queue in progress.

* TransactionPool.

* Prepare for txpool release.

* Miner refactor [WiP]

* WiP reworking miner.

* Make it compile.

* Add some docs.

* Split blockchain access to a separate file.

* Work on miner API.

* Fix ethcore tests.

* Refactor miner interface for sealing/work packages.

* Implement next nonce.

* RPC compiles.

* Implement couple of missing methdods for RPC.

* Add transaction queue listeners.

* Compiles!

* Clean-up and parallelize.

* Get rid of RefCell in header.

* Revert "Get rid of RefCell in header."

This reverts commit 0f2424c9b7319a786e1565ea2a8a6d801a21b4fb.

* Override Sync requirement.

* Fix status display.

* Unify logging.

* Extract some cheap checks.

* Measurements and optimizations.

* Fix scoring bug, heap size of bug and add cache

* Disable tx queueing and parallel verification.

* Make ethcore and ethcore-miner compile again.

* Make RPC compile again.

* Bunch of txpool tests.

* Migrate transaction queue tests.

* Nonce Cap

* Nonce cap cache and tests.

* Remove stale future transactions from the queue.

* Optimize scoring and write some tests.

* Simple penalization.

* Clean up and support for different scoring algorithms.

* Add CLI parameters for the new queue.

* Remove banning queue.

* Disable debug build.

* Change per_sender limit to be 1% instead of 5%

* Avoid cloning when propagating transactions.

* Remove old todo.

* Post-review fixes.

* Fix miner options default.

* Implement back ready transactions for light client.

* Get rid of from_pending_block

* Pass rejection reason.

* Add more details to drop.

* Rollback heap size of.

* Avoid cloning hashes when propagating and include more details on rejection.

* Fix tests.

* Introduce nonces cache.

* Remove uneccessary hashes allocation.

* Lower the mem limit.

* Re-enable parallel verification.

* Add miner log. Don't check the type if not below min_gas_price.

* Add more traces, fix disabling miner.

* Fix creating pending blocks twice on AuRa authorities.

* Fix tests.

* re-use pending blocks in AuRa

* Use reseal_min_period to prevent too frequent update_sealing.

* Fix log to contain hash not sender.

* Optimize local transactions.

* Fix aura tests.

* Update locks comments.

* Get rid of unsafe Sync impl.

* Review fixes.

* Remove excessive matches.

* Fix compilation errors.

* Use new pool in private transactions.

* Fix private-tx test.

* Fix secret store tests.

* Actually use gas_floor_target

* Fix config tests.

* Fix pool tests.

* Address grumbles.

transaction-pool/src/error.rs

@@ -21,17 +21,17 @@ error_chain! {
 		/// Transaction is already imported
 		AlreadyImported(hash: H256) {
 			description("transaction is already in the pool"),
-			display("[{:?}] transaction already imported", hash)
+			display("[{:?}] already imported", hash)
 		}
 		/// Transaction is too cheap to enter the queue
-		TooCheapToEnter(hash: H256) {
+		TooCheapToEnter(hash: H256, min_score: String) {
 			description("the pool is full and transaction is too cheap to replace any transaction"),
-			display("[{:?}] transaction too cheap to enter the pool", hash)
+			display("[{:?}] too cheap to enter the pool. Min score: {}", hash, min_score)
 		}
 		/// Transaction is too cheap to replace existing transaction that occupies the same slot.
 		TooCheapToReplace(old_hash: H256, hash: H256) {
 			description("transaction is too cheap to replace existing transaction in the pool"),
-			display("[{:?}] transaction too cheap to replace: {:?}", hash, old_hash)
+			display("[{:?}] too cheap to replace: {:?}", hash, old_hash)
 		}
 	}
 }
@@ -43,7 +43,7 @@ impl PartialEq for ErrorKind {
 
 		match (self, other) {
 			(&AlreadyImported(ref h1), &AlreadyImported(ref h2)) => h1 == h2,
-			(&TooCheapToEnter(ref h1), &TooCheapToEnter(ref h2)) => h1 == h2,
+			(&TooCheapToEnter(ref h1, ref s1), &TooCheapToEnter(ref h2, ref s2)) => h1 == h2 && s1 == s2,
 			(&TooCheapToReplace(ref old1, ref new1), &TooCheapToReplace(ref old2, ref new2)) => old1 == old2 && new1 == new2,
 			_ => false,
 		}

transaction-pool/src/lib.rs

@@ -76,6 +76,8 @@ extern crate error_chain;
 #[macro_use]
 extern crate log;
 
+extern crate trace_time;
+
 #[cfg(test)]
 mod tests;
 
@@ -90,6 +92,7 @@ mod verifier;
 
 pub mod scoring;
 
+pub use self::error::{Error, ErrorKind};
 pub use self::listener::{Listener, NoopListener};
 pub use self::options::Options;
 pub use self::pool::{Pool, PendingIterator};

transaction-pool/src/listener.rs

@@ -15,6 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use std::sync::Arc;
+use error::ErrorKind;
 
 /// Transaction pool listener.
 ///
@@ -28,16 +29,16 @@ pub trait Listener<T> {
 
 	/// The transaction was rejected from the pool.
 	/// It means that it was too cheap to replace any transaction already in the pool.
-	fn rejected(&mut self, _tx: T) {}
+	fn rejected(&mut self, _tx: &Arc<T>, _reason: &ErrorKind) {}
 
-	/// The transaction was dropped from the pool because of a limit.
-	fn dropped(&mut self, _tx: &Arc<T>) {}
+	/// The transaction was pushed out from the pool because of the limit.
+	fn dropped(&mut self, _tx: &Arc<T>, _by: Option<&T>) {}
 
 	/// The transaction was marked as invalid by executor.
 	fn invalid(&mut self, _tx: &Arc<T>) {}
 
-	/// The transaction has been cancelled.
-	fn cancelled(&mut self, _tx: &Arc<T>) {}
+	/// The transaction has been canceled.
+	fn canceled(&mut self, _tx: &Arc<T>) {}
 
 	/// The transaction has been mined.
 	fn mined(&mut self, _tx: &Arc<T>) {}
@@ -47,3 +48,38 @@ pub trait Listener<T> {
 #[derive(Debug)]
 pub struct NoopListener;
 impl<T> Listener<T> for NoopListener {}
+
+impl<T, A, B> Listener<T> for (A, B) where
+	A: Listener<T>,
+	B: Listener<T>,
+{
+	fn added(&mut self, tx: &Arc<T>, old: Option<&Arc<T>>) {
+		self.0.added(tx, old);
+		self.1.added(tx, old);
+	}
+
+	fn rejected(&mut self, tx: &Arc<T>, reason: &ErrorKind) {
+		self.0.rejected(tx, reason);
+		self.1.rejected(tx, reason);
+	}
+
+	fn dropped(&mut self, tx: &Arc<T>, by: Option<&T>) {
+		self.0.dropped(tx, by);
+		self.1.dropped(tx, by);
+	}
+
+	fn invalid(&mut self, tx: &Arc<T>) {
+		self.0.invalid(tx);
+		self.1.invalid(tx);
+	}
+
+	fn canceled(&mut self, tx: &Arc<T>) {
+		self.0.canceled(tx);
+		self.1.canceled(tx);
+	}
+
+	fn mined(&mut self, tx: &Arc<T>) {
+		self.0.mined(tx);
+		self.1.mined(tx);
+	}
+}

transaction-pool/src/options.rs

@@ -15,7 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 /// Transaction Pool options.
-#[derive(Debug)]
+#[derive(Clone, Debug, PartialEq)]
 pub struct Options {
 	/// Maximal number of transactions in the pool.
 	pub max_count: usize,

transaction-pool/src/pool.rs

@@ -109,17 +109,17 @@ impl<T, S, L> Pool<T, S, L> where
 
 		ensure!(!self.by_hash.contains_key(transaction.hash()), error::ErrorKind::AlreadyImported(*transaction.hash()));
 
-		// TODO [ToDr] Most likely move this after the transsaction is inserted.
+		// TODO [ToDr] Most likely move this after the transaction is inserted.
 		// Avoid using should_replace, but rather use scoring for that.
 		{
 			let remove_worst = |s: &mut Self, transaction| {
 				match s.remove_worst(&transaction) {
 					Err(err) => {
-						s.listener.rejected(transaction);
+						s.listener.rejected(&Arc::new(transaction), err.kind());
 						Err(err)
 					},
 					Ok(removed) => {
-						s.listener.dropped(&removed);
+						s.listener.dropped(&removed, Some(&transaction));
 						s.finalize_remove(removed.hash());
 						Ok(transaction)
 					},
@@ -127,10 +127,12 @@ impl<T, S, L> Pool<T, S, L> where
 			};
 
 			while self.by_hash.len() + 1 > self.options.max_count {
+				trace!("Count limit reached: {} > {}", self.by_hash.len() + 1, self.options.max_count);
 				transaction = remove_worst(self, transaction)?;
 			}
 
 			while self.mem_usage + mem_usage > self.options.max_mem_usage {
+				trace!("Mem limit reached: {} > {}", self.mem_usage + mem_usage, self.options.max_mem_usage);
 				transaction = remove_worst(self, transaction)?;
 			}
 		}
@@ -160,14 +162,14 @@ impl<T, S, L> Pool<T, S, L> where
 				Ok(new)
 			},
 			AddResult::TooCheap { new, old } => {
-				let hash = *new.hash();
-				self.listener.rejected(new);
-				bail!(error::ErrorKind::TooCheapToReplace(*old.hash(), hash))
+				let error = error::ErrorKind::TooCheapToReplace(*old.hash(), *new.hash());
+				self.listener.rejected(&Arc::new(new), &error);
+				bail!(error)
 			},
-			AddResult::TooCheapToEnter(new) => {
-				let hash = *new.hash();
-				self.listener.rejected(new);
-				bail!(error::ErrorKind::TooCheapToEnter(hash))
+			AddResult::TooCheapToEnter(new, score) => {
+				let error = error::ErrorKind::TooCheapToEnter(*new.hash(), format!("{:?}", score));
+				self.listener.rejected(&Arc::new(new), &error);
+				bail!(error)
 			}
 		}
 	}
@@ -241,14 +243,14 @@ impl<T, S, L> Pool<T, S, L> where
 			// No elements to remove? and the pool is still full?
 			None => {
 				warn!("The pool is full but there are no transactions to remove.");
-				return Err(error::ErrorKind::TooCheapToEnter(*transaction.hash()).into());
+				return Err(error::ErrorKind::TooCheapToEnter(*transaction.hash(), "unknown".into()).into());
 			},
 			Some(old) => if self.scoring.should_replace(&old.transaction, transaction) {
 				// New transaction is better than the worst one so we can replace it.
 				old.clone()
 			} else {
 				// otherwise fail
-				return Err(error::ErrorKind::TooCheapToEnter(*transaction.hash()).into())
+				return Err(error::ErrorKind::TooCheapToEnter(*transaction.hash(), format!("{:?}", old.score)).into())
 			},
 		};
 
@@ -256,6 +258,7 @@ impl<T, S, L> Pool<T, S, L> where
 		self.remove_from_set(to_remove.transaction.sender(), |set, scoring| {
 			set.remove(&to_remove.transaction, scoring)
 		});
+
 		Ok(to_remove.transaction)
 	}
 
@@ -283,7 +286,7 @@ impl<T, S, L> Pool<T, S, L> where
 		self.worst_transactions.clear();
 
 		for (_hash, tx) in self.by_hash.drain() {
-			self.listener.dropped(&tx)
+			self.listener.dropped(&tx, None)
 		}
 	}
 
@@ -298,7 +301,7 @@ impl<T, S, L> Pool<T, S, L> where
 			if is_invalid {
 				self.listener.invalid(&tx);
 			} else {
-				self.listener.cancelled(&tx);
+				self.listener.canceled(&tx);
 			}
 			Some(tx)
 		} else {
@@ -345,6 +348,16 @@ impl<T, S, L> Pool<T, S, L> where
 		removed
 	}
 
+	/// Returns a transaction if it's part of the pool or `None` otherwise.
+	pub fn find(&self, hash: &H256) -> Option<Arc<T>> {
+		self.by_hash.get(hash).cloned()
+	}
+
+	/// Returns worst transaction in the queue (if any).
+	pub fn worst_transaction(&self) -> Option<Arc<T>> {
+		self.worst_transactions.iter().next().map(|x| x.transaction.clone())
+	}
+
 	/// Returns an iterator of pending (ready) transactions.
 	pub fn pending<R: Ready<T>>(&self, ready: R) -> PendingIterator<T, R, S, L> {
 		PendingIterator {
@@ -354,6 +367,41 @@ impl<T, S, L> Pool<T, S, L> where
 		}
 	}
 
+	/// Returns pending (ready) transactions from given sender.
+	pub fn pending_from_sender<R: Ready<T>>(&self, ready: R, sender: &Sender) -> PendingIterator<T, R, S, L> {
+		let best_transactions = self.transactions.get(sender)
+			.and_then(|transactions| transactions.worst_and_best())
+			.map(|(_, best)| ScoreWithRef::new(best.0, best.1))
+			.map(|s| {
+				let mut set = BTreeSet::new();
+				set.insert(s);
+				set
+			})
+			.unwrap_or_default();
+
+		PendingIterator {
+			ready,
+			best_transactions,
+			pool: self
+		}
+	}
+
+	/// Update score of transactions of a particular sender.
+	pub fn update_scores(&mut self, sender: &Sender, event: S::Event) {
+		let res = if let Some(set) = self.transactions.get_mut(sender) {
+			let prev = set.worst_and_best();
+			set.update_scores(&self.scoring, event);
+			let current = set.worst_and_best();
+			Some((prev, current))
+		} else {
+			None
+		};
+
+		if let Some((prev, current)) = res {
+			self.update_senders_worst_and_best(prev, current);
+		}
+	}
+
 	/// Computes the full status of the pool (including readiness).
 	pub fn status<R: Ready<T>>(&self, mut ready: R) -> Status {
 		let mut status = Status::default();
@@ -383,6 +431,21 @@ impl<T, S, L> Pool<T, S, L> where
 			senders: self.transactions.len(),
 		}
 	}
+
+	/// Returns current pool options.
+	pub fn options(&self) -> Options {
+		self.options.clone()
+	}
+
+	/// Borrows listener instance.
+	pub fn listener(&self) -> &L {
+		&self.listener
+	}
+
+	/// Borrows listener mutably.
+	pub fn listener_mut(&mut self) -> &mut L {
+		&mut self.listener
+	}
 }
 
 /// An iterator over all pending (ready) transactions.
@@ -424,7 +487,7 @@ impl<'a, T, R, S, L> Iterator for PendingIterator<'a, T, R, S, L> where
 
 					return Some(best.transaction)
 				},
-				state => warn!("[{:?}] Ignoring {:?} transaction.", best.transaction.hash(), state),
+				state => trace!("[{:?}] Ignoring {:?} transaction.", best.transaction.hash(), state),
 			}
 		}
 

transaction-pool/src/scoring.rs

@@ -42,7 +42,7 @@ pub enum Choice {
 /// The `Scoring` implementations can use this information
 /// to update the `Score` table more efficiently.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub enum Change {
+pub enum Change<T = ()> {
 	/// New transaction has been inserted at given index.
 	/// The Score at that index is initialized with default value
 	/// and needs to be filled in.
@@ -56,8 +56,12 @@ pub enum Change {
 	/// The score at that index needs to be update (it contains value from previous transaction).
 	ReplacedAt(usize),
 	/// Given number of stalled transactions has been culled from the beginning.
-	/// Usually the score will have to be re-computed from scratch.
+	/// The scores has been removed from the beginning as well.
+	/// For simple scoring algorithms no action is required here.
 	Culled(usize),
+	/// Custom event to update the score triggered outside of the pool.
+	/// Handling this event is up to scoring implementation.
+	Event(T),
 }
 
 /// A transaction ordering.
@@ -69,7 +73,7 @@ pub enum Change {
 /// Implementation notes:
 /// - Returned `Score`s should match ordering of `compare` method.
 /// - `compare` will be called only within a context of transactions from the same sender.
-/// - `choose` will be called only if `compare` returns `Ordering::Equal`
+/// - `choose` may be called even if `compare` returns `Ordering::Equal`
 /// - `should_replace` is used to decide if new transaction should push out an old transaction already in the queue.
 /// - `Score`s and `compare` should align with `Ready` implementation.
 ///
@@ -79,9 +83,11 @@ pub enum Change {
 /// - `update_scores`: score defined as `gasPrice` if `n==0` and `max(scores[n-1], gasPrice)` if `n>0`
 /// - `should_replace`: compares `gasPrice` (decides if transaction from a different sender is more valuable)
 ///
-pub trait Scoring<T> {
+pub trait Scoring<T>: fmt::Debug {
 	/// A score of a transaction.
 	type Score: cmp::Ord + Clone + Default + fmt::Debug;
+	/// Custom scoring update event type.
+	type Event: fmt::Debug;
 
 	/// Decides on ordering of `T`s from a particular sender.
 	fn compare(&self, old: &T, other: &T) -> cmp::Ordering;
@@ -92,7 +98,7 @@ pub trait Scoring<T> {
 	/// Updates the transaction scores given a list of transactions and a change to previous scoring.
 	/// NOTE: you can safely assume that both slices have the same length.
 	/// (i.e. score at index `i` represents transaction at the same index)
-	fn update_scores(&self, txs: &[Arc<T>], scores: &mut [Self::Score], change: Change);
+	fn update_scores(&self, txs: &[Arc<T>], scores: &mut [Self::Score], change: Change<Self::Event>);
 
 	/// Decides if `new` should push out `old` transaction from the pool.
 	fn should_replace(&self, old: &T, new: &T) -> bool;

transaction-pool/src/status.rs

@@ -16,7 +16,7 @@
 
 /// Light pool status.
 /// This status is cheap to compute and can be called frequently.
-#[derive(Default, Debug, PartialEq, Eq)]
+#[derive(Default, Debug, Clone, PartialEq, Eq)]
 pub struct LightStatus {
 	/// Memory usage in bytes.
 	pub mem_usage: usize,
@@ -29,7 +29,7 @@ pub struct LightStatus {
 /// A full queue status.
 /// To compute this status it is required to provide `Ready`.
 /// NOTE: To compute the status we need to visit each transaction in the pool.
-#[derive(Default, Debug, PartialEq, Eq)]
+#[derive(Default, Debug, Clone, PartialEq, Eq)]
 pub struct Status {
 	/// Number of stalled transactions.
 	pub stalled: usize,

transaction-pool/src/tests/helpers.rs

@@ -21,11 +21,12 @@ use ethereum_types::U256;
 use {scoring, Scoring, Ready, Readiness, Address as Sender};
 use super::{Transaction, SharedTransaction};
 
-#[derive(Default)]
+#[derive(Debug, Default)]
 pub struct DummyScoring;
 
 impl Scoring<Transaction> for DummyScoring {
 	type Score = U256;
+	type Event = ();
 
 	fn compare(&self, old: &Transaction, new: &Transaction) -> cmp::Ordering {
 		old.nonce.cmp(&new.nonce)
@@ -43,9 +44,17 @@ impl Scoring<Transaction> for DummyScoring {
 		}
 	}
 
-	fn update_scores(&self, txs: &[SharedTransaction], scores: &mut [Self::Score], _change: scoring::Change) {
-		for i in 0..txs.len() {
-			scores[i] = txs[i].gas_price;
+	fn update_scores(&self, txs: &[SharedTransaction], scores: &mut [Self::Score], change: scoring::Change) {
+		if let scoring::Change::Event(_) = change {
+			// In case of event reset all scores to 0
+			for i in 0..txs.len() {
+				scores[i] = 0.into();
+			}
+		} else {
+			// Set to a gas price otherwise
+			for i in 0..txs.len() {
+				scores[i] = txs[i].gas_price;
+			}
 		}
 	}
 

transaction-pool/src/tests/mod.rs

@@ -125,7 +125,7 @@ fn should_reject_if_above_count() {
 	let tx2 = b.tx().nonce(1).new();
 	let hash = *tx2.hash();
 	txq.import(tx1).unwrap();
-	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash));
+	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
 	assert_eq!(txq.light_status().transaction_count, 1);
 
 	txq.clear();
@@ -151,7 +151,7 @@ fn should_reject_if_above_mem_usage() {
 	let tx2 = b.tx().nonce(2).mem_usage(2).new();
 	let hash = *tx2.hash();
 	txq.import(tx1).unwrap();
-	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash));
+	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
 	assert_eq!(txq.light_status().transaction_count, 1);
 
 	txq.clear();
@@ -177,7 +177,7 @@ fn should_reject_if_above_sender_count() {
 	let tx2 = b.tx().nonce(2).new();
 	let hash = *tx2.hash();
 	txq.import(tx1).unwrap();
-	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash));
+	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
 	assert_eq!(txq.light_status().transaction_count, 1);
 
 	txq.clear();
@@ -188,7 +188,7 @@ fn should_reject_if_above_sender_count() {
 	let hash = *tx2.hash();
 	txq.import(tx1).unwrap();
 	// This results in error because we also compare nonces
-	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash));
+	assert_eq!(txq.import(tx2).unwrap_err().kind(), &error::ErrorKind::TooCheapToEnter(hash, "0x0".into()));
 	assert_eq!(txq.light_status().transaction_count, 1);
 }
 
@@ -249,6 +249,66 @@ fn should_construct_pending() {
 	assert_eq!(pending.next(), None);
 }
 
+#[test]
+fn should_update_scoring_correctly() {
+	// given
+	let b = TransactionBuilder::default();
+	let mut txq = TestPool::default();
+
+	let tx0 = txq.import(b.tx().nonce(0).gas_price(5).new()).unwrap();
+	let tx1 = txq.import(b.tx().nonce(1).gas_price(5).new()).unwrap();
+	let tx2 = txq.import(b.tx().nonce(2).new()).unwrap();
+	// this transaction doesn't get to the block despite high gas price
+	// because of block gas limit and simplistic ordering algorithm.
+	txq.import(b.tx().nonce(3).gas_price(4).new()).unwrap();
+	//gap
+	txq.import(b.tx().nonce(5).new()).unwrap();
+
+	let tx5 = txq.import(b.tx().sender(1).nonce(0).new()).unwrap();
+	let tx6 = txq.import(b.tx().sender(1).nonce(1).new()).unwrap();
+	let tx7 = txq.import(b.tx().sender(1).nonce(2).new()).unwrap();
+	let tx8 = txq.import(b.tx().sender(1).nonce(3).gas_price(4).new()).unwrap();
+	// gap
+	txq.import(b.tx().sender(1).nonce(5).new()).unwrap();
+
+	let tx9 = txq.import(b.tx().sender(2).nonce(0).new()).unwrap();
+	assert_eq!(txq.light_status().transaction_count, 11);
+	assert_eq!(txq.status(NonceReady::default()), Status {
+		stalled: 0,
+		pending: 9,
+		future: 2,
+	});
+	assert_eq!(txq.status(NonceReady::new(1)), Status {
+		stalled: 3,
+		pending: 6,
+		future: 2,
+	});
+
+	txq.update_scores(&0.into(), ());
+
+	// when
+	let mut current_gas = U256::zero();
+	let limit = (21_000 * 8).into();
+	let mut pending = txq.pending(NonceReady::default()).take_while(|tx| {
+		let should_take = tx.gas + current_gas <= limit;
+		if should_take {
+			current_gas = current_gas + tx.gas
+		}
+		should_take
+	});
+
+	assert_eq!(pending.next(), Some(tx9));
+	assert_eq!(pending.next(), Some(tx5));
+	assert_eq!(pending.next(), Some(tx6));
+	assert_eq!(pending.next(), Some(tx7));
+	assert_eq!(pending.next(), Some(tx8));
+	// penalized transactions
+	assert_eq!(pending.next(), Some(tx0));
+	assert_eq!(pending.next(), Some(tx1));
+	assert_eq!(pending.next(), Some(tx2));
+	assert_eq!(pending.next(), None);
+}
+
 #[test]
 fn should_remove_transaction() {
 	// given
@@ -375,6 +435,20 @@ fn should_re_insert_after_cull() {
 	});
 }
 
+#[test]
+fn should_return_worst_transaction() {
+	// given
+	let b = TransactionBuilder::default();
+	let mut txq = TestPool::default();
+	assert!(txq.worst_transaction().is_none());
+
+	// when
+	txq.import(b.tx().nonce(0).gas_price(5).new()).unwrap();
+
+	// then
+	assert!(txq.worst_transaction().is_some());
+}
+
 mod listener {
 	use std::cell::RefCell;
 	use std::rc::Rc;
@@ -389,11 +463,11 @@ mod listener {
 			self.0.borrow_mut().push(if old.is_some() { "replaced" } else { "added" });
 		}
 
-		fn rejected(&mut self, _tx: Transaction) {
+		fn rejected(&mut self, _tx: &SharedTransaction, _reason: &error::ErrorKind) {
 			self.0.borrow_mut().push("rejected".into());
 		}
 
-		fn dropped(&mut self, _tx: &SharedTransaction) {
+		fn dropped(&mut self, _tx: &SharedTransaction, _new: Option<&Transaction>) {
 			self.0.borrow_mut().push("dropped".into());
 		}
 
@@ -401,8 +475,8 @@ mod listener {
 			self.0.borrow_mut().push("invalid".into());
 		}
 
-		fn cancelled(&mut self, _tx: &SharedTransaction) {
-			self.0.borrow_mut().push("cancelled".into());
+		fn canceled(&mut self, _tx: &SharedTransaction) {
+			self.0.borrow_mut().push("canceled".into());
 		}
 
 		fn mined(&mut self, _tx: &SharedTransaction) {
@@ -461,9 +535,9 @@ mod listener {
 
 		// then
 		txq.remove(&tx1.hash(), false);
-		assert_eq!(*results.borrow(), &["added", "added", "cancelled"]);
+		assert_eq!(*results.borrow(), &["added", "added", "canceled"]);
 		txq.remove(&tx2.hash(), true);
-		assert_eq!(*results.borrow(), &["added", "added", "cancelled", "invalid"]);
+		assert_eq!(*results.borrow(), &["added", "added", "canceled", "invalid"]);
 		assert_eq!(txq.light_status().transaction_count, 0);
 	}
 

transaction-pool/src/transactions.rs

@@ -23,9 +23,9 @@ use ready::{Ready, Readiness};
 use scoring::{self, Scoring};
 
 #[derive(Debug)]
-pub enum AddResult<T> {
+pub enum AddResult<T, S> {
 	Ok(Arc<T>),
-	TooCheapToEnter(T),
+	TooCheapToEnter(T, S),
 	TooCheap {
 		old: Arc<T>,
 		new: T,
@@ -93,10 +93,11 @@ impl<T: fmt::Debug, S: Scoring<T>> Transactions<T, S> {
 		})
 	}
 
-	fn push_cheapest_transaction(&mut self, tx: T, scoring: &S, max_count: usize) -> AddResult<T> {
+	fn push_cheapest_transaction(&mut self, tx: T, scoring: &S, max_count: usize) -> AddResult<T, S::Score> {
 		let index = self.transactions.len();
 		if index == max_count {
-			AddResult::TooCheapToEnter(tx)
+			let min_score = self.scores[index - 1].clone();
+			AddResult::TooCheapToEnter(tx, min_score)
 		} else {
 			let shared = Arc::new(tx);
 			self.transactions.push(shared.clone());
@@ -107,7 +108,11 @@ impl<T: fmt::Debug, S: Scoring<T>> Transactions<T, S> {
 		}
 	}
 
-	pub fn add(&mut self, tx: T, scoring: &S, max_count: usize) -> AddResult<T> {
+	pub fn update_scores(&mut self, scoring: &S, event: S::Event) {
+		scoring.update_scores(&self.transactions, &mut self.scores, scoring::Change::Event(event));
+	}
+
+	pub fn add(&mut self, tx: T, scoring: &S, max_count: usize) -> AddResult<T, S::Score> {
 		let index = match self.transactions.binary_search_by(|old| scoring.compare(old, &tx)) {
 			Ok(index) => index,
 			Err(index) => index,
@@ -192,6 +197,10 @@ impl<T: fmt::Debug, S: Scoring<T>> Transactions<T, S> {
 			}
 		}
 
+		if first_non_stalled == 0 {
+			return result;
+		}
+
 		// reverse the vectors to easily remove first elements.
 		self.transactions.reverse();
 		self.scores.reverse();