Skip to content

Commit 011ca48

Browse files
majectymajecty
committed
Move to Commit state if enough precommits are collected
1 parent 1532d5c commit 011ca48

File tree

2 files changed

+199
-41
lines changed

2 files changed

+199
-41
lines changed

‎core/src/consensus/tendermint/types.rs‎

Lines changed: 82 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ use rlp::{Decodable, DecoderError, Encodable, RlpStream, UntrustedRlp};
2323
use super::super::BitSet;
2424
use super::message::VoteStep;
2525
use crate::block::{IsBlock, SealedBlock};
26+
use consensus::tendermint::BlockHash;
2627

2728
pub type Height = u64;
2829
pub type View = u64;
@@ -41,8 +42,14 @@ pub enum TendermintState {
4142
},
4243
Prevote,
4344
Precommit,
44-
Commit,
45-
CommitTimedout,
45+
Commit {
46+
view: View,
47+
block_hash: H256,
48+
},
49+
CommitTimedout {
50+
view: View,
51+
block_hash: H256,
52+
},
4653
}
4754

4855
impl TendermintState {
@@ -60,25 +67,85 @@ impl TendermintState {
6067
} => Step::Propose,
6168
TendermintState::Prevote => Step::Prevote,
6269
TendermintState::Precommit => Step::Precommit,
63-
TendermintState::Commit => Step::Commit,
64-
TendermintState::CommitTimedout => Step::Commit,
70+
TendermintState::Commit {
71+
..
72+
} => Step::Commit,
73+
TendermintState::CommitTimedout {
74+
..
75+
} => Step::Commit,
6576
}
6677
}
6778

6879
pub fn is_commit(&self) -> bool {
6980
match self {
70-
TendermintState::Commit => true,
71-
TendermintState::CommitTimedout => true,
81+
TendermintState::Commit {
82+
..
83+
} => true,
84+
TendermintState::CommitTimedout {
85+
..
86+
} => true,
7287
_ => false,
7388
}
7489
}
7590

7691
pub fn is_commit_timedout(&self) -> bool {
7792
match self {
78-
TendermintState::CommitTimedout => true,
93+
TendermintState::CommitTimedout {
94+
..
95+
} => true,
7996
_ => false,
8097
}
8198
}
99+
100+
pub fn committed_block_hash(&self) -> Option<BlockHash> {
101+
match self {
102+
TendermintState::Commit {
103+
block_hash,
104+
..
105+
} => Some(*block_hash),
106+
TendermintState::CommitTimedout {
107+
block_hash,
108+
..
109+
} => Some(*block_hash),
110+
TendermintState::Propose => None,
111+
TendermintState::ProposeWaitBlockGeneration {
112+
..
113+
} => None,
114+
TendermintState::ProposeWaitImported {
115+
..
116+
} => None,
117+
TendermintState::ProposeWaitEmptyBlockTimer {
118+
..
119+
} => None,
120+
TendermintState::Prevote => None,
121+
TendermintState::Precommit => None,
122+
}
123+
}
124+
125+
pub fn committed_view(&self) -> Option<View> {
126+
match self {
127+
TendermintState::Commit {
128+
view,
129+
..
130+
} => Some(*view),
131+
TendermintState::CommitTimedout {
132+
view,
133+
..
134+
} => Some(*view),
135+
TendermintState::Propose => None,
136+
TendermintState::ProposeWaitBlockGeneration {
137+
..
138+
} => None,
139+
TendermintState::ProposeWaitImported {
140+
..
141+
} => None,
142+
TendermintState::ProposeWaitEmptyBlockTimer {
143+
..
144+
} => None,
145+
TendermintState::Prevote => None,
146+
TendermintState::Precommit => None,
147+
}
148+
}
82149
}
83150

84151
impl fmt::Debug for TendermintState {
@@ -96,8 +163,14 @@ impl fmt::Debug for TendermintState {
96163
} => write!(f, "TendermintState::ProposeWaitEmptyBlockTimer({})", block.header().hash()),
97164
TendermintState::Prevote => write!(f, "TendermintState::Prevote"),
98165
TendermintState::Precommit => write!(f, "TendermintState::Precommit"),
99-
TendermintState::Commit => write!(f, "TendermintState::Commit"),
100-
TendermintState::CommitTimedout => write!(f, "TendermintState::CommitTimedout"),
166+
TendermintState::Commit {
167+
block_hash,
168+
view,
169+
} => write!(f, "TendermintState::Commit({}, {})", block_hash, view),
170+
TendermintState::CommitTimedout {
171+
block_hash,
172+
view,
173+
} => write!(f, "TendermintState::CommitTimedout({}, {})", block_hash, view),
101174
}
102175
}
103176
}

‎core/src/consensus/tendermint/worker.rs‎

Lines changed: 117 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,10 @@ impl Worker {
405405

406406
/// Check Tendermint can move from the commit step to the propose step
407407
fn can_move_from_commit_to_propose(&self) -> bool {
408+
if !self.step.is_commit() {
409+
return false
410+
}
411+
408412
let vote_step = VoteStep::new(self.height, self.last_confirmed_view, Step::Precommit);
409413
if self.step.is_commit_timedout() && self.check_current_block_exists() {
410414
cinfo!(ENGINE, "Transition to Propose because best block is changed after commit timeout");
@@ -442,6 +446,20 @@ impl Worker {
442446
Some((proposal.signature, proposal.signer_index, bytes))
443447
}
444448

449+
fn is_proposal_received(&self, height: Height, view: View, block_hash: BlockHash) -> bool {
450+
let all_votes = self.votes.get_all_votes_in_round(&VoteStep {
451+
height,
452+
view,
453+
step: Step::Propose,
454+
});
455+
456+
if let Some(proposal) = all_votes.first() {
457+
proposal.on.block_hash.expect("Proposal message always include block hash") == block_hash
458+
} else {
459+
false
460+
}
461+
}
462+
445463
pub fn vote_step(&self) -> VoteStep {
446464
VoteStep {
447465
height: self.height,
@@ -592,6 +610,7 @@ impl Worker {
592610
self.votes_received = BitSet::new();
593611
}
594612

613+
#[allow(clippy::cognitive_complexity)]
595614
fn move_to_step(&mut self, state: TendermintState, is_restoring: bool) {
596615
ctrace!(ENGINE, "Transition to {:?} triggered.", state);
597616
let prev_step = mem::replace(&mut self.step, state.clone());
@@ -707,6 +726,47 @@ impl Worker {
707726
}
708727
Step::Commit => {
709728
cinfo!(ENGINE, "move_to_step: Commit.");
729+
let view = state.committed_view().expect("commit always has committed_view");
730+
let block_hash = state.committed_block_hash().expect("commit always has committed_block_hash");
731+
self.save_last_confirmed_view(view);
732+
733+
let proposal_received = self.is_proposal_received(self.height, view, block_hash);
734+
let proposal_imported = self.client().block(&block_hash.into()).is_some();
735+
let best_block_header = self.client().best_block_header();
736+
if best_block_header.number() > self.height {
737+
cwarn!(
738+
ENGINE,
739+
"Best block's number {} is larger than tendermint height {} in Commit state",
740+
best_block_header.number(),
741+
self.height
742+
);
743+
return
744+
} else if best_block_header.number() == self.height {
745+
cwarn!(
746+
ENGINE,
747+
"Best block's number {} is the same as the tendermint height {} in Commit state",
748+
best_block_header.number(),
749+
self.height
750+
);
751+
return
752+
}
753+
let best_block_changed = best_block_header.hash() == block_hash;
754+
755+
match (proposal_received, proposal_imported, best_block_changed) {
756+
(false, false, _) => {
757+
cdebug!(ENGINE, "committed but proposal({}) is not received wait the proposal", block_hash);
758+
}
759+
(true, false, _) => {
760+
cdebug!(ENGINE, "committed but proposal({}) is not imported wait the proposal", block_hash);
761+
}
762+
(_, true, false) => {
763+
cdebug!(ENGINE, "committed but proposal({}) is imported update the best block", block_hash);
764+
self.client().update_best_as_committed(block_hash);
765+
}
766+
(_, true, true) => {
767+
cdebug!(ENGINE, "committed proposal({}) is imported and best is changed", block_hash);
768+
}
769+
}
710770
}
711771
}
712772
}
@@ -818,28 +878,39 @@ impl Worker {
818878
self.last_two_thirds_majority =
819879
TwoThirdsMajority::from_message(message.on.step.view, message.on.block_hash);
820880
}
881+
882+
if vote_step.step == Step::Precommit
883+
&& self.height == vote_step.height
884+
&& message.on.block_hash.is_some()
885+
&& has_enough_aligned_votes
886+
{
887+
if self.can_move_from_commit_to_propose() {
888+
let height = self.height;
889+
self.move_to_height(height + 1);
890+
self.move_to_step(TendermintState::Propose, is_restoring);
891+
return
892+
}
893+
894+
let block_hash = message.on.block_hash.expect("Upper if already checked block hash");
895+
if !self.step.is_commit() {
896+
self.move_to_step(
897+
TendermintState::Commit {
898+
block_hash,
899+
view: vote_step.view,
900+
},
901+
is_restoring,
902+
);
903+
return
904+
}
905+
}
906+
821907
// Check if it can affect the step transition.
822908
if self.is_step(message) {
823909
let next_step = match self.step {
824910
TendermintState::Precommit if message.on.block_hash.is_none() && has_enough_aligned_votes => {
825911
self.increment_view(1);
826912
Some(TendermintState::Propose)
827913
}
828-
TendermintState::Precommit if has_enough_aligned_votes => {
829-
let bh = message.on.block_hash.expect("previous guard ensures is_some; qed");
830-
if self.client().block(&BlockId::Hash(bh)).is_some() {
831-
// Commit the block, and update the last confirmed view
832-
self.save_last_confirmed_view(message.on.step.view);
833-
834-
// Update the best block hash as the hash of the committed block
835-
self.client().update_best_as_committed(bh);
836-
Some(TendermintState::Commit)
837-
} else {
838-
cwarn!(ENGINE, "Cannot find a proposal which committed");
839-
self.increment_view(1);
840-
Some(TendermintState::Propose)
841-
}
842-
}
843914
// Avoid counting votes twice.
844915
TendermintState::Prevote if lock_change => Some(TendermintState::Precommit),
845916
TendermintState::Prevote if has_enough_aligned_votes => Some(TendermintState::Precommit),
@@ -850,14 +921,6 @@ impl Worker {
850921
self.move_to_step(step, is_restoring);
851922
return
852923
}
853-
} else if vote_step.step == Step::Precommit
854-
&& self.height == vote_step.height
855-
&& self.can_move_from_commit_to_propose()
856-
{
857-
let height = self.height;
858-
self.move_to_height(height + 1);
859-
self.move_to_step(TendermintState::Propose, is_restoring);
860-
return
861924
}
862925

863926
// self.move_to_step() calls self.broadcast_state()
@@ -1227,18 +1290,27 @@ impl Worker {
12271290
cinfo!(ENGINE, "Precommit timeout without enough votes.");
12281291
TendermintState::Precommit
12291292
}
1230-
TendermintState::Commit => {
1293+
TendermintState::Commit {
1294+
block_hash,
1295+
view,
1296+
} => {
12311297
cinfo!(ENGINE, "Commit timeout.");
1232-
if !self.check_current_block_exists() {
1298+
if self.client().block(&block_hash.into()).is_none() {
12331299
cwarn!(ENGINE, "Best chain is not updated yet, wait until imported");
1234-
self.step = TendermintState::CommitTimedout;
1300+
self.step = TendermintState::CommitTimedout {
1301+
block_hash,
1302+
view,
1303+
};
12351304
return
12361305
}
1306+
12371307
let height = self.height;
12381308
self.move_to_height(height + 1);
12391309
TendermintState::Propose
12401310
}
1241-
TendermintState::CommitTimedout => unreachable!(),
1311+
TendermintState::CommitTimedout {
1312+
..
1313+
} => unreachable!(),
12421314
};
12431315

12441316
self.move_to_step(next_step, false);
@@ -1439,6 +1511,24 @@ impl Worker {
14391511
}
14401512
};
14411513

1514+
if self.step.is_commit() && (imported.len() + enacted.len() == 1) {
1515+
let committed_block_hash = self.step.committed_block_hash().expect("Commit state always has block_hash");
1516+
if imported.first() == Some(&committed_block_hash) {
1517+
cdebug!(ENGINE, "Committed block {} is committed_block_hash", committed_block_hash);
1518+
self.client().update_best_as_committed(committed_block_hash);
1519+
return
1520+
}
1521+
if enacted.first() == Some(&committed_block_hash) {
1522+
cdebug!(ENGINE, "Committed block {} is now the best block", committed_block_hash);
1523+
if self.can_move_from_commit_to_propose() {
1524+
let new_height = self.height + 1;
1525+
self.move_to_height(new_height);
1526+
self.move_to_step(TendermintState::Propose, false);
1527+
return
1528+
}
1529+
}
1530+
}
1531+
14421532
if !imported.is_empty() {
14431533
let mut height_changed = false;
14441534
for hash in imported {
@@ -1463,11 +1553,6 @@ impl Worker {
14631553
return
14641554
}
14651555
}
1466-
if !enacted.is_empty() && self.can_move_from_commit_to_propose() {
1467-
let new_height = self.height + 1;
1468-
self.move_to_height(new_height);
1469-
self.move_to_step(TendermintState::Propose, false)
1470-
}
14711556
}
14721557

14731558
fn send_proposal_block(

0 commit comments

Comments
 (0)