Skip to content

Stop generating blocks if the consensus cannot make a seal #1792

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions core/src/miner/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ impl Miner {
&self,
parent_block_id: BlockId,
chain: &C,
) -> Result<(ClosedBlock, Option<H256>), Error> {
) -> Result<Option<(ClosedBlock, Option<H256>)>, Error> {
let (transactions, mut open_block, original_work_hash, block_number) = {
let sealing_work = self.sealing_work.lock();

Expand Down Expand Up @@ -491,7 +491,7 @@ impl Miner {
if let Some(seal_bytes) = seal.seal_fields() {
open_block.seal(self.engine.borrow(), seal_bytes).expect("Sealing always success");
} else {
panic!("Seal should not be none")
return Ok(None)
}
}

Expand Down Expand Up @@ -576,7 +576,7 @@ impl Miner {
chain.chain_info().best_block_timestamp,
);
}
Ok((block, original_work_hash))
Ok(Some((block, original_work_hash)))
}

/// Attempts to perform internal sealing (one that does not require work) and handles the result depending on the type of Seal.
Expand Down Expand Up @@ -798,9 +798,12 @@ impl MinerService for Miner {
// | Make sure to release the locks before calling that method. |
// --------------------------------------------------------------------------
match self.prepare_block(BlockId::Latest, client) {
Ok((block, original_work_hash)) => {
Ok(Some((block, original_work_hash))) => {
self.prepare_work(block, original_work_hash);
}
Ok(None) => {
ctrace!(MINER, "prepare_work_sealing: cannot prepare block");
}
Err(err) => {
ctrace!(MINER, "prepare_work_sealing: cannot prepare block: {:?}", err);
}
Expand Down Expand Up @@ -837,13 +840,17 @@ impl MinerService for Miner {
let parent_block_number = chain.block_header(&parent_block).expect("Parent is always exist").number();
if self.requires_reseal(parent_block_number) {
let (block, original_work_hash) = match self.prepare_block(parent_block, chain) {
Ok((block, original_work_hash)) => {
Ok(Some((block, original_work_hash))) => {
Copy link
Contributor

@foriequal0 foriequal0 Sep 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, block and original_work_hash are duplicated 3 times. It can be reduced with

let (block, original_work_hash) = match ... {
  Ok(Some(prepared @ (block, _))) => { 
    ...
    prepared
  }
  ...
};

if !allow_empty_block && block.block().transactions().is_empty() {
ctrace!(MINER, "update_sealing: block is empty, and allow_empty_block is false");
return
}
(block, original_work_hash)
}
Ok(None) => {
ctrace!(MINER, "update_sealing: cannot prepare block");
return
}
Err(err) => {
ctrace!(MINER, "update_sealing: cannot prepare block: {:?}", err);
return
Expand Down