diff --git a/config/src/config/consensus_config.rs b/config/src/config/consensus_config.rs index 56b432a7f28b7..f907072edfb67 100644 --- a/config/src/config/consensus_config.rs +++ b/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, + // Deprecated pub qc_aggregator_type: QcAggregatorType, // Max blocks allowed for block retrieval requests pub max_blocks_per_sending_request: u64, @@ -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, } @@ -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, @@ -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, } } diff --git a/consensus/consensus-types/src/order_vote.rs b/consensus/consensus-types/src/order_vote.rs index c2a257cdf83c5..b487f9a1668d8 100644 --- a/consensus/consensus-types/src/order_vote.rs +++ b/consensus/consensus-types/src/order_vote.rs @@ -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}; @@ -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 { @@ -48,7 +51,7 @@ impl OrderVote { Self { author, ledger_info, - signature, + signature: SignatureWithStatus::from(signature), } } @@ -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() } @@ -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(()) diff --git a/consensus/consensus-types/src/vote.rs b/consensus/consensus-types/src/vote.rs index 87359c31cfe70..20400be9a92c8 100644 --- a/consensus/consensus-types/src/vote.rs +++ b/consensus/consensus-types/src/vote.rs @@ -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}; @@ -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)>, } @@ -83,7 +84,7 @@ impl Vote { vote_data, author, ledger_info, - signature, + signature: SignatureWithStatus::from(signature), two_chain_timeout: None, } } @@ -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( @@ -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!( diff --git a/consensus/src/block_storage/block_store_test.rs b/consensus/src/block_storage/block_store_test.rs index 41def8f1c322d..09513e648bca5 100644 --- a/consensus/src/block_storage/block_store_test.rs +++ b/consensus/src/block_storage/block_store_test.rs @@ -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 @@ -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()); diff --git a/consensus/src/epoch_manager.rs b/consensus/src/epoch_manager.rs index 15abd03e92fc9..c0e1ba3ac7ecb 100644 --- a/consensus/src/epoch_manager.rs +++ b/consensus/src/epoch_manager.rs @@ -1063,7 +1063,13 @@ impl EpochManager

{ 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()); diff --git a/consensus/src/liveness/round_state.rs b/consensus/src/liveness/round_state.rs index 8e7602e61c0d5..37912ecbdaaa7 100644 --- a/consensus/src/liveness/round_state.rs +++ b/consensus/src/liveness/round_state.rs @@ -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::{ @@ -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}; @@ -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, } @@ -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(), diff --git a/consensus/src/pending_order_votes.rs b/consensus/src/pending_order_votes.rs index 46cf23cfe2b90..58aaf8b90a852 100644 --- a/consensus/src/pending_order_votes.rs +++ b/consensus/src/pending_order_votes.rs @@ -2,12 +2,12 @@ // Parts of the project are originally copyright © Meta Platforms, Inc. // SPDX-License-Identifier: Apache-2.0 +use crate::counters; use aptos_consensus_types::{common::Author, order_vote::OrderVote, quorum_cert::QuorumCert}; use aptos_crypto::{hash::CryptoHash, HashValue}; use aptos_logger::prelude::*; use aptos_types::{ - aggregate_signature::PartialSignatures, - ledger_info::{LedgerInfo, LedgerInfoWithSignatures, LedgerInfoWithVerifiedSignatures}, + ledger_info::{LedgerInfo, LedgerInfoWithSignatures, LedgerInfoWithUnverifiedSignatures}, validator_verifier::{ValidatorVerifier, VerifyError}, }; use std::{collections::HashMap, sync::Arc}; @@ -33,12 +33,12 @@ pub enum OrderVoteReceptionResult { #[derive(Debug, PartialEq, Eq)] enum OrderVoteStatus { EnoughVotes(LedgerInfoWithSignatures), - NotEnoughVotes(LedgerInfoWithVerifiedSignatures), + NotEnoughVotes(LedgerInfoWithUnverifiedSignatures), } /// A PendingVotes structure keep track of order votes for the last few rounds pub struct PendingOrderVotes { - /// Maps LedgerInfo digest to associated signatures (contained in a partial LedgerInfoWithSignatures). + /// Maps LedgerInfo digest to associated signatures. /// Order vote status stores caches the information on whether the votes are enough to form a QC. /// We also store the QC that the order votes certify. li_digest_to_votes: @@ -75,9 +75,8 @@ impl PendingOrderVotes { verified_quorum_cert.expect( "Quorum Cert is expected when creating a new entry in pending order votes", ), - OrderVoteStatus::NotEnoughVotes(LedgerInfoWithVerifiedSignatures::new( + OrderVoteStatus::NotEnoughVotes(LedgerInfoWithUnverifiedSignatures::new( order_vote.ledger_info().clone(), - PartialSignatures::empty(), )), ) }); @@ -110,16 +109,20 @@ impl PendingOrderVotes { order_vote.author() ); } - li_with_sig.add_signature(order_vote.author(), order_vote.signature().clone()); - // check if we have enough signatures to create a QC - match validator_verifier.check_voting_power(li_with_sig.signatures().keys(), true) { - // a quorum of signature was reached, a new QC is formed + li_with_sig.add_signature(order_vote.author(), order_vote.signature_with_status()); + match li_with_sig.check_voting_power(validator_verifier, true) { Ok(aggregated_voting_power) => { assert!( aggregated_voting_power >= validator_verifier.quorum_voting_power(), "QC aggregation should not be triggered if we don't have enough votes to form a QC" ); - match li_with_sig.aggregate_signatures(validator_verifier) { + let verification_result = { + let _timer = counters::VERIFY_MSG + .with_label_values(&["order_vote_aggregate_and_verify"]) + .start_timer(); + li_with_sig.aggregate_and_verify(validator_verifier) + }; + match verification_result { Ok(ledger_info_with_sig) => { *status = OrderVoteStatus::EnoughVotes(ledger_info_with_sig.clone()); @@ -128,16 +131,15 @@ impl PendingOrderVotes { ledger_info_with_sig, )) }, + Err(VerifyError::TooLittleVotingPower { voting_power, .. }) => { + OrderVoteReceptionResult::VoteAdded(voting_power) + }, Err(e) => OrderVoteReceptionResult::ErrorAggregatingSignature(e), } }, - - // not enough votes Err(VerifyError::TooLittleVotingPower { voting_power, .. }) => { OrderVoteReceptionResult::VoteAdded(voting_power) }, - - // error Err(error) => { error!( "MUST_FIX: order vote received could not be added: {}, order vote: {}", @@ -175,11 +177,11 @@ impl PendingOrderVotes { #[cfg(test)] mod tests { - use super::{OrderVoteReceptionResult, PendingOrderVotes}; + use super::{OrderVoteReceptionResult, OrderVoteStatus, PendingOrderVotes}; use aptos_consensus_types::{order_vote::OrderVote, quorum_cert::QuorumCert}; - use aptos_crypto::HashValue; + use aptos_crypto::{bls12381, hash::CryptoHash, HashValue}; use aptos_types::{ - block_info::BlockInfo, ledger_info::LedgerInfo, + aggregate_signature::PartialSignatures, block_info::BlockInfo, ledger_info::LedgerInfo, validator_verifier::random_validator_verifier, }; @@ -195,7 +197,7 @@ mod tests { fn order_vote_aggregation() { ::aptos_logger::Logger::init_for_testing(); // set up 4 validators - let (signers, validator) = random_validator_verifier(4, Some(2), false); + let (signers, verifier) = random_validator_verifier(4, Some(2), false); let mut pending_order_votes = PendingOrderVotes::new(); @@ -209,22 +211,19 @@ mod tests { ); // first time a new order vote is added -> OrderVoteAdded + order_vote_1_author_0.set_verified(); assert_eq!( pending_order_votes.insert_order_vote( &order_vote_1_author_0, - &validator, + &verifier, Some(qc.clone()) ), - OrderVoteReceptionResult::VoteAdded(1), + OrderVoteReceptionResult::VoteAdded(1) ); // same author voting for the same thing -> OrderVoteAdded assert_eq!( - pending_order_votes.insert_order_vote( - &order_vote_1_author_0, - &validator, - Some(qc.clone()) - ), + pending_order_votes.insert_order_vote(&order_vote_1_author_0, &verifier, None), OrderVoteReceptionResult::VoteAdded(1) ); @@ -235,13 +234,14 @@ mod tests { li2.clone(), signers[1].sign(&li2).expect("Unable to sign ledger info"), ); + order_vote_2_author_1.set_verified(); assert_eq!( pending_order_votes.insert_order_vote( &order_vote_2_author_1, - &validator, + &verifier, Some(qc.clone()) ), - OrderVoteReceptionResult::VoteAdded(1), + OrderVoteReceptionResult::VoteAdded(1) ); assert!(!pending_order_votes.has_enough_order_votes(&li1)); @@ -252,13 +252,10 @@ mod tests { li2.clone(), signers[2].sign(&li2).expect("Unable to sign ledger info"), ); - match pending_order_votes.insert_order_vote( - &order_vote_2_author_2, - &validator, - Some(qc.clone()), - ) { - OrderVoteReceptionResult::NewLedgerInfoWithSignatures((_, li_with_sig)) => { - assert!(li_with_sig.check_voting_power(&validator).is_ok()); + order_vote_2_author_2.set_verified(); + match pending_order_votes.insert_order_vote(&order_vote_2_author_2, &verifier, None) { + OrderVoteReceptionResult::NewLedgerInfoWithSignatures((_qc, li_with_sig)) => { + assert!(li_with_sig.check_voting_power(&verifier).is_ok()); }, _ => { panic!("No QC formed."); @@ -271,4 +268,111 @@ mod tests { assert!(!pending_order_votes.has_enough_order_votes(&li1)); assert!(!pending_order_votes.has_enough_order_votes(&li2)); } + + #[test] + fn order_vote_aggregation_with_unverified_votes() { + ::aptos_logger::Logger::init_for_testing(); + + let (signers, verifier) = random_validator_verifier(5, Some(3), false); + let mut pending_order_votes = PendingOrderVotes::new(); + let mut partial_signatures = PartialSignatures::empty(); + let qc = QuorumCert::dummy(); + + // create random vote from validator[0] + let li = random_ledger_info(); + let li_hash = li.hash(); + let vote_0 = OrderVote::new_with_signature( + signers[0].author(), + li.clone(), + signers[0].sign(&li).expect("Unable to sign ledger info"), + ); + partial_signatures.add_signature(signers[0].author(), vote_0.signature().clone()); + + let vote_1 = OrderVote::new_with_signature( + signers[1].author(), + li.clone(), + signers[1].sign(&li).expect("Unable to sign ledger info"), + ); + partial_signatures.add_signature(signers[1].author(), vote_1.signature().clone()); + + let vote_2 = OrderVote::new_with_signature( + signers[2].author(), + li.clone(), + bls12381::Signature::dummy_signature(), + ); + + let vote_3 = OrderVote::new_with_signature( + signers[3].author(), + li.clone(), + signers[3].sign(&li).expect("Unable to sign ledger info"), + ); + partial_signatures.add_signature(signers[3].author(), vote_3.signature().clone()); + + let vote_4 = OrderVote::new_with_signature( + signers[4].author(), + li.clone(), + signers[4].sign(&li).expect("Unable to sign ledger info"), + ); + + assert_eq!( + pending_order_votes.insert_order_vote(&vote_0, &verifier, Some(qc.clone())), + OrderVoteReceptionResult::VoteAdded(1) + ); + + vote_0.set_verified(); + assert_eq!( + pending_order_votes.insert_order_vote(&vote_0, &verifier, None), + OrderVoteReceptionResult::VoteAdded(1) + ); + + assert_eq!( + pending_order_votes.insert_order_vote(&vote_1, &verifier, None), + OrderVoteReceptionResult::VoteAdded(2) + ); + + assert_eq!(verifier.pessimistic_verify_set().len(), 0); + assert_eq!( + pending_order_votes.insert_order_vote(&vote_2, &verifier, None), + OrderVoteReceptionResult::VoteAdded(2) + ); + assert_eq!(verifier.pessimistic_verify_set().len(), 1); + let (_, order_vote_status) = pending_order_votes + .li_digest_to_votes + .get(&li_hash) + .unwrap(); + match order_vote_status { + OrderVoteStatus::NotEnoughVotes(li_with_sig) => { + assert_eq!(li_with_sig.verified_voters().count(), 2); + assert_eq!(li_with_sig.unverified_voters().count(), 0); + }, + _ => { + panic!("QC should not be formed yet."); + }, + } + + let aggregate_sig = verifier + .aggregate_signatures(partial_signatures.signatures_iter()) + .unwrap(); + match pending_order_votes.insert_order_vote(&vote_3, &verifier, None) { + OrderVoteReceptionResult::NewLedgerInfoWithSignatures((_qc, li_with_sig)) => { + assert!(li_with_sig.check_voting_power(&verifier).is_ok()); + + assert_eq!(li_with_sig.signatures().clone(), aggregate_sig.clone()); + }, + _ => { + panic!("No QC formed."); + }, + }; + + match pending_order_votes.insert_order_vote(&vote_4, &verifier, None) { + OrderVoteReceptionResult::NewLedgerInfoWithSignatures((_qc, li_with_sig)) => { + assert!(li_with_sig.check_voting_power(&verifier).is_ok()); + + assert_eq!(li_with_sig.signatures().clone(), aggregate_sig.clone()); + }, + _ => { + panic!("No QC formed."); + }, + }; + } } diff --git a/consensus/src/pending_votes.rs b/consensus/src/pending_votes.rs index a11b9d50dc976..221ed6bca1970 100644 --- a/consensus/src/pending_votes.rs +++ b/consensus/src/pending_votes.rs @@ -19,15 +19,10 @@ use aptos_consensus_types::{ use aptos_crypto::{hash::CryptoHash, HashValue}; use aptos_logger::prelude::*; use aptos_types::{ - aggregate_signature::PartialSignatures, - ledger_info::LedgerInfoWithVerifiedSignatures, + ledger_info::{LedgerInfoWithSignatures, LedgerInfoWithUnverifiedSignatures}, validator_verifier::{ValidatorVerifier, VerifyError}, }; -use std::{ - collections::{BTreeMap, HashMap}, - fmt, - sync::Arc, -}; +use std::{collections::HashMap, fmt, sync::Arc}; /// Result of the vote processing. The failure case (Verification error) is returned /// as the Error part of the result. @@ -54,15 +49,22 @@ pub enum VoteReceptionResult { UnexpectedRound(u64, u64), /// Receive f+1 timeout to trigger a local timeout, return the amount of voting power TC currently has. EchoTimeout(u128), + /// The author of the vote is unknown + UnknownAuthor(Author), +} + +#[derive(Debug, PartialEq, Eq)] +pub enum VoteStatus { + EnoughVotes(LedgerInfoWithSignatures), + NotEnoughVotes(LedgerInfoWithUnverifiedSignatures), } /// A PendingVotes structure keep track of votes pub struct PendingVotes { - /// Maps LedgerInfo digest to associated signatures (contained in a partial LedgerInfoWithSignatures). + /// Maps LedgerInfo digest to associated signatures. /// This might keep multiple LedgerInfos for the current round: either due to different proposals (byzantine behavior) /// or due to different NIL proposals (clients can have a different view of what block to extend). - li_digest_to_votes: - HashMap, + li_digest_to_votes: HashMap, /// Tracks all the signatures of the 2-chain timeout for the given round. maybe_partial_2chain_tc: Option, /// Map of Author to (vote, li_digest). This is useful to discard multiple votes. @@ -210,21 +212,23 @@ impl PendingVotes { let len = self.li_digest_to_votes.len() + 1; // obtain the ledger info with signatures associated to the vote's ledger info - let (hash_index, li_with_sig) = - self.li_digest_to_votes.entry(li_digest).or_insert_with(|| { - // if the ledger info with signatures doesn't exist yet, create it - ( - len, - LedgerInfoWithVerifiedSignatures::new( - vote.ledger_info().clone(), - PartialSignatures::empty(), - ), - ) - }); - - let validator_voting_power = validator_verifier - .get_voting_power(&vote.author()) - .unwrap_or(0); + let (hash_index, status) = self.li_digest_to_votes.entry(li_digest).or_insert_with(|| { + ( + len, + VoteStatus::NotEnoughVotes(LedgerInfoWithUnverifiedSignatures::new( + vote.ledger_info().clone(), + )), + ) + }); + + let validator_voting_power = validator_verifier.get_voting_power(&vote.author()); + + if validator_voting_power.is_none() { + warn!("Received vote from an unknown author: {}", vote.author()); + return VoteReceptionResult::UnknownAuthor(vote.author()); + } + let validator_voting_power = + validator_voting_power.expect("Author must exist in the validator set."); if validator_voting_power == 0 { warn!("Received vote with no voting power, from {}", vote.author()); } @@ -245,39 +249,58 @@ impl PendingVotes { .set(cur_round); } - // add this vote to the ledger info with signatures - li_with_sig.add_signature(vote.author(), vote.signature().clone()); - - // check if we have enough signatures to create a QC - let voting_power = match validator_verifier - .check_voting_power(li_with_sig.signatures().keys(), true) - { - // a quorum of signature was reached, a new QC is formed - Ok(aggregated_voting_power) => { - assert!( - aggregated_voting_power >= validator_verifier.quorum_voting_power(), - "QC aggregation should not be triggered if we don't have enough votes to form a QC" - ); - match li_with_sig.aggregate_signatures(validator_verifier) { - Ok(ledger_info_with_sig) => { - return VoteReceptionResult::NewQuorumCertificate(Arc::new( - QuorumCert::new(vote.vote_data().clone(), ledger_info_with_sig), - )) - }, - Err(e) => return VoteReceptionResult::ErrorAggregatingSignature(e), - } + let voting_power = match status { + VoteStatus::EnoughVotes(li_with_sig) => { + return VoteReceptionResult::NewQuorumCertificate(Arc::new(QuorumCert::new( + vote.vote_data().clone(), + li_with_sig.clone(), + ))); }, + VoteStatus::NotEnoughVotes(li_with_sig) => { + // add this vote to the ledger info with signatures + li_with_sig.add_signature(vote.author(), vote.signature_with_status()); + + // check if we have enough signatures to create a QC + match li_with_sig.check_voting_power(validator_verifier, true) { + // a quorum of signature was reached, a new QC is formed + Ok(aggregated_voting_power) => { + assert!( + aggregated_voting_power >= validator_verifier.quorum_voting_power(), + "QC aggregation should not be triggered if we don't have enough votes to form a QC" + ); + let verification_result = { + let _timer = counters::VERIFY_MSG + .with_label_values(&["vote_aggregate_and_verify"]) + .start_timer(); + + li_with_sig.aggregate_and_verify(validator_verifier) + }; + match verification_result { + Ok(ledger_info_with_sig) => { + *status = VoteStatus::EnoughVotes(ledger_info_with_sig.clone()); + return VoteReceptionResult::NewQuorumCertificate(Arc::new( + QuorumCert::new(vote.vote_data().clone(), ledger_info_with_sig), + )); + }, + Err(VerifyError::TooLittleVotingPower { voting_power, .. }) => { + voting_power + }, + Err(e) => return VoteReceptionResult::ErrorAggregatingSignature(e), + } + }, - // not enough votes - Err(VerifyError::TooLittleVotingPower { voting_power, .. }) => voting_power, + // not enough votes + Err(VerifyError::TooLittleVotingPower { voting_power, .. }) => voting_power, - // error - Err(error) => { - error!( - "MUST_FIX: vote received could not be added: {}, vote: {}", - error, vote - ); - return VoteReceptionResult::ErrorAddingVote(error); + // error + Err(error) => { + error!( + "MUST_FIX: vote received could not be added: {}, vote: {}", + error, vote + ); + return VoteReceptionResult::ErrorAddingVote(error); + }, + } }, }; @@ -342,7 +365,7 @@ impl PendingVotes { pub fn drain_votes( &mut self, ) -> ( - Vec<(HashValue, LedgerInfoWithVerifiedSignatures)>, + Vec<(HashValue, VoteStatus)>, Option, ) { for (hash_index, _) in self.li_digest_to_votes.values() { @@ -364,7 +387,7 @@ impl PendingVotes { ( self.li_digest_to_votes .drain() - .map(|(key, (_, li))| (key, li)) + .map(|(key, (_, vote_status))| (key, vote_status)) .collect(), self.maybe_partial_2chain_tc.take(), ) @@ -385,12 +408,24 @@ fn hash_index_to_str(hash_index: usize) -> String { impl fmt::Display for PendingVotes { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - // collect votes per ledger info - let votes = self - .li_digest_to_votes - .iter() - .map(|(li_digest, (_, li))| (li_digest, li.signatures().keys().collect::>())) - .collect::>(); + write!(f, "PendingVotes: [")?; + + for (li_digest, (_, status)) in self.li_digest_to_votes.iter() { + match status { + VoteStatus::EnoughVotes(_li) => { + write!(f, "LI {} has aggregated QC", li_digest)?; + }, + VoteStatus::NotEnoughVotes(li) => { + write!( + f, + "LI {} has {} verified votes, {} unverified votes", + li_digest, + li.verified_voters().count(), + li.unverified_voters().count(), + )?; + }, + } + } // collect timeout votes let timeout_votes = self @@ -398,13 +433,6 @@ impl fmt::Display for PendingVotes { .as_ref() .map(|partial_tc| partial_tc.signers().collect::>()); - // write - write!(f, "PendingVotes: [")?; - - for (hash, authors) in votes { - write!(f, "LI {} has {} votes {:?} ", hash, authors.len(), authors)?; - } - if let Some(authors) = timeout_votes { write!(f, "{} timeout {:?}", authors.len(), authors)?; } @@ -419,13 +447,13 @@ impl fmt::Display for PendingVotes { #[cfg(test)] mod tests { - use super::{PendingVotes, VoteReceptionResult}; + use super::{PendingVotes, VoteReceptionResult, VoteStatus}; use aptos_consensus_types::{ block::block_test_utils::certificate_for_genesis, vote::Vote, vote_data::VoteData, }; - use aptos_crypto::HashValue; + use aptos_crypto::{bls12381, hash::CryptoHash, HashValue}; use aptos_types::{ - block_info::BlockInfo, ledger_info::LedgerInfo, + aggregate_signature::PartialSignatures, block_info::BlockInfo, ledger_info::LedgerInfo, validator_verifier::random_validator_verifier, }; use itertools::Itertools; @@ -450,7 +478,7 @@ mod tests { ::aptos_logger::Logger::init_for_testing(); // set up 4 validators - let (signers, validator) = random_validator_verifier(4, Some(2), false); + let (signers, validator_verifier) = random_validator_verifier(4, Some(2), false); let mut pending_votes = PendingVotes::new(); // create random vote from validator[0] @@ -459,15 +487,16 @@ mod tests { let vote_data_1_author_0 = Vote::new(vote_data_1, signers[0].author(), li1, &signers[0]).unwrap(); + vote_data_1_author_0.set_verified(); // first time a new vote is added -> VoteAdded assert_eq!( - pending_votes.insert_vote(&vote_data_1_author_0, &validator), + pending_votes.insert_vote(&vote_data_1_author_0, &validator_verifier), VoteReceptionResult::VoteAdded(1) ); // same author voting for the same thing -> DuplicateVote assert_eq!( - pending_votes.insert_vote(&vote_data_1_author_0, &validator), + pending_votes.insert_vote(&vote_data_1_author_0, &validator_verifier), VoteReceptionResult::DuplicateVote ); @@ -481,8 +510,9 @@ mod tests { &signers[0], ) .unwrap(); + vote_data_2_author_0.set_verified(); assert_eq!( - pending_votes.insert_vote(&vote_data_2_author_0, &validator), + pending_votes.insert_vote(&vote_data_2_author_0, &validator_verifier), VoteReceptionResult::EquivocateVote ); @@ -494,22 +524,160 @@ mod tests { &signers[1], ) .unwrap(); + vote_data_2_author_1.set_verified(); assert_eq!( - pending_votes.insert_vote(&vote_data_2_author_1, &validator), + pending_votes.insert_vote(&vote_data_2_author_1, &validator_verifier), VoteReceptionResult::VoteAdded(1) ); // two votes for the ledger info -> NewQuorumCertificate let vote_data_2_author_2 = Vote::new(vote_data_2, signers[2].author(), li2, &signers[2]).unwrap(); - match pending_votes.insert_vote(&vote_data_2_author_2, &validator) { + vote_data_2_author_2.set_verified(); + match pending_votes.insert_vote(&vote_data_2_author_2, &validator_verifier) { + VoteReceptionResult::NewQuorumCertificate(qc) => { + assert!(qc + .ledger_info() + .check_voting_power(&validator_verifier) + .is_ok()); + }, + _ => { + panic!("No QC formed."); + }, + }; + } + + #[test] + fn test_qc_aggregation_with_unverified_votes() { + ::aptos_logger::Logger::init_for_testing(); + + // set up 4 validators + let (signers, validator_verifier) = random_validator_verifier(7, Some(3), false); + let mut pending_votes = PendingVotes::new(); + + // create random vote from validator[0] + let mut li = random_ledger_info(); + let vote_data = random_vote_data(); + li.set_consensus_data_hash(vote_data.hash()); + let li_hash = li.hash(); + + let mut partial_sigs = PartialSignatures::empty(); + + let vote_0 = Vote::new( + vote_data.clone(), + signers[0].author(), + li.clone(), + &signers[0], + ) + .unwrap(); + + let vote_1 = Vote::new( + vote_data.clone(), + signers[1].author(), + li.clone(), + &signers[1], + ) + .unwrap(); + + let vote_2 = Vote::new_with_signature( + vote_data.clone(), + signers[2].author(), + li.clone(), + bls12381::Signature::dummy_signature(), + ); + + let vote_3 = Vote::new( + vote_data.clone(), + signers[3].author(), + li.clone(), + &signers[3], + ) + .unwrap(); + + let vote_4 = Vote::new( + vote_data.clone(), + signers[4].author(), + li.clone(), + &signers[4], + ) + .unwrap(); + + // first time a new vote is added -> VoteAdded + assert_eq!( + pending_votes.insert_vote(&vote_0, &validator_verifier), + VoteReceptionResult::VoteAdded(1) + ); + partial_sigs.add_signature(signers[0].author(), vote_0.signature().clone()); + + // same author voting for the same thing -> DuplicateVote + vote_0.set_verified(); + assert_eq!( + pending_votes.insert_vote(&vote_0, &validator_verifier), + VoteReceptionResult::DuplicateVote + ); + + assert_eq!( + pending_votes.insert_vote(&vote_1, &validator_verifier), + VoteReceptionResult::VoteAdded(2) + ); + partial_sigs.add_signature(signers[1].author(), vote_1.signature().clone()); + + assert_eq!(validator_verifier.pessimistic_verify_set().len(), 0); + + assert_eq!( + pending_votes.insert_vote(&vote_2, &validator_verifier), + VoteReceptionResult::VoteAdded(2) + ); + + assert_eq!(validator_verifier.pessimistic_verify_set().len(), 1); + let (_, vote_status) = pending_votes.li_digest_to_votes.get(&li_hash).unwrap(); + match vote_status { + VoteStatus::NotEnoughVotes(li_with_sig) => { + assert_eq!(li_with_sig.verified_voters().count(), 2); + assert_eq!(li_with_sig.unverified_voters().count(), 0); + }, + _ => { + panic!("QC should not be formed yet."); + }, + } + + partial_sigs.add_signature(signers[3].author(), vote_3.signature().clone()); + let aggregated_sig = validator_verifier + .aggregate_signatures(partial_sigs.signatures_iter()) + .unwrap(); + match pending_votes.insert_vote(&vote_3, &validator_verifier) { VoteReceptionResult::NewQuorumCertificate(qc) => { - assert!(qc.ledger_info().check_voting_power(&validator).is_ok()); + assert!(qc + .ledger_info() + .check_voting_power(&validator_verifier) + .is_ok()); + assert_eq!( + qc.ledger_info().signatures().clone(), + aggregated_sig.clone() + ); }, _ => { panic!("No QC formed."); }, }; + + match pending_votes.insert_vote(&vote_4, &validator_verifier) { + VoteReceptionResult::NewQuorumCertificate(qc) => { + assert!(qc + .ledger_info() + .check_voting_power(&validator_verifier) + .is_ok()); + assert_eq!( + qc.ledger_info().signatures().clone(), + aggregated_sig.clone() + ); + }, + _ => { + panic!("No QC formed."); + }, + }; + + assert_eq!(validator_verifier.pessimistic_verify_set().len(), 1); } #[test] @@ -517,7 +685,7 @@ mod tests { ::aptos_logger::Logger::init_for_testing(); // set up 4 validators - let (signers, validator) = random_validator_verifier(4, None, false); + let (signers, validator_verifier) = random_validator_verifier(4, None, false); let mut pending_votes = PendingVotes::new(); // submit a new vote from validator[0] -> VoteAdded @@ -525,8 +693,9 @@ mod tests { let vote0 = random_vote_data(); let mut vote0_author_0 = Vote::new(vote0, signers[0].author(), li0, &signers[0]).unwrap(); + vote0_author_0.set_verified(); assert_eq!( - pending_votes.insert_vote(&vote0_author_0, &validator), + pending_votes.insert_vote(&vote0_author_0, &validator_verifier), VoteReceptionResult::VoteAdded(1) ); @@ -536,7 +705,7 @@ mod tests { vote0_author_0.add_2chain_timeout(timeout, signature); assert_eq!( - pending_votes.insert_vote(&vote0_author_0, &validator), + pending_votes.insert_vote(&vote0_author_0, &validator_verifier), VoteReceptionResult::VoteAdded(1) ); @@ -544,8 +713,9 @@ mod tests { let li1 = random_ledger_info(); let vote1 = random_vote_data(); let mut vote1_author_1 = Vote::new(vote1, signers[1].author(), li1, &signers[1]).unwrap(); + vote1_author_1.set_verified(); assert_eq!( - pending_votes.insert_vote(&vote1_author_1, &validator), + pending_votes.insert_vote(&vote1_author_1, &validator_verifier), VoteReceptionResult::VoteAdded(1) ); @@ -553,7 +723,7 @@ mod tests { let timeout = vote1_author_1.generate_2chain_timeout(certificate_for_genesis()); let signature = timeout.sign(&signers[1]).unwrap(); vote1_author_1.add_2chain_timeout(timeout, signature); - match pending_votes.insert_vote(&vote1_author_1, &validator) { + match pending_votes.insert_vote(&vote1_author_1, &validator_verifier) { VoteReceptionResult::EchoTimeout(voting_power) => { assert_eq!(voting_power, 2); }, @@ -570,14 +740,16 @@ mod tests { let timeout = vote2_author_2.generate_2chain_timeout(certificate_for_genesis()); let signature = timeout.sign(&signers[2]).unwrap(); vote2_author_2.add_2chain_timeout(timeout, signature); - - match pending_votes.insert_vote(&vote2_author_2, &validator) { + vote2_author_2.set_verified(); + match pending_votes.insert_vote(&vote2_author_2, &validator_verifier) { VoteReceptionResult::New2ChainTimeoutCertificate(tc) => { - assert!(validator + assert!(validator_verifier .check_voting_power( tc.signatures_with_rounds() .get_voters( - &validator.get_ordered_account_addresses_iter().collect_vec() + &validator_verifier + .get_ordered_account_addresses_iter() + .collect_vec() ) .iter(), true diff --git a/consensus/src/round_manager.rs b/consensus/src/round_manager.rs index 0cb07ce9b9f24..fedfd10e30c53 100644 --- a/consensus/src/round_manager.rs +++ b/consensus/src/round_manager.rs @@ -26,7 +26,7 @@ use crate::{ network::NetworkSender, network_interface::ConsensusMsg, pending_order_votes::{OrderVoteReceptionResult, PendingOrderVotes}, - pending_votes::VoteReceptionResult, + pending_votes::{VoteReceptionResult, VoteStatus}, persistent_liveness_storage::PersistentLivenessStorage, quorum_store::types::BatchMsg, rand::rand_gen::types::{FastShare, RandConfig, Share, TShare}, @@ -39,7 +39,9 @@ use aptos_consensus_types::{ block::Block, block_data::BlockType, common::{Author, Round}, + order_vote::OrderVote, order_vote_msg::OrderVoteMsg, + pipelined_block::PipelinedBlock, proof_of_store::{ProofCache, ProofOfStoreMsg, SignedBatchInfoMsg}, proposal_msg::ProposalMsg, quorum_cert::QuorumCert, @@ -78,7 +80,7 @@ use tokio::{ time::{sleep, Instant}, }; -#[derive(Serialize, Clone)] +#[derive(Debug, Serialize, Clone)] pub enum UnverifiedEvent { ProposalMsg(Box), VoteMsg(Box), @@ -435,10 +437,17 @@ impl RoundManager { let prev_round_votes_for_li = new_round_event .prev_round_votes .iter() - .map(|(_, li_with_sig)| { - let (voting_power, votes): (Vec<_>, Vec<_>) = li_with_sig - .signatures() - .keys() + .map(|(_, vote_status)| { + let all_voters = match vote_status { + VoteStatus::EnoughVotes(li_with_sig) => epoch_state + .verifier + .aggregate_signature_authors(li_with_sig.signatures()), + VoteStatus::NotEnoughVotes(li_with_sig) => { + li_with_sig.all_voters().collect::>() + }, + }; + let (voting_power, votes): (Vec<_>, Vec<_>) = all_voters + .into_iter() .map(|author| { epoch_state .verifier @@ -505,8 +514,9 @@ impl RoundManager { &self, new_round_event: NewRoundEvent, ) -> anyhow::Result { + let epoch = self.epoch_state.epoch; Self::generate_proposal( - self.epoch_state().epoch, + epoch, new_round_event, self.block_store.sync_info(), self.network.clone(), @@ -614,17 +624,15 @@ impl RoundManager { ); // Some information in SyncInfo is ahead of what we have locally. // First verify the SyncInfo (didn't verify it in the yet). - sync_info - .verify(&self.epoch_state().verifier) - .map_err(|e| { - error!( - SecurityEvent::InvalidSyncInfoMsg, - sync_info = sync_info, - remote_peer = author, - error = ?e, - ); - VerifyError::from(e) - })?; + sync_info.verify(&self.epoch_state.verifier).map_err(|e| { + error!( + SecurityEvent::InvalidSyncInfoMsg, + sync_info = sync_info, + remote_peer = author, + error = ?e, + ); + VerifyError::from(e) + })?; SYNC_INFO_RECEIVED_WITH_NEWER_CERT.inc(); let result = self .block_store @@ -721,7 +729,7 @@ impl RoundManager { timeout } else { let timeout = TwoChainTimeout::new( - self.epoch_state().epoch, + self.epoch_state.epoch, round, self.block_store.highest_quorum_cert().as_ref().clone(), ); @@ -1045,12 +1053,28 @@ impl RoundManager { } } - pub async fn process_verified_proposal(&mut self, proposal: Block) -> anyhow::Result<()> { - let proposal_round = proposal.round(); + async fn create_vote(&mut self, proposal: Block) -> anyhow::Result { let vote = self .vote_block(proposal) .await .context("[RoundManager] Process proposal")?; + + fail_point!("consensus::create_invalid_vote", |_| { + use aptos_crypto::bls12381; + let faulty_vote = Vote::new_with_signature( + vote.vote_data().clone(), + vote.author(), + vote.ledger_info().clone(), + bls12381::Signature::dummy_signature(), + ); + Ok(faulty_vote) + }); + Ok(vote) + } + + pub async fn process_verified_proposal(&mut self, proposal: Block) -> anyhow::Result<()> { + let proposal_round = proposal.round(); + let vote = self.create_vote(proposal).await?; self.round_state.record_vote(vote.clone()); let vote_msg = VoteMsg::new(vote.clone(), self.block_store.sync_info()); @@ -1153,7 +1177,7 @@ impl RoundManager { let start = Instant::now(); order_vote_msg .quorum_cert() - .verify(&self.epoch_state().verifier) + .verify(&self.epoch_state.verifier) .context("[OrderVoteMsg QuorumCert verification failed")?; counters::VERIFY_MSG .with_label_values(&["order_vote_qc"]) @@ -1195,6 +1219,33 @@ impl RoundManager { Ok(()) } + async fn create_order_vote( + &mut self, + block: Arc, + qc: Arc, + ) -> anyhow::Result { + let order_vote_proposal = block.order_vote_proposal(qc); + let order_vote_result = self + .safety_rules + .lock() + .construct_and_sign_order_vote(&order_vote_proposal); + let order_vote = order_vote_result.context(format!( + "[RoundManager] SafetyRules Rejected {} for order vote", + block.block() + ))?; + + fail_point!("consensus::create_invalid_order_vote", |_| { + use aptos_crypto::bls12381; + let faulty_order_vote = OrderVote::new_with_signature( + order_vote.author(), + order_vote.ledger_info().clone(), + bls12381::Signature::dummy_signature(), + ); + Ok(faulty_order_vote) + }); + Ok(order_vote) + } + async fn broadcast_order_vote( &mut self, vote: &Vote, @@ -1202,22 +1253,16 @@ impl RoundManager { ) -> anyhow::Result<()> { if let Some(proposed_block) = self.block_store.get_block(vote.vote_data().proposed().id()) { // Generate an order vote with ledger_info = proposed_block - let order_vote_proposal = proposed_block.order_vote_proposal(qc.clone()); - let order_vote_result = self - .safety_rules - .lock() - .construct_and_sign_order_vote(&order_vote_proposal); - let order_vote = order_vote_result.context(format!( - "[RoundManager] SafetyRules Rejected {} for order vote", - proposed_block.block() - ))?; + let order_vote = self + .create_order_vote(proposed_block.clone(), qc.clone()) + .await?; if !proposed_block.block().is_nil_block() { observe_block( proposed_block.block().timestamp_usecs(), BlockStage::ORDER_VOTED, ); } - let order_vote_msg = OrderVoteMsg::new(order_vote.clone(), qc.as_ref().clone()); + let order_vote_msg = OrderVoteMsg::new(order_vote, qc.as_ref().clone()); info!( self.new_log(LogEvent::BroadcastOrderVote), "{}", order_vote_msg @@ -1579,10 +1624,6 @@ impl RoundManager { self.safety_rules = safety_rules } - pub fn epoch_state(&self) -> &EpochState { - &self.epoch_state - } - pub fn round_state(&self) -> &RoundState { &self.round_state } @@ -1591,7 +1632,7 @@ impl RoundManager { Self::new_log_with_round_epoch( event, self.round_state().current_round(), - self.epoch_state().epoch, + self.epoch_state.epoch, ) } @@ -1610,7 +1651,7 @@ impl RoundManager { mut buffered_proposal_rx: aptos_channel::Receiver, close_rx: oneshot::Receiver>, ) { - info!(epoch = self.epoch_state().epoch, "RoundManager started"); + info!(epoch = self.epoch_state.epoch, "RoundManager started"); let mut close_rx = close_rx.into_stream(); loop { tokio::select! { @@ -1620,7 +1661,7 @@ impl RoundManager { ack_sender.send(()).expect("[RoundManager] Fail to ack shutdown"); } break; - }, + } proposal = buffered_proposal_rx.select_next_some() => { let mut proposals = vec![proposal]; while let Some(Some(proposal)) = buffered_proposal_rx.next().now_or_never() { @@ -1717,7 +1758,7 @@ impl RoundManager { }, } } - info!(epoch = self.epoch_state().epoch, "RoundManager stopped"); + info!(epoch = self.epoch_state.epoch, "RoundManager stopped"); } #[cfg(feature = "failpoints")] diff --git a/consensus/src/round_manager_test.rs b/consensus/src/round_manager_test.rs index d06636a66d458..b17787865dd9e 100644 --- a/consensus/src/round_manager_test.rs +++ b/consensus/src/round_manager_test.rs @@ -646,6 +646,7 @@ fn process_and_vote_on_proposal( info!("Processing votes on node {}", proposer_node.identity_desc()); if process_votes { for vote_msg in votes { + vote_msg.vote().set_verified(); timed_block_on( runtime, proposer_node.round_manager.process_vote_msg(vote_msg), @@ -697,6 +698,7 @@ fn new_round_on_quorum_cert() { .await .unwrap(); let vote_msg = node.next_vote().await; + vote_msg.vote().set_verified(); // Adding vote to form a QC node.round_manager.process_vote_msg(vote_msg).await.unwrap(); @@ -1641,7 +1643,7 @@ fn sync_on_partial_newer_sync_info() { runtime.spawn(playground.start()); timed_block_on(&runtime, async { // commit block 1 after 4 rounds - for _ in 1..=4 { + for i in 1..=4 { let proposal_msg = node.next_proposal().await; node.round_manager @@ -1649,6 +1651,9 @@ fn sync_on_partial_newer_sync_info() { .await .unwrap(); let vote_msg = node.next_vote().await; + if i < 2 { + vote_msg.vote().set_verified(); + } // Adding vote to form a QC node.round_manager.process_vote_msg(vote_msg).await.unwrap(); } @@ -1745,6 +1750,7 @@ fn safety_rules_crash() { // sign proposal reset_safety_rules(&mut node); + vote_msg.vote().set_verified(); node.round_manager.process_vote_msg(vote_msg).await.unwrap(); } @@ -1784,6 +1790,9 @@ fn echo_timeout() { // node 0 doesn't timeout and should echo the timeout after 2 timeout message for i in 0..3 { let timeout_vote = node_0.next_vote().await; + if i < 2 { + timeout_vote.vote().set_verified(); + } let result = node_0.round_manager.process_vote_msg(timeout_vote).await; // first and third message should not timeout if i == 0 || i == 2 { @@ -1797,8 +1806,13 @@ fn echo_timeout() { let node_1 = &mut nodes[1]; // it receives 4 timeout messages (1 from each) and doesn't echo since it already timeout - for _ in 0..4 { + for i in 0..4 { let timeout_vote = node_1.next_vote().await; + // Verifying only some vote messages to check that round manager can accept both + // verified and unverified votes + if i < 2 { + timeout_vote.vote().set_verified(); + } node_1 .round_manager .process_vote_msg(timeout_vote) @@ -2120,6 +2134,7 @@ pub fn forking_retrieval_test() { } let vote_msg_on_timeout = node.next_vote().await; + vote_msg_on_timeout.vote().set_verified(); assert!(vote_msg_on_timeout.vote().is_timeout()); if node.id != behind_node { let result = node diff --git a/testsuite/smoke-test/src/consensus/consensus_fault_tolerance.rs b/testsuite/smoke-test/src/consensus/consensus_fault_tolerance.rs index 30cb64a7ade9f..834e82a7c8daf 100644 --- a/testsuite/smoke-test/src/consensus/consensus_fault_tolerance.rs +++ b/testsuite/smoke-test/src/consensus/consensus_fault_tolerance.rs @@ -213,6 +213,54 @@ async fn test_no_failures() { .unwrap(); } +#[tokio::test] +async fn test_faulty_votes() { + let num_validators = 7; + + let swarm = create_swarm(num_validators, 1).await; + + let (validator_clients, public_info) = { + ( + swarm.get_validator_clients_with_names(), + swarm.aptos_public_info(), + ) + }; + test_consensus_fault_tolerance( + validator_clients, + public_info, + 3, + 5.0, + 1, + Box::new(FailPointFailureInjection::new(Box::new(move |cycle, _| { + ( + vec![ + ( + cycle % num_validators, + "consensus::create_invalid_vote".to_string(), + format!("{}%return", 50), + ), + ( + (cycle + 1) % num_validators, + "consensus::create_invalid_order_vote".to_string(), + format!("{}%return", 50), + ), + ], + true, + ) + }))), + Box::new( + move |_, executed_epochs, executed_rounds, executed_transactions, _, _| { + successful_criteria(executed_epochs, executed_rounds, executed_transactions); + Ok(()) + }, + ), + true, + false, + ) + .await + .unwrap(); +} + #[tokio::test] async fn test_ordered_only_cert() { let num_validators = 3; diff --git a/types/src/ledger_info.rs b/types/src/ledger_info.rs index b72cd7634b001..840c2c2f62793 100644 --- a/types/src/ledger_info.rs +++ b/types/src/ledger_info.rs @@ -14,16 +14,19 @@ use crate::{ }; use aptos_crypto::{bls12381, hash::HashValue}; use aptos_crypto_derive::{BCSCryptoHash, CryptoHasher}; +use derivative::Derivative; #[cfg(any(test, feature = "fuzzing"))] use proptest_derive::Arbitrary; -use rayon::iter::{IntoParallelIterator, ParallelIterator}; use serde::{Deserialize, Serialize}; use std::{ collections::BTreeMap, fmt::{Display, Formatter}, mem, ops::{Deref, DerefMut}, - sync::Arc, + sync::{ + atomic::{AtomicBool, Ordering}, + Arc, + }, }; /// This structure serves a dual purpose. @@ -376,9 +379,54 @@ impl LedgerInfoWithVerifiedSignatures { } } -pub enum SignatureWithStatus { - Verified(bls12381::Signature), - Unverified(bls12381::Signature), +#[derive(Clone, Debug, Derivative)] +#[derivative(PartialEq, Eq)] +pub struct SignatureWithStatus { + signature: bls12381::Signature, + #[derivative(PartialEq = "ignore")] + // false if the signature not verified. + // true if the signature is verified. + verification_status: Arc, +} + +impl SignatureWithStatus { + pub fn set_verified(&self) { + self.verification_status.store(true, Ordering::SeqCst); + } + + pub fn signature(&self) -> &bls12381::Signature { + &self.signature + } + + pub fn from(signature: bls12381::Signature) -> Self { + Self { + signature, + verification_status: Arc::new(AtomicBool::new(false)), + } + } + + pub fn is_verified(&self) -> bool { + self.verification_status.load(Ordering::SeqCst) + } +} + +impl Serialize for SignatureWithStatus { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.signature.serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for SignatureWithStatus { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let signature = bls12381::Signature::deserialize(deserializer)?; + Ok(SignatureWithStatus::from(signature)) + } } /// This data structure is used to support the optimistic signature verification feature. @@ -390,10 +438,7 @@ pub enum SignatureWithStatus { #[derive(Clone, Debug, Eq, PartialEq)] pub struct LedgerInfoWithUnverifiedSignatures { ledger_info: LedgerInfo, - // These signatures are not yet verified. For efficiency, once enough unverified signatures are collected, - // they will be aggregated and verified. - unverified_signatures: PartialSignatures, - verified_signatures: PartialSignatures, + signatures: BTreeMap, } impl Display for LedgerInfoWithUnverifiedSignatures { @@ -406,8 +451,7 @@ impl LedgerInfoWithUnverifiedSignatures { pub fn new(ledger_info: LedgerInfo) -> Self { Self { ledger_info, - unverified_signatures: PartialSignatures::empty(), - verified_signatures: PartialSignatures::empty(), + signatures: BTreeMap::default(), } } @@ -415,53 +459,32 @@ impl LedgerInfoWithUnverifiedSignatures { self.ledger_info.commit_info() } - fn add_verified_signature( - &mut self, - validator: AccountAddress, - signature: bls12381::Signature, - ) { - self.verified_signatures.add_signature(validator, signature); - self.unverified_signatures.remove_signature(validator); - } - - fn add_unverified_signature( - &mut self, - validator: AccountAddress, - signature: bls12381::Signature, - ) { - if self.verified_signatures.contains_voter(&validator) { - return; - } - self.unverified_signatures - .add_signature(validator, signature); - } - - pub fn add_signature( - &mut self, - validator: AccountAddress, - signature_with_status: SignatureWithStatus, - ) { - match signature_with_status { - SignatureWithStatus::Verified(signature) => { - self.add_verified_signature(validator, signature) - }, - SignatureWithStatus::Unverified(signature) => { - self.add_unverified_signature(validator, signature) - }, - }; + pub fn add_signature(&mut self, validator: AccountAddress, signature: &SignatureWithStatus) { + self.signatures.insert(validator, signature.clone()); } pub fn verified_voters(&self) -> impl Iterator { - self.verified_signatures.signatures().keys() + self.signatures.iter().filter_map(|(voter, signature)| { + if signature.is_verified() { + Some(voter) + } else { + None + } + }) } pub fn unverified_voters(&self) -> impl Iterator { - self.unverified_signatures.signatures().keys() + self.signatures.iter().filter_map(|(voter, signature)| { + if signature.is_verified() { + None + } else { + Some(voter) + } + }) } - // Collecting all the authors from verified signatures, unverified signatures and the aggregated signature. pub fn all_voters(&self) -> impl Iterator { - self.verified_voters().chain(self.unverified_voters()) + self.signatures.keys() } pub fn check_voting_power( @@ -475,66 +498,43 @@ impl LedgerInfoWithUnverifiedSignatures { fn try_aggregate( &mut self, - epoch_state: &EpochState, + verifier: &ValidatorVerifier, ) -> Result { - self.check_voting_power(&epoch_state.verifier, true)?; + self.check_voting_power(verifier, true)?; let all_signatures = self - .verified_signatures - .signatures_iter() - .chain(self.unverified_signatures.signatures_iter()); - - epoch_state.verifier.aggregate_signatures(all_signatures) - } - - /// Merge unverified signatures into verified signatures if they are valid. - fn merge_signatures(&mut self, verifier: &ValidatorVerifier, need_verify: bool) { - let unverified_signatures = - mem::replace(&mut self.unverified_signatures, PartialSignatures::empty()).unpack(); - let valid_signatures: Vec<_> = unverified_signatures - .into_par_iter() - .flat_map(|(account_address, signature)| { - if !need_verify - || verifier - .verify(account_address, self.ledger_info(), &signature) - .is_ok() - { - Some((account_address, signature)) - } else { - verifier.add_pessimistic_verify_set(account_address); - None - } - }) - .collect(); - for (account_address, signature) in valid_signatures { - self.verified_signatures - .add_signature(account_address, signature); - } + .signatures + .iter() + .map(|(voter, sig)| (voter, sig.signature())); + verifier.aggregate_signatures(all_signatures) + } + + fn filter_invalid_signatures(&mut self, verifier: &ValidatorVerifier) { + let signatures = mem::take(&mut self.signatures); + self.signatures = verifier.filter_invalid_signatures(self.ledger_info(), signatures); } /// Try to aggregate all signatures if the voting power is enough. If the aggregated signature is /// valid, return the LedgerInfoWithSignatures. Also merge valid unverified signatures into verified. pub fn aggregate_and_verify( &mut self, - epoch_state: Arc, + verifier: &ValidatorVerifier, ) -> Result { - let aggregated_sig = self.try_aggregate(&epoch_state)?; + let aggregated_sig = self.try_aggregate(verifier)?; - match epoch_state - .verifier - .verify_multi_signatures(self.ledger_info(), &aggregated_sig) - { + match verifier.verify_multi_signatures(self.ledger_info(), &aggregated_sig) { Ok(_) => { - self.merge_signatures(&epoch_state.verifier, false); + // We are not marking all the signatures as "verified" here, as two malicious + // voters can collude and create a valid aggregated signature. Ok(LedgerInfoWithSignatures::new( self.ledger_info.clone(), aggregated_sig, )) }, Err(_) => { - self.merge_signatures(&epoch_state.verifier, true); + self.filter_invalid_signatures(verifier); - let aggregate_sig = self.try_aggregate(&epoch_state)?; + let aggregate_sig = self.try_aggregate(verifier)?; Ok(LedgerInfoWithSignatures::new( self.ledger_info.clone(), aggregate_sig, @@ -593,6 +593,54 @@ impl Arbitrary for LedgerInfoWithV0 { mod tests { use super::*; use crate::{validator_signer::ValidatorSigner, validator_verifier::ValidatorConsensusInfo}; + // Write a test case to serialize and deserialize SignatureWithStatus + #[test] + fn test_signature_with_status_bcs() { + let signature = bls12381::Signature::dummy_signature(); + let signature_with_status_1 = SignatureWithStatus { + signature: signature.clone(), + verification_status: Arc::new(AtomicBool::new(true)), + }; + let signature_with_status_2 = SignatureWithStatus { + signature: signature.clone(), + verification_status: Arc::new(AtomicBool::new(false)), + }; + let serialized_signature_with_status_1 = + bcs::to_bytes(&signature_with_status_1).expect("Failed to serialize signature"); + let serialized_signature_with_status_2 = + bcs::to_bytes(&signature_with_status_2).expect("Failed to serialize signature"); + assert!(serialized_signature_with_status_1 == serialized_signature_with_status_2); + + let deserialized_signature_with_status: SignatureWithStatus = + bcs::from_bytes(&serialized_signature_with_status_1) + .expect("Failed to deserialize signature"); + assert_eq!(*deserialized_signature_with_status.signature(), signature); + assert!(!deserialized_signature_with_status.is_verified()); + } + + #[test] + fn test_signature_with_status_serde() { + let signature = bls12381::Signature::dummy_signature(); + let signature_with_status_1 = SignatureWithStatus { + signature: signature.clone(), + verification_status: Arc::new(AtomicBool::new(true)), + }; + let signature_with_status_2 = SignatureWithStatus { + signature: signature.clone(), + verification_status: Arc::new(AtomicBool::new(false)), + }; + let serialized_signature_with_status_1 = + serde_json::to_string(&signature_with_status_1).expect("Failed to serialize signature"); + let serialized_signature_with_status_2 = + serde_json::to_string(&signature_with_status_2).expect("Failed to serialize signature"); + assert!(serialized_signature_with_status_1 == serialized_signature_with_status_2); + + let deserialized_signature_with_status: SignatureWithStatus = + serde_json::from_str(&serialized_signature_with_status_1) + .expect("Failed to deserialize signature"); + assert_eq!(*deserialized_signature_with_status.signature(), signature); + assert!(!deserialized_signature_with_status.is_verified()); + } #[test] fn test_signatures_hash() { @@ -653,7 +701,7 @@ mod tests { } #[test] - fn test_ledger_info_with_mixed_signatures() { + fn test_ledger_info_with_unverified_signatures() { let ledger_info = LedgerInfo::new(BlockInfo::empty(), HashValue::random()); const NUM_SIGNERS: u8 = 7; // Generate NUM_SIGNERS random signers. @@ -673,214 +721,217 @@ mod tests { let validator_verifier = ValidatorVerifier::new_with_quorum_voting_power(validator_infos, 5) .expect("Incorrect quorum size."); - let epoch_state = Arc::new(EpochState::new(10, validator_verifier)); - let mut ledger_info_with_mixed_signatures = + let mut ledger_info_with_unverified_signatures = LedgerInfoWithUnverifiedSignatures::new(ledger_info.clone()); let mut partial_sig = PartialSignatures::empty(); - ledger_info_with_mixed_signatures.add_signature( - validator_signers[0].author(), - SignatureWithStatus::Verified(validator_signers[0].sign(&ledger_info).unwrap()), - ); + let sig = SignatureWithStatus::from(validator_signers[0].sign(&ledger_info).unwrap()); + sig.set_verified(); + ledger_info_with_unverified_signatures.add_signature(validator_signers[0].author(), &sig); + partial_sig.add_signature( validator_signers[0].author(), validator_signers[0].sign(&ledger_info).unwrap(), ); - ledger_info_with_mixed_signatures.add_signature( + ledger_info_with_unverified_signatures.add_signature( validator_signers[1].author(), - SignatureWithStatus::Unverified(validator_signers[1].sign(&ledger_info).unwrap()), + &SignatureWithStatus::from(validator_signers[1].sign(&ledger_info).unwrap()), ); partial_sig.add_signature( validator_signers[1].author(), validator_signers[1].sign(&ledger_info).unwrap(), ); - ledger_info_with_mixed_signatures.add_signature( - validator_signers[2].author(), - SignatureWithStatus::Verified(validator_signers[2].sign(&ledger_info).unwrap()), - ); + let sig2 = SignatureWithStatus::from(validator_signers[2].sign(&ledger_info).unwrap()); + sig2.set_verified(); + ledger_info_with_unverified_signatures.add_signature(validator_signers[2].author(), &sig2); partial_sig.add_signature( validator_signers[2].author(), validator_signers[2].sign(&ledger_info).unwrap(), ); - ledger_info_with_mixed_signatures.add_signature( + ledger_info_with_unverified_signatures.add_signature( validator_signers[3].author(), - SignatureWithStatus::Unverified(validator_signers[3].sign(&ledger_info).unwrap()), + &SignatureWithStatus::from(validator_signers[3].sign(&ledger_info).unwrap()), ); partial_sig.add_signature( validator_signers[3].author(), validator_signers[3].sign(&ledger_info).unwrap(), ); - assert_eq!(ledger_info_with_mixed_signatures.all_voters().count(), 4); assert_eq!( - ledger_info_with_mixed_signatures - .unverified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures.all_voters().count(), + 4 + ); + assert_eq!( + ledger_info_with_unverified_signatures + .unverified_voters() + .count(), 2 ); assert_eq!( - ledger_info_with_mixed_signatures - .verified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures + .verified_voters() + .count(), 2 ); assert_eq!( - ledger_info_with_mixed_signatures.check_voting_power(&epoch_state.verifier, true), + ledger_info_with_unverified_signatures.check_voting_power(&validator_verifier, true), Err(VerifyError::TooLittleVotingPower { voting_power: 4, expected_voting_power: 5 }) ); - ledger_info_with_mixed_signatures.add_signature( + ledger_info_with_unverified_signatures.add_signature( validator_signers[4].author(), - SignatureWithStatus::Unverified(bls12381::Signature::dummy_signature()), + &SignatureWithStatus::from(bls12381::Signature::dummy_signature()), ); - assert_eq!(ledger_info_with_mixed_signatures.all_voters().count(), 5); assert_eq!( - ledger_info_with_mixed_signatures - .unverified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures.all_voters().count(), + 5 + ); + assert_eq!( + ledger_info_with_unverified_signatures + .unverified_voters() + .count(), 3 ); assert_eq!( - ledger_info_with_mixed_signatures - .verified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures + .verified_voters() + .count(), 2 ); assert_eq!( - ledger_info_with_mixed_signatures - .check_voting_power(&epoch_state.verifier, true) + ledger_info_with_unverified_signatures + .check_voting_power(&validator_verifier, true) .unwrap(), 5 ); assert_eq!( - ledger_info_with_mixed_signatures.aggregate_and_verify(epoch_state.clone()), + ledger_info_with_unverified_signatures.aggregate_and_verify(&validator_verifier), Err(VerifyError::TooLittleVotingPower { voting_power: 4, expected_voting_power: 5 }) ); assert_eq!( - ledger_info_with_mixed_signatures - .unverified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures + .unverified_voters() + .count(), 0 ); assert_eq!( - ledger_info_with_mixed_signatures - .verified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures + .verified_voters() + .count(), + 4 + ); + assert_eq!( + ledger_info_with_unverified_signatures.all_voters().count(), 4 ); - assert_eq!(ledger_info_with_mixed_signatures.all_voters().count(), 4); - assert_eq!(epoch_state.verifier.pessimistic_verify_set().len(), 1); + assert_eq!(validator_verifier.pessimistic_verify_set().len(), 1); - ledger_info_with_mixed_signatures.add_signature( + ledger_info_with_unverified_signatures.add_signature( validator_signers[5].author(), - SignatureWithStatus::Unverified(validator_signers[5].sign(&ledger_info).unwrap()), + &SignatureWithStatus::from(validator_signers[5].sign(&ledger_info).unwrap()), ); partial_sig.add_signature( validator_signers[5].author(), validator_signers[5].sign(&ledger_info).unwrap(), ); - assert_eq!(ledger_info_with_mixed_signatures.all_voters().count(), 5); assert_eq!( - ledger_info_with_mixed_signatures - .unverified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures.all_voters().count(), + 5 + ); + assert_eq!( + ledger_info_with_unverified_signatures + .unverified_voters() + .count(), 1 ); assert_eq!( - ledger_info_with_mixed_signatures - .verified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures + .verified_voters() + .count(), 4 ); assert_eq!( - ledger_info_with_mixed_signatures - .check_voting_power(&epoch_state.verifier, true) + ledger_info_with_unverified_signatures + .check_voting_power(&validator_verifier, true) .unwrap(), 5 ); let aggregate_sig = LedgerInfoWithSignatures::new( ledger_info.clone(), - epoch_state - .verifier + validator_verifier .aggregate_signatures(partial_sig.signatures_iter()) .unwrap(), ); assert_eq!( - ledger_info_with_mixed_signatures - .aggregate_and_verify(epoch_state.clone()) + ledger_info_with_unverified_signatures + .aggregate_and_verify(&validator_verifier) .unwrap(), aggregate_sig ); assert_eq!( - ledger_info_with_mixed_signatures - .unverified_signatures - .signatures() - .len(), - 0 + ledger_info_with_unverified_signatures + .unverified_voters() + .count(), + 1 ); assert_eq!( - ledger_info_with_mixed_signatures - .verified_signatures - .signatures() - .len(), - 5 + ledger_info_with_unverified_signatures + .verified_voters() + .count(), + 4 ); - assert_eq!(epoch_state.verifier.pessimistic_verify_set().len(), 1); + assert_eq!(validator_verifier.pessimistic_verify_set().len(), 1); - ledger_info_with_mixed_signatures.add_signature( + ledger_info_with_unverified_signatures.add_signature( validator_signers[6].author(), - SignatureWithStatus::Unverified(bls12381::Signature::dummy_signature()), + &SignatureWithStatus::from(bls12381::Signature::dummy_signature()), ); - assert_eq!(ledger_info_with_mixed_signatures.all_voters().count(), 6); assert_eq!( - ledger_info_with_mixed_signatures - .check_voting_power(&epoch_state.verifier, true) + ledger_info_with_unverified_signatures.all_voters().count(), + 6 + ); + assert_eq!( + ledger_info_with_unverified_signatures + .check_voting_power(&validator_verifier, true) .unwrap(), 6 ); assert_eq!( - ledger_info_with_mixed_signatures - .aggregate_and_verify(epoch_state.clone()) + ledger_info_with_unverified_signatures + .aggregate_and_verify(&validator_verifier) .unwrap(), aggregate_sig ); assert_eq!( - ledger_info_with_mixed_signatures - .unverified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures + .unverified_voters() + .count(), 0 ); assert_eq!( - ledger_info_with_mixed_signatures - .verified_signatures - .signatures() - .len(), + ledger_info_with_unverified_signatures + .verified_voters() + .count(), + 5 + ); + assert_eq!( + ledger_info_with_unverified_signatures.all_voters().count(), 5 ); - assert_eq!(ledger_info_with_mixed_signatures.all_voters().count(), 5); - assert_eq!(epoch_state.verifier.pessimistic_verify_set().len(), 2); + assert_eq!(validator_verifier.pessimistic_verify_set().len(), 2); } } diff --git a/types/src/validator_verifier.rs b/types/src/validator_verifier.rs index 12500f29000eb..95ebfa637e16f 100644 --- a/types/src/validator_verifier.rs +++ b/types/src/validator_verifier.rs @@ -6,7 +6,7 @@ use crate::validator_signer::ValidatorSigner; use crate::{ account_address::AccountAddress, aggregate_signature::AggregateSignature, - on_chain_config::ValidatorSet, + ledger_info::SignatureWithStatus, on_chain_config::ValidatorSet, }; use anyhow::{ensure, Result}; use aptos_bitvec::BitVec; @@ -21,6 +21,7 @@ use derivative::Derivative; use itertools::Itertools; #[cfg(any(test, feature = "fuzzing"))] use proptest_derive::Arbitrary; +use rayon::iter::{IndexedParallelIterator, IntoParallelIterator, ParallelIterator}; use serde::{Deserialize, Deserializer, Serialize}; use std::{ collections::{BTreeMap, HashMap}, @@ -154,6 +155,10 @@ pub struct ValidatorVerifier { #[serde(skip)] #[derivative(PartialEq = "ignore")] pessimistic_verify_set: DashSet, + /// This is the feature flag indicating whether the optimistic signature verification feature is enabled. + #[serde(skip)] + #[derivative(PartialEq = "ignore")] + optimistic_sig_verification: bool, } /// Reconstruct fields from the raw data upon deserialization. @@ -193,6 +198,7 @@ impl ValidatorVerifier { total_voting_power, address_to_validator_index, pessimistic_verify_set: DashSet::new(), + optimistic_sig_verification: false, } } @@ -228,6 +234,10 @@ impl ValidatorVerifier { )) } + pub fn set_optimistic_sig_verification_flag(&mut self, flag: bool) { + self.optimistic_sig_verification = flag; + } + pub fn add_pessimistic_verify_set(&self, author: AccountAddress) { self.pessimistic_verify_set.insert(author); } @@ -257,6 +267,47 @@ impl ValidatorVerifier { } } + pub fn optimistic_verify( + &self, + author: AccountAddress, + message: &T, + signature_with_status: &SignatureWithStatus, + ) -> std::result::Result<(), VerifyError> { + if (!self.optimistic_sig_verification || self.pessimistic_verify_set.contains(&author)) + && !signature_with_status.is_verified() + { + self.verify(author, message, signature_with_status.signature())?; + signature_with_status.set_verified(); + } + Ok(()) + } + + pub fn filter_invalid_signatures( + &self, + message: &T, + signatures: BTreeMap, + ) -> BTreeMap { + signatures + .into_iter() + .collect_vec() + .into_par_iter() + .with_min_len(4) // At least 4 signatures are verified in each task + .filter_map(|(account_address, signature)| { + if signature.is_verified() + || self + .verify(account_address, message, signature.signature()) + .is_ok() + { + signature.set_verified(); + Some((account_address, signature)) + } else { + self.add_pessimistic_verify_set(account_address); + None + } + }) + .collect() + } + // Generates a multi signature or aggregate signature // from partial signatures as well as returns the aggregated pub key along with // list of pub keys used in signature aggregation. @@ -426,6 +477,19 @@ impl ValidatorVerifier { Ok(aggregated_voting_power) } + pub fn aggregate_signature_authors( + &self, + aggregated_signature: &AggregateSignature, + ) -> Vec<&AccountAddress> { + let mut authors = vec![]; + for index in aggregated_signature.get_signers_bitvec().iter_ones() { + if let Some(validator) = self.validator_infos.get(index) { + authors.push(&validator.address); + } + } + authors + } + /// Returns the public key for this address. pub fn get_public_key(&self, author: &AccountAddress) -> Option { self.address_to_validator_index