Merge pull request #1669 from input-output-hk/djo/1648/ord_on_signed_…

…entity_type_discriminants Ord on signed entity type discriminants
input-output-hk · May 6, 2024 · 204c7e4 · 204c7e4
2 parents f36ddb2 + 3f3fb02
commit 204c7e4
Show file tree

Hide file tree

Showing 5 changed files with 75 additions and 23 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "mithril-aggregator"
-version = "0.5.2"
+version = "0.5.3"
 description = "A Mithril Aggregator server"
 authors = { workspace = true }
 edition = { workspace = true }

diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs
@@ -5,6 +5,7 @@ use mithril_common::crypto_helper::ProtocolGenesisSigner;
 use mithril_common::era::adapters::EraReaderAdapterType;
 use mithril_doc::{Documenter, DocumenterDefault, StructDoc};
 use serde::{Deserialize, Serialize};
+use std::collections::BTreeSet;
 use std::path::PathBuf;
 use std::str::FromStr;
 
@@ -267,11 +268,11 @@ impl Configuration {
     /// The signed entity types are discarded if they are not declared in the [SignedEntityType] enum.
     pub fn list_allowed_signed_entity_types_discriminants(
         &self,
-    ) -> StdResult<Vec<SignedEntityTypeDiscriminants>> {
-        let default_discriminants = vec![
+    ) -> StdResult<BTreeSet<SignedEntityTypeDiscriminants>> {
+        let default_discriminants = BTreeSet::from([
             SignedEntityTypeDiscriminants::MithrilStakeDistribution,
             SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
-        ];
+        ]);
 
         let mut all_discriminants = default_discriminants;
 
@@ -280,9 +281,7 @@ impl Configuration {
             .split(',')
             .filter_map(|name| SignedEntityTypeDiscriminants::from_str(name.trim()).ok())
         {
-            if !all_discriminants.contains(&discriminant) {
-                all_discriminants.push(discriminant)
-            }
+            all_discriminants.insert(discriminant);
         }
 
         Ok(all_discriminants)
@@ -538,10 +537,10 @@ mod test {
             .unwrap();
 
         assert_eq!(
-            vec![
+            BTreeSet::from([
                 SignedEntityTypeDiscriminants::MithrilStakeDistribution,
                 SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
-            ],
+            ]),
             discriminants
         );
     }
@@ -559,10 +558,10 @@ mod test {
             .unwrap();
 
         assert_eq!(
-            vec![
+            BTreeSet::from([
                 SignedEntityTypeDiscriminants::MithrilStakeDistribution,
                 SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
-            ],
+            ]),
             discriminants
         );
     }
@@ -583,10 +582,10 @@ mod test {
             .unwrap();
 
         assert_eq!(
-            vec![
+            BTreeSet::from([
                 SignedEntityTypeDiscriminants::MithrilStakeDistribution,
                 SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
-            ],
+            ]),
             discriminants
         );
     }
@@ -604,12 +603,12 @@ mod test {
             .unwrap();
 
         assert_eq!(
-            vec![
+            BTreeSet::from([
                 SignedEntityTypeDiscriminants::MithrilStakeDistribution,
                 SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
                 SignedEntityTypeDiscriminants::CardanoStakeDistribution,
                 SignedEntityTypeDiscriminants::CardanoTransactions,
-            ],
+            ]),
             discriminants
         );
     }
@@ -630,11 +629,11 @@ mod test {
             .unwrap();
 
         assert_eq!(
-            vec![
+            BTreeSet::from([
                 SignedEntityTypeDiscriminants::MithrilStakeDistribution,
-                SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
                 SignedEntityTypeDiscriminants::CardanoStakeDistribution,
-            ],
+                SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
+            ]),
             discriminants
         );
     }
@@ -657,8 +656,8 @@ mod test {
         assert_eq!(
             vec![
                 SignedEntityType::MithrilStakeDistribution(beacon.epoch),
-                SignedEntityType::CardanoImmutableFilesFull(beacon.clone()),
                 SignedEntityType::CardanoStakeDistribution(beacon.epoch),
+                SignedEntityType::CardanoImmutableFilesFull(beacon.clone()),
                 SignedEntityType::CardanoTransactions(beacon.clone()),
             ],
             signed_entity_types

diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "mithril-common"
-version = "0.4.2"
+version = "0.4.3"
 description = "Common types, interfaces, and utilities for Mithril nodes."
 authors = { workspace = true }
 edition = { workspace = true }

diff --git a/mithril-common/src/entities/signed_entity_type.rs b/mithril-common/src/entities/signed_entity_type.rs
@@ -26,6 +26,7 @@ const ENTITY_TYPE_CARDANO_TRANSACTIONS: usize = 3;
 /// are identified by their discriminant (i.e. index in the enum), thus the
 /// modification of this type should only ever consist of appending new
 /// variants.
+// Important note: The order of the variants is important as it is used for the derived Ord trait.
 #[derive(Display, Debug, Clone, PartialEq, Eq, Serialize, Deserialize, EnumDiscriminants)]
 #[strum(serialize_all = "PascalCase")]
 #[strum_discriminants(derive(EnumString, AsRefStr, Serialize, Deserialize, PartialOrd, Ord))]
@@ -162,4 +163,56 @@ impl SignedEntityTypeDiscriminants {
 }
 
 #[cfg(test)]
-mod tests {}
+mod tests {
+    use super::*;
+
+    // Expected ord:
+    // MithrilStakeDistribution < CardanoStakeDistribution < CardanoImmutableFilesFull < CardanoTransactions
+    #[test]
+    fn ordering_discriminant() {
+        let mut list = vec![
+            SignedEntityTypeDiscriminants::CardanoStakeDistribution,
+            SignedEntityTypeDiscriminants::CardanoTransactions,
+            SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
+            SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+        ];
+        list.sort();
+
+        assert_eq!(
+            list,
+            vec![
+                SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+                SignedEntityTypeDiscriminants::CardanoStakeDistribution,
+                SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
+                SignedEntityTypeDiscriminants::CardanoTransactions,
+            ]
+        );
+    }
+
+    #[test]
+    fn ordering_discriminant_with_duplicate() {
+        let mut list = vec![
+            SignedEntityTypeDiscriminants::CardanoStakeDistribution,
+            SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+            SignedEntityTypeDiscriminants::CardanoTransactions,
+            SignedEntityTypeDiscriminants::CardanoStakeDistribution,
+            SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
+            SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+            SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+        ];
+        list.sort();
+
+        assert_eq!(
+            list,
+            vec![
+                SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+                SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+                SignedEntityTypeDiscriminants::MithrilStakeDistribution,
+                SignedEntityTypeDiscriminants::CardanoStakeDistribution,
+                SignedEntityTypeDiscriminants::CardanoStakeDistribution,
+                SignedEntityTypeDiscriminants::CardanoImmutableFilesFull,
+                SignedEntityTypeDiscriminants::CardanoTransactions,
+            ]
+        );
+    }
+}