Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimistic signature verification for votes and order votes #14642

Merged
merged 50 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 49 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
c906718
Deprecate delayed QC aggregate msg
vusirikala Sep 15, 2024
c4c2cdd
Ledger info with mixed signatures
vusirikala Sep 15, 2024
08f45c9
Minor change
vusirikala Sep 15, 2024
1e85a83
Optimistic signature verification for votes and order votes
vusirikala Sep 15, 2024
8d197db
Resetting minor changes
vusirikala Sep 15, 2024
9a43029
Resetting minor changes
vusirikala Sep 15, 2024
2237342
Minor change
vusirikala Sep 15, 2024
1df442b
Addressing PR comments
vusirikala Sep 17, 2024
6fce605
Merge branch 'satya/ledger_info_with_mixed_signatures' into satya/osv…
vusirikala Sep 17, 2024
1e3b58d
Addressing PR comments
vusirikala Sep 17, 2024
f87fb36
Addressing PR comments
vusirikala Sep 17, 2024
637311b
Addressing PR comments
vusirikala Sep 17, 2024
e744449
Addressing PR comments
vusirikala Sep 18, 2024
cd56587
Addressing PR comments
vusirikala Sep 18, 2024
16370eb
Addressing PR comments
vusirikala Sep 18, 2024
a7b5e82
Minor changes
vusirikala Sep 18, 2024
2e46bb3
Minor change
vusirikala Sep 18, 2024
a2ebb6c
Merge branch 'main' into satya/ledger_info_with_mixed_signatures
vusirikala Sep 18, 2024
065d760
Changing names
vusirikala Sep 18, 2024
5ed8ad5
Addressing PR comments
vusirikala Sep 19, 2024
3be24e2
Merge branch 'satya/ledger_info_with_mixed_signatures' into satya/osv…
vusirikala Sep 19, 2024
541d58d
comments
Sep 21, 2024
20cdb9c
Fixing typos
vusirikala Sep 23, 2024
ffb1e22
Rust lint
vusirikala Sep 23, 2024
fb72780
Changed to VerificationStatus
vusirikala Sep 23, 2024
5f8f183
Merge branch 'satya/ledger_info_with_mixed_signatures' into satya/osv…
vusirikala Sep 23, 2024
25e9031
Fixing errors with rebasing
vusirikala Sep 23, 2024
85c7051
Using signature with status
vusirikala Sep 24, 2024
c78c06a
Minor fixes
vusirikala Sep 24, 2024
97af917
Minor comments
vusirikala Sep 24, 2024
c441175
Adding more tests
vusirikala Sep 24, 2024
7275380
Add smoke test
vusirikala Sep 25, 2024
07cb130
Change failpoint names
vusirikala Sep 25, 2024
d5d4381
Use AtomicBool
vusirikala Sep 26, 2024
beb0f7d
Using vector instead of partialsignatures
vusirikala Sep 26, 2024
bf385f9
Use single flag
vusirikala Sep 26, 2024
7a9cce3
Moving optimistic_signature_verification from epoch manager to vote a…
vusirikala Sep 26, 2024
3ecabd7
Merge branch 'main' into satya/osv_votes_and_order_votes
vusirikala Sep 26, 2024
f34cfe9
Fixing errors with rebase
vusirikala Sep 26, 2024
15fe76d
Move optimistic signature verification to validator verifier
vusirikala Sep 30, 2024
ddc6e93
Move more code to validator verifier
vusirikala Sep 30, 2024
ce2cb9b
Minor comment
vusirikala Sep 30, 2024
9b1bd38
Minor changes
vusirikala Sep 30, 2024
edd23b8
Addressing PR comments
vusirikala Oct 1, 2024
c611001
Merge branch 'main' into satya/osv_votes_and_order_votes
vusirikala Oct 2, 2024
b8928f1
Merge branch 'main' into satya/osv_votes_and_order_votes
vusirikala Oct 3, 2024
abd1e32
Rebasing
vusirikala Oct 3, 2024
c79012e
Addressing PR comments
vusirikala Oct 4, 2024
d030c93
Disabling the flag
vusirikala Oct 4, 2024
9cf9e3a
Minor fix in test
vusirikala Oct 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion config/src/config/consensus_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ pub struct ConsensusConfig {
// must match one of the CHAIN_HEALTH_WINDOW_SIZES values.
pub window_for_chain_health: usize,
pub chain_health_backoff: Vec<ChainHealthBackoffValues>,
// Deprecated
pub qc_aggregator_type: QcAggregatorType,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's local config, why not remove it?

// Max blocks allowed for block retrieval requests
pub max_blocks_per_sending_request: u64,
Expand All @@ -90,6 +91,7 @@ pub struct ConsensusConfig {
pub num_bounded_executor_tasks: u64,
pub enable_pre_commit: bool,
pub max_pending_rounds_in_commit_vote_cache: u64,
pub optimistic_sig_verification: bool,
pub enable_round_timeout_msg: bool,
}

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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