Skip to content

Commit

Permalink
2652 Membership::set_first_epoch
Browse files Browse the repository at this point in the history
  • Loading branch information
pls148 committed Feb 24, 2025
1 parent 88775ce commit 4bf4660
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 48 deletions.
26 changes: 21 additions & 5 deletions hotshot-task-impls/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use hotshot_task::dependency::{Dependency, EventDependency};
use hotshot_types::{
consensus::OuterConsensus,
data::{Leaf2, QuorumProposalWrapper, ViewChangeEvidence2},
drb::INITIAL_DRB_RESULT,
event::{Event, EventType, LeafInfo},
message::{Proposal, UpgradeLock},
request_response::ProposalRequestPayload,
Expand All @@ -37,6 +38,7 @@ use hotshot_types::{
use hotshot_utils::anytrace::*;
use tokio::time::timeout;
use tracing::instrument;
use vbs::version::StaticVersionType;

use crate::{events::HotShotEvent, quorum_proposal_recv::ValidationInfo, request::REQUEST_TIMEOUT};

Expand Down Expand Up @@ -254,7 +256,7 @@ impl<TYPES: NodeType + Default> Default for LeafChainTraversalOutcome<TYPES> {
/// # Panics
/// If the leaf chain contains no decided leaf while reaching a decided view, which should be
/// impossible.
pub async fn decide_from_proposal_2<TYPES: NodeType>(
pub async fn decide_from_proposal_2<TYPES: NodeType, V: Versions>(
proposal: &QuorumProposalWrapper<TYPES>,
consensus: OuterConsensus<TYPES>,
existing_upgrade_cert: Arc<RwLock<Option<UpgradeCertificate<TYPES>>>>,
Expand Down Expand Up @@ -375,7 +377,7 @@ pub async fn decide_from_proposal_2<TYPES: NodeType>(
/// # Panics
/// If the leaf chain contains no decided leaf while reaching a decided view, which should be
/// impossible.
pub async fn decide_from_proposal<TYPES: NodeType>(
pub async fn decide_from_proposal<TYPES: NodeType, V: Versions>(
proposal: &QuorumProposalWrapper<TYPES>,
consensus: OuterConsensus<TYPES>,
existing_upgrade_cert: Arc<RwLock<Option<UpgradeCertificate<TYPES>>>>,
Expand Down Expand Up @@ -488,17 +490,31 @@ pub async fn decide_from_proposal<TYPES: NodeType>(
tracing::debug!("Leaf ascension failed; error={e}");
}

if with_epochs && res.new_decided_view_number.is_some() {
let epoch_height = consensus_reader.epoch_height;
drop(consensus_reader);
let epoch_height = consensus_reader.epoch_height;
drop(consensus_reader);

if with_epochs && res.new_decided_view_number.is_some() {
if let Some(decided_leaf_info) = res.leaf_views.last() {
decide_epoch_root(&decided_leaf_info.leaf, epoch_height, membership).await;
} else {
tracing::info!("No decided leaf while a view has been decided.");
}
}

if let Some(ref decided_upgrade_cert) = res.decided_upgrade_cert {
if decided_upgrade_cert.data.new_version == V::Epochs::VERSION {
if let Some(decided_leaf_info) = res.leaf_views.last() {
let decided_block_number = decided_leaf_info.leaf.block_header().block_number();
let first_epoch_number =
TYPES::Epoch::new(epoch_from_block_number(decided_block_number, epoch_height));
membership
.write()
.await
.set_first_epoch(first_epoch_number, INITIAL_DRB_RESULT);
}
}
}

res
}

Expand Down
4 changes: 2 additions & 2 deletions hotshot-task-impls/src/quorum_vote/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ pub(crate) async fn handle_quorum_proposal_validated<
included_txns,
decided_upgrade_cert,
} = if version >= V::Epochs::VERSION {
decide_from_proposal_2(
decide_from_proposal_2::<TYPES, V>(
proposal,
OuterConsensus::new(Arc::clone(&task_state.consensus.inner_consensus)),
Arc::clone(&task_state.upgrade_lock.decided_upgrade_certificate),
Expand All @@ -379,7 +379,7 @@ pub(crate) async fn handle_quorum_proposal_validated<
)
.await
} else {
decide_from_proposal(
decide_from_proposal::<TYPES, V>(
proposal,
OuterConsensus::new(Arc::clone(&task_state.consensus.inner_consensus)),
Arc::clone(&task_state.upgrade_lock.decided_upgrade_certificate),
Expand Down
11 changes: 10 additions & 1 deletion hotshot-testing/src/test_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use hotshot_example_types::{
};
use hotshot_fakeapi::fake_solver::FakeSolverState;
use hotshot_task_impls::events::HotShotEvent;
use hotshot_types::drb::INITIAL_DRB_RESULT;
use hotshot_types::{
consensus::ConsensusMetricsValue,
constants::EVENT_CHANNEL_SIZE,
Expand All @@ -44,6 +45,7 @@ use tide_disco::Url;
use tokio::{spawn, task::JoinHandle};
#[allow(deprecated)]
use tracing::info;
use vbs::version::StaticVersionType;

use super::{
completion_task::CompletionTask, consistency_task::ConsistencyTask, txn_task::TxnTask,
Expand Down Expand Up @@ -394,10 +396,17 @@ where
let config = self.launcher.metadata.test_config.clone();

// TODO This is only a workaround. Number of nodes changes from epoch to epoch. Builder should be made epoch-aware.
let temp_memberships = <TYPES as NodeType>::Membership::new(
let mut temp_memberships = <TYPES as NodeType>::Membership::new(
config.known_nodes_with_stake.clone(),
config.known_da_nodes.clone(),
);

// if we're doing epochs, then tell the membership
if V::Base::VERSION >= V::Epochs::VERSION {
temp_memberships
.set_first_epoch(<TYPES as NodeType>::Epoch::new(1), INITIAL_DRB_RESULT);
}

// #3967 is it enough to check versions now? Or should we also be checking epoch_height?
let num_nodes = temp_memberships.total_nodes(genesis_epoch_from_version::<V, TYPES>());
let (mut builder_tasks, builder_urls, fallback_builder_url) =
Expand Down
32 changes: 15 additions & 17 deletions hotshot-testing/tests/tests_1/quorum_proposal_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@ use hotshot_testing::{
view_generator::TestViewGenerator,
};
use hotshot_types::{
data::{null_block, EpochNumber, Leaf2, ViewChangeEvidence2, ViewNumber},
data::{null_block, Leaf2, ViewChangeEvidence2, ViewNumber},
simple_vote::{TimeoutData2, ViewSyncFinalizeData2},
traits::{
node_implementation::{ConsensusTime, Versions},
},
traits::node_implementation::{ConsensusTime, Versions},
utils::BuilderCommitment,
};
use sha2::Digest;
Expand Down Expand Up @@ -59,7 +57,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_1() {
let payload_commitment = build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(node_id),
Some(EpochNumber::new(1)),
None,
version,
)
.await;
Expand Down Expand Up @@ -206,7 +204,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(1),
Some(EpochNumber::new(1)),
None,
version_1,
)
.await,
Expand All @@ -227,7 +225,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(2),
Some(EpochNumber::new(1)),
None,
version_2,
)
.await,
Expand All @@ -246,7 +244,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(3),
Some(EpochNumber::new(1)),
None,
version_3,
)
.await,
Expand All @@ -265,7 +263,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(4),
Some(EpochNumber::new(1)),
None,
version_4,
)
.await,
Expand All @@ -284,7 +282,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(5),
Some(EpochNumber::new(1)),
None,
version_5,
)
.await,
Expand Down Expand Up @@ -338,7 +336,7 @@ async fn test_quorum_proposal_task_qc_timeout() {
let payload_commitment = build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(node_id),
Some(EpochNumber::new(1)),
None,
version,
)
.await;
Expand Down Expand Up @@ -433,7 +431,7 @@ async fn test_quorum_proposal_task_view_sync() {
let payload_commitment = build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(node_id),
Some(EpochNumber::new(1)),
None,
version,
)
.await;
Expand Down Expand Up @@ -574,7 +572,7 @@ async fn test_quorum_proposal_task_liveness_check() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(1),
Some(EpochNumber::new(1)),
None,
version_1,
)
.await,
Expand All @@ -595,7 +593,7 @@ async fn test_quorum_proposal_task_liveness_check() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(2),
Some(EpochNumber::new(1)),
None,
version_2,
)
.await,
Expand All @@ -614,7 +612,7 @@ async fn test_quorum_proposal_task_liveness_check() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(3),
Some(EpochNumber::new(1)),
None,
version_3,
)
.await,
Expand All @@ -633,7 +631,7 @@ async fn test_quorum_proposal_task_liveness_check() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(4),
Some(EpochNumber::new(1)),
None,
version_4,
)
.await,
Expand All @@ -652,7 +650,7 @@ async fn test_quorum_proposal_task_liveness_check() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(5),
Some(EpochNumber::new(1)),
None,
version_5,
)
.await,
Expand Down
4 changes: 1 addition & 3 deletions hotshot-testing/tests/tests_1/transaction_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ use hotshot_task_impls::{
use hotshot_testing::helpers::build_system_handle;
use hotshot_types::{
data::{null_block, EpochNumber, PackedBundle, ViewNumber},
traits::{
node_implementation::{ConsensusTime, Versions},
},
traits::node_implementation::{ConsensusTime, Versions},
};
use vbs::version::StaticVersionType;

Expand Down
6 changes: 3 additions & 3 deletions hotshot-testing/tests/tests_1/upgrade_task_with_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ async fn test_upgrade_task_with_proposal() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(1),
Some(EpochNumber::new(1)),
None,
version_1,
)
.await,
Expand All @@ -180,7 +180,7 @@ async fn test_upgrade_task_with_proposal() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(2),
Some(EpochNumber::new(1)),
None,
version_2,
)
.await,
Expand All @@ -200,7 +200,7 @@ async fn test_upgrade_task_with_proposal() {
build_payload_commitment::<TestTypes, TestVersions>(
&membership,
ViewNumber::new(3),
Some(EpochNumber::new(1)),
None,
version_3,
)
.await,
Expand Down
6 changes: 6 additions & 0 deletions hotshot-types/src/traits/election.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,10 @@ pub trait Membership<TYPES: NodeType>: Debug + Send + Sync {
/// Called to notify the Membership when a new DRB result has been calculated.
/// Observes the same semantics as add_epoch_root
fn add_drb_result(&mut self, _epoch: TYPES::Epoch, _drb_result: DrbResult);

/// Called to notify the Membership that Epochs are enabled.
/// Implementations should copy the pre-epoch stake table into epoch and epoch+1
/// when this is called. The value of initial_drb_result should be used for DRB
/// calculations for epochs (epoch+1) and earlier.
fn set_first_epoch(&mut self, _epoch: TYPES::Epoch, _initial_drb_result: DrbResult);
}
11 changes: 11 additions & 0 deletions hotshot/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub mod documentation;

use committable::Committable;
use futures::future::{select, Either};
use hotshot_types::drb::INITIAL_DRB_RESULT;
use hotshot_types::{
message::UpgradeLock,
traits::{network::BroadcastDelay, node_implementation::Versions},
Expand Down Expand Up @@ -297,6 +298,16 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> SystemContext<T
config.epoch_height,
);

if let Some(epoch) = epoch {
// #2652 REVIEW NOTE: This epoch can't be right; what if a node starts up between when we upgrade to epochs
// and when we're at epoch+2? This would cause the current node to propagate the non-epochs stake table
// up.
memberships
.write()
.await
.set_first_epoch(epoch, INITIAL_DRB_RESULT);
}

// Insert the validated state to state map.
let mut validated_state_map = BTreeMap::default();
validated_state_map.insert(
Expand Down
4 changes: 3 additions & 1 deletion hotshot/src/traits/election/randomized_committee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,5 +252,7 @@ impl<TYPES: NodeType> Membership<TYPES> for Committee<TYPES> {
.unwrap()
}

fn add_drb_result(&mut self, _epoch: <TYPES as NodeType>::Epoch, _drb: DrbResult) {}
fn add_drb_result(&mut self, _epoch: <TYPES as NodeType>::Epoch, _drb_result: DrbResult) {}

fn set_first_epoch(&mut self, _epoch: TYPES::Epoch, _initial_drb_result: DrbResult) {}
}
2 changes: 2 additions & 0 deletions hotshot/src/traits/election/randomized_committee_members.rs
Original file line number Diff line number Diff line change
Expand Up @@ -449,4 +449,6 @@ impl<TYPES: NodeType, CONFIG: QuorumFilterConfig> Membership<TYPES>
}

fn add_drb_result(&mut self, _epoch: <TYPES as NodeType>::Epoch, _drb_result: DrbResult) {}

fn set_first_epoch(&mut self, _epoch: TYPES::Epoch, _initial_drb_result: DrbResult) {}
}
Loading

0 comments on commit 4bf4660

Please sign in to comment.