Skip to content

Commit

Permalink
Remove unused ValidateStateUpdate Event, consolidate map insertion
Browse files Browse the repository at this point in the history
  • Loading branch information
bfish713 committed Nov 7, 2024
1 parent 42cf3cc commit 6c5fc2f
Show file tree
Hide file tree
Showing 14 changed files with 145 additions and 314 deletions.
20 changes: 1 addition & 19 deletions crates/hotshot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> SystemContext<T
inner
}

/// "Starts" consensus by sending a `QcFormed`, `ViewChange`, and `ValidatedStateUpdated` events
/// "Starts" consensus by sending a `QcFormed`, `ViewChange` events
///
/// # Panics
/// Panics if sending genesis fails
Expand Down Expand Up @@ -398,24 +398,6 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> SystemContext<T
.await;
}
});
{
if let Some(validated_state) = consensus.validated_state_map().get(&self.start_view) {
#[allow(clippy::panic)]
self.internal_event_stream
.0
.broadcast_direct(Arc::new(HotShotEvent::ValidatedStateUpdated(
TYPES::View::new(*self.start_view),
validated_state.clone(),
)))
.await
.unwrap_or_else(|_| {
panic!(
"Genesis Broadcast failed; event = ValidatedStateUpdated({:?})",
self.start_view,
)
});
}
}
#[allow(clippy::panic)]
self.internal_event_stream
.0
Expand Down
11 changes: 3 additions & 8 deletions crates/task-impls/src/da.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use async_std::task::spawn_blocking;
use async_trait::async_trait;
use hotshot_task::task::TaskState;
use hotshot_types::{
consensus::{Consensus, OuterConsensus, View},
consensus::{Consensus, OuterConsensus},
data::{DaProposal, PackedBundle},
event::{Event, EventType},
message::{Proposal, UpgradeLock},
Expand All @@ -28,7 +28,6 @@ use hotshot_types::{
signature_key::SignatureKey,
storage::Storage,
},
utils::ViewInner,
vote::HasViewNumber,
};
use sha2::{Digest, Sha256};
Expand Down Expand Up @@ -215,12 +214,8 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> DaTaskState<TYP
let mut consensus_writer = self.consensus.write().await;

// Ensure this view is in the view map for garbage collection.
let view = View {
view_inner: ViewInner::Da { payload_commitment },
};
if let Err(e) =
consensus_writer.update_validated_state_map(view_number, view.clone())
{

if let Err(e) = consensus_writer.update_da_view(view_number, payload_commitment) {
tracing::trace!("{e:?}");
}

Expand Down
12 changes: 2 additions & 10 deletions crates/task-impls/src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use hotshot_types::{
block_contents::BuilderFee, network::DataRequest, node_implementation::NodeType,
signature_key::SignatureKey, BlockPayload,
},
utils::{BuilderCommitment, View},
utils::BuilderCommitment,
vid::VidCommitment,
vote::HasViewNumber,
};
Expand Down Expand Up @@ -203,10 +203,6 @@ pub enum HotShotEvent<TYPES: NodeType> {
/// Upgrade certificate has been sent to the network
UpgradeCertificateFormed(UpgradeCertificate<TYPES>),

/* Consensus State Update Events */
/// A undecided view has been created and added to the validated state storage.
ValidatedStateUpdated(TYPES::View, View<TYPES>),

/// A new locked view has been created (2-chain)
LockedViewUpdated(TYPES::View),

Expand Down Expand Up @@ -323,8 +319,7 @@ impl<TYPES: NodeType> HotShotEvent<TYPES> {
| HotShotEvent::Timeout(view_number)
| HotShotEvent::BlockReady(_, view_number)
| HotShotEvent::LockedViewUpdated(view_number)
| HotShotEvent::LastDecidedViewUpdated(view_number)
| HotShotEvent::ValidatedStateUpdated(view_number, _) => Some(*view_number),
| HotShotEvent::LastDecidedViewUpdated(view_number) => Some(*view_number),
HotShotEvent::DaCertificateRecv(cert) | HotShotEvent::DacSend(cert, _) => {
Some(cert.view_number())
}
Expand Down Expand Up @@ -572,9 +567,6 @@ impl<TYPES: NodeType> Display for HotShotEvent<TYPES> {
proposal.data.view_number
)
}
HotShotEvent::ValidatedStateUpdated(view_number, _) => {
write!(f, "ValidatedStateUpdated(view_number={view_number:?})")
}
HotShotEvent::LockedViewUpdated(view_number) => {
write!(f, "LockedViewUpdated(view_number={view_number:?})")
}
Expand Down
46 changes: 15 additions & 31 deletions crates/task-impls/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,26 +140,19 @@ pub(crate) async fn fetch_proposal<TYPES: NodeType, V: Versions>(
<TYPES::ValidatedState as ValidatedState<TYPES>>::from_header(&proposal.data.block_header),
);

if let Err(e) = consensus_writer
.update_leaf(leaf.clone(), Arc::clone(&state), None, upgrade_lock)
.await
{
tracing::trace!("{e:?}");
}
let view = View {
view_inner: ViewInner::Leaf {
leaf: leaf.commit(upgrade_lock).await,
state,
delta: None,
},
};
if let Err(e) = consensus_writer.update_validated_state_map(view_number, view.clone()) {
tracing::trace!("{e:?}");
}

consensus_writer
.update_saved_leaves(leaf.clone(), upgrade_lock)
.await;

broadcast_event(
HotShotEvent::ValidatedStateUpdated(view_number, view.clone()).into(),
&event_sender,
)
.await;
Ok((leaf, view))
}

Expand Down Expand Up @@ -454,22 +447,20 @@ pub async fn validate_proposal_safety_and_liveness<
let state = Arc::new(
<TYPES::ValidatedState as ValidatedState<TYPES>>::from_header(&proposal.data.block_header),
);
let view = View {
view_inner: ViewInner::Leaf {
leaf: proposed_leaf.commit(&validation_info.upgrade_lock).await,
state,
delta: None, // May be updated to `Some` in the vote task.
},
};

{
let mut consensus_writer = validation_info.consensus.write().await;
if let Err(e) = consensus_writer.update_validated_state_map(view_number, view.clone()) {
if let Err(e) = consensus_writer
.update_leaf(
proposed_leaf.clone(),
state,
None,
&validation_info.upgrade_lock,
)
.await
{
tracing::trace!("{e:?}");
}
consensus_writer
.update_saved_leaves(proposed_leaf.clone(), &validation_info.upgrade_lock)
.await;

// Update our internal storage of the proposal. The proposal is valid, so
// we swallow this error and just log if it occurs.
Expand All @@ -478,13 +469,6 @@ pub async fn validate_proposal_safety_and_liveness<
};
}

// Broadcast that we've updated our consensus state so that other tasks know it's safe to grab.
broadcast_event(
Arc::new(HotShotEvent::ValidatedStateUpdated(view_number, view)),
&event_stream,
)
.await;

let cur_epoch = validation_info.cur_epoch;
UpgradeCertificate::validate(
&proposal.data.upgrade_certificate,
Expand Down
26 changes: 5 additions & 21 deletions crates/task-impls/src/quorum_proposal_recv/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,31 +44,22 @@ use crate::{
#[instrument(skip_all)]
async fn validate_proposal_liveness<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>(
proposal: &Proposal<TYPES, QuorumProposal<TYPES>>,
event_sender: &Sender<Arc<HotShotEvent<TYPES>>>,
validation_info: &ValidationInfo<TYPES, I, V>,
) -> Result<()> {
let view_number = proposal.data.view_number();
let mut consensus_writer = validation_info.consensus.write().await;

let leaf = Leaf::from_quorum_proposal(&proposal.data);

let state = Arc::new(
<TYPES::ValidatedState as ValidatedState<TYPES>>::from_header(&proposal.data.block_header),
);
let view = View {
view_inner: ViewInner::Leaf {
leaf: leaf.commit(&validation_info.upgrade_lock).await,
state,
delta: None, // May be updated to `Some` in the vote task.
},
};

if let Err(e) = consensus_writer.update_validated_state_map(view_number, view.clone()) {
if let Err(e) = consensus_writer
.update_leaf(leaf.clone(), state, None, &validation_info.upgrade_lock)
.await
{
tracing::trace!("{e:?}");
}
consensus_writer
.update_saved_leaves(leaf.clone(), &validation_info.upgrade_lock)
.await;

if let Err(e) = validation_info
.storage
Expand All @@ -88,13 +79,6 @@ async fn validate_proposal_liveness<TYPES: NodeType, I: NodeImplementation<TYPES

drop(consensus_writer);

// Broadcast that we've updated our consensus state so that other tasks know it's safe to grab.
broadcast_event(
HotShotEvent::ValidatedStateUpdated(view_number, view).into(),
event_sender,
)
.await;

if !liveness_check {
bail!("Quorum Proposal failed the liveness check");
}
Expand Down Expand Up @@ -248,7 +232,7 @@ pub(crate) async fn handle_quorum_proposal_recv<
"Proposal's parent missing from storage with commitment: {:?}",
justify_qc.data.leaf_commit
);
validate_proposal_liveness(proposal, event_sender, &validation_info).await?;
validate_proposal_liveness(proposal, &validation_info).await?;
broadcast_event(
Arc::new(HotShotEvent::ViewChange(view_number)),
event_sender,
Expand Down
28 changes: 8 additions & 20 deletions crates/task-impls/src/quorum_vote/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use hotshot_types::{
storage::Storage,
ValidatedState,
},
utils::{View, ViewInner},
vote::HasViewNumber,
};
use tracing::instrument;
Expand Down Expand Up @@ -257,34 +256,23 @@ pub(crate) async fn update_shared_state<
// We will defer broadcast until all states are updated to avoid holding onto the lock during a network call.
let mut consensus_writer = consensus.write().await;

let view = View {
view_inner: ViewInner::Leaf {
leaf: proposed_leaf.commit(&upgrade_lock).await,
state: Arc::clone(&state),
delta: Some(Arc::clone(&delta)),
},
};
if let Err(e) =
consensus_writer.update_validated_state_map(proposed_leaf.view_number(), view.clone())
if let Err(e) = consensus_writer
.update_leaf(
proposed_leaf.clone(),
Arc::clone(&state),
Some(Arc::clone(&delta)),
&upgrade_lock,
)
.await
{
tracing::trace!("{e:?}");
}
consensus_writer
.update_saved_leaves(proposed_leaf.clone(), &upgrade_lock)
.await;

// Kick back our updated structures for downstream usage.
let new_leaves = consensus_writer.saved_leaves().clone();
let new_state = consensus_writer.validated_state_map().clone();
drop(consensus_writer);

// Broadcast now that the lock is dropped.
broadcast_event(
HotShotEvent::ValidatedStateUpdated(proposed_leaf.view_number(), view).into(),
&sender,
)
.await;

// Send the new state up to the sequencer.
storage
.write()
Expand Down
11 changes: 0 additions & 11 deletions crates/testing/src/predicates/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,17 +262,6 @@ where
Box::new(EventPredicate { check, info })
}

pub fn validated_state_updated<TYPES>() -> Box<EventPredicate<TYPES>>
where
TYPES: NodeType,
{
let info = "ValidatedStateUpdated".to_string();
let check: EventCallback<TYPES> = Arc::new(move |e: Arc<HotShotEvent<TYPES>>| {
matches!(e.as_ref(), ValidatedStateUpdated(..))
});
Box::new(EventPredicate { check, info })
}

pub fn vid_share_validated<TYPES>() -> Box<EventPredicate<TYPES>>
where
TYPES: NodeType,
Expand Down
Loading

0 comments on commit 6c5fc2f

Please sign in to comment.