From e13b151ef1f4d8306d5d193afe5d274480d438f2 Mon Sep 17 00:00:00 2001 From: muXxer Date: Wed, 30 Oct 2024 10:12:02 +0100 Subject: [PATCH] feat(node): Remove multiple versions of types (Part 3) (#3759) * 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 --- crates/iota-core/src/epoch/randomness.rs | 15 +--- crates/iota-core/src/state_accumulator.rs | 2 +- .../tests/protocol_version_tests.rs | 6 +- .../tests/traffic_control_tests.rs | 2 +- crates/iota-storage/tests/key_value_tests.rs | 2 - .../iota-types/src/iota_system_state/mod.rs | 12 ++-- .../simtest_iota_system_state_inner.rs | 10 +-- crates/iota-types/src/messages_consensus.rs | 12 ---- narwhal/types/src/consensus.rs | 71 ++----------------- 9 files changed, 23 insertions(+), 109 deletions(-) diff --git a/crates/iota-core/src/epoch/randomness.rs b/crates/iota-core/src/epoch/randomness.rs index 677f6552e5a..acbb37baad7 100644 --- a/crates/iota-core/src/epoch/randomness.rs +++ b/crates/iota-core/src/epoch/randomness.rs @@ -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), } 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 { - if let VersionedProcessedMessage::V1(msg) = self { - msg - } else { - panic!("BUG: expected message version is 1") + match self { + VersionedProcessedMessage::V1(msg) => msg, } } @@ -110,7 +104,6 @@ impl VersionedProcessedMessage { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum VersionedUsedProcessedMessages { - V0(), // deprecated V1(dkg_v1::UsedProcessedMessages), } @@ -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 diff --git a/crates/iota-core/src/state_accumulator.rs b/crates/iota-core/src/state_accumulator.rs index 755f624274a..7cfde29131f 100644 --- a/crates/iota-core/src/state_accumulator.rs +++ b/crates/iota-core/src/state_accumulator.rs @@ -396,7 +396,7 @@ impl StateAccumulatorV1 { epoch_store: Arc, last_checkpoint_of_epoch: CheckpointSequenceNumber, ) -> IotaResult { - 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"); diff --git a/crates/iota-e2e-tests/tests/protocol_version_tests.rs b/crates/iota-e2e-tests/tests/protocol_version_tests.rs index 2deda364079..55705a25d67 100644 --- a/crates/iota-e2e-tests/tests/protocol_version_tests.rs +++ b/crates/iota-e2e-tests/tests/protocol_version_tests.rs @@ -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] @@ -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( @@ -963,7 +963,7 @@ mod sim_only_tests { ) .unwrap(); } else { - panic!("Expecting SimTestDeepV2 type"); + panic!("Expecting SimTestDeepV1 type"); } } diff --git a/crates/iota-e2e-tests/tests/traffic_control_tests.rs b/crates/iota-e2e-tests/tests/traffic_control_tests.rs index 93104798017..285ee7e49e6 100644 --- a/crates/iota-e2e-tests/tests/traffic_control_tests.rs +++ b/crates/iota-e2e-tests/tests/traffic_control_tests.rs @@ -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 diff --git a/crates/iota-storage/tests/key_value_tests.rs b/crates/iota-storage/tests/key_value_tests.rs index f71ee6e9cbb..8f20904994f 100644 --- a/crates/iota-storage/tests/key_value_tests.rs +++ b/crates/iota-storage/tests/key_value_tests.rs @@ -432,9 +432,7 @@ mod simtests { }; use axum::{ - body::Body, extract::{Request, State}, - response::Response, routing::get, }; use iota_macros::sim_test; diff --git a/crates/iota-types/src/iota_system_state/mod.rs b/crates/iota-types/src/iota_system_state/mod.rs index bdadfcada9a..5e5634a53d5 100644 --- a/crates/iota-types/src/iota_system_state/mod.rs +++ b/crates/iota-types/src/iota_system_state/mod.rs @@ -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"); @@ -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. @@ -283,7 +283,7 @@ pub fn get_iota_system_state(object_store: &dyn ObjectStore) -> Result { @@ -296,7 +296,7 @@ pub fn get_iota_system_state(object_store: &dyn ObjectStore) -> Result Err(IotaError::IotaSystemStateRead(format!( "Unsupported IotaSystemState version: {}", @@ -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!( diff --git a/crates/iota-types/src/iota_system_state/simtest_iota_system_state_inner.rs b/crates/iota-types/src/iota_system_state/simtest_iota_system_state_inner.rs index 2cbba91fb62..e5b5a5376f8 100644 --- a/crates/iota-types/src/iota_system_state/simtest_iota_system_state_inner.rs +++ b/crates/iota-types/src/iota_system_state/simtest_iota_system_state_inner.rs @@ -335,14 +335,14 @@ impl IotaSystemStateTrait for SimTestIotaSystemStateShallowV1 { } #[derive(Debug, Serialize, Deserialize, Clone, Eq, PartialEq)] -pub struct SimTestValidatorSetDeepV2 { - pub active_validators: Vec, +pub struct SimTestValidatorSetDeepV1 { + pub active_validators: Vec, 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)] @@ -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()) @@ -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, diff --git a/crates/iota-types/src/messages_consensus.rs b/crates/iota-types/src/messages_consensus.rs index 4c0efa531ec..dd8821af98d 100644 --- a/crates/iota-types/src/messages_consensus.rs +++ b/crates/iota-types/src/messages_consensus.rs @@ -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), } #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum VersionedDkgConfirmation { - V0(), // deprecated V1(dkg::Confirmation), } 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()={}", @@ -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, } } @@ -265,7 +261,6 @@ impl VersionedDkgMessage { pub fn unwrap_v1(self) -> dkg_v1::Message { match self { VersionedDkgMessage::V1(msg) => msg, - _ => panic!("BUG: expected V1 message"), } } @@ -277,18 +272,12 @@ 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(), } } @@ -296,7 +285,6 @@ impl VersionedDkgConfirmation { pub fn unwrap_v1(&self) -> &dkg::Confirmation { match self { VersionedDkgConfirmation::V1(msg) => msg, - _ => panic!("BUG: expected V1 confirmation"), } } diff --git a/narwhal/types/src/consensus.rs b/narwhal/types/src/consensus.rs index 2633ca815d8..0e9c138addb 100644 --- a/narwhal/types/src/consensus.rs +++ b/narwhal/types/src/consensus.rs @@ -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, - /// 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 { - 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, /// The leader certificate's digest responsible of committing this sub-dag. @@ -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(), @@ -337,7 +293,7 @@ impl ConsensusCommitV2 { } } -impl ConsensusCommitAPI for ConsensusCommitV2 { +impl ConsensusCommitAPI for ConsensusCommitV1 { fn certificates(&self) -> Vec { self.certificates.clone() } @@ -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 { 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(), } } }