Merge branch 'master' into rpc-logs

Conflicts:
	rpc/src/v1/tests/ethcore.rs
	rpc/src/v1/traits/ethcore.rs

miner/src/lib.rs

@@ -38,7 +38,7 @@
 //! fn main() {
 //! 	let mut service = NetworkService::start(NetworkConfiguration::new()).unwrap();
 //! 	let dir = env::temp_dir();
-//! 	let client = Client::new(ClientConfig::default(), ethereum::new_frontier(), &dir, service.io().channel()).unwrap();
+//! 	let client = Client::new(ClientConfig::default(), ethereum::new_frontier(), &dir, service.io().channel());
 //!
 //!		let miner: Miner = Miner::default();
 //!		// get status
@@ -100,6 +100,12 @@ pub trait MinerService : Send + Sync {
 	/// Set the gas limit we wish to target when sealing a new block.
 	fn set_gas_floor_target(&self, target: U256);
 
+	/// Get current transactions limit in queue.
+	fn transactions_limit(&self) -> usize;
+
+	/// Set maximal number of transactions kept in the queue (both current and future).
+	fn set_transactions_limit(&self, limit: usize);
+
 	/// Imports transactions to transaction queue.
 	fn import_transactions<T>(&self, transactions: Vec<SignedTransaction>, fetch_account: T) ->
 		Vec<Result<TransactionImportResult, Error>>

miner/src/miner.rs

@@ -205,6 +205,14 @@ impl MinerService for Miner {
 		*self.gas_floor_target.read().unwrap() / x!(5)
 	}
 
+	fn transactions_limit(&self) -> usize {
+		self.transaction_queue.lock().unwrap().limit()
+	}
+
+	fn set_transactions_limit(&self, limit: usize) {
+		self.transaction_queue.lock().unwrap().set_limit(limit)
+	}
+
 	/// Get the author that we will seal blocks as.
 	fn author(&self) -> Address {
 		*self.author.read().unwrap()

miner/src/transaction_queue.rs

@@ -86,7 +86,8 @@
 
 use std::default::Default;
 use std::cmp::{Ordering};
-use std::collections::{HashMap, HashSet, BTreeSet};
+use std::cmp;
+use std::collections::{HashMap, BTreeSet};
 use util::numbers::{Uint, U256};
 use util::hash::{Address, H256};
 use util::table::*;
@@ -204,8 +205,8 @@ impl TransactionSet {
 	/// Remove low priority transactions if there is more then specified by given `limit`.
 	///
 	/// It drops transactions from this set but also removes associated `VerifiedTransaction`.
-	/// Returns hashes of transactions removed because of limit.
-	fn enforce_limit(&mut self, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> Option<HashSet<H256>> {
+	/// Returns addresses and highes nonces of transactions removed because of limit.
+	fn enforce_limit(&mut self, by_hash: &mut HashMap<H256, VerifiedTransaction>) -> Option<HashMap<Address, U256>> {
 		let len = self.by_priority.len();
 		if len <= self.limit {
 			return None;
@@ -221,17 +222,18 @@ impl TransactionSet {
 				.collect()
 		};
 
-		Some(to_drop
-			.into_iter()
-			.map(|(sender, nonce)| {
+		Some(to_drop.into_iter()
+			.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.");
 
 				by_hash.remove(&order.hash)
 					.expect("Hash found in `by_priorty` matches the one dropped; so it is included in `by_hash`");
-				order.hash
-			})
-			.collect::<HashSet<H256>>())
+
+				let max = removed.get(&sender).map(|val| cmp::max(*val, nonce)).unwrap_or(nonce);
+				removed.insert(sender, max);
+				removed
+			}))
 	}
 
 	/// Drop transaction from this set (remove from `by_priority` and `by_address`)
@@ -248,6 +250,12 @@ impl TransactionSet {
 		self.by_priority.clear();
 		self.by_address.clear();
 	}
+
+	/// Sets new limit for number of transactions in this `TransactionSet`.
+	/// Note the limit is not applied (no transactions are removed) by calling this method.
+	fn set_limit(&mut self, limit: usize) {
+		self.limit = limit;
+	}
 }
 
 #[derive(Debug)]
@@ -305,20 +313,21 @@ impl Default for TransactionQueue {
 impl TransactionQueue {
 	/// Creates new instance of this Queue
 	pub fn new() -> Self {
-		Self::with_limits(1024, 1024)
+		Self::with_limit(1024)
 	}
 
 	/// Create new instance of this Queue with specified limits
-	pub fn with_limits(current_limit: usize, future_limit: usize) -> Self {
+	pub fn with_limit(limit: usize) -> Self {
 		let current = TransactionSet {
 			by_priority: BTreeSet::new(),
 			by_address: Table::new(),
-			limit: current_limit,
+			limit: limit,
 		};
+
 		let future = TransactionSet {
 			by_priority: BTreeSet::new(),
 			by_address: Table::new(),
-			limit: future_limit,
+			limit: limit,
 		};
 
 		TransactionQueue {
@@ -331,6 +340,20 @@ impl TransactionQueue {
 		}
 	}
 
+	/// Set the new limit for `current` and `future` queue.
+	pub fn set_limit(&mut self, limit: usize) {
+		self.current.set_limit(limit);
+		self.future.set_limit(limit);
+		// And ensure the limits
+		self.current.enforce_limit(&mut self.by_hash);
+		self.future.enforce_limit(&mut self.by_hash);
+	}
+
+	/// Returns current limit of transactions in the queue.
+	pub fn limit(&self) -> usize {
+		self.current.limit
+	}
+
 	/// Get the minimal gas price.
 	pub fn minimal_gas_price(&self) -> &U256 {
 		&self.minimal_gas_price
@@ -595,7 +618,6 @@ impl TransactionQueue {
 
 		let address = tx.sender();
 		let nonce = tx.nonce();
-		let hash = tx.hash();
 
 		let next_nonce = self.last_nonces
 			.get(&address)
@@ -606,7 +628,7 @@ impl TransactionQueue {
 		if nonce > next_nonce {
 			// We have a gap - put to future
 			try!(check_too_cheap(Self::replace_transaction(tx, next_nonce, &mut self.future, &mut self.by_hash)));
-			try!(check_if_removed(&hash, self.future.enforce_limit(&mut self.by_hash)));
+			try!(check_if_removed(&address, &nonce, self.future.enforce_limit(&mut self.by_hash)));
 			return Ok(TransactionImportResult::Future);
 		} else if nonce < state_nonce {
 			// Droping transaction
@@ -628,13 +650,31 @@ impl TransactionQueue {
 			let future_tx = self.by_hash.remove(&order.hash).unwrap();
 			try!(check_too_cheap(Self::replace_transaction(future_tx, state_nonce, &mut self.current, &mut self.by_hash)));
 		}
+
 		// Also enforce the limit
-		try!(check_if_removed(&hash, self.current.enforce_limit(&mut self.by_hash)));
+		let removed = self.current.enforce_limit(&mut self.by_hash);
+		// If some transaction were removed because of limit we need to update last_nonces also.
+		self.update_last_nonces(&removed);
+		// Trigger error if we were removed.
+		try!(check_if_removed(&address, &nonce, removed));
 
 		trace!(target: "miner", "status: {:?}", self.status());
 		Ok(TransactionImportResult::Current)
 	}
 
+	/// Updates
+	fn update_last_nonces(&mut self, removed_max_nonces: &Option<HashMap<Address, U256>>) {
+		if let Some(ref max_nonces) = *removed_max_nonces {
+			for (sender, nonce) in max_nonces.iter() {
+				if *nonce == U256::zero() {
+					self.last_nonces.remove(sender);
+				} else {
+					self.last_nonces.insert(*sender, *nonce - U256::one());
+				}
+			}
+		}
+	}
+
 	/// Replaces transaction in given set (could be `future` or `current`).
 	///
 	/// If there is already transaction with same `(sender, nonce)` it will be replaced iff `gas_price` is higher.
@@ -679,12 +719,15 @@ fn check_too_cheap(is_in: bool) -> Result<(), TransactionError> {
 	}
 }
 
-fn check_if_removed(hash: &H256, dropped: Option<HashSet<H256>>) -> Result<(), TransactionError> {
+fn check_if_removed(sender: &Address, nonce: &U256, dropped: Option<HashMap<Address, U256>>) -> Result<(), TransactionError> {
 	match dropped {
-		Some(ref dropped) if dropped.contains(hash) => {
-			Err(TransactionError::LimitReached)
+		Some(ref dropped) => match dropped.get(sender) {
+			Some(max) if nonce <= max => {
+				Err(TransactionError::LimitReached)
+			},
+			_ => Ok(()),
 		},
-		_ => Ok(())
+		_ => Ok(()),
 	}
 }
 
@@ -1155,8 +1198,10 @@ mod test {
 	#[test]
 	fn should_drop_old_transactions_when_hitting_the_limit() {
 		// given
-		let mut txq = TransactionQueue::with_limits(1, 1);
+		let mut txq = TransactionQueue::with_limit(1);
 		let (tx, tx2) = new_txs(U256::one());
+		let sender = tx.sender().unwrap();
+		let nonce = tx.nonce;
 		txq.add(tx.clone(), &default_nonce).unwrap();
 		assert_eq!(txq.status().pending, 1);
 
@@ -1169,11 +1214,35 @@ mod test {
 		assert_eq!(txq.status().pending, 1);
 		assert_eq!(t.len(), 1);
 		assert_eq!(t[0], tx);
+		assert_eq!(txq.last_nonce(&sender), Some(nonce));
+	}
+
+	#[test]
+	fn should_return_correct_nonces_when_dropped_because_of_limit() {
+		// given
+		let mut txq = TransactionQueue::with_limit(2);
+		let tx = new_tx();
+		let (tx1, tx2) = new_txs(U256::one());
+		let sender = tx1.sender().unwrap();
+		let nonce = tx1.nonce;
+		txq.add(tx1.clone(), &default_nonce).unwrap();
+		txq.add(tx2.clone(), &default_nonce).unwrap();
+		assert_eq!(txq.status().pending, 2);
+		assert_eq!(txq.last_nonce(&sender), Some(nonce + U256::one()));
+
+		// when
+		let res = txq.add(tx.clone(), &default_nonce);
+
+		// then
+		assert_eq!(res.unwrap(), TransactionImportResult::Current);
+		assert_eq!(txq.status().pending, 2);
+		assert_eq!(txq.last_nonce(&sender), Some(nonce));
 	}
 
 	#[test]
 	fn should_limit_future_transactions() {
-		let mut txq = TransactionQueue::with_limits(10, 1);
+		let mut txq = TransactionQueue::with_limit(1);
+		txq.current.set_limit(10);
 		let (tx1, tx2) = new_txs_with_gas_price_diff(U256::from(4), U256::from(1));
 		let (tx3, tx4) = new_txs_with_gas_price_diff(U256::from(4), U256::from(2));
 		txq.add(tx1.clone(), &default_nonce).unwrap();