Skip to content

Commit

Permalink
Optimistic signature verification for votes and order votes (#14642)
Browse files Browse the repository at this point in the history
  • Loading branch information
vusirikala authored Oct 7, 2024
1 parent ea47ce5 commit 2deaf9f
Show file tree
Hide file tree
Showing 13 changed files with 910 additions and 371 deletions.
4 changes: 3 additions & 1 deletion config/src/config/consensus_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ pub struct ConsensusConfig {
// must match one of the CHAIN_HEALTH_WINDOW_SIZES values.
pub window_for_chain_health: usize,
pub chain_health_backoff: Vec<ChainHealthBackoffValues>,
// Deprecated
pub qc_aggregator_type: QcAggregatorType,
// Max blocks allowed for block retrieval requests
pub max_blocks_per_sending_request: u64,
Expand All @@ -90,6 +91,7 @@ pub struct ConsensusConfig {
pub num_bounded_executor_tasks: u64,
pub enable_pre_commit: bool,
pub max_pending_rounds_in_commit_vote_cache: u64,
pub optimistic_sig_verification: bool,
pub enable_round_timeout_msg: bool,
}

Expand Down Expand Up @@ -301,7 +303,6 @@ impl Default for ConsensusConfig {
backoff_proposal_delay_ms: 300,
},
],

qc_aggregator_type: QcAggregatorType::default(),
// This needs to fit into the network message size, so with quorum store it can be much bigger
max_blocks_per_sending_request: 10,
Expand All @@ -320,6 +321,7 @@ impl Default for ConsensusConfig {
num_bounded_executor_tasks: 16,
enable_pre_commit: true,
max_pending_rounds_in_commit_vote_cache: 100,
optimistic_sig_verification: false,
enable_round_timeout_msg: false,
}
}
Expand Down
29 changes: 24 additions & 5 deletions consensus/consensus-types/src/order_vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ use crate::common::Author;
use anyhow::{ensure, Context};
use aptos_crypto::{bls12381, HashValue};
use aptos_short_hex_str::AsShortHexStr;
use aptos_types::{ledger_info::LedgerInfo, validator_verifier::ValidatorVerifier};
use aptos_types::{
ledger_info::{LedgerInfo, SignatureWithStatus},
validator_verifier::ValidatorVerifier,
};
use serde::{Deserialize, Serialize};
use std::fmt::{Debug, Display, Formatter};

Expand All @@ -16,8 +19,8 @@ pub struct OrderVote {
author: Author,
/// LedgerInfo of a block that is going to be ordered in case this vote gathers QC.
ledger_info: LedgerInfo,
/// Signature of the LedgerInfo.
signature: bls12381::Signature,
/// Signature on the LedgerInfo along with a status on whether the signature is verified.
signature: SignatureWithStatus,
}

impl Display for OrderVote {
Expand Down Expand Up @@ -48,7 +51,7 @@ impl OrderVote {
Self {
author,
ledger_info,
signature,
signature: SignatureWithStatus::from(signature),
}
}

Expand All @@ -61,9 +64,25 @@ impl OrderVote {
}

pub fn signature(&self) -> &bls12381::Signature {
self.signature.signature()
}

// Question: SignatureWithStatus has interior mutability. Is it okay to expose this?
pub fn signature_with_status(&self) -> &SignatureWithStatus {
&self.signature
}

pub fn is_verified(&self) -> bool {
self.signature.is_verified()
}

/// Only the verify method in validator verifier can set the signature status verified.
/// This method additionally lets the tests to set the status to verified.
#[cfg(any(test, feature = "fuzzing"))]
pub fn set_verified(&self) {
self.signature.set_verified();
}

pub fn epoch(&self) -> u64 {
self.ledger_info.epoch()
}
Expand All @@ -75,7 +94,7 @@ impl OrderVote {
"Failed to verify OrderVote. Consensus data hash is not Zero"
);
validator
.verify(self.author(), &self.ledger_info, &self.signature)
.optimistic_verify(self.author(), &self.ledger_info, &self.signature)
.context("Failed to verify OrderVote")?;

Ok(())
Expand Down
29 changes: 23 additions & 6 deletions consensus/consensus-types/src/vote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use anyhow::{ensure, Context};
use aptos_crypto::{bls12381, hash::CryptoHash, CryptoMaterialError};
use aptos_short_hex_str::AsShortHexStr;
use aptos_types::{
ledger_info::LedgerInfo, validator_signer::ValidatorSigner,
ledger_info::{LedgerInfo, SignatureWithStatus},
validator_signer::ValidatorSigner,
validator_verifier::ValidatorVerifier,
};
use serde::{Deserialize, Serialize};
Expand All @@ -21,14 +22,14 @@ use std::fmt::{Debug, Display, Formatter};
/// is gathers QuorumCertificate (see the detailed explanation in the comments of `LedgerInfo`).
#[derive(Deserialize, Serialize, Clone, PartialEq, Eq)]
pub struct Vote {
/// The data of the vote
/// The data of the vote.
vote_data: VoteData,
/// The identity of the voter.
author: Author,
/// LedgerInfo of a block that is going to be committed in case this vote gathers QC.
ledger_info: LedgerInfo,
/// Signature of the LedgerInfo
signature: bls12381::Signature,
/// Signature on the LedgerInfo along with a status on whether the signature is verified.
signature: SignatureWithStatus,
/// The 2-chain timeout and corresponding signature.
two_chain_timeout: Option<(TwoChainTimeout, bls12381::Signature)>,
}
Expand Down Expand Up @@ -83,7 +84,7 @@ impl Vote {
vote_data,
author,
ledger_info,
signature,
signature: SignatureWithStatus::from(signature),
two_chain_timeout: None,
}
}
Expand All @@ -109,9 +110,25 @@ impl Vote {

/// Return the signature of the vote
pub fn signature(&self) -> &bls12381::Signature {
self.signature.signature()
}

pub fn signature_with_status(&self) -> &SignatureWithStatus {
&self.signature
}

/// Returns whether the signature is verified
pub fn is_verified(&self) -> bool {
self.signature.is_verified()
}

/// Only the verify method in validator verifier can set the signature status verified.
/// This method additionally lets the tests to set the status to verified.
#[cfg(any(test, feature = "fuzzing"))]
pub fn set_verified(&self) {
self.signature.set_verified();
}

/// Returns the 2-chain timeout.
pub fn generate_2chain_timeout(&self, qc: QuorumCert) -> TwoChainTimeout {
TwoChainTimeout::new(
Expand Down Expand Up @@ -147,7 +164,7 @@ impl Vote {
"Vote's hash mismatch with LedgerInfo"
);
validator
.verify(self.author(), &self.ledger_info, &self.signature)
.optimistic_verify(self.author(), &self.ledger_info, &self.signature)
.context("Failed to verify Vote")?;
if let Some((timeout, signature)) = &self.two_chain_timeout {
ensure!(
Expand Down
2 changes: 2 additions & 0 deletions consensus/src/block_storage/block_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ async fn test_insert_vote() {
voter,
)
.unwrap();
vote.set_verified();
let vote_res = pending_votes.insert_vote(&vote, &validator_verifier);

// first vote of an author is accepted
Expand Down Expand Up @@ -329,6 +330,7 @@ async fn test_insert_vote() {
final_voter,
)
.unwrap();
vote.set_verified();
match pending_votes.insert_vote(&vote, &validator_verifier) {
VoteReceptionResult::NewQuorumCertificate(qc) => {
assert_eq!(qc.certified_block().id(), block.id());
Expand Down
8 changes: 7 additions & 1 deletion consensus/src/epoch_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,13 @@ impl<P: OnChainConfigProvider> EpochManager<P> {
let validator_set: ValidatorSet = payload
.get()
.expect("failed to get ValidatorSet from payload");
let epoch_state = Arc::new(EpochState::new(payload.epoch(), (&validator_set).into()));
let mut verifier: ValidatorVerifier = (&validator_set).into();
verifier.set_optimistic_sig_verification_flag(self.config.optimistic_sig_verification);

let epoch_state = Arc::new(EpochState {
epoch: payload.epoch(),
verifier: verifier.into(),
});

self.epoch_state = Some(epoch_state.clone());

Expand Down
12 changes: 5 additions & 7 deletions consensus/src/liveness/round_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

use crate::{
counters,
pending_votes::{PendingVotes, VoteReceptionResult},
pending_votes::{PendingVotes, VoteReceptionResult, VoteStatus},
util::time_service::{SendTask, TimeService},
};
use aptos_consensus_types::{
Expand All @@ -13,9 +13,7 @@ use aptos_consensus_types::{
};
use aptos_crypto::HashValue;
use aptos_logger::{prelude::*, Schema};
use aptos_types::{
ledger_info::LedgerInfoWithVerifiedSignatures, validator_verifier::ValidatorVerifier,
};
use aptos_types::validator_verifier::ValidatorVerifier;
use futures::future::AbortHandle;
use serde::Serialize;
use std::{fmt, sync::Arc, time::Duration};
Expand Down Expand Up @@ -45,7 +43,7 @@ pub struct NewRoundEvent {
pub round: Round,
pub reason: NewRoundReason,
pub timeout: Duration,
pub prev_round_votes: Vec<(HashValue, LedgerInfoWithVerifiedSignatures)>,
pub prev_round_votes: Vec<(HashValue, VoteStatus)>,
pub prev_round_timeout_votes: Option<TwoChainTimeoutWithPartialSignatures>,
}

Expand Down Expand Up @@ -279,10 +277,10 @@ impl RoundState {
pub fn insert_vote(
&mut self,
vote: &Vote,
verifier: &ValidatorVerifier,
validator_verifier: &ValidatorVerifier,
) -> VoteReceptionResult {
if vote.vote_data().proposed().round() == self.current_round {
self.pending_votes.insert_vote(vote, verifier)
self.pending_votes.insert_vote(vote, validator_verifier)
} else {
VoteReceptionResult::UnexpectedRound(
vote.vote_data().proposed().round(),
Expand Down
Loading

0 comments on commit 2deaf9f

Please sign in to comment.