From 8fa9a240ccfe1e4b32ef611939f1bf8d1cfcf66e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= <tomasz@ethcore.io>
Date: Mon, 20 Jun 2016 18:33:08 +0200
Subject: [PATCH] Fixing last nonce values in case transaction is replaced

---
 ethcore/src/miner/transaction_queue.rs | 53 +++++++++++++++++++-------
 1 file changed, 40 insertions(+), 13 deletions(-)

diff --git a/ethcore/src/miner/transaction_queue.rs b/ethcore/src/miner/transaction_queue.rs
index 30ebc133f..cc8430b42 100644
--- a/ethcore/src/miner/transaction_queue.rs
+++ b/ethcore/src/miner/transaction_queue.rs
@@ -236,8 +236,8 @@ impl TransactionSet {
 		self.by_priority.insert(order.clone());
 		let r = self.by_address.insert(sender, nonce, order);
 		// If transaction was replaced remove it from priority queue
-		if let Some(ref order) = r {
-			self.by_priority.remove(order);
+		if let Some(ref old_order) = r {
+			self.by_priority.remove(old_order);
 		}
 		r
 	}
@@ -517,16 +517,9 @@ impl TransactionQueue {
 		// Remove from current
 		let order = self.current.drop(&sender, &nonce);
 		if order.is_some() {
-			// We will either move transaction to future or remove it completely
-			// so there will be no transactions from this sender in current
-			self.last_nonces.remove(&sender);
-			// First update height of transactions in future to avoid collisions
-			self.update_future(&sender, current_nonce);
-			// This should move all current transactions to future and remove old transactions
-			self.move_all_to_future(&sender, current_nonce);
-			// And now lets check if there is some chain of transactions in future
-			// that should be placed in current. It should also update last_nonces.
-			self.move_matching_future_to_current(sender, current_nonce, current_nonce);
+			// This will keep consistency in queue
+			// Moves all to future and then promotes a batch from current:
+			self.remove_all(sender, current_nonce);
 			return;
 		}
 	}
@@ -682,7 +675,8 @@ impl TransactionQueue {
 
 		try!(check_too_cheap(Self::replace_transaction(tx, state_nonce, &mut self.current, &mut self.by_hash)));
 		// Keep track of highest nonce stored in current
-		self.last_nonces.insert(address, nonce);
+		let new_max = self.last_nonces.get(&address).map_or(nonce, |n| cmp::max(nonce, *n));
+		self.last_nonces.insert(address, new_max);
 		// Update nonces of transactions in future
 		self.update_future(&address, state_nonce);
 		// Maybe there are some more items waiting in future?
@@ -1597,4 +1591,37 @@ mod test {
 		assert_eq!(txq.future.by_priority.iter().next().unwrap().hash, tx1.hash());
 	}
 
+	#[test]
+	fn should_return_correct_last_nonce() {
+		// given
+		let mut txq = TransactionQueue::new();
+		let (tx1, tx2, tx2_2, tx3) = {
+			let keypair = KeyPair::create().unwrap();
+			let secret = &keypair.secret();
+			let nonce = U256::from(123);
+			let tx = new_unsigned_tx(nonce);
+			let tx2 = new_unsigned_tx(nonce + 1.into());
+			let mut tx2_2 = new_unsigned_tx(nonce + 1.into());
+			tx2_2.gas_price = U256::from(5);
+			let tx3 = new_unsigned_tx(nonce + 2.into());
+
+
+			(tx.sign(secret), tx2.sign(secret), tx2_2.sign(secret), tx3.sign(secret))
+		};
+		let sender = tx1.sender().unwrap();
+		txq.add(tx1, &default_nonce, TransactionOrigin::Local).unwrap();
+		txq.add(tx2, &default_nonce, TransactionOrigin::Local).unwrap();
+		txq.add(tx3, &default_nonce, TransactionOrigin::Local).unwrap();
+		assert_eq!(txq.future.by_priority.len(), 0);
+		assert_eq!(txq.current.by_priority.len(), 3);
+
+		// when
+		let res = txq.add(tx2_2, &default_nonce, TransactionOrigin::Local);
+
+		// then
+		assert_eq!(txq.last_nonce(&sender).unwrap(), 125.into());
+		assert_eq!(res.unwrap(), TransactionImportResult::Current);
+		assert_eq!(txq.current.by_priority.len(), 3);
+	}
+
 }