Optimistic signature verification for commit votes

config/src/config/consensus_config.rs

@@ -321,7 +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,
+            optimistic_sig_verification: true,
             enable_round_timeout_msg: false,
         }
     }

consensus/consensus-types/src/pipeline/commit_vote.rs

@@ -7,7 +7,9 @@ use anyhow::Context;
 use aptos_crypto::{bls12381, CryptoMaterialError};
 use aptos_short_hex_str::AsShortHexStr;
 use aptos_types::{
-    block_info::BlockInfo, ledger_info::LedgerInfo, validator_signer::ValidatorSigner,
+    block_info::BlockInfo,
+    ledger_info::{LedgerInfo, SignatureWithStatus},
+    validator_signer::ValidatorSigner,
     validator_verifier::ValidatorVerifier,
 };
 use serde::{Deserialize, Serialize};
@@ -17,7 +19,8 @@ use std::fmt::{Debug, Display, Formatter};
 pub struct CommitVote {
     author: Author,
     ledger_info: LedgerInfo,
-    signature: bls12381::Signature,
+    /// Signature on the LedgerInfo along with a status on whether the signature is verified.
+    signature: SignatureWithStatus,
 }
 
 // this is required by structured log
@@ -62,7 +65,7 @@ impl CommitVote {
         Self {
             author,
             ledger_info,
-            signature,
+            signature: SignatureWithStatus::from(signature),
         }
     }
 
@@ -78,6 +81,13 @@ impl CommitVote {
 
     /// Return the signature of the vote
     pub fn signature(&self) -> &bls12381::Signature {
+        self.signature.signature()
+    }
+
+    /// Returns the signature along with the verification status of the signature.
+    // Note: SignatureWithStatus has interior mutability for verification status.
+    // Need to make sure the verification status is set to true only the verification is successful.
+    pub fn signature_with_status(&self) -> &SignatureWithStatus {
         &self.signature
     }
 
@@ -93,7 +103,7 @@ impl CommitVote {
     /// and then verifies the signature.
     pub fn verify(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
         validator
-            .verify(self.author(), &self.ledger_info, &self.signature)
+            .optimistic_verify(self.author(), &self.ledger_info, &self.signature)
             .context("Failed to verify Commit Vote")
     }
 

consensus/src/pipeline/buffer_item.rs

@@ -2,7 +2,11 @@
 // Parts of the project are originally copyright © Meta Platforms, Inc.
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::{pipeline::hashable::Hashable, state_replication::StateComputerCommitCallBackType};
+use std::collections::HashMap;
+
+use crate::{
+    counters, pipeline::hashable::Hashable, state_replication::StateComputerCommitCallBackType,
+};
 use anyhow::anyhow;
 use aptos_consensus_types::{
     common::{Author, Round},
@@ -16,7 +20,9 @@ use aptos_reliable_broadcast::DropGuard;
 use aptos_types::{
     aggregate_signature::PartialSignatures,
     block_info::BlockInfo,
-    ledger_info::{LedgerInfo, LedgerInfoWithSignatures, LedgerInfoWithVerifiedSignatures},
+    ledger_info::{
+        LedgerInfo, LedgerInfoWithSignatures, LedgerInfoWithUnverifiedSignatures,
+    },
     validator_verifier::ValidatorVerifier,
 };
 use futures::future::BoxFuture;
@@ -38,47 +44,18 @@ fn generate_commit_ledger_info(
     )
 }
 
-fn verify_signatures(
-    unverified_signatures: PartialSignatures,
-    validator: &ValidatorVerifier,
+fn ledger_info_with_unverified_signatures(
+    unverified_votes: HashMap<Author, CommitVote>,
     commit_ledger_info: &LedgerInfo,
-) -> PartialSignatures {
-    // Returns a valid partial signature from a set of unverified signatures.
-    // TODO: Validating individual signatures in expensive. Replace this with optimistic signature
-    // verification for BLS. Here, we can implement a tree-based batch verification technique that
-    // filters out invalid signature shares much faster when there are only a few of them
-    // (e.g., [LM07]: Finding Invalid Signatures in Pairing-Based Batches,
-    // by Law, Laurie and Matt, Brian J., in Cryptography and Coding, 2007).
-    PartialSignatures::new(
-        unverified_signatures
-            .signatures()
-            .iter()
-            .filter(|(author, sig)| validator.verify(**author, commit_ledger_info, sig).is_ok())
-            .map(|(author, sig)| (*author, sig.clone()))
-            .collect(),
-    )
-}
-
-fn generate_executed_item_from_ordered(
-    commit_info: BlockInfo,
-    executed_blocks: Vec<PipelinedBlock>,
-    verified_signatures: PartialSignatures,
-    callback: StateComputerCommitCallBackType,
-    ordered_proof: LedgerInfoWithSignatures,
-    order_vote_enabled: bool,
-) -> BufferItem {
-    debug!("{} advance to executed from ordered", commit_info);
-    let partial_commit_proof = LedgerInfoWithVerifiedSignatures::new(
-        generate_commit_ledger_info(&commit_info, &ordered_proof, order_vote_enabled),
-        verified_signatures,
-    );
-    BufferItem::Executed(Box::new(ExecutedItem {
-        executed_blocks,
-        partial_commit_proof,
-        callback,
-        commit_info,
-        ordered_proof,
-    }))
+) -> LedgerInfoWithUnverifiedSignatures {
+    let mut li_with_sig = LedgerInfoWithUnverifiedSignatures::new(commit_ledger_info.clone());
+    for vote in unverified_votes.values() {
+        let sig = vote.signature_with_status();
+        if vote.ledger_info() == commit_ledger_info {
+            li_with_sig.add_signature(vote.author(), sig);
+        }
+    }
+    li_with_sig
 }
 
 fn aggregate_commit_proof(
@@ -95,7 +72,7 @@ fn aggregate_commit_proof(
 // we differentiate buffer items at different stages
 // for better code readability
 pub struct OrderedItem {
-    pub unverified_signatures: PartialSignatures,
+    pub unverified_votes: HashMap<Author, CommitVote>,
     // This can happen in the fast forward sync path, where we can receive the commit proof
     // from peers.
     pub commit_proof: Option<LedgerInfoWithSignatures>,
@@ -106,15 +83,15 @@ pub struct OrderedItem {
 
 pub struct ExecutedItem {
     pub executed_blocks: Vec<PipelinedBlock>,
-    pub partial_commit_proof: LedgerInfoWithVerifiedSignatures,
+    pub partial_commit_proof: LedgerInfoWithUnverifiedSignatures,
     pub callback: StateComputerCommitCallBackType,
     pub commit_info: BlockInfo,
     pub ordered_proof: LedgerInfoWithSignatures,
 }
 
 pub struct SignedItem {
     pub executed_blocks: Vec<PipelinedBlock>,
-    pub partial_commit_proof: LedgerInfoWithVerifiedSignatures,
+    pub partial_commit_proof: LedgerInfoWithUnverifiedSignatures,
     pub callback: StateComputerCommitCallBackType,
     pub commit_vote: CommitVote,
     pub rb_handle: Option<(Instant, DropGuard)>,
@@ -146,10 +123,10 @@ impl BufferItem {
         ordered_blocks: Vec<PipelinedBlock>,
         ordered_proof: LedgerInfoWithSignatures,
         callback: StateComputerCommitCallBackType,
-        unverified_signatures: PartialSignatures,
+        unverified_votes: HashMap<Author, CommitVote>,
     ) -> Self {
         Self::Ordered(Box::new(OrderedItem {
-            unverified_signatures,
+            unverified_votes,
             commit_proof: None,
             callback,
             ordered_blocks,
@@ -170,7 +147,7 @@ impl BufferItem {
                 let OrderedItem {
                     ordered_blocks,
                     commit_proof,
-                    unverified_signatures,
+                    unverified_votes,
                     callback,
                     ordered_proof,
                 } = *ordered_item;
@@ -211,16 +188,11 @@ impl BufferItem {
                         order_vote_enabled,
                     );
 
-                    let verified_signatures =
-                        verify_signatures(unverified_signatures, validator, &commit_ledger_info);
-                    if (validator.check_voting_power(verified_signatures.signatures().keys(), true))
-                        .is_ok()
-                    {
-                        let commit_proof = aggregate_commit_proof(
-                            &commit_ledger_info,
-                            &verified_signatures,
-                            validator,
-                        );
+                    let mut partial_commit_proof = ledger_info_with_unverified_signatures(
+                        unverified_votes,
+                        &commit_ledger_info,
+                    );
+                    if let Ok(commit_proof) = partial_commit_proof.aggregate_and_verify(validator) {
                         debug!(
                             "{} advance to aggregated from ordered",
                             commit_proof.commit_info()
@@ -231,14 +203,13 @@ impl BufferItem {
                             callback,
                         }))
                     } else {
-                        generate_executed_item_from_ordered(
-                            commit_info,
+                        Self::Executed(Box::new(ExecutedItem {
                             executed_blocks,
-                            verified_signatures,
+                            partial_commit_proof,
                             callback,
+                            commit_info,
                             ordered_proof,
-                            order_vote_enabled,
-                        )
+                        }))
                     }
                 }
             },
@@ -294,7 +265,7 @@ impl BufferItem {
                     partial_commit_proof: local_commit_proof,
                     ..
                 } = *signed_item;
-                assert_eq!(local_commit_proof.commit_info(), commit_proof.commit_info(),);
+                assert_eq!(local_commit_proof.commit_info(), commit_proof.commit_info());
                 debug!(
                     "{} advance to aggregated with commit decision",
                     commit_proof.commit_info()
@@ -348,43 +319,51 @@ impl BufferItem {
     pub fn try_advance_to_aggregated(self, validator: &ValidatorVerifier) -> Self {
         match self {
             Self::Signed(signed_item) => {
-                if validator
-                    .check_voting_power(signed_item.partial_commit_proof.signatures().keys(), true)
+                if signed_item
+                    .partial_commit_proof
+                    .check_voting_power(validator, true)
                     .is_ok()
                 {
-                    Self::Aggregated(Box::new(AggregatedItem {
-                        executed_blocks: signed_item.executed_blocks,
-                        commit_proof: aggregate_commit_proof(
-                            signed_item.partial_commit_proof.ledger_info(),
-                            signed_item.partial_commit_proof.partial_sigs(),
-                            validator,
-                        ),
-                        callback: signed_item.callback,
-                    }))
-                } else {
-                    Self::Signed(signed_item)
+                    let _time = counters::VERIFY_MSG
+                        .with_label_values(&["commit_vote_aggregate_and_verify"])
+                        .start_timer();
+                    if let Ok(commit_proof) = signed_item
+                        .partial_commit_proof
+                        .clone()
+                        .aggregate_and_verify(validator)
+                    {
+                        return Self::Aggregated(Box::new(AggregatedItem {
+                            executed_blocks: signed_item.executed_blocks,
+                            commit_proof,
+                            callback: signed_item.callback,
+                        }));
+                    }
                 }
+                Self::Signed(signed_item)
             },
             Self::Executed(executed_item) => {
-                if validator
-                    .check_voting_power(
-                        executed_item.partial_commit_proof.signatures().keys(),
-                        true,
-                    )
+                if executed_item
+                    .partial_commit_proof
+                    .check_voting_power(validator, true)
                     .is_ok()
                 {
-                    Self::Aggregated(Box::new(AggregatedItem {
-                        executed_blocks: executed_item.executed_blocks,
-                        commit_proof: aggregate_commit_proof(
-                            executed_item.partial_commit_proof.ledger_info(),
-                            executed_item.partial_commit_proof.partial_sigs(),
-                            validator,
-                        ),
-                        callback: executed_item.callback,
-                    }))
-                } else {
-                    Self::Executed(executed_item)
+                    let _time = counters::VERIFY_MSG
+                        .with_label_values(&["commit_vote_aggregate_and_verify"])
+                        .start_timer();
+
+                    if let Ok(commit_proof) = executed_item
+                        .partial_commit_proof
+                        .clone()
+                        .aggregate_and_verify(validator)
+                    {
+                        return Self::Aggregated(Box::new(AggregatedItem {
+                            executed_blocks: executed_item.executed_blocks,
+                            commit_proof,
+                            callback: executed_item.callback,
+                        }));
+                    }
                 }
+                Self::Executed(executed_item)
             },
             _ => self,
         }
@@ -417,7 +396,7 @@ impl BufferItem {
     pub fn add_signature_if_matched(&mut self, vote: CommitVote) -> anyhow::Result<()> {
         let target_commit_info = vote.commit_info();
         let author = vote.author();
-        let signature = vote.signature().clone();
+        let signature = vote.signature_with_status();
         match self {
             Self::Ordered(ordered) => {
                 if ordered
@@ -429,9 +408,7 @@ impl BufferItem {
                     // when advancing to executed item, we will check if the sigs are valid.
                     // each author at most stores a single sig for each item,
                     // so an adversary will not be able to flood our memory.
-                    ordered
-                        .unverified_signatures
-                        .add_signature(author, signature);
+                    ordered.unverified_votes.insert(author, vote);
                     return Ok(());
                 }
             },