refactor(common, protocol-config, signer): switch from Hashmap to a Struct to represent SignedEntityConfiguration to avoid inconsistent data

turmelclem · turmelclem · commit 43c9c72b6418 · 2025-10-13T11:06:34.000+02:00
diff --git a/internal/mithril-protocol-config/src/http_client/http_impl.rs b/internal/mithril-protocol-config/src/http_client/http_impl.rs
@@ -2,10 +2,10 @@
 
 use anyhow::anyhow;
 use async_trait::async_trait;
-use std::{collections::HashMap, sync::Arc, time::Duration};
+use std::{sync::Arc, time::Duration};
 
+use mithril_common::StdResult;
 use mithril_common::api_version::APIVersionProvider;
-use mithril_common::{StdResult, entities::SignedEntityTypeDiscriminants};
 
 use crate::{
     aggregator_client::{AggregatorClient, AggregatorHTTPClient, HTTP_REQUEST_TIMEOUT_DURATION},
@@ -48,15 +48,14 @@ impl MithrilNetworkConfigurationProvider for HttpMithrilNetworkConfigurationProv
         let aggregator_features = self.aggregator_client.retrieve_aggregator_features().await?;
         let available_signed_entity_types = aggregator_features.capabilities.signed_entity_types;
 
-        let mut signed_entity_types_config = HashMap::new();
-        signed_entity_types_config.insert(
-            SignedEntityTypeDiscriminants::CardanoTransactions,
-            SignedEntityTypeConfiguration::CardanoTransactions(
-                epoch_settings.cardano_transactions_signing_config.ok_or_else(|| {
-                    anyhow!("Cardano transactions signing config is missing in epoch settings")
-                })?,
-            ),
-        );
+        let cardano_transactions =
+            epoch_settings.cardano_transactions_signing_config.ok_or_else(|| {
+                anyhow!("Cardano transactions signing config is missing in epoch settings")
+            })?;
+
+        let signed_entity_types_config = SignedEntityTypeConfiguration {
+            cardano_transactions: Some(cardano_transactions),
+        };
 
         Ok(MithrilNetworkConfiguration {
             epoch: epoch_settings.epoch,
diff --git a/internal/mithril-protocol-config/src/model.rs b/internal/mithril-protocol-config/src/model.rs
@@ -1,6 +1,6 @@
 //! Model definitions for Mithril Protocol Configuration.
 
-use std::collections::{BTreeSet, HashMap};
+use std::collections::BTreeSet;
 
 use mithril_common::entities::{
     CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityTypeDiscriminants,
@@ -9,9 +9,9 @@ use mithril_common::entities::{
 #[derive(PartialEq, Clone, Debug)]
 
 /// Custom configuration for a signed entity type
-pub enum SignedEntityTypeConfiguration {
+pub struct SignedEntityTypeConfiguration {
     /// Cardano Transactions
-    CardanoTransactions(CardanoTransactionsSigningConfig),
+    pub cardano_transactions: Option<CardanoTransactionsSigningConfig>,
 }
 
 /// A Mithril network configuration
@@ -27,70 +27,5 @@ pub struct MithrilNetworkConfiguration {
     pub available_signed_entity_types: BTreeSet<SignedEntityTypeDiscriminants>,
 
     /// Custom configurations for signed entity types
-    pub signed_entity_types_config:
-        HashMap<SignedEntityTypeDiscriminants, SignedEntityTypeConfiguration>,
-}
-
-impl MithrilNetworkConfiguration {
-    /// Get the Cardano Transactions signing configuration
-    pub fn get_cardano_transactions_signing_config(
-        &self,
-    ) -> Option<CardanoTransactionsSigningConfig> {
-        match self
-            .signed_entity_types_config
-            .get(&SignedEntityTypeDiscriminants::CardanoTransactions)
-        {
-            Some(SignedEntityTypeConfiguration::CardanoTransactions(config)) => {
-                Some(config.clone())
-            }
-            _ => None,
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use std::collections::HashMap;
-
-    use mithril_common::{
-        entities::{BlockNumber, CardanoTransactionsSigningConfig, SignedEntityTypeDiscriminants},
-        test::double::Dummy,
-    };
-
-    use crate::model::{MithrilNetworkConfiguration, SignedEntityTypeConfiguration};
-
-    #[test]
-    fn test_get_cardano_transactions_signing_config_should_return_config_if_cardano_transactions_exist()
-     {
-        let config = MithrilNetworkConfiguration {
-            signed_entity_types_config: HashMap::from([(
-                SignedEntityTypeDiscriminants::CardanoTransactions,
-                SignedEntityTypeConfiguration::CardanoTransactions(
-                    CardanoTransactionsSigningConfig {
-                        security_parameter: BlockNumber(42),
-                        step: BlockNumber(26),
-                    },
-                ),
-            )]),
-            ..Dummy::dummy()
-        };
-
-        let result = config.get_cardano_transactions_signing_config();
-        assert!(result.is_some());
-        let unwrapped_result = result.unwrap();
-        assert_eq!(unwrapped_result.security_parameter, BlockNumber(42));
-        assert_eq!(unwrapped_result.step, BlockNumber(26));
-    }
-
-    #[test]
-    fn test_get_cardano_transactions_signing_config_should_return_none_if_there_is_no_cardano_transactions()
-     {
-        let config = MithrilNetworkConfiguration {
-            signed_entity_types_config: HashMap::new(),
-            ..Dummy::dummy()
-        };
-
-        let result = config.get_cardano_transactions_signing_config();
-        assert!(result.is_none());
-    }
+    pub signed_entity_types_config: SignedEntityTypeConfiguration,
 }
diff --git a/internal/mithril-protocol-config/src/test/double/configuration_provider.rs b/internal/mithril-protocol-config/src/test/double/configuration_provider.rs
@@ -1,8 +1,5 @@
 //! provides test doubles for MithrilNetworkConfigurationProvider
-use std::{
-    collections::{BTreeSet, HashMap},
-    sync::Arc,
-};
+use std::{collections::BTreeSet, sync::Arc};
 
 use tokio::sync::RwLock;
 
@@ -26,8 +23,7 @@ pub struct FakeMithrilNetworkConfigurationProvider {
     pub available_signed_entity_types: RwLock<BTreeSet<SignedEntityTypeDiscriminants>>,
 
     /// The configuration for each signed entity type
-    pub signed_entity_types_config:
-        HashMap<SignedEntityTypeDiscriminants, SignedEntityTypeConfiguration>,
+    pub signed_entity_types_config: SignedEntityTypeConfiguration,
 
     ticker_service: Arc<MithrilTickerService>,
 }
@@ -37,10 +33,7 @@ impl FakeMithrilNetworkConfigurationProvider {
     pub fn new(
         signer_registration_protocol_parameters: ProtocolParameters,
         available_signed_entity_types: BTreeSet<SignedEntityTypeDiscriminants>,
-        signed_entity_types_config: HashMap<
-            SignedEntityTypeDiscriminants,
-            SignedEntityTypeConfiguration,
-        >,
+        signed_entity_types_config: SignedEntityTypeConfiguration,
         ticker_service: Arc<MithrilTickerService>,
     ) -> Self {
         Self {
@@ -81,10 +74,7 @@ impl MithrilNetworkConfigurationProvider for FakeMithrilNetworkConfigurationProv
 
 #[cfg(test)]
 mod tests {
-    use std::{
-        collections::{BTreeSet, HashMap},
-        sync::Arc,
-    };
+    use std::{collections::BTreeSet, sync::Arc};
 
     use mithril_common::{
         entities::{
@@ -128,13 +118,12 @@ mod tests {
             SignedEntityTypeDiscriminants::MithrilStakeDistribution,
             SignedEntityTypeDiscriminants::CardanoTransactions,
         ]);
-        let signed_entity_types_config = HashMap::from([(
-            SignedEntityTypeDiscriminants::CardanoTransactions,
-            SignedEntityTypeConfiguration::CardanoTransactions(CardanoTransactionsSigningConfig {
+        let signed_entity_types_config = SignedEntityTypeConfiguration {
+            cardano_transactions: Some(CardanoTransactionsSigningConfig {
                 security_parameter: BlockNumber(12),
                 step: BlockNumber(10),
             }),
-        )]);
+        };
 
         let mithril_network_configuration_provider = FakeMithrilNetworkConfigurationProvider::new(
             signer_registration_protocol_parameters.clone(),
@@ -151,11 +140,7 @@ mod tests {
         assert_eq!(actual_config.epoch, Epoch(1));
         assert_eq!(
             actual_config.signer_registration_protocol_parameters,
-            ProtocolParameters {
-                k: 2,
-                m: 3,
-                phi_f: 0.5
-            }
+            signer_registration_protocol_parameters
         );
         assert_eq!(
             actual_config.available_signed_entity_types,
diff --git a/internal/mithril-protocol-config/src/test/double/dummies.rs b/internal/mithril-protocol-config/src/test/double/dummies.rs
@@ -1,4 +1,4 @@
-use std::collections::{BTreeSet, HashMap};
+use std::collections::BTreeSet;
 
 use mithril_common::{
     entities::{CardanoTransactionsSigningConfig, SignedEntityTypeDiscriminants},
@@ -15,13 +15,9 @@ impl Dummy for MithrilNetworkConfiguration {
         let mut available_signed_entity_types = BTreeSet::new();
 
         available_signed_entity_types.insert(SignedEntityTypeDiscriminants::CardanoTransactions);
-        let mut signed_entity_types_config = HashMap::new();
-        signed_entity_types_config.insert(
-            SignedEntityTypeDiscriminants::CardanoTransactions,
-            SignedEntityTypeConfiguration::CardanoTransactions(
-                CardanoTransactionsSigningConfig::dummy(),
-            ),
-        );
+        let signed_entity_types_config = SignedEntityTypeConfiguration {
+            cardano_transactions: Some(CardanoTransactionsSigningConfig::dummy()),
+        };
 
         Self {
             epoch: beacon.epoch,
diff --git a/mithril-common/src/entities/signed_entity_type.rs b/mithril-common/src/entities/signed_entity_type.rs
@@ -49,7 +49,6 @@ const ENTITY_TYPE_CARDANO_DATABASE: usize = 4;
     PartialOrd,
     Ord,
     EnumIter,
-    Hash,
 ))]
 pub enum SignedEntityType {
     /// Mithril stake distribution
diff --git a/mithril-signer/src/runtime/runner.rs b/mithril-signer/src/runtime/runner.rs
@@ -360,7 +360,7 @@ mod tests {
     use mithril_protocol_config::model::SignedEntityTypeConfiguration;
     use mockall::mock;
     use mockall::predicate::eq;
-    use std::collections::{BTreeSet, HashMap};
+    use std::collections::BTreeSet;
     use std::{path::Path, sync::Arc};
     use tokio::sync::RwLock;
 
@@ -545,12 +545,9 @@ mod tests {
         let network_configuration_service = Arc::new(FakeMithrilNetworkConfigurationProvider::new(
             ProtocolParameters::new(1000, 100, 0.1234),
             SignedEntityTypeDiscriminants::all(),
-            HashMap::from([(
-                SignedEntityTypeDiscriminants::CardanoTransactions,
-                SignedEntityTypeConfiguration::CardanoTransactions(
-                    CardanoTransactionsSigningConfig::dummy(),
-                ),
-            )]),
+            SignedEntityTypeConfiguration {
+                cardano_transactions: Some(CardanoTransactionsSigningConfig::dummy()),
+            },
             ticker_service.clone(),
         ));
 
diff --git a/mithril-signer/src/runtime/state_machine.rs b/mithril-signer/src/runtime/state_machine.rs
@@ -489,9 +489,14 @@ impl StateMachine {
 mod tests {
     use anyhow::anyhow;
     use chrono::DateTime;
-    use mithril_protocol_config::model::MithrilNetworkConfiguration;
-
-    use mithril_common::entities::{ChainPoint, Epoch, ProtocolMessage, SignedEntityType};
+    use mithril_protocol_config::model::{
+        MithrilNetworkConfiguration, SignedEntityTypeConfiguration,
+    };
+
+    use mithril_common::entities::{
+        CardanoTransactionsSigningConfig, ChainPoint, Epoch, ProtocolMessage, SignedEntityType,
+        SignedEntityTypeDiscriminants,
+    };
     use mithril_common::test::double::{Dummy, fake_data};
 
     use crate::SignerEpochSettings;
@@ -565,8 +570,10 @@ mod tests {
                 Ok(MithrilNetworkConfiguration {
                     epoch: Epoch(999),
                     signer_registration_protocol_parameters: fake_data::protocol_parameters(),
-                    available_signed_entity_types: Default::default(),
-                    signed_entity_types_config: Default::default(),
+                    available_signed_entity_types: SignedEntityTypeDiscriminants::all(),
+                    signed_entity_types_config: SignedEntityTypeConfiguration {
+                        cardano_transactions: Some(CardanoTransactionsSigningConfig::dummy()),
+                    },
                 })
             });
         runner.expect_get_current_time_point().once().returning(|| {
diff --git a/mithril-signer/src/services/epoch_service.rs b/mithril-signer/src/services/epoch_service.rs
@@ -192,7 +192,8 @@ impl EpochService for MithrilEpochService {
             mithril_network_configuration.available_signed_entity_types.clone();
 
         let cardano_transactions_signing_config = mithril_network_configuration
-            .get_cardano_transactions_signing_config()
+            .signed_entity_types_config
+            .cardano_transactions
             .clone();
 
         self.epoch_data = Some(EpochData {
@@ -424,7 +425,6 @@ pub(crate) mod mock_epoch_service {
 #[cfg(test)]
 mod tests {
     use mithril_protocol_config::model::SignedEntityTypeConfiguration;
-    use std::collections::HashMap;
     use std::sync::Arc;
     use tokio::sync::RwLock;
 
@@ -727,7 +727,9 @@ mod tests {
 
         // Check cardano_transactions_signing_config
         assert_eq!(
-            mithril_network_configuration.get_cardano_transactions_signing_config(),
+            mithril_network_configuration
+                .signed_entity_types_config
+                .cardano_transactions,
             *service.cardano_transactions_signing_config().unwrap()
         );
     }
@@ -895,7 +897,9 @@ mod tests {
                     fake_data::beacon().epoch,
                     MithrilNetworkConfiguration {
                         available_signed_entity_types: BTreeSet::new(),
-                        signed_entity_types_config: HashMap::new(),
+                        signed_entity_types_config: SignedEntityTypeConfiguration {
+                            cardano_transactions: None,
+                        },
                         ..MithrilNetworkConfiguration::dummy()
                     },
                     current_signers,
@@ -914,13 +918,9 @@ mod tests {
             let allowed_discriminants =
                 BTreeSet::from([SignedEntityTypeDiscriminants::CardanoImmutableFilesFull]);
 
-            let mut signed_entity_types_config = HashMap::new();
-            signed_entity_types_config.insert(
-                SignedEntityTypeDiscriminants::CardanoTransactions,
-                SignedEntityTypeConfiguration::CardanoTransactions(
-                    CardanoTransactionsSigningConfig::dummy(),
-                ),
-            );
+            let signed_entity_types_config = SignedEntityTypeConfiguration {
+                cardano_transactions: Some(CardanoTransactionsSigningConfig::dummy()),
+            };
 
             // Signers
             let signers = fake_data::signers(5);
diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs
@@ -9,7 +9,7 @@ use prometheus_parse::Value;
 use slog::Drain;
 use slog_scope::debug;
 use std::{
-    collections::{BTreeMap, BTreeSet, HashMap},
+    collections::{BTreeMap, BTreeSet},
     fmt::Debug,
     ops::RangeInclusive,
     path::Path,
@@ -177,12 +177,9 @@ impl StateMachineTester {
         let network_configuration_service = Arc::new(FakeMithrilNetworkConfigurationProvider::new(
             fake_data::protocol_parameters(),
             SignedEntityTypeDiscriminants::all(),
-            HashMap::from([(
-                SignedEntityTypeDiscriminants::CardanoTransactions,
-                SignedEntityTypeConfiguration::CardanoTransactions(
-                    cardano_transactions_signing_config.clone(),
-                ),
-            )]),
+            SignedEntityTypeConfiguration {
+                cardano_transactions: Some(cardano_transactions_signing_config.clone()),
+            },
             ticker_service.clone(),
         ));
 
