From bd2e4f9c131f544a8fde62df2370bd93bc3d2944 Mon Sep 17 00:00:00 2001
From: Andrew Jones <ascjones@gmail.com>
Date: Tue, 13 Aug 2019 14:20:59 +0100
Subject: [PATCH] tx-pool: accept local tx with higher gas price when pool full
 (#10901)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* tx-pool: accept local tx with higher gas price when pool full

* Revert "tx-pool: accept local tx with higher gas price when pool full"

This reverts commit 9d4adc5a

* tx-pool: new tx with same nonce as existing is ready

* tx-pool: explicit check for replacement tx (same sender & nonce)

* Update comment

Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Replace `ReplaceOld` with `InsertNew` for replacement txs
---
 miner/src/pool/replace.rs | 98 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/miner/src/pool/replace.rs b/miner/src/pool/replace.rs
index b1112dcae..0655af599 100644
--- a/miner/src/pool/replace.rs
+++ b/miner/src/pool/replace.rs
@@ -71,6 +71,20 @@ where
 			let old_score = (old.priority(), old.gas_price());
 			let new_score = (new.priority(), new.gas_price());
 			if new_score > old_score {
+				// Check if this is a replacement transaction.
+				//
+				// With replacement transactions we can safely return `InsertNew` here, because
+				// we don't need to remove `old` (worst transaction in the pool) since `new` will replace
+			    // some other transaction in the pool so we will never go above limit anyway.
+				if let Some(txs) = new.pooled_by_sender {
+					if let Ok(index) = txs.binary_search_by(|old| self.scoring.compare(old, new)) {
+						return match self.scoring.choose(&txs[index], new) {
+							Choice::ReplaceOld => Choice::InsertNew,
+							choice => choice,
+						}
+					}
+				}
+
 				let state = &self.client;
 				// calculate readiness based on state nonce + pooled txs from same sender
 				let is_ready = |replace: &ReplaceTransaction<T>| {
@@ -412,4 +426,88 @@ mod tests {
 
 		assert_eq!(replace.should_replace(&old, &new), ReplaceOld);
 	}
+
+	#[test]
+	fn should_accept_local_tx_with_same_sender_and_nonce_with_better_gas_price() {
+		let scoring = NonceAndGasPrice(PrioritizationStrategy::GasPriceOnly);
+		let client = TestClient::new().with_nonce(1);
+		let replace = ReplaceByScoreAndReadiness::new(scoring, client);
+
+		// current transaction is ready
+		let old_tx = {
+			let tx = Tx {
+				nonce: 1,
+				gas_price: 1,
+				..Default::default()
+			};
+			tx.signed().verified()
+		};
+
+		let new_sender = Random.generate().unwrap();
+		let tx_new_ready_1 = local_tx_verified(Tx {
+			nonce: 1,
+			gas_price: 1,
+			..Default::default()
+		}, &new_sender);
+
+		let tx_new_ready_2 = local_tx_verified(Tx {
+			nonce: 1,
+			gas_price: 2, // same nonce, higher gas price
+			..Default::default()
+		}, &new_sender);
+
+		let old_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(old_tx) };
+
+		let new_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_2) };
+		let pooled_txs = [
+			txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_1) },
+		];
+
+		let old = ReplaceTransaction::new(&old_tx, None);
+		let new = ReplaceTransaction::new(&new_tx, Some(&pooled_txs));
+
+		assert_eq!(replace.should_replace(&old, &new), InsertNew);
+	}
+
+	#[test]
+	fn should_reject_local_tx_with_same_sender_and_nonce_with_worse_gas_price() {
+		let scoring = NonceAndGasPrice(PrioritizationStrategy::GasPriceOnly);
+		let client = TestClient::new().with_nonce(1);
+		let replace = ReplaceByScoreAndReadiness::new(scoring, client);
+
+		// current transaction is ready
+		let old_tx = {
+			let tx = Tx {
+				nonce: 1,
+				gas_price: 1,
+				..Default::default()
+			};
+			tx.signed().verified()
+		};
+
+		let new_sender = Random.generate().unwrap();
+		let tx_new_ready_1 = local_tx_verified(Tx {
+			nonce: 1,
+			gas_price: 2,
+			..Default::default()
+		}, &new_sender);
+
+		let tx_new_ready_2 = local_tx_verified(Tx {
+			nonce: 1,
+			gas_price: 1, // same nonce, lower gas price
+			..Default::default()
+		}, &new_sender);
+
+		let old_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(old_tx) };
+
+		let new_tx = txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_2) };
+		let pooled_txs = [
+			txpool::Transaction { insertion_id: 0, transaction: Arc::new(tx_new_ready_1) },
+		];
+
+		let old = ReplaceTransaction::new(&old_tx, None);
+		let new = ReplaceTransaction::new(&new_tx, Some(&pooled_txs));
+
+		assert_eq!(replace.should_replace(&old, &new), RejectNew);
+	}
 }