Optimistic signature verification for commit votes

config/src/config/consensus_config.rs

@@ -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,
@@ -89,50 +90,16 @@ pub struct ConsensusConfig {
     pub rand_rb_config: ReliableBroadcastConfig,
     pub num_bounded_executor_tasks: u64,
     pub enable_pre_commit: bool,
+    pub optimistic_sig_verification_for_votes: bool,
+    pub optimistic_sig_verification_for_order_votes: bool,
+    pub optimistic_sig_verification_for_commit_votes: bool,
 }
 
+/// Deprecated
 #[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
 pub enum QcAggregatorType {
     #[default]
     NoDelay,
-    Delayed(DelayedQcAggregatorConfig),
-}
-
-impl QcAggregatorType {
-    pub fn default_delayed() -> Self {
-        // TODO: Enable the delayed aggregation by default once we have tested it more.
-        Self::Delayed(DelayedQcAggregatorConfig::default())
-    }
-}
-
-#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
-pub struct DelayedQcAggregatorConfig {
-    // Maximum Delay for a QC to be aggregated after round start (in milliseconds). This assumes that
-    // we have enough voting power to form a QC. If we don't have enough voting power, we will wait
-    // until we have enough voting power to form a QC.
-    pub max_delay_after_round_start_ms: u64,
-    // Percentage of aggregated voting power to wait for before aggregating a QC. For example, if this
-    // is set to 95% then, a QC is formed as soon as we have 95% of the voting power aggregated without
-    // any additional waiting.
-    pub aggregated_voting_power_pct_to_wait: usize,
-    // This knob control what is the % of the time (as compared to time between round start and time when we
-    // have enough voting power to form a QC) we wait after we have enough voting power to form a QC. In a sense,
-    // this knobs controls how much slower we are willing to make consensus to wait for more votes.
-    pub pct_delay_after_qc_aggregated: usize,
-    // In summary, let's denote the time we have enough voting power (2f + 1) to form a QC as T1 and
-    // the time we have aggregated `aggregated_voting_power_pct_to_wait` as T2. Then, we wait for
-    // min((T1 + `pct_delay_after_qc_aggregated` * T1 / 100), `max_delay_after_round_start_ms`, T2)
-    // before forming a QC.
-}
-
-impl Default for DelayedQcAggregatorConfig {
-    fn default() -> Self {
-        Self {
-            max_delay_after_round_start_ms: 700,
-            aggregated_voting_power_pct_to_wait: 90,
-            pct_delay_after_qc_aggregated: 30,
-        }
-    }
 }
 
 /// Execution backpressure which handles gas/s variance,
@@ -336,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,
@@ -354,6 +320,9 @@ impl Default for ConsensusConfig {
             },
             num_bounded_executor_tasks: 16,
             enable_pre_commit: true,
+            optimistic_sig_verification_for_votes: true,
+            optimistic_sig_verification_for_order_votes: true,
+            optimistic_sig_verification_for_commit_votes: true,
         }
     }
 }

consensus/consensus-types/src/lib.rs

@@ -8,7 +8,6 @@ pub mod block;
 pub mod block_data;
 pub mod block_retrieval;
 pub mod common;
-pub mod delayed_qc_msg;
 pub mod epoch_retrieval;
 pub mod order_vote;
 pub mod order_vote_msg;

consensus/consensus-types/src/order_vote.rs

@@ -68,16 +68,27 @@ impl OrderVote {
         self.ledger_info.epoch()
     }
 
-    /// Verifies the signature on LedgerInfo.
-    pub fn verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+    /// Performs basic checks, excluding the signature verification.
+    pub fn partial_verify(&self) -> anyhow::Result<()> {
         ensure!(
             self.ledger_info.consensus_data_hash() == HashValue::zero(),
             "Failed to verify OrderVote. Consensus data hash is not Zero"
         );
+        Ok(())
+    }
+
+    /// Verifies the signature on the LedgerInfo.
+    pub fn signature_verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
         validator
             .verify(self.author(), &self.ledger_info, &self.signature)
             .context("Failed to verify OrderVote")?;
+        Ok(())
+    }
 
+    /// Performs full verification including the signature verification.
+    pub fn verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+        self.partial_verify()?;
+        self.signature_verify(validator)?;
         Ok(())
     }
 }

consensus/consensus-types/src/order_vote_msg.rs

@@ -44,13 +44,19 @@ impl OrderVoteMsg {
         self.order_vote.epoch()
     }
 
-    /// This function verifies the order_vote component in the order_vote_msg.
-    /// The quorum cert is verified in the round manager when the quorum certificate is used.
-    pub fn verify_order_vote(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+    /// Performs basic checks, excluding the signature verification.
+    pub fn partial_verify(&self) -> anyhow::Result<()> {
         ensure!(
             self.quorum_cert().certified_block() == self.order_vote().ledger_info().commit_info(),
             "QuorumCert and OrderVote do not match"
         );
+        self.order_vote.partial_verify()
+    }
+
+    /// This function verifies the order_vote component in the order_vote_msg.
+    /// The quorum cert is verified in the round manager when the quorum certificate is used.
+    pub fn verify_order_vote(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+        self.partial_verify()?;
         self.order_vote
             .verify(validator)
             .context("[OrderVoteMsg] OrderVote verification failed")?;

consensus/consensus-types/src/vote.rs

@@ -137,16 +137,13 @@ impl Vote {
         self.two_chain_timeout.is_some()
     }
 
-    /// Verifies that the consensus data hash of LedgerInfo corresponds to the vote info,
-    /// and then verifies the signature.
-    pub fn verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+    /// Peforms basic verification such as verifying that the consensus data hash of LedgerInfo
+    /// corresponds to the vote info. Does not verify signature on the LedgerInfo.
+    pub fn partial_verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
         ensure!(
             self.ledger_info.consensus_data_hash() == self.vote_data.hash(),
             "Vote's hash mismatch with LedgerInfo"
         );
-        validator
-            .verify(self.author(), &self.ledger_info, &self.signature)
-            .context("Failed to verify Vote")?;
         if let Some((timeout, signature)) = &self.two_chain_timeout {
             ensure!(
                 (timeout.epoch(), timeout.round())
@@ -162,4 +159,18 @@ impl Vote {
         self.vote_data().verify()?;
         Ok(())
     }
+
+    /// Verifies the signature on the LedgerInfo.
+    pub fn signature_verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+        validator
+            .verify(self.author(), &self.ledger_info, &self.signature)
+            .context("Failed to verify Vote signature")?;
+        Ok(())
+    }
+
+    /// Performs full verification including the signature verification.
+    pub fn verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+        self.partial_verify(validator)?;
+        self.signature_verify(validator)
+    }
 }

consensus/consensus-types/src/vote_msg.rs

@@ -54,7 +54,9 @@ impl VoteMsg {
         self.vote.vote_data().proposed().id()
     }
 
-    pub fn verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+    // Performs the basic checks on the vote message, excluding the signature verification on
+    // the vote.
+    pub fn partial_verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
         ensure!(
             self.vote().epoch() == self.sync_info.epoch(),
             "VoteMsg has different epoch"
@@ -72,6 +74,12 @@ impl VoteMsg {
         // We're not verifying SyncInfo here yet: we are going to verify it only in case we need
         // it. This way we avoid verifying O(n) SyncInfo messages while aggregating the votes
         // (O(n^2) signature verifications).
-        self.vote().verify(validator)
+        self.vote().partial_verify(validator)
+    }
+
+    /// Verifies the vote message, including the signature verification on the vote.
+    pub fn verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
+        self.partial_verify(validator)?;
+        self.vote().signature_verify(validator)
     }
 }

consensus/src/block_storage/block_store_test.rs

@@ -8,9 +8,7 @@ use crate::{
     test_utils::{
         build_empty_tree, build_simple_tree, consensus_runtime, timed_block_on, TreeInserter,
     },
-    util::mock_time_service::SimulatedTimeService,
 };
-use aptos_config::config::QcAggregatorType;
 use aptos_consensus_types::{
     block::{
         block_test_utils::{
@@ -25,9 +23,9 @@ use aptos_consensus_types::{
 };
 use aptos_crypto::{HashValue, PrivateKey};
 use aptos_types::{
-    validator_signer::ValidatorSigner, validator_verifier::random_validator_verifier,
+    epoch_state::EpochState, validator_signer::ValidatorSigner,
+    validator_verifier::random_validator_verifier,
 };
-use futures_channel::mpsc::unbounded;
 use proptest::prelude::*;
 use std::{cmp::min, collections::HashSet, sync::Arc};
 
@@ -277,18 +275,16 @@ async fn test_insert_vote() {
     ::aptos_logger::Logger::init_for_testing();
     // Set up enough different authors to support different votes for the same block.
     let (signers, validator_verifier) = random_validator_verifier(11, Some(10), false);
+    let epoch_state = Arc::new(EpochState::new(5, validator_verifier));
     let my_signer = signers[10].clone();
     let mut inserter = TreeInserter::new(my_signer);
     let block_store = inserter.block_store();
     let genesis = block_store.ordered_root();
     let block = inserter
         .insert_block_with_qc(certificate_for_genesis(), &genesis, 1)
         .await;
-    let time_service = Arc::new(SimulatedTimeService::new());
-    let (delayed_qc_tx, _) = unbounded();
 
-    let mut pending_votes =
-        PendingVotes::new(time_service, delayed_qc_tx, QcAggregatorType::NoDelay);
+    let mut pending_votes = PendingVotes::new();
 
     assert!(block_store.get_quorum_cert_for_block(block.id()).is_none());
     for (i, voter) in signers.iter().enumerate().take(10).skip(1) {
@@ -306,13 +302,13 @@ async fn test_insert_vote() {
             voter,
         )
         .unwrap();
-        let vote_res = pending_votes.insert_vote(&vote, &validator_verifier);
+        let vote_res = pending_votes.insert_vote(&vote, epoch_state.clone(), true);
 
         // first vote of an author is accepted
         assert_eq!(vote_res, VoteReceptionResult::VoteAdded(i as u128));
         // filter out duplicates
         assert_eq!(
-            pending_votes.insert_vote(&vote, &validator_verifier),
+            pending_votes.insert_vote(&vote, epoch_state.clone(), true),
             VoteReceptionResult::DuplicateVote,
         );
         // qc is still not there
@@ -335,7 +331,7 @@ async fn test_insert_vote() {
         final_voter,
     )
     .unwrap();
-    match pending_votes.insert_vote(&vote, &validator_verifier) {
+    match pending_votes.insert_vote(&vote, epoch_state.clone(), true) {
         VoteReceptionResult::NewQuorumCertificate(qc) => {
             assert_eq!(qc.certified_block().id(), block.id());
             block_store