Skip to content

Do not panic when the height, view or step changes while creating a block #1795

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 2 commits into from
Sep 26, 2019
Merged
Show file tree
Hide file tree
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
5 changes: 0 additions & 5 deletions core/src/client/importer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use super::{BlockChainTrait, Client, ClientConfig};
use crate::block::{enact, IsBlock, LockedBlock};
use crate::blockchain::{BodyProvider, HeaderProvider, ImportRoute};
use crate::consensus::CodeChainEngine;
use crate::encoded;
use crate::error::Error;
use crate::miner::{Miner, MinerService};
use crate::service::ClientIoMessage;
Expand Down Expand Up @@ -117,10 +116,6 @@ impl Importer {
continue
}
if let Ok(closed_block) = self.check_and_close_block(&block, client) {
if self.engine.is_proposal(&block.header) {
self.engine.on_verified_proposal(encoded::Block::new(block.bytes.clone()))
}

imported_blocks.push(header.hash());
let route = self.commit_block(&closed_block, &header, &block.bytes, client);
import_results.push(route);
Expand Down
5 changes: 0 additions & 5 deletions core/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ use crate::account_provider::AccountProvider;
use crate::block::{ExecutedBlock, SealedBlock};
use crate::client::ConsensusClient;
use crate::codechain_machine::CodeChainMachine;
use crate::encoded;
use crate::error::Error;
use crate::transaction::UnverifiedTransaction;
use crate::views::HeaderView;
Expand Down Expand Up @@ -243,10 +242,6 @@ pub trait ConsensusEngine: Sync + Send {
false
}

/// Called when proposal block is verified.
/// Consensus many hold the verified proposal block until it should be imported.
fn on_verified_proposal(&self, _verified_block_data: encoded::Block) {}

/// Register an account which signs consensus messages.
fn set_signer(&self, _ap: Arc<AccountProvider>, _address: Address) {}

Expand Down
25 changes: 24 additions & 1 deletion core/src/consensus/tendermint/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,24 @@ impl Worker {
}

fn proposal_generated(&mut self, sealed_block: &SealedBlock) {
let proposal_height = sealed_block.header().number();
let proposal_seal = sealed_block.header().seal();
let proposal_view = TendermintSealView::new(proposal_seal)
.consensus_view()
.expect("Generated proposal should have a valid seal");
assert!(proposal_height <= self.height, "A proposal cannot be generated on the future height");
if proposal_height < self.height || (proposal_height == self.height && proposal_view != self.view) {
ctrace!(
ENGINE,
"Proposal is generated on the height {} and view {}. Current height is {} and view is {}",
proposal_height,
proposal_view,
self.height,
self.view,
);
return
}

let header = sealed_block.header();
let hash = header.hash();
let parent_hash = header.parent_hash();
Expand All @@ -1116,7 +1134,12 @@ impl Worker {
parent_hash, expected_parent_hash
);
} else {
panic!("Block is generated at unexpected step {:?}", self.step);
ctrace!(
ENGINE,
"Proposal is generated after step is changed. Expected step is ProposeWaitBlockGeneration but current step is {:?}",
self.step,
);
return
}
let prev_proposer_idx = self.block_proposer_idx(*parent_hash).expect("Prev block must exists");

Expand Down