Skip to content

Commit

Permalink
[consensus] add one missing checks on 2-chain timeout
Browse files Browse the repository at this point in the history
Closes: diem#10069
  • Loading branch information
Zekun Li authored and bors-libra committed Dec 22, 2021
1 parent 556025a commit 4c2f5fb
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 11 deletions.
17 changes: 11 additions & 6 deletions consensus/consensus-types/src/timeout_2chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,15 @@ impl TwoChainTimeout {
hqc_round: self.hqc_round(),
}
}

pub fn verify(&self, validators: &ValidatorVerifier) -> anyhow::Result<()> {
ensure!(
self.hqc_round() < self.round(),
"Timeout round should be larger than the QC round"
);
self.quorum_cert.verify(validators)?;
Ok(())
}
}

impl Display for TwoChainTimeout {
Expand Down Expand Up @@ -121,13 +130,9 @@ impl TwoChainTimeoutCertificate {
/// 2. all signatures are properly formed (timeout.epoch, timeout.round, round)
/// 3. timeout.hqc_round == max(signed round)
pub fn verify(&self, validators: &ValidatorVerifier) -> anyhow::Result<()> {
// Verify the highest quorum cert validity.
// Verify the highest timeout validity.
self.timeout.verify(validators)?;
let hqc_round = self.timeout.hqc_round();
ensure!(
hqc_round < self.timeout.round(),
"Timeout round should be larger than the QC round"
);
self.timeout.quorum_cert().verify(validators)?;
let mut signed_round = 0;
validators.check_voting_power(self.signatures.keys())?;
for (author, (qc_round, signature)) in &self.signatures {
Expand Down
5 changes: 1 addition & 4 deletions consensus/consensus-types/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,10 +193,7 @@ impl Vote {
== (self.epoch(), self.vote_data.proposed().round()),
"2-chain timeout has different (epoch, round) than Vote"
);
timeout
.quorum_cert()
.verify(validator)
.context("Failed to verify QC from 2-chain timeout")?;
timeout.verify(validator)?;
validator
.verify(self.author(), &timeout.signing_format(), signature)
.context("Failed to verify 2-chain timeout signature")?;
Expand Down
4 changes: 4 additions & 0 deletions consensus/consensus-types/src/vote_msg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ impl VoteMsg {
self.vote().epoch() == self.sync_info.epoch(),
"VoteMsg has different epoch"
);
ensure!(
self.vote().vote_data().proposed().round() > self.sync_info.highest_round(),
"Vote Round should be higher than SyncInfo"
);
if let Some((timeout, _)) = self.vote().two_chain_timeout() {
ensure!(
timeout.hqc_round() <= self.sync_info.highest_certified_round(),
Expand Down
2 changes: 2 additions & 0 deletions consensus/safety-rules/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ pub enum Error {
InvalidOrderedLedgerInfo(String),
#[error("Waypoint out of date: Previous waypoint version {0}, updated version {1}, current epoch {2}, provided epoch {3}")]
WaypointOutOfDate(u64, u64, u64, u64),
#[error("Invalid Timeout: {0}")]
InvalidTimeout(String),
}

impl From<serde_json::Error> for Error {
Expand Down
4 changes: 3 additions & 1 deletion consensus/safety-rules/src/safety_rules_2chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ impl SafetyRules {
self.signer()?;
let mut safety_data = self.persistent_storage.safety_data()?;
self.verify_epoch(timeout.epoch(), &safety_data)?;
self.verify_qc(timeout.quorum_cert())?;
timeout
.verify(&self.epoch_state()?.verifier)
.map_err(|e| Error::InvalidTimeout(e.to_string()))?;
if let Some(tc) = timeout_cert {
self.verify_tc(tc)?;
}
Expand Down
9 changes: 9 additions & 0 deletions consensus/safety-rules/src/tests/suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,15 @@ fn test_2chain_timeout(constructor: &Callback) {
.unwrap_err(),
Error::NotSafeToTimeout(4, 1, 3, 2)
);
assert!(matches!(
safety_rules
.sign_timeout_with_qc(
&TwoChainTimeout::new(1, 1, a3.vote_proposal.block().quorum_cert().clone(),),
Some(make_timeout_cert(2, &genesis_qc, &signer)).as_ref()
)
.unwrap_err(),
Error::InvalidTimeout(_)
));
}

/// Test that we can succesfully sign a valid commit vote
Expand Down

0 comments on commit 4c2f5fb

Please sign in to comment.