diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index 6b1cc0c6568..a5a3537022c 100644 --- a/ethcore/src/client/client.rs +++ b/ethcore/src/client/client.rs @@ -1078,7 +1078,7 @@ impl BlockChainClient for Client { } fn pending_transactions(&self) -> Vec { - self.miner.pending_transactions() + self.miner.pending_transactions(self.chain.read().best_block_number()) } } diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 11929294f46..76451e7933a 100644 --- a/ethcore/src/client/test_client.rs +++ b/ethcore/src/client/test_client.rs @@ -602,6 +602,6 @@ impl BlockChainClient for TestBlockChainClient { } fn pending_transactions(&self) -> Vec { - self.miner.pending_transactions() + self.miner.pending_transactions(self.chain_info().best_block_number) } } diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index 38fc199cd4d..0d02f863196 100644 --- a/ethcore/src/miner/miner.rs +++ b/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(&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 { - 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 { + fn pending_transactions(&self, best_block: BlockNumber) -> Vec { 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 { + fn pending_transactions_hashes(&self, best_block: BlockNumber) -> Vec { 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 { + fn transaction(&self, best_block: BlockNumber, hash: &H256) -> Option { 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 { - 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 { + self.from_pending_block( + best_block, + || None, + |pending| { let txs = pending.transactions(); txs.iter() .map(|t| t.hash()) @@ -801,15 +846,15 @@ impl MinerService for Miner { logs: receipt.logs.clone(), } }) - }, - _ => None - } + } + ) } - fn pending_receipts(&self) -> BTreeMap { - 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 { + self.from_pending_block( + best_block, + || BTreeMap::new(), + |pending| { let hashes = pending.transactions() .iter() .map(|t| t.hash()); @@ -817,9 +862,8 @@ impl MinerService for Miner { let receipts = pending.receipts().iter().cloned(); hashes.zip(receipts).collect() - }, - _ => BTreeMap::new() - } + } + ) } fn last_nonce(&self, address: &Address) -> Option { @@ -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)); } diff --git a/ethcore/src/miner/mod.rs b/ethcore/src/miner/mod.rs index e95ce758ac2..c6bb00dceb1 100644 --- a/ethcore/src/miner/mod.rs +++ b/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; /// Returns hashes of transactions currently in pending - fn pending_transactions_hashes(&self) -> Vec; + fn pending_transactions_hashes(&self, best_block: BlockNumber) -> Vec; /// 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; + fn transaction(&self, best_block: BlockNumber, hash: &H256) -> Option; /// Get a list of all transactions. fn all_transactions(&self) -> Vec; /// Get a list of all pending transactions. - fn pending_transactions(&self) -> Vec; + fn pending_transactions(&self, best_block: BlockNumber) -> Vec; /// Get a list of all pending receipts. - fn pending_receipts(&self) -> BTreeMap; + fn pending_receipts(&self, best_block: BlockNumber) -> BTreeMap; /// Get a particular reciept. - fn pending_receipt(&self, hash: &H256) -> Option; + fn pending_receipt(&self, best_block: BlockNumber, hash: &H256) -> Option; /// Returns highest transaction nonce for given address. fn last_nonce(&self, address: &Address) -> Option; diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index b174e406ef6..c45f62c75db 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/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 EthClient where } } -pub fn pending_logs(miner: &M, filter: &EthcoreFilter) -> Vec where M: MinerService { - let receipts = miner.pending_receipts(); +pub fn pending_logs(miner: &M, best_block: EthBlockNumber, filter: &EthcoreFilter) -> Vec 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::>()) @@ -426,7 +426,8 @@ impl Eth for EthClient 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, Error> { @@ -445,8 +446,9 @@ impl Eth for EthClient 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 Eth for EthClient where .collect::>(); 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); } diff --git a/rpc/src/v1/impls/eth_filter.rs b/rpc/src/v1/impls/eth_filter.rs index 03d9d72159c..dd1c937ac43 100644 --- a/rpc/src/v1/impls/eth_filter.rs +++ b/rpc/src/v1/impls/eth_filter.rs @@ -81,7 +81,8 @@ impl EthFilter for EthFilterClient try!(self.active()); let mut polls = self.polls.lock(); - let pending_transactions = take_weak!(self.miner).pending_transactions_hashes(); + let best_block = take_weak!(self.client).chain_info().best_block_number; + let pending_transactions = take_weak!(self.miner).pending_transactions_hashes(best_block); let id = polls.create_poll(PollFilter::PendingTransaction(pending_transactions)); Ok(id.into()) } @@ -108,7 +109,8 @@ impl EthFilter for EthFilterClient }, PollFilter::PendingTransaction(ref mut previous_hashes) => { // get hashes of pending transactions - let current_hashes = take_weak!(self.miner).pending_transactions_hashes(); + let best_block = take_weak!(self.client).chain_info().best_block_number; + let current_hashes = take_weak!(self.miner).pending_transactions_hashes(best_block); let new_hashes = { @@ -149,7 +151,8 @@ impl EthFilter for EthFilterClient // additionally retrieve pending logs if include_pending { - let pending_logs = pending_logs(&*take_weak!(self.miner), &filter); + let best_block = take_weak!(self.client).chain_info().best_block_number; + let pending_logs = pending_logs(&*take_weak!(self.miner), best_block, &filter); // remove logs about which client was already notified about let new_pending_logs: Vec<_> = pending_logs.iter() @@ -190,7 +193,8 @@ impl EthFilter for EthFilterClient .collect::>(); if include_pending { - logs.extend(pending_logs(&*take_weak!(self.miner), &filter)); + let best_block = take_weak!(self.client).chain_info().best_block_number; + logs.extend(pending_logs(&*take_weak!(self.miner), best_block, &filter)); } let logs = limit_logs(logs, filter.limit); diff --git a/rpc/src/v1/tests/helpers/miner_service.rs b/rpc/src/v1/tests/helpers/miner_service.rs index ddc0b057b14..0787f2102fd 100644 --- a/rpc/src/v1/tests/helpers/miner_service.rs +++ b/rpc/src/v1/tests/helpers/miner_service.rs @@ -21,6 +21,7 @@ use util::standard::*; use ethcore::error::{Error, CallError}; use ethcore::client::{MiningBlockChainClient, Executed, CallAnalytics}; use ethcore::block::{ClosedBlock, IsBlock}; +use ethcore::header::BlockNumber; use ethcore::transaction::SignedTransaction; use ethcore::receipt::{Receipt, RichReceipt}; use ethcore::miner::{MinerService, MinerStatus, TransactionImportResult}; @@ -162,7 +163,7 @@ impl MinerService for TestMinerService { } /// Returns hashes of transactions currently in pending - fn pending_transactions_hashes(&self) -> Vec { + fn pending_transactions_hashes(&self, _best_block: BlockNumber) -> Vec { vec![] } @@ -186,7 +187,7 @@ impl MinerService for TestMinerService { Some(f(&open_block.close())) } - fn transaction(&self, hash: &H256) -> Option { + fn transaction(&self, _best_block: BlockNumber, hash: &H256) -> Option { self.pending_transactions.lock().get(hash).cloned() } @@ -194,13 +195,13 @@ impl MinerService for TestMinerService { self.pending_transactions.lock().values().cloned().collect() } - fn pending_transactions(&self) -> Vec { + fn pending_transactions(&self, _best_block: BlockNumber) -> Vec { self.pending_transactions.lock().values().cloned().collect() } - fn pending_receipt(&self, hash: &H256) -> Option { + fn pending_receipt(&self, _best_block: BlockNumber, hash: &H256) -> Option { // Not much point implementing this since the logic is complex and the only thing it relies on is pending_receipts, which is already tested. - self.pending_receipts().get(hash).map(|r| + self.pending_receipts(0).get(hash).map(|r| RichReceipt { transaction_hash: Default::default(), transaction_index: Default::default(), @@ -212,7 +213,7 @@ impl MinerService for TestMinerService { ) } - fn pending_receipts(&self) -> BTreeMap { + fn pending_receipts(&self, _best_block: BlockNumber) -> BTreeMap { self.pending_receipts.lock().clone() }