Skip to content

Commit

Permalink
Add handling for UpgradeCertificate (#2557)
Browse files Browse the repository at this point in the history
  • Loading branch information
ss-es authored Feb 14, 2024
1 parent 9610778 commit e388f91
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 2 deletions.
4 changes: 4 additions & 0 deletions hotshot/src/tasks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{types::SystemContextHandle, HotShotConsensusApi};
use async_broadcast::{Receiver, Sender};
use async_compatibility_layer::art::{async_sleep, async_spawn};

use hotshot_constants::VERSION_0_1;
use hotshot_task::task::{Task, TaskRegistry};
use hotshot_task_impls::{
consensus::{CommitmentAndMetadata, ConsensusTaskState},
Expand Down Expand Up @@ -171,6 +172,9 @@ pub async fn create_consensus_state<TYPES: NodeType, I: NodeImplementation<TYPES
timeout_vote_collector: None.into(),
timeout_task: None,
timeout_cert: None,
upgrade_cert: None,
decided_upgrade_cert: None,
current_network_version: VERSION_0_1,
output_event_stream: output_stream,
vid_shares: BTreeMap::new(),
current_proposal: None,
Expand Down
77 changes: 76 additions & 1 deletion task-impls/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use async_lock::{RwLock, RwLockUpgradableReadGuard};
use async_std::task::JoinHandle;
use commit::Committable;
use core::time::Duration;
use hotshot_constants::Version;
use hotshot_constants::LOOK_AHEAD;
use hotshot_task::task::{Task, TaskState};

Expand All @@ -19,7 +20,7 @@ use hotshot_types::{
data::{Leaf, QuorumProposal, VidCommitment, VidDisperse},
event::{Event, EventType},
message::{GeneralConsensusMessage, Proposal},
simple_certificate::{QuorumCertificate, TimeoutCertificate},
simple_certificate::{QuorumCertificate, TimeoutCertificate, UpgradeCertificate},
simple_vote::{QuorumData, QuorumVote, TimeoutData, TimeoutVote},
traits::{
block_contents::BlockHeader,
Expand Down Expand Up @@ -120,6 +121,16 @@ pub struct ConsensusTaskState<
/// last Timeout Certificate this node formed
pub timeout_cert: Option<TimeoutCertificate<TYPES>>,

/// last Upgrade Certificate this node formed
pub upgrade_cert: Option<UpgradeCertificate<TYPES>>,

/// most recent decided upgrade certificate
pub decided_upgrade_cert: Option<UpgradeCertificate<TYPES>>,

/// The current version of the network.
/// Updated on view change based on the most recent decided upgrade certificate.
pub current_network_version: Version,

/// Output events to application
pub output_event_stream: async_broadcast::Sender<Event<TYPES>>,

Expand Down Expand Up @@ -465,6 +476,28 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
return;
}

// Validate the upgrade certificate, if one is attached.
// Continue unless the certificate is invalid.
//
// Note: we are *not* directly voting on the upgrade certificate here.
// Once a certificate has been (allegedly) formed, it has already been voted on.
// The certificate is either valid or invalid, and we are simply validating it.
//
// SS: It is possible that we may wish to vote against any quorum proposal
// if it attaches an upgrade certificate that we cannot support.
// But I don't think there's much point in this -- if the UpgradeCertificate
// threshhold (90%) has been reached, voting against the QuorumProposal on that basis
// will probably be completely symbolic anyway.
//
// We should just make sure we don't *sign* an UpgradeCertificate for an upgrade
// that we do not support.
if let Some(ref upgrade_cert) = proposal.data.upgrade_certificate {
if !upgrade_cert.is_valid_cert(self.quorum_membership.as_ref()) {
error!("Invalid upgrade_cert in proposal for view {}", *view);
return;
}
}

// NOTE: We could update our view with a valid TC but invalid QC, but that is not what we do here
self.update_view(view, &event_stream).await;

Expand Down Expand Up @@ -695,6 +728,10 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
.last_synced_block_height
.set(usize::try_from(leaf.get_height()).unwrap_or(0));
}
if let Some(ref upgrade_cert) = proposal.data.upgrade_certificate {
info!("Updating consensus state with decided upgrade certificate: {:?}", upgrade_cert);
self.decided_upgrade_cert = Some(upgrade_cert.clone());
}
// If the block payload is available for this leaf, include it in
// the leaf chain that we send to the client.
if let Some(encoded_txns) =
Expand Down Expand Up @@ -968,6 +1005,17 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
}
}
}
HotShotEvent::UpgradeCertificateFormed(cert) => {
debug!(
"Upgrade certificate received for view {}!",
*cert.view_number
);

// Update our current upgrade_cert as long as it's still relevant.
if cert.view_number >= self.cur_view {
self.upgrade_cert = Some(cert);
}
}
HotShotEvent::DACRecv(cert) => {
debug!("DAC Received for view {}!", *cert.view_number);
let view = cert.view_number;
Expand Down Expand Up @@ -1055,6 +1103,16 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
return;
}

// If we have a decided upgrade certificate,
// we may need to upgrade the protocol version on a view change.
if let Some(ref cert) = self.decided_upgrade_cert {
if new_view >= cert.data.new_version_first_block {
self.current_network_version = cert.data.new_version;
// Discard the old upgrade certificate, which is no longer relevant.
self.decided_upgrade_cert = None;
}
}

broadcast_event(
Event {
view_number: old_view_number,
Expand Down Expand Up @@ -1242,12 +1300,29 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, A: ConsensusApi<TYPES, I> +
error!("Failed to sign leaf.commit()!");
return false;
};

let upgrade_cert = if self
.upgrade_cert
.as_ref()
.is_some_and(|cert| cert.view_number == view)
{
// If the cert view number matches, set upgrade_cert to self.upgrade_cert
// and set self.upgrade_cert to None.
//
// Note: the certificate is discarded, regardless of whether the vote on the proposal succeeds or not.
self.upgrade_cert.take()
} else {
// Otherwise, set upgrade_cert to None.
None
};

// TODO: DA cert is sent as part of the proposal here, we should split this out so we don't have to wait for it.
let proposal = QuorumProposal {
block_header,
view_number: leaf.view_number,
justify_qc: consensus.high_qc.clone(),
timeout_certificate: timeout_certificate.or_else(|| None),
upgrade_certificate: upgrade_cert,
proposer_id: leaf.proposer_id,
};

Expand Down
2 changes: 2 additions & 0 deletions testing/src/task_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ async fn build_quorum_proposal_and_signature(
view_number: ViewNumber::new(1),
justify_qc: QuorumCertificate::genesis(),
timeout_certificate: None,
upgrade_certificate: None,
proposer_id: leaf.proposer_id,
};

Expand Down Expand Up @@ -324,6 +325,7 @@ async fn build_quorum_proposal_and_signature(
view_number: ViewNumber::new(cur_view),
justify_qc: created_qc,
timeout_certificate: None,
upgrade_certificate: None,
proposer_id: leaf_new_view.clone().proposer_id,
};
proposal = proposal_new_view;
Expand Down
5 changes: 4 additions & 1 deletion types/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
//! `HotShot`'s version of a block, and proposals, messages upon which to reach the consensus.
use crate::{
simple_certificate::{QuorumCertificate, TimeoutCertificate},
simple_certificate::{QuorumCertificate, TimeoutCertificate, UpgradeCertificate},
simple_vote::UpgradeProposalData,
traits::{
block_contents::{vid_commitment, BlockHeader, TestableBlock},
Expand Down Expand Up @@ -215,6 +215,9 @@ pub struct QuorumProposal<TYPES: NodeType> {
/// Possible timeout certificate. Only present if the justify_qc is not for the preceding view
pub timeout_certificate: Option<TimeoutCertificate<TYPES>>,

/// Possible upgrade certificate, which the leader may optionally attach.
pub upgrade_certificate: Option<UpgradeCertificate<TYPES>>,

/// the propser id
pub proposer_id: TYPES::SignatureKey,
}
Expand Down

0 comments on commit e388f91

Please sign in to comment.