Skip to content

Commit

Permalink
[TECH_DEBT] Remove unused Parameters in TaskStates (#3892)
Browse files Browse the repository at this point in the history
  • Loading branch information
bfish713 authored Nov 20, 2024
1 parent 4f1116c commit 3005537
Show file tree
Hide file tree
Showing 12 changed files with 22 additions and 78 deletions.
4 changes: 2 additions & 2 deletions crates/hotshot/src/tasks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ pub fn add_network_event_task<
pub async fn add_consensus_tasks<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>(
handle: &mut SystemContextHandle<TYPES, I, V>,
) {
handle.add_task(ViewSyncTaskState::<TYPES, I, V>::create_from(handle).await);
handle.add_task(ViewSyncTaskState::<TYPES, V>::create_from(handle).await);
handle.add_task(VidTaskState::<TYPES, I>::create_from(handle).await);
handle.add_task(DaTaskState::<TYPES, I, V>::create_from(handle).await);
handle.add_task(TransactionTaskState::<TYPES, I, V>::create_from(handle).await);
Expand All @@ -240,7 +240,7 @@ pub async fn add_consensus_tasks<TYPES: NodeType, I: NodeImplementation<TYPES>,

// only spawn the upgrade task if we are actually configured to perform an upgrade.
if V::Base::VERSION < V::Upgrade::VERSION {
handle.add_task(UpgradeTaskState::<TYPES, I, V>::create_from(handle).await);
handle.add_task(UpgradeTaskState::<TYPES, V>::create_from(handle).await);
}

{
Expand Down
14 changes: 2 additions & 12 deletions crates/hotshot/src/tasks/task_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState

#[async_trait]
impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState<TYPES, I, V>
for UpgradeTaskState<TYPES, I, V>
for UpgradeTaskState<TYPES, V>
{
async fn create_from(handle: &SystemContextHandle<TYPES, I, V>) -> Self {
#[cfg(not(feature = "example-upgrade"))]
Expand All @@ -72,7 +72,6 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
cur_view: handle.cur_view().await,
cur_epoch: handle.cur_epoch().await,
quorum_membership: handle.hotshot.memberships.quorum_membership.clone().into(),
network: Arc::clone(&handle.hotshot.network),
vote_collectors: BTreeMap::default(),
public_key: handle.public_key().clone(),
private_key: handle.private_key().clone(),
Expand Down Expand Up @@ -121,7 +120,6 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
consensus: OuterConsensus::new(handle.hotshot.consensus()),
cur_view: handle.cur_view().await,
cur_epoch: handle.cur_epoch().await,
vote_collector: None,
network: Arc::clone(&handle.hotshot.network),
membership: handle.hotshot.memberships.quorum_membership.clone().into(),
public_key: handle.public_key().clone(),
Expand Down Expand Up @@ -156,7 +154,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState

#[async_trait]
impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState<TYPES, I, V>
for ViewSyncTaskState<TYPES, I, V>
for ViewSyncTaskState<TYPES, V>
{
async fn create_from(handle: &SystemContextHandle<TYPES, I, V>) -> Self {
let cur_view = handle.cur_view().await;
Expand All @@ -165,7 +163,6 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
cur_view,
next_view: cur_view,
cur_epoch: handle.cur_epoch().await,
network: Arc::clone(&handle.hotshot.network),
membership: handle.hotshot.memberships.quorum_membership.clone().into(),
public_key: handle.public_key().clone(),
private_key: handle.private_key().clone(),
Expand Down Expand Up @@ -193,7 +190,6 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
consensus: OuterConsensus::new(handle.hotshot.consensus()),
cur_view: handle.cur_view().await,
cur_epoch: handle.cur_epoch().await,
network: Arc::clone(&handle.hotshot.network),
membership: handle.hotshot.memberships.quorum_membership.clone().into(),
public_key: handle.public_key().clone(),
private_key: handle.private_key().clone(),
Expand Down Expand Up @@ -256,11 +252,8 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
Self {
latest_proposed_view: handle.cur_view().await,
proposal_dependencies: BTreeMap::new(),
network: Arc::clone(&handle.hotshot.network),
output_event_stream: handle.hotshot.external_event_stream.0.clone(),
consensus: OuterConsensus::new(consensus),
instance_state: handle.hotshot.instance_state(),
timeout_membership: handle.hotshot.memberships.quorum_membership.clone().into(),
quorum_membership: handle.hotshot.memberships.quorum_membership.clone().into(),
public_key: handle.public_key().clone(),
private_key: handle.private_key().clone(),
Expand Down Expand Up @@ -312,12 +305,9 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
private_key: handle.private_key().clone(),
instance_state: handle.hotshot.instance_state(),
network: Arc::clone(&handle.hotshot.network),
timeout_membership: handle.hotshot.memberships.quorum_membership.clone().into(),
quorum_membership: handle.hotshot.memberships.quorum_membership.clone().into(),
committee_membership: handle.hotshot.memberships.da_membership.clone().into(),
vote_collectors: BTreeMap::default(),
timeout_vote_collectors: BTreeMap::default(),
storage: Arc::clone(&handle.storage),
cur_view: handle.cur_view().await,
cur_view_time: Utc::now().timestamp(),
cur_epoch: handle.cur_epoch().await,
Expand Down
4 changes: 2 additions & 2 deletions crates/task-impls/src/consensus/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ pub(crate) async fn handle_timeout_vote_recv<
// Are we the leader for this view?
ensure!(
task_state
.timeout_membership
.quorum_membership
.leader(vote.view_number() + 1, task_state.cur_epoch)?
== task_state.public_key,
info!(
Expand Down Expand Up @@ -276,7 +276,7 @@ pub(crate) async fn handle_timeout<TYPES: NodeType, I: NodeImplementation<TYPES>

ensure!(
task_state
.timeout_membership
.quorum_membership
.has_stake(&task_state.public_key, task_state.cur_epoch),
debug!("We were not chosen for the consensus committee for view {view_number:?}")
);
Expand Down
10 changes: 0 additions & 10 deletions crates/task-impls/src/consensus/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use std::sync::Arc;

use async_broadcast::{Receiver, Sender};
use async_lock::RwLock;
use async_trait::async_trait;
use hotshot_task::task::TaskState;
use hotshot_types::{
Expand Down Expand Up @@ -47,25 +46,16 @@ pub struct ConsensusTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V:
/// The underlying network
pub network: Arc<I::Network>,

/// Membership for Timeout votes/certs
pub timeout_membership: Arc<TYPES::Membership>,

/// Membership for Quorum Certs/votes
pub quorum_membership: Arc<TYPES::Membership>,

/// Membership for DA committee Votes/certs
pub committee_membership: Arc<TYPES::Membership>,

/// A map of `QuorumVote` collector tasks.
pub vote_collectors: VoteCollectorsMap<TYPES, QuorumVote2<TYPES>, QuorumCertificate2<TYPES>, V>,

/// A map of `TimeoutVote` collector tasks.
pub timeout_vote_collectors:
VoteCollectorsMap<TYPES, TimeoutVote<TYPES>, TimeoutCertificate<TYPES>, V>,

/// This node's storage ref
pub storage: Arc<RwLock<I::Storage>>,

/// The view number that this node is currently executing in.
pub cur_view: TYPES::View,

Expand Down
10 changes: 0 additions & 10 deletions crates/task-impls/src/quorum_proposal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use hotshot_task::{
};
use hotshot_types::{
consensus::OuterConsensus,
event::Event,
message::UpgradeLock,
simple_certificate::{QuorumCertificate2, UpgradeCertificate},
traits::{
Expand Down Expand Up @@ -45,18 +44,9 @@ pub struct QuorumProposalTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>
/// Table for the in-progress proposal dependency tasks.
pub proposal_dependencies: BTreeMap<TYPES::View, JoinHandle<()>>,

/// The underlying network
pub network: Arc<I::Network>,

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

/// Immutable instance state
pub instance_state: Arc<TYPES::InstanceState>,

/// Membership for Timeout votes/certs
pub timeout_membership: Arc<TYPES::Membership>,

/// Membership for Quorum Certs/votes
pub quorum_membership: Arc<TYPES::Membership>,

Expand Down
3 changes: 0 additions & 3 deletions crates/task-impls/src/transactions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ pub struct TransactionTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V
/// Reference to consensus. Leader will require a read lock on this.
pub consensus: OuterConsensus<TYPES>,

/// The underlying network
pub network: Arc<I::Network>,

/// Membership for the quorum
pub membership: Arc<TYPES::Membership>,

Expand Down
13 changes: 4 additions & 9 deletions crates/task-impls/src/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use hotshot_types::{
simple_vote::{UpgradeProposalData, UpgradeVote},
traits::{
election::Membership,
node_implementation::{ConsensusTime, NodeImplementation, NodeType, Versions},
node_implementation::{ConsensusTime, NodeType, Versions},
signature_key::SignatureKey,
},
vote::HasViewNumber,
Expand All @@ -38,7 +38,7 @@ use crate::{
};

/// Tracks state of an upgrade task
pub struct UpgradeTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> {
pub struct UpgradeTaskState<TYPES: NodeType, V: Versions> {
/// Output events to application
pub output_event_stream: async_broadcast::Sender<Event<TYPES>>,

Expand All @@ -51,9 +51,6 @@ pub struct UpgradeTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Ve
/// Membership for Quorum Certs/votes
pub quorum_membership: Arc<TYPES::Membership>,

/// The underlying network
pub network: Arc<I::Network>,

/// A map of `UpgradeVote` collector tasks
pub vote_collectors: VoteCollectorsMap<TYPES, UpgradeVote<TYPES>, UpgradeCertificate<TYPES>, V>,

Expand Down Expand Up @@ -94,7 +91,7 @@ pub struct UpgradeTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Ve
pub upgrade_lock: UpgradeLock<TYPES, V>,
}

impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> UpgradeTaskState<TYPES, I, V> {
impl<TYPES: NodeType, V: Versions> UpgradeTaskState<TYPES, V> {
/// Check if we have decided on an upgrade certificate
async fn upgraded(&self) -> bool {
self.upgrade_lock
Expand Down Expand Up @@ -324,9 +321,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> UpgradeTaskStat

#[async_trait]
/// task state implementation for the upgrade task
impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> TaskState
for UpgradeTaskState<TYPES, I, V>
{
impl<TYPES: NodeType, V: Versions> TaskState for UpgradeTaskState<TYPES, V> {
type Event = HotShotEvent<TYPES>;

async fn handle_event(
Expand Down
3 changes: 0 additions & 3 deletions crates/task-impls/src/vid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,6 @@ pub struct VidTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>> {
/// Our Private Key
pub private_key: <TYPES::SignatureKey as SignatureKey>::PrivateKey,

/// The view and ID of the current vote collection task, if there is one.
pub vote_collector: Option<(TYPES::View, usize, usize)>,

/// This state's ID
pub id: u64,
}
Expand Down
31 changes: 9 additions & 22 deletions crates/task-impls/src/view_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use hotshot_types::{
},
traits::{
election::Membership,
node_implementation::{ConsensusTime, NodeImplementation, NodeType, Versions},
node_implementation::{ConsensusTime, NodeType, Versions},
signature_key::SignatureKey,
},
vote::{Certificate, HasViewNumber, Vote},
Expand Down Expand Up @@ -62,7 +62,7 @@ type RelayMap<TYPES, VOTE, CERT, V> = HashMap<
>;

/// Main view sync task state
pub struct ViewSyncTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> {
pub struct ViewSyncTaskState<TYPES: NodeType, V: Versions> {
/// View HotShot is currently in
pub cur_view: TYPES::View,

Expand All @@ -72,9 +72,6 @@ pub struct ViewSyncTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: V
/// Epoch HotShot is currently in
pub cur_epoch: TYPES::Epoch,

/// The underlying network
pub network: Arc<I::Network>,

/// Membership for the quorum
pub membership: Arc<TYPES::Membership>,

Expand All @@ -91,7 +88,7 @@ pub struct ViewSyncTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: V
pub num_timeouts_tracked: u64,

/// Map of running replica tasks
pub replica_task_map: RwLock<HashMap<TYPES::View, ViewSyncReplicaTaskState<TYPES, I, V>>>,
pub replica_task_map: RwLock<HashMap<TYPES::View, ViewSyncReplicaTaskState<TYPES, V>>>,

/// Map of pre-commit vote accumulates for the relay
pub pre_commit_relay_map: RwLock<
Expand All @@ -118,9 +115,7 @@ pub struct ViewSyncTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: V
}

#[async_trait]
impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> TaskState
for ViewSyncTaskState<TYPES, I, V>
{
impl<TYPES: NodeType, V: Versions> TaskState for ViewSyncTaskState<TYPES, V> {
type Event = HotShotEvent<TYPES>;

async fn handle_event(
Expand All @@ -136,7 +131,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> TaskState
}

/// State of a view sync replica task
pub struct ViewSyncReplicaTaskState<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> {
pub struct ViewSyncReplicaTaskState<TYPES: NodeType, V: Versions> {
/// Timeout for view sync rounds
pub view_sync_timeout: Duration,

Expand Down Expand Up @@ -164,9 +159,6 @@ pub struct ViewSyncReplicaTaskState<TYPES: NodeType, I: NodeImplementation<TYPES
/// Our node id; for logging
pub id: u64,

/// The underlying network
pub network: Arc<I::Network>,

/// Membership for the quorum
pub membership: Arc<TYPES::Membership>,

Expand All @@ -181,9 +173,7 @@ pub struct ViewSyncReplicaTaskState<TYPES: NodeType, I: NodeImplementation<TYPES
}

#[async_trait]
impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> TaskState
for ViewSyncReplicaTaskState<TYPES, I, V>
{
impl<TYPES: NodeType, V: Versions> TaskState for ViewSyncReplicaTaskState<TYPES, V> {
type Event = HotShotEvent<TYPES>;

async fn handle_event(
Expand All @@ -200,7 +190,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> TaskState
fn cancel_subtasks(&mut self) {}
}

impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ViewSyncTaskState<TYPES, I, V> {
impl<TYPES: NodeType, V: Versions> ViewSyncTaskState<TYPES, V> {
#[instrument(skip_all, fields(id = self.id, view = *self.cur_view), name = "View Sync Main Task", level = "error")]
#[allow(clippy::type_complexity)]
/// Handles incoming events for the main view sync task
Expand Down Expand Up @@ -236,7 +226,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ViewSyncTaskSta
}

// We do not have a replica task already running, so start one
let mut replica_state: ViewSyncReplicaTaskState<TYPES, I, V> = ViewSyncReplicaTaskState {
let mut replica_state: ViewSyncReplicaTaskState<TYPES, V> = ViewSyncReplicaTaskState {
cur_view: view,
next_view: view,
cur_epoch: self.cur_epoch,
Expand All @@ -245,7 +235,6 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ViewSyncTaskSta
sent_view_change_event: false,
timeout_task: None,
membership: Arc::clone(&self.membership),
network: Arc::clone(&self.network),
public_key: self.public_key.clone(),
private_key: self.private_key.clone(),
view_sync_timeout: self.view_sync_timeout,
Expand Down Expand Up @@ -523,9 +512,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> ViewSyncTaskSta
}
}

impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>
ViewSyncReplicaTaskState<TYPES, I, V>
{
impl<TYPES: NodeType, V: Versions> ViewSyncReplicaTaskState<TYPES, V> {
#[instrument(skip_all, fields(id = self.id, view = *self.cur_view, epoch = *self.cur_epoch), name = "View Sync Replica Task", level = "error")]
/// Handle incoming events for the view sync replica task
pub async fn handle(
Expand Down
3 changes: 1 addition & 2 deletions crates/testing/tests/tests_1/upgrade_task_with_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ async fn test_upgrade_task_with_proposal() {

let proposal_state =
QuorumProposalTaskState::<TestTypes, MemoryImpl, TestVersions>::create_from(&handle).await;
let upgrade_state =
UpgradeTaskState::<TestTypes, MemoryImpl, TestVersions>::create_from(&handle).await;
let upgrade_state = UpgradeTaskState::<TestTypes, TestVersions>::create_from(&handle).await;

let upgrade_vote_recvs: Vec<_> = upgrade_votes.into_iter().map(UpgradeVoteRecv).collect();

Expand Down
3 changes: 1 addition & 2 deletions crates/testing/tests/tests_1/view_sync_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ async fn test_view_sync_task() {
));
output.push(HotShotEvent::ViewSyncPreCommitVoteSend(vote.clone()));

let view_sync_state =
ViewSyncTaskState::<TestTypes, MemoryImpl, TestVersions>::create_from(&handle).await;
let view_sync_state = ViewSyncTaskState::<TestTypes, TestVersions>::create_from(&handle).await;
run_harness(input, output, view_sync_state, false).await;
}
2 changes: 1 addition & 1 deletion crates/types/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,7 @@ impl<TYPES: NodeType> Consensus<TYPES> {
let mut is_leaf_extended = true;
if let Err(e) = self.visit_leaf_ancestors(
leaf_view,
Terminator::Inclusive(leaf_view - 2),
Terminator::Inclusive(leaf_view - 1),
true,
|leaf, _, _| {
tracing::trace!(
Expand Down

0 comments on commit 3005537

Please sign in to comment.