From e41b6c410fe0cfb665ccde0c5f48fbf78fc71776 Mon Sep 17 00:00:00 2001 From: keorn Date: Tue, 13 Sep 2016 12:52:14 +0200 Subject: [PATCH] split requires_reseal, add test and new test miner --- ethcore/src/engines/instant_seal.rs | 10 +-- ethcore/src/miner/miner.rs | 133 ++++++++++++++++------------ ethcore/src/spec/spec.rs | 5 ++ 3 files changed, 83 insertions(+), 65 deletions(-) diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index 9153d1913..e88c1d102 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -73,18 +73,12 @@ mod tests { use spec::Spec; use block::*; - /// Create a new test chain spec with `BasicAuthority` consensus engine. - fn new_test_instant() -> Spec { - let bytes: &[u8] = include_bytes!("../../res/instant_seal.json"); - Spec::load(bytes).expect("invalid chain spec") - } - #[test] fn instant_can_seal() { let tap = AccountProvider::transient_provider(); let addr = tap.insert_account("".sha3(), "").unwrap(); - let spec = new_test_instant(); + let spec = Spec::new_test_instant(); let engine = &*spec.engine; let genesis_header = spec.genesis_header(); let mut db_result = get_temp_journal_db(); @@ -100,7 +94,7 @@ mod tests { #[test] fn instant_cant_verify() { - let engine = new_test_instant().engine; + let engine = Spec::new_test_instant().engine; let mut header: Header = Header::default(); assert!(engine.verify_block_basic(&header, None).is_ok()); diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index abf13fc6a..152b0e994 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -34,6 +34,7 @@ use miner::{MinerService, MinerStatus, TransactionQueue, AccountDetails, Transac use miner::work_notify::WorkPoster; use client::TransactionImportResult; use miner::price_info::PriceInfo; +use header::BlockNumber; /// Different possible definitions for pending transaction set. #[derive(Debug, PartialEq)] @@ -334,6 +335,36 @@ impl Miner { (block, original_work_hash) } + /// Check is reseal is allowed and necessary. + fn requires_reseal(&self, best_block: BlockNumber) -> bool { + let has_local_transactions = self.transaction_queue.lock().has_local_pending_transactions(); + let mut sealing_work = self.sealing_work.lock(); + if sealing_work.enabled { + trace!(target: "miner", "requires_reseal: sealing enabled"); + let last_request = *self.sealing_block_last_request.lock(); + let should_disable_sealing = !self.forced_sealing() + && !has_local_transactions + && best_block > last_request + && best_block - last_request > SEALING_TIMEOUT_IN_BLOCKS; + + trace!(target: "miner", "requires_reseal: should_disable_sealing={}; best_block={}, last_request={}", should_disable_sealing, best_block, last_request); + + if should_disable_sealing { + trace!(target: "miner", "Miner sleeping (current {}, last {})", best_block, last_request); + sealing_work.enabled = false; + sealing_work.queue.reset(); + false + } else { + // sealing enabled and we don't want to sleep. + *self.next_allowed_reseal.lock() = Instant::now() + self.options.reseal_min_period; + true + } + } else { + // sealing is disabled. + false + } + } + /// Attempts to perform internal sealing (one that does not require work) to return Ok(sealed), /// Err(Some(block)) returns for unsuccesful sealing while Err(None) indicates misspecified engine. fn seal_block_internally(&self, block: ClosedBlock) -> Result> { @@ -777,41 +808,13 @@ impl MinerService for Miner { self.transaction_queue.lock().last_nonce(address) } + /// Update sealing if required. /// Prepare the block and work if the Engine does not seal internally. fn update_sealing(&self, chain: &MiningBlockChainClient) { trace!(target: "miner", "update_sealing"); - let requires_reseal = { - let has_local_transactions = self.transaction_queue.lock().has_local_pending_transactions(); - let mut sealing_work = self.sealing_work.lock(); - if sealing_work.enabled { - trace!(target: "miner", "update_sealing: sealing enabled"); - let current_no = chain.chain_info().best_block_number; - let last_request = *self.sealing_block_last_request.lock(); - let should_disable_sealing = !self.forced_sealing() - && !has_local_transactions - && current_no > last_request - && current_no - last_request > SEALING_TIMEOUT_IN_BLOCKS; - trace!(target: "miner", "update_sealing: should_disable_sealing={}; current_no={}, last_request={}", should_disable_sealing, current_no, last_request); - - if should_disable_sealing { - trace!(target: "miner", "Miner sleeping (current {}, last {})", current_no, last_request); - sealing_work.enabled = false; - sealing_work.queue.reset(); - false - } else { - // sealing enabled and we don't want to sleep. - *self.next_allowed_reseal.lock() = Instant::now() + self.options.reseal_min_period; - true - } - } else { - // sealing is disabled. - false - } - }; - - if requires_reseal { + if self.requires_reseal(chain.chain_info().best_block_number) { // -------------------------------------------------------------------------- // | NOTE Code below requires transaction_queue and sealing_work locks. | // | Make sure to release the locks before calling that method. | @@ -937,11 +940,12 @@ mod tests { use super::*; use util::*; use ethkey::{Generator, Random}; - use client::{TestBlockChainClient, EachBlockWith}; - use client::{TransactionImportResult}; - use types::transaction::{Transaction, Action}; + use client::{BlockChainClient, TestBlockChainClient, EachBlockWith, TransactionImportResult}; + use header::BlockNumber; + use types::transaction::{Transaction, SignedTransaction, Action}; use block::*; use spec::Spec; + use tests::helpers::{generate_dummy_client}; #[test] fn should_prepare_block_to_seal() { @@ -995,23 +999,24 @@ mod tests { )).ok().expect("Miner was just created.") } + fn transaction() -> SignedTransaction { + let keypair = Random.generate().unwrap(); + Transaction { + action: Action::Create, + value: U256::zero(), + data: "3331600055".from_hex().unwrap(), + gas: U256::from(100_000), + gas_price: U256::zero(), + nonce: U256::zero(), + }.sign(keypair.secret()) + } + #[test] fn should_make_pending_block_when_importing_own_transaction() { // given let client = TestBlockChainClient::default(); let miner = miner(); - let transaction = { - let keypair = Random.generate().unwrap(); - Transaction { - action: Action::Create, - value: U256::zero(), - data: "3331600055".from_hex().unwrap(), - gas: U256::from(100_000), - gas_price: U256::zero(), - nonce: U256::zero(), - }.sign(keypair.secret()) - }; - + let transaction = transaction(); // when let res = miner.import_own_transaction(&client, transaction); @@ -1030,18 +1035,7 @@ mod tests { // given let client = TestBlockChainClient::default(); let miner = miner(); - let transaction = { - let keypair = Random.generate().unwrap(); - Transaction { - action: Action::Create, - value: U256::zero(), - data: "3331600055".from_hex().unwrap(), - gas: U256::from(100_000), - gas_price: U256::zero(), - nonce: U256::zero(), - }.sign(keypair.secret()) - }; - + let transaction = transaction(); // when let res = miner.import_external_transactions(&client, vec![transaction]).pop().unwrap(); @@ -1054,4 +1048,29 @@ mod tests { // This method will let us know if pending block was created (before calling that method) assert_eq!(miner.prepare_work_sealing(&client), true); } + + #[test] + fn internal_seals_without_work() { + let miner = Miner::with_spec(&Spec::new_test_instant()); + { + let mut sealing_work = miner.sealing_work.lock(); + sealing_work.enabled = true; + } + let c = generate_dummy_client(2); + let client = c.reference().as_ref(); + + assert_eq!(miner.import_external_transactions(client, vec![transaction()]).pop().unwrap().unwrap(), TransactionImportResult::Current); + + miner.update_sealing(client); + client.flush_queue(); + assert!(miner.pending_block().is_none()); + assert_eq!(client.chain_info().best_block_number, 3 as BlockNumber); + + assert_eq!(miner.import_own_transaction(client, transaction()).unwrap(), TransactionImportResult::Current); + + miner.update_sealing(client); + client.flush_queue(); + assert!(miner.pending_block().is_none()); + assert_eq!(client.chain_info().best_block_number, 4 as BlockNumber); + } } diff --git a/ethcore/src/spec/spec.rs b/ethcore/src/spec/spec.rs index 58317e97b..a6b5ad649 100644 --- a/ethcore/src/spec/spec.rs +++ b/ethcore/src/spec/spec.rs @@ -260,6 +260,11 @@ impl Spec { pub fn new_null() -> Self { Spec::load(include_bytes!("../../res/null.json") as &[u8]).expect("null.json is invalid") } + + /// Create a new Spec with InstantSeal consensus which does internal sealing (not requiring work). + pub fn new_test_instant() -> Self { + Spec::load(include_bytes!("../../res/instant_seal.json") as &[u8]).expect("instant_seal.json is invalid") + } } #[cfg(test)]