Skip to content

Commit

Permalink
Revert "QuorumProposal2 has an epoch field"
Browse files Browse the repository at this point in the history
This reverts commit 61e1661.
  • Loading branch information
lukaszrzasik committed Jan 31, 2025
1 parent 60dc88c commit ae9d53c
Show file tree
Hide file tree
Showing 18 changed files with 277 additions and 130 deletions.
22 changes: 20 additions & 2 deletions crates/example-types/src/storage_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use hotshot_types::{
data::{
vid_disperse::{ADVZDisperseShare, VidDisperseShare2},
DaProposal, DaProposal2, Leaf, Leaf2, QuorumProposal, QuorumProposal2,
QuorumProposalWrapper,
},
event::HotShotAction,
message::Proposal,
Expand Down Expand Up @@ -50,6 +51,7 @@ pub struct TestStorageState<TYPES: NodeType> {
da2s: HashMap<TYPES::View, Proposal<TYPES, DaProposal2<TYPES>>>,
proposals: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposal<TYPES>>>,
proposals2: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposal2<TYPES>>>,
proposals_wrapper: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposalWrapper<TYPES>>>,
high_qc: Option<hotshot_types::simple_certificate::QuorumCertificate<TYPES>>,
high_qc2: Option<hotshot_types::simple_certificate::QuorumCertificate2<TYPES>>,
next_epoch_high_qc2:
Expand All @@ -67,6 +69,7 @@ impl<TYPES: NodeType> Default for TestStorageState<TYPES> {
da2s: HashMap::new(),
proposals: BTreeMap::new(),
proposals2: BTreeMap::new(),
proposals_wrapper: BTreeMap::new(),
high_qc: None,
next_epoch_high_qc2: None,
high_qc2: None,
Expand Down Expand Up @@ -109,8 +112,8 @@ impl<TYPES: NodeType> TestableDelay for TestStorage<TYPES> {
impl<TYPES: NodeType> TestStorage<TYPES> {
pub async fn proposals_cloned(
&self,
) -> BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposal2<TYPES>>> {
self.inner.read().await.proposals2.clone()
) -> BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposalWrapper<TYPES>>> {
self.inner.read().await.proposals_wrapper.clone()
}

pub async fn high_qc_cloned(&self) -> Option<QuorumCertificate2<TYPES>> {
Expand Down Expand Up @@ -232,6 +235,21 @@ impl<TYPES: NodeType> Storage<TYPES> for TestStorage<TYPES> {
Ok(())
}

async fn append_proposal_wrapper(
&self,
proposal: &Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
) -> Result<()> {
if self.should_return_err {
bail!("Failed to append Quorum proposal (wrapped) to storage");
}
Self::run_delay_settings_from_config(&self.delay_config).await;
let mut inner = self.inner.write().await;
inner
.proposals_wrapper
.insert(proposal.data.view_number(), proposal.clone());
Ok(())
}

async fn record_action(
&self,
view: <TYPES as NodeType>::View,
Expand Down
5 changes: 3 additions & 2 deletions crates/hotshot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ pub mod traits;
pub mod types;

pub mod tasks;
use hotshot_types::data::QuorumProposalWrapper;

/// Contains helper functions for the crate
pub mod helpers;
Expand Down Expand Up @@ -1015,7 +1016,7 @@ pub struct HotShotInitializer<TYPES: NodeType> {
pub next_epoch_high_qc: Option<NextEpochQuorumCertificate2<TYPES>>,

/// Proposals we have sent out to provide to others for catchup
pub saved_proposals: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposal2<TYPES>>>,
pub saved_proposals: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposalWrapper<TYPES>>>,

/// Previously decided upgrade certificate; this is necessary if an upgrade has happened and we are not restarting with the new version
pub decided_upgrade_certificate: Option<UpgradeCertificate<TYPES>>,
Expand Down Expand Up @@ -1115,7 +1116,7 @@ impl<TYPES: NodeType> HotShotInitializer<TYPES> {
QuorumCertificate2<TYPES>,
Option<NextEpochQuorumCertificate2<TYPES>>,
),
saved_proposals: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposal2<TYPES>>>,
saved_proposals: BTreeMap<TYPES::View, Proposal<TYPES, QuorumProposalWrapper<TYPES>>>,
saved_vid_shares: VidShares<TYPES>,
decided_upgrade_certificate: Option<UpgradeCertificate<TYPES>>,
) -> Self {
Expand Down
8 changes: 5 additions & 3 deletions crates/hotshot/src/types/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use hotshot_task::{
use hotshot_task_impls::{events::HotShotEvent, helpers::broadcast_event};
use hotshot_types::{
consensus::Consensus,
data::{Leaf2, QuorumProposal2},
data::{Leaf2, QuorumProposalWrapper},
error::HotShotError,
message::{Message, MessageKind, Proposal, RecipientList},
request_response::ProposalRequestPayload,
Expand Down Expand Up @@ -140,7 +140,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES> + 'static, V: Versions>
&self,
view: TYPES::View,
leaf_commitment: Commitment<Leaf2<TYPES>>,
) -> Result<impl futures::Future<Output = Result<Proposal<TYPES, QuorumProposal2<TYPES>>>>>
) -> Result<impl futures::Future<Output = Result<Proposal<TYPES, QuorumProposalWrapper<TYPES>>>>>
{
// We need to be able to sign this request before submitting it to the network. Compute the
// payload first.
Expand All @@ -158,6 +158,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES> + 'static, V: Versions>
let mem = Arc::clone(&self.memberships);
let receiver = self.internal_event_stream.1.activate_cloned();
let sender = self.internal_event_stream.0.clone();
let epoch_height = self.epoch_height;
Ok(async move {
// First, broadcast that we need a proposal
broadcast_event(
Expand Down Expand Up @@ -186,7 +187,8 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES> + 'static, V: Versions>
{
// Make sure that the quorum_proposal is valid
let mem_reader = mem.read().await;
if let Err(err) = quorum_proposal.validate_signature(&mem_reader) {
if let Err(err) = quorum_proposal.validate_signature(&mem_reader, epoch_height)
{
tracing::warn!("Invalid Proposal Received after Request. Err {:?}", err);
continue;
}
Expand Down
25 changes: 17 additions & 8 deletions crates/task-impls/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ use either::Either;
use hotshot_task::task::TaskEvent;
use hotshot_types::{
data::{
DaProposal2, Leaf2, PackedBundle, QuorumProposal2, UpgradeProposal, VidDisperse,
VidDisperseShare,
DaProposal2, Leaf2, PackedBundle, QuorumProposal2, QuorumProposalWrapper, UpgradeProposal,
VidDisperse, VidDisperseShare,
},
message::Proposal,
request_response::ProposalRequestPayload,
Expand Down Expand Up @@ -72,7 +72,10 @@ pub enum HotShotEvent<TYPES: NodeType> {
/// Shutdown the task
Shutdown,
/// A quorum proposal has been received from the network; handled by the consensus task
QuorumProposalRecv(Proposal<TYPES, QuorumProposal2<TYPES>>, TYPES::SignatureKey),
QuorumProposalRecv(
Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
TYPES::SignatureKey,
),
/// A quorum vote has been received from the network; handled by the consensus task
QuorumVoteRecv(QuorumVote2<TYPES>),
/// A timeout vote received from the network; handled by consensus task
Expand All @@ -90,7 +93,10 @@ pub enum HotShotEvent<TYPES: NodeType> {
/// A DAC is validated.
DaCertificateValidated(DaCertificate2<TYPES>),
/// Send a quorum proposal to the network; emitted by the leader in the consensus task
QuorumProposalSend(Proposal<TYPES, QuorumProposal2<TYPES>>, TYPES::SignatureKey),
QuorumProposalSend(
Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
TYPES::SignatureKey,
),
/// Send a quorum vote to the next leader; emitted by a replica in the consensus task after seeing a valid quorum proposal
QuorumVoteSend(QuorumVote2<TYPES>),
/// Broadcast a quorum vote to form an eQC; emitted by a replica in the consensus task after seeing a valid quorum proposal
Expand All @@ -101,7 +107,7 @@ pub enum HotShotEvent<TYPES: NodeType> {
/// 2. The proposal has been correctly signed by the leader of the current view
/// 3. The justify QC is valid
/// 4. The proposal passes either liveness or safety check.
QuorumProposalValidated(Proposal<TYPES, QuorumProposal2<TYPES>>, Leaf2<TYPES>),
QuorumProposalValidated(Proposal<TYPES, QuorumProposalWrapper<TYPES>>, Leaf2<TYPES>),
/// A quorum proposal is missing for a view that we need.
QuorumProposalRequestSend(
ProposalRequestPayload<TYPES>,
Expand All @@ -113,9 +119,12 @@ pub enum HotShotEvent<TYPES: NodeType> {
<TYPES::SignatureKey as SignatureKey>::PureAssembledSignatureType,
),
/// A quorum proposal was missing for a view. As the leader, we send a reply to the recipient with their key.
QuorumProposalResponseSend(TYPES::SignatureKey, Proposal<TYPES, QuorumProposal2<TYPES>>),
QuorumProposalResponseSend(
TYPES::SignatureKey,
Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
),
/// A quorum proposal was requested by a node for a view.
QuorumProposalResponseRecv(Proposal<TYPES, QuorumProposal2<TYPES>>),
QuorumProposalResponseRecv(Proposal<TYPES, QuorumProposalWrapper<TYPES>>),
/// Send a DA proposal to the DA committee; emitted by the DA leader (which is the same node as the leader of view v + 1) in the DA task
DaProposalSend(Proposal<TYPES, DaProposal2<TYPES>>, TYPES::SignatureKey),
/// Send a DA vote to the DA leader; emitted by DA committee members in the DA task after seeing a valid DA proposal
Expand Down Expand Up @@ -208,7 +217,7 @@ pub enum HotShotEvent<TYPES: NodeType> {
/// 1. The proposal is not for an old view
/// 2. The proposal has been correctly signed by the leader of the current view
/// 3. The justify QC is valid
QuorumProposalPreliminarilyValidated(Proposal<TYPES, QuorumProposal2<TYPES>>),
QuorumProposalPreliminarilyValidated(Proposal<TYPES, QuorumProposalWrapper<TYPES>>),

/// Send a VID request to the network; emitted to on of the members of DA committee.
/// Includes the data request, node's public key and signature as well as public key of DA committee who we want to send to.
Expand Down
24 changes: 15 additions & 9 deletions crates/task-impls/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use committable::{Commitment, Committable};
use hotshot_task::dependency::{Dependency, EventDependency};
use hotshot_types::{
consensus::OuterConsensus,
data::{Leaf2, QuorumProposal2, ViewChangeEvidence2},
data::{Leaf2, QuorumProposalWrapper, ViewChangeEvidence2},
event::{Event, EventType, LeafInfo},
message::{Proposal, UpgradeLock},
request_response::ProposalRequestPayload,
Expand All @@ -29,7 +29,8 @@ use hotshot_types::{
BlockPayload, ValidatedState,
},
utils::{
epoch_from_block_number, is_epoch_root, is_last_block_in_epoch, Terminator, View, ViewInner,
epoch_from_block_number, is_epoch_root, is_last_block_in_epoch,
option_epoch_from_block_number, Terminator, View, ViewInner,
},
vote::{Certificate, HasViewNumber},
};
Expand Down Expand Up @@ -108,7 +109,7 @@ pub(crate) async fn fetch_proposal<TYPES: NodeType, V: Versions>(
{
// Make sure that the quorum_proposal is valid
let mem_reader = mem.read().await;
if quorum_proposal.validate_signature(&mem_reader).is_ok() {
if quorum_proposal.validate_signature(&mem_reader, epoch_height).is_ok() {
proposal = Some(quorum_proposal.clone());
}

Expand Down Expand Up @@ -250,7 +251,7 @@ impl<TYPES: NodeType + Default> Default for LeafChainTraversalOutcome<TYPES> {
/// If the leaf chain contains no decided leaf while reaching a decided view, which should be
/// impossible.
pub async fn decide_from_proposal_2<TYPES: NodeType>(
proposal: &QuorumProposal2<TYPES>,
proposal: &QuorumProposalWrapper<TYPES>,
consensus: OuterConsensus<TYPES>,
existing_upgrade_cert: Arc<RwLock<Option<UpgradeCertificate<TYPES>>>>,
public_key: &TYPES::SignatureKey,
Expand Down Expand Up @@ -371,7 +372,7 @@ pub async fn decide_from_proposal_2<TYPES: NodeType>(
/// If the leaf chain contains no decided leaf while reaching a decided view, which should be
/// impossible.
pub async fn decide_from_proposal<TYPES: NodeType>(
proposal: &QuorumProposal2<TYPES>,
proposal: &QuorumProposalWrapper<TYPES>,
consensus: OuterConsensus<TYPES>,
existing_upgrade_cert: Arc<RwLock<Option<UpgradeCertificate<TYPES>>>>,
public_key: &TYPES::SignatureKey,
Expand Down Expand Up @@ -573,7 +574,7 @@ pub async fn validate_proposal_safety_and_liveness<
I: NodeImplementation<TYPES>,
V: Versions,
>(
proposal: Proposal<TYPES, QuorumProposal2<TYPES>>,
proposal: Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
parent_leaf: Leaf2<TYPES>,
validation_info: &ValidationInfo<TYPES, I, V>,
event_stream: Sender<Arc<HotShotEvent<TYPES>>>,
Expand Down Expand Up @@ -724,7 +725,7 @@ pub(crate) async fn validate_proposal_view_and_certs<
I: NodeImplementation<TYPES>,
V: Versions,
>(
proposal: &Proposal<TYPES, QuorumProposal2<TYPES>>,
proposal: &Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
validation_info: &ValidationInfo<TYPES, I, V>,
) -> Result<()> {
let view_number = proposal.data.view_number();
Expand All @@ -736,7 +737,7 @@ pub(crate) async fn validate_proposal_view_and_certs<

// Validate the proposal's signature. This should also catch if the leaf_commitment does not equal our calculated parent commitment
let membership_reader = validation_info.membership.read().await;
proposal.validate_signature(&membership_reader)?;
proposal.validate_signature(&membership_reader, validation_info.epoch_height)?;
drop(membership_reader);

// Verify a timeout certificate OR a view sync certificate exists and is valid.
Expand Down Expand Up @@ -808,10 +809,15 @@ pub(crate) async fn validate_proposal_view_and_certs<
// Validate the upgrade certificate -- this is just a signature validation.
// Note that we don't do anything with the certificate directly if this passes; it eventually gets stored as part of the leaf if nothing goes wrong.
{
let epoch = option_epoch_from_block_number::<TYPES>(
proposal.data.epoch().is_some(),
proposal.data.block_header().block_number(),
validation_info.epoch_height,
);
UpgradeCertificate::validate(
proposal.data.upgrade_certificate(),
&validation_info.membership,
proposal.data.epoch(),
epoch,
&validation_info.upgrade_lock,
)
.await?;
Expand Down
4 changes: 2 additions & 2 deletions crates/task-impls/src/network.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<TYPES: NodeType, V: Versions> NetworkMessageTaskState<TYPES, V> {
tracing::warn!("received GeneralConsensusMessage::Proposal2 for view {} but epochs are not enabled for that view", proposal.data.view_number());
return;
}
HotShotEvent::QuorumProposalRecv(proposal, sender)
HotShotEvent::QuorumProposalRecv(convert_proposal(proposal), sender)
}
GeneralConsensusMessage::ProposalRequested(req, sig) => {
HotShotEvent::QuorumProposalRequestRecv(req, sig)
Expand All @@ -120,7 +120,7 @@ impl<TYPES: NodeType, V: Versions> NetworkMessageTaskState<TYPES, V> {
tracing::warn!("received GeneralConsensusMessage::ProposalResponse2 for view {} but epochs are not enabled for that view", proposal.data.view_number());
return;
}
HotShotEvent::QuorumProposalResponseRecv(proposal)
HotShotEvent::QuorumProposalResponseRecv(convert_proposal(proposal))
}
GeneralConsensusMessage::Vote(vote) => {
if self.upgrade_lock.epochs_enabled(vote.view_number()).await {
Expand Down
22 changes: 12 additions & 10 deletions crates/task-impls/src/quorum_proposal/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use hotshot_task::{
};
use hotshot_types::{
consensus::{CommitmentAndMetadata, OuterConsensus},
data::{Leaf2, QuorumProposal2, VidDisperse, ViewChangeEvidence2},
data::{Leaf2, QuorumProposal2, QuorumProposalWrapper, VidDisperse, ViewChangeEvidence2},
message::Proposal,
simple_certificate::{NextEpochQuorumCertificate2, QuorumCertificate2, UpgradeCertificate},
traits::{
Expand Down Expand Up @@ -414,15 +414,17 @@ impl<TYPES: NodeType, V: Versions> ProposalDependencyHandle<TYPES, V> {
} else {
None
};
let proposal = QuorumProposal2 {
block_header,
view_number: self.view_number,
epoch,
justify_qc: parent_qc,
next_epoch_justify_qc: next_epoch_qc,
upgrade_certificate,
view_change_evidence: proposal_certificate,
next_drb_result,
let proposal = QuorumProposalWrapper {
proposal: QuorumProposal2 {
block_header,
view_number: self.view_number,
epoch,
justify_qc: parent_qc,
next_epoch_justify_qc: next_epoch_qc,
upgrade_certificate,
view_change_evidence: proposal_certificate,
next_drb_result,
},
};

let proposed_leaf = Leaf2::from_quorum_proposal(&proposal);
Expand Down
13 changes: 9 additions & 4 deletions crates/task-impls/src/quorum_proposal_recv/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use async_lock::{RwLock, RwLockUpgradableReadGuard};
use committable::Committable;
use hotshot_types::{
consensus::OuterConsensus,
data::{Leaf2, QuorumProposal, QuorumProposal2},
data::{Leaf2, QuorumProposal, QuorumProposalWrapper},
message::Proposal,
simple_certificate::QuorumCertificate,
simple_vote::HasEpoch,
Expand Down Expand Up @@ -45,7 +45,7 @@ use crate::{
/// Update states in the event that the parent state is not found for a given `proposal`.
#[instrument(skip_all)]
async fn validate_proposal_liveness<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>(
proposal: &Proposal<TYPES, QuorumProposal2<TYPES>>,
proposal: &Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
validation_info: &ValidationInfo<TYPES, I, V>,
) -> Result<()> {
let mut consensus_writer = validation_info.consensus.write().await;
Expand Down Expand Up @@ -140,7 +140,7 @@ pub(crate) async fn handle_quorum_proposal_recv<
I: NodeImplementation<TYPES>,
V: Versions,
>(
proposal: &Proposal<TYPES, QuorumProposal2<TYPES>>,
proposal: &Proposal<TYPES, QuorumProposalWrapper<TYPES>>,
quorum_proposal_sender_key: &TYPES::SignatureKey,
event_sender: &Sender<Arc<HotShotEvent<TYPES>>>,
event_receiver: &Receiver<Arc<HotShotEvent<TYPES>>>,
Expand All @@ -161,7 +161,12 @@ pub(crate) async fn handle_quorum_proposal_recv<
let justify_qc = proposal.data.justify_qc().clone();
let maybe_next_epoch_justify_qc = proposal.data.next_epoch_justify_qc().clone();

let proposal_epoch = proposal.data.epoch();
let proposal_block_number = proposal.data.block_header().block_number();
let proposal_epoch = option_epoch_from_block_number::<TYPES>(
proposal.data.epoch().is_some(),
proposal_block_number,
validation_info.epoch_height,
);

let membership_reader = validation_info.membership.read().await;
let membership_stake_table = membership_reader.stake_table(justify_qc.data.epoch);
Expand Down
Loading

0 comments on commit ae9d53c

Please sign in to comment.