From c879f561511216facc0637882a3cf7ec5853076e Mon Sep 17 00:00:00 2001 From: tbro Date: Thu, 7 Nov 2024 17:33:17 -0600 Subject: [PATCH 01/10] replace calls to Memberships Delete Memberships and replace functionality. Add some methods to `Membership` trait to deal w/ collapsing into one type both kinds of memberships (stake and DA). * avoid passing membership into `is_valid_cert * for DA, avoid proxying threshold through `Threshold` trait * remove `Topic` param from `Membership::new * Split cert impls by marker (#3891) * add membership methods to Cert trait * remove non-existent tests from justfile --- crates/hotshot/src/tasks/task_state.rs | 3 ++ .../traits/election/randomized_committee.rs | 1 + .../src/traits/election/static_committee.rs | 1 + crates/hotshot/src/types/handle.rs | 3 +- crates/task-impls/src/da.rs | 1 - .../src/quorum_proposal/handlers.rs | 1 + crates/task-impls/src/quorum_proposal/mod.rs | 37 +++++++++++-------- crates/task-impls/src/upgrade.rs | 13 +++---- crates/types/src/traits/election.rs | 15 ++++++++ 9 files changed, 49 insertions(+), 26 deletions(-) diff --git a/crates/hotshot/src/tasks/task_state.rs b/crates/hotshot/src/tasks/task_state.rs index 1df18f9b5e..6752aae43f 100644 --- a/crates/hotshot/src/tasks/task_state.rs +++ b/crates/hotshot/src/tasks/task_state.rs @@ -72,6 +72,7 @@ impl, V: Versions> CreateTaskState cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, quorum_membership: (*handle.hotshot.memberships).clone().into(), + network: Arc::clone(&handle.hotshot.network), vote_collectors: BTreeMap::default(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), @@ -162,6 +163,7 @@ impl, V: Versions> CreateTaskState cur_view, next_view: cur_view, cur_epoch: handle.cur_epoch().await, + network: Arc::clone(&handle.hotshot.network), membership: (*handle.hotshot.memberships).clone().into(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), @@ -189,6 +191,7 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(handle.hotshot.consensus()), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, + network: Arc::clone(&handle.hotshot.network), membership: (*handle.hotshot.memberships).clone().into(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), diff --git a/crates/hotshot/src/traits/election/randomized_committee.rs b/crates/hotshot/src/traits/election/randomized_committee.rs index 2b721a66e0..0c11414631 100644 --- a/crates/hotshot/src/traits/election/randomized_committee.rs +++ b/crates/hotshot/src/traits/election/randomized_committee.rs @@ -94,6 +94,7 @@ impl Membership for RandomizedCommittee { Self { eligible_leaders, + da_eligible_leaders, stake_table: members, da_stake_table: da_members, indexed_stake_table, diff --git a/crates/hotshot/src/traits/election/static_committee.rs b/crates/hotshot/src/traits/election/static_committee.rs index fa904c66cf..2e82fcee4a 100644 --- a/crates/hotshot/src/traits/election/static_committee.rs +++ b/crates/hotshot/src/traits/election/static_committee.rs @@ -91,6 +91,7 @@ impl Membership for StaticCommittee { Self { eligible_leaders, + da_eligible_leaders, stake_table: members, da_stake_table: da_members, indexed_stake_table, diff --git a/crates/hotshot/src/types/handle.rs b/crates/hotshot/src/types/handle.rs index 7b1fd5a424..caeba9a518 100644 --- a/crates/hotshot/src/types/handle.rs +++ b/crates/hotshot/src/types/handle.rs @@ -184,7 +184,6 @@ impl + 'static, V: Versions> .ok_or(anyhow!("Event dependency failed to get event"))?; // Then, if it's `Some`, make sure that the data is correct - if let HotShotEvent::QuorumProposalResponseRecv(quorum_proposal) = hs_event.as_ref() { // Make sure that the quorum_proposal is valid @@ -197,7 +196,7 @@ impl + 'static, V: Versions> if commit == leaf_commitment { return Ok(quorum_proposal.clone()); } - tracing::warn!("Proposal received from request has different commitment than expected.\nExpected = {:?}\nReceived{:?}", leaf_commitment, commit); + tracing::warn!("Proposal receied from request has different commitment than expected.\nExpected = {:?}\nReceived{:?}", leaf_commitment, commit); } } }) diff --git a/crates/task-impls/src/da.rs b/crates/task-impls/src/da.rs index 68df39136f..2c503d2b13 100644 --- a/crates/task-impls/src/da.rs +++ b/crates/task-impls/src/da.rs @@ -120,7 +120,6 @@ impl, V: Versions> DaTaskState. -use std::{collections::BTreeMap, sync::Arc, time::Instant}; +use std::{collections::BTreeMap, num::NonZeroU64, sync::Arc, time::Instant}; use async_broadcast::{Receiver, Sender}; use async_lock::RwLock; @@ -48,7 +48,7 @@ pub struct QuorumProposalTaskState pub instance_state: Arc, /// Membership for Quorum Certs/votes - pub quorum_membership: Arc, + pub membership: Arc, /// Our public key pub public_key: TYPES::SignatureKey, @@ -277,7 +277,7 @@ impl, V: Versions> ) -> Result<()> { // Don't even bother making the task if we are not entitled to propose anyway. ensure!( - self.quorum_membership.leader(view_number, epoch_number)? == self.public_key, + self.membership.leader(view_number, epoch_number)? == self.public_key, debug!("We are not the leader of the next view") ); @@ -306,7 +306,7 @@ impl, V: Versions> view_number, sender: event_sender, receiver: event_receiver, - quorum_membership: Arc::clone(&self.quorum_membership), + quorum_membership: Arc::clone(&self.membership), public_key: self.public_key.clone(), private_key: self.private_key.clone(), instance_state: Arc::clone(&self.instance_state), @@ -441,18 +441,23 @@ impl, V: Versions> let epoch_number = self.consensus.read().await.cur_epoch(); ensure!( - certificate - .is_valid_cert( - self.quorum_membership.stake_table(epoch_number), - self.quorum_membership.success_threshold(), - &self.upgrade_lock - ) - .await, - warn!( - "View Sync Finalize certificate {:?} was invalid", - certificate.data() - ) - ); + certificate + .is_valid_cert( + <<<<<<< HEAD + self.quorum_membership.stake_table(epoch_number), + self.quorum_membership.success_threshold(), + ======= + self.membership.stake_table(epoch_number), + self.membership.success_threshold(), + >>>>>>> 5f41f111dd (replace calls to Memberships) + &self.upgrade_lock + ) + .await, + warn!( + "View Sync Finalize certificate {:?} was invalid", + certificate.data() + ) + ); let view_number = certificate.view_number; diff --git a/crates/task-impls/src/upgrade.rs b/crates/task-impls/src/upgrade.rs index a8dec2e3f3..0f3686185f 100644 --- a/crates/task-impls/src/upgrade.rs +++ b/crates/task-impls/src/upgrade.rs @@ -49,7 +49,7 @@ pub struct UpgradeTaskState { pub cur_epoch: TYPES::Epoch, /// Membership for Quorum Certs/votes - pub quorum_membership: Arc, + pub membership: Arc, /// A map of `UpgradeVote` collector tasks pub vote_collectors: VoteCollectorsMap, UpgradeCertificate, V>, @@ -179,7 +179,7 @@ impl UpgradeTaskState { ); // We then validate that the proposal was issued by the leader for the view. - let view_leader_key = self.quorum_membership.leader(view, self.cur_epoch)?; + let view_leader_key = self.membership.leader(view, self.cur_epoch)?; ensure!( view_leader_key == *sender, info!( @@ -223,12 +223,11 @@ impl UpgradeTaskState { { let view = vote.view_number(); ensure!( - self.quorum_membership.leader(view, self.cur_epoch)? == self.public_key, + self.membership.leader(view, self.cur_epoch)? == self.public_key, debug!( "We are not the leader for view {} are we leader for next view? {}", *view, - self.quorum_membership.leader(view + 1, self.cur_epoch)? - == self.public_key + self.membership.leader(view + 1, self.cur_epoch)? == self.public_key ) ); } @@ -237,7 +236,7 @@ impl UpgradeTaskState { &mut self.vote_collectors, vote, self.public_key.clone(), - &self.quorum_membership, + &self.membership, self.cur_epoch, self.id, &event, @@ -270,7 +269,7 @@ impl UpgradeTaskState { && time >= self.start_proposing_time && time < self.stop_proposing_time && !self.upgraded().await - && self.quorum_membership.leader( + && self.membership.leader( TYPES::View::new(view + UPGRADE_PROPOSE_OFFSET), self.cur_epoch, )? == self.public_key diff --git a/crates/types/src/traits/election.rs b/crates/types/src/traits/election.rs index 04aa76ccb4..1205de0437 100644 --- a/crates/types/src/traits/election.rs +++ b/crates/types/src/traits/election.rs @@ -95,6 +95,21 @@ pub trait Membership: Clone + Debug + Send + Sync { )) } + /// The DA 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. + fn da_leader(&self, view: TYPES::View, epoch: TYPES::Epoch) -> Result { + use utils::anytrace::*; + + self.lookup_da_leader(view, epoch).wrap().context(info!( + "Failed to get leader for view {view} in epoch {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 From 03a05a3450cff70b874352fbc85d69b7dd7a8d7a Mon Sep 17 00:00:00 2001 From: tbro Date: Tue, 19 Nov 2024 19:30:07 -0300 Subject: [PATCH 02/10] cleanup --- crates/task-impls/src/quorum_proposal/handlers.rs | 1 - crates/task-impls/src/quorum_proposal/mod.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/task-impls/src/quorum_proposal/handlers.rs b/crates/task-impls/src/quorum_proposal/handlers.rs index 614ea1905e..55f283bcd8 100644 --- a/crates/task-impls/src/quorum_proposal/handlers.rs +++ b/crates/task-impls/src/quorum_proposal/handlers.rs @@ -9,7 +9,6 @@ use std::{ marker::PhantomData, - num::NonZeroU64, sync::Arc, time::{Duration, Instant}, }; diff --git a/crates/task-impls/src/quorum_proposal/mod.rs b/crates/task-impls/src/quorum_proposal/mod.rs index 7e4532659b..e48878d0cb 100644 --- a/crates/task-impls/src/quorum_proposal/mod.rs +++ b/crates/task-impls/src/quorum_proposal/mod.rs @@ -4,7 +4,7 @@ // You should have received a copy of the MIT License // along with the HotShot repository. If not, see . -use std::{collections::BTreeMap, num::NonZeroU64, sync::Arc, time::Instant}; +use std::{collections::BTreeMap, sync::Arc, time::Instant}; use async_broadcast::{Receiver, Sender}; use async_lock::RwLock; From 8ff1be39f68cb783ef0a5b076a09993c628ce397 Mon Sep 17 00:00:00 2001 From: tbro Date: Tue, 19 Nov 2024 19:55:06 -0300 Subject: [PATCH 03/10] conflict resolution --- crates/testing/tests/tests_1/message.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/testing/tests/tests_1/message.rs b/crates/testing/tests/tests_1/message.rs index 3778aa4737..a1ab371f79 100644 --- a/crates/testing/tests/tests_1/message.rs +++ b/crates/testing/tests/tests_1/message.rs @@ -67,6 +67,7 @@ async fn test_certificate2_validity() { use hotshot_types::{ data::{EpochNumber, Leaf, Leaf2}, traits::{election::Membership, node_implementation::ConsensusTime}, + vote::Certificate, }; From e3a348f3b79e12e217f28e576e6bb31d7ed1e80c Mon Sep 17 00:00:00 2001 From: tbro Date: Wed, 20 Nov 2024 18:20:39 -0300 Subject: [PATCH 04/10] revert some unnecessary name changes We can keep the old name where we only have one membership type to keep the diff smaller. --- crates/task-impls/src/quorum_proposal/mod.rs | 35 +++++++++----------- crates/task-impls/src/upgrade.rs | 13 ++++---- crates/testing/src/helpers.rs | 4 +-- crates/testing/tests/tests_1/message.rs | 1 - 4 files changed, 24 insertions(+), 29 deletions(-) diff --git a/crates/task-impls/src/quorum_proposal/mod.rs b/crates/task-impls/src/quorum_proposal/mod.rs index e48878d0cb..034db12812 100644 --- a/crates/task-impls/src/quorum_proposal/mod.rs +++ b/crates/task-impls/src/quorum_proposal/mod.rs @@ -48,7 +48,7 @@ pub struct QuorumProposalTaskState pub instance_state: Arc, /// Membership for Quorum Certs/votes - pub membership: Arc, + pub quorum_membership: Arc, /// Our public key pub public_key: TYPES::SignatureKey, @@ -277,7 +277,7 @@ impl, V: Versions> ) -> Result<()> { // Don't even bother making the task if we are not entitled to propose anyway. ensure!( - self.membership.leader(view_number, epoch_number)? == self.public_key, + self.quorum_membership.leader(view_number, epoch_number)? == self.public_key, debug!("We are not the leader of the next view") ); @@ -306,7 +306,7 @@ impl, V: Versions> view_number, sender: event_sender, receiver: event_receiver, - quorum_membership: Arc::clone(&self.membership), + quorum_membership: Arc::clone(&self.quorum_membership), public_key: self.public_key.clone(), private_key: self.private_key.clone(), instance_state: Arc::clone(&self.instance_state), @@ -441,23 +441,18 @@ impl, V: Versions> let epoch_number = self.consensus.read().await.cur_epoch(); ensure!( - certificate - .is_valid_cert( - <<<<<<< HEAD - self.quorum_membership.stake_table(epoch_number), - self.quorum_membership.success_threshold(), - ======= - self.membership.stake_table(epoch_number), - self.membership.success_threshold(), - >>>>>>> 5f41f111dd (replace calls to Memberships) - &self.upgrade_lock - ) - .await, - warn!( - "View Sync Finalize certificate {:?} was invalid", - certificate.data() - ) - ); + certificate + .is_valid_cert( + self.quorum_membership.stake_table(epoch_number), + self.quorum_membership.success_threshold(), + &self.upgrade_lock + ) + .await, + warn!( + "View Sync Finalize certificate {:?} was invalid", + certificate.data() + ) + ); let view_number = certificate.view_number; diff --git a/crates/task-impls/src/upgrade.rs b/crates/task-impls/src/upgrade.rs index 0f3686185f..a8dec2e3f3 100644 --- a/crates/task-impls/src/upgrade.rs +++ b/crates/task-impls/src/upgrade.rs @@ -49,7 +49,7 @@ pub struct UpgradeTaskState { pub cur_epoch: TYPES::Epoch, /// Membership for Quorum Certs/votes - pub membership: Arc, + pub quorum_membership: Arc, /// A map of `UpgradeVote` collector tasks pub vote_collectors: VoteCollectorsMap, UpgradeCertificate, V>, @@ -179,7 +179,7 @@ impl UpgradeTaskState { ); // We then validate that the proposal was issued by the leader for the view. - let view_leader_key = self.membership.leader(view, self.cur_epoch)?; + let view_leader_key = self.quorum_membership.leader(view, self.cur_epoch)?; ensure!( view_leader_key == *sender, info!( @@ -223,11 +223,12 @@ impl UpgradeTaskState { { let view = vote.view_number(); ensure!( - self.membership.leader(view, self.cur_epoch)? == self.public_key, + self.quorum_membership.leader(view, self.cur_epoch)? == self.public_key, debug!( "We are not the leader for view {} are we leader for next view? {}", *view, - self.membership.leader(view + 1, self.cur_epoch)? == self.public_key + self.quorum_membership.leader(view + 1, self.cur_epoch)? + == self.public_key ) ); } @@ -236,7 +237,7 @@ impl UpgradeTaskState { &mut self.vote_collectors, vote, self.public_key.clone(), - &self.membership, + &self.quorum_membership, self.cur_epoch, self.id, &event, @@ -269,7 +270,7 @@ impl UpgradeTaskState { && time >= self.start_proposing_time && time < self.stop_proposing_time && !self.upgraded().await - && self.membership.leader( + && self.quorum_membership.leader( TYPES::View::new(view + UPGRADE_PROPOSE_OFFSET), self.cur_epoch, )? == self.public_key diff --git a/crates/testing/src/helpers.rs b/crates/testing/src/helpers.rs index 6759d17e69..3f7b300f48 100644 --- a/crates/testing/src/helpers.rs +++ b/crates/testing/src/helpers.rs @@ -140,7 +140,7 @@ pub async fn build_cert< CERT: Certificate, >( data: DATAType, - membership: &TYPES::Membership, + da_membership: &TYPES::Membership, view: TYPES::View, epoch: TYPES::Epoch, public_key: &TYPES::SignatureKey, @@ -149,7 +149,7 @@ pub async fn build_cert< ) -> CERT { let real_qc_sig = build_assembled_sig::( &data, - membership, + da_membership, view, epoch, upgrade_lock, diff --git a/crates/testing/tests/tests_1/message.rs b/crates/testing/tests/tests_1/message.rs index a1ab371f79..3778aa4737 100644 --- a/crates/testing/tests/tests_1/message.rs +++ b/crates/testing/tests/tests_1/message.rs @@ -67,7 +67,6 @@ async fn test_certificate2_validity() { use hotshot_types::{ data::{EpochNumber, Leaf, Leaf2}, traits::{election::Membership, node_implementation::ConsensusTime}, - vote::Certificate, }; From 07d9ba4d8500e762721b3421bea8fcb349a8ed71 Mon Sep 17 00:00:00 2001 From: tbro Date: Fri, 22 Nov 2024 16:13:46 -0300 Subject: [PATCH 05/10] Leaders are Leaders Always use quorum for leader selection --- crates/hotshot/src/traits/election/randomized_committee.rs | 1 - crates/hotshot/src/traits/election/static_committee.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/crates/hotshot/src/traits/election/randomized_committee.rs b/crates/hotshot/src/traits/election/randomized_committee.rs index 0c11414631..2b721a66e0 100644 --- a/crates/hotshot/src/traits/election/randomized_committee.rs +++ b/crates/hotshot/src/traits/election/randomized_committee.rs @@ -94,7 +94,6 @@ impl Membership for RandomizedCommittee { Self { eligible_leaders, - da_eligible_leaders, stake_table: members, da_stake_table: da_members, indexed_stake_table, diff --git a/crates/hotshot/src/traits/election/static_committee.rs b/crates/hotshot/src/traits/election/static_committee.rs index 2e82fcee4a..fa904c66cf 100644 --- a/crates/hotshot/src/traits/election/static_committee.rs +++ b/crates/hotshot/src/traits/election/static_committee.rs @@ -91,7 +91,6 @@ impl Membership for StaticCommittee { Self { eligible_leaders, - da_eligible_leaders, stake_table: members, da_stake_table: da_members, indexed_stake_table, From 6c5d74b3a300d95dab5b2600e51da57ff25f8209 Mon Sep 17 00:00:00 2001 From: pls148 <184445976+pls148@users.noreply.github.com> Date: Wed, 6 Nov 2024 14:18:02 -0800 Subject: [PATCH 06/10] Add randomized committees for testing --- crates/example-types/src/node_types.rs | 42 ++- crates/hotshot/src/traits/election/helpers.rs | 282 ++++++++++++++++++ .../traits/{election.rs => election/mod.rs} | 8 + .../election/randomized_committee_members.rs | 250 ++++++++++++++++ .../src/traits/networking/memory_network.rs | 55 +++- crates/macros/src/lib.rs | 75 ++++- crates/task-impls/src/consensus/handlers.rs | 5 +- crates/testing/src/test_runner.rs | 2 +- crates/testing/tests/tests_1/block_builder.rs | 8 +- crates/testing/tests/tests_1/test_success.rs | 21 +- 10 files changed, 715 insertions(+), 33 deletions(-) create mode 100644 crates/hotshot/src/traits/election/helpers.rs rename crates/hotshot/src/traits/{election.rs => election/mod.rs} (80%) create mode 100644 crates/hotshot/src/traits/election/randomized_committee_members.rs diff --git a/crates/example-types/src/node_types.rs b/crates/example-types/src/node_types.rs index 80b634515c..b45e2e7377 100644 --- a/crates/example-types/src/node_types.rs +++ b/crates/example-types/src/node_types.rs @@ -6,7 +6,9 @@ use hotshot::traits::{ election::{ - randomized_committee::RandomizedCommittee, static_committee::StaticCommittee, + randomized_committee::RandomizedCommittee, + randomized_committee_members::RandomizedCommitteeMembers, + static_committee::StaticCommittee, static_committee_leader_two_views::StaticCommitteeLeaderForTwoViews, }, implementations::{CombinedNetworks, Libp2pNetwork, MemoryNetwork, PushCdnNetwork}, @@ -87,6 +89,42 @@ impl NodeType for TestTypesRandomizedLeader { type BuilderSignatureKey = BuilderKey; } +#[derive( + Copy, + Clone, + Debug, + Default, + Hash, + PartialEq, + Eq, + PartialOrd, + Ord, + serde::Serialize, + serde::Deserialize, +)] +/// filler struct to implement node type and allow us +/// to select our traits +pub struct TestTypesRandomizedCommitteeMembers; +impl NodeType + for TestTypesRandomizedCommitteeMembers +{ + type AuctionResult = TestAuctionResult; + type View = ViewNumber; + type Epoch = EpochNumber; + type BlockHeader = TestBlockHeader; + type BlockPayload = TestBlockPayload; + type SignatureKey = BLSPubKey; + type Transaction = TestTransaction; + type ValidatedState = TestValidatedState; + type InstanceState = TestInstanceState; + type Membership = RandomizedCommitteeMembers< + TestTypesRandomizedCommitteeMembers, + SEED, + OVERLAP, + >; + type BuilderSignatureKey = BuilderKey; +} + #[derive( Copy, Clone, @@ -133,7 +171,7 @@ pub struct Libp2pImpl; #[derive(Clone, Debug, Deserialize, Serialize, Hash, Eq, PartialEq)] pub struct WebImpl; -/// Combined Network implementation (libp2p + web sever) +/// Combined Network implementation (libp2p + web server) #[derive(Clone, Debug, Deserialize, Serialize, Hash, Eq, PartialEq)] pub struct CombinedImpl; diff --git a/crates/hotshot/src/traits/election/helpers.rs b/crates/hotshot/src/traits/election/helpers.rs new file mode 100644 index 0000000000..334458f57d --- /dev/null +++ b/crates/hotshot/src/traits/election/helpers.rs @@ -0,0 +1,282 @@ +// Copyright (c) 2021-2024 Espresso Systems (espressosys.com) +// This file is part of the HotShot repository. + +// You should have received a copy of the MIT License +// along with the HotShot repository. If not, see . + +use std::collections::BTreeSet; + +use rand::{rngs::StdRng, Rng, SeedableRng}; + +/// Helper which allows producing random numbers within a range and preventing duplicates +/// If consumed as a regular iterator, will return a randomly ordered permutation of all +/// values from 0..max +struct NonRepeatValueIterator { + /// Random number generator to use + rng: StdRng, + + /// Values which have already been emitted, to avoid duplicates + values: BTreeSet, + + /// Maximum value, open-ended. Numbers returned will be 0..max + max: u64, +} + +impl NonRepeatValueIterator { + /// Create a new NonRepeatValueIterator + pub fn new(rng: StdRng, max: u64) -> Self { + Self { + rng, + values: BTreeSet::new(), + max, + } + } +} + +impl Iterator for NonRepeatValueIterator { + type Item = u64; + + fn next(&mut self) -> Option { + if self.values.len() as u64 >= self.max { + return None; + } + + loop { + let v = self.rng.gen_range(0..self.max); + if !self.values.contains(&v) { + self.values.insert(v); + return Some(v); + } + } + } +} + +/// Create a single u64 seed by merging two u64s. Done this way to allow easy seeding of the number generator +/// from both a stable SOUND as well as a moving value ROUND (typically, epoch). +fn make_seed(seed: u64, round: u64) -> u64 { + seed.wrapping_add(round.wrapping_shl(8)) +} + +/// Create a pair of PRNGs for the given SEED and ROUND. Prev_rng is the PRNG for the previous ROUND, used to +/// deterministically replay random numbers generated for the previous ROUND. +fn make_rngs(seed: u64, round: u64) -> (StdRng, StdRng) { + let prev_rng = SeedableRng::seed_from_u64(make_seed(seed, round.wrapping_sub(1))); + let this_rng = SeedableRng::seed_from_u64(make_seed(seed, round)); + + (prev_rng, this_rng) +} + +/// Iterator which returns odd/even values for a given COUNT of nodes. For OVERLAP=0, this will return +/// [0, 2, 4, 6, ...] for an even round, and [1, 3, 5, 7, ...] for an odd round. Setting OVERLAP>0 will +/// randomly introduce OVERLAP elements from the previous round, so an even round with OVERLAP=2 will contain +/// something like [1, 7, 2, 4, 0, ...]. Note that the total number of nodes will always be COUNT/2, so +/// for OVERLAP>0 a random number of nodes which would have been in the round for OVERLAP=0 will be dropped. +/// Ordering of nodes is random. Outputs is deterministic when prev_rng and this_rng are provided by make_rngs +/// using the same values for SEED and ROUND. +pub struct StableQuorumIterator { + /// PRNG from the previous round + prev_rng: NonRepeatValueIterator, + + /// PRNG for the current round + this_rng: NonRepeatValueIterator, + + /// Current ROUND + round: u64, + + /// Count of nodes in the source quorum being filtered against + count: u64, + + /// OVERLAP of nodes to be carried over from the previous round + overlap: u64, + + /// The next call to next() will emit the value with this index. Starts at 0 and is incremented for each + /// call to next() + index: u64, +} + +/// Determines how many possible values can be made for the given odd/even +/// E.g. if count is 5, then possible values would be [0, 1, 2, 3, 4] +/// if odd = true, slots = 2 (1 or 3), else slots = 3 (0, 2, 4) +fn calc_num_slots(count: u64, odd: bool) -> u64 { + (count / 2) + if odd { count % 2 } else { 0 } +} + +impl StableQuorumIterator { + /// Create a new StableQuorumIterator + pub fn new(seed: u64, round: u64, count: u64, overlap: u64) -> Self { + assert!( + count / 2 > overlap, + "Overlap cannot be greater than the entire set size" + ); + + let (prev_rng, this_rng) = make_rngs(seed, round); + + Self { + prev_rng: NonRepeatValueIterator::new(prev_rng, calc_num_slots(count, round % 2 == 0)), + this_rng: NonRepeatValueIterator::new(this_rng, calc_num_slots(count, round % 2 == 1)), + round, + count, + overlap, + index: 0, + } + } +} + +impl Iterator for StableQuorumIterator { + type Item = u64; + + fn next(&mut self) -> Option { + if self.index >= (self.count / 2) { + None + } else if self.index < self.overlap { + // Generate enough values for the previous round + let v = self.prev_rng.next().unwrap(); + self.index += 1; + Some(v * 2 + self.round % 2) + } else { + // Generate new values + let v = self.this_rng.next().unwrap(); + self.index += 1; + Some(v * 2 + (1 - self.round % 2)) + } + } +} + +/// Helper function to convert the arguments to a StableQuorumIterator into an ordered set of values. +pub fn stable_quorum_filter(seed: u64, round: u64, count: usize, overlap: u64) -> BTreeSet { + StableQuorumIterator::new(seed, round, count as u64, overlap) + // We should never have more than u32_max members in a test + .map(|x| usize::try_from(x).unwrap()) + .collect() +} + +/// Constructs a quorum with a random number of members and overlaps. Functions similar to StableQuorumIterator, +/// except that the number of MEMBERS and OVERLAP are also (deterministically) random, to allow additional variance +/// in testing. +pub struct RandomOverlapQuorumIterator { + /// PRNG from the previous round + prev_rng: NonRepeatValueIterator, + + /// PRNG for the current round + this_rng: NonRepeatValueIterator, + + /// Current ROUND + round: u64, + + /// Number of members to emit for the current round + members: u64, + + /// OVERLAP of nodes to be carried over from the previous round + overlap: u64, + + /// The next call to next() will emit the value with this index. Starts at 0 and is incremented for each + /// call to next() + index: u64, +} + +impl RandomOverlapQuorumIterator { + /// Create a new RandomOverlapQuorumIterator + pub fn new( + seed: u64, + round: u64, + count: u64, + members_min: u64, + members_max: u64, + overlap_min: u64, + overlap_max: u64, + ) -> Self { + assert!( + members_min <= members_max, + "Members_min cannot be greater than members_max" + ); + assert!( + overlap_min <= overlap_max, + "Overlap_min cannot be greater than overlap_max" + ); + assert!( + overlap_max < members_min, + "Overlap_max must be less than members_min" + ); + assert!( + count / 2 > overlap_max, + "Overlap cannot be greater than the entire set size" + ); + + let (mut prev_rng, mut this_rng) = make_rngs(seed, round); + + // Consume two values from prev_rng to advance it to the same state it was at the beginning of the previous round + let _prev_members = prev_rng.gen_range(members_min..=members_max); + let _prev_overlap = prev_rng.gen_range(overlap_min..=overlap_max); + let this_members = this_rng.gen_range(members_min..=members_max); + let this_overlap = this_rng.gen_range(overlap_min..=overlap_max); + + Self { + prev_rng: NonRepeatValueIterator::new(prev_rng, calc_num_slots(count, round % 2 == 0)), + this_rng: NonRepeatValueIterator::new(this_rng, calc_num_slots(count, round % 2 == 1)), + round, + members: this_members, + overlap: this_overlap, + index: 0, + } + } +} + +impl Iterator for RandomOverlapQuorumIterator { + type Item = u64; + + fn next(&mut self) -> Option { + if self.index >= self.members { + None + } else if self.index < self.overlap { + // Generate enough values for the previous round + let v = self.prev_rng.next().unwrap(); + self.index += 1; + Some(v * 2 + self.round % 2) + } else { + // Generate new values + let v = self.this_rng.next().unwrap(); + self.index += 1; + Some(v * 2 + (1 - self.round % 2)) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_stable() { + for _ in 0..100 { + let seed = rand::random::(); + let prev_set: Vec = StableQuorumIterator::new(seed, 1, 10, 2).collect(); + let this_set: Vec = StableQuorumIterator::new(seed, 2, 10, 2).collect(); + + // The first two elements from prev_set are from its previous round. But its 2nd and 3rd elements + // are new, and should be carried over to become the first two elements from this_set. + assert_eq!( + prev_set[2..4], + this_set[0..2], + "prev_set={prev_set:?}, this_set={this_set:?}" + ); + } + } + + #[test] + fn test_random_overlap() { + for _ in 0..100 { + let seed = rand::random::(); + let prev_set: Vec = + RandomOverlapQuorumIterator::new(seed, 1, 20, 5, 10, 2, 3).collect(); + let this_set: Vec = + RandomOverlapQuorumIterator::new(seed, 2, 20, 5, 10, 2, 3).collect(); + + // Similar to the overlap before, but there are 4 possible cases: the previous set might have had + // either 2 or 3 overlaps, meaning we should start with index 2 or 3, and the overlap size might + // be either 2 or 3. We'll just check for 2 overlaps, meaning we have two possible overlap cases + // to verify. + let matched = (prev_set[2..4] == this_set[0..2]) || (prev_set[3..5] == this_set[0..2]); + assert!(matched, "prev_set={prev_set:?}, this_set={this_set:?}"); + } + } +} diff --git a/crates/hotshot/src/traits/election.rs b/crates/hotshot/src/traits/election/mod.rs similarity index 80% rename from crates/hotshot/src/traits/election.rs rename to crates/hotshot/src/traits/election/mod.rs index 4f9212705f..8020aa9d08 100644 --- a/crates/hotshot/src/traits/election.rs +++ b/crates/hotshot/src/traits/election/mod.rs @@ -8,7 +8,15 @@ /// leader completely randomized every view pub mod randomized_committee; + +/// quorum randomized every view, with configurable overlap +pub mod randomized_committee_members; + /// static (round robin) committee election pub mod static_committee; + /// static (round robin leader for 2 consecutive views) committee election pub mod static_committee_leader_two_views; + +/// general helpers +mod helpers; diff --git a/crates/hotshot/src/traits/election/randomized_committee_members.rs b/crates/hotshot/src/traits/election/randomized_committee_members.rs new file mode 100644 index 0000000000..ab8f8a534e --- /dev/null +++ b/crates/hotshot/src/traits/election/randomized_committee_members.rs @@ -0,0 +1,250 @@ +// Copyright (c) 2021-2024 Espresso Systems (espressosys.com) +// This file is part of the HotShot repository. + +// You should have received a copy of the MIT License +// along with the HotShot repository. If not, see . + +use std::{ + cmp::max, + collections::{BTreeMap, BTreeSet}, + num::NonZeroU64, +}; + +use ethereum_types::U256; +use hotshot_types::{ + traits::{ + election::Membership, + network::Topic, + node_implementation::{ConsensusTime, NodeType}, + signature_key::{SignatureKey, StakeTableEntryType}, + }, + PeerConfig, +}; +use rand::{rngs::StdRng, Rng}; +use utils::anytrace::Result; + +use super::helpers::stable_quorum_filter; + +#[derive(Clone, Debug, Eq, PartialEq, Hash)] + +/// The static committee election + +pub struct RandomizedCommitteeMembers { + /// The nodes eligible for leadership. + /// NOTE: This is currently a hack because the DA leader needs to be the quorum + /// leader but without voting rights. + eligible_leaders: Vec<::StakeTableEntry>, + + /// The nodes on the committee and their stake + stake_table: Vec<::StakeTableEntry>, + + /// The nodes on the committee and their stake, indexed by public key + indexed_stake_table: + BTreeMap::StakeTableEntry>, + + /// The network topic of the committee + committee_topic: Topic, +} + +impl + RandomizedCommitteeMembers +{ + /// Creates a set of indices into the stake_table which reference the nodes selected for this epoch's committee + fn make_quorum_filter(&self, epoch: ::Epoch) -> BTreeSet { + stable_quorum_filter(SEED, epoch.u64(), self.stake_table.len(), OVERLAP) + } +} + +impl Membership + for RandomizedCommitteeMembers +{ + type Error = utils::anytrace::Error; + + /// Create a new election + fn new( + eligible_leaders: Vec::SignatureKey>>, + committee_members: Vec::SignatureKey>>, + committee_topic: Topic, + ) -> Self { + // The only time these two are unequal is in a benchmarking scenario that isn't covered by this test case + assert_eq!( + eligible_leaders, committee_members, + "eligible_leaders should be the same as committee_members!" + ); + + // For each eligible leader, get the stake table entry + let eligible_leaders: Vec<::StakeTableEntry> = + eligible_leaders + .iter() + .map(|member| member.stake_table_entry.clone()) + .filter(|entry| entry.stake() > U256::zero()) + .collect(); + + // For each member, get the stake table entry + let members: Vec<::StakeTableEntry> = + committee_members + .iter() + .map(|member| member.stake_table_entry.clone()) + .filter(|entry| entry.stake() > U256::zero()) + .collect(); + + // Index the stake table by public key + let indexed_stake_table: BTreeMap< + TYPES::SignatureKey, + ::StakeTableEntry, + > = members + .iter() + .map(|entry| (TYPES::SignatureKey::public_key(entry), entry.clone())) + .collect(); + + Self { + eligible_leaders, + stake_table: members, + indexed_stake_table, + committee_topic, + } + } + + /// Get the stake table for the current view + fn stake_table( + &self, + epoch: ::Epoch, + ) -> Vec<<::SignatureKey as SignatureKey>::StakeTableEntry> { + let filter = self.make_quorum_filter(epoch); + //self.stake_table.clone()s + self.stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| v.clone()) + .collect() + } + + /// Get all members of the committee for the current view + fn committee_members( + &self, + _view_number: ::View, + epoch: ::Epoch, + ) -> BTreeSet<::SignatureKey> { + let filter = self.make_quorum_filter(epoch); + self.stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| TYPES::SignatureKey::public_key(v)) + .collect() + } + + /// Get all eligible leaders of the committee for the current view + fn committee_leaders( + &self, + view_number: ::View, + epoch: ::Epoch, + ) -> BTreeSet<::SignatureKey> { + self.committee_members(view_number, epoch) + } + + /// Get the stake table entry for a public key + fn stake( + &self, + pub_key: &::SignatureKey, + epoch: ::Epoch, + ) -> Option<::StakeTableEntry> { + let filter = self.make_quorum_filter(epoch); + let actual_members: BTreeSet<_> = self + .stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| TYPES::SignatureKey::public_key(v)) + .collect(); + + if actual_members.contains(pub_key) { + // Only return the stake if it is above zero + self.indexed_stake_table.get(pub_key).cloned() + } else { + // Skip members which aren't included based on the quorum filter + None + } + } + + /// Check if a node has stake in the committee + fn has_stake( + &self, + pub_key: &::SignatureKey, + epoch: ::Epoch, + ) -> bool { + let filter = self.make_quorum_filter(epoch); + let actual_members: BTreeSet<_> = self + .stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| TYPES::SignatureKey::public_key(v)) + .collect(); + + if actual_members.contains(pub_key) { + self.indexed_stake_table + .get(pub_key) + .is_some_and(|x| x.stake() > U256::zero()) + } else { + // Skip members which aren't included based on the quorum filter + false + } + } + + /// 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, + view_number: TYPES::View, + epoch: ::Epoch, + ) -> Result { + let filter = self.make_quorum_filter(epoch); + let leader_vec: Vec<_> = self + .stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| v.clone()) + .collect(); + + let mut rng: StdRng = rand::SeedableRng::seed_from_u64(*view_number); + + let randomized_view_number: u64 = rng.gen_range(0..=u64::MAX); + #[allow(clippy::cast_possible_truncation)] + let index = randomized_view_number as usize % leader_vec.len(); + + let res = leader_vec[index].clone(); + + Ok(TYPES::SignatureKey::public_key(&res)) + } + + /// Get the total number of nodes in the committee + fn total_nodes(&self, epoch: ::Epoch) -> usize { + self.make_quorum_filter(epoch).len() + } + + /// Get the voting success threshold for the committee + fn success_threshold(&self) -> NonZeroU64 { + let len = self.stake_table.len() / 2; // Divide by two as we are flipping between odds and evens + NonZeroU64::new(((len as u64 * 2) / 3) + 1).unwrap() + } + + /// Get the voting failure threshold for the committee + fn failure_threshold(&self) -> NonZeroU64 { + let len = self.stake_table.len() / 2; // Divide by two as we are flipping between odds and evens + NonZeroU64::new(((len as u64) / 3) + 1).unwrap() + } + + /// Get the voting upgrade threshold for the committee + fn upgrade_threshold(&self) -> NonZeroU64 { + let len = self.stake_table.len() / 2; // Divide by two as we are flipping between odds and evens + + NonZeroU64::new(max((len as u64 * 9) / 10, ((len as u64 * 2) / 3) + 1)).unwrap() + } +} diff --git a/crates/hotshot/src/traits/networking/memory_network.rs b/crates/hotshot/src/traits/networking/memory_network.rs index d48f6f5f79..5925a85eff 100644 --- a/crates/hotshot/src/traits/networking/memory_network.rs +++ b/crates/hotshot/src/traits/networking/memory_network.rs @@ -88,7 +88,7 @@ struct MemoryNetworkInner { /// This provides an in memory simulation of a networking implementation, allowing nodes running on /// the same machine to mock networking while testing other functionality. /// -/// Under the hood, this simply maintains mpmc channels to every other `MemoryNetwork` insane of the +/// Under the hood, this simply maintains mpmc channels to every other `MemoryNetwork` instance of the /// same group. #[derive(Clone)] pub struct MemoryNetwork { @@ -297,22 +297,53 @@ impl ConnectedNetwork for MemoryNetwork { &self, message: Vec, recipients: Vec, - broadcast_delay: BroadcastDelay, + _broadcast_delay: BroadcastDelay, ) -> Result<(), NetworkError> { - // Iterate over all topics, compare to recipients, and get the `Topic` - let topic = self + trace!(?message, "Broadcasting message to DA"); + for node in self .inner .master_map .subscribed_map + .entry(Topic::Da) + .or_default() .iter() - .find(|v| v.value().iter().all(|(k, _)| recipients.contains(k))) - .map(|v| v.key().clone()) - .ok_or(NetworkError::MessageSendError( - "no topic found for recipients".to_string(), - ))?; - - self.broadcast_message(message, topic, broadcast_delay) - .await + { + if !recipients.contains(&node.0) { + tracing::error!("Skipping node because not in recipient list: {:?}", &node.0); + continue; + } + // TODO delay/drop etc here + let (key, node) = node; + trace!(?key, "Sending message to node"); + if let Some(ref config) = &self.inner.reliability_config { + { + let node2 = node.clone(); + let fut = config.chaos_send_msg( + message.clone(), + Arc::new(move |msg: Vec| { + let node3 = (node2).clone(); + boxed_sync(async move { + let _res = node3.input(msg).await; + // NOTE we're dropping metrics here but this is only for testing + // purposes. I think that should be okay + }) + }), + ); + spawn(fut); + } + } else { + let res = node.input(message.clone()).await; + match res { + Ok(()) => { + trace!(?key, "Delivered message to remote"); + } + Err(e) => { + warn!(?e, ?key, "Error sending broadcast message to node"); + } + } + } + } + Ok(()) } #[instrument(name = "MemoryNetwork::direct_message")] diff --git a/crates/macros/src/lib.rs b/crates/macros/src/lib.rs index e1920bf4fa..2666b8fbca 100644 --- a/crates/macros/src/lib.rs +++ b/crates/macros/src/lib.rs @@ -11,25 +11,41 @@ use proc_macro2::TokenStream as TokenStream2; use quote::{format_ident, quote}; use syn::{ parse::{Parse, ParseStream, Result}, - parse_macro_input, Expr, ExprArray, ExprPath, ExprTuple, Ident, LitBool, Token, + parse_macro_input, + punctuated::Punctuated, + Expr, ExprArray, ExprPath, ExprTuple, Ident, LitBool, Token, TypePath, }; +/// Bracketed types, e.g. [A, B, C] +/// These types can have generic parameters, whereas ExprArray items must be Expr. +#[derive(derive_builder::Builder, Debug, Clone)] +struct TypePathBracketedArray { + /// elems + pub elems: Punctuated, +} + /// description of a crosstest #[derive(derive_builder::Builder, Debug, Clone)] struct CrossTestData { /// imlementations impls: ExprArray, + /// builder impl #[builder(default = "syn::parse_str(\"[SimpleBuilderImplementation]\").unwrap()")] builder_impls: ExprArray, + /// versions versions: ExprArray, + /// types - types: ExprArray, + types: TypePathBracketedArray, + /// name of the test test_name: Ident, + /// test description/spec metadata: Expr, + /// whether or not to ignore ignore: LitBool, } @@ -51,17 +67,23 @@ impl CrossTestDataBuilder { #[derive(derive_builder::Builder, Debug, Clone)] struct TestData { /// type - ty: ExprPath, + ty: TypePath, + /// impl imply: ExprPath, + /// builder implementation builder_impl: ExprPath, + /// impl version: ExprPath, + /// name of test test_name: Ident, + /// test description metadata: Expr, + /// whether or not to ignore the test ignore: LitBool, } @@ -86,6 +108,20 @@ impl ToLowerSnakeStr for ExprPath { } } +impl ToLowerSnakeStr for TypePath { + fn to_lower_snake_str(&self) -> String { + self.path + .segments + .iter() + .fold(String::new(), |mut acc, s| { + acc.push_str(&s.ident.to_string().to_lowercase()); + acc.push('_'); + acc + }) + .to_lowercase() + } +} + impl ToLowerSnakeStr for ExprTuple { /// allow panic because this is a compiler error #[allow(clippy::panic)] @@ -149,6 +185,28 @@ mod keywords { syn::custom_keyword!(Versions); } +impl Parse for TypePathBracketedArray { + /// allow panic because this is a compiler error + #[allow(clippy::panic)] + fn parse(input: ParseStream<'_>) -> Result { + let content; + syn::bracketed!(content in input); + let mut elems = Punctuated::new(); + + while !content.is_empty() { + let first: TypePath = content.parse()?; + elems.push_value(first); + if content.is_empty() { + break; + } + let punct = content.parse()?; + elems.push_punct(punct); + } + + Ok(Self { elems }) + } +} + impl Parse for CrossTestData { /// allow panic because this is a compiler error #[allow(clippy::panic)] @@ -159,7 +217,7 @@ impl Parse for CrossTestData { if input.peek(keywords::Types) { let _ = input.parse::()?; input.parse::()?; - let types = input.parse::()?; + let types = input.parse::()?; //ExprArray>()?; description.types(types); } else if input.peek(keywords::Impls) { let _ = input.parse::()?; @@ -216,13 +274,8 @@ fn cross_tests_internal(test_spec: CrossTestData) -> TokenStream { }; p }); - // - let types = test_spec.types.elems.iter().map(|t| { - let Expr::Path(p) = t else { - panic!("Expected Path for Type! Got {t:?}"); - }; - p - }); + + let types = test_spec.types.elems.iter(); let versions = test_spec.versions.elems.iter().map(|t| { let Expr::Path(p) = t else { diff --git a/crates/task-impls/src/consensus/handlers.rs b/crates/task-impls/src/consensus/handlers.rs index 6dab4f938c..27fc0a7b43 100644 --- a/crates/task-impls/src/consensus/handlers.rs +++ b/crates/task-impls/src/consensus/handlers.rs @@ -286,7 +286,10 @@ pub(crate) async fn handle_timeout task_state .membership .has_stake(&task_state.public_key, task_state.cur_epoch), - debug!("We were not chosen for the consensus committee for view {view_number:?}") + debug!( + "We were not chosen for the consensus committee for view {:?}", + view_number + ) ); let vote = TimeoutVote::create_signed_vote( diff --git a/crates/testing/src/test_runner.rs b/crates/testing/src/test_runner.rs index 414e6e0f4b..ffee9b39e5 100644 --- a/crates/testing/src/test_runner.rs +++ b/crates/testing/src/test_runner.rs @@ -308,7 +308,7 @@ where for node in &mut *nodes { node.handle.shut_down().await; } - tracing::info!("Nodes shtudown"); + tracing::info!("Nodes shutdown"); completion_handle.abort(); diff --git a/crates/testing/tests/tests_1/block_builder.rs b/crates/testing/tests/tests_1/block_builder.rs index 5b0a6cf5c2..572741607a 100644 --- a/crates/testing/tests/tests_1/block_builder.rs +++ b/crates/testing/tests/tests_1/block_builder.rs @@ -21,14 +21,13 @@ use hotshot_testing::block_builder::{ use hotshot_types::{ network::RandomBuilderConfig, traits::{ - block_contents::vid_commitment, - node_implementation::{NodeType, Versions}, - signature_key::SignatureKey, + block_contents::vid_commitment, node_implementation::NodeType, signature_key::SignatureKey, BlockPayload, }, }; use tide_disco::Url; use tokio::time::sleep; +use vbs::version::StaticVersion; #[cfg(test)] #[tokio::test(flavor = "multi_thread")] @@ -50,8 +49,7 @@ async fn test_random_block_builder() { let builder_started = Instant::now(); - let client: BuilderClient::Base> = - BuilderClient::new(api_url); + let client: BuilderClient> = BuilderClient::new(api_url); assert!(client.connect(Duration::from_millis(100)).await); let (pub_key, private_key) = diff --git a/crates/testing/tests/tests_1/test_success.rs b/crates/testing/tests/tests_1/test_success.rs index e81060aedb..a40d865ed6 100644 --- a/crates/testing/tests/tests_1/test_success.rs +++ b/crates/testing/tests/tests_1/test_success.rs @@ -9,7 +9,7 @@ use std::time::Duration; use hotshot_example_types::{ node_types::{ EpochsTestVersions, Libp2pImpl, MemoryImpl, PushCdnImpl, TestConsecutiveLeaderTypes, - TestTypes, TestTypesRandomizedLeader, TestVersions, + TestTypes, TestTypesRandomizedCommitteeMembers, TestTypesRandomizedLeader, TestVersions, }, testable_delay::{DelayConfig, DelayOptions, DelaySettings, SupportedTraitTypesForAsyncDelay}, }; @@ -41,6 +41,25 @@ cross_tests!( }, ); +cross_tests!( + TestName: test_epoch_success, + Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], + Types: [TestTypes, TestTypesRandomizedLeader, TestTypesRandomizedCommitteeMembers<123, 2>], + Versions: [EpochsTestVersions], + Ignore: false, + Metadata: { + TestDescription { + // allow more time to pass in CI + completion_task_description: CompletionTaskDescription::TimeBasedCompletionTaskBuilder( + TimeBasedCompletionTaskDescription { + duration: Duration::from_secs(60), + }, + ), + ..TestDescription::default() + } + }, +); + cross_tests!( TestName: test_success_with_async_delay, Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], From e6fc75455dce47c547a5cf71a944b01280c0c319 Mon Sep 17 00:00:00 2001 From: pls148 <184445976+pls148@users.noreply.github.com> Date: Tue, 12 Nov 2024 15:01:07 -0800 Subject: [PATCH 07/10] add randomized overlap committee generator --- crates/example-types/src/node_types.rs | 25 +-- crates/hotshot/src/traits/election/helpers.rs | 178 +++++++++++++++++- crates/hotshot/src/traits/election/mod.rs | 2 +- .../election/randomized_committee_members.rs | 22 +-- crates/macros/src/lib.rs | 40 +++- crates/task-impls/src/quorum_proposal/mod.rs | 2 +- crates/testing/tests/tests_1/block_builder.rs | 2 +- crates/testing/tests/tests_1/test_success.rs | 38 ++-- 8 files changed, 255 insertions(+), 54 deletions(-) diff --git a/crates/example-types/src/node_types.rs b/crates/example-types/src/node_types.rs index b45e2e7377..01160e1cb9 100644 --- a/crates/example-types/src/node_types.rs +++ b/crates/example-types/src/node_types.rs @@ -4,9 +4,14 @@ // You should have received a copy of the MIT License // along with the HotShot repository. If not, see . +use std::marker::PhantomData; + +pub use hotshot::traits::election::helpers::{ + RandomOverlapQuorumFilterConfig, StableQuorumFilterConfig, +}; use hotshot::traits::{ election::{ - randomized_committee::RandomizedCommittee, + helpers::QuorumFilterConfig, randomized_committee::RandomizedCommittee, randomized_committee_members::RandomizedCommitteeMembers, static_committee::StaticCommittee, static_committee_leader_two_views::StaticCommitteeLeaderForTwoViews, @@ -104,10 +109,11 @@ impl NodeType for TestTypesRandomizedLeader { )] /// filler struct to implement node type and allow us /// to select our traits -pub struct TestTypesRandomizedCommitteeMembers; -impl NodeType - for TestTypesRandomizedCommitteeMembers -{ +pub struct TestTypesRandomizedCommitteeMembers { + _pd: PhantomData, +} + +impl NodeType for TestTypesRandomizedCommitteeMembers { type AuctionResult = TestAuctionResult; type View = ViewNumber; type Epoch = EpochNumber; @@ -117,11 +123,8 @@ impl NodeType type Transaction = TestTransaction; type ValidatedState = TestValidatedState; type InstanceState = TestInstanceState; - type Membership = RandomizedCommitteeMembers< - TestTypesRandomizedCommitteeMembers, - SEED, - OVERLAP, - >; + type Membership = + RandomizedCommitteeMembers, CONFIG>; type BuilderSignatureKey = BuilderKey; } @@ -261,7 +264,7 @@ impl Versions for EpochsTestVersions { 0, 0, ]; - type Marketplace = StaticVersion<0, 3>; + type Marketplace = StaticVersion<0, 99>; type Epochs = StaticVersion<0, 4>; } diff --git a/crates/hotshot/src/traits/election/helpers.rs b/crates/hotshot/src/traits/election/helpers.rs index 334458f57d..2a2c7fe172 100644 --- a/crates/hotshot/src/traits/election/helpers.rs +++ b/crates/hotshot/src/traits/election/helpers.rs @@ -4,7 +4,7 @@ // You should have received a copy of the MIT License // along with the HotShot repository. If not, see . -use std::collections::BTreeSet; +use std::{collections::BTreeSet, hash::Hash}; use rand::{rngs::StdRng, Rng, SeedableRng}; @@ -52,7 +52,11 @@ impl Iterator for NonRepeatValueIterator { } /// Create a single u64 seed by merging two u64s. Done this way to allow easy seeding of the number generator -/// from both a stable SOUND as well as a moving value ROUND (typically, epoch). +/// from both a stable SOUND as well as a moving value ROUND (typically, epoch). Shift left by 8 to avoid +/// scenarios where someone manually stepping seeds would pass over the same space of random numbers across +/// sequential rounds. Doesn't have to be 8, but has to be large enough that it is unlikely that a given +/// test run will collide; using 8 means that 256 rounds (epochs) would have to happen inside of a test before +/// the test starts repeating values from SEED+1. fn make_seed(seed: u64, round: u64) -> u64 { seed.wrapping_add(round.wrapping_shl(8)) } @@ -98,11 +102,16 @@ pub struct StableQuorumIterator { /// E.g. if count is 5, then possible values would be [0, 1, 2, 3, 4] /// if odd = true, slots = 2 (1 or 3), else slots = 3 (0, 2, 4) fn calc_num_slots(count: u64, odd: bool) -> u64 { - (count / 2) + if odd { count % 2 } else { 0 } + (count / 2) + if odd { 0 } else { count % 2 } } impl StableQuorumIterator { + #[must_use] /// Create a new StableQuorumIterator + /// + /// # Panics + /// + /// panics if overlap is greater than half of count pub fn new(seed: u64, round: u64, count: u64, overlap: u64) -> Self { assert!( count / 2 > overlap, @@ -127,22 +136,33 @@ impl Iterator for StableQuorumIterator { fn next(&mut self) -> Option { if self.index >= (self.count / 2) { + // Always return exactly half of the possible values. If we have OVERLAP>0 then + // we need to return (COUNT/2)-OVERLAP of the current set, even if there are additional + // even (or odd) numbers that we can return. None } else if self.index < self.overlap { - // Generate enough values for the previous round + // Generate enough values for the previous round. If the current round is odd, then + // we want to pick even values that were selected from the previous round to create OVERLAP + // even values. let v = self.prev_rng.next().unwrap(); self.index += 1; - Some(v * 2 + self.round % 2) + Some(v * 2 + (1 - self.round % 2)) } else { - // Generate new values + // Generate new values. If our current round is odd, we'll be creating (COUNT/2)-OVERLAP + // odd values here. let v = self.this_rng.next().unwrap(); self.index += 1; - Some(v * 2 + (1 - self.round % 2)) + Some(v * 2 + self.round % 2) } } } +#[must_use] /// Helper function to convert the arguments to a StableQuorumIterator into an ordered set of values. +/// +/// # Panics +/// +/// panics if the arguments are invalid for StableQuorumIterator::new pub fn stable_quorum_filter(seed: u64, round: u64, count: usize, overlap: u64) -> BTreeSet { StableQuorumIterator::new(seed, round, count as u64, overlap) // We should never have more than u32_max members in a test @@ -175,7 +195,12 @@ pub struct RandomOverlapQuorumIterator { } impl RandomOverlapQuorumIterator { + #[must_use] /// Create a new RandomOverlapQuorumIterator + /// + /// # Panics + /// + /// panics if overlap and members can produce invalid results or if ranges are invalid pub fn new( seed: u64, round: u64, @@ -231,16 +256,109 @@ impl Iterator for RandomOverlapQuorumIterator { // Generate enough values for the previous round let v = self.prev_rng.next().unwrap(); self.index += 1; - Some(v * 2 + self.round % 2) + Some(v * 2 + (1 - self.round % 2)) } else { // Generate new values let v = self.this_rng.next().unwrap(); self.index += 1; - Some(v * 2 + (1 - self.round % 2)) + Some(v * 2 + self.round % 2) } } } +#[must_use] +/// Helper function to convert the arguments to a StableQuorumIterator into an ordered set of values. +/// +/// # Panics +/// +/// panics if the arguments are invalid for RandomOverlapQuorumIterator::new +pub fn random_overlap_quorum_filter( + seed: u64, + round: u64, + count: usize, + members_min: u64, + members_max: u64, + overlap_min: u64, + overlap_max: u64, +) -> BTreeSet { + RandomOverlapQuorumIterator::new( + seed, + round, + count as u64, + members_min, + members_max, + overlap_min, + overlap_max, + ) + // We should never have more than u32_max members in a test + .map(|x| usize::try_from(x).unwrap()) + .collect() +} + +/// Trait wrapping a config for quorum filters. This allows selection between either the StableQuorumIterator or the +/// RandomOverlapQuorumIterator functionality from above +pub trait QuorumFilterConfig: + Copy + + Clone + + std::fmt::Debug + + Default + + Send + + Sync + + Ord + + PartialOrd + + Eq + + PartialEq + + Hash + + 'static +{ + /// Called to run the filter and return a set of indices + fn execute(epoch: u64, count: usize) -> BTreeSet; +} + +#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +/// Provides parameters to use the StableQuorumIterator +pub struct StableQuorumFilterConfig {} + +impl QuorumFilterConfig + for StableQuorumFilterConfig +{ + fn execute(epoch: u64, count: usize) -> BTreeSet { + stable_quorum_filter(SEED, epoch, count, OVERLAP) + } +} + +#[derive(Debug, Copy, Clone, Default, Eq, PartialEq, Hash, Ord, PartialOrd)] +/// Provides parameters to use the RandomOverlapQuorumIterator +pub struct RandomOverlapQuorumFilterConfig< + const SEED: u64, + const MEMBERS_MIN: u64, + const MEMBERS_MAX: u64, + const OVERLAP_MIN: u64, + const OVERLAP_MAX: u64, +> {} + +impl< + const SEED: u64, + const MEMBERS_MIN: u64, + const MEMBERS_MAX: u64, + const OVERLAP_MIN: u64, + const OVERLAP_MAX: u64, + > QuorumFilterConfig + for RandomOverlapQuorumFilterConfig +{ + fn execute(epoch: u64, count: usize) -> BTreeSet { + random_overlap_quorum_filter( + SEED, + epoch, + count, + MEMBERS_MIN, + MEMBERS_MAX, + OVERLAP_MIN, + OVERLAP_MAX, + ) + } +} + #[cfg(test)] mod tests { use super::*; @@ -279,4 +397,46 @@ mod tests { assert!(matched, "prev_set={prev_set:?}, this_set={this_set:?}"); } } + + #[test] + fn test_odd_even() { + for _ in 0..100 { + let seed = rand::random::(); + + let odd_set: Vec = StableQuorumIterator::new(seed, 1, 10, 2).collect(); + let even_set: Vec = StableQuorumIterator::new(seed, 2, 10, 2).collect(); + + assert!( + odd_set[2] % 2 == 1, + "odd set non-overlap value should be odd (stable)" + ); + assert!( + even_set[2] % 2 == 0, + "even set non-overlap value should be even (stable)" + ); + + let odd_set: Vec = + RandomOverlapQuorumIterator::new(seed, 1, 20, 5, 10, 2, 3).collect(); + let even_set: Vec = + RandomOverlapQuorumIterator::new(seed, 2, 20, 5, 10, 2, 3).collect(); + + assert!( + odd_set[3] % 2 == 1, + "odd set non-overlap value should be odd (random overlap)" + ); + assert!( + even_set[3] % 2 == 0, + "even set non-overlap value should be even (random overlap)" + ); + } + } + + #[test] + fn calc_num_slots_test() { + assert_eq!(calc_num_slots(5, true), 2); + assert_eq!(calc_num_slots(5, false), 3); + + assert_eq!(calc_num_slots(6, true), 3); + assert_eq!(calc_num_slots(6, false), 3); + } } diff --git a/crates/hotshot/src/traits/election/mod.rs b/crates/hotshot/src/traits/election/mod.rs index 8020aa9d08..914b9bbb33 100644 --- a/crates/hotshot/src/traits/election/mod.rs +++ b/crates/hotshot/src/traits/election/mod.rs @@ -19,4 +19,4 @@ pub mod static_committee; pub mod static_committee_leader_two_views; /// general helpers -mod helpers; +pub mod helpers; diff --git a/crates/hotshot/src/traits/election/randomized_committee_members.rs b/crates/hotshot/src/traits/election/randomized_committee_members.rs index ab8f8a534e..ec0ef1231b 100644 --- a/crates/hotshot/src/traits/election/randomized_committee_members.rs +++ b/crates/hotshot/src/traits/election/randomized_committee_members.rs @@ -7,6 +7,7 @@ use std::{ cmp::max, collections::{BTreeMap, BTreeSet}, + marker::PhantomData, num::NonZeroU64, }; @@ -23,13 +24,11 @@ use hotshot_types::{ use rand::{rngs::StdRng, Rng}; use utils::anytrace::Result; -use super::helpers::stable_quorum_filter; +use crate::traits::election::helpers::QuorumFilterConfig; #[derive(Clone, Debug, Eq, PartialEq, Hash)] - /// The static committee election - -pub struct RandomizedCommitteeMembers { +pub struct RandomizedCommitteeMembers { /// The nodes eligible for leadership. /// NOTE: This is currently a hack because the DA leader needs to be the quorum /// leader but without voting rights. @@ -44,19 +43,20 @@ pub struct RandomizedCommitteeMembers, } -impl - RandomizedCommitteeMembers -{ +impl RandomizedCommitteeMembers { /// Creates a set of indices into the stake_table which reference the nodes selected for this epoch's committee fn make_quorum_filter(&self, epoch: ::Epoch) -> BTreeSet { - stable_quorum_filter(SEED, epoch.u64(), self.stake_table.len(), OVERLAP) + CONFIG::execute(epoch.u64(), self.stake_table.len()) } } -impl Membership - for RandomizedCommitteeMembers +impl Membership + for RandomizedCommitteeMembers { type Error = utils::anytrace::Error; @@ -102,6 +102,7 @@ impl Membership stake_table: members, indexed_stake_table, committee_topic, + _pd: PhantomData, } } @@ -244,7 +245,6 @@ impl Membership /// Get the voting upgrade threshold for the committee fn upgrade_threshold(&self) -> NonZeroU64 { let len = self.stake_table.len() / 2; // Divide by two as we are flipping between odds and evens - NonZeroU64::new(max((len as u64 * 9) / 10, ((len as u64 * 2) / 3) + 1)).unwrap() } } diff --git a/crates/macros/src/lib.rs b/crates/macros/src/lib.rs index 2666b8fbca..80f49e1d6a 100644 --- a/crates/macros/src/lib.rs +++ b/crates/macros/src/lib.rs @@ -13,7 +13,7 @@ use syn::{ parse::{Parse, ParseStream, Result}, parse_macro_input, punctuated::Punctuated, - Expr, ExprArray, ExprPath, ExprTuple, Ident, LitBool, Token, TypePath, + Expr, ExprArray, ExprPath, ExprTuple, Ident, LitBool, PathArguments, Token, TypePath, }; /// Bracketed types, e.g. [A, B, C] @@ -108,6 +108,37 @@ impl ToLowerSnakeStr for ExprPath { } } +impl ToLowerSnakeStr for syn::GenericArgument { + /// allow panic because this is a compiler error + #[allow(clippy::panic)] + fn to_lower_snake_str(&self) -> String { + match self { + syn::GenericArgument::Lifetime(l) => l.ident.to_string().to_lowercase(), + syn::GenericArgument::Type(t) => match t { + syn::Type::Path(p) => p.to_lower_snake_str(), + _ => { + panic!("Unexpected type for GenericArgument::Type: {t:?}"); + } + }, + syn::GenericArgument::Const(c) => match c { + syn::Expr::Lit(l) => match &l.lit { + syn::Lit::Str(v) => format!("{}_", v.value().to_lowercase()), + syn::Lit::Int(v) => format!("{}_", v.base10_digits()), + _ => { + panic!("Unexpected type for GenericArgument::Const::Lit: {l:?}"); + } + }, + _ => { + panic!("Unexpected type for GenericArgument::Const: {c:?}"); + } + }, + _ => { + panic!("Unexpected type for GenericArgument: {self:?}"); + } + } + } +} + impl ToLowerSnakeStr for TypePath { fn to_lower_snake_str(&self) -> String { self.path @@ -115,6 +146,13 @@ impl ToLowerSnakeStr for TypePath { .iter() .fold(String::new(), |mut acc, s| { acc.push_str(&s.ident.to_string().to_lowercase()); + if let PathArguments::AngleBracketed(a) = &s.arguments { + acc.push('_'); + for arg in &a.args { + acc.push_str(&arg.to_lower_snake_str()); + } + } + acc.push('_'); acc }) diff --git a/crates/task-impls/src/quorum_proposal/mod.rs b/crates/task-impls/src/quorum_proposal/mod.rs index 034db12812..79299119fb 100644 --- a/crates/task-impls/src/quorum_proposal/mod.rs +++ b/crates/task-impls/src/quorum_proposal/mod.rs @@ -512,7 +512,7 @@ impl, V: Versions> &self.upgrade_lock ) .await, - warn!("Qurom certificate {:?} was invalid", qc.data()) + warn!("Quorum certificate {:?} was invalid", qc.data()) ); self.highest_qc = qc.clone(); } diff --git a/crates/testing/tests/tests_1/block_builder.rs b/crates/testing/tests/tests_1/block_builder.rs index 572741607a..fc29b1c01d 100644 --- a/crates/testing/tests/tests_1/block_builder.rs +++ b/crates/testing/tests/tests_1/block_builder.rs @@ -12,7 +12,7 @@ use std::{ use hotshot_builder_api::v0_1::block_info::AvailableBlockData; use hotshot_example_types::{ block_types::{TestBlockPayload, TestMetadata, TestTransaction}, - node_types::{TestTypes, TestVersions}, + node_types::TestTypes, }; use hotshot_task_impls::builder::{BuilderClient, BuilderClientError}; use hotshot_testing::block_builder::{ diff --git a/crates/testing/tests/tests_1/test_success.rs b/crates/testing/tests/tests_1/test_success.rs index a40d865ed6..982b7018f6 100644 --- a/crates/testing/tests/tests_1/test_success.rs +++ b/crates/testing/tests/tests_1/test_success.rs @@ -9,7 +9,7 @@ use std::time::Duration; use hotshot_example_types::{ node_types::{ EpochsTestVersions, Libp2pImpl, MemoryImpl, PushCdnImpl, TestConsecutiveLeaderTypes, - TestTypes, TestTypesRandomizedCommitteeMembers, TestTypesRandomizedLeader, TestVersions, + TestTypes, TestTypesRandomizedLeader, TestVersions, }, testable_delay::{DelayConfig, DelayOptions, DelaySettings, SupportedTraitTypesForAsyncDelay}, }; @@ -41,24 +41,24 @@ cross_tests!( }, ); -cross_tests!( - TestName: test_epoch_success, - Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], - Types: [TestTypes, TestTypesRandomizedLeader, TestTypesRandomizedCommitteeMembers<123, 2>], - Versions: [EpochsTestVersions], - Ignore: false, - Metadata: { - TestDescription { - // allow more time to pass in CI - completion_task_description: CompletionTaskDescription::TimeBasedCompletionTaskBuilder( - TimeBasedCompletionTaskDescription { - duration: Duration::from_secs(60), - }, - ), - ..TestDescription::default() - } - }, -); +// cross_tests!( +// TestName: test_epoch_success, +// Impls: [MemoryImpl, Libp2pImpl, PushCdnImpl], +// Types: [TestTypes, TestTypesRandomizedLeader, TestTypesRandomizedCommitteeMembers>, TestTypesRandomizedCommitteeMembers>], +// Versions: [EpochsTestVersions], +// Ignore: false, +// Metadata: { +// TestDescription { +// // allow more time to pass in CI +// completion_task_description: CompletionTaskDescription::TimeBasedCompletionTaskBuilder( +// TimeBasedCompletionTaskDescription { +// duration: Duration::from_secs(60), +// }, +// ), +// ..TestDescription::default() +// } +// }, +// ); cross_tests!( TestName: test_success_with_async_delay, From 7efc255250c3800254399cddad104616a47a2f92 Mon Sep 17 00:00:00 2001 From: pls148 <184445976+pls148@users.noreply.github.com> Date: Wed, 13 Nov 2024 16:03:00 -0800 Subject: [PATCH 08/10] Add epoch to threshold functions to allow dynamically sized committees --- .../traits/election/randomized_committee.rs | 6 ++-- .../election/randomized_committee_members.rs | 14 ++++---- .../src/traits/election/static_committee.rs | 6 ++-- .../static_committee_leader_two_views.rs | 6 ++-- crates/testing/src/helpers.rs | 2 +- crates/types/src/simple_certificate.rs | 35 +++++++++++++------ crates/types/src/traits/election.rs | 6 ++-- crates/types/src/vote.rs | 9 +++-- 8 files changed, 51 insertions(+), 33 deletions(-) diff --git a/crates/hotshot/src/traits/election/randomized_committee.rs b/crates/hotshot/src/traits/election/randomized_committee.rs index 2b721a66e0..d520bcef0d 100644 --- a/crates/hotshot/src/traits/election/randomized_committee.rs +++ b/crates/hotshot/src/traits/election/randomized_committee.rs @@ -226,7 +226,7 @@ impl Membership for RandomizedCommittee { self.da_stake_table.len() } /// Get the voting success threshold for the committee - fn success_threshold(&self) -> NonZeroU64 { + fn success_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64 * 2) / 3) + 1).unwrap() } @@ -236,12 +236,12 @@ impl Membership for RandomizedCommittee { } /// Get the voting failure threshold for the committee - fn failure_threshold(&self) -> NonZeroU64 { + fn failure_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64) / 3) + 1).unwrap() } /// Get the voting upgrade threshold for the committee - fn upgrade_threshold(&self) -> NonZeroU64 { + fn upgrade_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(max( (self.stake_table.len() as u64 * 9) / 10, ((self.stake_table.len() as u64 * 2) / 3) + 1, diff --git a/crates/hotshot/src/traits/election/randomized_committee_members.rs b/crates/hotshot/src/traits/election/randomized_committee_members.rs index ec0ef1231b..1bd3d1d54d 100644 --- a/crates/hotshot/src/traits/election/randomized_committee_members.rs +++ b/crates/hotshot/src/traits/election/randomized_committee_members.rs @@ -11,7 +11,6 @@ use std::{ num::NonZeroU64, }; -use ethereum_types::U256; use hotshot_types::{ traits::{ election::Membership, @@ -21,6 +20,7 @@ use hotshot_types::{ }, PeerConfig, }; +use primitive_types::U256; use rand::{rngs::StdRng, Rng}; use utils::anytrace::Result; @@ -231,20 +231,20 @@ impl Membership } /// Get the voting success threshold for the committee - fn success_threshold(&self) -> NonZeroU64 { - let len = self.stake_table.len() / 2; // Divide by two as we are flipping between odds and evens + fn success_threshold(&self, epoch: ::Epoch) -> NonZeroU64 { + let len = self.total_nodes(epoch); NonZeroU64::new(((len as u64 * 2) / 3) + 1).unwrap() } /// Get the voting failure threshold for the committee - fn failure_threshold(&self) -> NonZeroU64 { - let len = self.stake_table.len() / 2; // Divide by two as we are flipping between odds and evens + fn failure_threshold(&self, epoch: ::Epoch) -> NonZeroU64 { + let len = self.total_nodes(epoch); NonZeroU64::new(((len as u64) / 3) + 1).unwrap() } /// Get the voting upgrade threshold for the committee - fn upgrade_threshold(&self) -> NonZeroU64 { - let len = self.stake_table.len() / 2; // Divide by two as we are flipping between odds and evens + fn upgrade_threshold(&self, epoch: ::Epoch) -> NonZeroU64 { + let len = self.total_nodes(epoch); NonZeroU64::new(max((len as u64 * 9) / 10, ((len as u64 * 2) / 3) + 1)).unwrap() } } diff --git a/crates/hotshot/src/traits/election/static_committee.rs b/crates/hotshot/src/traits/election/static_committee.rs index fa904c66cf..0324b64350 100644 --- a/crates/hotshot/src/traits/election/static_committee.rs +++ b/crates/hotshot/src/traits/election/static_committee.rs @@ -215,7 +215,7 @@ impl Membership for StaticCommittee { } /// Get the voting success threshold for the committee - fn success_threshold(&self) -> NonZeroU64 { + fn success_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64 * 2) / 3) + 1).unwrap() } @@ -225,12 +225,12 @@ impl Membership for StaticCommittee { } /// Get the voting failure threshold for the committee - fn failure_threshold(&self) -> NonZeroU64 { + fn failure_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64) / 3) + 1).unwrap() } /// Get the voting upgrade threshold for the committee - fn upgrade_threshold(&self) -> NonZeroU64 { + fn upgrade_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(max( (self.stake_table.len() as u64 * 9) / 10, ((self.stake_table.len() as u64 * 2) / 3) + 1, 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 41ed1d046e..4b8380621d 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 @@ -217,7 +217,7 @@ impl Membership for StaticCommitteeLeaderForTwoViews NonZeroU64 { + fn success_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64 * 2) / 3) + 1).unwrap() } @@ -227,12 +227,12 @@ impl Membership for StaticCommitteeLeaderForTwoViews NonZeroU64 { + fn failure_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64) / 3) + 1).unwrap() } /// Get the voting upgrade threshold for the committee - fn upgrade_threshold(&self) -> NonZeroU64 { + fn upgrade_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.stake_table.len() as u64 * 9) / 10) + 1).unwrap() } } diff --git a/crates/testing/src/helpers.rs b/crates/testing/src/helpers.rs index 3f7b300f48..cc468a1859 100644 --- a/crates/testing/src/helpers.rs +++ b/crates/testing/src/helpers.rs @@ -215,7 +215,7 @@ pub async fn build_assembled_sig< let real_qc_pp: ::QcParams = ::public_parameter( stake_table.clone(), - U256::from(CERT::threshold(membership)), + U256::from(CERT::threshold(membership, epoch)), ); let total_nodes = stake_table.len(); let signers = bitvec![1; total_nodes]; diff --git a/crates/types/src/simple_certificate.rs b/crates/types/src/simple_certificate.rs index 271d5b0729..8d544b5cdb 100644 --- a/crates/types/src/simple_certificate.rs +++ b/crates/types/src/simple_certificate.rs @@ -39,7 +39,10 @@ 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) -> u64; + fn threshold>( + membership: &MEMBERSHIP, + epoch: ::Epoch, + ) -> u64; } /// Defines a threshold which is 2f + 1 (Amount needed for Quorum) @@ -47,8 +50,11 @@ pub trait Threshold { pub struct SuccessThreshold {} impl Threshold for SuccessThreshold { - fn threshold>(membership: &MEMBERSHIP) -> u64 { - membership.success_threshold().into() + fn threshold>( + membership: &MEMBERSHIP, + epoch: ::Epoch, + ) -> u64 { + membership.success_threshold(epoch).into() } } @@ -57,8 +63,11 @@ impl Threshold for SuccessThreshold { pub struct OneHonestThreshold {} impl Threshold for OneHonestThreshold { - fn threshold>(membership: &MEMBERSHIP) -> u64 { - membership.failure_threshold().into() + fn threshold>( + membership: &MEMBERSHIP, + epoch: ::Epoch, + ) -> u64 { + membership.failure_threshold(epoch).into() } } @@ -67,8 +76,11 @@ impl Threshold for OneHonestThreshold { pub struct UpgradeThreshold {} impl Threshold for UpgradeThreshold { - fn threshold>(membership: &MEMBERSHIP) -> u64 { - membership.upgrade_threshold().into() + fn threshold>( + membership: &MEMBERSHIP, + epoch: ::Epoch, + ) -> u64 { + membership.upgrade_threshold(epoch).into() } } @@ -254,8 +266,11 @@ impl>(membership: &MEMBERSHIP) -> u64 { - THRESHOLD::threshold(membership) + fn threshold>( + membership: &MEMBERSHIP, + epoch: ::Epoch, + ) -> u64 { + THRESHOLD::threshold(membership, epoch) } fn stake_table_entry>( @@ -345,7 +360,7 @@ impl UpgradeCertificate { ensure!( cert.is_valid_cert( quorum_membership.stake_table(epoch), - quorum_membership.upgrade_threshold(), + quorum_membership.upgrade_threshold(epoch), upgrade_lock ) .await, diff --git a/crates/types/src/traits/election.rs b/crates/types/src/traits/election.rs index 1205de0437..94e456e73a 100644 --- a/crates/types/src/traits/election.rs +++ b/crates/types/src/traits/election.rs @@ -130,14 +130,14 @@ pub trait Membership: Clone + Debug + Send + Sync { fn da_total_nodes(&self, epoch: TYPES::Epoch) -> usize; /// Returns the threshold for a specific `Membership` implementation - fn success_threshold(&self) -> NonZeroU64; + fn success_threshold(&self, epoch: TYPES::Epoch) -> NonZeroU64; /// Returns the DA threshold for a specific `Membership` implementation fn da_success_threshold(&self) -> NonZeroU64; /// Returns the threshold for a specific `Membership` implementation - fn failure_threshold(&self) -> NonZeroU64; + fn failure_threshold(&self, epoch: TYPES::Epoch) -> NonZeroU64; /// Returns the threshold required to upgrade the network protocol - fn upgrade_threshold(&self) -> NonZeroU64; + fn upgrade_threshold(&self, epoch: TYPES::Epoch) -> NonZeroU64; } diff --git a/crates/types/src/vote.rs b/crates/types/src/vote.rs index bdff9d4bb5..13112afa12 100644 --- a/crates/types/src/vote.rs +++ b/crates/types/src/vote.rs @@ -81,7 +81,10 @@ 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) -> u64; + fn threshold>( + membership: &MEMBERSHIP, + epoch: ::Epoch, + ) -> u64; /// Get Stake Table from Membership implementation. fn stake_table>( @@ -220,12 +223,12 @@ impl< *total_stake_casted += stake_table_entry.stake(); total_vote_map.insert(key, (vote.signature(), vote_commitment)); - if *total_stake_casted >= CERT::threshold(membership).into() { + if *total_stake_casted >= CERT::threshold(membership, epoch).into() { // Assemble QC let real_qc_pp: <::SignatureKey as SignatureKey>::QcParams = ::public_parameter( stake_table, - U256::from(CERT::threshold(membership)), + U256::from(CERT::threshold(membership, epoch)), ); let real_qc_sig = ::assemble( From 957fcbbe031c3e765311439df8fb721b6896d605 Mon Sep 17 00:00:00 2001 From: pls148 <184445976+pls148@users.noreply.github.com> Date: Fri, 22 Nov 2024 13:19:53 -0800 Subject: [PATCH 09/10] rework randomized_committee_members to work with flattened Membership --- .../traits/election/randomized_committee.rs | 2 +- .../election/randomized_committee_members.rs | 161 ++++++++++++++++-- .../src/traits/election/static_committee.rs | 9 +- .../static_committee_leader_two_views.rs | 2 +- crates/task-impls/src/helpers.rs | 10 +- .../src/quorum_proposal/handlers.rs | 3 +- crates/task-impls/src/quorum_proposal/mod.rs | 4 +- .../src/quorum_proposal_recv/handlers.rs | 4 +- crates/task-impls/src/quorum_vote/mod.rs | 2 +- crates/task-impls/src/view_sync.rs | 6 +- crates/testing/tests/tests_1/message.rs | 4 +- crates/types/src/simple_certificate.rs | 11 +- crates/types/src/simple_vote.rs | 16 +- crates/types/src/traits/election.rs | 2 +- 14 files changed, 186 insertions(+), 50 deletions(-) diff --git a/crates/hotshot/src/traits/election/randomized_committee.rs b/crates/hotshot/src/traits/election/randomized_committee.rs index d520bcef0d..4046123553 100644 --- a/crates/hotshot/src/traits/election/randomized_committee.rs +++ b/crates/hotshot/src/traits/election/randomized_committee.rs @@ -231,7 +231,7 @@ impl Membership for RandomizedCommittee { } /// Get the voting success threshold for the committee - fn da_success_threshold(&self) -> NonZeroU64 { + fn da_success_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.da_stake_table.len() as u64 * 2) / 3) + 1).unwrap() } diff --git a/crates/hotshot/src/traits/election/randomized_committee_members.rs b/crates/hotshot/src/traits/election/randomized_committee_members.rs index 1bd3d1d54d..01f4ad6383 100644 --- a/crates/hotshot/src/traits/election/randomized_committee_members.rs +++ b/crates/hotshot/src/traits/election/randomized_committee_members.rs @@ -14,7 +14,6 @@ use std::{ use hotshot_types::{ traits::{ election::Membership, - network::Topic, node_implementation::{ConsensusTime, NodeType}, signature_key::{SignatureKey, StakeTableEntryType}, }, @@ -37,12 +36,16 @@ pub struct RandomizedCommitteeMembers { /// The nodes on the committee and their stake stake_table: Vec<::StakeTableEntry>, + /// The nodes on the da committee and their stake + da_stake_table: Vec<::StakeTableEntry>, + /// The nodes on the committee and their stake, indexed by public key indexed_stake_table: BTreeMap::StakeTableEntry>, - /// The network topic of the committee - committee_topic: Topic, + /// The nodes on the da committee and their stake, indexed by public key + indexed_da_stake_table: + BTreeMap::StakeTableEntry>, /// Phantom _pd: PhantomData, @@ -53,6 +56,11 @@ impl RandomizedCommitteeMembers::Epoch) -> BTreeSet { CONFIG::execute(epoch.u64(), self.stake_table.len()) } + + /// Creates a set of indices into the da_stake_table which reference the nodes selected for this epoch's da committee + fn make_da_quorum_filter(&self, epoch: ::Epoch) -> BTreeSet { + CONFIG::execute(epoch.u64(), self.da_stake_table.len()) + } } impl Membership @@ -62,19 +70,12 @@ impl Membership /// Create a new election fn new( - eligible_leaders: Vec::SignatureKey>>, committee_members: Vec::SignatureKey>>, - committee_topic: Topic, + da_members: Vec::SignatureKey>>, ) -> Self { - // The only time these two are unequal is in a benchmarking scenario that isn't covered by this test case - assert_eq!( - eligible_leaders, committee_members, - "eligible_leaders should be the same as committee_members!" - ); - // For each eligible leader, get the stake table entry let eligible_leaders: Vec<::StakeTableEntry> = - eligible_leaders + committee_members .iter() .map(|member| member.stake_table_entry.clone()) .filter(|entry| entry.stake() > U256::zero()) @@ -88,6 +89,13 @@ impl Membership .filter(|entry| entry.stake() > U256::zero()) .collect(); + // For each da member, get the stake table entry + let da_members: Vec<::StakeTableEntry> = da_members + .iter() + .map(|member| member.stake_table_entry.clone()) + .filter(|entry| entry.stake() > U256::zero()) + .collect(); + // Index the stake table by public key let indexed_stake_table: BTreeMap< TYPES::SignatureKey, @@ -97,11 +105,21 @@ impl Membership .map(|entry| (TYPES::SignatureKey::public_key(entry), entry.clone())) .collect(); + // Index the stake table by public key + let indexed_da_stake_table: BTreeMap< + TYPES::SignatureKey, + ::StakeTableEntry, + > = da_members + .iter() + .map(|entry| (TYPES::SignatureKey::public_key(entry), entry.clone())) + .collect(); + Self { eligible_leaders, stake_table: members, + da_stake_table: da_members, indexed_stake_table, - committee_topic, + indexed_da_stake_table, _pd: PhantomData, } } @@ -121,6 +139,21 @@ impl Membership .collect() } + /// Get the da stake table for the current view + fn da_stake_table( + &self, + epoch: ::Epoch, + ) -> Vec<<::SignatureKey as SignatureKey>::StakeTableEntry> { + let filter = self.make_da_quorum_filter(epoch); + //self.stake_table.clone()s + self.da_stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| v.clone()) + .collect() + } + /// Get all members of the committee for the current view fn committee_members( &self, @@ -136,6 +169,21 @@ impl Membership .collect() } + /// Get all members of the committee for the current view + fn da_committee_members( + &self, + _view_number: ::View, + epoch: ::Epoch, + ) -> BTreeSet<::SignatureKey> { + let filter = self.make_da_quorum_filter(epoch); + self.da_stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| TYPES::SignatureKey::public_key(v)) + .collect() + } + /// Get all eligible leaders of the committee for the current view fn committee_leaders( &self, @@ -169,6 +217,30 @@ impl Membership } } + /// Get the da stake table entry for a public key + fn da_stake( + &self, + pub_key: &::SignatureKey, + epoch: ::Epoch, + ) -> Option<::StakeTableEntry> { + let filter = self.make_da_quorum_filter(epoch); + let actual_members: BTreeSet<_> = self + .da_stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| TYPES::SignatureKey::public_key(v)) + .collect(); + + if actual_members.contains(pub_key) { + // Only return the stake if it is above zero + self.indexed_da_stake_table.get(pub_key).cloned() + } else { + // Skip members which aren't included based on the quorum filter + None + } + } + /// Check if a node has stake in the committee fn has_stake( &self, @@ -194,9 +266,29 @@ impl Membership } } - /// Get the network topic for the committee - fn committee_topic(&self) -> Topic { - self.committee_topic.clone() + /// Check if a node has stake in the committee + fn has_da_stake( + &self, + pub_key: &::SignatureKey, + epoch: ::Epoch, + ) -> bool { + let filter = self.make_da_quorum_filter(epoch); + let actual_members: BTreeSet<_> = self + .da_stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| TYPES::SignatureKey::public_key(v)) + .collect(); + + if actual_members.contains(pub_key) { + self.indexed_da_stake_table + .get(pub_key) + .is_some_and(|x| x.stake() > U256::zero()) + } else { + // Skip members which aren't included based on the quorum filter + false + } } /// Index the vector of public keys with the current view number @@ -225,17 +317,54 @@ impl Membership Ok(TYPES::SignatureKey::public_key(&res)) } + /// Index the vector of public keys with the current view number + fn lookup_da_leader( + &self, + view_number: TYPES::View, + epoch: ::Epoch, + ) -> Result { + let filter = self.make_da_quorum_filter(epoch); + let leader_vec: Vec<_> = self + .da_stake_table + .iter() + .enumerate() + .filter(|(idx, _)| filter.contains(idx)) + .map(|(_, v)| v.clone()) + .collect(); + + let mut rng: StdRng = rand::SeedableRng::seed_from_u64(*view_number); + + let randomized_view_number: u64 = rng.gen_range(0..=u64::MAX); + #[allow(clippy::cast_possible_truncation)] + let index = randomized_view_number as usize % leader_vec.len(); + + let res = leader_vec[index].clone(); + + Ok(TYPES::SignatureKey::public_key(&res)) + } + /// Get the total number of nodes in the committee fn total_nodes(&self, epoch: ::Epoch) -> usize { self.make_quorum_filter(epoch).len() } + /// Get the total number of nodes in the committee + fn da_total_nodes(&self, epoch: ::Epoch) -> usize { + self.make_da_quorum_filter(epoch).len() + } + /// Get the voting success threshold for the committee fn success_threshold(&self, epoch: ::Epoch) -> NonZeroU64 { let len = self.total_nodes(epoch); NonZeroU64::new(((len as u64 * 2) / 3) + 1).unwrap() } + /// Get the voting success threshold for the committee + fn da_success_threshold(&self, epoch: ::Epoch) -> NonZeroU64 { + let len = self.da_total_nodes(epoch); + NonZeroU64::new(((len as u64 * 2) / 3) + 1).unwrap() + } + /// Get the voting failure threshold for the committee fn failure_threshold(&self, epoch: ::Epoch) -> NonZeroU64 { let len = self.total_nodes(epoch); diff --git a/crates/hotshot/src/traits/election/static_committee.rs b/crates/hotshot/src/traits/election/static_committee.rs index 0324b64350..d2b62f80b7 100644 --- a/crates/hotshot/src/traits/election/static_committee.rs +++ b/crates/hotshot/src/traits/election/static_committee.rs @@ -220,7 +220,7 @@ impl Membership for StaticCommittee { } /// Get the voting success threshold for the committee - fn da_success_threshold(&self) -> NonZeroU64 { + fn da_success_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.da_stake_table.len() as u64 * 2) / 3) + 1).unwrap() } @@ -231,10 +231,7 @@ impl Membership for StaticCommittee { /// Get the voting upgrade threshold for the committee fn upgrade_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { - NonZeroU64::new(max( - (self.stake_table.len() as u64 * 9) / 10, - ((self.stake_table.len() as u64 * 2) / 3) + 1, - )) - .unwrap() + let len = self.stake_table.len(); + NonZeroU64::new(max((len as u64 * 9) / 10, ((len as u64 * 2) / 3) + 1)).unwrap() } } 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 4b8380621d..8833d06872 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 @@ -222,7 +222,7 @@ impl Membership for StaticCommitteeLeaderForTwoViews NonZeroU64 { + fn da_success_threshold(&self, _epoch: ::Epoch) -> NonZeroU64 { NonZeroU64::new(((self.da_stake_table.len() as u64 * 2) / 3) + 1).unwrap() } diff --git a/crates/task-impls/src/helpers.rs b/crates/task-impls/src/helpers.rs index e04a808ca3..242bf3db0d 100644 --- a/crates/task-impls/src/helpers.rs +++ b/crates/task-impls/src/helpers.rs @@ -127,7 +127,7 @@ pub(crate) async fn fetch_proposal( if !justify_qc .is_valid_cert( quorum_membership.stake_table(cur_epoch), - quorum_membership.success_threshold(), + quorum_membership.success_threshold(cur_epoch), upgrade_lock, ) .await @@ -686,7 +686,9 @@ pub(crate) async fn validate_proposal_view_and_certs< validation_info .quorum_membership .stake_table(validation_info.cur_epoch), - validation_info.quorum_membership.success_threshold(), + validation_info + .quorum_membership + .success_threshold(validation_info.cur_epoch), &validation_info.upgrade_lock ) .await, @@ -709,7 +711,9 @@ pub(crate) async fn validate_proposal_view_and_certs< validation_info .quorum_membership .stake_table(validation_info.cur_epoch), - validation_info.quorum_membership.success_threshold(), + validation_info + .quorum_membership + .success_threshold(validation_info.cur_epoch), &validation_info.upgrade_lock ) .await, diff --git a/crates/task-impls/src/quorum_proposal/handlers.rs b/crates/task-impls/src/quorum_proposal/handlers.rs index 55f283bcd8..b1cc8e36fa 100644 --- a/crates/task-impls/src/quorum_proposal/handlers.rs +++ b/crates/task-impls/src/quorum_proposal/handlers.rs @@ -128,7 +128,8 @@ impl ProposalDependencyHandle { // TODO take epoch from `qc` // https://github.com/EspressoSystems/HotShot/issues/3917 self.quorum_membership.stake_table(TYPES::Epoch::new(0)), - self.quorum_membership.success_threshold(), + self.quorum_membership + .success_threshold(TYPES::Epoch::new(0)), &self.upgrade_lock, ) .await diff --git a/crates/task-impls/src/quorum_proposal/mod.rs b/crates/task-impls/src/quorum_proposal/mod.rs index 79299119fb..06150dff97 100644 --- a/crates/task-impls/src/quorum_proposal/mod.rs +++ b/crates/task-impls/src/quorum_proposal/mod.rs @@ -444,7 +444,7 @@ impl, V: Versions> certificate .is_valid_cert( self.quorum_membership.stake_table(epoch_number), - self.quorum_membership.success_threshold(), + self.quorum_membership.success_threshold(epoch_number), &self.upgrade_lock ) .await, @@ -508,7 +508,7 @@ impl, V: Versions> ensure!( qc.is_valid_cert( self.quorum_membership.stake_table(epoch_number), - self.quorum_membership.success_threshold(), + self.quorum_membership.success_threshold(epoch_number), &self.upgrade_lock ) .await, diff --git a/crates/task-impls/src/quorum_proposal_recv/handlers.rs b/crates/task-impls/src/quorum_proposal_recv/handlers.rs index 73f6addef0..3d5c010058 100644 --- a/crates/task-impls/src/quorum_proposal_recv/handlers.rs +++ b/crates/task-impls/src/quorum_proposal_recv/handlers.rs @@ -157,7 +157,9 @@ pub(crate) async fn handle_quorum_proposal_recv< validation_info .quorum_membership .stake_table(validation_info.cur_epoch), - validation_info.quorum_membership.success_threshold(), + validation_info + .quorum_membership + .success_threshold(validation_info.cur_epoch), &validation_info.upgrade_lock, ) .await diff --git a/crates/task-impls/src/quorum_vote/mod.rs b/crates/task-impls/src/quorum_vote/mod.rs index 0567329788..7f4178171d 100644 --- a/crates/task-impls/src/quorum_vote/mod.rs +++ b/crates/task-impls/src/quorum_vote/mod.rs @@ -482,7 +482,7 @@ impl, V: Versions> QuorumVoteTaskS ensure!( cert.is_valid_cert( self.membership.da_stake_table(cur_epoch), - self.membership.da_success_threshold(), + self.membership.da_success_threshold(cur_epoch), &self.upgrade_lock ) .await, diff --git a/crates/task-impls/src/view_sync.rs b/crates/task-impls/src/view_sync.rs index efb87f9cd5..6bb83dfc53 100644 --- a/crates/task-impls/src/view_sync.rs +++ b/crates/task-impls/src/view_sync.rs @@ -535,7 +535,7 @@ impl ViewSyncReplicaTaskState { if !certificate .is_valid_cert( self.membership.stake_table(self.cur_epoch), - self.membership.failure_threshold(), + self.membership.failure_threshold(self.cur_epoch), &self.upgrade_lock, ) .await @@ -621,7 +621,7 @@ impl ViewSyncReplicaTaskState { if !certificate .is_valid_cert( self.membership.stake_table(self.cur_epoch), - self.membership.success_threshold(), + self.membership.success_threshold(self.cur_epoch), &self.upgrade_lock, ) .await @@ -718,7 +718,7 @@ impl ViewSyncReplicaTaskState { if !certificate .is_valid_cert( self.membership.stake_table(self.cur_epoch), - self.membership.success_threshold(), + self.membership.success_threshold(self.cur_epoch), &self.upgrade_lock, ) .await diff --git a/crates/testing/tests/tests_1/message.rs b/crates/testing/tests/tests_1/message.rs index 3778aa4737..9536cf0f22 100644 --- a/crates/testing/tests/tests_1/message.rs +++ b/crates/testing/tests/tests_1/message.rs @@ -104,7 +104,7 @@ async fn test_certificate2_validity() { assert!( qc.is_valid_cert( membership.stake_table(EpochNumber::new(0)), - membership.success_threshold(), + membership.success_threshold(EpochNumber::new(0)), &handle.hotshot.upgrade_lock ) .await @@ -113,7 +113,7 @@ async fn test_certificate2_validity() { assert!( qc2.is_valid_cert( membership.stake_table(EpochNumber::new(0)), - membership.success_threshold(), + membership.success_threshold(EpochNumber::new(0)), &handle.hotshot.upgrade_lock ) .await diff --git a/crates/types/src/simple_certificate.rs b/crates/types/src/simple_certificate.rs index 8d544b5cdb..f2cc8cd689 100644 --- a/crates/types/src/simple_certificate.rs +++ b/crates/types/src/simple_certificate.rs @@ -24,7 +24,7 @@ use crate::{ data::serialize_signature2, message::UpgradeLock, simple_vote::{ - DaData, QuorumData, QuorumData2, QuorumMaker, TimeoutData, UpgradeProposalData, + DaData, QuorumData, QuorumData2, QuorumMarker, TimeoutData, UpgradeProposalData, VersionedVoteData, ViewSyncCommitData, ViewSyncFinalizeData, ViewSyncPreCommitData, Voteable, }, @@ -204,8 +204,11 @@ impl> Certificate ) -> usize { membership.da_total_nodes(epoch) } - fn threshold>(membership: &MEMBERSHIP) -> u64 { - membership.da_success_threshold().into() + fn threshold>( + membership: &MEMBERSHIP, + epoch: ::Epoch, + ) -> u64 { + membership.da_success_threshold(epoch).into() } fn data(&self) -> &Self::Voteable { &self.data @@ -222,7 +225,7 @@ impl> Certificate } } -impl> +impl> Certificate for SimpleCertificate { type Voteable = VOTEABLE; diff --git a/crates/types/src/simple_vote.rs b/crates/types/src/simple_vote.rs index 35997d3725..138f730fb9 100644 --- a/crates/types/src/simple_vote.rs +++ b/crates/types/src/simple_vote.rs @@ -25,7 +25,7 @@ use crate::{ }; /// Marker that data should use the quorum cert type -pub(crate) trait QuorumMaker {} +pub(crate) trait QuorumMarker {} #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Hash, Eq)] /// Data used for a yes vote. @@ -117,13 +117,13 @@ mod sealed { impl Sealed for C {} } -impl QuorumMaker for QuorumData {} -impl QuorumMaker for QuorumData2 {} -impl QuorumMaker for TimeoutData {} -impl QuorumMaker for ViewSyncPreCommitData {} -impl QuorumMaker for ViewSyncCommitData {} -impl QuorumMaker for ViewSyncFinalizeData {} -impl QuorumMaker for UpgradeProposalData {} +impl QuorumMarker for QuorumData {} +impl QuorumMarker for QuorumData2 {} +impl QuorumMarker for TimeoutData {} +impl QuorumMarker for ViewSyncPreCommitData {} +impl QuorumMarker for ViewSyncCommitData {} +impl QuorumMarker for ViewSyncFinalizeData {} +impl QuorumMarker for UpgradeProposalData {} /// A simple yes vote over some votable type. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Hash, Eq)] diff --git a/crates/types/src/traits/election.rs b/crates/types/src/traits/election.rs index 94e456e73a..2dbccda807 100644 --- a/crates/types/src/traits/election.rs +++ b/crates/types/src/traits/election.rs @@ -133,7 +133,7 @@ pub trait Membership: Clone + Debug + Send + Sync { fn success_threshold(&self, epoch: TYPES::Epoch) -> NonZeroU64; /// Returns the DA threshold for a specific `Membership` implementation - fn da_success_threshold(&self) -> NonZeroU64; + fn da_success_threshold(&self, epoch: TYPES::Epoch) -> NonZeroU64; /// Returns the threshold for a specific `Membership` implementation fn failure_threshold(&self, epoch: TYPES::Epoch) -> NonZeroU64; From 77a88f1023f1a4a1620eda26f6ced20d9c12955c Mon Sep 17 00:00:00 2001 From: pls148 <184445976+pls148@users.noreply.github.com> Date: Mon, 2 Dec 2024 10:09:05 -0800 Subject: [PATCH 10/10] fix lingering da_leader things --- crates/hotshot/src/tasks/task_state.rs | 3 --- .../election/randomized_committee_members.rs | 26 ------------------- crates/hotshot/src/types/handle.rs | 2 +- crates/types/src/traits/election.rs | 15 ----------- crates/types/src/traits/metrics.rs | 3 ++- 5 files changed, 3 insertions(+), 46 deletions(-) diff --git a/crates/hotshot/src/tasks/task_state.rs b/crates/hotshot/src/tasks/task_state.rs index 6752aae43f..1df18f9b5e 100644 --- a/crates/hotshot/src/tasks/task_state.rs +++ b/crates/hotshot/src/tasks/task_state.rs @@ -72,7 +72,6 @@ impl, V: Versions> CreateTaskState cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, quorum_membership: (*handle.hotshot.memberships).clone().into(), - network: Arc::clone(&handle.hotshot.network), vote_collectors: BTreeMap::default(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), @@ -163,7 +162,6 @@ impl, V: Versions> CreateTaskState cur_view, next_view: cur_view, cur_epoch: handle.cur_epoch().await, - network: Arc::clone(&handle.hotshot.network), membership: (*handle.hotshot.memberships).clone().into(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), @@ -191,7 +189,6 @@ impl, V: Versions> CreateTaskState consensus: OuterConsensus::new(handle.hotshot.consensus()), cur_view: handle.cur_view().await, cur_epoch: handle.cur_epoch().await, - network: Arc::clone(&handle.hotshot.network), membership: (*handle.hotshot.memberships).clone().into(), public_key: handle.public_key().clone(), private_key: handle.private_key().clone(), diff --git a/crates/hotshot/src/traits/election/randomized_committee_members.rs b/crates/hotshot/src/traits/election/randomized_committee_members.rs index 01f4ad6383..5c85ad9c07 100644 --- a/crates/hotshot/src/traits/election/randomized_committee_members.rs +++ b/crates/hotshot/src/traits/election/randomized_committee_members.rs @@ -317,32 +317,6 @@ impl Membership Ok(TYPES::SignatureKey::public_key(&res)) } - /// Index the vector of public keys with the current view number - fn lookup_da_leader( - &self, - view_number: TYPES::View, - epoch: ::Epoch, - ) -> Result { - let filter = self.make_da_quorum_filter(epoch); - let leader_vec: Vec<_> = self - .da_stake_table - .iter() - .enumerate() - .filter(|(idx, _)| filter.contains(idx)) - .map(|(_, v)| v.clone()) - .collect(); - - let mut rng: StdRng = rand::SeedableRng::seed_from_u64(*view_number); - - let randomized_view_number: u64 = rng.gen_range(0..=u64::MAX); - #[allow(clippy::cast_possible_truncation)] - let index = randomized_view_number as usize % leader_vec.len(); - - let res = leader_vec[index].clone(); - - Ok(TYPES::SignatureKey::public_key(&res)) - } - /// Get the total number of nodes in the committee fn total_nodes(&self, epoch: ::Epoch) -> usize { self.make_quorum_filter(epoch).len() diff --git a/crates/hotshot/src/types/handle.rs b/crates/hotshot/src/types/handle.rs index caeba9a518..9ea46b34d7 100644 --- a/crates/hotshot/src/types/handle.rs +++ b/crates/hotshot/src/types/handle.rs @@ -196,7 +196,7 @@ impl + 'static, V: Versions> if commit == leaf_commitment { return Ok(quorum_proposal.clone()); } - tracing::warn!("Proposal receied from request has different commitment than expected.\nExpected = {:?}\nReceived{:?}", leaf_commitment, commit); + tracing::warn!("Proposal received from request has different commitment than expected.\nExpected = {:?}\nReceived{:?}", leaf_commitment, commit); } } }) diff --git a/crates/types/src/traits/election.rs b/crates/types/src/traits/election.rs index 2dbccda807..5b72ea4f84 100644 --- a/crates/types/src/traits/election.rs +++ b/crates/types/src/traits/election.rs @@ -95,21 +95,6 @@ pub trait Membership: Clone + Debug + Send + Sync { )) } - /// The DA 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. - fn da_leader(&self, view: TYPES::View, epoch: TYPES::Epoch) -> Result { - use utils::anytrace::*; - - self.lookup_da_leader(view, epoch).wrap().context(info!( - "Failed to get leader for view {view} in epoch {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 diff --git a/crates/types/src/traits/metrics.rs b/crates/types/src/traits/metrics.rs index 38d9a8d6ea..cd29c75da5 100644 --- a/crates/types/src/traits/metrics.rs +++ b/crates/types/src/traits/metrics.rs @@ -212,13 +212,14 @@ pub trait Counter: Send + Sync + Debug + DynClone { /// Add a value to the counter fn add(&self, amount: usize); } + /// A gauge that stores the latest value. pub trait Gauge: Send + Sync + Debug + DynClone { /// Set the gauge value fn set(&self, amount: usize); /// Update the gauge value - fn update(&self, delts: i64); + fn update(&self, delta: i64); } /// A histogram which will record a series of points.