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 36d8702 commit a97d405
Show file tree
Hide file tree
Showing 13 changed files with 148 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
38 changes: 19 additions & 19 deletions hotshot-testing/tests/tests_1/quorum_proposal_task.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ 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::{
election::Membership,
Expand Down Expand Up @@ -60,7 +60,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 @@ -100,7 +100,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_1() {
membership
.read()
.await
.total_nodes(Some(EpochNumber::new(1))),
.total_nodes(None),
<TestVersions as Versions>::Base::VERSION,
*ViewNumber::new(1),
)
Expand Down Expand Up @@ -195,7 +195,7 @@ async fn test_quorum_proposal_task_quorum_proposal_view_gt_1() {
membership
.read()
.await
.total_nodes(Some(EpochNumber::new(1))),
.total_nodes(None),
<TestVersions as Versions>::Base::VERSION,
*ViewNumber::new(1),
)
Expand All @@ -215,7 +215,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 @@ -236,7 +236,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 @@ -255,7 +255,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 @@ -274,7 +274,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 @@ -293,7 +293,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 @@ -347,7 +347,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 @@ -400,7 +400,7 @@ async fn test_quorum_proposal_task_qc_timeout() {
membership
.read()
.await
.total_nodes(Some(EpochNumber::new(1))),
.total_nodes(None),
<TestVersions as Versions>::Base::VERSION,
*ViewNumber::new(3),
)
Expand Down Expand Up @@ -446,7 +446,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 @@ -501,7 +501,7 @@ async fn test_quorum_proposal_task_view_sync() {
membership
.read()
.await
.total_nodes(Some(EpochNumber::new(1))),
.total_nodes(None),
<TestVersions as Versions>::Base::VERSION,
*ViewNumber::new(2),
)
Expand Down Expand Up @@ -571,7 +571,7 @@ async fn test_quorum_proposal_task_liveness_check() {
membership
.read()
.await
.total_nodes(Some(EpochNumber::new(1))),
.total_nodes(None),
<TestVersions as Versions>::Base::VERSION,
*ViewNumber::new(1),
)
Expand All @@ -595,7 +595,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 @@ -616,7 +616,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 @@ -635,7 +635,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 @@ -654,7 +654,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 @@ -673,7 +673,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
8 changes: 4 additions & 4 deletions hotshot-testing/tests/tests_1/upgrade_task_with_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ async fn test_upgrade_task_with_proposal() {
membership
.read()
.await
.total_nodes(Some(EpochNumber::new(1))),
.total_nodes(None),
<TestVersions as Versions>::Base::VERSION,
*ViewNumber::new(1),
)
Expand Down Expand Up @@ -163,7 +163,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 @@ -184,7 +184,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 @@ -204,7 +204,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 a97d405

Please sign in to comment.