Skip to content

Commit 7d6cb20

Browse files
committed
Add VoteRegressionChecker
1 parent 88c667c commit 7d6cb20

File tree

3 files changed

+198
-1
lines changed

3 files changed

+198
-1
lines changed

core/src/consensus/tendermint/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ mod network;
2222
mod params;
2323
pub mod types;
2424
pub mod vote_collector;
25+
mod vote_regression_checker;
2526
mod worker;
2627

2728
use std::sync::atomic::AtomicBool;
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
use consensus::{Step, VoteOn};
2+
use std::cmp::Ordering;
3+
4+
pub struct VoteRegressionChecker {
5+
last_vote: Option<VoteOn>,
6+
}
7+
8+
impl VoteRegressionChecker {
9+
pub fn new() -> VoteRegressionChecker {
10+
VoteRegressionChecker {
11+
last_vote: None,
12+
}
13+
}
14+
15+
pub fn check(&mut self, vote_on: &VoteOn) -> bool {
16+
assert!(match vote_on.step.step {
17+
Step::Propose | Step::Prevote | Step::Precommit => true,
18+
_ => false,
19+
});
20+
21+
let monotonic = if let Some(last_vote) = &self.last_vote {
22+
match last_vote.step.cmp(&vote_on.step) {
23+
Ordering::Less => true,
24+
Ordering::Greater => false,
25+
Ordering::Equal => last_vote.block_hash == vote_on.block_hash,
26+
}
27+
} else {
28+
true
29+
};
30+
31+
if monotonic {
32+
self.last_vote = Some(vote_on.clone());
33+
}
34+
monotonic
35+
}
36+
}
37+
38+
#[cfg(test)]
39+
mod tests {
40+
use super::*;
41+
use consensus::VoteStep;
42+
use primitives::H256;
43+
44+
#[test]
45+
fn test_initial_set() {
46+
let mut checker = VoteRegressionChecker::new();
47+
48+
let random_step = VoteStep::new(100, 10, Step::Prevote);
49+
let random_hash = Some(H256::random());
50+
assert!(checker.check(&VoteOn {
51+
step: random_step,
52+
block_hash: random_hash
53+
}))
54+
}
55+
56+
#[test]
57+
#[should_panic]
58+
fn test_disallow_commit() {
59+
let mut checker = VoteRegressionChecker::new();
60+
61+
let random_commit_step = VoteStep::new(100, 10, Step::Commit);
62+
let random_hash = Some(H256::random());
63+
assert!(checker.check(&VoteOn {
64+
step: random_commit_step,
65+
block_hash: random_hash
66+
}))
67+
}
68+
69+
#[test]
70+
fn test_allow_height_increase() {
71+
let mut checker = VoteRegressionChecker::new();
72+
73+
checker.check(&VoteOn {
74+
step: VoteStep::new(100, 10, Step::Prevote),
75+
block_hash: Some(H256::random()),
76+
});
77+
78+
assert!(checker.check(&VoteOn {
79+
step: VoteStep::new(101, 10, Step::Prevote),
80+
block_hash: Some(H256::random())
81+
}))
82+
}
83+
84+
#[test]
85+
fn test_disallow_height_decrease() {
86+
let mut checker = VoteRegressionChecker::new();
87+
88+
checker.check(&VoteOn {
89+
step: VoteStep::new(100, 10, Step::Prevote),
90+
block_hash: Some(H256::random()),
91+
});
92+
93+
assert!(!checker.check(&VoteOn {
94+
step: VoteStep::new(99, 10, Step::Prevote),
95+
block_hash: Some(H256::random())
96+
}))
97+
}
98+
99+
#[test]
100+
fn test_allow_view_increase() {
101+
let mut checker = VoteRegressionChecker::new();
102+
103+
checker.check(&VoteOn {
104+
step: VoteStep::new(100, 10, Step::Prevote),
105+
block_hash: Some(H256::random()),
106+
});
107+
108+
assert!(checker.check(&VoteOn {
109+
step: VoteStep::new(100, 11, Step::Prevote),
110+
block_hash: Some(H256::random())
111+
}))
112+
}
113+
114+
#[test]
115+
fn test_disallow_view_decrease() {
116+
let mut checker = VoteRegressionChecker::new();
117+
118+
checker.check(&VoteOn {
119+
step: VoteStep::new(100, 10, Step::Prevote),
120+
block_hash: Some(H256::random()),
121+
});
122+
123+
assert!(!checker.check(&VoteOn {
124+
step: VoteStep::new(100, 9, Step::Prevote),
125+
block_hash: Some(H256::random())
126+
}))
127+
}
128+
129+
#[test]
130+
fn test_allow_step_increased() {
131+
let mut checker = VoteRegressionChecker::new();
132+
133+
checker.check(&VoteOn {
134+
step: VoteStep::new(100, 10, Step::Prevote),
135+
block_hash: Some(H256::random()),
136+
});
137+
138+
assert!(checker.check(&VoteOn {
139+
step: VoteStep::new(100, 10, Step::Precommit),
140+
block_hash: Some(H256::random())
141+
}))
142+
}
143+
144+
#[test]
145+
fn test_disallow_step_decreased() {
146+
let mut checker = VoteRegressionChecker::new();
147+
148+
checker.check(&VoteOn {
149+
step: VoteStep::new(100, 10, Step::Prevote),
150+
block_hash: Some(H256::random()),
151+
});
152+
153+
assert!(!checker.check(&VoteOn {
154+
step: VoteStep::new(100, 10, Step::Propose),
155+
block_hash: Some(H256::random())
156+
}))
157+
}
158+
159+
#[test]
160+
fn test_allow_same_hash() {
161+
let mut checker = VoteRegressionChecker::new();
162+
163+
let block_hash = Some(H256::random());
164+
checker.check(&VoteOn {
165+
step: VoteStep::new(100, 10, Step::Prevote),
166+
block_hash,
167+
});
168+
169+
assert!(checker.check(&VoteOn {
170+
step: VoteStep::new(100, 10, Step::Prevote),
171+
block_hash,
172+
}))
173+
}
174+
175+
#[test]
176+
fn test_disallow_hash_change() {
177+
let mut checker = VoteRegressionChecker::new();
178+
179+
checker.check(&VoteOn {
180+
step: VoteStep::new(100, 10, Step::Prevote),
181+
block_hash: Some(H256::random()),
182+
});
183+
184+
assert!(!checker.check(&VoteOn {
185+
step: VoteStep::new(100, 10, Step::Prevote),
186+
block_hash: Some(H256::random()),
187+
}))
188+
}
189+
}

core/src/consensus/tendermint/worker.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ use super::params::TimeGapParams;
3737
use super::stake::CUSTOM_ACTION_HANDLER_ID;
3838
use super::types::{Height, Proposal, Step, TendermintSealView, TendermintState, TwoThirdsMajority, View};
3939
use super::vote_collector::{DoubleVote, VoteCollector};
40+
use super::vote_regression_checker::VoteRegressionChecker;
4041
use super::{
4142
BlockHash, ENGINE_TIMEOUT_BROADCAST_STEP_STATE, ENGINE_TIMEOUT_EMPTY_PROPOSAL, ENGINE_TIMEOUT_TOKEN_NONCE_BASE,
4243
SEAL_FIELDS,
@@ -93,6 +94,7 @@ struct Worker {
9394
extension: EventSender<network::Event>,
9495
time_gap_params: TimeGapParams,
9596
timeout_token_nonce: usize,
97+
vote_regression_checker: VoteRegressionChecker,
9698
}
9799

98100
pub enum Event {
@@ -196,6 +198,7 @@ impl Worker {
196198
votes_received_changed: false,
197199
time_gap_params,
198200
timeout_token_nonce: ENGINE_TIMEOUT_TOKEN_NONCE_BASE,
201+
vote_regression_checker: VoteRegressionChecker::new(),
199202
}
200203
}
201204

@@ -1087,12 +1090,12 @@ impl Worker {
10871090
}
10881091

10891092
let header = sealed_block.header();
1090-
let parent_hash = header.parent_hash();
10911093

10921094
if let TendermintState::ProposeWaitBlockGeneration {
10931095
parent_hash: expected_parent_hash,
10941096
} = self.step
10951097
{
1098+
let parent_hash = header.parent_hash();
10961099
assert_eq!(
10971100
*parent_hash, expected_parent_hash,
10981101
"Generated hash({:?}) is different from expected({:?})",
@@ -1498,6 +1501,8 @@ impl Worker {
14981501
step: VoteStep::new(self.height, self.view, self.step.to_step()),
14991502
block_hash,
15001503
};
1504+
assert!(self.vote_regression_checker.check(&on), "Vote should not regress");
1505+
15011506
let signature = self.signer.sign(on.hash())?;
15021507

15031508
let vote = ConsensusMessage {
@@ -1523,6 +1528,8 @@ impl Worker {
15231528
step: VoteStep::new(self.height, self.view, Step::Propose),
15241529
block_hash: Some(*header.parent_hash()),
15251530
};
1531+
assert!(self.vote_regression_checker.check(&on), "Vote should not regress");
1532+
15261533
let signature = self.signer.sign(on.hash())?;
15271534

15281535
let vote = ConsensusMessage {

0 commit comments

Comments
 (0)