Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Commit

Permalink
Make InstantSeal Instant again (#11186)
Browse files Browse the repository at this point in the history
  • Loading branch information
seunlanlege committed Nov 11, 2019
1 parent 6891ad9 commit d43ae58
Show file tree
Hide file tree
Showing 13 changed files with 123 additions and 63 deletions.
4 changes: 2 additions & 2 deletions ethcore/light/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

use std::sync::{Weak, Arc};

use ethcore::client::{ClientReport, EnvInfo, ClientIoMessage};
use ethcore::client::{ClientReport, EnvInfo, ClientIoMessage, traits::ForceUpdateSealing};
use ethcore::engines::{epoch, EthEngine, EpochChange, EpochTransition, Proof};
use ethcore::machine::EthereumMachine;
use ethcore::error::{Error, EthcoreResult};
Expand Down Expand Up @@ -620,7 +620,7 @@ impl<T: ChainDataFetcher> ::ethcore::client::ChainInfo for Client<T> {
}

impl<T: ChainDataFetcher> ::ethcore::client::EngineClient for Client<T> {
fn update_sealing(&self) { }
fn update_sealing(&self, _force: ForceUpdateSealing) {}
fn submit_seal(&self, _block_hash: H256, _seal: Vec<Vec<u8>>) { }
fn broadcast_consensus_message(&self, _message: Vec<u8>) { }

Expand Down
64 changes: 47 additions & 17 deletions ethcore/src/client/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ use client::{
BlockId, TransactionId, UncleId, TraceId, ClientConfig, BlockChainClient,
TraceFilter, CallAnalytics, Mode,
ChainNotify, NewBlocks, ChainRoute, PruningInfo, ProvingBlockChainClient, EngineInfo, ChainMessageType,
IoClient, BadBlocks,
IoClient, BadBlocks, traits::ForceUpdateSealing
};
use client::bad_blocks;
use engines::{MAX_UNCLE_AGE, EthEngine, EpochTransition, ForkChoice, EngineError};
Expand Down Expand Up @@ -2424,7 +2424,9 @@ impl ImportSealedBlock for Client {
let raw = block.rlp_bytes();
let header = block.header.clone();
let hash = header.hash();
self.notify(|n| n.block_pre_import(&raw, &hash, header.difficulty()));
self.notify(|n| {
n.block_pre_import(&raw, &hash, header.difficulty())
});

let route = {
// Do a super duper basic verification to detect potential bugs
Expand Down Expand Up @@ -2512,19 +2514,22 @@ impl ::miner::TransactionVerifierClient for Client {}
impl ::miner::BlockChainClient for Client {}

impl super::traits::EngineClient for Client {
fn update_sealing(&self) {
self.importer.miner.update_sealing(self)
fn update_sealing(&self, force: ForceUpdateSealing) {
self.importer.miner.update_sealing(self, force)
}

fn submit_seal(&self, block_hash: H256, seal: Vec<Bytes>) {
let import = self.importer.miner.submit_seal(block_hash, seal).and_then(|block| self.import_sealed_block(block));
let import = self.importer.miner.submit_seal(block_hash, seal)
.and_then(|block| self.import_sealed_block(block));
if let Err(err) = import {
warn!(target: "poa", "Wrong internal seal submission! {:?}", err);
}
}

fn broadcast_consensus_message(&self, message: Bytes) {
self.notify(|notify| notify.broadcast(ChainMessageType::Consensus(message.clone())));
self.notify(|notify| {
notify.broadcast(ChainMessageType::Consensus(message.clone()))
});
}

fn epoch_transition_for(&self, parent_hash: H256) -> Option<::engines::EpochTransition> {
Expand Down Expand Up @@ -2598,13 +2603,21 @@ impl ImportExportBlocks for Client {
if i % 10000 == 0 {
info!("#{}", i);
}
let b = self.block(BlockId::Number(i)).ok_or("Error exporting incomplete chain")?.into_inner();
let b = self.block(BlockId::Number(i))
.ok_or("Error exporting incomplete chain")?
.into_inner();
match format {
DataFormat::Binary => {
out.write(&b).map_err(|e| format!("Couldn't write to stream. Cause: {}", e))?;
out.write(&b)
.map_err(|e| {
format!("Couldn't write to stream. Cause: {}", e)
})?;
}
DataFormat::Hex => {
out.write_fmt(format_args!("{}\n", b.pretty())).map_err(|e| format!("Couldn't write to stream. Cause: {}", e))?;
out.write_fmt(format_args!("{}\n", b.pretty()))
.map_err(|e| {
format!("Couldn't write to stream. Cause: {}", e)
})?;
}
}
}
Expand All @@ -2624,7 +2637,10 @@ impl ImportExportBlocks for Client {
let format = match format {
Some(format) => format,
None => {
first_read = source.read(&mut first_bytes).map_err(|_| "Error reading from the file/stream.")?;
first_read = source.read(&mut first_bytes)
.map_err(|_| {
"Error reading from the file/stream."
})?;
match first_bytes[0] {
0xf9 => DataFormat::Binary,
_ => DataFormat::Hex,
Expand All @@ -2635,7 +2651,9 @@ impl ImportExportBlocks for Client {
let do_import = |bytes: Vec<u8>| {
let block = Unverified::from_rlp(bytes).map_err(|_| "Invalid block rlp")?;
let number = block.header.number();
while self.queue_info().is_full() { std::thread::sleep(Duration::from_secs(1)); }
while self.queue_info().is_full() {
std::thread::sleep(Duration::from_secs(1));
}
match self.import_block(block) {
Err(Error(EthcoreErrorKind::Import(ImportErrorKind::AlreadyInChain), _)) => {
trace!("Skipping block #{}: already in chain.", number);
Expand All @@ -2656,33 +2674,45 @@ impl ImportExportBlocks for Client {
} else {
let mut bytes = vec![0; READAHEAD_BYTES];
let n = source.read(&mut bytes)
.map_err(|err| format!("Error reading from the file/stream: {:?}", err))?;
.map_err(|err| {
format!("Error reading from the file/stream: {:?}", err)
})?;
(bytes, n)
};
if n == 0 { break; }
first_read = 0;
let s = PayloadInfo::from(&bytes)
.map_err(|e| format!("Invalid RLP in the file/stream: {:?}", e))?.total();
.map_err(|e| {
format!("Invalid RLP in the file/stream: {:?}", e)
})?.total();
bytes.resize(s, 0);
source.read_exact(&mut bytes[n..])
.map_err(|err| format!("Error reading from the file/stream: {:?}", err))?;
.map_err(|err| {
format!("Error reading from the file/stream: {:?}", err)
})?;
do_import(bytes)?;
}
}
DataFormat::Hex => {
for line in BufReader::new(source).lines() {
let s = line
.map_err(|err| format!("Error reading from the file/stream: {:?}", err))?;
.map_err(|err| {
format!("Error reading from the file/stream: {:?}", err)
})?;
let s = if first_read > 0 {
from_utf8(&first_bytes)
.map_err(|err| format!("Invalid UTF-8: {:?}", err))?
.map_err(|err| {
format!("Invalid UTF-8: {:?}", err)
})?
.to_owned() + &(s[..])
} else {
s
};
first_read = 0;
let bytes = s.from_hex()
.map_err(|err| format!("Invalid hex in file/stream: {:?}", err))?;
.map_err(|err| {
format!("Invalid hex in file/stream: {:?}", err)
})?;
do_import(bytes)?;
}
}
Expand Down
6 changes: 3 additions & 3 deletions ethcore/src/client/test_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use client::{
TransactionId, UncleId, TraceId, TraceFilter, LastHashes, CallAnalytics,
ProvingBlockChainClient, ScheduleInfo, ImportSealedBlock, BroadcastProposalBlock, ImportBlock, StateOrBlock,
Call, StateClient, EngineInfo, AccountData, BlockChain, BlockProducer, SealedBlockImporter, IoClient,
BadBlocks,
BadBlocks, traits::ForceUpdateSealing
};
use engines::EthEngine;
use error::{Error, EthcoreResult};
Expand Down Expand Up @@ -931,8 +931,8 @@ impl ProvingBlockChainClient for TestBlockChainClient {
}

impl super::traits::EngineClient for TestBlockChainClient {
fn update_sealing(&self) {
self.miner.update_sealing(self)
fn update_sealing(&self, force: ForceUpdateSealing) {
self.miner.update_sealing(self, force)
}

fn submit_seal(&self, block_hash: H256, seal: Vec<Bytes>) {
Expand Down
11 changes: 10 additions & 1 deletion ethcore/src/client/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,10 +423,19 @@ pub trait BroadcastProposalBlock {
/// Provides methods to import sealed block and broadcast a block proposal
pub trait SealedBlockImporter: ImportSealedBlock + BroadcastProposalBlock {}

/// Do we want to force update sealing?
#[derive(Debug, Copy, Clone, PartialEq)]
pub enum ForceUpdateSealing {
/// Ideally you want to use `No` at all times as `Yes` skips `reseal_required` checks.
Yes,
/// Don't skip `reseal_required` checks
No
}

/// Client facilities used by internally sealing Engines.
pub trait EngineClient: Sync + Send + ChainInfo {
/// Make a new block and seal it.
fn update_sealing(&self);
fn update_sealing(&self, force: ForceUpdateSealing);

/// Submit a seal for a block in the mining queue.
fn submit_seal(&self, block_hash: H256, seal: Vec<Bytes>);
Expand Down
6 changes: 3 additions & 3 deletions ethcore/src/engines/authority_round/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use std::sync::{Weak, Arc};
use std::time::{UNIX_EPOCH, Duration};

use block::*;
use client::EngineClient;
use client::{EngineClient, traits::ForceUpdateSealing};
use engines::{Engine, Seal, EngineError, ConstructedVerifier};
use engines::block_reward;
use engines::block_reward::{BlockRewardContract, RewardKind};
Expand Down Expand Up @@ -908,7 +908,7 @@ impl IoHandler<()> for TransitionHandler {
self.step.can_propose.store(true, AtomicOrdering::SeqCst);
if let Some(ref weak) = *self.client.read() {
if let Some(c) = weak.upgrade() {
c.update_sealing();
c.update_sealing(ForceUpdateSealing::No);
}
}
}
Expand Down Expand Up @@ -936,7 +936,7 @@ impl Engine<EthereumMachine> for AuthorityRound {
self.step.can_propose.store(true, AtomicOrdering::SeqCst);
if let Some(ref weak) = *self.client.read() {
if let Some(c) = weak.upgrade() {
c.update_sealing();
c.update_sealing(ForceUpdateSealing::No);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions ethcore/src/engines/clique/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ use std::time::{Instant, Duration, SystemTime, UNIX_EPOCH};

use block::ExecutedBlock;
use bytes::Bytes;
use client::{BlockId, EngineClient};
use client::{BlockId, EngineClient, traits::ForceUpdateSealing};
use engines::clique::util::{extract_signers, recover_creator};
use engines::{Engine, EngineError, Seal};
use error::{BlockError, Error};
Expand Down Expand Up @@ -749,7 +749,7 @@ impl Engine<EthereumMachine> for Clique {
if self.signer.read().is_some() {
if let Some(ref weak) = *self.client.read() {
if let Some(c) = weak.upgrade() {
c.update_sealing();
c.update_sealing(ForceUpdateSealing::No);
}
}
}
Expand Down
7 changes: 7 additions & 0 deletions ethcore/src/engines/instant_seal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@ impl<M: Machine> Engine<M> for InstantSeal<M> {

fn seals_internally(&self) -> Option<bool> { Some(true) }

fn should_reseal_on_update(&self) -> bool {
// We would like for the miner to `update_sealing` if there are local_pending_transactions
// in the pool to prevent transactions sent in parallel from stalling in the transaction
// pool. (see #9660)
true
}

fn generate_seal(&self, block: &ExecutedBlock, _parent: &Header) -> Seal {
if !block.transactions.is_empty() {
let block_number = block.header.number();
Expand Down
8 changes: 8 additions & 0 deletions ethcore/src/engines/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,14 @@ pub trait Engine<M: Machine>: Sync + Send {
/// Some(false) means that the node might seal internally but is not qualified now.
fn seals_internally(&self) -> Option<bool> { None }

/// Called in `miner.chain_new_blocks` if the engine wishes to `update_sealing`
/// after a block was recently sealed.
///
/// returns false by default
fn should_reseal_on_update(&self) -> bool {
false
}

/// Attempt to seal the block internally.
///
/// If `Some` is returned, then you get a valid seal.
Expand Down
12 changes: 6 additions & 6 deletions ethcore/src/engines/validator_set/multi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ mod tests {
use std::collections::BTreeMap;
use hash::keccak;
use accounts::AccountProvider;
use client::{BlockChainClient, ChainInfo, BlockInfo, ImportBlock};
use client::{BlockChainClient, ChainInfo, BlockInfo, ImportBlock, traits::ForceUpdateSealing};
use engines::EpochChange;
use engines::validator_set::ValidatorSet;
use ethkey::Secret;
Expand Down Expand Up @@ -181,24 +181,24 @@ mod tests {
let signer = Box::new((tap.clone(), v1, "".into()));
client.miner().set_author(miner::Author::Sealer(signer));
client.transact_contract(Default::default(), Default::default()).unwrap();
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
assert_eq!(client.chain_info().best_block_number, 0);
// Right signer for the first block.
let signer = Box::new((tap.clone(), v0, "".into()));
client.miner().set_author(miner::Author::Sealer(signer));
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
assert_eq!(client.chain_info().best_block_number, 1);
// This time v0 is wrong.
client.transact_contract(Default::default(), Default::default()).unwrap();
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
assert_eq!(client.chain_info().best_block_number, 1);
let signer = Box::new((tap.clone(), v1, "".into()));
client.miner().set_author(miner::Author::Sealer(signer));
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
assert_eq!(client.chain_info().best_block_number, 2);
// v1 is still good.
client.transact_contract(Default::default(), Default::default()).unwrap();
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
assert_eq!(client.chain_info().best_block_number, 3);

// Check syncing.
Expand Down
10 changes: 5 additions & 5 deletions ethcore/src/engines/validator_set/safe_contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ mod tests {
use spec::Spec;
use accounts::AccountProvider;
use types::transaction::{Transaction, Action};
use client::{ChainInfo, BlockInfo, ImportBlock};
use client::{ChainInfo, BlockInfo, ImportBlock, traits::{ForceUpdateSealing, EngineClient}};
use ethkey::Secret;
use miner::{self, MinerService};
use test_helpers::{generate_dummy_client_with_spec, generate_dummy_client_with_spec_and_data};
Expand Down Expand Up @@ -488,7 +488,7 @@ mod tests {
data: "bfc708a000000000000000000000000082a978b3f5962a5b0957d9ee9eef472ee55b42f1".from_hex().unwrap(),
}.sign(&s0, Some(chain_id));
client.miner().import_own_transaction(client.as_ref(), tx.into()).unwrap();
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
assert_eq!(client.chain_info().best_block_number, 1);
// Add "1" validator back in.
let tx = Transaction {
Expand All @@ -500,14 +500,14 @@ mod tests {
data: "4d238c8e00000000000000000000000082a978b3f5962a5b0957d9ee9eef472ee55b42f1".from_hex().unwrap(),
}.sign(&s0, Some(chain_id));
client.miner().import_own_transaction(client.as_ref(), tx.into()).unwrap();
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
// The transaction is not yet included so still unable to seal.
assert_eq!(client.chain_info().best_block_number, 1);

// Switch to the validator that is still there.
let signer = Box::new((tap.clone(), v0, "".into()));
client.miner().set_author(miner::Author::Sealer(signer));
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
assert_eq!(client.chain_info().best_block_number, 2);
// Switch back to the added validator, since the state is updated.
let signer = Box::new((tap.clone(), v1, "".into()));
Expand All @@ -521,7 +521,7 @@ mod tests {
data: Vec::new(),
}.sign(&s0, Some(chain_id));
client.miner().import_own_transaction(client.as_ref(), tx.into()).unwrap();
::client::EngineClient::update_sealing(&*client);
EngineClient::update_sealing(&*client, ForceUpdateSealing::No);
// Able to seal again.
assert_eq!(client.chain_info().best_block_number, 3);

Expand Down
Loading

0 comments on commit d43ae58

Please sign in to comment.