Skip to content

Commit d448428

Browse files
committed
Handle double voted proposals correctly in Tendermint
Previous code assumes that there is only one proposal in a view. Tendermint was always using the first proposal even though there are many proposals in a view. This commit makes Tendermint find proper proposal in the situation.
1 parent dff4ff4 commit d448428

File tree

1 file changed

+22
-20
lines changed

1 file changed

+22
-20
lines changed

core/src/consensus/tendermint/worker.rs

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,8 @@ impl Worker {
454454
self.validators.next_block_proposer(prev_block_hash, view)
455455
}
456456

457-
pub fn proposal_at(&self, height: Height, view: View) -> Option<(SchnorrSignature, usize, Bytes)> {
457+
/// If there are more than one proposals(DoubleVote), send the first.
458+
pub fn first_proposal_at(&self, height: Height, view: View) -> Option<(SchnorrSignature, usize, Bytes)> {
458459
let vote_step = VoteStep {
459460
height,
460461
view,
@@ -476,11 +477,9 @@ impl Worker {
476477
step: Step::Propose,
477478
});
478479

479-
if let Some(proposal) = all_votes.first() {
480-
proposal.on.block_hash.expect("Proposal message always include block hash") == block_hash
481-
} else {
482-
false
483-
}
480+
all_votes
481+
.into_iter()
482+
.any(|proposal| proposal.on.block_hash.expect("Proposal message always include block hash") == block_hash)
484483
}
485484

486485
pub fn vote_step(&self) -> VoteStep {
@@ -670,6 +669,7 @@ impl Worker {
670669
match state.to_step() {
671670
Step::Propose => {
672671
cinfo!(ENGINE, "move_to_step: Propose.");
672+
// If there are double voted proposals, use the first proposal.
673673
if let Some(hash) = self.votes.get_block_hashes(&vote_step).first() {
674674
if self.client().block(&BlockId::Hash(*hash)).is_some() {
675675
self.proposal = Proposal::new_imported(*hash);
@@ -683,9 +683,9 @@ impl Worker {
683683
} else {
684684
let parent_block_hash = self.prev_block_hash();
685685
if self.is_signer_proposer(&parent_block_hash) {
686-
if let TwoThirdsMajority::Lock(lock_view, _) = self.last_two_thirds_majority {
686+
if let TwoThirdsMajority::Lock(lock_view, locked_block_hash) = &self.last_two_thirds_majority {
687687
cinfo!(ENGINE, "I am a proposer, I'll re-propose a locked block");
688-
match self.locked_proposal_block(lock_view) {
688+
match self.locked_proposal_block(*lock_view, *locked_block_hash) {
689689
Ok(block) => self.repropose_block(block),
690690
Err(error_msg) => cwarn!(ENGINE, "{}", error_msg),
691691
}
@@ -796,14 +796,15 @@ impl Worker {
796796
}
797797
}
798798

799-
fn locked_proposal_block(&self, locked_view: View) -> Result<encoded::Block, String> {
799+
fn locked_proposal_block(&self, locked_view: View, locked_proposal_hash: H256) -> Result<encoded::Block, String> {
800800
let vote_step = VoteStep::new(self.height, locked_view, Step::Propose);
801-
let locked_proposal_hash = self.votes.get_block_hashes(&vote_step).first().cloned();
801+
let received_locked_block =
802+
self.votes.get_block_hashes(&vote_step).into_iter().any(|block_hash| block_hash == locked_proposal_hash);
802803

803-
let locked_proposal_hash = locked_proposal_hash.ok_or_else(|| {
804+
if !received_locked_block {
804805
self.request_proposal_to_any(self.height, locked_view);
805-
format!("Have a lock on {}-{}, but do not received a locked proposal", self.height, locked_view)
806-
})?;
806+
return Err(format!("Have a lock on {}-{}, but do not received a locked proposal", self.height, locked_view))
807+
}
807808

808809
let locked_proposal_block = self.client().block(&BlockId::Hash(locked_proposal_hash)).ok_or_else(|| {
809810
format!(
@@ -971,8 +972,9 @@ impl Worker {
971972

972973
let current_height = self.height;
973974
let vote_step = VoteStep::new(self.height, self.view, self.step.to_step());
974-
let proposal_at_current_view = self.votes.get_block_hashes(&vote_step).first().cloned();
975-
if proposal_at_current_view == Some(proposal.hash()) {
975+
let is_current_proposal =
976+
self.votes.get_block_hashes(&vote_step).into_iter().any(|block_hash| block_hash == proposal.hash());
977+
if is_current_proposal {
976978
self.proposal = Proposal::new_imported(proposal.hash());
977979
let current_step = self.step.clone();
978980
match current_step {
@@ -1005,16 +1007,16 @@ impl Worker {
10051007
self.move_to_height(height);
10061008
let prev_block_view = TendermintSealView::new(proposal.seal()).previous_block_view().unwrap();
10071009
self.save_last_confirmed_view(prev_block_view);
1008-
let proposal_at_view_0 = self
1010+
let is_view0_proposal = self
10091011
.votes
10101012
.get_block_hashes(&VoteStep {
10111013
height,
10121014
view: 0,
10131015
step: Step::Propose,
10141016
})
1015-
.first()
1016-
.cloned();
1017-
if proposal_at_view_0 == Some(proposal.hash()) {
1017+
.into_iter()
1018+
.any(|block_hash| block_hash == proposal.hash());
1019+
if is_view0_proposal {
10181020
self.proposal = Proposal::new_imported(proposal.hash())
10191021
}
10201022
self.move_to_step(TendermintState::Prevote, false);
@@ -1896,7 +1898,7 @@ impl Worker {
18961898
return
18971899
}
18981900

1899-
if let Some((signature, _signer_index, block)) = self.proposal_at(request_height, request_view) {
1901+
if let Some((signature, _signer_index, block)) = self.first_proposal_at(request_height, request_view) {
19001902
ctrace!(ENGINE, "Send proposal {}-{} to {:?}", request_height, request_view, token);
19011903
self.send_proposal_block(signature, request_view, block, result);
19021904
return

0 commit comments

Comments
 (0)