Using pending block only if is not old

ethcore/src/client/client.rs

@@ -1078,7 +1078,7 @@ impl BlockChainClient for Client {
 	}
 
 	fn pending_transactions(&self) -> Vec<SignedTransaction> {
-		self.miner.pending_transactions()
+		self.miner.pending_transactions(self.chain.read().best_block_number())
 	}
 }
 

ethcore/src/client/test_client.rs

@@ -602,6 +602,6 @@ impl BlockChainClient for TestBlockChainClient {
 	}
 
 	fn pending_transactions(&self) -> Vec<SignedTransaction> {
-		self.miner.pending_transactions()
+		self.miner.pending_transactions(self.chain_info().best_block_number)
 	}
 }

ethcore/src/miner/miner.rs

@@ -493,6 +493,21 @@ impl Miner {
 
 	/// Are we allowed to do a non-mandatory reseal?
 	fn tx_reseal_allowed(&self) -> bool { Instant::now() > *self.next_allowed_reseal.lock() }
+
+	fn from_pending_block<H, F, G>(&self, latest_block_number: BlockNumber, from_chain: F, map_block: G) -> H
+		where F: Fn() -> H, G: Fn(&ClosedBlock) -> H {
+		let sealing_work = self.sealing_work.lock();
+		sealing_work.queue.peek_last_ref().map_or_else(
+			|| from_chain(),
+			|b| {
+				if b.block().header().number() > latest_block_number {
+					map_block(b)
+				} else {
+					from_chain()
+				}
+			}
+		)
+	}
 }
 
 const SEALING_TIMEOUT_IN_BLOCKS : u64 = 5;
@@ -565,29 +580,35 @@ impl MinerService for Miner {
 	}
 
 	fn balance(&self, chain: &MiningBlockChainClient, address: &Address) -> U256 {
-		let sealing_work = self.sealing_work.lock();
-		sealing_work.queue.peek_last_ref().map_or_else(
+		self.from_pending_block(
+			chain.chain_info().best_block_number,
 			|| chain.latest_balance(address),
 			|b| b.block().fields().state.balance(address)
 		)
 	}
 
 	fn storage_at(&self, chain: &MiningBlockChainClient, address: &Address, position: &H256) -> H256 {
-		let sealing_work = self.sealing_work.lock();
-		sealing_work.queue.peek_last_ref().map_or_else(
+		self.from_pending_block(
+			chain.chain_info().best_block_number,
 			|| chain.latest_storage_at(address, position),
 			|b| b.block().fields().state.storage_at(address, position)
 		)
 	}
 
 	fn nonce(&self, chain: &MiningBlockChainClient, address: &Address) -> U256 {
-		let sealing_work = self.sealing_work.lock();
-		sealing_work.queue.peek_last_ref().map_or_else(|| chain.latest_nonce(address), |b| b.block().fields().state.nonce(address))
+		self.from_pending_block(
+			chain.chain_info().best_block_number,
+			|| chain.latest_nonce(address),
+			|b| b.block().fields().state.nonce(address)
+		)
 	}
 
 	fn code(&self, chain: &MiningBlockChainClient, address: &Address) -> Option<Bytes> {
-		let sealing_work = self.sealing_work.lock();
-		sealing_work.queue.peek_last_ref().map_or_else(|| chain.latest_code(address), |b| b.block().fields().state.code(address).map(|c| (*c).clone()))
+		self.from_pending_block(
+			chain.chain_info().best_block_number,
+			|| chain.latest_code(address),
+			|b| b.block().fields().state.code(address).map(|c| (*c).clone())
+		)
 	}
 
 	fn set_author(&self, author: Address) {
@@ -737,50 +758,74 @@ impl MinerService for Miner {
 		queue.top_transactions()
 	}
 
-	fn pending_transactions(&self) -> Vec<SignedTransaction> {
+	fn pending_transactions(&self, best_block: BlockNumber) -> Vec<SignedTransaction> {
 		let queue = self.transaction_queue.lock();
-		let sw = self.sealing_work.lock();
-		// TODO: should only use the sealing_work when it's current (it could be an old block)
-		let sealing_set = match sw.enabled {
-			true => sw.queue.peek_last_ref(),
-			false => None,
-		};
-		match (&self.options.pending_set, sealing_set) {
-			(&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.top_transactions(),
-			(_, sealing) => sealing.map_or_else(Vec::new, |s| s.transactions().to_owned()),
+		match self.options.pending_set {
+			PendingSet::AlwaysQueue => queue.top_transactions(),
+			PendingSet::SealingOrElseQueue => {
+				self.from_pending_block(
+					best_block,
+					|| queue.top_transactions(),
+					|sealing| sealing.transactions().to_owned()
+				)
+			},
+			PendingSet::AlwaysSealing => {
+				self.from_pending_block(
+					best_block,
+					|| vec![],
+					|sealing| sealing.transactions().to_owned()
+				)
+			},
 		}
 	}
 
-	fn pending_transactions_hashes(&self) -> Vec<H256> {
+	fn pending_transactions_hashes(&self, best_block: BlockNumber) -> Vec<H256> {
 		let queue = self.transaction_queue.lock();
-		let sw = self.sealing_work.lock();
-		let sealing_set = match sw.enabled {
-			true => sw.queue.peek_last_ref(),
-			false => None,
-		};
-		match (&self.options.pending_set, sealing_set) {
-			(&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.pending_hashes(),
-			(_, sealing) => sealing.map_or_else(Vec::new, |s| s.transactions().iter().map(|t| t.hash()).collect()),
+		match self.options.pending_set {
+			PendingSet::AlwaysQueue => queue.pending_hashes(),
+			PendingSet::SealingOrElseQueue => {
+				self.from_pending_block(
+					best_block,
+					|| queue.pending_hashes(),
+					|sealing| sealing.transactions().iter().map(|t| t.hash()).collect()
+				)
+			},
+			PendingSet::AlwaysSealing => {
+				self.from_pending_block(
+					best_block,
+					|| vec![],
+					|sealing| sealing.transactions().iter().map(|t| t.hash()).collect()
+				)
+			},
 		}
 	}
 
-	fn transaction(&self, hash: &H256) -> Option<SignedTransaction> {
+	fn transaction(&self, best_block: BlockNumber, hash: &H256) -> Option<SignedTransaction> {
 		let queue = self.transaction_queue.lock();
-		let sw = self.sealing_work.lock();
-		let sealing_set = match sw.enabled {
-			true => sw.queue.peek_last_ref(),
-			false => None,
-		};
-		match (&self.options.pending_set, sealing_set) {
-			(&PendingSet::AlwaysQueue, _) | (&PendingSet::SealingOrElseQueue, None) => queue.find(hash),
-			(_, sealing) => sealing.and_then(|s| s.transactions().iter().find(|t| &t.hash() == hash).cloned()),
+		match self.options.pending_set {
+			PendingSet::AlwaysQueue => queue.find(hash),
+			PendingSet::SealingOrElseQueue => {
+				self.from_pending_block(
+					best_block,
+					|| queue.find(hash),
+					|sealing| sealing.transactions().iter().find(|t| &t.hash() == hash).cloned()
+				)
+			},
+			PendingSet::AlwaysSealing => {
+				self.from_pending_block(
+					best_block,
+					|| None,
+					|sealing| sealing.transactions().iter().find(|t| &t.hash() == hash).cloned()
+				)
+			},
 		}
 	}
 
-	fn pending_receipt(&self, hash: &H256) -> Option<RichReceipt> {
-		let sealing_work = self.sealing_work.lock();
-		match (sealing_work.enabled, sealing_work.queue.peek_last_ref()) {
-			(true, Some(pending)) => {
+	fn pending_receipt(&self, best_block: BlockNumber, hash: &H256) -> Option<RichReceipt> {
+		self.from_pending_block(
+			best_block,
+			|| None,
+			|pending| {
 				let txs = pending.transactions();
 				txs.iter()
 					.map(|t| t.hash())
@@ -801,25 +846,24 @@ impl MinerService for Miner {
 							logs: receipt.logs.clone(),
 						}
 					})
-			},
-			_ => None
-		}
+			}
+		)
 	}
 
-	fn pending_receipts(&self) -> BTreeMap<H256, Receipt> {
-		let sealing_work = self.sealing_work.lock();
-		match (sealing_work.enabled, sealing_work.queue.peek_last_ref()) {
-			(true, Some(pending)) => {
+	fn pending_receipts(&self, best_block: BlockNumber) -> BTreeMap<H256, Receipt> {
+		self.from_pending_block(
+			best_block,
+			|| BTreeMap::new(),
+			|pending| {
 				let hashes = pending.transactions()
 					.iter()
 					.map(|t| t.hash());
 
 				let receipts = pending.receipts().iter().cloned();
 
 				hashes.zip(receipts).collect()
-			},
-			_ => BTreeMap::new()
-		}
+			}
+		)
 	}
 
 	fn last_nonce(&self, address: &Address) -> Option<U256> {
@@ -1044,34 +1088,54 @@ mod tests {
 		let client = TestBlockChainClient::default();
 		let miner = miner();
 		let transaction = transaction();
+		let best_block = 0;
 		// when
 		let res = miner.import_own_transaction(&client, transaction);
 
 		// then
 		assert_eq!(res.unwrap(), TransactionImportResult::Current);
 		assert_eq!(miner.all_transactions().len(), 1);
-		assert_eq!(miner.pending_transactions().len(), 1);
-		assert_eq!(miner.pending_transactions_hashes().len(), 1);
-		assert_eq!(miner.pending_receipts().len(), 1);
+		assert_eq!(miner.pending_transactions(best_block).len(), 1);
+		assert_eq!(miner.pending_transactions_hashes(best_block).len(), 1);
+		assert_eq!(miner.pending_receipts(best_block).len(), 1);
 		// This method will let us know if pending block was created (before calling that method)
 		assert!(!miner.prepare_work_sealing(&client));
 	}
 
+	#[test]
+	fn should_not_use_pending_block_if_best_block_is_higher() {
+		// given
+		let client = TestBlockChainClient::default();
+		let miner = miner();
+		let transaction = transaction();
+		let best_block = 10;
+		// when
+		let res = miner.import_own_transaction(&client, transaction);
+
+		// then
+		assert_eq!(res.unwrap(), TransactionImportResult::Current);
+		assert_eq!(miner.all_transactions().len(), 1);
+		assert_eq!(miner.pending_transactions(best_block).len(), 0);
+		assert_eq!(miner.pending_transactions_hashes(best_block).len(), 0);
+		assert_eq!(miner.pending_receipts(best_block).len(), 0);
+	}
+
 	#[test]
 	fn should_import_external_transaction() {
 		// given
 		let client = TestBlockChainClient::default();
 		let miner = miner();
 		let transaction = transaction();
+		let best_block = 0;
 		// when
 		let res = miner.import_external_transactions(&client, vec![transaction]).pop().unwrap();
 
 		// then
 		assert_eq!(res.unwrap(), TransactionImportResult::Current);
 		assert_eq!(miner.all_transactions().len(), 1);
-		assert_eq!(miner.pending_transactions_hashes().len(), 0);
-		assert_eq!(miner.pending_transactions().len(), 0);
-		assert_eq!(miner.pending_receipts().len(), 0);
+		assert_eq!(miner.pending_transactions_hashes(best_block).len(), 0);
+		assert_eq!(miner.pending_transactions(best_block).len(), 0);
+		assert_eq!(miner.pending_receipts(best_block).len(), 0);
 		// This method will let us know if pending block was created (before calling that method)
 		assert!(miner.prepare_work_sealing(&client));
 	}

ethcore/src/miner/mod.rs

@@ -56,6 +56,7 @@ use std::collections::BTreeMap;
 use util::{H256, U256, Address, Bytes};
 use client::{MiningBlockChainClient, Executed, CallAnalytics};
 use block::ClosedBlock;
+use header::BlockNumber;
 use receipt::{RichReceipt, Receipt};
 use error::{Error, CallError};
 use transaction::SignedTransaction;
@@ -115,7 +116,7 @@ pub trait MinerService : Send + Sync {
 		Result<TransactionImportResult, Error>;
 
 	/// Returns hashes of transactions currently in pending
-	fn pending_transactions_hashes(&self) -> Vec<H256>;
+	fn pending_transactions_hashes(&self, best_block: BlockNumber) -> Vec<H256>;
 
 	/// Removes all transactions from the queue and restart mining operation.
 	fn clear_and_reset(&self, chain: &MiningBlockChainClient);
@@ -135,19 +136,19 @@ pub trait MinerService : Send + Sync {
 		where F: FnOnce(&ClosedBlock) -> T, Self: Sized;
 
 	/// Query pending transactions for hash.
-	fn transaction(&self, hash: &H256) -> Option<SignedTransaction>;
+	fn transaction(&self, best_block: BlockNumber, hash: &H256) -> Option<SignedTransaction>;
 
 	/// Get a list of all transactions.
 	fn all_transactions(&self) -> Vec<SignedTransaction>;
 
 	/// Get a list of all pending transactions.
-	fn pending_transactions(&self) -> Vec<SignedTransaction>;
+	fn pending_transactions(&self, best_block: BlockNumber) -> Vec<SignedTransaction>;
 
 	/// Get a list of all pending receipts.
-	fn pending_receipts(&self) -> BTreeMap<H256, Receipt>;
+	fn pending_receipts(&self, best_block: BlockNumber) -> BTreeMap<H256, Receipt>;
 
 	/// Get a particular reciept.
-	fn pending_receipt(&self, hash: &H256) -> Option<RichReceipt>;
+	fn pending_receipt(&self, best_block: BlockNumber, hash: &H256) -> Option<RichReceipt>;
 
 	/// Returns highest transaction nonce for given address.
 	fn last_nonce(&self, address: &Address) -> Option<U256>;

rpc/src/v1/impls/eth.rs

@@ -33,7 +33,7 @@ use util::{FromHex, Mutex};
 use rlp::{self, UntrustedRlp, View};
 use ethcore::account_provider::AccountProvider;
 use ethcore::client::{MiningBlockChainClient, BlockID, TransactionID, UncleID};
-use ethcore::header::Header as BlockHeader;
+use ethcore::header::{Header as BlockHeader, BlockNumber as EthBlockNumber};
 use ethcore::block::IsBlock;
 use ethcore::views::*;
 use ethcore::ethereum::Ethash;
@@ -198,8 +198,8 @@ impl<C, S: ?Sized, M, EM> EthClient<C, S, M, EM> where
 	}
 }
 
-pub fn pending_logs<M>(miner: &M, filter: &EthcoreFilter) -> Vec<Log> where M: MinerService {
-	let receipts = miner.pending_receipts();
+pub fn pending_logs<M>(miner: &M, best_block: EthBlockNumber, filter: &EthcoreFilter) -> Vec<Log> where M: MinerService {
+	let receipts = miner.pending_receipts(best_block);
 
 	let pending_logs = receipts.into_iter()
 		.flat_map(|(hash, r)| r.logs.into_iter().map(|l| (hash.clone(), l)).collect::<Vec<(H256, LogEntry)>>())
@@ -426,7 +426,8 @@ impl<C, S: ?Sized, M, EM> Eth for EthClient<C, S, M, EM> where
 		try!(self.active());
 		let hash: H256 = hash.into();
 		let miner = take_weak!(self.miner);
-		Ok(try!(self.transaction(TransactionID::Hash(hash))).or_else(|| miner.transaction(&hash).map(Into::into)))
+		let client = take_weak!(self.client);
+		Ok(try!(self.transaction(TransactionID::Hash(hash))).or_else(|| miner.transaction(client.chain_info().best_block_number, &hash).map(Into::into)))
 	}
 
 	fn transaction_by_block_hash_and_index(&self, hash: RpcH256, index: Index) -> Result<Option<Transaction>, Error> {
@@ -445,8 +446,9 @@ impl<C, S: ?Sized, M, EM> Eth for EthClient<C, S, M, EM> where
 		try!(self.active());
 
 		let miner = take_weak!(self.miner);
+		let best_block = take_weak!(self.client).chain_info().best_block_number;
 		let hash: H256 = hash.into();
-		match (miner.pending_receipt(&hash), self.options.allow_pending_receipt_query) {
+		match (miner.pending_receipt(best_block, &hash), self.options.allow_pending_receipt_query) {
 			(Some(receipt), true) => Ok(Some(receipt.into())),
 			_ => {
 				let client = take_weak!(self.client);
@@ -488,7 +490,8 @@ impl<C, S: ?Sized, M, EM> Eth for EthClient<C, S, M, EM> where
 			.collect::<Vec<Log>>();
 
 		if include_pending {
-			let pending = pending_logs(&*take_weak!(self.miner), &filter);
+			let best_block = take_weak!(self.client).chain_info().best_block_number;
+			let pending = pending_logs(&*take_weak!(self.miner), best_block, &filter);
 			logs.extend(pending);
 		}