From fd666afde8f0d68fd83691861fb4dd2270f4947b Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Mon, 27 Jan 2025 18:32:48 -0500 Subject: [PATCH 1/9] Initial types for Membership catchup --- Cargo.lock | 1 + crates/types/Cargo.toml | 1 + crates/types/src/epoch_membership.rs | 57 ++++++++++++++++++++++++++++ crates/types/src/lib.rs | 2 + 4 files changed, 61 insertions(+) create mode 100644 crates/types/src/epoch_membership.rs diff --git a/Cargo.lock b/Cargo.lock index 5726c12d79..eeb185bb9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3165,6 +3165,7 @@ dependencies = [ "ark-serialize", "ark-srs", "ark-std", + "async-broadcast", "async-lock 3.4.0", "async-trait", "bincode", diff --git a/crates/types/Cargo.toml b/crates/types/Cargo.toml index 48e4c30c6b..11d8b81dcc 100644 --- a/crates/types/Cargo.toml +++ b/crates/types/Cargo.toml @@ -7,6 +7,7 @@ version = "0.1.11" [dependencies] anyhow = { workspace = true } +async-broadcast = { workspace = true } ark-bn254 = { workspace = true } ark-ed-on-bn254 = { workspace = true } ark-ff = { workspace = true } diff --git a/crates/types/src/epoch_membership.rs b/crates/types/src/epoch_membership.rs new file mode 100644 index 0000000000..f3d362e629 --- /dev/null +++ b/crates/types/src/epoch_membership.rs @@ -0,0 +1,57 @@ +use std::{collections::HashMap, sync::Arc}; + +use async_broadcast::{broadcast, InactiveReceiver}; +use async_lock::{Mutex, RwLock}; + +use crate::traits::node_implementation::NodeType; + +#[derive(Clone, Debug)] +/// Struct to signal a catchup of a membership is complete +/// Used internally in `EpochMembershipCoordinator` only +struct CatchupSignal {} + +/// Struct to Coordinate membership catchup +pub struct EpochMembershipCoordinator { + /// The underlying membhersip + membership: Arc>, + + /// Any in progress attempts at catching up are stored in this map + /// Any new callers wantin an `EpochMembership` will await on the signal + /// alerting them the membership is ready. The first caller for an epoch will + /// wait for the actual catchup and allert future callers when it's done + catchup_map: Arc>>>, +} + +impl EpochMembershipCoordinator { + /// Get a Membership for a given Epoch, which is guarenteed to have a stake + /// table for the given Epoch + pub async fn membership_for_epoch(&self, epoch: u64) -> EpochMembership { + // CHECK IF MEMBERSHIP HAS EPOCH FIRST + let mut map = self.catchup_map.lock().await; + if let Some(inactive_rx) = map.get(&epoch) { + let mut rx = inactive_rx.activate_cloned(); + drop(map); + let _ = rx.recv().await; + } else { + let (tx, rx) = broadcast(1); + map.insert(epoch, rx.deactivate()); + drop(map); + // do catchup + let _ = tx.broadcast_direct(CatchupSignal {}).await; + }; + + EpochMembership { + epoch, + membership: Arc::clone(&self.membership), + } + } +} + +/// Wrapper around a membership that guarentees that the epoch +/// has a stake table +pub struct EpochMembership { + /// Epoch the `membership` is guarenteed to have a stake table for + pub epoch: u64, + /// Underlying membership + pub membership: Arc>, +} diff --git a/crates/types/src/lib.rs b/crates/types/src/lib.rs index 93076491e1..27dd134097 100644 --- a/crates/types/src/lib.rs +++ b/crates/types/src/lib.rs @@ -22,6 +22,8 @@ pub mod constants; pub mod data; /// Holds the types and functions for DRB computation. pub mod drb; +/// Epoch Membership wrappers +pub mod epoch_membership; pub mod error; pub mod event; /// Holds the configuration file specification for a HotShot node. From ebcd399a96b40bb0d88f2fe23ab048d7c8e318bb Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Thu, 30 Jan 2025 17:02:47 -0500 Subject: [PATCH 2/9] impl catchup --- crates/types/src/epoch_membership.rs | 82 ++++++++++++++++++++++++++-- crates/types/src/traits/election.rs | 8 +++ 2 files changed, 86 insertions(+), 4 deletions(-) diff --git a/crates/types/src/epoch_membership.rs b/crates/types/src/epoch_membership.rs index f3d362e629..aa5583919c 100644 --- a/crates/types/src/epoch_membership.rs +++ b/crates/types/src/epoch_membership.rs @@ -2,8 +2,11 @@ use std::{collections::HashMap, sync::Arc}; use async_broadcast::{broadcast, InactiveReceiver}; use async_lock::{Mutex, RwLock}; +use utils::anytrace::Result; -use crate::traits::node_implementation::NodeType; +use crate::traits::election::Membership; +use crate::traits::node_implementation::{ConsensusTime, NodeType}; +use crate::utils::root_block_in_epoch; #[derive(Clone, Debug)] /// Struct to signal a catchup of a membership is complete @@ -19,13 +22,22 @@ pub struct EpochMembershipCoordinator { /// Any new callers wantin an `EpochMembership` will await on the signal /// alerting them the membership is ready. The first caller for an epoch will /// wait for the actual catchup and allert future callers when it's done - catchup_map: Arc>>>, + catchup_map: Arc>>>, + + /// Number of blocks in an epoch + epoch_height: u64, } impl EpochMembershipCoordinator { /// Get a Membership for a given Epoch, which is guarenteed to have a stake /// table for the given Epoch - pub async fn membership_for_epoch(&self, epoch: u64) -> EpochMembership { + pub async fn membership_for_epoch(&self, epoch: TYPES::Epoch) -> EpochMembership { + if self.membership.read().await.has_epoch(epoch).await { + return EpochMembership { + epoch, + membership: Arc::clone(&self.membership), + }; + } // CHECK IF MEMBERSHIP HAS EPOCH FIRST let mut map = self.catchup_map.lock().await; if let Some(inactive_rx) = map.get(&epoch) { @@ -37,6 +49,7 @@ impl EpochMembershipCoordinator { map.insert(epoch, rx.deactivate()); drop(map); // do catchup + self.catchup(epoch).await; let _ = tx.broadcast_direct(CatchupSignal {}).await; }; @@ -45,13 +58,74 @@ impl EpochMembershipCoordinator { membership: Arc::clone(&self.membership), } } + + /// Catches the membership up to the epoch passed as an argument. + /// To do this try to get the stake table for the epoch containing this epoch's root + /// if the root does not exist recusively catchup until you've found it + /// + /// If there is another catchup in progress this will not duplicate efforts + /// e.g. if we start with only epoch 0 stake table and call catchup for epoch 10, then call catchup for epoch 20 + /// the first caller will actually do the work for to catchup to epoch 10 then the second caller will continue + /// catching up to epoch 20 + async fn catchup(&self, epoch: TYPES::Epoch) { + // recursively catchup until we have a stake table for the epoch containing our root + assert!( + *epoch != 0, + "We are trying to catchup to epoch 0! This means the initial stake table is missing!" + ); + let root_epoch = if *epoch == 1 { + TYPES::Epoch::new(*epoch - 1) + } else { + TYPES::Epoch::new(*epoch - 2) + }; + let root_membership = self.membership_for_epoch(root_epoch).await; + + // Get the epoch root headers and update our membership with them, finally sync them + // Verification of the root is handled in get_epoch_root + let Ok((next_epoch, header)) = root_membership + .get_epoch_root(root_block_in_epoch(*root_epoch, self.epoch_height)) + .await + else { + return; + }; + let Some(updater) = self + .membership + .read() + .await + .add_epoch_root(next_epoch, header) + .await + else { + return; + }; + updater(&mut *(self.membership.write().await)); + + let Some(sync) = self.membership.read().await.sync_l1().await else { + return; + }; + sync(&mut *(self.membership.write().await)); + } } /// Wrapper around a membership that guarentees that the epoch /// has a stake table pub struct EpochMembership { /// Epoch the `membership` is guarenteed to have a stake table for - pub epoch: u64, + pub epoch: TYPES::Epoch, /// Underlying membership pub membership: Arc>, } + +impl EpochMembership { + /// Wraps the same named Membership trait fn + async fn get_epoch_root( + &self, + block_height: u64, + ) -> Result<(TYPES::Epoch, TYPES::BlockHeader)> { + self.membership + .read() + .await + .get_epoch_root(block_height) + .await + } + // TODO add function wrappers +} diff --git a/crates/types/src/traits/election.rs b/crates/types/src/traits/election.rs index 4e5347f49d..bba46ccc8e 100644 --- a/crates/types/src/traits/election.rs +++ b/crates/types/src/traits/election.rs @@ -132,6 +132,14 @@ pub trait Membership: Debug + Send + Sync { /// Returns the threshold required to upgrade the network protocol fn upgrade_threshold(&self, epoch: Option) -> NonZeroU64; + /// Returns if the stake table is available for the current Epoch + async fn has_epoch(&self, epoch: TYPES::Epoch) -> bool; + + /// Gets the validated block header and epoch number of the epoch root + /// at the given block height + async fn get_epoch_root(&self, block_height: u64) + -> Result<(TYPES::Epoch, TYPES::BlockHeader)>; + #[allow(clippy::type_complexity)] /// Handles notifications that a new epoch root has been created /// Is called under a read lock to the Membership. Return a callback From b8dc43eeed7b7dbe717a9a9a6ff9e904fb359fb3 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Fri, 31 Jan 2025 14:56:06 -0500 Subject: [PATCH 3/9] add functons to membership --- .../traits/election/randomized_committee.rs | 15 +++++++++++-- .../election/randomized_committee_members.rs | 16 +++++++++++--- .../src/traits/election/static_committee.rs | 13 ++++++++++- .../static_committee_leader_two_views.rs | 13 ++++++++++- .../traits/election/two_static_committees.rs | 13 ++++++++++- crates/types/src/epoch_membership.rs | 2 +- crates/types/src/traits/election.rs | 22 ++++++++++--------- 7 files changed, 75 insertions(+), 19 deletions(-) diff --git a/crates/hotshot/src/traits/election/randomized_committee.rs b/crates/hotshot/src/traits/election/randomized_committee.rs index 80d9185841..3f9d29dd4c 100644 --- a/crates/hotshot/src/traits/election/randomized_committee.rs +++ b/crates/hotshot/src/traits/election/randomized_committee.rs @@ -6,6 +6,7 @@ use std::{cmp::max, collections::BTreeMap, num::NonZeroU64}; +use crate::traits::election::randomized_committee::anytrace::line_info; use hotshot_types::{ traits::{ election::Membership, @@ -16,8 +17,7 @@ use hotshot_types::{ }; use primitive_types::U256; use rand::{rngs::StdRng, Rng}; -use utils::anytrace::Result; - +use utils::anytrace::{self, Error, Level, Result}; #[derive(Clone, Debug, Eq, PartialEq, Hash)] /// The static committee election @@ -248,4 +248,15 @@ impl Membership for RandomizedCommittee { )) .unwrap() } + + fn has_epoch(&self, _epoch: TYPES::Epoch) -> bool { + true + } + + async fn get_epoch_root( + &self, + _block_height: u64, + ) -> Result<(TYPES::Epoch, TYPES::BlockHeader)> { + Err(anytrace::error!("Not implemented")) + } } diff --git a/crates/hotshot/src/traits/election/randomized_committee_members.rs b/crates/hotshot/src/traits/election/randomized_committee_members.rs index f5a60641e4..dfbf324a4c 100644 --- a/crates/hotshot/src/traits/election/randomized_committee_members.rs +++ b/crates/hotshot/src/traits/election/randomized_committee_members.rs @@ -11,6 +11,7 @@ use std::{ num::NonZeroU64, }; +use crate::traits::election::helpers::QuorumFilterConfig; use hotshot_types::{ traits::{ election::Membership, @@ -21,9 +22,8 @@ use hotshot_types::{ }; use primitive_types::U256; use rand::{rngs::StdRng, Rng}; -use utils::anytrace::Result; - -use crate::traits::election::helpers::QuorumFilterConfig; +use utils::anytrace::{self, Error, Level, Result}; +use utils::line_info; #[derive(Clone, Debug, Eq, PartialEq, Hash)] /// The static committee election @@ -412,4 +412,14 @@ impl Membership let len = self.total_nodes(epoch); NonZeroU64::new(max((len as u64 * 9) / 10, ((len as u64 * 2) / 3) + 1)).unwrap() } + fn has_epoch(&self, _epoch: TYPES::Epoch) -> bool { + true + } + + async fn get_epoch_root( + &self, + _block_height: u64, + ) -> Result<(TYPES::Epoch, TYPES::BlockHeader)> { + Err(anytrace::error!("Not implemented")) + } } diff --git a/crates/hotshot/src/traits/election/static_committee.rs b/crates/hotshot/src/traits/election/static_committee.rs index deae814155..7da2f30b22 100644 --- a/crates/hotshot/src/traits/election/static_committee.rs +++ b/crates/hotshot/src/traits/election/static_committee.rs @@ -15,7 +15,8 @@ use hotshot_types::{ PeerConfig, }; use primitive_types::U256; -use utils::anytrace::Result; +use utils::anytrace::{self, Error, Level, Result}; +use utils::line_info; #[derive(Clone, Debug, Eq, PartialEq, Hash)] /// The static committee election @@ -234,4 +235,14 @@ impl Membership for StaticCommittee { let len = self.stake_table.len(); NonZeroU64::new(max((len as u64 * 9) / 10, ((len as u64 * 2) / 3) + 1)).unwrap() } + fn has_epoch(&self, _epoch: TYPES::Epoch) -> bool { + true + } + + async fn get_epoch_root( + &self, + _block_height: u64, + ) -> Result<(TYPES::Epoch, TYPES::BlockHeader)> { + Err(anytrace::error!("Not implemented")) + } } diff --git a/crates/hotshot/src/traits/election/static_committee_leader_two_views.rs b/crates/hotshot/src/traits/election/static_committee_leader_two_views.rs index 60f98684b9..4681bc0b87 100644 --- a/crates/hotshot/src/traits/election/static_committee_leader_two_views.rs +++ b/crates/hotshot/src/traits/election/static_committee_leader_two_views.rs @@ -15,7 +15,8 @@ use hotshot_types::{ PeerConfig, }; use primitive_types::U256; -use utils::anytrace::Result; +use utils::anytrace::{self, Error, Level, Result}; +use utils::line_info; #[derive(Clone, Debug, Eq, PartialEq, Hash)] @@ -235,4 +236,14 @@ impl Membership for StaticCommitteeLeaderForTwoViews::Epoch>) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64 * 9) / 10) + 1).unwrap() } + fn has_epoch(&self, _epoch: TYPES::Epoch) -> bool { + true + } + + async fn get_epoch_root( + &self, + _block_height: u64, + ) -> Result<(TYPES::Epoch, TYPES::BlockHeader)> { + Err(anytrace::error!("Not implemented")) + } } diff --git a/crates/hotshot/src/traits/election/two_static_committees.rs b/crates/hotshot/src/traits/election/two_static_committees.rs index f6d5642d9f..8405a6e45e 100644 --- a/crates/hotshot/src/traits/election/two_static_committees.rs +++ b/crates/hotshot/src/traits/election/two_static_committees.rs @@ -15,7 +15,8 @@ use hotshot_types::{ PeerConfig, }; use primitive_types::U256; -use utils::anytrace::Result; +use utils::anytrace::{self, Error, Level, Result}; +use utils::line_info; /// Tuple type for eligible leaders type EligibleLeaders = ( @@ -424,4 +425,14 @@ impl Membership for TwoStaticCommittees { .unwrap() } } + fn has_epoch(&self, _epoch: TYPES::Epoch) -> bool { + true + } + + async fn get_epoch_root( + &self, + _block_height: u64, + ) -> Result<(TYPES::Epoch, TYPES::BlockHeader)> { + Err(anytrace::error!("Not implemented")) + } } diff --git a/crates/types/src/epoch_membership.rs b/crates/types/src/epoch_membership.rs index aa5583919c..def15399ca 100644 --- a/crates/types/src/epoch_membership.rs +++ b/crates/types/src/epoch_membership.rs @@ -32,7 +32,7 @@ impl EpochMembershipCoordinator { /// Get a Membership for a given Epoch, which is guarenteed to have a stake /// table for the given Epoch pub async fn membership_for_epoch(&self, epoch: TYPES::Epoch) -> EpochMembership { - if self.membership.read().await.has_epoch(epoch).await { + if self.membership.read().await.has_epoch(epoch) { return EpochMembership { epoch, membership: Arc::clone(&self.membership), diff --git a/crates/types/src/traits/election.rs b/crates/types/src/traits/election.rs index bba46ccc8e..e3461d33f1 100644 --- a/crates/types/src/traits/election.rs +++ b/crates/types/src/traits/election.rs @@ -7,13 +7,11 @@ //! The election trait, used to decide which node is the leader and determine if a vote is valid. use std::{collections::BTreeSet, fmt::Debug, num::NonZeroU64}; -use async_trait::async_trait; use utils::anytrace::Result; use super::node_implementation::NodeType; use crate::{traits::signature_key::SignatureKey, PeerConfig}; -#[async_trait] /// A protocol for determining membership in and participating in a committee. pub trait Membership: Debug + Send + Sync { /// The error type returned by methods like `lookup_leader`. @@ -133,12 +131,14 @@ pub trait Membership: Debug + Send + Sync { fn upgrade_threshold(&self, epoch: Option) -> NonZeroU64; /// Returns if the stake table is available for the current Epoch - async fn has_epoch(&self, epoch: TYPES::Epoch) -> bool; + fn has_epoch(&self, epoch: TYPES::Epoch) -> bool; /// Gets the validated block header and epoch number of the epoch root /// at the given block height - async fn get_epoch_root(&self, block_height: u64) - -> Result<(TYPES::Epoch, TYPES::BlockHeader)>; + fn get_epoch_root( + &self, + block_height: u64, + ) -> impl std::future::Future> + Send; #[allow(clippy::type_complexity)] /// Handles notifications that a new epoch root has been created @@ -146,18 +146,20 @@ pub trait Membership: Debug + Send + Sync { /// with Some to have that callback invoked under a write lock. /// /// #3967 REVIEW NOTE: this is only called if epoch is Some. Is there any reason to do otherwise? - async fn add_epoch_root( + fn add_epoch_root( &self, _epoch: TYPES::Epoch, _block_header: TYPES::BlockHeader, - ) -> Option> { - None + ) -> impl std::future::Future>> + Send { + async { None } } #[allow(clippy::type_complexity)] /// Called after add_epoch_root runs and any callback has been invoked. /// Causes a read lock to be reacquired for this functionality. - async fn sync_l1(&self) -> Option> { - None + fn sync_l1( + &self, + ) -> impl std::future::Future>> + Send { + async { None } } } From 440bfc67add540b3eb4072c5078a3d79556b76e6 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Mon, 3 Feb 2025 22:19:06 -0500 Subject: [PATCH 4/9] Refactor to use epochs in certs and votes --- .../traits/election/randomized_committee.rs | 5 - crates/task-impls/src/vote_collection.rs | 100 +++------ crates/testing/src/helpers.rs | 12 +- crates/types/src/epoch_membership.rs | 208 +++++++++++++++--- crates/types/src/simple_certificate.rs | 112 ++++------ crates/types/src/vote.rs | 42 ++-- 6 files changed, 279 insertions(+), 200 deletions(-) diff --git a/crates/hotshot/src/traits/election/randomized_committee.rs b/crates/hotshot/src/traits/election/randomized_committee.rs index 3f9d29dd4c..24e412c09b 100644 --- a/crates/hotshot/src/traits/election/randomized_committee.rs +++ b/crates/hotshot/src/traits/election/randomized_committee.rs @@ -195,11 +195,6 @@ impl Membership for RandomizedCommittee { .is_some_and(|x| x.stake() > U256::zero()) } - // /// Get the network topic for the committee - // fn committee_topic(&self) -> Topic { - // self.committee_topic.clone() - // } - /// Index the vector of public keys with the current view number fn lookup_leader( &self, diff --git a/crates/task-impls/src/vote_collection.rs b/crates/task-impls/src/vote_collection.rs index 2147c9ca15..a556f48651 100644 --- a/crates/task-impls/src/vote_collection.rs +++ b/crates/task-impls/src/vote_collection.rs @@ -7,6 +7,7 @@ use std::{ collections::{btree_map::Entry, BTreeMap, HashMap}, fmt::Debug, + future::Future, marker::PhantomData, sync::Arc, }; @@ -16,6 +17,7 @@ use async_lock::RwLock; use async_trait::async_trait; use either::Either::{Left, Right}; use hotshot_types::{ + epoch_membership::{EpochMembership, EpochMembershipCoordinator}, message::UpgradeLock, simple_certificate::{ DaCertificate2, NextEpochQuorumCertificate2, QuorumCertificate, QuorumCertificate2, @@ -26,10 +28,7 @@ use hotshot_types::{ DaVote2, NextEpochQuorumVote2, QuorumVote, QuorumVote2, TimeoutVote2, UpgradeVote, ViewSyncCommitVote2, ViewSyncFinalizeVote2, ViewSyncPreCommitVote2, }, - traits::{ - election::Membership, - node_implementation::{NodeType, Versions}, - }, + traits::node_implementation::{NodeType, Versions}, utils::EpochTransitionIndicator, vote::{Certificate, HasViewNumber, Vote, VoteAccumulator}, }; @@ -83,9 +82,8 @@ pub trait AggregatableVote< /// if the leader cannot be calculated fn leader( &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result; + membership: &EpochMembership, + ) -> impl Future>; /// return the Hotshot event for the completion of this CERT fn make_cert_event(certificate: CERT, key: &TYPES::SignatureKey) -> HotShotEvent; @@ -110,11 +108,14 @@ impl< sender_epoch: Option, event_stream: &Sender>>, ) -> Result> { + // TODO create this only once + let coordinator = EpochMembershipCoordinator::new(Arc::clone(&self.membership), 10); + let epoch_membership = coordinator.membership_for_epoch(sender_epoch).await; ensure!( matches!( self.transition_indicator, EpochTransitionIndicator::InTransition - ) || vote.leader(&*self.membership.read().await, self.epoch)? == self.public_key, + ) || vote.leader(&epoch_membership).await? == self.public_key, info!("Received vote for a view in which we were not the leader.") ); @@ -131,10 +132,7 @@ impl< "No accumulator to handle vote with. This shouldn't happen." ))?; - match accumulator - .accumulate(vote, &self.membership, sender_epoch) - .await - { + match accumulator.accumulate(vote, epoch_membership).await { None => Ok(None), Some(cert) => { tracing::debug!("Certificate Formed! {:?}", cert); @@ -356,12 +354,8 @@ type ViewSyncFinalizeVoteState = VoteCollectionTaskState< impl AggregatableVote, QuorumCertificate> for QuorumVote { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.view_number() + 1, epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership.leader(self.view_number() + 1).await } fn make_cert_event( certificate: QuorumCertificate, @@ -374,12 +368,8 @@ impl AggregatableVote, QuorumCertifica impl AggregatableVote, QuorumCertificate2> for QuorumVote2 { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.view_number() + 1, epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership.leader(self.view_number() + 1).await } fn make_cert_event( certificate: QuorumCertificate2, @@ -393,12 +383,8 @@ impl AggregatableVote, NextEpochQuorumCertificate2> for NextEpochQuorumVote2 { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.view_number() + 1, epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership.leader(self.view_number() + 1).await } fn make_cert_event( certificate: NextEpochQuorumCertificate2, @@ -411,12 +397,8 @@ impl impl AggregatableVote, UpgradeCertificate> for UpgradeVote { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.view_number(), epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership.leader(self.view_number()).await } fn make_cert_event( certificate: UpgradeCertificate, @@ -429,12 +411,8 @@ impl AggregatableVote, UpgradeCertifi impl AggregatableVote, DaCertificate2> for DaVote2 { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.view_number(), epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership.leader(self.view_number()).await } fn make_cert_event( certificate: DaCertificate2, @@ -447,12 +425,8 @@ impl AggregatableVote, DaCertificate2 AggregatableVote, TimeoutCertificate2> for TimeoutVote2 { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.view_number() + 1, epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership.leader(self.view_number() + 1).await } fn make_cert_event( certificate: TimeoutCertificate2, @@ -466,12 +440,10 @@ impl AggregatableVote, ViewSyncCommitCertificate2> for ViewSyncCommitVote2 { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.date().round + self.date().relay, epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership + .leader(self.date().round + self.date().relay) + .await } fn make_cert_event( certificate: ViewSyncCommitCertificate2, @@ -485,12 +457,10 @@ impl AggregatableVote, ViewSyncPreCommitCertificate2> for ViewSyncPreCommitVote2 { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.date().round + self.date().relay, epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership + .leader(self.date().round + self.date().relay) + .await } fn make_cert_event( certificate: ViewSyncPreCommitCertificate2, @@ -504,12 +474,10 @@ impl AggregatableVote, ViewSyncFinalizeCertificate2> for ViewSyncFinalizeVote2 { - fn leader( - &self, - membership: &TYPES::Membership, - epoch: Option, - ) -> Result { - membership.leader(self.date().round + self.date().relay, epoch) + async fn leader(&self, membership: &EpochMembership) -> Result { + membership + .leader(self.date().round + self.date().relay) + .await } fn make_cert_event( certificate: ViewSyncFinalizeCertificate2, diff --git a/crates/testing/src/helpers.rs b/crates/testing/src/helpers.rs index 202afa0f47..1afb17d43a 100644 --- a/crates/testing/src/helpers.rs +++ b/crates/testing/src/helpers.rs @@ -27,6 +27,7 @@ use hotshot_task_impls::events::HotShotEvent; use hotshot_types::{ consensus::ConsensusMetricsValue, data::{Leaf2, VidDisperse, VidDisperseShare2}, + epoch_membership::EpochMembership, message::{Proposal, UpgradeLock}, simple_certificate::DaCertificate2, simple_vote::{DaData2, DaVote2, SimpleVote, VersionedVoteData}, @@ -215,14 +216,17 @@ pub async fn build_assembled_sig< epoch: Option, upgrade_lock: &UpgradeLock, ) -> ::QcType { - let membership_reader = membership.read().await; - let stake_table = CERT::stake_table(&*membership_reader, epoch); + // TODO get actual height + let epoch_membership = EpochMembership { + epoch, + membership: Arc::clone(membership), + }; + let stake_table = CERT::stake_table(&epoch_membership).await; let real_qc_pp: ::QcParams = ::public_parameter( stake_table.clone(), - U256::from(CERT::threshold(&*membership_reader, epoch)), + U256::from(CERT::threshold(&epoch_membership).await), ); - drop(membership_reader); let total_nodes = stake_table.len(); let signers = bitvec![1; total_nodes]; diff --git a/crates/types/src/epoch_membership.rs b/crates/types/src/epoch_membership.rs index def15399ca..d2fd99a8a0 100644 --- a/crates/types/src/epoch_membership.rs +++ b/crates/types/src/epoch_membership.rs @@ -1,3 +1,5 @@ +use std::collections::BTreeSet; +use std::num::NonZeroU64; use std::{collections::HashMap, sync::Arc}; use async_broadcast::{broadcast, InactiveReceiver}; @@ -6,6 +8,7 @@ use utils::anytrace::Result; use crate::traits::election::Membership; use crate::traits::node_implementation::{ConsensusTime, NodeType}; +use crate::traits::signature_key::SignatureKey; use crate::utils::root_block_in_epoch; #[derive(Clone, Debug)] @@ -14,6 +17,7 @@ use crate::utils::root_block_in_epoch; struct CatchupSignal {} /// Struct to Coordinate membership catchup +#[derive(Clone)] pub struct EpochMembershipCoordinator { /// The underlying membhersip membership: Arc>, @@ -28,35 +32,53 @@ pub struct EpochMembershipCoordinator { epoch_height: u64, } -impl EpochMembershipCoordinator { +// async fn catchup_membership(coordinator: EpochMembershipCoordinator) { + +// } + +impl EpochMembershipCoordinator +where + Self: Send, +{ + /// Create an EpochMembershipCoordinator + pub fn new(membership: Arc>, epoch_height: u64) -> Self { + Self { + membership, + catchup_map: Arc::default(), + epoch_height, + } + } /// Get a Membership for a given Epoch, which is guarenteed to have a stake /// table for the given Epoch - pub async fn membership_for_epoch(&self, epoch: TYPES::Epoch) -> EpochMembership { + pub async fn membership_for_epoch( + &self, + maybe_epoch: Option, + ) -> EpochMembership { + let ret_val = EpochMembership { + epoch: maybe_epoch, + membership: Arc::clone(&self.membership), + }; + let Some(epoch) = maybe_epoch else { + return ret_val; + }; if self.membership.read().await.has_epoch(epoch) { - return EpochMembership { - epoch, - membership: Arc::clone(&self.membership), - }; + return ret_val; } - // CHECK IF MEMBERSHIP HAS EPOCH FIRST - let mut map = self.catchup_map.lock().await; - if let Some(inactive_rx) = map.get(&epoch) { - let mut rx = inactive_rx.activate_cloned(); - drop(map); - let _ = rx.recv().await; - } else { + let tx = { + let mut map = self.catchup_map.lock().await; + if let Some(inactive_rx) = map.get(&epoch) { + let mut rx = inactive_rx.activate_cloned(); + let _ = rx.recv().await; + return ret_val; + } let (tx, rx) = broadcast(1); map.insert(epoch, rx.deactivate()); - drop(map); - // do catchup - self.catchup(epoch).await; - let _ = tx.broadcast_direct(CatchupSignal {}).await; + tx }; - - EpochMembership { - epoch, - membership: Arc::clone(&self.membership), - } + // do catchup + self.catchup(epoch).await; + let _ = tx.broadcast_direct(CatchupSignal {}).await; + ret_val } /// Catches the membership up to the epoch passed as an argument. @@ -78,7 +100,7 @@ impl EpochMembershipCoordinator { } else { TYPES::Epoch::new(*epoch - 2) }; - let root_membership = self.membership_for_epoch(root_epoch).await; + let root_membership = Box::pin(self.membership_for_epoch(Some(root_epoch))).await; // Get the epoch root headers and update our membership with them, finally sync them // Verification of the root is handled in get_epoch_root @@ -110,7 +132,7 @@ impl EpochMembershipCoordinator { /// has a stake table pub struct EpochMembership { /// Epoch the `membership` is guarenteed to have a stake table for - pub epoch: TYPES::Epoch, + pub epoch: Option, /// Underlying membership pub membership: Arc>, } @@ -127,5 +149,141 @@ impl EpochMembership { .get_epoch_root(block_height) .await } - // TODO add function wrappers + + /// Get all participants in the committee (including their stake) for a specific epoch + pub async fn stake_table(&self) -> Vec<::StakeTableEntry> { + self.membership.read().await.stake_table(self.epoch) + } + + /// Get all participants in the committee (including their stake) for a specific epoch + pub async fn da_stake_table( + &self, + ) -> Vec<::StakeTableEntry> { + self.membership.read().await.da_stake_table(self.epoch) + } + + /// Get all participants in the committee for a specific view for a specific epoch + pub async fn committee_members( + &self, + view_number: TYPES::View, + ) -> BTreeSet { + self.membership + .read() + .await + .committee_members(view_number, self.epoch) + } + + /// Get all participants in the committee for a specific view for a specific epoch + pub async fn da_committee_members( + &self, + view_number: TYPES::View, + ) -> BTreeSet { + self.membership + .read() + .await + .da_committee_members(view_number, self.epoch) + } + + /// Get all leaders in the committee for a specific view for a specific epoch + pub async fn committee_leaders( + &self, + view_number: TYPES::View, + ) -> BTreeSet { + self.membership + .read() + .await + .committee_leaders(view_number, self.epoch) + } + + /// Get the stake table entry for a public key, returns `None` if the + /// key is not in the table for a specific epoch + pub async fn stake( + &self, + pub_key: &TYPES::SignatureKey, + ) -> Option<::StakeTableEntry> { + self.membership.read().await.stake(pub_key, self.epoch) + } + + /// Get the DA stake table entry for a public key, returns `None` if the + /// key is not in the table for a specific epoch + pub async fn da_stake( + &self, + pub_key: &TYPES::SignatureKey, + ) -> Option<::StakeTableEntry> { + self.membership.read().await.da_stake(pub_key, self.epoch) + } + + /// See if a node has stake in the committee in a specific epoch + pub async fn has_stake(&self, pub_key: &TYPES::SignatureKey) -> bool { + self.membership.read().await.has_stake(pub_key, self.epoch) + } + + /// See if a node has stake in the committee in a specific epoch + pub async fn has_da_stake(&self, pub_key: &TYPES::SignatureKey) -> bool { + self.membership + .read() + .await + .has_da_stake(pub_key, self.epoch) + } + + /// The leader of the committee for view `view_number` in `epoch`. + /// + /// Note: this function uses a HotShot-internal error type. + /// You should implement `lookup_leader`, rather than implementing this function directly. + /// + /// # Errors + /// Returns an error if the leader cannot be calculated. + pub async fn leader(&self, view: TYPES::View) -> Result { + self.membership.read().await.leader(view, self.epoch) + } + + /// The leader of the committee for view `view_number` in `epoch`. + /// + /// Note: There is no such thing as a DA leader, so any consumer + /// requiring a leader should call this. + /// + /// # Errors + /// Returns an error if the leader cannot be calculated + pub async fn lookup_leader( + &self, + view: TYPES::View, + ) -> std::result::Result< + TYPES::SignatureKey, + <::Membership as Membership>::Error, + > { + self.membership.read().await.lookup_leader(view, self.epoch) + } + + /// Returns the number of total nodes in the committee in an epoch `epoch` + pub async fn total_nodes(&self) -> usize { + self.membership.read().await.total_nodes(self.epoch) + } + + /// Returns the number of total DA nodes in the committee in an epoch `epoch` + pub async fn da_total_nodes(&self) -> usize { + self.membership.read().await.da_total_nodes(self.epoch) + } + + /// Returns the threshold for a specific `Membership` implementation + pub async fn success_threshold(&self) -> NonZeroU64 { + self.membership.read().await.success_threshold(self.epoch) + } + + /// Returns the DA threshold for a specific `Membership` implementation + pub async fn da_success_threshold(&self) -> NonZeroU64 { + self.membership + .read() + .await + .da_success_threshold(self.epoch) + } + + /// Returns the threshold for a specific `Membership` implementation + pub async fn failure_threshold(&self) -> NonZeroU64 { + self.membership.read().await.failure_threshold(self.epoch) + } + + /// Returns the threshold required to upgrade the network protocol + pub async fn upgrade_threshold(&self) -> NonZeroU64 { + self.membership.read().await.upgrade_threshold(self.epoch) + } } diff --git a/crates/types/src/simple_certificate.rs b/crates/types/src/simple_certificate.rs index a6b4a1b142..d4828346b3 100644 --- a/crates/types/src/simple_certificate.rs +++ b/crates/types/src/simple_certificate.rs @@ -8,6 +8,7 @@ use std::{ fmt::{self, Debug, Display, Formatter}, + future::Future, hash::Hash, marker::PhantomData, num::NonZeroU64, @@ -22,6 +23,7 @@ use utils::anytrace::*; use crate::{ data::serialize_signature2, + epoch_membership::EpochMembership, message::UpgradeLock, simple_vote::{ DaData, DaData2, NextEpochQuorumData2, QuorumData, QuorumData2, QuorumMarker, TimeoutData, @@ -40,10 +42,7 @@ use crate::{ /// Trait which allows use to inject different threshold calculations into a Certificate type pub trait Threshold { /// Calculate a threshold based on the membership - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64; + fn threshold(membership: &EpochMembership) -> impl Future + Send; } /// Defines a threshold which is 2f + 1 (Amount needed for Quorum) @@ -51,11 +50,8 @@ pub trait Threshold { pub struct SuccessThreshold {} impl Threshold for SuccessThreshold { - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64 { - membership.success_threshold(epoch).into() + async fn threshold(membership: &EpochMembership) -> u64 { + membership.success_threshold().await.into() } } @@ -64,11 +60,8 @@ impl Threshold for SuccessThreshold { pub struct OneHonestThreshold {} impl Threshold for OneHonestThreshold { - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64 { - membership.failure_threshold(epoch).into() + async fn threshold(membership: &EpochMembership) -> u64 { + membership.failure_threshold().await.into() } } @@ -77,11 +70,8 @@ impl Threshold for OneHonestThreshold { pub struct UpgradeThreshold {} impl Threshold for UpgradeThreshold { - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64 { - membership.upgrade_threshold(epoch).into() + async fn threshold(membership: &EpochMembership) -> u64 { + membership.upgrade_threshold().await.into() } } @@ -187,33 +177,25 @@ impl> Certificate ) } /// Proxy's to `Membership.stake` - fn stake_table_entry>( - membership: &MEMBERSHIP, + async fn stake_table_entry( + membership: &EpochMembership, pub_key: &TYPES::SignatureKey, - epoch: Option, ) -> Option<::StakeTableEntry> { - membership.da_stake(pub_key, epoch) + membership.da_stake(pub_key).await } /// Proxy's to `Membership.da_stake_table` - fn stake_table>( - membership: &MEMBERSHIP, - epoch: Option, + async fn stake_table( + membership: &EpochMembership, ) -> Vec<::StakeTableEntry> { - membership.da_stake_table(epoch) + membership.da_stake_table().await } /// Proxy's to `Membership.da_total_nodes` - fn total_nodes>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> usize { - membership.da_total_nodes(epoch) + async fn total_nodes(membership: &EpochMembership) -> usize { + membership.da_total_nodes().await } - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64 { - membership.da_success_threshold(epoch).into() + async fn threshold(membership: &EpochMembership) -> u64 { + membership.da_success_threshold().await.into() } fn data(&self) -> &Self::Voteable { &self.data @@ -275,33 +257,25 @@ impl> Certificate>( - membership: &MEMBERSHIP, + async fn stake_table_entry( + membership: &EpochMembership, pub_key: &TYPES::SignatureKey, - epoch: Option, ) -> Option<::StakeTableEntry> { - membership.da_stake(pub_key, epoch) + membership.da_stake(pub_key).await } /// Proxy's to `Membership.da_stake_table` - fn stake_table>( - membership: &MEMBERSHIP, - epoch: Option, + async fn stake_table( + membership: &EpochMembership, ) -> Vec<::StakeTableEntry> { - membership.da_stake_table(epoch) + membership.da_stake_table().await } /// Proxy's to `Membership.da_total_nodes` - fn total_nodes>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> usize { - membership.da_total_nodes(epoch) + async fn total_nodes(membership: &EpochMembership) -> usize { + membership.da_total_nodes().await } - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64 { - membership.da_success_threshold(epoch).into() + async fn threshold(membership: &EpochMembership) -> u64 { + membership.da_success_threshold().await.into() } fn data(&self) -> &Self::Voteable { &self.data @@ -365,34 +339,26 @@ impl< self.signatures.as_ref().unwrap(), ) } - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64 { - THRESHOLD::threshold(membership, epoch) + async fn threshold(membership: &EpochMembership) -> u64 { + THRESHOLD::threshold(membership).await } - fn stake_table_entry>( - membership: &MEMBERSHIP, + async fn stake_table_entry( + membership: &EpochMembership, pub_key: &TYPES::SignatureKey, - epoch: Option, ) -> Option<::StakeTableEntry> { - membership.stake(pub_key, epoch) + membership.stake(pub_key).await } - fn stake_table>( - membership: &MEMBERSHIP, - epoch: Option, + async fn stake_table( + membership: &EpochMembership, ) -> Vec<::StakeTableEntry> { - membership.stake_table(epoch) + membership.stake_table().await } /// Proxy's to `Membership.total_nodes` - fn total_nodes>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> usize { - membership.total_nodes(epoch) + async fn total_nodes(membership: &EpochMembership) -> usize { + membership.total_nodes().await } fn data(&self) -> &Self::Voteable { diff --git a/crates/types/src/vote.rs b/crates/types/src/vote.rs index 7f91dfd01c..8731c3b2f7 100644 --- a/crates/types/src/vote.rs +++ b/crates/types/src/vote.rs @@ -8,12 +8,11 @@ use std::{ collections::{BTreeMap, HashMap}, + future::Future, marker::PhantomData, num::NonZeroU64, - sync::Arc, }; -use async_lock::RwLock; use bitvec::{bitvec, vec::BitVec}; use committable::{Commitment, Committable}; use primitive_types::U256; @@ -21,11 +20,11 @@ use tracing::error; use utils::anytrace::Result; use crate::{ + epoch_membership::EpochMembership, message::UpgradeLock, simple_certificate::Threshold, simple_vote::{VersionedVoteData, Voteable}, traits::{ - election::Membership, node_implementation::{NodeType, Versions}, signature_key::{SignatureKey, StakeTableEntryType}, }, @@ -82,29 +81,21 @@ pub trait Certificate: HasViewNumber { ) -> impl std::future::Future; /// Returns the amount of stake needed to create this certificate // TODO: Make this a static ratio of the total stake of `Membership` - fn threshold>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> u64; + fn threshold(membership: &EpochMembership) -> impl Future + Send; /// Get Stake Table from Membership implementation. - fn stake_table>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> Vec<::StakeTableEntry>; + fn stake_table( + membership: &EpochMembership, + ) -> impl Future::StakeTableEntry>> + Send; /// Get Total Nodes from Membership implementation. - fn total_nodes>( - membership: &MEMBERSHIP, - epoch: Option, - ) -> usize; + fn total_nodes(membership: &EpochMembership) -> impl Future + Send; /// Get `StakeTableEntry` from Membership implementation. - fn stake_table_entry>( - membership: &MEMBERSHIP, + fn stake_table_entry( + membership: &EpochMembership, pub_key: &TYPES::SignatureKey, - epoch: Option, - ) -> Option<::StakeTableEntry>; + ) -> impl Future::StakeTableEntry>> + Send; /// Get the commitment which was voted on fn data(&self) -> &Self::Voteable; @@ -163,8 +154,7 @@ impl< pub async fn accumulate( &mut self, vote: &VOTE, - membership: &Arc>, - epoch: Option, + membership: EpochMembership, ) -> Option { let key = vote.signing_key(); @@ -187,12 +177,10 @@ impl< return None; } - let membership_reader = membership.read().await; - let stake_table_entry = CERT::stake_table_entry(&*membership_reader, &key, epoch)?; - let stake_table = CERT::stake_table(&*membership_reader, epoch); - let total_nodes = CERT::total_nodes(&*membership_reader, epoch); - let threshold = CERT::threshold(&*membership_reader, epoch); - drop(membership_reader); + let stake_table_entry = CERT::stake_table_entry(&membership, &key).await?; + let stake_table = CERT::stake_table(&membership).await; + let total_nodes = CERT::total_nodes(&membership).await; + let threshold = CERT::threshold(&membership).await; let vote_node_id = stake_table .iter() From 55b8283eb703014cde405d50a3c263bb9ff20434 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Tue, 4 Feb 2025 17:01:41 -0500 Subject: [PATCH 5/9] middle of refacto --- crates/examples/infra/mod.rs | 2 +- crates/hotshot/src/lib.rs | 28 ++++++++++--------- crates/hotshot/src/tasks/mod.rs | 8 +++--- crates/hotshot/src/tasks/task_state.rs | 20 ++++++------- crates/hotshot/src/types/handle.rs | 12 ++++---- crates/task-impls/src/helpers.rs | 16 +++++------ crates/task-impls/src/quorum_vote/handlers.rs | 6 ++-- crates/task-impls/src/quorum_vote/mod.rs | 16 +++-------- crates/testing/src/overall_safety_task.rs | 2 +- crates/testing/src/spinning_task.rs | 2 +- crates/testing/tests/tests_1/da_task.rs | 18 +++++++++--- crates/testing/tests/tests_1/message.rs | 2 +- .../tests_1/quorum_proposal_recv_task.rs | 4 +-- .../tests/tests_1/quorum_proposal_task.rs | 12 ++++---- .../testing/tests/tests_1/quorum_vote_task.rs | 6 ++-- .../testing/tests/tests_1/transaction_task.rs | 2 +- .../tests_1/upgrade_task_with_proposal.rs | 2 +- .../tests/tests_1/upgrade_task_with_vote.rs | 2 +- crates/testing/tests/tests_1/vid_task.rs | 2 +- .../tests/tests_1/vote_dependency_handle.rs | 4 +-- crates/types/src/epoch_membership.rs | 25 +++++++++++++++-- crates/types/src/message.rs | 11 ++++++-- 22 files changed, 114 insertions(+), 88 deletions(-) diff --git a/crates/examples/infra/mod.rs b/crates/examples/infra/mod.rs index ecd48fb406..7b02206df1 100755 --- a/crates/examples/infra/mod.rs +++ b/crates/examples/infra/mod.rs @@ -524,7 +524,7 @@ pub trait RunDa< } let num_eligible_leaders = context .hotshot - .memberships + .membership_coordinator .read() .await .committee_leaders( diff --git a/crates/hotshot/src/lib.rs b/crates/hotshot/src/lib.rs index 27030c672d..7e89fe19fc 100644 --- a/crates/hotshot/src/lib.rs +++ b/crates/hotshot/src/lib.rs @@ -14,6 +14,7 @@ pub mod documentation; use committable::Committable; use futures::future::{select, Either}; use hotshot_types::{ + epoch_membership::EpochMembershipCoordinator, message::UpgradeLock, traits::{network::BroadcastDelay, node_implementation::Versions}, }; @@ -108,7 +109,7 @@ pub struct SystemContext, V: Versi pub network: Arc, /// Memberships used by consensus - pub memberships: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// the metrics that the implementor is using. metrics: Arc, @@ -163,7 +164,7 @@ impl, V: Versions> Clone private_key: self.private_key.clone(), config: self.config.clone(), network: Arc::clone(&self.network), - memberships: Arc::clone(&self.memberships), + membership_coordinator: self.membership_coordinator.clone(), metrics: Arc::clone(&self.metrics), consensus: self.consensus.clone(), instance_state: Arc::clone(&self.instance_state), @@ -199,7 +200,7 @@ impl, V: Versions> SystemContext::PrivateKey, nonce: u64, config: HotShotConfig, - memberships: Arc>, + memberships: EpochMembershipCoordinator, network: Arc, initializer: HotShotInitializer, metrics: ConsensusMetricsValue, @@ -253,7 +254,7 @@ impl, V: Versions> SystemContext::PrivateKey, nonce: u64, config: HotShotConfig, - memberships: Arc>, + membership_coordinator: EpochMembershipCoordinator, network: Arc, initializer: HotShotInitializer, metrics: ConsensusMetricsValue, @@ -358,7 +359,7 @@ impl, V: Versions> SystemContext, V: Versions> SystemContext, V: Versions> SystemContext::PrivateKey, node_id: u64, config: HotShotConfig, - memberships: Arc>, + memberships: EpochMembershipCoordinator, network: Arc, initializer: HotShotInitializer, metrics: ConsensusMetricsValue, @@ -671,7 +673,7 @@ impl, V: Versions> SystemContext::PrivateKey, nonce: u64, config: HotShotConfig, - memberships: Arc>, + memberships: EpochMembershipCoordinator, network: Arc, initializer: HotShotInitializer, metrics: ConsensusMetricsValue, @@ -853,7 +855,7 @@ where hotshot: Arc::clone(&left_system_context), storage: Arc::clone(&left_system_context.storage), network: Arc::clone(&left_system_context.network), - memberships: Arc::clone(&left_system_context.memberships), + membership_coordinator: Arc::clone(&left_system_context.membership_coordinator), epoch_height, }; @@ -865,7 +867,7 @@ where hotshot: Arc::clone(&right_system_context), storage: Arc::clone(&right_system_context.storage), network: Arc::clone(&right_system_context.network), - memberships: Arc::clone(&right_system_context.memberships), + membership_coordinator: Arc::clone(&right_system_context.membership_coordinator), epoch_height, }; diff --git a/crates/hotshot/src/tasks/mod.rs b/crates/hotshot/src/tasks/mod.rs index 1d50c16bd7..c0734a5f15 100644 --- a/crates/hotshot/src/tasks/mod.rs +++ b/crates/hotshot/src/tasks/mod.rs @@ -82,7 +82,7 @@ pub fn add_response_task, V: Versi ) { let state = NetworkResponseState::::new( handle.hotshot.consensus(), - Arc::clone(&handle.memberships), + Arc::clone(&handle.membership_coordinator), handle.public_key().clone(), handle.private_key().clone(), handle.hotshot.id, @@ -363,7 +363,7 @@ where hotshot: Arc::clone(&hotshot), storage: Arc::clone(&hotshot.storage), network: Arc::clone(&hotshot.network), - memberships: Arc::clone(&hotshot.memberships), + membership_coordinator: Arc::clone(&hotshot.membership_coordinator), epoch_height, }; @@ -523,7 +523,7 @@ where /// Adds the `NetworkEventTaskState` tasks possibly modifying them as well. fn add_network_event_tasks(&self, handle: &mut SystemContextHandle) { let network = Arc::clone(&handle.network); - let memberships = Arc::clone(&handle.memberships); + let memberships = Arc::clone(&handle.membership_coordinator); self.add_network_event_task(handle, network, memberships); } @@ -571,6 +571,6 @@ pub fn add_network_event_tasks, V: add_network_event_task( handle, Arc::clone(&handle.network), - Arc::clone(&handle.memberships), + Arc::clone(&handle.membership_coordinator), ); } diff --git a/crates/hotshot/src/tasks/task_state.rs b/crates/hotshot/src/tasks/task_state.rs index ef8e811a5f..783f130527 100644 --- a/crates/hotshot/src/tasks/task_state.rs +++ b/crates/hotshot/src/tasks/task_state.rs @@ -51,7 +51,7 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(handle.hotshot.consensus()), view: handle.cur_view().await, delay: handle.hotshot.config.data_request_delay, - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), id: handle.hotshot.id, @@ -72,7 +72,7 @@ impl, V: Versions> CreateTaskState output_event_stream: handle.hotshot.external_event_stream.0.clone(), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), vote_collectors: BTreeMap::default(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), @@ -122,7 +122,7 @@ impl, V: Versions> CreateTaskState cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, network: Arc::clone(&handle.hotshot.network), - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), id: handle.hotshot.id, @@ -140,7 +140,7 @@ impl, V: Versions> CreateTaskState Self { consensus: OuterConsensus::new(handle.hotshot.consensus()), output_event_stream: handle.hotshot.external_event_stream.0.clone(), - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), network: Arc::clone(&handle.hotshot.network), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, @@ -165,7 +165,7 @@ impl, V: Versions> CreateTaskState cur_view, next_view: cur_view, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), num_timeouts_tracked: 0, @@ -192,7 +192,7 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(handle.hotshot.consensus()), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), instance_state: handle.hotshot.instance_state(), @@ -237,7 +237,7 @@ impl, V: Versions> CreateTaskState latest_voted_view: handle.cur_view().await, vote_dependencies: BTreeMap::new(), network: Arc::clone(&handle.hotshot.network), - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), drb_computation: None, output_event_stream: handle.hotshot.external_event_stream.0.clone(), id: handle.hotshot.id, @@ -262,7 +262,7 @@ impl, V: Versions> CreateTaskState proposal_dependencies: BTreeMap::new(), consensus: OuterConsensus::new(consensus), instance_state: handle.hotshot.instance_state(), - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), storage: Arc::clone(&handle.storage), @@ -289,7 +289,7 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(consensus), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), timeout: handle.hotshot.config.next_view_timeout, output_event_stream: handle.hotshot.external_event_stream.0.clone(), storage: Arc::clone(&handle.storage), @@ -313,7 +313,7 @@ impl, V: Versions> CreateTaskState private_key: handle.private_key().clone(), instance_state: handle.hotshot.instance_state(), network: Arc::clone(&handle.hotshot.network), - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), vote_collectors: BTreeMap::default(), next_epoch_vote_collectors: BTreeMap::default(), timeout_vote_collectors: BTreeMap::default(), diff --git a/crates/hotshot/src/types/handle.rs b/crates/hotshot/src/types/handle.rs index f8c82dfad6..40467d55a8 100644 --- a/crates/hotshot/src/types/handle.rs +++ b/crates/hotshot/src/types/handle.rs @@ -21,6 +21,7 @@ use hotshot_task_impls::{events::HotShotEvent, helpers::broadcast_event}; use hotshot_types::{ consensus::Consensus, data::{Leaf2, QuorumProposalWrapper}, + epoch_membership::EpochMembershipCoordinator, error::HotShotError, message::{Message, MessageKind, Proposal, RecipientList}, request_response::ProposalRequestPayload, @@ -68,7 +69,7 @@ pub struct SystemContextHandle, V: pub network: Arc, /// Memberships used by consensus - pub memberships: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// Number of blocks in an epoch, zero means there are no epochs pub epoch_height: u64, @@ -155,7 +156,7 @@ impl + 'static, V: Versions> signed_proposal_request.commit().as_ref(), )?; - let mem = Arc::clone(&self.memberships); + let mem = self.membership_coordinator.clone(); let receiver = self.internal_event_stream.1.activate_cloned(); let sender = self.internal_event_stream.0.clone(); let epoch_height = self.epoch_height; @@ -186,13 +187,10 @@ impl + 'static, V: Versions> if let HotShotEvent::QuorumProposalResponseRecv(quorum_proposal) = hs_event.as_ref() { // Make sure that the quorum_proposal is valid - let mem_reader = mem.read().await; - if let Err(err) = quorum_proposal.validate_signature(&mem_reader, epoch_height) - { + if let Err(err) = quorum_proposal.validate_signature(&mem) { tracing::warn!("Invalid Proposal Received after Request. Err {:?}", err); continue; } - drop(mem_reader); let proposed_leaf = Leaf2::from_quorum_proposal(&quorum_proposal.data); let commit = proposed_leaf.commit(); if commit == leaf_commitment { @@ -327,7 +325,7 @@ impl + 'static, V: Versions> epoch_number: Option, ) -> Result { self.hotshot - .memberships + .membership_coordinator .read() .await .leader(view_number, epoch_number) diff --git a/crates/task-impls/src/helpers.rs b/crates/task-impls/src/helpers.rs index 6b6f35cd55..10c86fbb64 100644 --- a/crates/task-impls/src/helpers.rs +++ b/crates/task-impls/src/helpers.rs @@ -16,6 +16,7 @@ use hotshot_task::dependency::{Dependency, EventDependency}; use hotshot_types::{ consensus::OuterConsensus, data::{Leaf2, QuorumProposalWrapper, ViewChangeEvidence2}, + epoch_membership::EpochMembership, event::{Event, EventType, LeafInfo}, message::{Proposal, UpgradeLock}, request_response::ProposalRequestPayload, @@ -47,7 +48,7 @@ pub(crate) async fn fetch_proposal( view_number: TYPES::View, event_sender: Sender>>, event_receiver: Receiver>>, - membership: Arc>, + membership: EpochMembership, consensus: OuterConsensus, sender_public_key: TYPES::SignatureKey, sender_private_key: ::PrivateKey, @@ -76,7 +77,7 @@ pub(crate) async fn fetch_proposal( ) .await; - let mem = Arc::clone(&membership); + let mem = membership.clone(); // Make a background task to await the arrival of the event data. let Ok(Some(proposal)) = // We want to explicitly timeout here so we aren't waiting around for the data. @@ -108,8 +109,7 @@ pub(crate) async fn fetch_proposal( hs_event.as_ref() { // Make sure that the quorum_proposal is valid - let mem_reader = mem.read().await; - if quorum_proposal.validate_signature(&mem_reader, epoch_height).is_ok() { + if quorum_proposal.validate_signature(&mem, epoch_height).is_ok() { proposal = Some(quorum_proposal.clone()); } @@ -131,10 +131,8 @@ pub(crate) async fn fetch_proposal( let justify_qc_epoch = justify_qc.data.epoch(); - let membership_reader = membership.read().await; - let membership_stake_table = membership_reader.stake_table(justify_qc_epoch); - let membership_success_threshold = membership_reader.success_threshold(justify_qc_epoch); - drop(membership_reader); + let membership_stake_table = membership.stake_table().await; + let membership_success_threshold = membership.success_threshold().await; justify_qc .is_valid_cert( @@ -508,7 +506,7 @@ pub async fn decide_from_proposal( pub(crate) async fn parent_leaf_and_state( event_sender: &Sender>>, event_receiver: &Receiver>>, - membership: Arc>, + membership: EpochMembership, public_key: TYPES::SignatureKey, private_key: ::PrivateKey, consensus: OuterConsensus, diff --git a/crates/task-impls/src/quorum_vote/handlers.rs b/crates/task-impls/src/quorum_vote/handlers.rs index 38b63fe4ea..57df212ce8 100644 --- a/crates/task-impls/src/quorum_vote/handlers.rs +++ b/crates/task-impls/src/quorum_vote/handlers.rs @@ -19,7 +19,6 @@ use async_broadcast::{InactiveReceiver, Sender}; use async_lock::RwLock; use chrono::Utc; use committable::Committable; -use hotshot_types::simple_vote::HasEpoch; use hotshot_types::{ consensus::OuterConsensus, data::{Leaf2, QuorumProposalWrapper, VidDisperseShare}, @@ -41,6 +40,7 @@ use hotshot_types::{ }, vote::HasViewNumber, }; +use hotshot_types::{epoch_membership::EpochMembership, simple_vote::HasEpoch}; use tokio::spawn; use tracing::instrument; use utils::anytrace::*; @@ -455,7 +455,7 @@ pub(crate) async fn update_shared_state< consensus: OuterConsensus, sender: Sender>>, receiver: InactiveReceiver>>, - membership: Arc>, + membership: EpochMembership, public_key: TYPES::SignatureKey, private_key: ::PrivateKey, upgrade_lock: UpgradeLock, @@ -494,7 +494,7 @@ pub(crate) async fn update_shared_state< justify_qc.view_number(), sender.clone(), receiver.activate_cloned(), - Arc::clone(&membership), + membership.clone(), OuterConsensus::new(Arc::clone(&consensus.inner_consensus)), public_key.clone(), private_key.clone(), diff --git a/crates/task-impls/src/quorum_vote/mod.rs b/crates/task-impls/src/quorum_vote/mod.rs index 81ee73c0d3..4e0d60b2cc 100644 --- a/crates/task-impls/src/quorum_vote/mod.rs +++ b/crates/task-impls/src/quorum_vote/mod.rs @@ -16,21 +16,13 @@ use hotshot_task::{ task::TaskState, }; use hotshot_types::{ - consensus::{ConsensusMetricsValue, OuterConsensus}, - data::{Leaf2, QuorumProposalWrapper}, - drb::DrbComputation, - event::Event, - message::{Proposal, UpgradeLock}, - simple_vote::HasEpoch, - traits::{ + consensus::{ConsensusMetricsValue, OuterConsensus}, data::{Leaf2, QuorumProposalWrapper}, drb::DrbComputation, epoch_membership::EpochMembershipCoordinator, event::Event, message::{Proposal, UpgradeLock}, simple_vote::HasEpoch, traits::{ block_contents::BlockHeader, election::Membership, node_implementation::{ConsensusTime, NodeImplementation, NodeType, Versions}, signature_key::SignatureKey, storage::Storage, - }, - utils::{epoch_from_block_number, option_epoch_from_block_number}, - vote::{Certificate, HasViewNumber}, + }, utils::{epoch_from_block_number, option_epoch_from_block_number}, vote::{Certificate, HasViewNumber} }; use tokio::task::JoinHandle; use tracing::instrument; @@ -72,7 +64,7 @@ pub struct VoteDependencyHandle, V pub instance_state: Arc, /// Membership for Quorum certs/votes. - pub membership: Arc>, + pub membership: EpochMembershipCoordinator, /// Reference to the storage. pub storage: Arc>, @@ -214,7 +206,7 @@ impl + 'static, V: Versions> Handl OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)), self.sender.clone(), self.receiver.clone(), - Arc::clone(&self.membership), + self.membership.clone(), self.public_key.clone(), self.private_key.clone(), self.upgrade_lock.clone(), diff --git a/crates/testing/src/overall_safety_task.rs b/crates/testing/src/overall_safety_task.rs index c824cc19b6..a872d381cf 100644 --- a/crates/testing/src/overall_safety_task.rs +++ b/crates/testing/src/overall_safety_task.rs @@ -149,7 +149,7 @@ impl, V: Versions> TestTas .first() .unwrap() .handle - .memberships, + .membership_coordinator, ); let public_key = self.handles.read().await[id].handle.public_key(); let OverallSafetyPropertiesDescription:: { diff --git a/crates/testing/src/spinning_task.rs b/crates/testing/src/spinning_task.rs index 17f783a008..d22bfe5dbf 100644 --- a/crates/testing/src/spinning_task.rs +++ b/crates/testing/src/spinning_task.rs @@ -235,7 +235,7 @@ where }; let storage = node.handle.storage().clone(); - let memberships = Arc::clone(&node.handle.memberships); + let memberships = Arc::clone(&node.handle.membership_coordinator); let config = node.handle.hotshot.config.clone(); let marketplace_config = node.handle.hotshot.marketplace_config.clone(); diff --git a/crates/testing/tests/tests_1/da_task.rs b/crates/testing/tests/tests_1/da_task.rs index 5006cab51d..b397cae482 100644 --- a/crates/testing/tests/tests_1/da_task.rs +++ b/crates/testing/tests/tests_1/da_task.rs @@ -39,7 +39,7 @@ async fn test_da_task() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let default_version = Version { major: 0, minor: 0 }; // Make some empty encoded transactions, we just care about having a commitment handy for the @@ -48,7 +48,12 @@ async fn test_da_task() { let encoded_transactions: Arc<[u8]> = Arc::from(TestTransaction::encode(&transactions)); let payload_commit = hotshot_types::traits::block_contents::vid_commitment::( &encoded_transactions, - handle.hotshot.memberships.read().await.total_nodes(None), + handle + .hotshot + .membership_coordinator + .read() + .await + .total_nodes(None), default_version, ); @@ -148,7 +153,7 @@ async fn test_da_task_storage_failure() { // Set the error flag here for the system handle. This causes it to emit an error on append. handle.storage().write().await.should_return_err = true; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let default_version = Version { major: 0, minor: 0 }; // Make some empty encoded transactions, we just care about having a commitment handy for the @@ -157,7 +162,12 @@ async fn test_da_task_storage_failure() { let encoded_transactions: Arc<[u8]> = Arc::from(TestTransaction::encode(&transactions)); let payload_commit = hotshot_types::traits::block_contents::vid_commitment::( &encoded_transactions, - handle.hotshot.memberships.read().await.total_nodes(None), + handle + .hotshot + .membership_coordinator + .read() + .await + .total_nodes(None), default_version, ); diff --git a/crates/testing/tests/tests_1/message.rs b/crates/testing/tests/tests_1/message.rs index c2970befd5..46957d2324 100644 --- a/crates/testing/tests/tests_1/message.rs +++ b/crates/testing/tests/tests_1/message.rs @@ -79,7 +79,7 @@ async fn test_certificate2_validity() { let handle = build_system_handle::(node_id) .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); diff --git a/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs b/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs index 29d2389f2d..b7fec2c51b 100644 --- a/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs +++ b/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs @@ -54,7 +54,7 @@ async fn test_quorum_proposal_recv_task() { let handle = build_system_handle::(2) .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let consensus = handle.hotshot.consensus(); let mut consensus_writer = consensus.write().await; @@ -129,7 +129,7 @@ async fn test_quorum_proposal_recv_task_liveness_check() { let handle = build_system_handle::(4) .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let consensus = handle.hotshot.consensus(); let mut consensus_writer = consensus.write().await; diff --git a/crates/testing/tests/tests_1/quorum_proposal_task.rs b/crates/testing/tests/tests_1/quorum_proposal_task.rs index 9545f79cfc..75b4e8ad58 100644 --- a/crates/testing/tests/tests_1/quorum_proposal_task.rs +++ b/crates/testing/tests/tests_1/quorum_proposal_task.rs @@ -51,7 +51,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_1() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let version = handle .hotshot .upgrade_lock @@ -155,7 +155,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(membership.clone()); @@ -338,7 +338,7 @@ async fn test_quorum_proposal_task_qc_timeout() { let handle = build_system_handle::(node_id) .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let version = handle .hotshot .upgrade_lock @@ -437,7 +437,7 @@ async fn test_quorum_proposal_task_view_sync() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let version = handle .hotshot .upgrade_lock @@ -536,7 +536,7 @@ async fn test_quorum_proposal_task_liveness_check() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); @@ -714,7 +714,7 @@ async fn test_quorum_proposal_task_with_incomplete_events() { let handle = build_system_handle::(2) .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(membership); diff --git a/crates/testing/tests/tests_1/quorum_vote_task.rs b/crates/testing/tests/tests_1/quorum_vote_task.rs index 4ea37ebe00..4d2ed7e6c0 100644 --- a/crates/testing/tests/tests_1/quorum_vote_task.rs +++ b/crates/testing/tests/tests_1/quorum_vote_task.rs @@ -45,7 +45,7 @@ async fn test_quorum_vote_task_success() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(membership); @@ -112,7 +112,7 @@ async fn test_quorum_vote_task_miss_dependency() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(membership); @@ -196,7 +196,7 @@ async fn test_quorum_vote_task_incorrect_dependency() { .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(membership); diff --git a/crates/testing/tests/tests_1/transaction_task.rs b/crates/testing/tests/tests_1/transaction_task.rs index a5e2f48278..d83bc1c79d 100644 --- a/crates/testing/tests/tests_1/transaction_task.rs +++ b/crates/testing/tests/tests_1/transaction_task.rs @@ -54,7 +54,7 @@ async fn test_transaction_task_leader_two_views_in_a_row() { null_block::builder_fee::( handle .hotshot - .memberships + .membership_coordinator .read() .await .total_nodes(Some(EpochNumber::new(0))), diff --git a/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs b/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs index 820fb3a562..c213068482 100644 --- a/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs +++ b/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs @@ -83,7 +83,7 @@ async fn test_upgrade_task_with_proposal() { let consensus = handle.hotshot.consensus(); let mut consensus_writer = consensus.write().await; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); diff --git a/crates/testing/tests/tests_1/upgrade_task_with_vote.rs b/crates/testing/tests/tests_1/upgrade_task_with_vote.rs index 841fd86a86..42cb73c3be 100644 --- a/crates/testing/tests/tests_1/upgrade_task_with_vote.rs +++ b/crates/testing/tests/tests_1/upgrade_task_with_vote.rs @@ -70,7 +70,7 @@ async fn test_upgrade_task_with_vote() { let consensus = handle.hotshot.consensus().clone(); let mut consensus_writer = consensus.write().await; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(membership); for view in (&mut generator).take(2).collect::>().await { diff --git a/crates/testing/tests/tests_1/vid_task.rs b/crates/testing/tests/tests_1/vid_task.rs index 0cb13a0f45..9e367d00f9 100644 --- a/crates/testing/tests/tests_1/vid_task.rs +++ b/crates/testing/tests/tests_1/vid_task.rs @@ -45,7 +45,7 @@ async fn test_vid_task() { .0; let pub_key = handle.public_key(); - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let default_version = Version { major: 0, minor: 0 }; diff --git a/crates/testing/tests/tests_1/vote_dependency_handle.rs b/crates/testing/tests/tests_1/vote_dependency_handle.rs index a6e36c800f..9b206bfdb2 100644 --- a/crates/testing/tests/tests_1/vote_dependency_handle.rs +++ b/crates/testing/tests/tests_1/vote_dependency_handle.rs @@ -36,7 +36,7 @@ async fn test_vote_dependency_handle() { let handle = build_system_handle::(node_id) .await .0; - let membership = Arc::clone(&handle.hotshot.memberships); + let membership = Arc::clone(&handle.hotshot.membership_coordinator); let mut generator = TestViewGenerator::::generate(membership); @@ -90,7 +90,7 @@ async fn test_vote_dependency_handle() { consensus: OuterConsensus::new(consensus.clone()), consensus_metrics: Arc::clone(&consensus.read().await.metrics), instance_state: handle.hotshot.instance_state(), - membership: Arc::clone(&handle.hotshot.memberships), + membership: Arc::clone(&handle.hotshot.membership_coordinator), storage: Arc::clone(&handle.storage()), view_number, sender: event_sender.clone(), diff --git a/crates/types/src/epoch_membership.rs b/crates/types/src/epoch_membership.rs index d2fd99a8a0..70e3048603 100644 --- a/crates/types/src/epoch_membership.rs +++ b/crates/types/src/epoch_membership.rs @@ -17,7 +17,6 @@ use crate::utils::root_block_in_epoch; struct CatchupSignal {} /// Struct to Coordinate membership catchup -#[derive(Clone)] pub struct EpochMembershipCoordinator { /// The underlying membhersip membership: Arc>, @@ -29,9 +28,18 @@ pub struct EpochMembershipCoordinator { catchup_map: Arc>>>, /// Number of blocks in an epoch - epoch_height: u64, + pub epoch_height: u64, } +impl Clone for EpochMembershipCoordinator { + fn clone(&self) -> Self { + Self { + membership: Arc::clone(&self.membership), + catchup_map: Arc::clone(&self.catchup_map), + epoch_height: self.epoch_height, + } + } +} // async fn catchup_membership(coordinator: EpochMembershipCoordinator) { // } @@ -137,7 +145,20 @@ pub struct EpochMembership { pub membership: Arc>, } +impl Clone for EpochMembership { + fn clone(&self) -> Self { + Self { + membership: Arc::clone(&self.membership), + epoch: self.epoch, + } + } +} + impl EpochMembership { + /// Get the epoch this membership is good for + pub fn epoch(&self) -> Option { + self.epoch + } /// Wraps the same named Membership trait fn async fn get_epoch_root( &self, diff --git a/crates/types/src/message.rs b/crates/types/src/message.rs index 92998ca974..0cbb8bcdcb 100644 --- a/crates/types/src/message.rs +++ b/crates/types/src/message.rs @@ -30,6 +30,7 @@ use crate::{ DaProposal, DaProposal2, Leaf, Leaf2, QuorumProposal, QuorumProposal2, QuorumProposalWrapper, UpgradeProposal, }, + epoch_membership::EpochMembership, request_response::ProposalRequestPayload, simple_certificate::{ DaCertificate, DaCertificate2, QuorumCertificate2, UpgradeCertificate, @@ -579,9 +580,9 @@ where /// Checks that the signature of the quorum proposal is valid. /// # Errors /// Returns an error when the proposal signature is invalid. - pub fn validate_signature( + pub async fn validate_signature( &self, - membership: &TYPES::Membership, + membership: &EpochMembership, epoch_height: u64, ) -> Result<()> { let view_number = self.data.proposal.view_number(); @@ -590,7 +591,11 @@ where self.data.block_header().block_number(), epoch_height, ); - let view_leader_key = membership.leader(view_number, proposal_epoch)?; + ensure!( + proposal_epoch == membership.epoch(), + "proposal epoch does not match membership epoch" + ); + let view_leader_key = membership.leader(view_number).await?; let proposed_leaf = Leaf2::from_quorum_proposal(&self.data); ensure!( From e35c8ccc4c03426ca04f8d533561048d240ce506 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Wed, 5 Feb 2025 15:53:12 -0500 Subject: [PATCH 6/9] more refactor --- crates/hotshot/src/lib.rs | 6 +- crates/hotshot/src/tasks/mod.rs | 8 +- crates/hotshot/src/tasks/task_state.rs | 20 +-- .../src/traits/networking/libp2p_network.rs | 2 +- crates/hotshot/src/types/handle.rs | 14 +- crates/task-impls/src/consensus/handlers.rs | 1 - crates/task-impls/src/consensus/mod.rs | 2 +- crates/task-impls/src/da.rs | 2 +- crates/task-impls/src/helpers.rs | 49 ++++--- crates/task-impls/src/network.rs | 2 +- .../src/quorum_proposal/handlers.rs | 27 ++-- crates/task-impls/src/quorum_proposal/mod.rs | 39 +++--- .../src/quorum_proposal_recv/handlers.rs | 25 ++-- .../src/quorum_proposal_recv/mod.rs | 11 +- crates/task-impls/src/quorum_vote/handlers.rs | 41 +++--- crates/task-impls/src/quorum_vote/mod.rs | 56 +++++--- crates/task-impls/src/request.rs | 2 +- crates/task-impls/src/response.rs | 7 +- crates/task-impls/src/transactions.rs | 2 +- crates/task-impls/src/upgrade.rs | 2 +- crates/task-impls/src/vid.rs | 2 +- crates/task-impls/src/view_sync.rs | 4 +- crates/task-impls/src/vote_collection.rs | 4 +- .../src/byzantine/byzantine_behaviour.rs | 2 +- crates/testing/src/helpers.rs | 9 +- crates/testing/src/view_generator.rs | 17 +-- .../tests/tests_1/vote_dependency_handle.rs | 2 +- crates/types/src/consensus.rs | 5 +- crates/types/src/data.rs | 18 +-- crates/types/src/data/vid_disperse.rs | 22 ++- crates/types/src/epoch_membership.rs | 126 +++++++++++++++--- crates/types/src/message.rs | 18 +-- crates/types/src/simple_certificate.rs | 10 +- crates/types/src/traits/network.rs | 4 +- 34 files changed, 328 insertions(+), 233 deletions(-) diff --git a/crates/hotshot/src/lib.rs b/crates/hotshot/src/lib.rs index 7e89fe19fc..c33ff46621 100644 --- a/crates/hotshot/src/lib.rs +++ b/crates/hotshot/src/lib.rs @@ -794,7 +794,7 @@ where private_key.clone(), nonce, config.clone(), - Arc::clone(&memberships), + memberships.clone(), Arc::clone(&network), initializer.clone(), metrics.clone(), @@ -855,7 +855,7 @@ where hotshot: Arc::clone(&left_system_context), storage: Arc::clone(&left_system_context.storage), network: Arc::clone(&left_system_context.network), - membership_coordinator: Arc::clone(&left_system_context.membership_coordinator), + membership_coordinator: left_system_context.membership_coordinator.clone(), epoch_height, }; @@ -867,7 +867,7 @@ where hotshot: Arc::clone(&right_system_context), storage: Arc::clone(&right_system_context.storage), network: Arc::clone(&right_system_context.network), - membership_coordinator: Arc::clone(&right_system_context.membership_coordinator), + membership_coordinator: right_system_context.membership_coordinator.clone(), epoch_height, }; diff --git a/crates/hotshot/src/tasks/mod.rs b/crates/hotshot/src/tasks/mod.rs index c0734a5f15..0fe9745cdb 100644 --- a/crates/hotshot/src/tasks/mod.rs +++ b/crates/hotshot/src/tasks/mod.rs @@ -82,7 +82,7 @@ pub fn add_response_task, V: Versi ) { let state = NetworkResponseState::::new( handle.hotshot.consensus(), - Arc::clone(&handle.membership_coordinator), + handle.membership_coordinator.clone(), handle.public_key().clone(), handle.private_key().clone(), handle.hotshot.id, @@ -196,13 +196,13 @@ pub fn add_network_event_task< >( handle: &mut SystemContextHandle, network: Arc, - membership: Arc>, + membership_coordinator: EpochMembershipCoordinator, ) { let network_state: NetworkEventTaskState<_, V, _, _> = NetworkEventTaskState { network, view: TYPES::View::genesis(), epoch: genesis_epoch_from_version::(), - membership, + membership_coordinator, storage: Arc::clone(&handle.storage()), consensus: OuterConsensus::new(handle.consensus()), upgrade_lock: handle.hotshot.upgrade_lock.clone(), @@ -533,7 +533,7 @@ where &self, handle: &mut SystemContextHandle, channel: Arc<>::Network>, - membership: Arc>, + membership_coordinator: EpochMembershipCoordinator, ) { add_network_event_task(handle, channel, membership); } diff --git a/crates/hotshot/src/tasks/task_state.rs b/crates/hotshot/src/tasks/task_state.rs index 783f130527..42da960eb9 100644 --- a/crates/hotshot/src/tasks/task_state.rs +++ b/crates/hotshot/src/tasks/task_state.rs @@ -51,7 +51,7 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(handle.hotshot.consensus()), view: handle.cur_view().await, delay: handle.hotshot.config.data_request_delay, - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), id: handle.hotshot.id, @@ -72,7 +72,7 @@ impl, V: Versions> CreateTaskState output_event_stream: handle.hotshot.external_event_stream.0.clone(), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), vote_collectors: BTreeMap::default(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), @@ -122,7 +122,7 @@ impl, V: Versions> CreateTaskState cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, network: Arc::clone(&handle.hotshot.network), - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), id: handle.hotshot.id, @@ -140,7 +140,7 @@ impl, V: Versions> CreateTaskState Self { consensus: OuterConsensus::new(handle.hotshot.consensus()), output_event_stream: handle.hotshot.external_event_stream.0.clone(), - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), network: Arc::clone(&handle.hotshot.network), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, @@ -165,7 +165,7 @@ impl, V: Versions> CreateTaskState cur_view, next_view: cur_view, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), num_timeouts_tracked: 0, @@ -192,7 +192,7 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(handle.hotshot.consensus()), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), instance_state: handle.hotshot.instance_state(), @@ -237,7 +237,7 @@ impl, V: Versions> CreateTaskState latest_voted_view: handle.cur_view().await, vote_dependencies: BTreeMap::new(), network: Arc::clone(&handle.hotshot.network), - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership: handle.hotshot.membership_coordinator.clone(), drb_computation: None, output_event_stream: handle.hotshot.external_event_stream.0.clone(), id: handle.hotshot.id, @@ -262,7 +262,7 @@ impl, V: Versions> CreateTaskState proposal_dependencies: BTreeMap::new(), consensus: OuterConsensus::new(consensus), instance_state: handle.hotshot.instance_state(), - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), storage: Arc::clone(&handle.storage), @@ -289,7 +289,7 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(consensus), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership: handle.hotshot.membership_coordinator.clone(), timeout: handle.hotshot.config.next_view_timeout, output_event_stream: handle.hotshot.external_event_stream.0.clone(), storage: Arc::clone(&handle.storage), @@ -313,7 +313,7 @@ impl, V: Versions> CreateTaskState private_key: handle.private_key().clone(), instance_state: handle.hotshot.instance_state(), network: Arc::clone(&handle.hotshot.network), - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), vote_collectors: BTreeMap::default(), next_epoch_vote_collectors: BTreeMap::default(), timeout_vote_collectors: BTreeMap::default(), diff --git a/crates/hotshot/src/traits/networking/libp2p_network.rs b/crates/hotshot/src/traits/networking/libp2p_network.rs index d288c3e360..0c6cdfe05e 100644 --- a/crates/hotshot/src/traits/networking/libp2p_network.rs +++ b/crates/hotshot/src/traits/networking/libp2p_network.rs @@ -992,7 +992,7 @@ impl ConnectedNetwork for Libp2pNetwork { &'a self, view: u64, epoch: Option, - membership: Arc>, + membership_coordinator: EpochMembershipCoordinator, ) where TYPES: NodeType + 'a, { diff --git a/crates/hotshot/src/types/handle.rs b/crates/hotshot/src/types/handle.rs index 40467d55a8..c0c0a563a5 100644 --- a/crates/hotshot/src/types/handle.rs +++ b/crates/hotshot/src/types/handle.rs @@ -32,6 +32,7 @@ use hotshot_types::{ node_implementation::NodeType, signature_key::SignatureKey, }, + utils::{epoch_from_block_number, option_epoch_from_block_number}, }; use tracing::instrument; @@ -186,8 +187,14 @@ impl + 'static, V: Versions> // Then, if it's `Some`, make sure that the data is correct if let HotShotEvent::QuorumProposalResponseRecv(quorum_proposal) = hs_event.as_ref() { + let maybe_epoch = option_epoch_from_block_number( + quorum_proposal.data.epoch.is_some(), + quorum_proposal.data.block_header().height, + epoch_height, + ); + let membership = mem.membership_for_epoch(maybe_epoch).await; // Make sure that the quorum_proposal is valid - if let Err(err) = quorum_proposal.validate_signature(&mem) { + if let Err(err) = quorum_proposal.validate_signature(&membership) { tracing::warn!("Invalid Proposal Received after Request. Err {:?}", err); continue; } @@ -326,9 +333,10 @@ impl + 'static, V: Versions> ) -> Result { self.hotshot .membership_coordinator - .read() + .membership_for_epoch(epoch_number) + .await + .leader(view_number) .await - .leader(view_number, epoch_number) .context("Failed to lookup leader") } diff --git a/crates/task-impls/src/consensus/handlers.rs b/crates/task-impls/src/consensus/handlers.rs index 402c7151bb..cc01abfa29 100644 --- a/crates/task-impls/src/consensus/handlers.rs +++ b/crates/task-impls/src/consensus/handlers.rs @@ -12,7 +12,6 @@ use hotshot_types::{ event::{Event, EventType}, simple_vote::{HasEpoch, QuorumVote2, TimeoutData2, TimeoutVote2}, traits::{ - election::Membership, node_implementation::{ConsensusTime, NodeImplementation, NodeType}, }, utils::EpochTransitionIndicator, diff --git a/crates/task-impls/src/consensus/mod.rs b/crates/task-impls/src/consensus/mod.rs index d117b5fec0..960f8a956f 100644 --- a/crates/task-impls/src/consensus/mod.rs +++ b/crates/task-impls/src/consensus/mod.rs @@ -51,7 +51,7 @@ pub struct ConsensusTaskState, V: pub network: Arc, /// Membership for Quorum Certs/votes - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// A map of `QuorumVote` collector tasks. pub vote_collectors: VoteCollectorsMap, QuorumCertificate2, V>, diff --git a/crates/task-impls/src/da.rs b/crates/task-impls/src/da.rs index 09320e015d..4963aec5bf 100644 --- a/crates/task-impls/src/da.rs +++ b/crates/task-impls/src/da.rs @@ -58,7 +58,7 @@ pub struct DaTaskState, V: Version /// Membership for the DA committee and quorum committee. /// We need the latter only for calculating the proper VID scheme /// from the number of nodes in the quorum. - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// The underlying network pub network: Arc, diff --git a/crates/task-impls/src/helpers.rs b/crates/task-impls/src/helpers.rs index 10c86fbb64..c1eb14bc71 100644 --- a/crates/task-impls/src/helpers.rs +++ b/crates/task-impls/src/helpers.rs @@ -16,7 +16,7 @@ use hotshot_task::dependency::{Dependency, EventDependency}; use hotshot_types::{ consensus::OuterConsensus, data::{Leaf2, QuorumProposalWrapper, ViewChangeEvidence2}, - epoch_membership::EpochMembership, + epoch_membership::EpochMembershipCoordinator, event::{Event, EventType, LeafInfo}, message::{Proposal, UpgradeLock}, request_response::ProposalRequestPayload, @@ -48,7 +48,7 @@ pub(crate) async fn fetch_proposal( view_number: TYPES::View, event_sender: Sender>>, event_receiver: Receiver>>, - membership: EpochMembership, + membership_coordinator: EpochMembershipCoordinator, consensus: OuterConsensus, sender_public_key: TYPES::SignatureKey, sender_private_key: ::PrivateKey, @@ -77,7 +77,7 @@ pub(crate) async fn fetch_proposal( ) .await; - let mem = membership.clone(); + let mem_coordinator = membership_coordinator.clone(); // Make a background task to await the arrival of the event data. let Ok(Some(proposal)) = // We want to explicitly timeout here so we aren't waiting around for the data. @@ -108,8 +108,14 @@ pub(crate) async fn fetch_proposal( if let HotShotEvent::QuorumProposalResponseRecv(quorum_proposal) = hs_event.as_ref() { + let proposal_epoch = option_epoch_from_block_number::( + quorum_proposal.data.proposal.epoch().is_some(), + quorum_proposal.data.block_header().block_number(), + epoch_height, + ); + let mem = mem_coordinator.membership_for_epoch(proposal_epoch).await; // Make sure that the quorum_proposal is valid - if quorum_proposal.validate_signature(&mem, epoch_height).is_ok() { + if quorum_proposal.validate_signature(&mem).await.is_ok() { proposal = Some(quorum_proposal.clone()); } @@ -131,8 +137,11 @@ pub(crate) async fn fetch_proposal( let justify_qc_epoch = justify_qc.data.epoch(); - let membership_stake_table = membership.stake_table().await; - let membership_success_threshold = membership.success_threshold().await; + let mem = membership_coordinator + .membership_for_epoch(justify_qc_epoch) + .await; + let membership_stake_table = mem.stake_table().await; + let membership_success_threshold = mem.success_threshold().await; justify_qc .is_valid_cert( @@ -506,7 +515,7 @@ pub async fn decide_from_proposal( pub(crate) async fn parent_leaf_and_state( event_sender: &Sender>>, event_receiver: &Receiver>>, - membership: EpochMembership, + membership: EpochMembershipCoordinator, public_key: TYPES::SignatureKey, private_key: ::PrivateKey, consensus: OuterConsensus, @@ -738,9 +747,8 @@ 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, validation_info.epoch_height)?; - drop(membership_reader); + let mut membership = validation_info.membership.clone(); + proposal.validate_signature(&membership).await?; // Verify a timeout certificate OR a view sync certificate exists and is valid. if proposal.data.justify_qc().view_number() != view_number - 1 { @@ -758,12 +766,12 @@ pub(crate) async fn validate_proposal_view_and_certs< *view_number ); let timeout_cert_epoch = timeout_cert.data().epoch(); + if timeout_cert_epoch < membership.epoch() { + membership = membership.prev_epoch().await; + } - let membership_reader = validation_info.membership.read().await; - let membership_stake_table = membership_reader.stake_table(timeout_cert_epoch); - let membership_success_threshold = - membership_reader.success_threshold(timeout_cert_epoch); - drop(membership_reader); + let membership_stake_table = membership.stake_table().await; + let membership_success_threshold = membership.success_threshold().await; timeout_cert .is_valid_cert( @@ -788,12 +796,11 @@ pub(crate) async fn validate_proposal_view_and_certs< ); let view_sync_cert_epoch = view_sync_cert.data().epoch(); - - let membership_reader = validation_info.membership.read().await; - let membership_stake_table = membership_reader.stake_table(view_sync_cert_epoch); - let membership_success_threshold = - membership_reader.success_threshold(view_sync_cert_epoch); - drop(membership_reader); + if view_sync_cert_epoch < membership.epoch() { + membership = membership.prev_epoch().await; + } + let membership_stake_table = membership.stake_table().await; + let membership_success_threshold = membership.success_threshold().await; // View sync certs must also be valid. view_sync_cert diff --git a/crates/task-impls/src/network.rs b/crates/task-impls/src/network.rs index 14cfb08116..c251e59789 100644 --- a/crates/task-impls/src/network.rs +++ b/crates/task-impls/src/network.rs @@ -478,7 +478,7 @@ pub struct NetworkEventTaskState< pub epoch: Option, /// network memberships - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// Storage to store actionable events pub storage: Arc>, diff --git a/crates/task-impls/src/quorum_proposal/handlers.rs b/crates/task-impls/src/quorum_proposal/handlers.rs index 4f7ff5719a..2d9f4db1ac 100644 --- a/crates/task-impls/src/quorum_proposal/handlers.rs +++ b/crates/task-impls/src/quorum_proposal/handlers.rs @@ -25,6 +25,7 @@ use hotshot_task::{ use hotshot_types::{ consensus::{CommitmentAndMetadata, OuterConsensus}, data::{Leaf2, QuorumProposal2, QuorumProposalWrapper, VidDisperse, ViewChangeEvidence2}, + epoch_membership::EpochMembership, message::Proposal, simple_certificate::{NextEpochQuorumCertificate2, QuorumCertificate2, UpgradeCertificate}, traits::{ @@ -84,7 +85,7 @@ pub struct ProposalDependencyHandle { pub instance_state: Arc, /// Membership for Quorum Certs/votes - pub membership: Arc>, + pub membership: EpochMembership, /// Our public key pub public_key: TYPES::SignatureKey, @@ -130,11 +131,15 @@ impl ProposalDependencyHandle { ) -> Option> { while let Ok(event) = rx.recv_direct().await { if let HotShotEvent::HighQcRecv(qc, _sender) = event.as_ref() { - let membership_reader = self.membership.read().await; - let membership_stake_table = membership_reader.stake_table(qc.data.epoch); - let membership_success_threshold = - membership_reader.success_threshold(qc.data.epoch); - drop(membership_reader); + let prev_epoch = qc.data.epoch; + let mem = if prev_epoch < self.membership.epoch() { + &self.membership.prev_epoch().await + } else { + &self.membership + }; + let membership_stake_table = mem.stake_table().await; + let membership_success_threshold = mem.success_threshold().await; + drop(mem); if qc .is_valid_cert( @@ -272,7 +277,7 @@ impl ProposalDependencyHandle { let (parent_leaf, state) = parent_leaf_and_state( &self.sender, &self.receiver, - Arc::clone(&self.membership), + self.membership.coordinator.clone(), self.public_key.clone(), self.private_key.clone(), OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)), @@ -377,13 +382,7 @@ impl ProposalDependencyHandle { // Make sure we are the leader for the view and epoch. // We might have ended up here because we were in the epoch transition. - if self - .membership - .read() - .await - .leader(self.view_number, epoch)? - != self.public_key - { + if self.membership.leader(self.view_number).await? != self.public_key { tracing::debug!( "We are not the leader in the epoch for which we are about to propose. Do not send the quorum proposal." ); diff --git a/crates/task-impls/src/quorum_proposal/mod.rs b/crates/task-impls/src/quorum_proposal/mod.rs index 90cdd53d27..2b524607ae 100644 --- a/crates/task-impls/src/quorum_proposal/mod.rs +++ b/crates/task-impls/src/quorum_proposal/mod.rs @@ -17,6 +17,7 @@ use hotshot_task::{ }; use hotshot_types::{ consensus::OuterConsensus, + epoch_membership::EpochMembershipCoordinator, message::UpgradeLock, simple_certificate::{QuorumCertificate2, UpgradeCertificate}, traits::{ @@ -52,7 +53,7 @@ pub struct QuorumProposalTaskState pub instance_state: Arc, /// Membership for Quorum Certs/votes - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// Our public key pub public_key: TYPES::SignatureKey, @@ -280,9 +281,11 @@ impl, V: Versions> event: Arc>, epoch_transition_indicator: EpochTransitionIndicator, ) -> Result<()> { - let membership_reader = self.membership.read().await; - let leader_in_current_epoch = - membership_reader.leader(view_number, epoch_number)? == self.public_key; + let mem = self + .membership_coordinator + .membership_for_epoch(epoch_number) + .await; + let leader_in_current_epoch = mem.leader(view_number).await? == self.public_key; // If we are in the epoch transition and we are the leader in the next epoch, // we might want to start collecting dependencies for our next epoch proposal. @@ -291,9 +294,7 @@ impl, V: Versions> epoch_transition_indicator, EpochTransitionIndicator::InTransition ) - && membership_reader.leader(view_number, epoch_number.map(|x| x + 1))? - == self.public_key; - drop(membership_reader); + && mem.next_epoch().await.leader(view_number).await? == self.public_key; // Don't even bother making the task if we are not entitled to propose anyway. ensure!( @@ -326,7 +327,7 @@ impl, V: Versions> view_number, sender: event_sender, receiver: event_receiver, - membership: Arc::clone(&self.membership), + membership: mem, public_key: self.public_key.clone(), private_key: self.private_key.clone(), instance_state: Arc::clone(&self.instance_state), @@ -470,12 +471,13 @@ impl, V: Versions> } HotShotEvent::ViewSyncFinalizeCertificateRecv(certificate) => { let epoch_number = certificate.data.epoch; + let mem = self + .membership_coordinator + .membership_for_epoch(epoch_number) + .await; - let membership_reader = self.membership.read().await; - let membership_stake_table = membership_reader.stake_table(epoch_number); - let membership_success_threshold = - membership_reader.success_threshold(epoch_number); - drop(membership_reader); + let membership_stake_table = mem.stake_table().await; + let membership_success_threshold = mem.success_threshold().await; certificate .is_valid_cert( @@ -556,11 +558,12 @@ impl, V: Versions> ensure!(qc.view_number() > self.highest_qc.view_number()); let cert_epoch_number = qc.data.epoch; - let membership_reader = self.membership.read().await; - let membership_stake_table = membership_reader.stake_table(cert_epoch_number); - let membership_success_threshold = - membership_reader.success_threshold(cert_epoch_number); - drop(membership_reader); + let mem = self + .membership_coordinator + .membership_for_epoch(cert_epoch_number) + .await; + let membership_stake_table = mem.stake_table().await; + let membership_success_threshold = mem.success_threshold().await; qc.is_valid_cert( membership_stake_table, diff --git a/crates/task-impls/src/quorum_proposal_recv/handlers.rs b/crates/task-impls/src/quorum_proposal_recv/handlers.rs index d497276279..f4423e7a8a 100644 --- a/crates/task-impls/src/quorum_proposal_recv/handlers.rs +++ b/crates/task-impls/src/quorum_proposal_recv/handlers.rs @@ -14,6 +14,7 @@ use committable::Committable; use hotshot_types::{ consensus::OuterConsensus, data::{Leaf2, QuorumProposal, QuorumProposalWrapper}, + epoch_membership::EpochMembershipCoordinator, message::Proposal, simple_certificate::QuorumCertificate, simple_vote::HasEpoch, @@ -101,7 +102,7 @@ fn spawn_fetch_proposal( view: TYPES::View, event_sender: Sender>>, event_receiver: Receiver>>, - membership: Arc>, + membership: EpochMembershipCoordinator, consensus: OuterConsensus, sender_public_key: TYPES::SignatureKey, sender_private_key: ::PrivateKey, @@ -168,10 +169,13 @@ pub(crate) async fn handle_quorum_proposal_recv< validation_info.epoch_height, ); - let membership_reader = validation_info.membership.read().await; - let membership_stake_table = membership_reader.stake_table(justify_qc.data.epoch); - let membership_success_threshold = membership_reader.success_threshold(justify_qc.data.epoch); - drop(membership_reader); + let mut qc_mem = validation_info.membership.clone(); + if qc_mem.epoch > justify_qc.data.epoch { + qc_mem = qc_mem.prev_epoch().await; + } + + let membership_stake_table = qc_mem.stake_table().await; + let membership_success_threshold = qc_mem.success_threshold().await; { let consensus_reader = validation_info.consensus.read().await; @@ -198,12 +202,9 @@ pub(crate) async fn handle_quorum_proposal_recv< bail!("Next epoch justify qc exists but it's not equal with justify qc."); } - let membership_reader = validation_info.membership.read().await; - let membership_next_stake_table = - membership_reader.stake_table(justify_qc.data.epoch.map(|x| x + 1)); - let membership_next_success_threshold = - membership_reader.success_threshold(justify_qc.data.epoch.map(|x| x + 1)); - drop(membership_reader); + let qc_mem = qc_mem.next_epoch().await; + let membership_next_stake_table = qc_mem.stake_table().await; + let membership_next_success_threshold = qc_mem.success_threshold().await; // Validate the next epoch justify qc as well next_epoch_justify_qc @@ -238,7 +239,7 @@ pub(crate) async fn handle_quorum_proposal_recv< justify_qc.view_number(), event_sender.clone(), event_receiver.clone(), - Arc::clone(&validation_info.membership), + validation_info.membership.coordinator.clone(), OuterConsensus::new(Arc::clone(&validation_info.consensus.inner_consensus)), // Note that we explicitly use the node key here instead of the provided key in the signature. // This is because the key that we receive is for the prior leader, so the payload would be routed diff --git a/crates/task-impls/src/quorum_proposal_recv/mod.rs b/crates/task-impls/src/quorum_proposal_recv/mod.rs index 3251fa42a4..581e70e0d7 100644 --- a/crates/task-impls/src/quorum_proposal_recv/mod.rs +++ b/crates/task-impls/src/quorum_proposal_recv/mod.rs @@ -17,9 +17,11 @@ use hotshot_task::task::{Task, TaskState}; use hotshot_types::{ consensus::{Consensus, OuterConsensus}, data::{EpochNumber, Leaf, ViewChangeEvidence2}, + epoch_membership::{EpochMembership, EpochMembershipCoordinator}, event::Event, message::UpgradeLock, simple_certificate::UpgradeCertificate, + simple_vote::HasEpoch, traits::{ node_implementation::{ConsensusTime, NodeImplementation, NodeType, Versions}, signature_key::SignatureKey, @@ -58,7 +60,7 @@ pub struct QuorumProposalRecvTaskState, /// Membership for Quorum Certs/votes - pub membership: Arc>, + pub membership: EpochMembershipCoordinator, /// View timeout from config. pub timeout: u64, @@ -99,7 +101,7 @@ pub(crate) struct ValidationInfo, pub(crate) consensus: OuterConsensus, /// Membership for Quorum Certs/votes - pub membership: Arc>, + pub membership: EpochMembership, /// Output events to application pub output_event_stream: async_broadcast::Sender>, @@ -150,7 +152,10 @@ impl, V: Versions> public_key: self.public_key.clone(), private_key: self.private_key.clone(), consensus: self.consensus.clone(), - membership: Arc::clone(&self.membership), + membership: self + .membership + .membership_for_epoch(proposal.data.epoch()) + .await, output_event_stream: self.output_event_stream.clone(), storage: Arc::clone(&self.storage), upgrade_lock: self.upgrade_lock.clone(), diff --git a/crates/task-impls/src/quorum_vote/handlers.rs b/crates/task-impls/src/quorum_vote/handlers.rs index 57df212ce8..0b57e097e9 100644 --- a/crates/task-impls/src/quorum_vote/handlers.rs +++ b/crates/task-impls/src/quorum_vote/handlers.rs @@ -23,12 +23,12 @@ use hotshot_types::{ consensus::OuterConsensus, data::{Leaf2, QuorumProposalWrapper, VidDisperseShare}, drb::{compute_drb_result, DrbResult}, + epoch_membership::EpochMembershipCoordinator, event::{Event, EventType}, message::{convert_proposal, Proposal, UpgradeLock}, simple_vote::{QuorumData2, QuorumVote2}, traits::{ block_contents::BlockHeader, - election::Membership, node_implementation::{ConsensusTime, NodeImplementation, NodeType}, signature_key::SignatureKey, storage::Storage, @@ -133,13 +133,13 @@ async fn verify_drb_result, V: Ver .next_drb_result() .context(info!("Proposal is missing the DRB result."))?; - let membership_reader = task_state.membership.read().await; - if let Some(epoch_val) = epoch { - let has_stake_current_epoch = - membership_reader.has_stake(&task_state.public_key, Some(epoch_val)); - - drop(membership_reader); + let has_stake_current_epoch = task_state + .membership + .membership_for_epoch(epoch) + .await + .has_stake(&task_state.public_key) + .await; if has_stake_current_epoch { let computed_result = @@ -174,9 +174,10 @@ async fn start_drb_task, V: Versio // Start the new task if we're in the committee for this epoch if task_state .membership - .read() + .membership_for_epoch(Some(current_epoch_number)) + .await + .has_stake(&task_state.public_key) .await - .has_stake(&task_state.public_key, Some(current_epoch_number)) { let new_epoch_number = current_epoch_number + 1; @@ -347,7 +348,7 @@ pub(crate) async fn handle_quorum_proposal_validated< Arc::clone(&task_state.upgrade_lock.decided_upgrade_certificate), &task_state.public_key, version >= V::Epochs::VERSION, - &task_state.membership, + task_state.membership.membership(), ) .await } else { @@ -357,7 +358,7 @@ pub(crate) async fn handle_quorum_proposal_validated< Arc::clone(&task_state.upgrade_lock.decided_upgrade_certificate), &task_state.public_key, version >= V::Epochs::VERSION, - &task_state.membership, + task_state.membership.membership(), ) .await }; @@ -455,7 +456,7 @@ pub(crate) async fn update_shared_state< consensus: OuterConsensus, sender: Sender>>, receiver: InactiveReceiver>>, - membership: EpochMembership, + membership: EpochMembershipCoordinator, public_key: TYPES::SignatureKey, private_key: ::PrivateKey, upgrade_lock: UpgradeLock, @@ -581,7 +582,7 @@ pub(crate) async fn update_shared_state< #[allow(clippy::too_many_arguments)] pub(crate) async fn submit_vote, V: Versions>( sender: Sender>>, - membership: Arc>, + membership: EpochMembership, public_key: TYPES::SignatureKey, private_key: ::PrivateKey, upgrade_lock: UpgradeLock, @@ -592,20 +593,12 @@ pub(crate) async fn submit_vote, V extended_vote: bool, epoch_height: u64, ) -> Result<()> { - let epoch_number = option_epoch_from_block_number::( - leaf.with_epoch, - leaf.block_header().block_number(), - epoch_height, - ); - - let membership_reader = membership.read().await; - let committee_member_in_current_epoch = membership_reader.has_stake(&public_key, epoch_number); + let committee_member_in_current_epoch = membership.has_stake(&public_key).await; // If the proposed leaf is for the last block in the epoch and the node is part of the quorum committee // in the next epoch, the node should vote to achieve the double quorum. let committee_member_in_next_epoch = leaf.with_epoch && is_last_block_in_epoch(leaf.height(), epoch_height) - && membership_reader.has_stake(&public_key, epoch_number.map(|x| x + 1)); - drop(membership_reader); + && membership.next_epoch().await.has_stake(&public_key).await; ensure!( committee_member_in_current_epoch || committee_member_in_next_epoch, @@ -619,7 +612,7 @@ pub(crate) async fn submit_vote, V let vote = QuorumVote2::::create_signed_vote( QuorumData2 { leaf_commit: leaf.commit(), - epoch: epoch_number, + epoch: membership.epoch(), }, view_number, &public_key, diff --git a/crates/task-impls/src/quorum_vote/mod.rs b/crates/task-impls/src/quorum_vote/mod.rs index 4e0d60b2cc..770d7281db 100644 --- a/crates/task-impls/src/quorum_vote/mod.rs +++ b/crates/task-impls/src/quorum_vote/mod.rs @@ -16,13 +16,22 @@ use hotshot_task::{ task::TaskState, }; use hotshot_types::{ - consensus::{ConsensusMetricsValue, OuterConsensus}, data::{Leaf2, QuorumProposalWrapper}, drb::DrbComputation, epoch_membership::EpochMembershipCoordinator, event::Event, message::{Proposal, UpgradeLock}, simple_vote::HasEpoch, traits::{ + consensus::{ConsensusMetricsValue, OuterConsensus}, + data::{Leaf2, QuorumProposalWrapper}, + drb::DrbComputation, + epoch_membership::EpochMembershipCoordinator, + event::Event, + message::{Proposal, UpgradeLock}, + simple_vote::HasEpoch, + traits::{ block_contents::BlockHeader, election::Membership, node_implementation::{ConsensusTime, NodeImplementation, NodeType, Versions}, signature_key::SignatureKey, storage::Storage, - }, utils::{epoch_from_block_number, option_epoch_from_block_number}, vote::{Certificate, HasViewNumber} + }, + utils::{epoch_from_block_number, option_epoch_from_block_number}, + vote::{Certificate, HasViewNumber}, }; use tokio::task::JoinHandle; use tracing::instrument; @@ -64,7 +73,7 @@ pub struct VoteDependencyHandle, V pub instance_state: Arc, /// Membership for Quorum certs/votes. - pub membership: EpochMembershipCoordinator, + pub membership_coordinator: EpochMembershipCoordinator, /// Reference to the storage. pub storage: Arc>, @@ -206,7 +215,7 @@ impl + 'static, V: Versions> Handl OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)), self.sender.clone(), self.receiver.clone(), - self.membership.clone(), + self.membership_coordinator.clone(), self.public_key.clone(), self.private_key.clone(), self.upgrade_lock.clone(), @@ -240,9 +249,13 @@ impl + 'static, V: Versions> Handl ) .await; + let mem = self + .membership_coordinator + .membership_for_epoch(cur_epoch) + .await; if let Err(e) = submit_vote::( self.sender.clone(), - Arc::clone(&self.membership), + mem, self.public_key.clone(), self.private_key.clone(), self.upgrade_lock.clone(), @@ -286,7 +299,7 @@ pub struct QuorumVoteTaskState, V: pub network: Arc, /// Membership for Quorum certs/votes and DA committee certs/votes. - pub membership: Arc>, + pub membership: EpochMembershipCoordinator, /// In-progress DRB computation task. pub drb_computation: DrbComputation, @@ -404,7 +417,7 @@ impl, V: Versions> QuorumVoteTaskS private_key: self.private_key.clone(), consensus: OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)), instance_state: Arc::clone(&self.instance_state), - membership: Arc::clone(&self.membership), + membership_coordinator: self.membership.clone(), storage: Arc::clone(&self.storage), view_number, sender: event_sender.clone(), @@ -516,11 +529,9 @@ impl, V: Versions> QuorumVoteTaskS let cert_epoch = cert.data.epoch; - let membership_reader = self.membership.read().await; - let membership_da_stake_table = membership_reader.da_stake_table(cert_epoch); - let membership_da_success_threshold = - membership_reader.da_success_threshold(cert_epoch); - drop(membership_reader); + let mem = self.membership.membership_for_epoch(cert_epoch).await; + let membership_da_stake_table = mem.da_stake_table().await; + let membership_da_success_threshold = mem.da_success_threshold().await; // Validate the DAC. cert.is_valid_cert( @@ -569,18 +580,23 @@ impl, V: Versions> QuorumVoteTaskS let vid_epoch = share.data.epoch(); let target_epoch = share.data.target_epoch(); - let membership_reader = self.membership.read().await; + let membership_reader = self.membership.membership_for_epoch(vid_epoch).await; // ensure that the VID share was sent by a DA member OR the view leader ensure!( membership_reader - .da_committee_members(view, vid_epoch) + .da_committee_members(view) + .await .contains(sender) - || *sender == membership_reader.leader(view, vid_epoch)?, + || *sender == membership_reader.leader(view).await?, "VID share was not sent by a DA member or the view leader." ); - let membership_total_nodes = membership_reader.total_nodes(target_epoch); - drop(membership_reader); + let membership_total_nodes = self + .membership + .membership_for_epoch(target_epoch) + .await + .total_nodes() + .await; if let Err(()) = share.data.verify_share(membership_total_nodes) { bail!("Failed to verify VID share"); @@ -697,7 +713,7 @@ impl, V: Versions> QuorumVoteTaskS OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)), event_sender.clone(), event_receiver.clone().deactivate(), - Arc::clone(&self.membership), + self.membership.clone(), self.public_key.clone(), self.private_key.clone(), self.upgrade_lock.clone(), @@ -744,7 +760,9 @@ impl, V: Versions> QuorumVoteTaskS submit_vote::( event_sender.clone(), - Arc::clone(&self.membership), + self.membership + .membership_for_epoch(Some(current_epoch)) + .await, self.public_key.clone(), self.private_key.clone(), self.upgrade_lock.clone(), diff --git a/crates/task-impls/src/request.rs b/crates/task-impls/src/request.rs index 9f6b144194..0f1b174493 100644 --- a/crates/task-impls/src/request.rs +++ b/crates/task-impls/src/request.rs @@ -68,7 +68,7 @@ pub struct NetworkRequestState> { pub delay: Duration, /// Membership (Used here only for DA) - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// This nodes public key pub public_key: TYPES::SignatureKey, diff --git a/crates/task-impls/src/response.rs b/crates/task-impls/src/response.rs index f0000a7c14..cbc0fa1084 100644 --- a/crates/task-impls/src/response.rs +++ b/crates/task-impls/src/response.rs @@ -12,6 +12,7 @@ use committable::Committable; use hotshot_types::{ consensus::{Consensus, LockedConsensusState, OuterConsensus}, data::VidDisperseShare, + epoch_membership::EpochMembershipCoordinator, message::{Proposal, UpgradeLock}, traits::{ election::Membership, @@ -36,7 +37,7 @@ pub struct NetworkResponseState { consensus: LockedConsensusState, /// Quorum membership for checking if requesters have state - membership: Arc>, + membership: EpochMembershipCoordinator, /// This replicas public key pub_key: TYPES::SignatureKey, @@ -55,7 +56,7 @@ impl NetworkResponseState { /// Create the network request state with the info it needs pub fn new( consensus: LockedConsensusState, - membership: Arc>, + membership: EpochMembershipCoordinator, pub_key: TYPES::SignatureKey, private_key: ::PrivateKey, id: u64, @@ -188,7 +189,7 @@ impl NetworkResponseState { OuterConsensus::new(Arc::clone(&self.consensus)), view, target_epoch, - Arc::clone(&self.membership), + self.membership.clone(), &self.private_key, &self.upgrade_lock, ) diff --git a/crates/task-impls/src/transactions.rs b/crates/task-impls/src/transactions.rs index e85ad658ff..e7f40691a8 100644 --- a/crates/task-impls/src/transactions.rs +++ b/crates/task-impls/src/transactions.rs @@ -92,7 +92,7 @@ pub struct TransactionTaskState, V pub consensus: OuterConsensus, /// Membership for the quorum - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// Builder 0.1 API clients pub builder_clients: Vec>, diff --git a/crates/task-impls/src/upgrade.rs b/crates/task-impls/src/upgrade.rs index 008ccfde4b..ffd1b012bb 100644 --- a/crates/task-impls/src/upgrade.rs +++ b/crates/task-impls/src/upgrade.rs @@ -47,7 +47,7 @@ pub struct UpgradeTaskState { pub cur_epoch: Option, /// Membership for Quorum Certs/votes - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// A map of `UpgradeVote` collector tasks pub vote_collectors: VoteCollectorsMap, UpgradeCertificate, V>, diff --git a/crates/task-impls/src/vid.rs b/crates/task-impls/src/vid.rs index 20476bdfca..2b6669d710 100644 --- a/crates/task-impls/src/vid.rs +++ b/crates/task-impls/src/vid.rs @@ -47,7 +47,7 @@ pub struct VidTaskState, V: Versio pub network: Arc, /// Membership for the quorum - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// This Nodes Public Key pub public_key: TYPES::SignatureKey, diff --git a/crates/task-impls/src/view_sync.rs b/crates/task-impls/src/view_sync.rs index 579785a89b..fd91bf831a 100644 --- a/crates/task-impls/src/view_sync.rs +++ b/crates/task-impls/src/view_sync.rs @@ -74,7 +74,7 @@ pub struct ViewSyncTaskState { pub cur_epoch: Option, /// Membership for the quorum - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// This Nodes Public Key pub public_key: TYPES::SignatureKey, @@ -161,7 +161,7 @@ pub struct ViewSyncReplicaTaskState { pub id: u64, /// Membership for the quorum - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// This Nodes Public Key pub public_key: TYPES::SignatureKey, diff --git a/crates/task-impls/src/vote_collection.rs b/crates/task-impls/src/vote_collection.rs index a556f48651..0cbeafb203 100644 --- a/crates/task-impls/src/vote_collection.rs +++ b/crates/task-impls/src/vote_collection.rs @@ -51,7 +51,7 @@ pub struct VoteCollectionTaskState< pub public_key: TYPES::SignatureKey, /// Membership for voting - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// accumulator handles aggregating the votes pub accumulator: Option>, @@ -178,7 +178,7 @@ pub struct AccumulatorInfo { pub public_key: TYPES::SignatureKey, /// Membership we are accumulation votes for - pub membership: Arc>, + pub membership_coordinator: EpochMembershipCoordinator, /// View of the votes we are collecting pub view: TYPES::View, diff --git a/crates/testing/src/byzantine/byzantine_behaviour.rs b/crates/testing/src/byzantine/byzantine_behaviour.rs index 8fab295bd4..dcc18b8ad0 100644 --- a/crates/testing/src/byzantine/byzantine_behaviour.rs +++ b/crates/testing/src/byzantine/byzantine_behaviour.rs @@ -339,7 +339,7 @@ impl + std::fmt::Debug, V: Version &self, handle: &mut SystemContextHandle, network: Arc<>::Network>, - membership: Arc>, + membership_coordinator: EpochMembershipCoordinator, ) { let network_state: NetworkEventTaskState<_, V, _, _> = NetworkEventTaskState { network, diff --git a/crates/testing/src/helpers.rs b/crates/testing/src/helpers.rs index 37b59ab56a..c17c4c8834 100644 --- a/crates/testing/src/helpers.rs +++ b/crates/testing/src/helpers.rs @@ -146,7 +146,7 @@ pub async fn build_cert< CERT: Certificate, >( data: DATAType, - membership: &Arc>, + epoch_membership: &EpochMembership, view: TYPES::View, epoch: Option, public_key: &TYPES::SignatureKey, @@ -212,16 +212,11 @@ pub async fn build_assembled_sig< DATAType: Committable + Clone + Eq + Hash + Serialize + Debug + 'static, >( data: &DATAType, - membership: &Arc>, + epoch_membership: &Arc>, view: TYPES::View, epoch: Option, upgrade_lock: &UpgradeLock, ) -> ::QcType { - // TODO get actual height - let epoch_membership = EpochMembership { - epoch, - membership: Arc::clone(membership), - }; let stake_table = CERT::stake_table(&epoch_membership).await; let real_qc_pp: ::QcParams = ::public_parameter( diff --git a/crates/testing/src/view_generator.rs b/crates/testing/src/view_generator.rs index abe4277590..37cddfcd09 100644 --- a/crates/testing/src/view_generator.rs +++ b/crates/testing/src/view_generator.rs @@ -25,22 +25,17 @@ use hotshot_types::{ data::{ DaProposal2, EpochNumber, Leaf2, QuorumProposal2, QuorumProposalWrapper, VidDisperse, VidDisperseShare, ViewChangeEvidence2, ViewNumber, - }, - message::{Proposal, UpgradeLock}, - simple_certificate::{ + }, epoch_membership::EpochMembershipCoordinator, message::{Proposal, UpgradeLock}, simple_certificate::{ DaCertificate2, QuorumCertificate2, TimeoutCertificate2, UpgradeCertificate, ViewSyncFinalizeCertificate2, - }, - simple_vote::{ + }, simple_vote::{ DaData2, DaVote2, QuorumData2, QuorumVote2, TimeoutData2, TimeoutVote2, UpgradeProposalData, UpgradeVote, ViewSyncFinalizeData2, ViewSyncFinalizeVote2, - }, - traits::{ + }, traits::{ consensus_api::ConsensusApi, node_implementation::{ConsensusTime, NodeType, Versions}, BlockPayload, - }, - utils::genesis_epoch_from_version, + }, utils::genesis_epoch_from_version }; use rand::{thread_rng, Rng}; use sha2::{Digest, Sha256}; @@ -56,7 +51,7 @@ pub struct TestView { pub leaf: Leaf2, pub view_number: ViewNumber, pub epoch_number: Option, - pub membership: Arc::Membership>>, + pub membership: EpochMembershipCoordinator, pub vid_disperse: Proposal>, pub vid_proposal: ( Vec>>, @@ -74,7 +69,7 @@ pub struct TestView { impl TestView { pub async fn genesis( - membership: &Arc::Membership>>, + membership: &EpochMembershipCoordinator<::Membership>, ) -> Self { let genesis_view = ViewNumber::new(1); let genesis_epoch = genesis_epoch_from_version::(); diff --git a/crates/testing/tests/tests_1/vote_dependency_handle.rs b/crates/testing/tests/tests_1/vote_dependency_handle.rs index 9b206bfdb2..867bcaa81b 100644 --- a/crates/testing/tests/tests_1/vote_dependency_handle.rs +++ b/crates/testing/tests/tests_1/vote_dependency_handle.rs @@ -90,7 +90,7 @@ async fn test_vote_dependency_handle() { consensus: OuterConsensus::new(consensus.clone()), consensus_metrics: Arc::clone(&consensus.read().await.metrics), instance_state: handle.hotshot.instance_state(), - membership: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: Arc::clone(&handle.hotshot.membership_coordinator), storage: Arc::clone(&handle.storage()), view_number, sender: event_sender.clone(), diff --git a/crates/types/src/consensus.rs b/crates/types/src/consensus.rs index a5b75a1b3c..5fe4828512 100644 --- a/crates/types/src/consensus.rs +++ b/crates/types/src/consensus.rs @@ -23,6 +23,7 @@ pub use crate::utils::{View, ViewInner}; use crate::{ data::{Leaf2, QuorumProposalWrapper, VidDisperse, VidDisperseShare}, drb::DrbSeedsAndResults, + epoch_membership::EpochMembershipCoordinator, error::HotShotError, event::{HotShotAction, LeafInfo}, message::{Proposal, UpgradeLock}, @@ -956,7 +957,7 @@ impl Consensus { consensus: OuterConsensus, view: ::View, target_epoch: Option<::Epoch>, - membership: Arc>, + membership_coordinator: EpochMembershipCoordinator, private_key: &::PrivateKey, upgrade_lock: &UpgradeLock, ) -> Option<()> { @@ -971,7 +972,7 @@ impl Consensus { let vid = VidDisperse::calculate_vid_disperse::( payload.as_ref(), - &membership, + &membership_coordinator, view, target_epoch, epoch, diff --git a/crates/types/src/data.rs b/crates/types/src/data.rs index 47fae4a823..927fd0f33b 100644 --- a/crates/types/src/data.rs +++ b/crates/types/src/data.rs @@ -29,16 +29,11 @@ use vec1::Vec1; use vid_disperse::{ADVZDisperse, ADVZDisperseShare, VidDisperseShare2}; use crate::{ - drb::DrbResult, - impl_has_epoch, impl_has_none_epoch, - message::{convert_proposal, Proposal, UpgradeLock}, - simple_certificate::{ + drb::DrbResult, epoch_membership::EpochMembershipCoordinator, impl_has_epoch, impl_has_none_epoch, message::{convert_proposal, Proposal, UpgradeLock}, simple_certificate::{ NextEpochQuorumCertificate2, QuorumCertificate, QuorumCertificate2, TimeoutCertificate, TimeoutCertificate2, UpgradeCertificate, ViewSyncFinalizeCertificate, ViewSyncFinalizeCertificate2, - }, - simple_vote::{HasEpoch, QuorumData, QuorumData2, UpgradeProposalData, VersionedVoteData}, - traits::{ + }, simple_vote::{HasEpoch, QuorumData, QuorumData2, UpgradeProposalData, VersionedVoteData}, traits::{ block_contents::{ vid_commitment, BlockHeader, BuilderFee, EncodeBytes, TestableBlock, GENESIS_VID_NUM_STORAGE_NODES, @@ -47,10 +42,7 @@ use crate::{ signature_key::SignatureKey, states::TestableState, BlockPayload, - }, - utils::{bincode_opts, genesis_epoch_from_version, option_epoch_from_block_number}, - vid::{VidCommitment, VidCommon, VidSchemeType}, - vote::{Certificate, HasViewNumber}, + }, utils::{bincode_opts, genesis_epoch_from_version, option_epoch_from_block_number}, vid::{VidCommitment, VidCommon, VidSchemeType}, vote::{Certificate, HasViewNumber} }; /// Implements `ConsensusTime`, `Display`, `Add`, `AddAssign`, `Deref` and `Sub` @@ -240,7 +232,7 @@ impl VidDisperse { pub async fn from_membership( view_number: TYPES::View, vid_disperse: JfVidDisperse, - membership: &Arc>, + membership: &EpochMembershipCoordinator, target_epoch: Option, data_epoch: Option, data_epoch_payload_commitment: Option, @@ -266,7 +258,7 @@ impl VidDisperse { #[allow(clippy::panic)] pub async fn calculate_vid_disperse( payload: &TYPES::BlockPayload, - membership: &Arc>, + membership: &EpochMembershipCoordinator, view: TYPES::View, target_epoch: Option, data_epoch: Option, diff --git a/crates/types/src/data/vid_disperse.rs b/crates/types/src/data/vid_disperse.rs index b81f6505e8..545260d0b6 100644 --- a/crates/types/src/data/vid_disperse.rs +++ b/crates/types/src/data/vid_disperse.rs @@ -15,6 +15,7 @@ use tokio::task::spawn_blocking; use utils::anytrace::*; use crate::{ + epoch_membership::EpochMembershipCoordinator, impl_has_epoch, message::Proposal, simple_vote::HasEpoch, @@ -60,15 +61,16 @@ impl ADVZDisperse { pub async fn from_membership( view_number: TYPES::View, mut vid_disperse: JfVidDisperse, - membership: &Arc>, + membership: &EpochMembershipCoordinator, target_epoch: Option, data_epoch: Option, data_epoch_payload_commitment: Option, ) -> Self { let shares = membership - .read() + .membership_for_epoch(target_epoch) + .await + .committee_members(view_number) .await - .committee_members(view_number, target_epoch) .iter() .map(|node| (node.clone(), vid_disperse.shares.remove(0))) .collect(); @@ -92,12 +94,16 @@ impl ADVZDisperse { #[allow(clippy::panic)] pub async fn calculate_vid_disperse( payload: &TYPES::BlockPayload, - membership: &Arc>, + membership: &EpochMembershipCoordinator, view: TYPES::View, target_epoch: Option, data_epoch: Option, ) -> Result { - let num_nodes = membership.read().await.total_nodes(target_epoch); + let num_nodes = membership + .membership_for_epoch(target_epoch) + .await + .total_nodes() + .await; let txns = payload.encode(); let txns_clone = Arc::clone(&txns); @@ -113,7 +119,11 @@ impl ADVZDisperse { let payload_commitment = if target_epoch == data_epoch { None } else { - let num_nodes = membership.read().await.total_nodes(data_epoch); + let num_nodes = membership + .membership_for_epoch(data_epoch) + .await + .total_nodes() + .await; Some( spawn_blocking(move || advz_scheme(num_nodes).commit_only(&txns)) diff --git a/crates/types/src/epoch_membership.rs b/crates/types/src/epoch_membership.rs index 70e3048603..60068b41ad 100644 --- a/crates/types/src/epoch_membership.rs +++ b/crates/types/src/epoch_membership.rs @@ -56,6 +56,13 @@ where epoch_height, } } + + /// Get a reference to the membership + #[must_use] + pub fn membership(&self) -> &Arc> { + &self.membership + } + /// Get a Membership for a given Epoch, which is guarenteed to have a stake /// table for the given Epoch pub async fn membership_for_epoch( @@ -64,7 +71,7 @@ where ) -> EpochMembership { let ret_val = EpochMembership { epoch: maybe_epoch, - membership: Arc::clone(&self.membership), + coordinator: self.clone(), }; let Some(epoch) = maybe_epoch else { return ret_val; @@ -142,13 +149,13 @@ pub struct EpochMembership { /// Epoch the `membership` is guarenteed to have a stake table for pub epoch: Option, /// Underlying membership - pub membership: Arc>, + pub coordinator: EpochMembershipCoordinator, } impl Clone for EpochMembership { fn clone(&self) -> Self { Self { - membership: Arc::clone(&self.membership), + coordinator: self.coordinator.clone(), epoch: self.epoch, } } @@ -159,12 +166,36 @@ impl EpochMembership { pub fn epoch(&self) -> Option { self.epoch } + + /// Get a membership for the next epoch + pub async fn next_epoch(&self) -> Self { + if self.epoch.is_none() { + self.clone() + } else { + self.coordinator + .membership_for_epoch(self.epoch.map(|e| e + 1)) + .await + } + } + + /// Get the prior epoch + pub async fn prev_epoch(&self) -> Self { + let Some(epoch) = self.epoch else { + return self.clone(); + }; + if *epoch == 0 { + return self.clone(); + } + self.coordinator.membership_for_epoch(Some(epoch - 1)).await + } + /// Wraps the same named Membership trait fn async fn get_epoch_root( &self, block_height: u64, ) -> Result<(TYPES::Epoch, TYPES::BlockHeader)> { - self.membership + self.coordinator + .membership .read() .await .get_epoch_root(block_height) @@ -173,14 +204,22 @@ impl EpochMembership { /// Get all participants in the committee (including their stake) for a specific epoch pub async fn stake_table(&self) -> Vec<::StakeTableEntry> { - self.membership.read().await.stake_table(self.epoch) + self.coordinator + .membership + .read() + .await + .stake_table(self.epoch) } /// Get all participants in the committee (including their stake) for a specific epoch pub async fn da_stake_table( &self, ) -> Vec<::StakeTableEntry> { - self.membership.read().await.da_stake_table(self.epoch) + self.coordinator + .membership + .read() + .await + .da_stake_table(self.epoch) } /// Get all participants in the committee for a specific view for a specific epoch @@ -188,7 +227,8 @@ impl EpochMembership { &self, view_number: TYPES::View, ) -> BTreeSet { - self.membership + self.coordinator + .membership .read() .await .committee_members(view_number, self.epoch) @@ -199,7 +239,8 @@ impl EpochMembership { &self, view_number: TYPES::View, ) -> BTreeSet { - self.membership + self.coordinator + .membership .read() .await .da_committee_members(view_number, self.epoch) @@ -210,7 +251,8 @@ impl EpochMembership { &self, view_number: TYPES::View, ) -> BTreeSet { - self.membership + self.coordinator + .membership .read() .await .committee_leaders(view_number, self.epoch) @@ -222,7 +264,11 @@ impl EpochMembership { &self, pub_key: &TYPES::SignatureKey, ) -> Option<::StakeTableEntry> { - self.membership.read().await.stake(pub_key, self.epoch) + self.coordinator + .membership + .read() + .await + .stake(pub_key, self.epoch) } /// Get the DA stake table entry for a public key, returns `None` if the @@ -231,17 +277,26 @@ impl EpochMembership { &self, pub_key: &TYPES::SignatureKey, ) -> Option<::StakeTableEntry> { - self.membership.read().await.da_stake(pub_key, self.epoch) + self.coordinator + .membership + .read() + .await + .da_stake(pub_key, self.epoch) } /// See if a node has stake in the committee in a specific epoch pub async fn has_stake(&self, pub_key: &TYPES::SignatureKey) -> bool { - self.membership.read().await.has_stake(pub_key, self.epoch) + self.coordinator + .membership + .read() + .await + .has_stake(pub_key, self.epoch) } /// See if a node has stake in the committee in a specific epoch pub async fn has_da_stake(&self, pub_key: &TYPES::SignatureKey) -> bool { - self.membership + self.coordinator + .membership .read() .await .has_da_stake(pub_key, self.epoch) @@ -255,7 +310,11 @@ impl EpochMembership { /// # Errors /// Returns an error if the leader cannot be calculated. pub async fn leader(&self, view: TYPES::View) -> Result { - self.membership.read().await.leader(view, self.epoch) + self.coordinator + .membership + .read() + .await + .leader(view, self.epoch) } /// The leader of the committee for view `view_number` in `epoch`. @@ -272,27 +331,44 @@ impl EpochMembership { TYPES::SignatureKey, <::Membership as Membership>::Error, > { - self.membership.read().await.lookup_leader(view, self.epoch) + self.coordinator + .membership + .read() + .await + .lookup_leader(view, self.epoch) } /// Returns the number of total nodes in the committee in an epoch `epoch` pub async fn total_nodes(&self) -> usize { - self.membership.read().await.total_nodes(self.epoch) + self.coordinator + .membership + .read() + .await + .total_nodes(self.epoch) } /// Returns the number of total DA nodes in the committee in an epoch `epoch` pub async fn da_total_nodes(&self) -> usize { - self.membership.read().await.da_total_nodes(self.epoch) + self.coordinator + .membership + .read() + .await + .da_total_nodes(self.epoch) } /// Returns the threshold for a specific `Membership` implementation pub async fn success_threshold(&self) -> NonZeroU64 { - self.membership.read().await.success_threshold(self.epoch) + self.coordinator + .membership + .read() + .await + .success_threshold(self.epoch) } /// Returns the DA threshold for a specific `Membership` implementation pub async fn da_success_threshold(&self) -> NonZeroU64 { - self.membership + self.coordinator + .membership .read() .await .da_success_threshold(self.epoch) @@ -300,11 +376,19 @@ impl EpochMembership { /// Returns the threshold for a specific `Membership` implementation pub async fn failure_threshold(&self) -> NonZeroU64 { - self.membership.read().await.failure_threshold(self.epoch) + self.coordinator + .membership + .read() + .await + .failure_threshold(self.epoch) } /// Returns the threshold required to upgrade the network protocol pub async fn upgrade_threshold(&self) -> NonZeroU64 { - self.membership.read().await.upgrade_threshold(self.epoch) + self.coordinator + .membership + .read() + .await + .upgrade_threshold(self.epoch) } } diff --git a/crates/types/src/message.rs b/crates/types/src/message.rs index 0cbb8bcdcb..a96832aec3 100644 --- a/crates/types/src/message.rs +++ b/crates/types/src/message.rs @@ -43,13 +43,12 @@ use crate::{ ViewSyncPreCommitVote, ViewSyncPreCommitVote2, }, traits::{ - block_contents::BlockHeader, election::Membership, network::{DataRequest, ResponseMessage, ViewMessage}, node_implementation::{ConsensusTime, NodeType, Versions}, signature_key::SignatureKey, }, - utils::{mnemonic, option_epoch_from_block_number}, + utils::mnemonic, vote::HasViewNumber, }; @@ -580,21 +579,8 @@ where /// Checks that the signature of the quorum proposal is valid. /// # Errors /// Returns an error when the proposal signature is invalid. - pub async fn validate_signature( - &self, - membership: &EpochMembership, - epoch_height: u64, - ) -> Result<()> { + pub async fn validate_signature(&self, membership: &EpochMembership) -> Result<()> { let view_number = self.data.proposal.view_number(); - let proposal_epoch = option_epoch_from_block_number::( - self.data.proposal.epoch().is_some(), - self.data.block_header().block_number(), - epoch_height, - ); - ensure!( - proposal_epoch == membership.epoch(), - "proposal epoch does not match membership epoch" - ); let view_leader_key = membership.leader(view_number).await?; let proposed_leaf = Leaf2::from_quorum_proposal(&self.data); diff --git a/crates/types/src/simple_certificate.rs b/crates/types/src/simple_certificate.rs index 56ada4544c..2ef2fc1f08 100644 --- a/crates/types/src/simple_certificate.rs +++ b/crates/types/src/simple_certificate.rs @@ -32,7 +32,6 @@ use crate::{ ViewSyncPreCommitData2, Voteable, }, traits::{ - election::Membership, node_implementation::{ConsensusTime, NodeType, Versions}, signature_key::SignatureKey, }, @@ -432,15 +431,14 @@ impl UpgradeCertificate { /// Returns an error when the upgrade certificate is invalid. pub async fn validate( upgrade_certificate: &Option, - membership: &RwLock, + membership: &EpochMembership, epoch: Option, upgrade_lock: &UpgradeLock, ) -> Result<()> { + ensure!(epoch == membership.epoch(), "Epochs don't match!"); if let Some(ref cert) = upgrade_certificate { - let membership_reader = membership.read().await; - let membership_stake_table = membership_reader.stake_table(epoch); - let membership_upgrade_threshold = membership_reader.upgrade_threshold(epoch); - drop(membership_reader); + let membership_stake_table = membership.stake_table().await; + let membership_upgrade_threshold = membership.upgrade_threshold().await; cert.is_valid_cert( membership_stake_table, diff --git a/crates/types/src/traits/network.rs b/crates/types/src/traits/network.rs index 4d85ee82dc..7a5455c7c6 100644 --- a/crates/types/src/traits/network.rs +++ b/crates/types/src/traits/network.rs @@ -30,7 +30,7 @@ use thiserror::Error; use tokio::{sync::mpsc::error::TrySendError, time::sleep}; use super::{node_implementation::NodeType, signature_key::SignatureKey}; -use crate::{data::ViewNumber, message::SequencingMessage, BoxSyncFuture}; +use crate::{data::ViewNumber, epoch_membership::EpochMembershipCoordinator, message::SequencingMessage, BoxSyncFuture}; /// Centralized server specific errors #[derive(Debug, Error, Serialize, Deserialize)] @@ -263,7 +263,7 @@ pub trait ConnectedNetwork: Clone + Send + Sync + 'st &'a self, _view: u64, _epoch: Option, - _membership: Arc>, + _membership_coordinator: EpochMembershipCoordinator, ) where TYPES: NodeType + 'a, { From b62c00bd9be2bb30431cf3ceaaedffe201017db2 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Wed, 5 Feb 2025 17:05:09 -0500 Subject: [PATCH 7/9] little more --- crates/task-impls/src/consensus/handlers.rs | 67 +++++++++---------- crates/task-impls/src/consensus/mod.rs | 1 + crates/task-impls/src/da.rs | 52 ++++++++------- crates/task-impls/src/network.rs | 72 ++++++++++++++++----- crates/task-impls/src/request.rs | 1 + crates/task-impls/src/vote_collection.rs | 23 ++----- crates/testing/src/view_generator.rs | 14 ++-- crates/types/src/data.rs | 15 ++++- crates/types/src/traits/network.rs | 5 +- 9 files changed, 152 insertions(+), 98 deletions(-) diff --git a/crates/task-impls/src/consensus/handlers.rs b/crates/task-impls/src/consensus/handlers.rs index cc01abfa29..cbca95042a 100644 --- a/crates/task-impls/src/consensus/handlers.rs +++ b/crates/task-impls/src/consensus/handlers.rs @@ -8,12 +8,11 @@ use std::{sync::Arc, time::Duration}; use async_broadcast::Sender; use chrono::Utc; +use futures::task; use hotshot_types::{ event::{Event, EventType}, simple_vote::{HasEpoch, QuorumVote2, TimeoutData2, TimeoutVote2}, - traits::{ - node_implementation::{ConsensusTime, NodeImplementation, NodeType}, - }, + traits::node_implementation::{ConsensusTime, NodeImplementation, NodeType}, utils::EpochTransitionIndicator, vote::{HasViewNumber, Vote}, }; @@ -44,12 +43,12 @@ pub(crate) async fn handle_quorum_vote_recv< .read() .await .is_high_qc_for_last_block(); - let we_are_leader = task_state - .membership - .read() - .await - .leader(vote.view_number() + 1, vote.data.epoch)? - == task_state.public_key; + let mem = task_state + .membership_coordinator + .membership_for_epoch(vote.data.epoch) + .await; + + let we_are_leader = mem.leader(vote.view_number() + 1).await? == task_state.public_key; ensure!( in_transition || we_are_leader, info!( @@ -67,7 +66,7 @@ pub(crate) async fn handle_quorum_vote_recv< &mut task_state.vote_collectors, vote, task_state.public_key.clone(), - &task_state.membership, + &mem, vote.data.epoch, task_state.id, &event, @@ -79,17 +78,13 @@ pub(crate) async fn handle_quorum_vote_recv< if let Some(vote_epoch) = vote.epoch() { // If the vote sender belongs to the next epoch, collect it separately to form the second QC - let has_stake = task_state - .membership - .read() - .await - .has_stake(&vote.signing_key(), Some(vote_epoch + 1)); + let has_stake = mem.next_epoch().await.has_stake(&vote.signing_key()).await; if has_stake { handle_vote( &mut task_state.next_epoch_vote_collectors, &vote.clone().into(), task_state.public_key.clone(), - &task_state.membership, + &mem, vote.data.epoch, task_state.id, &event, @@ -115,14 +110,13 @@ pub(crate) async fn handle_timeout_vote_recv< sender: &Sender>>, task_state: &mut ConsensusTaskState, ) -> Result<()> { + let mem = task_state + .membership_coordinator + .membership_for_epoch(task_state.cur_epoch) + .await; // Are we the leader for this view? ensure!( - task_state - .membership - .read() - .await - .leader(vote.view_number() + 1, task_state.cur_epoch)? - == task_state.public_key, + mem.leader(vote.view_number() + 1).await? == task_state.public_key, info!( "We are not the leader for view {:?}", vote.view_number() + 1 @@ -133,7 +127,7 @@ pub(crate) async fn handle_timeout_vote_recv< &mut task_state.timeout_vote_collectors, vote, task_state.public_key.clone(), - &task_state.membership, + &mem, vote.data.epoch, task_state.id, &event, @@ -164,10 +158,11 @@ pub async fn send_high_qc ensure!( task_state - .membership - .read() + .membership_coordinator + .membership_for_epoch(epoch) .await - .has_stake(&task_state.public_key, epoch), + .has_stake(&task_state.public_key), debug!( "We were not chosen for the consensus committee for view {:?}", view_number @@ -377,10 +373,11 @@ pub(crate) async fn handle_timeout .await; let leader = task_state - .membership - .read() + .membership_coordinator + .membership_for_epoch(task_state.cur_epoch) .await - .leader(view_number, task_state.cur_epoch); + .leader(view_number) + .await; let consensus_reader = task_state.consensus.read().await; consensus_reader.metrics.number_of_timeouts.add(1); diff --git a/crates/task-impls/src/consensus/mod.rs b/crates/task-impls/src/consensus/mod.rs index 960f8a956f..f0b9d561da 100644 --- a/crates/task-impls/src/consensus/mod.rs +++ b/crates/task-impls/src/consensus/mod.rs @@ -13,6 +13,7 @@ use either::Either; use hotshot_task::task::TaskState; use hotshot_types::{ consensus::OuterConsensus, + epoch_membership::EpochMembershipCoordinator, event::Event, message::UpgradeLock, simple_certificate::{NextEpochQuorumCertificate2, QuorumCertificate2, TimeoutCertificate2}, diff --git a/crates/task-impls/src/da.rs b/crates/task-impls/src/da.rs index 4963aec5bf..cdf8bf6a35 100644 --- a/crates/task-impls/src/da.rs +++ b/crates/task-impls/src/da.rs @@ -10,6 +10,7 @@ use async_broadcast::{Receiver, Sender}; use async_lock::RwLock; use async_trait::async_trait; use hotshot_task::task::TaskState; +use hotshot_types::epoch_membership::EpochMembershipCoordinator; use hotshot_types::simple_vote::HasEpoch; use hotshot_types::{ consensus::{Consensus, OuterConsensus}, @@ -118,10 +119,11 @@ impl, V: Versions> DaTaskState, V: Versions> DaTaskState, V: Versions> DaTaskState, V: Versions> DaTaskState( OuterConsensus::new(Arc::clone(&consensus.inner_consensus)), view_number, target_epoch, - membership, + membership.coordinator.clone(), &pk, &upgrade_lock, ) @@ -284,23 +285,25 @@ impl, V: Versions> DaTaskState, V: Versions> DaTaskState l, Err(e) => { @@ -819,7 +821,13 @@ impl< *maybe_action = Some(HotShotAction::DaVote); let view_number = vote.view_number(); let epoch = vote.data.epoch; - let leader = match self.membership.read().await.leader(view_number, epoch) { + let leader = match self + .membership_coordinator + .membership_for_epoch(vote.epoch()) + .await + .leader(view_number) + .await + { Ok(l) => l, Err(e) => { tracing::warn!( @@ -863,7 +871,13 @@ impl< } HotShotEvent::ViewSyncPreCommitVoteSend(vote) => { let view_number = vote.view_number() + vote.date().relay; - let leader = match self.membership.read().await.leader(view_number, self.epoch) { + let leader = match self + .membership_coordinator + .membership_for_epoch(self.epoch) + .await + .leader(view_number) + .await + { Ok(l) => l, Err(e) => { tracing::warn!( @@ -889,7 +903,13 @@ impl< HotShotEvent::ViewSyncCommitVoteSend(vote) => { *maybe_action = Some(HotShotAction::ViewSyncVote); let view_number = vote.view_number() + vote.date().relay; - let leader = match self.membership.read().await.leader(view_number, self.epoch) { + let leader = match self + .membership_coordinator + .membership_for_epoch(self.epoch) + .await + .leader(view_number) + .await + { Ok(l) => l, Err(e) => { tracing::warn!( @@ -915,7 +935,13 @@ impl< HotShotEvent::ViewSyncFinalizeVoteSend(vote) => { *maybe_action = Some(HotShotAction::ViewSyncVote); let view_number = vote.view_number() + vote.date().relay; - let leader = match self.membership.read().await.leader(view_number, self.epoch) { + let leader = match self + .membership_coordinator + .membership_for_epoch(self.epoch) + .await + .leader(view_number) + .await + { Ok(l) => l, Err(e) => { tracing::warn!( @@ -983,7 +1009,13 @@ impl< HotShotEvent::TimeoutVoteSend(vote) => { *maybe_action = Some(HotShotAction::Vote); let view_number = vote.view_number() + 1; - let leader = match self.membership.read().await.leader(view_number, self.epoch) { + let leader = match self + .membership_coordinator + .membership_for_epoch(self.epoch) + .await + .leader(view_number) + .await + { Ok(l) => l, Err(e) => { tracing::warn!( @@ -1016,7 +1048,13 @@ impl< HotShotEvent::UpgradeVoteSend(vote) => { tracing::error!("Sending upgrade vote!"); let view_number = vote.view_number(); - let leader = match self.membership.read().await.leader(view_number, self.epoch) { + let leader = match self + .membership_coordinator + .membership_for_epoch(self.epoch) + .await + .leader(view_number) + .await + { Ok(l) => l, Err(e) => { tracing::warn!( @@ -1044,7 +1082,7 @@ impl< self.cancel_tasks(keep_view); let net = Arc::clone(&self.network); let epoch = self.epoch.map(|x| x.u64()); - let mem = Arc::clone(&self.membership); + let mem = self.membership_coordinator.clone(); spawn(async move { net.update_view::(*keep_view, epoch, mem).await; }); @@ -1136,10 +1174,11 @@ impl< let epoch = message.kind.epoch(); let committee_topic = Topic::Global; let da_committee = self - .membership - .read() + .membership_coordinator + .membership_for_epoch(self.epoch) .await - .da_committee_members(view_number, self.epoch); + .da_committee_members(view_number) + .await; let network = Arc::clone(&self.network); let storage = Arc::clone(&self.storage); let consensus = OuterConsensus::new(Arc::clone(&self.consensus.inner_consensus)); @@ -1217,6 +1256,7 @@ pub mod test { use std::ops::{Deref, DerefMut}; use async_trait::async_trait; + use hotshot_types::epoch_membership::EpochMembershipCoordinator; use super::{ Arc, ConnectedNetwork, HotShotEvent, MessageKind, NetworkEventTaskState, NodeType, @@ -1229,7 +1269,7 @@ pub mod test { &mut ::SignatureKey, &mut MessageKind, &mut TransmitType, - &::Membership, + &EpochMembershipCoordinator, ) + Send + Sync; @@ -1261,14 +1301,12 @@ pub mod test { self.parse_event(event, &mut maybe_action).await { // Modify the values acquired by parsing the event. - let membership_reader = self.membership.read().await; (self.modifier)( &mut sender, &mut message_kind, &mut transmit, - &membership_reader, + &self.membership_coordinator, ); - drop(membership_reader); self.spawn_transmit_task(message_kind, maybe_action, transmit, sender) .await; diff --git a/crates/task-impls/src/request.rs b/crates/task-impls/src/request.rs index 0f1b174493..567a296d96 100644 --- a/crates/task-impls/src/request.rs +++ b/crates/task-impls/src/request.rs @@ -22,6 +22,7 @@ use hotshot_task::{ }; use hotshot_types::{ consensus::OuterConsensus, + epoch_membership::EpochMembershipCoordinator, simple_vote::HasEpoch, traits::{ block_contents::BlockHeader, diff --git a/crates/task-impls/src/vote_collection.rs b/crates/task-impls/src/vote_collection.rs index 0cbeafb203..7932d53651 100644 --- a/crates/task-impls/src/vote_collection.rs +++ b/crates/task-impls/src/vote_collection.rs @@ -51,7 +51,7 @@ pub struct VoteCollectionTaskState< pub public_key: TYPES::SignatureKey, /// Membership for voting - pub membership_coordinator: EpochMembershipCoordinator, + pub membership: EpochMembership, /// accumulator handles aggregating the votes pub accumulator: Option>, @@ -59,9 +59,6 @@ pub struct VoteCollectionTaskState< /// The view which we are collecting votes for pub view: TYPES::View, - /// The epoch which we are collecting votes for - pub epoch: Option, - /// Node id pub id: u64, @@ -109,13 +106,11 @@ impl< event_stream: &Sender>>, ) -> Result> { // TODO create this only once - let coordinator = EpochMembershipCoordinator::new(Arc::clone(&self.membership), 10); - let epoch_membership = coordinator.membership_for_epoch(sender_epoch).await; ensure!( matches!( self.transition_indicator, EpochTransitionIndicator::InTransition - ) || vote.leader(&epoch_membership).await? == self.public_key, + ) || vote.leader(&self.membership).await? == self.public_key, info!("Received vote for a view in which we were not the leader.") ); @@ -132,7 +127,7 @@ impl< "No accumulator to handle vote with. This shouldn't happen." ))?; - match accumulator.accumulate(vote, epoch_membership).await { + match accumulator.accumulate(vote, self.membership).await { None => Ok(None), Some(cert) => { tracing::debug!("Certificate Formed! {:?}", cert); @@ -178,14 +173,11 @@ pub struct AccumulatorInfo { pub public_key: TYPES::SignatureKey, /// Membership we are accumulation votes for - pub membership_coordinator: EpochMembershipCoordinator, + pub membership: EpochMembership, /// View of the votes we are collecting pub view: TYPES::View, - /// Epoch of the votes we are collecting - pub epoch: Option, - /// This nodes id pub id: u64, } @@ -227,11 +219,10 @@ where }; let mut state = VoteCollectionTaskState:: { - membership: Arc::clone(&info.membership), + membership: info.membership.clone(), public_key: info.public_key.clone(), accumulator: Some(new_accumulator), view: info.view, - epoch: info.epoch, id: info.id, transition_indicator, }; @@ -259,7 +250,7 @@ pub async fn handle_vote< collectors: &mut VoteCollectorsMap, vote: &VOTE, public_key: TYPES::SignatureKey, - membership: &Arc>, + membership: &EpochMembership, epoch: Option, id: u64, event: &Arc>, @@ -275,7 +266,7 @@ where tracing::debug!("Starting vote handle for view {:?}", vote.view_number()); let info = AccumulatorInfo { public_key, - membership: Arc::clone(membership), + membership: membership.clone(), view: vote.view_number(), epoch, id, diff --git a/crates/testing/src/view_generator.rs b/crates/testing/src/view_generator.rs index 37cddfcd09..41c894bb03 100644 --- a/crates/testing/src/view_generator.rs +++ b/crates/testing/src/view_generator.rs @@ -25,17 +25,23 @@ use hotshot_types::{ data::{ DaProposal2, EpochNumber, Leaf2, QuorumProposal2, QuorumProposalWrapper, VidDisperse, VidDisperseShare, ViewChangeEvidence2, ViewNumber, - }, epoch_membership::EpochMembershipCoordinator, message::{Proposal, UpgradeLock}, simple_certificate::{ + }, + epoch_membership::EpochMembershipCoordinator, + message::{Proposal, UpgradeLock}, + simple_certificate::{ DaCertificate2, QuorumCertificate2, TimeoutCertificate2, UpgradeCertificate, ViewSyncFinalizeCertificate2, - }, simple_vote::{ + }, + simple_vote::{ DaData2, DaVote2, QuorumData2, QuorumVote2, TimeoutData2, TimeoutVote2, UpgradeProposalData, UpgradeVote, ViewSyncFinalizeData2, ViewSyncFinalizeVote2, - }, traits::{ + }, + traits::{ consensus_api::ConsensusApi, node_implementation::{ConsensusTime, NodeType, Versions}, BlockPayload, - }, utils::genesis_epoch_from_version + }, + utils::genesis_epoch_from_version, }; use rand::{thread_rng, Rng}; use sha2::{Digest, Sha256}; diff --git a/crates/types/src/data.rs b/crates/types/src/data.rs index 927fd0f33b..f2c6e22d48 100644 --- a/crates/types/src/data.rs +++ b/crates/types/src/data.rs @@ -29,11 +29,17 @@ use vec1::Vec1; use vid_disperse::{ADVZDisperse, ADVZDisperseShare, VidDisperseShare2}; use crate::{ - drb::DrbResult, epoch_membership::EpochMembershipCoordinator, impl_has_epoch, impl_has_none_epoch, message::{convert_proposal, Proposal, UpgradeLock}, simple_certificate::{ + drb::DrbResult, + epoch_membership::EpochMembershipCoordinator, + impl_has_epoch, impl_has_none_epoch, + message::{convert_proposal, Proposal, UpgradeLock}, + simple_certificate::{ NextEpochQuorumCertificate2, QuorumCertificate, QuorumCertificate2, TimeoutCertificate, TimeoutCertificate2, UpgradeCertificate, ViewSyncFinalizeCertificate, ViewSyncFinalizeCertificate2, - }, simple_vote::{HasEpoch, QuorumData, QuorumData2, UpgradeProposalData, VersionedVoteData}, traits::{ + }, + simple_vote::{HasEpoch, QuorumData, QuorumData2, UpgradeProposalData, VersionedVoteData}, + traits::{ block_contents::{ vid_commitment, BlockHeader, BuilderFee, EncodeBytes, TestableBlock, GENESIS_VID_NUM_STORAGE_NODES, @@ -42,7 +48,10 @@ use crate::{ signature_key::SignatureKey, states::TestableState, BlockPayload, - }, utils::{bincode_opts, genesis_epoch_from_version, option_epoch_from_block_number}, vid::{VidCommitment, VidCommon, VidSchemeType}, vote::{Certificate, HasViewNumber} + }, + utils::{bincode_opts, genesis_epoch_from_version, option_epoch_from_block_number}, + vid::{VidCommitment, VidCommon, VidSchemeType}, + vote::{Certificate, HasViewNumber}, }; /// Implements `ConsensusTime`, `Display`, `Add`, `AddAssign`, `Deref` and `Sub` diff --git a/crates/types/src/traits/network.rs b/crates/types/src/traits/network.rs index 7a5455c7c6..aeb1509401 100644 --- a/crates/types/src/traits/network.rs +++ b/crates/types/src/traits/network.rs @@ -30,7 +30,10 @@ use thiserror::Error; use tokio::{sync::mpsc::error::TrySendError, time::sleep}; use super::{node_implementation::NodeType, signature_key::SignatureKey}; -use crate::{data::ViewNumber, epoch_membership::EpochMembershipCoordinator, message::SequencingMessage, BoxSyncFuture}; +use crate::{ + data::ViewNumber, epoch_membership::EpochMembershipCoordinator, message::SequencingMessage, + BoxSyncFuture, +}; /// Centralized server specific errors #[derive(Debug, Error, Serialize, Deserialize)] From 492c36f68ea8307e668d8e4c69f7bb8defec9c72 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Fri, 7 Feb 2025 12:15:08 -0500 Subject: [PATCH 8/9] building? --- crates/examples/infra/mod.rs | 12 +- crates/hotshot/src/lib.rs | 1 - crates/hotshot/src/tasks/mod.rs | 23 ++-- .../src/traits/networking/combined_network.rs | 3 +- .../src/traits/networking/libp2p_network.rs | 38 ++++--- crates/hotshot/src/types/handle.rs | 12 +- crates/task-impls/src/consensus/handlers.rs | 9 +- crates/task-impls/src/consensus/mod.rs | 1 - crates/task-impls/src/da.rs | 3 - crates/task-impls/src/network.rs | 7 +- .../src/quorum_proposal/handlers.rs | 4 +- crates/task-impls/src/quorum_proposal/mod.rs | 1 - crates/task-impls/src/quorum_vote/mod.rs | 1 - crates/task-impls/src/request.rs | 29 +++-- crates/task-impls/src/response.rs | 10 +- crates/task-impls/src/transactions.rs | 24 +++- crates/task-impls/src/upgrade.rs | 50 +++++---- crates/task-impls/src/vid.rs | 14 +-- crates/task-impls/src/view_sync.rs | 96 ++++++++-------- crates/task-impls/src/vote_collection.rs | 38 ++----- .../src/byzantine/byzantine_behaviour.rs | 3 +- crates/testing/src/helpers.rs | 56 ++++------ crates/testing/src/overall_safety_task.rs | 58 +++++----- crates/testing/src/spinning_task.rs | 4 +- crates/testing/src/test_builder.rs | 8 +- crates/testing/src/test_runner.rs | 7 +- crates/testing/src/view_generator.rs | 46 ++++---- crates/testing/tests/tests_1/da_task.rs | 41 +++---- crates/testing/tests/tests_1/message.rs | 13 +-- crates/testing/tests/tests_1/network_task.rs | 12 +- .../tests_1/quorum_proposal_recv_task.rs | 4 +- .../tests/tests_1/quorum_proposal_task.rs | 104 ++++++++---------- .../testing/tests/tests_1/quorum_vote_task.rs | 6 +- .../testing/tests/tests_1/transaction_task.rs | 10 +- .../tests_1/upgrade_task_with_proposal.rs | 21 ++-- .../tests/tests_1/upgrade_task_with_vote.rs | 2 +- crates/testing/tests/tests_1/vid_task.rs | 11 +- .../tests/tests_1/vote_dependency_handle.rs | 4 +- crates/types/src/data/vid_disperse.rs | 4 +- crates/types/src/traits/network.rs | 1 - 40 files changed, 376 insertions(+), 415 deletions(-) diff --git a/crates/examples/infra/mod.rs b/crates/examples/infra/mod.rs index 7b02206df1..e7b3a26518 100755 --- a/crates/examples/infra/mod.rs +++ b/crates/examples/infra/mod.rs @@ -51,6 +51,7 @@ use hotshot_testing::block_builder::{ use hotshot_types::{ consensus::ConsensusMetricsValue, data::{Leaf, TestableLeaf}, + epoch_membership::EpochMembershipCoordinator, event::{Event, EventType}, network::{BuilderType, NetworkConfig, NetworkConfigFile, NetworkConfigSource}, traits::{ @@ -386,13 +387,14 @@ pub trait RunDa< // TODO: we need to pass a valid fallback builder url here somehow fallback_builder_url: config.config.builder_urls.first().clone(), }; + let epoch_height = config.config.epoch_height; SystemContext::init( pk, sk, config.node_index, config.config, - membership, + EpochMembershipCoordinator::new(membership, epoch_height), Arc::from(network), initializer, ConsensusMetricsValue::default(), @@ -525,12 +527,10 @@ pub trait RunDa< let num_eligible_leaders = context .hotshot .membership_coordinator - .read() + .membership_for_epoch(genesis_epoch_from_version::()) + .await + .committee_leaders(TYPES::View::genesis()) .await - .committee_leaders( - TYPES::View::genesis(), - genesis_epoch_from_version::(), - ) .len(); let consensus_lock = context.hotshot.consensus(); let consensus_reader = consensus_lock.read().await; diff --git a/crates/hotshot/src/lib.rs b/crates/hotshot/src/lib.rs index c33ff46621..5e7efb202b 100644 --- a/crates/hotshot/src/lib.rs +++ b/crates/hotshot/src/lib.rs @@ -57,7 +57,6 @@ use hotshot_types::{ simple_certificate::{NextEpochQuorumCertificate2, QuorumCertificate2, UpgradeCertificate}, traits::{ consensus_api::ConsensusApi, - election::Membership, network::ConnectedNetwork, node_implementation::{ConsensusTime, NodeType}, signature_key::SignatureKey, diff --git a/crates/hotshot/src/tasks/mod.rs b/crates/hotshot/src/tasks/mod.rs index 0fe9745cdb..7fb570a87a 100644 --- a/crates/hotshot/src/tasks/mod.rs +++ b/crates/hotshot/src/tasks/mod.rs @@ -10,6 +10,7 @@ pub mod task_state; use std::{collections::BTreeMap, fmt::Debug, num::NonZeroUsize, sync::Arc, time::Duration}; +use crate::EpochMembershipCoordinator; use async_broadcast::{broadcast, RecvError}; use async_lock::RwLock; use async_trait::async_trait; @@ -196,13 +197,12 @@ pub fn add_network_event_task< >( handle: &mut SystemContextHandle, network: Arc, - membership_coordinator: EpochMembershipCoordinator, ) { let network_state: NetworkEventTaskState<_, V, _, _> = NetworkEventTaskState { network, view: TYPES::View::genesis(), epoch: genesis_epoch_from_version::(), - membership_coordinator, + membership_coordinator: handle.membership_coordinator.clone(), storage: Arc::clone(&handle.storage()), consensus: OuterConsensus::new(handle.consensus()), upgrade_lock: handle.hotshot.upgrade_lock.clone(), @@ -328,7 +328,7 @@ where private_key: ::PrivateKey, nonce: u64, config: HotShotConfig, - memberships: Arc>, + memberships: EpochMembershipCoordinator, network: Arc, initializer: HotShotInitializer, metrics: ConsensusMetricsValue, @@ -336,12 +336,13 @@ where marketplace_config: MarketplaceConfig, ) -> SystemContextHandle { let epoch_height = config.epoch_height; + let hotshot = SystemContext::new( public_key, private_key, nonce, config, - memberships, + memberships.clone(), network, initializer, metrics, @@ -363,7 +364,7 @@ where hotshot: Arc::clone(&hotshot), storage: Arc::clone(&hotshot.storage), network: Arc::clone(&hotshot.network), - membership_coordinator: Arc::clone(&hotshot.membership_coordinator), + membership_coordinator: memberships.clone(), epoch_height, }; @@ -523,9 +524,8 @@ where /// Adds the `NetworkEventTaskState` tasks possibly modifying them as well. fn add_network_event_tasks(&self, handle: &mut SystemContextHandle) { let network = Arc::clone(&handle.network); - let memberships = Arc::clone(&handle.membership_coordinator); - self.add_network_event_task(handle, network, memberships); + self.add_network_event_task(handle, network); } /// Adds a `NetworkEventTaskState` task. Can be reimplemented to modify its behaviour. @@ -533,9 +533,8 @@ where &self, handle: &mut SystemContextHandle, channel: Arc<>::Network>, - membership_coordinator: EpochMembershipCoordinator, ) { - add_network_event_task(handle, channel, membership); + add_network_event_task(handle, channel); } } @@ -568,9 +567,5 @@ pub async fn add_network_message_and_request_receiver_tasks< pub fn add_network_event_tasks, V: Versions>( handle: &mut SystemContextHandle, ) { - add_network_event_task( - handle, - Arc::clone(&handle.network), - Arc::clone(&handle.membership_coordinator), - ); + add_network_event_task(handle, Arc::clone(&handle.network)); } diff --git a/crates/hotshot/src/traits/networking/combined_network.rs b/crates/hotshot/src/traits/networking/combined_network.rs index 438f790d68..87b4a873c2 100644 --- a/crates/hotshot/src/traits/networking/combined_network.rs +++ b/crates/hotshot/src/traits/networking/combined_network.rs @@ -32,6 +32,7 @@ use hotshot_types::{ COMBINED_NETWORK_MIN_PRIMARY_FAILURES, COMBINED_NETWORK_PRIMARY_CHECK_INTERVAL, }, data::ViewNumber, + epoch_membership::EpochMembershipCoordinator, traits::{ network::{BroadcastDelay, ConnectedNetwork, Topic}, node_implementation::NodeType, @@ -471,7 +472,7 @@ impl ConnectedNetwork for CombinedNetworks &'a self, view: u64, epoch: Option, - membership: Arc>, + membership: EpochMembershipCoordinator, ) where T: NodeType + 'a, { diff --git a/crates/hotshot/src/traits/networking/libp2p_network.rs b/crates/hotshot/src/traits/networking/libp2p_network.rs index 0c6cdfe05e..64852669a5 100644 --- a/crates/hotshot/src/traits/networking/libp2p_network.rs +++ b/crates/hotshot/src/traits/networking/libp2p_network.rs @@ -7,21 +7,7 @@ //! Libp2p based/production networking implementation //! This module provides a libp2p based networking implementation where each node in the //! network forms a tcp or udp connection to a subset of other nodes in the network -#[cfg(feature = "hotshot-testing")] -use std::str::FromStr; -use std::{ - cmp::min, - collections::{BTreeSet, HashSet}, - fmt::Debug, - net::{IpAddr, ToSocketAddrs}, - num::NonZeroUsize, - sync::{ - atomic::{AtomicBool, AtomicU64, Ordering}, - Arc, - }, - time::Duration, -}; - +use crate::EpochMembershipCoordinator; use anyhow::{anyhow, Context}; use async_lock::RwLock; use async_trait::async_trait; @@ -37,7 +23,6 @@ use hotshot_types::{ data::ViewNumber, network::NetworkConfig, traits::{ - election::Membership, metrics::{Counter, Gauge, Metrics, NoMetrics}, network::{ConnectedNetwork, NetworkError, Topic}, node_implementation::{ConsensusTime, NodeType}, @@ -66,6 +51,20 @@ use libp2p_networking::{ }; use rand::{rngs::StdRng, seq::IteratorRandom, SeedableRng}; use serde::Serialize; +#[cfg(feature = "hotshot-testing")] +use std::str::FromStr; +use std::{ + cmp::min, + collections::{BTreeSet, HashSet}, + fmt::Debug, + net::{IpAddr, ToSocketAddrs}, + num::NonZeroUsize, + sync::{ + atomic::{AtomicBool, AtomicU64, Ordering}, + Arc, + }, + time::Duration, +}; use tokio::{ select, spawn, sync::{ @@ -999,7 +998,12 @@ impl ConnectedNetwork for Libp2pNetwork { let future_view = ::View::new(view) + LOOK_AHEAD; let epoch = epoch.map(::Epoch::new); - let future_leader = match membership.read().await.leader(future_view, epoch) { + let future_leader = match membership_coordinator + .membership_for_epoch(epoch) + .await + .leader(future_view) + .await + { Ok(l) => l, Err(e) => { return tracing::info!( diff --git a/crates/hotshot/src/types/handle.rs b/crates/hotshot/src/types/handle.rs index c0c0a563a5..1b6133f738 100644 --- a/crates/hotshot/src/types/handle.rs +++ b/crates/hotshot/src/types/handle.rs @@ -26,13 +26,13 @@ use hotshot_types::{ message::{Message, MessageKind, Proposal, RecipientList}, request_response::ProposalRequestPayload, traits::{ + block_contents::BlockHeader, consensus_api::ConsensusApi, - election::Membership, network::{BroadcastDelay, ConnectedNetwork, Topic}, node_implementation::NodeType, signature_key::SignatureKey, }, - utils::{epoch_from_block_number, option_epoch_from_block_number}, + utils::option_epoch_from_block_number, }; use tracing::instrument; @@ -187,14 +187,14 @@ impl + 'static, V: Versions> // Then, if it's `Some`, make sure that the data is correct if let HotShotEvent::QuorumProposalResponseRecv(quorum_proposal) = hs_event.as_ref() { - let maybe_epoch = option_epoch_from_block_number( - quorum_proposal.data.epoch.is_some(), - quorum_proposal.data.block_header().height, + let maybe_epoch = option_epoch_from_block_number::( + quorum_proposal.data.proposal.epoch.is_some(), + quorum_proposal.data.block_header().block_number(), epoch_height, ); let membership = mem.membership_for_epoch(maybe_epoch).await; // Make sure that the quorum_proposal is valid - if let Err(err) = quorum_proposal.validate_signature(&membership) { + if let Err(err) = quorum_proposal.validate_signature(&membership).await { tracing::warn!("Invalid Proposal Received after Request. Err {:?}", err); continue; } diff --git a/crates/task-impls/src/consensus/handlers.rs b/crates/task-impls/src/consensus/handlers.rs index cbca95042a..711c616c27 100644 --- a/crates/task-impls/src/consensus/handlers.rs +++ b/crates/task-impls/src/consensus/handlers.rs @@ -8,7 +8,6 @@ use std::{sync::Arc, time::Duration}; use async_broadcast::Sender; use chrono::Utc; -use futures::task; use hotshot_types::{ event::{Event, EventType}, simple_vote::{HasEpoch, QuorumVote2, TimeoutData2, TimeoutVote2}, @@ -67,7 +66,6 @@ pub(crate) async fn handle_quorum_vote_recv< vote, task_state.public_key.clone(), &mem, - vote.data.epoch, task_state.id, &event, sender, @@ -76,7 +74,7 @@ pub(crate) async fn handle_quorum_vote_recv< ) .await?; - if let Some(vote_epoch) = vote.epoch() { + if vote.epoch().is_some() { // If the vote sender belongs to the next epoch, collect it separately to form the second QC let has_stake = mem.next_epoch().await.has_stake(&vote.signing_key()).await; if has_stake { @@ -85,7 +83,6 @@ pub(crate) async fn handle_quorum_vote_recv< &vote.clone().into(), task_state.public_key.clone(), &mem, - vote.data.epoch, task_state.id, &event, sender, @@ -128,7 +125,6 @@ pub(crate) async fn handle_timeout_vote_recv< vote, task_state.public_key.clone(), &mem, - vote.data.epoch, task_state.id, &event, sender, @@ -327,7 +323,8 @@ pub(crate) async fn handle_timeout .membership_coordinator .membership_for_epoch(epoch) .await - .has_stake(&task_state.public_key), + .has_stake(&task_state.public_key) + .await, debug!( "We were not chosen for the consensus committee for view {:?}", view_number diff --git a/crates/task-impls/src/consensus/mod.rs b/crates/task-impls/src/consensus/mod.rs index f0b9d561da..5d39ababe1 100644 --- a/crates/task-impls/src/consensus/mod.rs +++ b/crates/task-impls/src/consensus/mod.rs @@ -7,7 +7,6 @@ use std::sync::Arc; use async_broadcast::{Receiver, Sender}; -use async_lock::RwLock; use async_trait::async_trait; use either::Either; use hotshot_task::task::TaskState; diff --git a/crates/task-impls/src/da.rs b/crates/task-impls/src/da.rs index cdf8bf6a35..824a409742 100644 --- a/crates/task-impls/src/da.rs +++ b/crates/task-impls/src/da.rs @@ -11,7 +11,6 @@ use async_lock::RwLock; use async_trait::async_trait; use hotshot_task::task::TaskState; use hotshot_types::epoch_membership::EpochMembershipCoordinator; -use hotshot_types::simple_vote::HasEpoch; use hotshot_types::{ consensus::{Consensus, OuterConsensus}, data::{DaProposal2, PackedBundle}, @@ -21,7 +20,6 @@ use hotshot_types::{ simple_vote::{DaData2, DaVote2}, traits::{ block_contents::vid_commitment, - election::Membership, network::ConnectedNetwork, node_implementation::{NodeImplementation, NodeType, Versions}, signature_key::SignatureKey, @@ -304,7 +302,6 @@ impl, V: Versions> DaTaskState { *maybe_action = Some(HotShotAction::DaVote); let view_number = vote.view_number(); - let epoch = vote.data.epoch; let leader = match self .membership_coordinator .membership_for_epoch(vote.epoch()) @@ -1256,7 +1254,6 @@ pub mod test { use std::ops::{Deref, DerefMut}; use async_trait::async_trait; - use hotshot_types::epoch_membership::EpochMembershipCoordinator; use super::{ Arc, ConnectedNetwork, HotShotEvent, MessageKind, NetworkEventTaskState, NodeType, @@ -1269,7 +1266,7 @@ pub mod test { &mut ::SignatureKey, &mut MessageKind, &mut TransmitType, - &EpochMembershipCoordinator, + &::Membership, ) + Send + Sync; @@ -1305,7 +1302,7 @@ pub mod test { &mut sender, &mut message_kind, &mut transmit, - &self.membership_coordinator, + &*self.membership_coordinator.membership().read().await, ); self.spawn_transmit_task(message_kind, maybe_action, transmit, sender) diff --git a/crates/task-impls/src/quorum_proposal/handlers.rs b/crates/task-impls/src/quorum_proposal/handlers.rs index 2d9f4db1ac..c3e49b2300 100644 --- a/crates/task-impls/src/quorum_proposal/handlers.rs +++ b/crates/task-impls/src/quorum_proposal/handlers.rs @@ -29,8 +29,7 @@ use hotshot_types::{ message::Proposal, simple_certificate::{NextEpochQuorumCertificate2, QuorumCertificate2, UpgradeCertificate}, traits::{ - block_contents::BlockHeader, election::Membership, node_implementation::NodeType, - signature_key::SignatureKey, + block_contents::BlockHeader, node_implementation::NodeType, signature_key::SignatureKey, }, utils::{is_last_block_in_epoch, option_epoch_from_block_number}, vote::{Certificate, HasViewNumber}, @@ -139,7 +138,6 @@ impl ProposalDependencyHandle { }; let membership_stake_table = mem.stake_table().await; let membership_success_threshold = mem.success_threshold().await; - drop(mem); if qc .is_valid_cert( diff --git a/crates/task-impls/src/quorum_proposal/mod.rs b/crates/task-impls/src/quorum_proposal/mod.rs index 2b524607ae..702fc6372f 100644 --- a/crates/task-impls/src/quorum_proposal/mod.rs +++ b/crates/task-impls/src/quorum_proposal/mod.rs @@ -21,7 +21,6 @@ use hotshot_types::{ message::UpgradeLock, simple_certificate::{QuorumCertificate2, UpgradeCertificate}, traits::{ - election::Membership, node_implementation::{ConsensusTime, NodeImplementation, NodeType, Versions}, signature_key::SignatureKey, storage::Storage, diff --git a/crates/task-impls/src/quorum_vote/mod.rs b/crates/task-impls/src/quorum_vote/mod.rs index 770d7281db..310a9c9359 100644 --- a/crates/task-impls/src/quorum_vote/mod.rs +++ b/crates/task-impls/src/quorum_vote/mod.rs @@ -25,7 +25,6 @@ use hotshot_types::{ simple_vote::HasEpoch, traits::{ block_contents::BlockHeader, - election::Membership, node_implementation::{ConsensusTime, NodeImplementation, NodeType, Versions}, signature_key::SignatureKey, storage::Storage, diff --git a/crates/task-impls/src/request.rs b/crates/task-impls/src/request.rs index 567a296d96..3a079436da 100644 --- a/crates/task-impls/src/request.rs +++ b/crates/task-impls/src/request.rs @@ -14,7 +14,6 @@ use std::{ }; use async_broadcast::{Receiver, Sender}; -use async_lock::RwLock; use async_trait::async_trait; use hotshot_task::{ dependency::{Dependency, EventDependency}, @@ -26,7 +25,6 @@ use hotshot_types::{ simple_vote::HasEpoch, traits::{ block_contents::BlockHeader, - election::Membership, network::{ConnectedNetwork, DataRequest, RequestKind}, node_implementation::{NodeImplementation, NodeType}, signature_key::SignatureKey, @@ -115,14 +113,20 @@ impl> TaskState for NetworkRequest HotShotEvent::QuorumProposalValidated(proposal, _) => { let prop_view = proposal.data.view_number(); let prop_epoch = proposal.data.epoch(); - let next_epoch = prop_epoch.map(|epoch| epoch + 1); // Request VID share only if: // 1. we are part of the current epoch or // 2. we are part of the next epoch and this is a proposal for the last block. - let membership_reader = self.membership.read().await; - if !membership_reader.has_stake(&self.public_key, prop_epoch) - && (!membership_reader.has_stake(&self.public_key, next_epoch) + let membership_reader = self + .membership_coordinator + .membership_for_epoch(prop_epoch) + .await; + if !membership_reader.has_stake(&self.public_key).await + && (!membership_reader + .next_epoch() + .await + .has_stake(&self.public_key) + .await || !is_last_block_in_epoch( proposal.data.block_header().block_number(), self.epoch_height, @@ -130,7 +134,6 @@ impl> TaskState for NetworkRequest { return Ok(()); } - drop(membership_reader); let consensus_reader = self.consensus.read().await; let maybe_vid_share = consensus_reader @@ -214,15 +217,19 @@ impl> NetworkRequestState = membership_reader - .da_committee_members(view, epoch) + .da_committee_members(view) + .await .into_iter() .collect(); drop(membership_reader); diff --git a/crates/task-impls/src/response.rs b/crates/task-impls/src/response.rs index cbc0fa1084..5480bcf987 100644 --- a/crates/task-impls/src/response.rs +++ b/crates/task-impls/src/response.rs @@ -7,7 +7,6 @@ use std::{sync::Arc, time::Duration}; use async_broadcast::{Receiver, Sender}; -use async_lock::RwLock; use committable::Committable; use hotshot_types::{ consensus::{Consensus, LockedConsensusState, OuterConsensus}, @@ -15,7 +14,6 @@ use hotshot_types::{ epoch_membership::EpochMembershipCoordinator, message::{Proposal, UpgradeLock}, traits::{ - election::Membership, network::DataRequest, node_implementation::{NodeType, Versions}, signature_key::SignatureKey, @@ -176,7 +174,7 @@ impl NetworkResponseState { OuterConsensus::new(Arc::clone(&self.consensus)), view, target_epoch, - Arc::clone(&self.membership), + self.membership.clone(), &self.private_key, &self.upgrade_lock, ) @@ -211,7 +209,11 @@ impl NetworkResponseState { sender: &TYPES::SignatureKey, epoch: Option, ) -> bool { - self.membership.read().await.has_stake(sender, epoch) + self.membership + .membership_for_epoch(epoch) + .await + .has_stake(sender) + .await } } diff --git a/crates/task-impls/src/transactions.rs b/crates/task-impls/src/transactions.rs index e7f40691a8..78567ab08f 100644 --- a/crates/task-impls/src/transactions.rs +++ b/crates/task-impls/src/transactions.rs @@ -10,7 +10,6 @@ use std::{ }; use async_broadcast::{Receiver, Sender}; -use async_lock::RwLock; use async_trait::async_trait; use futures::{future::join_all, stream::FuturesUnordered, StreamExt}; use hotshot_builder_api::v0_1::block_info::AvailableBlockInfo; @@ -18,12 +17,12 @@ use hotshot_task::task::TaskState; use hotshot_types::{ consensus::OuterConsensus, data::{null_block, PackedBundle}, + epoch_membership::EpochMembershipCoordinator, event::{Event, EventType}, message::UpgradeLock, traits::{ auction_results_provider::AuctionResultsProvider, block_contents::{BuilderFee, EncodeBytes}, - election::Membership, node_implementation::{ConsensusTime, HasUrls, NodeImplementation, NodeType, Versions}, signature_key::{BuilderSignatureKey, SignatureKey}, BlockPayload, @@ -212,7 +211,12 @@ impl, V: Versions> TransactionTask .number_of_empty_blocks_proposed .add(1); - let membership_total_nodes = self.membership.read().await.total_nodes(self.cur_epoch); + let membership_total_nodes = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await + .total_nodes() + .await; let Some(null_fee) = null_block::builder_fee::(membership_total_nodes, version, *block_view) else { @@ -355,7 +359,12 @@ impl, V: Versions> TransactionTask block_epoch: Option, version: Version, ) -> Option> { - let membership_total_nodes = self.membership.read().await.total_nodes(self.cur_epoch); + let membership_total_nodes = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await + .total_nodes() + .await; let Some(null_fee) = null_block::builder_fee::(membership_total_nodes, version, *block_view) else { @@ -488,7 +497,12 @@ impl, V: Versions> TransactionTask self.cur_view = view; self.cur_epoch = epoch; - let leader = self.membership.read().await.leader(view, epoch)?; + let leader = self + .membership_coordinator + .membership_for_epoch(epoch) + .await + .leader(view) + .await?; if leader == self.public_key { self.handle_view_change(&event_stream, view, epoch).await; return Ok(()); diff --git a/crates/task-impls/src/upgrade.rs b/crates/task-impls/src/upgrade.rs index ffd1b012bb..f82f56b7e0 100644 --- a/crates/task-impls/src/upgrade.rs +++ b/crates/task-impls/src/upgrade.rs @@ -7,18 +7,17 @@ use std::{marker::PhantomData, sync::Arc, time::SystemTime}; use async_broadcast::{Receiver, Sender}; -use async_lock::RwLock; use async_trait::async_trait; use committable::Committable; use hotshot_task::task::TaskState; use hotshot_types::{ data::UpgradeProposal, + epoch_membership::EpochMembershipCoordinator, event::{Event, EventType}, message::{Proposal, UpgradeLock}, simple_certificate::UpgradeCertificate, simple_vote::{UpgradeProposalData, UpgradeVote}, traits::{ - election::Membership, node_implementation::{ConsensusTime, NodeType, Versions}, signature_key::SignatureKey, }, @@ -177,7 +176,12 @@ impl UpgradeTaskState { ); // We then validate that the proposal was issued by the leader for the view. - let view_leader_key = self.membership.read().await.leader(view, self.cur_epoch)?; + let view_leader_key = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await + .leader(view) + .await?; ensure!( view_leader_key == *sender, info!( @@ -218,25 +222,25 @@ impl UpgradeTaskState { tracing::debug!("Upgrade vote recv, Main Task {:?}", vote.view_number()); // Check if we are the leader. - { - let view = vote.view_number(); - let membership_reader = self.membership.read().await; - ensure!( - membership_reader.leader(view, self.cur_epoch)? == self.public_key, - debug!( - "We are not the leader for view {} are we leader for next view? {}", - *view, - membership_reader.leader(view + 1, self.cur_epoch)? == self.public_key - ) - ); - } + let view = vote.view_number(); + let mem = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await; + ensure!( + mem.leader(view).await? == self.public_key, + debug!( + "We are not the leader for view {} are we leader for next view? {}", + *view, + mem.leader(view + 1).await? == self.public_key + ) + ); handle_vote( &mut self.vote_collectors, vote, self.public_key.clone(), - &self.membership, - self.cur_epoch, + &mem, self.id, &event, &tx, @@ -262,10 +266,14 @@ impl UpgradeTaskState { ))? .as_secs(); - let leader = self.membership.read().await.leader( - TYPES::View::new(view + TYPES::UPGRADE_CONSTANTS.propose_offset), - self.cur_epoch, - )?; + let leader = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await + .leader(TYPES::View::new( + view + TYPES::UPGRADE_CONSTANTS.propose_offset, + )) + .await?; // We try to form a certificate 5 views before we're leader. if view >= self.start_proposing_view diff --git a/crates/task-impls/src/vid.rs b/crates/task-impls/src/vid.rs index 2b6669d710..95c378eaec 100644 --- a/crates/task-impls/src/vid.rs +++ b/crates/task-impls/src/vid.rs @@ -7,17 +7,16 @@ use std::{marker::PhantomData, sync::Arc}; use async_broadcast::{Receiver, Sender}; -use async_lock::RwLock; use async_trait::async_trait; use hotshot_task::task::TaskState; use hotshot_types::{ consensus::OuterConsensus, data::{PackedBundle, VidDisperse, VidDisperseShare}, + epoch_membership::EpochMembershipCoordinator, message::{Proposal, UpgradeLock}, simple_vote::HasEpoch, traits::{ block_contents::BlockHeader, - election::Membership, node_implementation::{NodeImplementation, NodeType, Versions}, signature_key::SignatureKey, BlockPayload, @@ -88,10 +87,11 @@ impl, V: Versions> VidTaskState, V: Versions> VidTaskState( &payload, - &Arc::clone(&self.membership), + &self.membership_coordinator, *view_number, epoch, epoch, @@ -208,7 +208,7 @@ impl, V: Versions> VidTaskState( payload.as_ref(), - &Arc::clone(&self.membership), + &self.membership_coordinator, proposal_view_number, target_epoch, sender_epoch, diff --git a/crates/task-impls/src/view_sync.rs b/crates/task-impls/src/view_sync.rs index fd91bf831a..c680b23234 100644 --- a/crates/task-impls/src/view_sync.rs +++ b/crates/task-impls/src/view_sync.rs @@ -16,6 +16,7 @@ use async_lock::RwLock; use async_trait::async_trait; use hotshot_task::task::TaskState; use hotshot_types::{ + epoch_membership::{EpochMembership, EpochMembershipCoordinator}, message::UpgradeLock, simple_certificate::{ ViewSyncCommitCertificate2, ViewSyncFinalizeCertificate2, ViewSyncPreCommitCertificate2, @@ -25,7 +26,6 @@ use hotshot_types::{ ViewSyncPreCommitData2, ViewSyncPreCommitVote2, }, traits::{ - election::Membership, node_implementation::{ConsensusTime, NodeType, Versions}, signature_key::SignatureKey, }, @@ -142,9 +142,6 @@ pub struct ViewSyncReplicaTaskState { /// Round HotShot wishes to be in pub next_view: TYPES::View, - /// Current epoch HotShot is in - pub cur_epoch: Option, - /// The relay index we are currently on pub relay: u64, @@ -161,7 +158,7 @@ pub struct ViewSyncReplicaTaskState { pub id: u64, /// Membership for the quorum - pub membership_coordinator: EpochMembershipCoordinator, + pub membership: EpochMembership, /// This Nodes Public Key pub public_key: TYPES::SignatureKey, @@ -230,12 +227,14 @@ impl ViewSyncTaskState { let mut replica_state: ViewSyncReplicaTaskState = ViewSyncReplicaTaskState { cur_view: view, next_view: view, - cur_epoch: self.cur_epoch, relay: 0, finalized: false, sent_view_change_event: false, timeout_task: None, - membership: Arc::clone(&self.membership), + membership: self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await, public_key: self.public_key.clone(), private_key: self.private_key.clone(), view_sync_timeout: self.view_sync_timeout, @@ -309,22 +308,21 @@ impl ViewSyncTaskState { return Ok(()); } + let epoch_mem = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await; // We do not have a relay task already running, so start one ensure!( - self.membership - .read() - .await - .leader(vote_view + relay, self.cur_epoch)? - == self.public_key, + epoch_mem.leader(vote_view + relay).await? == self.public_key, "View sync vote sent to wrong leader" ); let info = AccumulatorInfo { public_key: self.public_key.clone(), - membership: Arc::clone(&self.membership), + membership: epoch_mem, view: vote_view, id: self.id, - epoch: vote.data.epoch, }; let vote_collector = create_vote_accumulator( &info, @@ -359,21 +357,20 @@ impl ViewSyncTaskState { } // We do not have a relay task already running, so start one + let epoch_mem = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await; ensure!( - self.membership - .read() - .await - .leader(vote_view + relay, self.cur_epoch)? - == self.public_key, + epoch_mem.leader(vote_view + relay).await? == self.public_key, debug!("View sync vote sent to wrong leader") ); let info = AccumulatorInfo { public_key: self.public_key.clone(), - membership: Arc::clone(&self.membership), + membership: epoch_mem, view: vote_view, id: self.id, - epoch: vote.data.epoch, }; let vote_collector = create_vote_accumulator( @@ -407,22 +404,21 @@ impl ViewSyncTaskState { return Ok(()); } + let epoch_mem = self + .membership_coordinator + .membership_for_epoch(self.cur_epoch) + .await; // We do not have a relay task already running, so start one ensure!( - self.membership - .read() - .await - .leader(vote_view + relay, self.cur_epoch)? - == self.public_key, + epoch_mem.leader(vote_view + relay).await? == self.public_key, debug!("View sync vote sent to wrong leader") ); let info = AccumulatorInfo { public_key: self.public_key.clone(), - membership: Arc::clone(&self.membership), + membership: epoch_mem, view: vote_view, id: self.id, - epoch: vote.data.epoch, }; let vote_collector = create_vote_accumulator( &info, @@ -487,10 +483,11 @@ impl ViewSyncTaskState { self.num_timeouts_tracked += 1; let leader = self - .membership - .read() + .membership_coordinator + .membership_for_epoch(self.cur_epoch) .await - .leader(view_number, self.cur_epoch)?; + .leader(view_number) + .await?; tracing::warn!( %leader, leader_mnemonic = hotshot_types::utils::mnemonic(&leader), @@ -530,7 +527,7 @@ impl ViewSyncTaskState { } impl ViewSyncReplicaTaskState { - #[instrument(skip_all, fields(id = self.id, view = *self.cur_view, epoch = self.cur_epoch.map(|x| *x)), name = "View Sync Replica Task", level = "error")] + #[instrument(skip_all, fields(id = self.id, view = *self.cur_view, epoch = self.membership.epoch().map(|x| *x)), name = "View Sync Replica Task", level = "error")] /// Handle incoming events for the view sync replica task pub async fn handle( &mut self, @@ -548,11 +545,8 @@ impl ViewSyncReplicaTaskState { return None; } - let membership_reader = self.membership.read().await; - let membership_stake_table = membership_reader.stake_table(self.cur_epoch); - let membership_failure_threshold = - membership_reader.failure_threshold(self.cur_epoch); - drop(membership_reader); + let membership_stake_table = self.membership.stake_table().await; + let membership_failure_threshold = self.membership.failure_threshold().await; // If certificate is not valid, return current state if let Err(e) = certificate @@ -642,11 +636,8 @@ impl ViewSyncReplicaTaskState { return None; } - let membership_reader = self.membership.read().await; - let membership_stake_table = membership_reader.stake_table(self.cur_epoch); - let membership_success_threshold = - membership_reader.success_threshold(self.cur_epoch); - drop(membership_reader); + let membership_stake_table = self.membership.stake_table().await; + let membership_success_threshold = self.membership.success_threshold().await; // If certificate is not valid, return current state if let Err(e) = certificate @@ -706,7 +697,10 @@ impl ViewSyncReplicaTaskState { // TODO: Figure out the correct way to view sync across epochs if needed broadcast_event( - Arc::new(HotShotEvent::ViewChange(self.next_view, self.cur_epoch)), + Arc::new(HotShotEvent::ViewChange( + self.next_view, + self.membership.epoch(), + )), &event_stream, ) .await; @@ -747,11 +741,8 @@ impl ViewSyncReplicaTaskState { return None; } - let membership_reader = self.membership.read().await; - let membership_stake_table = membership_reader.stake_table(self.cur_epoch); - let membership_success_threshold = - membership_reader.success_threshold(self.cur_epoch); - drop(membership_reader); + let membership_stake_table = self.membership.stake_table().await; + let membership_success_threshold = self.membership.success_threshold().await; // If certificate is not valid, return current state if let Err(e) = certificate @@ -787,7 +778,10 @@ impl ViewSyncReplicaTaskState { // TODO: Figure out the correct way to view sync across epochs if needed broadcast_event( - Arc::new(HotShotEvent::ViewChange(self.next_view, self.cur_epoch)), + Arc::new(HotShotEvent::ViewChange( + self.next_view, + self.membership.epoch(), + )), &event_stream, ) .await; @@ -801,7 +795,7 @@ impl ViewSyncReplicaTaskState { return None; } - let epoch = self.cur_epoch; + let epoch = self.membership.epoch(); let Ok(vote) = ViewSyncPreCommitVote2::::create_signed_vote( ViewSyncPreCommitData2 { relay: 0, @@ -862,7 +856,7 @@ impl ViewSyncReplicaTaskState { ViewSyncPreCommitData2 { relay: self.relay, round: self.next_view, - epoch: self.cur_epoch, + epoch: self.membership.epoch(), }, self.next_view, &self.public_key, diff --git a/crates/task-impls/src/vote_collection.rs b/crates/task-impls/src/vote_collection.rs index 7932d53651..ddd63a126b 100644 --- a/crates/task-impls/src/vote_collection.rs +++ b/crates/task-impls/src/vote_collection.rs @@ -13,11 +13,10 @@ use std::{ }; use async_broadcast::Sender; -use async_lock::RwLock; use async_trait::async_trait; use either::Either::{Left, Right}; use hotshot_types::{ - epoch_membership::{EpochMembership, EpochMembershipCoordinator}, + epoch_membership::EpochMembership, message::UpgradeLock, simple_certificate::{ DaCertificate2, NextEpochQuorumCertificate2, QuorumCertificate, QuorumCertificate2, @@ -102,7 +101,6 @@ impl< pub async fn accumulate_vote( &mut self, vote: &VOTE, - sender_epoch: Option, event_stream: &Sender>>, ) -> Result> { // TODO create this only once @@ -127,7 +125,7 @@ impl< "No accumulator to handle vote with. This shouldn't happen." ))?; - match accumulator.accumulate(vote, self.membership).await { + match accumulator.accumulate(vote, self.membership.clone()).await { None => Ok(None), Some(cert) => { tracing::debug!("Certificate Formed! {:?}", cert); @@ -251,7 +249,6 @@ pub async fn handle_vote< vote: &VOTE, public_key: TYPES::SignatureKey, membership: &EpochMembership, - epoch: Option, id: u64, event: &Arc>, event_stream: &Sender>>, @@ -268,7 +265,6 @@ where public_key, membership: membership.clone(), view: vote.view_number(), - epoch, id, }; let collector = create_vote_accumulator( @@ -490,9 +486,7 @@ impl sender: &Sender>>, ) -> Result>> { match event.as_ref() { - HotShotEvent::QuorumVoteRecv(vote) => { - self.accumulate_vote(vote, self.epoch, sender).await - } + HotShotEvent::QuorumVoteRecv(vote) => self.accumulate_vote(vote, sender).await, _ => Ok(None), } } @@ -515,12 +509,8 @@ impl match event.as_ref() { HotShotEvent::QuorumVoteRecv(vote) => { // #3967 REVIEW NOTE: Should we error if self.epoch is None? - let next_epoch = self - .epoch - .map(|x| x + 1) - .ok_or_else(|| error!("epoch should not be none in handle_vote_event"))?; - self.accumulate_vote(&vote.clone().into(), Some(next_epoch), sender) - .await + self.membership = self.membership.next_epoch().await; + self.accumulate_vote(&vote.clone().into(), sender).await } _ => Ok(None), } @@ -542,9 +532,7 @@ impl sender: &Sender>>, ) -> Result>> { match event.as_ref() { - HotShotEvent::UpgradeVoteRecv(vote) => { - self.accumulate_vote(vote, self.epoch, sender).await - } + HotShotEvent::UpgradeVoteRecv(vote) => self.accumulate_vote(vote, sender).await, _ => Ok(None), } } @@ -563,7 +551,7 @@ impl HandleVoteEvent, DaCert sender: &Sender>>, ) -> Result>> { match event.as_ref() { - HotShotEvent::DaVoteRecv(vote) => self.accumulate_vote(vote, self.epoch, sender).await, + HotShotEvent::DaVoteRecv(vote) => self.accumulate_vote(vote, sender).await, _ => Ok(None), } } @@ -583,9 +571,7 @@ impl sender: &Sender>>, ) -> Result>> { match event.as_ref() { - HotShotEvent::TimeoutVoteRecv(vote) => { - self.accumulate_vote(vote, self.epoch, sender).await - } + HotShotEvent::TimeoutVoteRecv(vote) => self.accumulate_vote(vote, sender).await, _ => Ok(None), } } @@ -606,7 +592,7 @@ impl ) -> Result>> { match event.as_ref() { HotShotEvent::ViewSyncPreCommitVoteRecv(vote) => { - self.accumulate_vote(vote, self.epoch, sender).await + self.accumulate_vote(vote, sender).await } _ => Ok(None), } @@ -627,9 +613,7 @@ impl sender: &Sender>>, ) -> Result>> { match event.as_ref() { - HotShotEvent::ViewSyncCommitVoteRecv(vote) => { - self.accumulate_vote(vote, self.epoch, sender).await - } + HotShotEvent::ViewSyncCommitVoteRecv(vote) => self.accumulate_vote(vote, sender).await, _ => Ok(None), } } @@ -650,7 +634,7 @@ impl ) -> Result>> { match event.as_ref() { HotShotEvent::ViewSyncFinalizeVoteRecv(vote) => { - self.accumulate_vote(vote, self.epoch, sender).await + self.accumulate_vote(vote, sender).await } _ => Ok(None), } diff --git a/crates/testing/src/byzantine/byzantine_behaviour.rs b/crates/testing/src/byzantine/byzantine_behaviour.rs index dcc18b8ad0..7816b3e354 100644 --- a/crates/testing/src/byzantine/byzantine_behaviour.rs +++ b/crates/testing/src/byzantine/byzantine_behaviour.rs @@ -339,13 +339,12 @@ impl + std::fmt::Debug, V: Version &self, handle: &mut SystemContextHandle, network: Arc<>::Network>, - membership_coordinator: EpochMembershipCoordinator, ) { let network_state: NetworkEventTaskState<_, V, _, _> = NetworkEventTaskState { network, view: TYPES::View::genesis(), epoch: None, - membership, + membership_coordinator: handle.membership_coordinator.clone(), storage: Arc::clone(&handle.storage()), consensus: OuterConsensus::new(handle.consensus()), upgrade_lock: handle.hotshot.upgrade_lock.clone(), diff --git a/crates/testing/src/helpers.rs b/crates/testing/src/helpers.rs index c17c4c8834..3168fb80b6 100644 --- a/crates/testing/src/helpers.rs +++ b/crates/testing/src/helpers.rs @@ -27,7 +27,7 @@ use hotshot_task_impls::events::HotShotEvent; use hotshot_types::{ consensus::ConsensusMetricsValue, data::{Leaf2, VidDisperse, VidDisperseShare}, - epoch_membership::EpochMembership, + epoch_membership::{EpochMembership, EpochMembershipCoordinator}, message::{Proposal, UpgradeLock}, simple_certificate::DaCertificate2, simple_vote::{DaData2, DaVote2, SimpleVote, VersionedVoteData}, @@ -119,12 +119,14 @@ pub async fn build_system_handle_from_launcher< hotshot_config.known_da_nodes.clone(), ))); + let coordinator = EpochMembershipCoordinator::new(memberships, hotshot_config.epoch_height); + SystemContext::init( public_key, private_key, node_id, hotshot_config, - memberships, + coordinator, network, initializer, ConsensusMetricsValue::default(), @@ -148,16 +150,14 @@ pub async fn build_cert< data: DATAType, epoch_membership: &EpochMembership, view: TYPES::View, - epoch: Option, public_key: &TYPES::SignatureKey, private_key: &::PrivateKey, upgrade_lock: &UpgradeLock, ) -> CERT { let real_qc_sig = build_assembled_sig::( &data, - membership, + epoch_membership, view, - epoch, upgrade_lock, ) .await; @@ -212,16 +212,15 @@ pub async fn build_assembled_sig< DATAType: Committable + Clone + Eq + Hash + Serialize + Debug + 'static, >( data: &DATAType, - epoch_membership: &Arc>, + epoch_membership: &EpochMembership, view: TYPES::View, - epoch: Option, upgrade_lock: &UpgradeLock, ) -> ::QcType { - let stake_table = CERT::stake_table(&epoch_membership).await; + let stake_table = CERT::stake_table(epoch_membership).await; let real_qc_pp: ::QcParams = ::public_parameter( stake_table.clone(), - U256::from(CERT::threshold(&epoch_membership).await), + U256::from(CERT::threshold(epoch_membership).await), ); let total_nodes = stake_table.len(); @@ -273,29 +272,21 @@ pub fn key_pair_for_id( /// TODO(Chengyu): use this version information #[must_use] pub async fn vid_scheme_from_view_number( - membership: &Arc>, + membership: &EpochMembership, view_number: TYPES::View, - epoch_number: Option, _version: Version, ) -> VidSchemeType { - let num_storage_nodes = membership - .read() - .await - .committee_members(view_number, epoch_number) - .len(); + let num_storage_nodes = membership.committee_members(view_number).await.len(); advz_scheme(num_storage_nodes) } pub async fn vid_payload_commitment( - membership: &Arc::Membership>>, + membership: &EpochMembership, view_number: TYPES::View, - epoch_number: Option, transactions: Vec, version: Version, ) -> VidCommitment { - let mut vid = - vid_scheme_from_view_number::(membership, view_number, epoch_number, version) - .await; + let mut vid = vid_scheme_from_view_number::(membership, view_number, version).await; let encoded_transactions = TestTransaction::encode(&transactions); let vid_disperse = vid.disperse(&encoded_transactions).unwrap(); @@ -303,51 +294,47 @@ pub async fn vid_payload_commitment( } pub async fn da_payload_commitment( - membership: &Arc::Membership>>, + membership: &EpochMembership, transactions: Vec, - epoch_number: Option, version: Version, ) -> VidCommitment { let encoded_transactions = TestTransaction::encode(&transactions); vid_commitment::( &encoded_transactions, - membership.read().await.total_nodes(epoch_number), + membership.total_nodes().await, version, ) } pub async fn build_payload_commitment( - membership: &Arc::Membership>>, + membership: &EpochMembership, view: TYPES::View, - epoch: Option, version: Version, ) -> ::Commit { // Make some empty encoded transactions, we just care about having a commitment handy for the // later calls. We need the VID commitment to be able to propose later. - let mut vid = vid_scheme_from_view_number::(membership, view, epoch, version).await; + let mut vid = vid_scheme_from_view_number::(membership, view, version).await; let encoded_transactions = Vec::new(); vid.commit_only(&encoded_transactions).unwrap() } /// TODO: pub async fn build_vid_proposal( - membership: &Arc::Membership>>, + membership: &EpochMembership, view_number: TYPES::View, epoch_number: Option, transactions: Vec, private_key: &::PrivateKey, version: Version, ) -> VidProposal { - let mut vid = - vid_scheme_from_view_number::(membership, view_number, epoch_number, version) - .await; + let mut vid = vid_scheme_from_view_number::(membership, view_number, version).await; let encoded_transactions = TestTransaction::encode(&transactions); let vid_disperse = VidDisperse::from_membership( view_number, vid.disperse(&encoded_transactions).unwrap(), - membership, + &membership.coordinator, epoch_number, epoch_number, None, @@ -378,7 +365,7 @@ pub async fn build_vid_proposal( #[allow(clippy::too_many_arguments)] pub async fn build_da_certificate( - membership: &Arc::Membership>>, + membership: &EpochMembership, view_number: TYPES::View, epoch_number: Option, transactions: Vec, @@ -390,7 +377,7 @@ pub async fn build_da_certificate( let da_payload_commitment = vid_commitment::( &encoded_transactions, - membership.read().await.total_nodes(epoch_number), + membership.total_nodes().await, upgrade_lock.version_infallible(view_number).await, ); @@ -403,7 +390,6 @@ pub async fn build_da_certificate( da_data, membership, view_number, - epoch_number, public_key, private_key, upgrade_lock, diff --git a/crates/testing/src/overall_safety_task.rs b/crates/testing/src/overall_safety_task.rs index a872d381cf..5ea4691755 100644 --- a/crates/testing/src/overall_safety_task.rs +++ b/crates/testing/src/overall_safety_task.rs @@ -16,12 +16,12 @@ use async_trait::async_trait; use hotshot::{traits::TestableNodeImplementation, HotShotError}; use hotshot_types::{ data::Leaf2, + epoch_membership::EpochMembershipCoordinator, error::RoundTimedoutState, event::{Event, EventType, LeafChain}, simple_certificate::QuorumCertificate2, traits::{ block_contents::BlockHeader, - election::Membership, node_implementation::{ConsensusTime, NodeType, Versions}, BlockPayload, }, @@ -141,16 +141,15 @@ impl, V: Versions> TestTas /// Handles an event from one of multiple receivers. async fn handle_event(&mut self, (message, id): (Self::Event, usize)) -> Result<()> { - let memberships_arc = Arc::clone( - &self - .handles - .read() - .await - .first() - .unwrap() - .handle - .membership_coordinator, - ); + let memberships_arc = &self + .handles + .read() + .await + .first() + .unwrap() + .handle + .membership_coordinator + .clone(); let public_key = self.handles.read().await[id].handle.public_key(); let OverallSafetyPropertiesDescription:: { check_leaf, @@ -170,9 +169,10 @@ impl, V: Versions> TestTas .await .cur_epoch(); if !memberships_arc - .read() + .membership_for_epoch(cur_epoch) + .await + .has_stake(&public_key) .await - .has_stake(&public_key, cur_epoch) { // Return early, this event comes from a node not belonging to the current epoch return Ok(()); @@ -217,7 +217,7 @@ impl, V: Versions> TestTas id, paired_up, maybe_block_size, - &memberships_arc, + memberships_arc, &public_key, self.epoch_height, ) @@ -231,7 +231,7 @@ impl, V: Versions> TestTas id, paired_up, maybe_block_size, - &memberships_arc, + memberships_arc, &public_key, self.epoch_height, ) @@ -255,9 +255,10 @@ impl, V: Versions> TestTas .await .cur_epoch(); if !memberships_arc - .read() + .membership_for_epoch(cur_epoch) + .await + .has_stake(&public_key) .await - .has_stake(&public_key, cur_epoch) { // Return early, this event comes from a node not belonging to the current epoch return Ok(()); @@ -275,10 +276,9 @@ impl, V: Versions> TestTas if let Some(keys) = keys { for key in keys { let key_epoch = key.epoch(self.epoch_height); - let memberships_reader = memberships_arc.read().await; - let key_len = memberships_reader.total_nodes(key_epoch); - let key_threshold = memberships_reader.success_threshold(key_epoch).get() as usize; - drop(memberships_reader); + let epoch_membership = memberships_arc.membership_for_epoch(key_epoch).await; + let key_len = epoch_membership.total_nodes().await; + let key_threshold = epoch_membership.success_threshold().await.get() as usize; let key_view_number = key.view_number(); let key_view = self.ctx.round_results.get_mut(&key_view_number).unwrap(); @@ -322,10 +322,9 @@ impl, V: Versions> TestTas .await .cur_epoch(); - let memberships_reader = memberships_arc.read().await; - let len = memberships_reader.total_nodes(cur_epoch); - let threshold = memberships_reader.success_threshold(cur_epoch).get() as usize; - drop(memberships_reader); + let epoch_membership = memberships_arc.membership_for_epoch(cur_epoch).await; + let len = epoch_membership.total_nodes().await; + let threshold = epoch_membership.success_threshold().await.get() as usize; if view.check_if_failed(threshold, len) { view.status = ViewStatus::Failed; @@ -509,7 +508,7 @@ impl RoundResult { idx: usize, result: (LeafChain, QuorumCertificate2), maybe_block_size: Option, - membership: &Arc>, + membership: &EpochMembershipCoordinator, public_key: &TYPES::SignatureKey, epoch_height: u64, ) -> Option> { @@ -517,7 +516,12 @@ impl RoundResult { if let Some(leaf_info) = maybe_leaf { let leaf = &leaf_info.leaf; let epoch = leaf.epoch(epoch_height); - if !membership.read().await.has_stake(public_key, epoch) { + if !membership + .membership_for_epoch(epoch) + .await + .has_stake(public_key) + .await + { // The node doesn't belong to the epoch, don't count towards total successes count return None; } diff --git a/crates/testing/src/spinning_task.rs b/crates/testing/src/spinning_task.rs index d22bfe5dbf..a757a46ea5 100644 --- a/crates/testing/src/spinning_task.rs +++ b/crates/testing/src/spinning_task.rs @@ -235,7 +235,7 @@ where }; let storage = node.handle.storage().clone(); - let memberships = Arc::clone(&node.handle.membership_coordinator); + let memberships = node.handle.membership_coordinator.clone(); let config = node.handle.hotshot.config.clone(); let marketplace_config = node.handle.hotshot.marketplace_config.clone(); @@ -287,7 +287,7 @@ where TestRunner::::add_node_with_config_and_channels( node_id, generated_network.clone(), - memberships, + Arc::clone(memberships.membership()), initializer, config, validator_config, diff --git a/crates/testing/src/test_builder.rs b/crates/testing/src/test_builder.rs index b2182e4d34..0515bc67bb 100644 --- a/crates/testing/src/test_builder.rs +++ b/crates/testing/src/test_builder.rs @@ -20,6 +20,7 @@ use hotshot_example_types::{ }; use hotshot_types::{ consensus::ConsensusMetricsValue, + epoch_membership::EpochMembershipCoordinator, traits::node_implementation::{NodeType, Versions}, HotShotConfig, PeerConfig, ValidatorConfig, }; @@ -250,6 +251,7 @@ pub async fn create_test_handle< // Get key pair for certificate aggregation let private_key = validator_config.private_key.clone(); let public_key = validator_config.public_key.clone(); + let membership_coordinator = EpochMembershipCoordinator::new(memberships, config.epoch_height); let behaviour = (metadata.behaviour)(node_id); match behaviour { @@ -261,7 +263,7 @@ pub async fn create_test_handle< private_key, node_id, config, - memberships, + membership_coordinator, network, initializer, ConsensusMetricsValue::default(), @@ -280,7 +282,7 @@ pub async fn create_test_handle< private_key, node_id, config, - memberships, + membership_coordinator, network, initializer, ConsensusMetricsValue::default(), @@ -295,7 +297,7 @@ pub async fn create_test_handle< private_key, node_id, config, - memberships, + membership_coordinator, network, initializer, ConsensusMetricsValue::default(), diff --git a/crates/testing/src/test_runner.rs b/crates/testing/src/test_runner.rs index 618ffa49e9..56d78637c9 100644 --- a/crates/testing/src/test_runner.rs +++ b/crates/testing/src/test_runner.rs @@ -31,6 +31,7 @@ use hotshot_types::{ consensus::ConsensusMetricsValue, constants::EVENT_CHANNEL_SIZE, data::Leaf2, + epoch_membership::EpochMembershipCoordinator, simple_certificate::QuorumCertificate2, traits::{ election::Membership, @@ -612,13 +613,14 @@ where // Get key pair for certificate aggregation let private_key = validator_config.private_key.clone(); let public_key = validator_config.public_key.clone(); + let epoch_height = config.epoch_height; SystemContext::new( public_key, private_key, node_id, config, - Arc::new(RwLock::new(memberships)), + EpochMembershipCoordinator::new(Arc::new(RwLock::new(memberships)), epoch_height), network, initializer, ConsensusMetricsValue::default(), @@ -650,13 +652,14 @@ where // Get key pair for certificate aggregation let private_key = validator_config.private_key.clone(); let public_key = validator_config.public_key.clone(); + let epoch_height = config.epoch_height; SystemContext::new_from_channels( public_key, private_key, node_id, config, - memberships, + EpochMembershipCoordinator::new(memberships, epoch_height), network, initializer, ConsensusMetricsValue::default(), diff --git a/crates/testing/src/view_generator.rs b/crates/testing/src/view_generator.rs index 41c894bb03..f9fd5d1ee1 100644 --- a/crates/testing/src/view_generator.rs +++ b/crates/testing/src/view_generator.rs @@ -12,7 +12,6 @@ use std::{ task::{Context, Poll}, }; -use async_lock::RwLock; use committable::Committable; use futures::{FutureExt, Stream}; use hotshot::types::{BLSPubKey, SignatureKey, SystemContextHandle}; @@ -57,7 +56,7 @@ pub struct TestView { pub leaf: Leaf2, pub view_number: ViewNumber, pub epoch_number: Option, - pub membership: EpochMembershipCoordinator, + pub membership: EpochMembershipCoordinator, pub vid_disperse: Proposal>, pub vid_proposal: ( Vec>>, @@ -74,9 +73,7 @@ pub struct TestView { } impl TestView { - pub async fn genesis( - membership: &EpochMembershipCoordinator<::Membership>, - ) -> Self { + pub async fn genesis(membership: &EpochMembershipCoordinator) -> Self { let genesis_view = ViewNumber::new(1); let genesis_epoch = genesis_epoch_from_version::(); let upgrade_lock = UpgradeLock::new(); @@ -102,17 +99,17 @@ impl TestView { let leader_public_key = public_key; let genesis_version = upgrade_lock.version_infallible(genesis_view).await; + let epoch_membership = membership.membership_for_epoch(genesis_epoch).await; let payload_commitment = da_payload_commitment::( - membership, + &epoch_membership, transactions.clone(), - genesis_epoch, genesis_version, ) .await; let (vid_disperse, vid_proposal) = build_vid_proposal::( - membership, + &epoch_membership, genesis_view, genesis_epoch, transactions.clone(), @@ -122,7 +119,7 @@ impl TestView { .await; let da_certificate = build_da_certificate( - membership, + &epoch_membership, genesis_view, genesis_epoch, transactions.clone(), @@ -227,8 +224,6 @@ impl TestView { // test view here. let next_view = max(old_view, self.view_number) + 1; - let membership = &self.membership; - let transactions = &self.transactions; let quorum_data = QuorumData2 { @@ -256,16 +251,19 @@ impl TestView { ); let version = self.upgrade_lock.version_infallible(next_view).await; + let membership = self + .membership + .membership_for_epoch(self.epoch_number) + .await; let payload_commitment = da_payload_commitment::( - membership, + &membership, transactions.clone(), - self.epoch_number, version, ) .await; let (vid_disperse, vid_proposal) = build_vid_proposal::( - membership, + &membership, next_view, self.epoch_number, transactions.clone(), @@ -275,7 +273,7 @@ impl TestView { .await; let da_certificate = build_da_certificate::( - membership, + &membership, next_view, self.epoch_number, transactions.clone(), @@ -293,9 +291,8 @@ impl TestView { QuorumCertificate2, >( quorum_data, - membership, + &membership, old_view, - self.epoch_number, &old_public_key, &old_private_key, &self.upgrade_lock, @@ -311,9 +308,8 @@ impl TestView { UpgradeCertificate, >( data.clone(), - membership, + &membership, next_view, - self.epoch_number, &public_key, &private_key, &self.upgrade_lock, @@ -334,9 +330,8 @@ impl TestView { ViewSyncFinalizeCertificate2, >( data.clone(), - membership, + &membership, next_view, - self.epoch_number, &public_key, &private_key, &self.upgrade_lock, @@ -357,9 +352,8 @@ impl TestView { TimeoutCertificate2, >( data.clone(), - membership, + &membership, next_view, - self.epoch_number, &public_key, &private_key, &self.upgrade_lock, @@ -517,12 +511,12 @@ impl TestView { pub struct TestViewGenerator { pub current_view: Option, - pub membership: Arc::Membership>>, + pub membership: EpochMembershipCoordinator, pub _pd: PhantomData, } impl TestViewGenerator { - pub fn generate(membership: Arc::Membership>>) -> Self { + pub fn generate(membership: EpochMembershipCoordinator) -> Self { TestViewGenerator { current_view: None, membership, @@ -603,7 +597,7 @@ impl Stream for TestViewGenerator { type Item = TestView; fn poll_next(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { - let mem = Arc::clone(&self.membership); + let mem = self.membership.clone(); let curr_view = &self.current_view.clone(); let mut fut = if let Some(ref view) = curr_view { diff --git a/crates/testing/tests/tests_1/da_task.rs b/crates/testing/tests/tests_1/da_task.rs index b397cae482..dd3b722989 100644 --- a/crates/testing/tests/tests_1/da_task.rs +++ b/crates/testing/tests/tests_1/da_task.rs @@ -24,10 +24,7 @@ use hotshot_testing::{ use hotshot_types::{ data::{null_block, PackedBundle, ViewNumber}, simple_vote::DaData2, - traits::{ - election::Membership, - node_implementation::{ConsensusTime, Versions}, - }, + traits::node_implementation::{ConsensusTime, Versions}, }; use vbs::version::{StaticVersionType, Version}; @@ -39,7 +36,7 @@ async fn test_da_task() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let default_version = Version { major: 0, minor: 0 }; // Make some empty encoded transactions, we just care about having a commitment handy for the @@ -48,12 +45,11 @@ async fn test_da_task() { let encoded_transactions: Arc<[u8]> = Arc::from(TestTransaction::encode(&transactions)); let payload_commit = hotshot_types::traits::block_contents::vid_commitment::( &encoded_transactions, - handle - .hotshot - .membership_coordinator - .read() + membership + .membership_for_epoch(None) .await - .total_nodes(None), + .total_nodes() + .await, default_version, ); @@ -113,7 +109,11 @@ async fn test_da_task() { ViewNumber::new(2), None, vec1::vec1![null_block::builder_fee::( - membership.read().await.total_nodes(None), + membership + .membership_for_epoch(None) + .await + .total_nodes() + .await, ::Base::VERSION, *ViewNumber::new(2), ) @@ -153,7 +153,7 @@ async fn test_da_task_storage_failure() { // Set the error flag here for the system handle. This causes it to emit an error on append. handle.storage().write().await.should_return_err = true; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let default_version = Version { major: 0, minor: 0 }; // Make some empty encoded transactions, we just care about having a commitment handy for the @@ -162,16 +162,15 @@ async fn test_da_task_storage_failure() { let encoded_transactions: Arc<[u8]> = Arc::from(TestTransaction::encode(&transactions)); let payload_commit = hotshot_types::traits::block_contents::vid_commitment::( &encoded_transactions, - handle - .hotshot - .membership_coordinator - .read() + membership + .membership_for_epoch(None) .await - .total_nodes(None), + .total_nodes() + .await, default_version, ); - let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); + let mut generator = TestViewGenerator::::generate(membership.clone()); let mut proposals = Vec::new(); let mut leaders = Vec::new(); @@ -227,7 +226,11 @@ async fn test_da_task_storage_failure() { ViewNumber::new(2), None, vec1::vec1![null_block::builder_fee::( - membership.read().await.total_nodes(None), + membership + .membership_for_epoch(None) + .await + .total_nodes() + .await, ::Base::VERSION, *ViewNumber::new(2), ) diff --git a/crates/testing/tests/tests_1/message.rs b/crates/testing/tests/tests_1/message.rs index 46957d2324..f40e45b5d0 100644 --- a/crates/testing/tests/tests_1/message.rs +++ b/crates/testing/tests/tests_1/message.rs @@ -6,7 +6,6 @@ #[cfg(test)] use std::marker::PhantomData; -use std::sync::Arc; use committable::Committable; use hotshot_example_types::node_types::TestTypes; @@ -69,7 +68,6 @@ async fn test_certificate2_validity() { use hotshot_testing::{helpers::build_system_handle, view_generator::TestViewGenerator}; use hotshot_types::{ data::{Leaf, Leaf2}, - traits::election::Membership, vote::Certificate, }; @@ -79,9 +77,9 @@ async fn test_certificate2_validity() { let handle = build_system_handle::(node_id) .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); - let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); + let mut generator = TestViewGenerator::::generate(membership.clone()); let mut proposals = Vec::new(); let mut leaders = Vec::new(); @@ -104,10 +102,9 @@ async fn test_certificate2_validity() { let qc2 = proposal.data.justify_qc().clone(); let qc = qc2.clone().to_qc(); - let membership_reader = membership.read().await; - let membership_stake_table = membership_reader.stake_table(None); - let membership_success_threshold = membership_reader.success_threshold(None); - drop(membership_reader); + let epoch_mem = membership.membership_for_epoch(None).await; + let membership_stake_table = epoch_mem.stake_table().await; + let membership_success_threshold = epoch_mem.success_threshold().await; assert!(qc .is_valid_cert( diff --git a/crates/testing/tests/tests_1/network_task.rs b/crates/testing/tests/tests_1/network_task.rs index f634c60113..99f34e1a16 100644 --- a/crates/testing/tests/tests_1/network_task.rs +++ b/crates/testing/tests/tests_1/network_task.rs @@ -36,6 +36,7 @@ async fn test_network_task() { use std::collections::BTreeMap; use futures::StreamExt; + use hotshot_types::epoch_membership::EpochMembershipCoordinator; hotshot::helpers::initialize_logging(); @@ -62,12 +63,13 @@ async fn test_network_task() { all_nodes.clone(), all_nodes, ))); + let coordinator = EpochMembershipCoordinator::new(membership, config.epoch_height); let network_state: NetworkEventTaskState, _> = NetworkEventTaskState { network: network.clone(), view: ViewNumber::new(0), epoch: None, - membership: Arc::clone(&membership), + membership_coordinator: coordinator.clone(), upgrade_lock: upgrade_lock.clone(), storage, consensus, @@ -80,7 +82,7 @@ async fn test_network_task() { let task = Task::new(network_state, tx.clone(), rx); task_reg.run_task(task); - let mut generator = TestViewGenerator::::generate(membership); + let mut generator = TestViewGenerator::::generate(coordinator); let view = generator.next().await.unwrap(); let (out_tx_internal, mut out_rx_internal) = async_broadcast::broadcast(10); @@ -209,6 +211,7 @@ async fn test_network_storage_fail() { use std::collections::BTreeMap; use futures::StreamExt; + use hotshot_types::epoch_membership::EpochMembershipCoordinator; hotshot::helpers::initialize_logging(); @@ -235,12 +238,13 @@ async fn test_network_storage_fail() { all_nodes.clone(), all_nodes, ))); + let coordinator = EpochMembershipCoordinator::new(membership, config.epoch_height); let network_state: NetworkEventTaskState, _> = NetworkEventTaskState { network: network.clone(), view: ViewNumber::new(0), epoch: None, - membership: Arc::clone(&membership), + membership_coordinator: coordinator.clone(), upgrade_lock: upgrade_lock.clone(), storage, consensus, @@ -253,7 +257,7 @@ async fn test_network_storage_fail() { let task = Task::new(network_state, tx.clone(), rx); task_reg.run_task(task); - let mut generator = TestViewGenerator::::generate(membership); + let mut generator = TestViewGenerator::::generate(coordinator); let view = generator.next().await.unwrap(); let (out_tx_internal, mut out_rx_internal): (Sender>>, _) = diff --git a/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs b/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs index b7fec2c51b..7aaadb0039 100644 --- a/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs +++ b/crates/testing/tests/tests_1/quorum_proposal_recv_task.rs @@ -54,7 +54,7 @@ async fn test_quorum_proposal_recv_task() { let handle = build_system_handle::(2) .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let consensus = handle.hotshot.consensus(); let mut consensus_writer = consensus.write().await; @@ -129,7 +129,7 @@ async fn test_quorum_proposal_recv_task_liveness_check() { let handle = build_system_handle::(4) .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let consensus = handle.hotshot.consensus(); let mut consensus_writer = consensus.write().await; diff --git a/crates/testing/tests/tests_1/quorum_proposal_task.rs b/crates/testing/tests/tests_1/quorum_proposal_task.rs index 75b4e8ad58..747564069a 100644 --- a/crates/testing/tests/tests_1/quorum_proposal_task.rs +++ b/crates/testing/tests/tests_1/quorum_proposal_task.rs @@ -27,10 +27,7 @@ use hotshot_testing::{ use hotshot_types::{ data::{null_block, EpochNumber, Leaf2, ViewChangeEvidence2, ViewNumber}, simple_vote::{TimeoutData2, ViewSyncFinalizeData2}, - traits::{ - election::Membership, - node_implementation::{ConsensusTime, Versions}, - }, + traits::node_implementation::{ConsensusTime, Versions}, utils::BuilderCommitment, }; use sha2::Digest; @@ -51,7 +48,10 @@ async fn test_quorum_proposal_task_quorum_proposal_view_1() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); + let epoch_1_mem = membership + .membership_for_epoch(Some(EpochNumber::new(1))) + .await; let version = handle .hotshot .upgrade_lock @@ -59,14 +59,13 @@ async fn test_quorum_proposal_task_quorum_proposal_view_1() { .await; let payload_commitment = build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(node_id), - Some(EpochNumber::new(1)), version, ) .await; - let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); + let mut generator = TestViewGenerator::::generate(membership.clone()); let mut proposals = Vec::new(); let mut leaders = Vec::new(); @@ -97,10 +96,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_1() { let genesis_cert = proposals[0].data.justify_qc().clone(); let builder_commitment = BuilderCommitment::from_raw_digest(sha2::Sha256::new().finalize()); let builder_fee = null_block::builder_fee::( - membership - .read() - .await - .total_nodes(Some(EpochNumber::new(1))), + epoch_1_mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(1), ) @@ -155,7 +151,10 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); + let epoch_1_mem = membership + .membership_for_epoch(Some(EpochNumber::new(1))) + .await; let mut generator = TestViewGenerator::::generate(membership.clone()); @@ -192,10 +191,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { let builder_commitment = BuilderCommitment::from_raw_digest(sha2::Sha256::new().finalize()); let builder_fee = null_block::builder_fee::( - membership - .read() - .await - .total_nodes(Some(EpochNumber::new(1))), + epoch_1_mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(1), ) @@ -213,9 +209,8 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { Qc2Formed(either::Left(genesis_cert.clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(1), - Some(EpochNumber::new(1)), version_1, ) .await, @@ -234,9 +229,8 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { Qc2Formed(either::Left(proposals[1].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(2), - Some(EpochNumber::new(1)), version_2, ) .await, @@ -253,9 +247,8 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { Qc2Formed(either::Left(proposals[2].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(3), - Some(EpochNumber::new(1)), version_3, ) .await, @@ -272,9 +265,8 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { Qc2Formed(either::Left(proposals[3].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(4), - Some(EpochNumber::new(1)), version_4, ) .await, @@ -291,9 +283,8 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() { Qc2Formed(either::Left(proposals[4].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(5), - Some(EpochNumber::new(1)), version_5, ) .await, @@ -338,7 +329,10 @@ async fn test_quorum_proposal_task_qc_timeout() { let handle = build_system_handle::(node_id) .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); + let epoch_1_mem = membership + .membership_for_epoch(Some(EpochNumber::new(1))) + .await; let version = handle .hotshot .upgrade_lock @@ -346,15 +340,14 @@ async fn test_quorum_proposal_task_qc_timeout() { .await; let payload_commitment = build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(node_id), - Some(EpochNumber::new(1)), version, ) .await; let builder_commitment = BuilderCommitment::from_raw_digest(sha2::Sha256::new().finalize()); - let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); + let mut generator = TestViewGenerator::::generate(membership.clone()); let mut proposals = Vec::new(); let mut leaders = Vec::new(); @@ -397,10 +390,7 @@ async fn test_quorum_proposal_task_qc_timeout() { }, ViewNumber::new(3), vec1![null_block::builder_fee::( - membership - .read() - .await - .total_nodes(Some(EpochNumber::new(1))), + epoch_1_mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(3), ) @@ -437,7 +427,10 @@ async fn test_quorum_proposal_task_view_sync() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); + let epoch_1_mem = membership + .membership_for_epoch(Some(EpochNumber::new(1))) + .await; let version = handle .hotshot .upgrade_lock @@ -445,15 +438,14 @@ async fn test_quorum_proposal_task_view_sync() { .await; let payload_commitment = build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(node_id), - Some(EpochNumber::new(1)), version, ) .await; let builder_commitment = BuilderCommitment::from_raw_digest(sha2::Sha256::new().finalize()); - let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); + let mut generator = TestViewGenerator::::generate(membership.clone()); let mut proposals = Vec::new(); let mut leaders = Vec::new(); @@ -498,10 +490,7 @@ async fn test_quorum_proposal_task_view_sync() { }, ViewNumber::new(2), vec1![null_block::builder_fee::( - membership - .read() - .await - .total_nodes(Some(EpochNumber::new(1))), + epoch_1_mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(2), ) @@ -536,9 +525,12 @@ async fn test_quorum_proposal_task_liveness_check() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); + let epoch_1_mem = membership + .membership_for_epoch(Some(EpochNumber::new(1))) + .await; - let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); + let mut generator = TestViewGenerator::::generate(membership.clone()); let mut proposals = Vec::new(); let mut leaders = Vec::new(); @@ -568,10 +560,7 @@ async fn test_quorum_proposal_task_liveness_check() { let builder_commitment = BuilderCommitment::from_raw_digest(sha2::Sha256::new().finalize()); let builder_fee = null_block::builder_fee::( - membership - .read() - .await - .total_nodes(Some(EpochNumber::new(1))), + epoch_1_mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(1), ) @@ -593,9 +582,8 @@ async fn test_quorum_proposal_task_liveness_check() { Qc2Formed(either::Left(genesis_cert.clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(1), - Some(EpochNumber::new(1)), version_1, ) .await, @@ -614,9 +602,8 @@ async fn test_quorum_proposal_task_liveness_check() { Qc2Formed(either::Left(proposals[1].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(2), - Some(EpochNumber::new(1)), version_2, ) .await, @@ -633,9 +620,8 @@ async fn test_quorum_proposal_task_liveness_check() { Qc2Formed(either::Left(proposals[2].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(3), - Some(EpochNumber::new(1)), version_3, ) .await, @@ -652,9 +638,8 @@ async fn test_quorum_proposal_task_liveness_check() { Qc2Formed(either::Left(proposals[3].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(4), - Some(EpochNumber::new(1)), version_4, ) .await, @@ -671,9 +656,8 @@ async fn test_quorum_proposal_task_liveness_check() { Qc2Formed(either::Left(proposals[4].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(5), - Some(EpochNumber::new(1)), version_5, ) .await, @@ -714,7 +698,7 @@ async fn test_quorum_proposal_task_with_incomplete_events() { let handle = build_system_handle::(2) .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let mut generator = TestViewGenerator::::generate(membership); diff --git a/crates/testing/tests/tests_1/quorum_vote_task.rs b/crates/testing/tests/tests_1/quorum_vote_task.rs index 4d2ed7e6c0..da1b81f6fb 100644 --- a/crates/testing/tests/tests_1/quorum_vote_task.rs +++ b/crates/testing/tests/tests_1/quorum_vote_task.rs @@ -45,7 +45,7 @@ async fn test_quorum_vote_task_success() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let mut generator = TestViewGenerator::::generate(membership); @@ -112,7 +112,7 @@ async fn test_quorum_vote_task_miss_dependency() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let mut generator = TestViewGenerator::::generate(membership); @@ -196,7 +196,7 @@ async fn test_quorum_vote_task_incorrect_dependency() { .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let mut generator = TestViewGenerator::::generate(membership); diff --git a/crates/testing/tests/tests_1/transaction_task.rs b/crates/testing/tests/tests_1/transaction_task.rs index d83bc1c79d..9bd689df7d 100644 --- a/crates/testing/tests/tests_1/transaction_task.rs +++ b/crates/testing/tests/tests_1/transaction_task.rs @@ -9,10 +9,7 @@ use hotshot_task_impls::{ use hotshot_testing::helpers::build_system_handle; use hotshot_types::{ data::{null_block, EpochNumber, PackedBundle, ViewNumber}, - traits::{ - election::Membership, - node_implementation::{ConsensusTime, Versions}, - }, + traits::node_implementation::{ConsensusTime, Versions}, }; use vbs::version::StaticVersionType; @@ -55,9 +52,10 @@ async fn test_transaction_task_leader_two_views_in_a_row() { handle .hotshot .membership_coordinator - .read() + .membership_for_epoch(Some(EpochNumber::new(1))) .await - .total_nodes(Some(EpochNumber::new(0))), + .total_nodes() + .await, ::Base::VERSION, *ViewNumber::new(4), ) diff --git a/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs b/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs index c213068482..588010cfb7 100644 --- a/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs +++ b/crates/testing/tests/tests_1/upgrade_task_with_proposal.rs @@ -83,9 +83,12 @@ async fn test_upgrade_task_with_proposal() { let consensus = handle.hotshot.consensus(); let mut consensus_writer = consensus.write().await; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); + let epoch_1_mem = membership + .membership_for_epoch(Some(EpochNumber::new(1))) + .await; - let mut generator = TestViewGenerator::::generate(Arc::clone(&membership)); + let mut generator = TestViewGenerator::::generate(membership.clone()); for view in (&mut generator).take(1).collect::>().await { proposals.push(view.quorum_proposal.clone()); @@ -126,10 +129,7 @@ async fn test_upgrade_task_with_proposal() { let genesis_cert = proposals[0].data.justify_qc().clone(); let builder_commitment = BuilderCommitment::from_raw_digest(sha2::Sha256::new().finalize()); let builder_fee = null_block::builder_fee::( - membership - .read() - .await - .total_nodes(Some(EpochNumber::new(1))), + epoch_1_mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(1), ) @@ -161,9 +161,8 @@ async fn test_upgrade_task_with_proposal() { Qc2Formed(either::Left(genesis_cert.clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(1), - Some(EpochNumber::new(1)), version_1, ) .await, @@ -182,9 +181,8 @@ async fn test_upgrade_task_with_proposal() { Qc2Formed(either::Left(proposals[1].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(2), - Some(EpochNumber::new(1)), version_2, ) .await, @@ -202,9 +200,8 @@ async fn test_upgrade_task_with_proposal() { Qc2Formed(either::Left(proposals[2].data.justify_qc().clone())), SendPayloadCommitmentAndMetadata( build_payload_commitment::( - &membership, + &epoch_1_mem, ViewNumber::new(3), - Some(EpochNumber::new(1)), version_3, ) .await, diff --git a/crates/testing/tests/tests_1/upgrade_task_with_vote.rs b/crates/testing/tests/tests_1/upgrade_task_with_vote.rs index 42cb73c3be..a43696e307 100644 --- a/crates/testing/tests/tests_1/upgrade_task_with_vote.rs +++ b/crates/testing/tests/tests_1/upgrade_task_with_vote.rs @@ -70,7 +70,7 @@ async fn test_upgrade_task_with_vote() { let consensus = handle.hotshot.consensus().clone(); let mut consensus_writer = consensus.write().await; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let mut generator = TestViewGenerator::::generate(membership); for view in (&mut generator).take(2).collect::>().await { diff --git a/crates/testing/tests/tests_1/vid_task.rs b/crates/testing/tests/tests_1/vid_task.rs index 9e367d00f9..aae6dd6c2f 100644 --- a/crates/testing/tests/tests_1/vid_task.rs +++ b/crates/testing/tests/tests_1/vid_task.rs @@ -24,7 +24,6 @@ use hotshot_types::{ data::{null_block, DaProposal, PackedBundle, VidDisperse, ViewNumber}, traits::{ consensus_api::ConsensusApi, - election::Membership, node_implementation::{ConsensusTime, NodeType, Versions}, BlockPayload, }, @@ -45,14 +44,13 @@ async fn test_vid_task() { .0; let pub_key = handle.public_key(); - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let default_version = Version { major: 0, minor: 0 }; let mut vid = vid_scheme_from_view_number::( - &membership, + &membership.membership_for_epoch(None).await, ViewNumber::new(0), - None, default_version, ) .await; @@ -104,6 +102,7 @@ async fn test_vid_task() { signature: message.signature.clone(), _pd: PhantomData, }; + let mem = membership.membership_for_epoch(None).await; let inputs = vec![ serial![ViewChange(ViewNumber::new(1), None)], serial![ @@ -116,7 +115,7 @@ async fn test_vid_task() { ViewNumber::new(2), None, vec1::vec1![null_block::builder_fee::( - membership.read().await.total_nodes(None), + mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(2), ) @@ -137,7 +136,7 @@ async fn test_vid_task() { }, ViewNumber::new(2), vec1![null_block::builder_fee::( - membership.read().await.total_nodes(None), + mem.total_nodes().await, ::Base::VERSION, *ViewNumber::new(2), ) diff --git a/crates/testing/tests/tests_1/vote_dependency_handle.rs b/crates/testing/tests/tests_1/vote_dependency_handle.rs index 867bcaa81b..828f0f4761 100644 --- a/crates/testing/tests/tests_1/vote_dependency_handle.rs +++ b/crates/testing/tests/tests_1/vote_dependency_handle.rs @@ -36,7 +36,7 @@ async fn test_vote_dependency_handle() { let handle = build_system_handle::(node_id) .await .0; - let membership = Arc::clone(&handle.hotshot.membership_coordinator); + let membership = handle.hotshot.membership_coordinator.clone(); let mut generator = TestViewGenerator::::generate(membership); @@ -90,7 +90,7 @@ async fn test_vote_dependency_handle() { consensus: OuterConsensus::new(consensus.clone()), consensus_metrics: Arc::clone(&consensus.read().await.metrics), instance_state: handle.hotshot.instance_state(), - membership_coordinator: Arc::clone(&handle.hotshot.membership_coordinator), + membership_coordinator: handle.hotshot.membership_coordinator.clone(), storage: Arc::clone(&handle.storage()), view_number, sender: event_sender.clone(), diff --git a/crates/types/src/data/vid_disperse.rs b/crates/types/src/data/vid_disperse.rs index 545260d0b6..b9c5f155d8 100644 --- a/crates/types/src/data/vid_disperse.rs +++ b/crates/types/src/data/vid_disperse.rs @@ -8,7 +8,6 @@ use std::{collections::BTreeMap, fmt::Debug, hash::Hash, marker::PhantomData, sync::Arc}; -use async_lock::RwLock; use jf_vid::{VidDisperse as JfVidDisperse, VidScheme}; use serde::{Deserialize, Serialize}; use tokio::task::spawn_blocking; @@ -20,8 +19,7 @@ use crate::{ message::Proposal, simple_vote::HasEpoch, traits::{ - block_contents::EncodeBytes, election::Membership, node_implementation::NodeType, - signature_key::SignatureKey, + block_contents::EncodeBytes, node_implementation::NodeType, signature_key::SignatureKey, }, vid::{advz_scheme, VidCommitment, VidCommon, VidSchemeType, VidShare}, vote::HasViewNumber, diff --git a/crates/types/src/traits/network.rs b/crates/types/src/traits/network.rs index aeb1509401..fc9086106f 100644 --- a/crates/types/src/traits/network.rs +++ b/crates/types/src/traits/network.rs @@ -17,7 +17,6 @@ use std::{ time::Duration, }; -use async_lock::RwLock; use async_trait::async_trait; use dyn_clone::DynClone; use futures::{future::join_all, Future}; From e1c1c45e7a789a4cc357919d4146b366998e7231 Mon Sep 17 00:00:00 2001 From: Brendon Fish Date: Fri, 7 Feb 2025 14:11:39 -0500 Subject: [PATCH 9/9] fix --- crates/task-impls/src/quorum_vote/handlers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/task-impls/src/quorum_vote/handlers.rs b/crates/task-impls/src/quorum_vote/handlers.rs index f9ca3ac9cd..5252918367 100644 --- a/crates/task-impls/src/quorum_vote/handlers.rs +++ b/crates/task-impls/src/quorum_vote/handlers.rs @@ -10,6 +10,7 @@ use async_broadcast::{InactiveReceiver, Sender}; use async_lock::RwLock; use chrono::Utc; use committable::Committable; +use hotshot_types::epoch_membership::EpochMembership; use hotshot_types::{ consensus::OuterConsensus, data::{Leaf2, QuorumProposalWrapper, VidDisperseShare}, @@ -31,7 +32,6 @@ use hotshot_types::{ }, vote::HasViewNumber, }; -use hotshot_types::{epoch_membership::EpochMembership, simple_vote::HasEpoch}; use tokio::spawn; use tracing::instrument; use utils::anytrace::*;