From ac6180a6feebb0be31c06ca77d5594698743bd04 Mon Sep 17 00:00:00 2001 From: keorn Date: Mon, 20 Feb 2017 15:35:53 +0000 Subject: [PATCH] seals_internally (#4613) --- ethcore/src/engines/authority_round.rs | 4 +-- ethcore/src/engines/basic_authority.rs | 9 ++++--- ethcore/src/engines/instant_seal.rs | 2 +- ethcore/src/engines/mod.rs | 9 +++---- ethcore/src/engines/tendermint/mod.rs | 4 +-- ethcore/src/miner/miner.rs | 37 +++++++++++++------------- 6 files changed, 33 insertions(+), 32 deletions(-) diff --git a/ethcore/src/engines/authority_round.rs b/ethcore/src/engines/authority_round.rs index 9c69ce6ad..4f99a644e 100644 --- a/ethcore/src/engines/authority_round.rs +++ b/ethcore/src/engines/authority_round.rs @@ -220,8 +220,8 @@ impl Engine for AuthorityRound { }); } - fn is_sealer(&self, author: &Address) -> Option { - Some(self.validators.contains(author)) + fn seals_internally(&self) -> Option { + Some(self.validators.contains(&self.signer.address())) } /// Attempt to seal the block internally. diff --git a/ethcore/src/engines/basic_authority.rs b/ethcore/src/engines/basic_authority.rs index 5d1abd064..50051bf7e 100644 --- a/ethcore/src/engines/basic_authority.rs +++ b/ethcore/src/engines/basic_authority.rs @@ -103,8 +103,8 @@ impl Engine for BasicAuthority { }); } - fn is_sealer(&self, author: &Address) -> Option { - Some(self.validators.contains(author)) + fn seals_internally(&self) -> Option { + Some(self.validators.contains(&self.signer.address())) } /// Attempt to seal the block internally. @@ -268,7 +268,8 @@ mod tests { let authority = tap.insert_account(Secret::from_slice(&"".sha3()).unwrap(), "").unwrap(); let engine = new_test_authority().engine; - assert!(!engine.is_sealer(&Address::default()).unwrap()); - assert!(engine.is_sealer(&authority).unwrap()); + assert!(!engine.seals_internally().unwrap()); + engine.set_signer(Arc::new(tap), authority, "".into()); + assert!(engine.seals_internally().unwrap()); } } diff --git a/ethcore/src/engines/instant_seal.rs b/ethcore/src/engines/instant_seal.rs index c709398e3..d2deea702 100644 --- a/ethcore/src/engines/instant_seal.rs +++ b/ethcore/src/engines/instant_seal.rs @@ -56,7 +56,7 @@ impl Engine for InstantSeal { Schedule::new_post_eip150(usize::max_value(), true, true, true) } - fn is_sealer(&self, _author: &Address) -> Option { Some(true) } + fn seals_internally(&self) -> Option { Some(true) } fn generate_seal(&self, _block: &ExecutedBlock) -> Seal { Seal::Regular(Vec::new()) diff --git a/ethcore/src/engines/mod.rs b/ethcore/src/engines/mod.rs index a22410cee..2320f49a0 100644 --- a/ethcore/src/engines/mod.rs +++ b/ethcore/src/engines/mod.rs @@ -128,11 +128,10 @@ pub trait Engine : Sync + Send { /// Block transformation functions, after the transactions. fn on_close_block(&self, _block: &mut ExecutedBlock) {} - /// If Some(true) this author is able to generate seals, generate_seal has to be implemented. - /// None indicates that this Engine never seals internally regardless of author (e.g. PoW). - fn is_sealer(&self, _author: &Address) -> Option { None } - /// Checks if default address is able to seal. - fn is_default_sealer(&self) -> Option { self.is_sealer(&Default::default()) } + /// None means that it requires external input (e.g. PoW) to seal a block. + /// Some(true) means the engine is currently prime for seal generation (i.e. node is the current validator). + /// Some(false) means that the node might seal internally but is not qualified now. + fn seals_internally(&self) -> Option { None } /// Attempt to seal the block internally. /// /// If `Some` is returned, then you get a valid seal. diff --git a/ethcore/src/engines/tendermint/mod.rs b/ethcore/src/engines/tendermint/mod.rs index e4bb2ea2f..bc4786526 100644 --- a/ethcore/src/engines/tendermint/mod.rs +++ b/ethcore/src/engines/tendermint/mod.rs @@ -410,8 +410,8 @@ impl Engine for Tendermint { } /// Should this node participate. - fn is_sealer(&self, address: &Address) -> Option { - Some(self.is_authority(address)) + fn seals_internally(&self) -> Option { + Some(self.is_authority(&self.signer.address())) } /// Attempt to seal generate a proposal seal. diff --git a/ethcore/src/miner/miner.rs b/ethcore/src/miner/miner.rs index b4a7f2327..410123c9a 100644 --- a/ethcore/src/miner/miner.rs +++ b/ethcore/src/miner/miner.rs @@ -215,8 +215,6 @@ pub struct Miner { sealing_block_last_request: Mutex, // for sealing... options: MinerOptions, - /// Does the node perform internal (without work) sealing. - pub seals_internally: bool, gas_range_target: RwLock<(U256, U256)>, author: RwLock
, @@ -275,9 +273,8 @@ impl Miner { queue: UsingQueue::new(options.work_queue_size), enabled: options.force_sealing || !options.new_work_notify.is_empty() - || spec.engine.is_default_sealer().unwrap_or(false) + || spec.engine.seals_internally().is_some() }), - seals_internally: spec.engine.is_default_sealer().is_some(), gas_range_target: RwLock::new((U256::zero(), U256::zero())), author: RwLock::new(Address::default()), extra_data: RwLock::new(Vec::new()), @@ -455,7 +452,7 @@ impl Miner { let last_request = *self.sealing_block_last_request.lock(); let should_disable_sealing = !self.forced_sealing() && !has_local_transactions - && !self.seals_internally + && self.engine.seals_internally().is_none() && best_block > last_request && best_block - last_request > SEALING_TIMEOUT_IN_BLOCKS; @@ -765,21 +762,21 @@ impl MinerService for Miner { } fn set_author(&self, author: Address) { - if self.seals_internally { + if self.engine.seals_internally().is_some() { let mut sealing_work = self.sealing_work.lock(); - sealing_work.enabled = self.engine.is_sealer(&author).unwrap_or(false); + sealing_work.enabled = true; } *self.author.write() = author; } fn set_engine_signer(&self, address: Address, password: String) -> Result<(), AccountError> { - if self.seals_internally { + if self.engine.seals_internally().is_some() { if let Some(ref ap) = self.accounts { ap.sign(address.clone(), Some(password.clone()), Default::default())?; // Limit the scope of the locks. { let mut sealing_work = self.sealing_work.lock(); - sealing_work.enabled = self.engine.is_sealer(&address).unwrap_or(false); + sealing_work.enabled = true; *self.author.write() = address; } // -------------------------------------------------------------------------- @@ -914,7 +911,7 @@ impl MinerService for Miner { if imported.is_ok() && self.options.reseal_on_own_tx && self.tx_reseal_allowed() { // Make sure to do it after transaction is imported and lock is droped. // We need to create pending block and enable sealing. - if self.seals_internally || !self.prepare_work_sealing(chain) { + if self.engine.seals_internally().unwrap_or(false) || !self.prepare_work_sealing(chain) { // If new block has not been prepared (means we already had one) // or Engine might be able to seal internally, // we need to update sealing. @@ -1071,14 +1068,18 @@ impl MinerService for Miner { // -------------------------------------------------------------------------- trace!(target: "miner", "update_sealing: preparing a block"); let (block, original_work_hash) = self.prepare_block(chain); - if self.seals_internally { - trace!(target: "miner", "update_sealing: engine indicates internal sealing"); - if self.seal_and_import_block_internally(chain, block) { - trace!(target: "miner", "update_sealing: imported internally sealed block"); - } - } else { - trace!(target: "miner", "update_sealing: engine does not seal internally, preparing work"); - self.prepare_work(block, original_work_hash); + match self.engine.seals_internally() { + Some(true) => { + trace!(target: "miner", "update_sealing: engine indicates internal sealing"); + if self.seal_and_import_block_internally(chain, block) { + trace!(target: "miner", "update_sealing: imported internally sealed block"); + } + }, + None => { + trace!(target: "miner", "update_sealing: engine does not seal internally, preparing work"); + self.prepare_work(block, original_work_hash) + }, + _ => trace!(target: "miner", "update_sealing: engine is not keen to seal internally right now") } } }