Skip to content

Commit

Permalink
Using signature with status
Browse files Browse the repository at this point in the history
  • Loading branch information
vusirikala committed Sep 24, 2024
1 parent 25e9031 commit 85c7051
Show file tree
Hide file tree
Showing 12 changed files with 260 additions and 260 deletions.
19 changes: 12 additions & 7 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 @@ -60,7 +63,7 @@ impl OrderVote {
&self.ledger_info
}

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

Expand All @@ -80,8 +83,10 @@ impl OrderVote {
/// Verifies the signature on the LedgerInfo.
pub fn verify_signature(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
validator
.verify(self.author(), &self.ledger_info, &self.signature)
.context("Failed to verify OrderVote")
.verify(self.author(), &self.ledger_info, self.signature.signature())
.context("Failed to verify OrderVote")?;
self.signature.set_verified();
Ok(())
}

/// Performs full verification including the signature verification.
Expand Down
14 changes: 8 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 @@ -27,8 +28,8 @@ pub struct Vote {
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 @@ -108,7 +109,7 @@ impl Vote {
}

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

Expand Down Expand Up @@ -163,8 +164,9 @@ impl Vote {
/// Verifies the signature on the LedgerInfo.
pub fn verify_signature(&self, validator: &ValidatorVerifier) -> anyhow::Result<()> {
validator
.verify(self.author(), &self.ledger_info, &self.signature)
.verify(self.author(), &self.ledger_info, self.signature.signature())
.context("Failed to verify Vote signature")?;
self.signature.set_verified();
Ok(())
}

Expand Down
2 changes: 1 addition & 1 deletion consensus/safety-rules/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub fn make_proposal_with_parent_and_overrides(
PartialSignatures::empty(),
);

ledger_info_with_signatures.add_signature(vote.author(), vote.signature().clone());
ledger_info_with_signatures.add_signature(vote.author(), vote.signature().signature().clone());

let qc = QuorumCert::new(
vote_data,
Expand Down
11 changes: 6 additions & 5 deletions consensus/src/block_storage/block_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use aptos_consensus_types::{
};
use aptos_crypto::{HashValue, PrivateKey};
use aptos_types::{
epoch_state::EpochState, ledger_info::VerificationStatus, validator_signer::ValidatorSigner,
epoch_state::EpochState, validator_signer::ValidatorSigner,
validator_verifier::random_validator_verifier,
};
use proptest::prelude::*;
Expand Down Expand Up @@ -302,14 +302,14 @@ async fn test_insert_vote() {
voter,
)
.unwrap();
let vote_res =
pending_votes.insert_vote(&vote, epoch_state.clone(), VerificationStatus::Verified);
vote.signature().set_verified();
let vote_res = pending_votes.insert_vote(&vote, epoch_state.clone());

// first vote of an author is accepted
assert_eq!(vote_res, VoteReceptionResult::VoteAdded(i as u128));
// filter out duplicates
assert_eq!(
pending_votes.insert_vote(&vote, epoch_state.clone(), VerificationStatus::Verified),
pending_votes.insert_vote(&vote, epoch_state.clone()),
VoteReceptionResult::DuplicateVote,
);
// qc is still not there
Expand All @@ -332,7 +332,8 @@ async fn test_insert_vote() {
final_voter,
)
.unwrap();
match pending_votes.insert_vote(&vote, epoch_state.clone(), VerificationStatus::Verified) {
vote.signature().set_verified();
match pending_votes.insert_vote(&vote, epoch_state.clone()) {
VoteReceptionResult::NewQuorumCertificate(qc) => {
assert_eq!(qc.certified_block().id(), block.id());
block_store
Expand Down
74 changes: 47 additions & 27 deletions consensus/src/epoch_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1445,20 +1445,29 @@ impl<P: OnChainConfigProvider> EpochManager<P> {
{
if let UnverifiedEvent::OrderVoteMsg(order_vote) = &unverified_event {
order_vote.verify_metadata()?;
Self::forward_event_to(
Self::forward_event(
quorum_store_msg_tx,
round_manager_tx,
(
peer_id,
discriminant(&VerifiedEvent::UnverifiedOrderVoteMsg(
order_vote.clone(),
)),
),
(
peer_id,
VerifiedEvent::UnverifiedOrderVoteMsg(order_vote.clone()),
),
)
.context("round manager sending unverified order vote to round manager")?;
buffered_proposal_tx,
peer_id,
VerifiedEvent::OrderVoteMsg(order_vote.clone()),
payload_manager,
pending_blocks,
);
// Self::forward_event_to(
// round_manager_tx,
// (
// peer_id,
// discriminant(&VerifiedEvent::OrderVoteMsg(
// order_vote.clone(),
// )),
// ),
// (
// peer_id,
// VerifiedEvent::OrderVoteMsg(order_vote.clone()),
// ),
// )
// .context("round manager sending unverified order vote to round manager")?;
return Ok(());
}
}
Expand All @@ -1478,21 +1487,32 @@ impl<P: OnChainConfigProvider> EpochManager<P> {
.observe(start_time.elapsed().as_secs_f64());
match result {
Ok(()) => {
if let Err(e) = Self::forward_event_to(
// if let Err(e) =
Self::forward_event(
quorum_store_msg_tx,
round_manager_tx,
(
peer_id,
discriminant(&VerifiedEvent::UnverifiedVoteMsg(
vote.clone(),
)),
),
(peer_id, VerifiedEvent::UnverifiedVoteMsg(vote.clone())),
)
.context(
"round manager sending unverified vote to round manager",
) {
warn!("Failed to forward unverified event: {}", e);
};
buffered_proposal_tx,
peer_id,
VerifiedEvent::VoteMsg(vote.clone()),
payload_manager,
pending_blocks,
);

// Self::forward_event_to(
// round_manager_tx,
// (
// peer_id,
// discriminant(&VerifiedEvent::VoteMsg(
// vote.clone(),
// )),
// ),
// (peer_id, VerifiedEvent::VoteMsg(vote)),
// )
// .context(
// "round manager sending unverified vote to round manager",
// ) {
// warn!("Failed to forward unverified event: {}", e);
// };
},
Err(e) => {
error!(
Expand Down
6 changes: 2 additions & 4 deletions consensus/src/liveness/round_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use aptos_consensus_types::{
};
use aptos_crypto::HashValue;
use aptos_logger::{prelude::*, Schema};
use aptos_types::{epoch_state::EpochState, ledger_info::VerificationStatus};
use aptos_types::epoch_state::EpochState;
use futures::future::AbortHandle;
use serde::Serialize;
use std::{fmt, sync::Arc, time::Duration};
Expand Down Expand Up @@ -274,11 +274,9 @@ impl RoundState {
&mut self,
vote: &Vote,
epoch_state: Arc<EpochState>,
verification_status: VerificationStatus,
) -> VoteReceptionResult {
if vote.vote_data().proposed().round() == self.current_round {
self.pending_votes
.insert_vote(vote, epoch_state, verification_status)
self.pending_votes.insert_vote(vote, epoch_state)
} else {
VoteReceptionResult::UnexpectedRound(
vote.vote_data().proposed().round(),
Expand Down
Loading

0 comments on commit 85c7051

Please sign in to comment.