Skip to content

Commit

Permalink
Remove EPOCH_HEIGHT as an associated constant in NodeType (#3997)
Browse files Browse the repository at this point in the history
  • Loading branch information
ss-es authored Jan 6, 2025
1 parent e8cd837 commit b8da9b4
Show file tree
Hide file tree
Showing 28 changed files with 93 additions and 75 deletions.
10 changes: 0 additions & 10 deletions crates/example-types/src/node_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ use crate::{
/// to select our traits
pub struct TestTypes;
impl NodeType for TestTypes {
const EPOCH_HEIGHT: u64 = 10;

type AuctionResult = TestAuctionResult;
type View = ViewNumber;
type Epoch = EpochNumber;
Expand Down Expand Up @@ -84,8 +82,6 @@ impl NodeType for TestTypes {
/// to select our traits
pub struct TestTypesRandomizedLeader;
impl NodeType for TestTypesRandomizedLeader {
const EPOCH_HEIGHT: u64 = 10;

type AuctionResult = TestAuctionResult;
type View = ViewNumber;
type Epoch = EpochNumber;
Expand Down Expand Up @@ -119,8 +115,6 @@ pub struct TestTypesRandomizedCommitteeMembers<CONFIG: QuorumFilterConfig> {
}

impl<CONFIG: QuorumFilterConfig> NodeType for TestTypesRandomizedCommitteeMembers<CONFIG> {
const EPOCH_HEIGHT: u64 = 10;

type AuctionResult = TestAuctionResult;
type View = ViewNumber;
type Epoch = EpochNumber;
Expand Down Expand Up @@ -152,8 +146,6 @@ impl<CONFIG: QuorumFilterConfig> NodeType for TestTypesRandomizedCommitteeMember
/// to select our traits
pub struct TestConsecutiveLeaderTypes;
impl NodeType for TestConsecutiveLeaderTypes {
const EPOCH_HEIGHT: u64 = 10;

type AuctionResult = TestAuctionResult;
type View = ViewNumber;
type Epoch = EpochNumber;
Expand Down Expand Up @@ -184,8 +176,6 @@ impl NodeType for TestConsecutiveLeaderTypes {
/// to select our traits
pub struct TestTwoStakeTablesTypes;
impl NodeType for TestTwoStakeTablesTypes {
const EPOCH_HEIGHT: u64 = 10;

type AuctionResult = TestAuctionResult;
type View = ViewNumber;
type Epoch = EpochNumber;
Expand Down
1 change: 1 addition & 0 deletions crates/hotshot/src/tasks/task_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> CreateTaskState
id: handle.hotshot.id,
shutdown_flag: Arc::new(AtomicBool::new(false)),
spawned_tasks: BTreeMap::new(),
epoch_height: handle.epoch_height,
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/task-impls/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ pub(crate) async fn fetch_proposal<TYPES: NodeType, V: Versions>(
leaf: leaf.commit(),
state,
delta: None,
epoch: leaf.epoch(),
epoch: leaf.epoch(epoch_height),
},
};
Ok((leaf, view))
Expand Down Expand Up @@ -814,7 +814,7 @@ pub(crate) async fn validate_proposal_view_and_certs<
{
let epoch = TYPES::Epoch::new(epoch_from_block_number(
proposal.data.block_header.block_number(),
TYPES::EPOCH_HEIGHT,
validation_info.epoch_height,
));
UpgradeCertificate::validate(
&proposal.data.upgrade_certificate,
Expand Down
7 changes: 4 additions & 3 deletions crates/task-impls/src/quorum_vote/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ async fn start_drb_task<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versio
) {
let current_epoch_number = TYPES::Epoch::new(epoch_from_block_number(
proposal.block_header.block_number(),
TYPES::EPOCH_HEIGHT,
task_state.epoch_height,
));

// Start the new task if we're in the committee for this epoch
Expand Down Expand Up @@ -615,17 +615,18 @@ pub(crate) async fn submit_vote<TYPES: NodeType, I: NodeImplementation<TYPES>, V
leaf: Leaf2<TYPES>,
vid_share: Proposal<TYPES, VidDisperseShare2<TYPES>>,
extended_vote: bool,
epoch_height: u64,
) -> Result<()> {
let epoch_number = TYPES::Epoch::new(epoch_from_block_number(
leaf.block_header().block_number(),
TYPES::EPOCH_HEIGHT,
epoch_height,
));

let membership_reader = membership.read().await;
let committee_member_in_current_epoch = membership_reader.has_stake(&public_key, epoch_number);
// If the proposed leaf is for the last block in the epoch and the node is part of the quorum committee
// in the next epoch, the node should vote to achieve the double quorum.
let committee_member_in_next_epoch = is_last_block_in_epoch(leaf.height(), TYPES::EPOCH_HEIGHT)
let committee_member_in_next_epoch = is_last_block_in_epoch(leaf.height(), epoch_height)
&& membership_reader.has_stake(&public_key, epoch_number + 1);
drop(membership_reader);

Expand Down
2 changes: 2 additions & 0 deletions crates/task-impls/src/quorum_vote/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES> + 'static, V: Versions> Handl
leaf,
vid_share,
false,
self.epoch_height,
)
.await
{
Expand Down Expand Up @@ -767,6 +768,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> QuorumVoteTaskS
proposed_leaf,
updated_vid,
is_vote_leaf_extended,
self.epoch_height,
)
.await
{
Expand Down
5 changes: 4 additions & 1 deletion crates/task-impls/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ pub struct NetworkRequestState<TYPES: NodeType, I: NodeImplementation<TYPES>> {

/// A flag indicating that `HotShotEvent::Shutdown` has been received
pub spawned_tasks: BTreeMap<TYPES::View, Vec<JoinHandle<()>>>,

/// Number of blocks in an epoch, zero means there are no epochs
pub epoch_height: u64,
}

impl<TYPES: NodeType, I: NodeImplementation<TYPES>> Drop for NetworkRequestState<TYPES, I> {
Expand Down Expand Up @@ -111,7 +114,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>> TaskState for NetworkRequest
let prop_view = proposal.data.view_number();
let prop_epoch = TYPES::Epoch::new(epoch_from_block_number(
proposal.data.block_header.block_number(),
TYPES::EPOCH_HEIGHT,
self.epoch_height,
));

// If we already have the VID shares for the next view, do nothing.
Expand Down
34 changes: 6 additions & 28 deletions crates/testing/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,12 @@ use hotshot_example_types::{
use hotshot_task_impls::events::HotShotEvent;
use hotshot_types::{
consensus::ConsensusMetricsValue,
data::{Leaf2, QuorumProposal2, VidDisperse, VidDisperseShare2},
message::{GeneralConsensusMessage, Proposal, UpgradeLock},
data::{Leaf2, VidDisperse, VidDisperseShare2},
message::{Proposal, UpgradeLock},
simple_certificate::DaCertificate2,
simple_vote::{DaData2, DaVote2, QuorumData2, QuorumVote2, SimpleVote, VersionedVoteData},
simple_vote::{DaData2, DaVote2, SimpleVote, VersionedVoteData},
traits::{
block_contents::vid_commitment,
consensus_api::ConsensusApi,
election::Membership,
node_implementation::{ConsensusTime, NodeType, Versions},
},
Expand All @@ -47,7 +46,7 @@ use serde::Serialize;

use crate::{test_builder::TestDescription, test_launcher::TestLauncher};

/// create the [`SystemContextHandle`] from a node id
/// create the [`SystemContextHandle`] from a node id, with no epochs
/// # Panics
/// if cannot create a [`HotShotInitializer`]
pub async fn build_system_handle<
Expand All @@ -65,7 +64,8 @@ pub async fn build_system_handle<
Sender<Arc<HotShotEvent<TYPES>>>,
Receiver<Arc<HotShotEvent<TYPES>>>,
) {
let builder: TestDescription<TYPES, I, V> = TestDescription::default_multiple_rounds();
let mut builder: TestDescription<TYPES, I, V> = TestDescription::default_multiple_rounds();
builder.epoch_height = 0;

let launcher = builder.gen_launcher(node_id);
build_system_handle_from_launcher(node_id, &launcher).await
Expand Down Expand Up @@ -397,28 +397,6 @@ pub async fn build_da_certificate<TYPES: NodeType, V: Versions>(
.await
}

pub async fn build_vote<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions>(
handle: &SystemContextHandle<TYPES, I, V>,
proposal: QuorumProposal2<TYPES>,
) -> GeneralConsensusMessage<TYPES> {
let view = proposal.view_number;

let leaf: Leaf2<_> = Leaf2::from_quorum_proposal(&proposal);
let vote = QuorumVote2::<TYPES>::create_signed_vote(
QuorumData2 {
leaf_commit: leaf.commit(),
epoch: leaf.epoch(),
},
view,
&handle.public_key(),
handle.private_key(),
&handle.hotshot.upgrade_lock,
)
.await
.expect("Failed to create quorum vote");
GeneralConsensusMessage::<TYPES>::Vote(vote.to_vote())
}

/// This function permutes the provided input vector `inputs`, given some order provided within the
/// `order` vector.
///
Expand Down
6 changes: 4 additions & 2 deletions crates/testing/src/overall_safety_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ pub struct OverallSafetyTask<TYPES: NodeType, I: TestableNodeImplementation<TYPE
pub error: Option<Box<OverallSafetyTaskErr<TYPES>>>,
/// sender to test event channel
pub test_sender: Sender<TestEvent>,
/// Number of blocks in an epoch, zero means there are no epochs
pub epoch_height: u64,
}

impl<TYPES: NodeType, I: TestableNodeImplementation<TYPES>, V: Versions>
Expand Down Expand Up @@ -186,8 +188,8 @@ impl<TYPES: NodeType, I: TestableNodeImplementation<TYPES>, V: Versions> TestTas
};

if let Some(ref key) = key {
if *key.epoch() > self.ctx.latest_epoch {
self.ctx.latest_epoch = *key.epoch();
if *key.epoch(self.epoch_height) > self.ctx.latest_epoch {
self.ctx.latest_epoch = *key.epoch(self.epoch_height);
}
}

Expand Down
19 changes: 7 additions & 12 deletions crates/testing/src/test_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@
// You should have received a copy of the MIT License
// along with the HotShot repository. If not, see <https://mit-license.org/>.

use std::{
any::TypeId, collections::HashMap, num::NonZeroUsize, rc::Rc, sync::Arc, time::Duration,
};
use std::{collections::HashMap, num::NonZeroUsize, rc::Rc, sync::Arc, time::Duration};

use anyhow::{ensure, Result};
use async_lock::RwLock;
Expand All @@ -17,8 +15,8 @@ use hotshot::{
HotShotInitializer, MarketplaceConfig, SystemContext, TwinsHandlerState,
};
use hotshot_example_types::{
auction_results_provider_types::TestAuctionResultsProvider, node_types::EpochsTestVersions,
state_types::TestInstanceState, storage_types::TestStorage, testable_delay::DelayConfig,
auction_results_provider_types::TestAuctionResultsProvider, state_types::TestInstanceState,
storage_types::TestStorage, testable_delay::DelayConfig,
};
use hotshot_types::{
consensus::ConsensusMetricsValue,
Expand Down Expand Up @@ -101,6 +99,8 @@ pub struct TestDescription<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Ver
pub start_solver: bool,
/// boxed closure used to validate the resulting transactions
pub validate_transactions: TransactionValidator,
/// Number of blocks in an epoch, zero means there are no epochs
pub epoch_height: u64,
}

pub fn nonempty_block_threshold(threshold: (u64, u64)) -> TransactionValidator {
Expand Down Expand Up @@ -387,6 +387,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> Default
fn default() -> Self {
let num_nodes_with_stake = 7;
Self {
epoch_height: 10,
timing_data: TimingData::default(),
num_nodes_with_stake,
start_nodes: num_nodes_with_stake,
Expand Down Expand Up @@ -487,12 +488,6 @@ where
// This is the config for node 0
0 < da_staked_committee_size,
);
// let da_committee_nodes = known_nodes[0..da_committee_size].to_vec();
let epoch_height = if TypeId::of::<V>() == TypeId::of::<EpochsTestVersions>() {
10
} else {
0
};
let config = HotShotConfig {
start_threshold: (1, 1),
num_nodes_with_stake: NonZeroUsize::new(num_nodes_with_stake).unwrap(),
Expand All @@ -516,7 +511,7 @@ where
stop_proposing_time: 0,
start_voting_time: u64::MAX,
stop_voting_time: 0,
epoch_height,
epoch_height: self.epoch_height,
};
let TimingData {
next_view_timeout,
Expand Down
1 change: 1 addition & 0 deletions crates/testing/src/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ where
// add safety task
let overall_safety_task_state = OverallSafetyTask {
handles: Arc::clone(&handles),
epoch_height: launcher.metadata.epoch_height,
ctx: RoundCtx::default(),
properties: launcher.metadata.overall_safety_properties.clone(),
error: None,
Expand Down
2 changes: 2 additions & 0 deletions crates/testing/tests/tests_1/libp2p.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ async fn libp2p_network() {
next_view_timeout: 4000,
..Default::default()
},
epoch_height: 0,
..TestDescription::default_multiple_rounds()
};

Expand Down Expand Up @@ -68,6 +69,7 @@ async fn libp2p_network_failures_2() {
next_view_timeout: 4000,
..Default::default()
},
epoch_height: 0,
..TestDescription::default_multiple_rounds()
};

Expand Down
11 changes: 11 additions & 0 deletions crates/testing/tests/tests_1/test_success.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ cross_tests!(
duration: Duration::from_secs(60),
},
),
epoch_height: 0,
..TestDescription::default()
}
},
Expand All @@ -56,6 +57,7 @@ cross_tests!(
duration: Duration::from_secs(60),
},
),
epoch_height: 10,
..TestDescription::default()
}
},
Expand All @@ -75,6 +77,7 @@ cross_tests!(
// duration: Duration::from_secs(60),
// },
// ),
// epoch_height: 10,
// ..TestDescription::default()
// }
// },
Expand All @@ -94,6 +97,7 @@ cross_tests!(
duration: Duration::from_secs(60),
},
),
epoch_height: 0,
..TestDescription::default()
};

Expand Down Expand Up @@ -126,6 +130,7 @@ cross_tests!(
duration: Duration::from_secs(60),
},
),
epoch_height: 10,
..TestDescription::default()
};

Expand Down Expand Up @@ -158,6 +163,7 @@ cross_tests!(
duration: Duration::from_secs(60),
},
),
epoch_height: 0,
..TestDescription::default()
};

Expand Down Expand Up @@ -198,6 +204,7 @@ cross_tests!(
duration: Duration::from_secs(60),
},
),
epoch_height: 10,
..TestDescription::default()
};

Expand Down Expand Up @@ -232,6 +239,7 @@ cross_tests!(
Ignore: false,
Metadata: {
let mut metadata = TestDescription::default_more_nodes();
metadata.epoch_height = 0;
metadata.num_bootstrap_nodes = 10;
metadata.num_nodes_with_stake = 12;
metadata.da_staked_committee_size = 12;
Expand All @@ -254,6 +262,7 @@ cross_tests!(
Metadata: {
let mut metadata = TestDescription::default_more_nodes();
metadata.num_bootstrap_nodes = 10;
metadata.epoch_height = 10;
metadata.num_nodes_with_stake = 12;
metadata.da_staked_committee_size = 12;
metadata.start_nodes = 12;
Expand Down Expand Up @@ -283,6 +292,7 @@ cross_tests!(
start_nodes: 11,
num_bootstrap_nodes: 11,
da_staked_committee_size: 11,
epoch_height: 10,
..TestDescription::default()
}
},
Expand All @@ -303,6 +313,7 @@ cross_tests!(
duration: Duration::from_millis(100000),
},
),
epoch_height: 10,
..TestDescription::default()
};
// after the first 3 leaders the next leader is down. It's a hack to make sure we decide in
Expand Down
Loading

0 comments on commit b8da9b4

Please sign in to comment.