Skip to content

Commit

Permalink
feat(node): Remove multiple versions of types (Part 3) (#3759)
Browse files Browse the repository at this point in the history
* fix(node): remove deprecated `VersionedUsedProcessedMessages::V0`

* fix(node): remove deprecated `VersionedDkgMessage::V0` and `VersionedDkgConfirmation::V0()`

* fix(node): remove deprecated `CommittedSubDagShell`

* fix(node): fix versions in `IotaSystemState` enum

* fix(node): remove version from `AccumulateEpoch` monitored scope

* fix(node): remove unused imports in simtests
  • Loading branch information
muXxer authored Oct 30, 2024
1 parent d91f3d7 commit e13b151
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 109 deletions.
15 changes: 3 additions & 12 deletions crates/iota-core/src/epoch/randomness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,25 +57,19 @@ pub const SINGLETON_KEY: u64 = 0;
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
#[allow(clippy::large_enum_variant)]
pub enum VersionedProcessedMessage {
V0(), // deprecated
V1(dkg_v1::ProcessedMessage<PkG, EncG>),
}

impl VersionedProcessedMessage {
pub fn sender(&self) -> PartyId {
match self {
VersionedProcessedMessage::V0() => {
panic!("BUG: invalid VersionedProcessedMessage version V0")
}
VersionedProcessedMessage::V1(msg) => msg.message.sender,
}
}

pub fn unwrap_v1(self) -> dkg_v1::ProcessedMessage<PkG, EncG> {
if let VersionedProcessedMessage::V1(msg) = self {
msg
} else {
panic!("BUG: expected message version is 1")
match self {
VersionedProcessedMessage::V1(msg) => msg,
}
}

Expand Down Expand Up @@ -110,7 +104,6 @@ impl VersionedProcessedMessage {

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum VersionedUsedProcessedMessages {
V0(), // deprecated
V1(dkg_v1::UsedProcessedMessages<PkG, EncG>),
}

Expand All @@ -123,9 +116,7 @@ impl VersionedUsedProcessedMessages {
// All inputs are verified in add_confirmation, so we can assume they are of the
// correct version.
let rng = &mut StdRng::from_rng(OsRng).expect("RNG construction should not fail");
let VersionedUsedProcessedMessages::V1(msg) = self else {
panic!("BUG: invalid VersionedUsedProcessedMessages version")
};
let VersionedUsedProcessedMessages::V1(msg) = self;
party.complete_v1(
msg,
&confirmations
Expand Down
2 changes: 1 addition & 1 deletion crates/iota-core/src/state_accumulator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ impl StateAccumulatorV1 {
epoch_store: Arc<AuthorityPerEpochStore>,
last_checkpoint_of_epoch: CheckpointSequenceNumber,
) -> IotaResult<Accumulator> {
let _scope = monitored_scope("AccumulateEpochV2");
let _scope = monitored_scope("AccumulateEpoch");
let running_root = epoch_store
.get_running_root_accumulator(&last_checkpoint_of_epoch)?
.expect("Expected running root accumulator to exist up to last checkpoint of epoch");
Expand Down
6 changes: 3 additions & 3 deletions crates/iota-e2e-tests/tests/protocol_version_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ mod sim_only_tests {
system_state.system_state_version(),
IOTA_SYSTEM_STATE_SIM_TEST_SHALLOW_V1
);
assert!(matches!(system_state, IotaSystemState::SimTestShallowV2(_)));
assert!(matches!(system_state, IotaSystemState::SimTestShallowV1(_)));
}

#[sim_test]
Expand Down Expand Up @@ -948,7 +948,7 @@ mod sim_only_tests {
system_state.system_state_version(),
IOTA_SYSTEM_STATE_SIM_TEST_DEEP_V1
);
if let IotaSystemState::SimTestDeepV2(inner) = system_state {
if let IotaSystemState::SimTestDeepV1(inner) = system_state {
// Make sure we have 1 inactive validator for latter testing.
assert_eq!(inner.validators.inactive_validators.size, 1);
get_validator_from_table(
Expand All @@ -963,7 +963,7 @@ mod sim_only_tests {
)
.unwrap();
} else {
panic!("Expecting SimTestDeepV2 type");
panic!("Expecting SimTestDeepV1 type");
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/iota-e2e-tests/tests/traffic_control_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ async fn test_validator_traffic_control_ok() -> Result<(), anyhow::Error> {
connection_blocklist_ttl_sec: 1,
proxy_blocklist_ttl_sec: 5,
// In this test, the validator gRPC API is receiving some requests that don't count towards
// the policy and two requests that do (/iota.validator.Validator/CertifiedTransactionV2 for
// the policy and two requests that do (/iota.validator.Validator/CertifiedTransactionV1 for
// an already executed transaction). However, the counter is updated only after the response
// is generated, but the limit is checked before we handle the request, so at the end we end
// up with 2 handled requests even if the limit is set to 1 and only the subsequent request
Expand Down
2 changes: 0 additions & 2 deletions crates/iota-storage/tests/key_value_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -432,9 +432,7 @@ mod simtests {
};

use axum::{
body::Body,
extract::{Request, State},
response::Response,
routing::get,
};
use iota_macros::sim_test;
Expand Down
12 changes: 6 additions & 6 deletions crates/iota-types/src/iota_system_state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ mod simtest_iota_system_state_inner;
#[cfg(msim)]
use self::simtest_iota_system_state_inner::{
SimTestIotaSystemStateDeepV1, SimTestIotaSystemStateShallowV1, SimTestIotaSystemStateV1,
SimTestValidatorDeepV2, SimTestValidatorV1,
SimTestValidatorDeepV1, SimTestValidatorV1,
};

const IOTA_SYSTEM_STATE_WRAPPER_STRUCT_NAME: &IdentStr = ident_str!("IotaSystemState");
Expand Down Expand Up @@ -193,9 +193,9 @@ pub enum IotaSystemState {
#[cfg(msim)]
SimTestV1(SimTestIotaSystemStateV1),
#[cfg(msim)]
SimTestShallowV2(SimTestIotaSystemStateShallowV1),
SimTestShallowV1(SimTestIotaSystemStateShallowV1),
#[cfg(msim)]
SimTestDeepV2(SimTestIotaSystemStateDeepV1),
SimTestDeepV1(SimTestIotaSystemStateDeepV1),
}

/// This is the fixed type used by genesis.
Expand Down Expand Up @@ -283,7 +283,7 @@ pub fn get_iota_system_state(object_store: &dyn ObjectStore) -> Result<IotaSyste
))
},
)?;
Ok(IotaSystemState::SimTestShallowV2(result))
Ok(IotaSystemState::SimTestShallowV1(result))
}
#[cfg(msim)]
IOTA_SYSTEM_STATE_SIM_TEST_DEEP_V1 => {
Expand All @@ -296,7 +296,7 @@ pub fn get_iota_system_state(object_store: &dyn ObjectStore) -> Result<IotaSyste
))
},
)?;
Ok(IotaSystemState::SimTestDeepV2(result))
Ok(IotaSystemState::SimTestDeepV1(result))
}
_ => Err(IotaError::IotaSystemStateRead(format!(
"Unsupported IotaSystemState version: {}",
Expand Down Expand Up @@ -352,7 +352,7 @@ where
}
#[cfg(msim)]
IOTA_SYSTEM_STATE_SIM_TEST_DEEP_V1 => {
let validator: SimTestValidatorDeepV2 =
let validator: SimTestValidatorDeepV1 =
get_dynamic_field_from_store(object_store, versioned.id.id.bytes, &version)
.map_err(|err| {
IotaError::IotaSystemStateRead(format!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,14 +335,14 @@ impl IotaSystemStateTrait for SimTestIotaSystemStateShallowV1 {
}

#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
pub struct SimTestValidatorSetDeepV2 {
pub active_validators: Vec<SimTestValidatorDeepV2>,
pub struct SimTestValidatorSetDeepV1 {
pub active_validators: Vec<SimTestValidatorDeepV1>,
pub inactive_validators: Table,
pub extra_fields: Bag,
}

#[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)]
pub struct SimTestValidatorDeepV2 {
pub struct SimTestValidatorDeepV1 {
pub new_dummy_field: u64,
metadata: SimTestValidatorMetadataV1,
#[serde(skip)]
Expand All @@ -352,7 +352,7 @@ pub struct SimTestValidatorDeepV2 {
pub extra_fields: Bag,
}

impl SimTestValidatorDeepV2 {
impl SimTestValidatorDeepV1 {
pub fn verified_metadata(&self) -> &VerifiedSimTestValidatorMetadataV1 {
self.verified_metadata
.get_or_init(|| self.metadata.verify())
Expand All @@ -370,7 +370,7 @@ pub struct SimTestIotaSystemStateDeepV1 {
pub protocol_version: u64,
pub system_state_version: u64,
pub iota_treasury_cap: IotaTreasuryCap,
pub validators: SimTestValidatorSetDeepV2,
pub validators: SimTestValidatorSetDeepV1,
pub storage_fund: Balance,
pub parameters: SimTestSystemParametersV1,
pub reference_gas_price: u64,
Expand Down
12 changes: 0 additions & 12 deletions crates/iota-types/src/messages_consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,20 +220,17 @@ impl ConsensusTransactionKind {
#[derive(Clone, PartialEq, Eq, Serialize, Deserialize)]
#[allow(clippy::large_enum_variant)]
pub enum VersionedDkgMessage {
V0(), // deprecated
V1(dkg_v1::Message<bls12381::G2Element, bls12381::G2Element>),
}

#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
pub enum VersionedDkgConfirmation {
V0(), // deprecated
V1(dkg::Confirmation<bls12381::G2Element>),
}

impl Debug for VersionedDkgMessage {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
VersionedDkgMessage::V0() => write!(f, "Deprecated VersionedDkgMessage version 0"),
VersionedDkgMessage::V1(msg) => write!(
f,
"DKG V1 Message with sender={}, vss_pk.degree={}, encrypted_shares.len()={}",
Expand All @@ -248,7 +245,6 @@ impl Debug for VersionedDkgMessage {
impl VersionedDkgMessage {
pub fn sender(&self) -> u16 {
match self {
VersionedDkgMessage::V0() => panic!("BUG: invalid VersionedDkgMessage version"),
VersionedDkgMessage::V1(msg) => msg.sender,
}
}
Expand All @@ -265,7 +261,6 @@ impl VersionedDkgMessage {
pub fn unwrap_v1(self) -> dkg_v1::Message<bls12381::G2Element, bls12381::G2Element> {
match self {
VersionedDkgMessage::V1(msg) => msg,
_ => panic!("BUG: expected V1 message"),
}
}

Expand All @@ -277,26 +272,19 @@ impl VersionedDkgMessage {
impl VersionedDkgConfirmation {
pub fn sender(&self) -> u16 {
match self {
VersionedDkgConfirmation::V0() => {
panic!("BUG: invalid VersionedDkgConfimation version")
}
VersionedDkgConfirmation::V1(msg) => msg.sender,
}
}

pub fn num_of_complaints(&self) -> usize {
match self {
VersionedDkgConfirmation::V0() => {
panic!("BUG: invalid VersionedDkgConfimation version")
}
VersionedDkgConfirmation::V1(msg) => msg.complaints.len(),
}
}

pub fn unwrap_v1(&self) -> &dkg::Confirmation<bls12381::G2Element> {
match self {
VersionedDkgConfirmation::V1(msg) => msg,
_ => panic!("BUG: expected V1 confirmation"),
}
}

Expand Down
71 changes: 4 additions & 67 deletions narwhal/types/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,52 +263,8 @@ trait ConsensusCommitAPI {
fn commit_timestamp(&self) -> TimestampMs;
}

// TODO: remove once the upgrade has been rolled out. We want to keep only the
// CommittedSubDag
#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct CommittedSubDagShell {
/// The sequence of committed certificates' digests.
pub certificates: Vec<CertificateDigest>,
/// The leader certificate's digest responsible of committing this sub-dag.
pub leader: CertificateDigest,
/// The round of the leader
pub leader_round: Round,
/// Sequence number of the CommittedSubDag
pub sub_dag_index: SequenceNumber,
/// The so far calculated reputation score for nodes
pub reputation_score: ReputationScores,
}

impl ConsensusCommitAPI for CommittedSubDagShell {
fn certificates(&self) -> Vec<CertificateDigest> {
self.certificates.clone()
}

fn leader(&self) -> CertificateDigest {
self.leader
}

fn leader_round(&self) -> Round {
self.leader_round
}

fn sub_dag_index(&self) -> SequenceNumber {
self.sub_dag_index
}

fn reputation_score(&self) -> ReputationScores {
self.reputation_score.clone()
}

fn commit_timestamp(&self) -> TimestampMs {
// We explicitly return 0 as we don't have this information stored already. This
// will be handle accordingly to the CommittedSubdag struct.
0
}
}

#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct ConsensusCommitV2 {
pub struct ConsensusCommitV1 {
/// The sequence of committed certificates' digests.
pub certificates: Vec<CertificateDigest>,
/// The leader certificate's digest responsible of committing this sub-dag.
Expand All @@ -324,7 +280,7 @@ pub struct ConsensusCommitV2 {
pub commit_timestamp: TimestampMs,
}

impl ConsensusCommitV2 {
impl ConsensusCommitV1 {
pub fn from_sub_dag(sub_dag: &CommittedSubDag) -> Self {
Self {
certificates: sub_dag.certificates.iter().map(|x| x.digest()).collect(),
Expand All @@ -337,7 +293,7 @@ impl ConsensusCommitV2 {
}
}

impl ConsensusCommitAPI for ConsensusCommitV2 {
impl ConsensusCommitAPI for ConsensusCommitV1 {
fn certificates(&self) -> Vec<CertificateDigest> {
self.certificates.clone()
}
Expand Down Expand Up @@ -366,62 +322,43 @@ impl ConsensusCommitAPI for ConsensusCommitV2 {
#[derive(Serialize, Deserialize, Clone, Debug)]
#[enum_dispatch(ConsensusCommitAPI)]
pub enum ConsensusCommit {
V1(CommittedSubDagShell),
V2(ConsensusCommitV2),
V1(ConsensusCommitV1),
}

impl ConsensusCommit {
pub fn certificates(&self) -> Vec<CertificateDigest> {
match self {
ConsensusCommit::V1(sub_dag) => sub_dag.certificates(),
ConsensusCommit::V2(sub_dag) => sub_dag.certificates(),
}
}

pub fn leader(&self) -> CertificateDigest {
match self {
ConsensusCommit::V1(sub_dag) => sub_dag.leader(),
ConsensusCommit::V2(sub_dag) => sub_dag.leader(),
}
}

pub fn leader_round(&self) -> Round {
match self {
ConsensusCommit::V1(sub_dag) => sub_dag.leader_round(),
ConsensusCommit::V2(sub_dag) => sub_dag.leader_round(),
}
}

pub fn sub_dag_index(&self) -> SequenceNumber {
match self {
ConsensusCommit::V1(sub_dag) => sub_dag.sub_dag_index(),
ConsensusCommit::V2(sub_dag) => sub_dag.sub_dag_index(),
}
}

pub fn reputation_score(&self) -> ReputationScores {
match self {
ConsensusCommit::V1(sub_dag) => sub_dag.reputation_score(),
ConsensusCommit::V2(sub_dag) => sub_dag.reputation_score(),
}
}

pub fn commit_timestamp(&self) -> TimestampMs {
match self {
ConsensusCommit::V1(sub_dag) => sub_dag.commit_timestamp(),
ConsensusCommit::V2(sub_dag) => sub_dag.commit_timestamp(),
}
}
}

impl CommittedSubDagShell {
pub fn from_sub_dag(sub_dag: &CommittedSubDag) -> Self {
Self {
certificates: sub_dag.certificates.iter().map(|x| x.digest()).collect(),
leader: sub_dag.leader.digest(),
leader_round: sub_dag.leader.round(),
sub_dag_index: sub_dag.sub_dag_index,
reputation_score: sub_dag.reputation_score.clone(),
}
}
}
Expand Down

0 comments on commit e13b151

Please sign in to comment.