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 ee13773527..a8485fd984 100644 --- a/crates/hotshot/src/traits/election/mod.rs +++ b/crates/hotshot/src/traits/election/mod.rs @@ -21,4 +21,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 cf06c620c6..bb82508067 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,