Improve building of EpochSettingMessage

Author: sfauvel

mithril-aggregator/src/http_server/routes/epoch_routes.rs

@@ -22,9 +22,7 @@ fn epoch_settings(
 mod handlers {
     use crate::dependency_injection::EpochServiceWrapper;
     use crate::http_server::routes::reply;
-    use crate::ToEpochSettingsMessageAdapter;
-    use mithril_common::entities::{EpochSettings, Signer};
-    use mithril_common::messages::ToMessageAdapter;
+    use mithril_common::messages::{EpochSettingsMessage, SignerMessagePart};
     use slog_scope::{debug, warn};
     use std::convert::Infallible;
     use warp::http::StatusCode;
@@ -40,32 +38,30 @@ mod handlers {
             epoch_service.epoch_of_current_data(),
             epoch_service.next_protocol_parameters(),
             epoch_service.upcoming_protocol_parameters(),
+            epoch_service.current_signers(),
+            epoch_service.next_signers(),
         ) {
-            (Ok(epoch), Ok(protocol_parameters), Ok(next_protocol_parameters)) => {
-                let epoch_settings = EpochSettings {
+            (
+                Ok(epoch),
+                Ok(protocol_parameters),
+                Ok(next_protocol_parameters),
+                Ok(current_signers),
+                Ok(next_signers),
+            ) => {
+                let epoch_settings_message = EpochSettingsMessage {
                     epoch,
                     protocol_parameters: protocol_parameters.clone(),
                     next_protocol_parameters: next_protocol_parameters.clone(),
-                    // TODO : create a test to check the affectation ?
-                    // TODO : create current_signers() and next_signers() in EpochService (with "cache")
-                    // TODO : How to handle unwrap  ?
-                    current_signers: epoch_service
-                        .current_signers_with_stake()
-                        .unwrap()
-                        .iter()
-                        .map(|s| Signer::from(s.clone()))
-                        .collect(),
-                    next_signers: epoch_service
-                        .next_signers_with_stake()
-                        .unwrap()
-                        .iter()
-                        .map(|s| Signer::from(s.clone()))
-                        .collect(),
+                    current_signers: SignerMessagePart::from_signers(current_signers.to_vec()),
+                    next_signers: SignerMessagePart::from_signers(next_signers.to_vec()),
                 };
-                let epoch_settings_message = ToEpochSettingsMessageAdapter::adapt(epoch_settings);
                 Ok(reply::json(&epoch_settings_message, StatusCode::OK))
             }
-            (Err(err), _, _) | (_, Err(err), _) | (_, _, Err(err)) => {
+            (Err(err), _, _, _, _)
+            | (_, Err(err), _, _, _)
+            | (_, _, Err(err), _, _)
+            | (_, _, _, Err(err), _)
+            | (_, _, _, _, Err(err)) => {
                 warn!("epoch_settings::error"; "error" => ?err);
                 Ok(reply::server_error(err))
             }

mithril-aggregator/src/services/epoch_service.rs

@@ -346,12 +346,12 @@ impl FakeEpochService {
                 .unwrap()
                 .build_multi_signer();
         let next_protocol_multi_signer =
-            SignerBuilder::new(current_signers_with_stake, protocol_parameters)
-            // TODO Is it correct to use current_signers_with_stake here and not next_signers_with_stake ?
-            SignerBuilder::new(current_signers_with_stake, protocol_parameters)
-                .with_context(|| "Could not build protocol_multi_signer for epoch service")
-                .unwrap()
-                .build_multi_signer();
+        //    SignerBuilder::new(current_signers_with_stake, protocol_parameters)
+        // TODO Is it correct to use current_signers_with_stake here and not next_signers_with_stake ?
+        SignerBuilder::new(current_signers_with_stake, protocol_parameters)
+            .with_context(|| "Could not build protocol_multi_signer for epoch service")
+            .unwrap()
+            .build_multi_signer();
 
         let current_signers_with_stake = current_signers_with_stake.to_vec();
         let next_signers_with_stake = next_signers_with_stake.to_vec();

mithril-common/src/messages/message_parts/signer.rs

@@ -210,6 +210,11 @@ impl SignerMessagePart {
         Ok(signers)
     }
 
+    /// Convert a set of signers into message parts
+    pub fn from_signers(signers: Vec<Signer>) -> Vec<Self> {
+        signers.into_iter().map(|signer| signer.into()).collect()
+    }
+
     cfg_test_tools! {
         /// Return a dummy test entity (test-only).
         pub fn dummy() -> Self {
@@ -226,6 +231,22 @@ impl SignerMessagePart {
     }
 }
 
+impl From<Signer> for SignerMessagePart {
+    fn from(value: Signer) -> Self {
+        Self {
+            party_id: value.party_id,
+            verification_key: value.verification_key.try_into().unwrap(),
+            verification_key_signature: value
+                .verification_key_signature
+                .map(|k| k.try_into().unwrap()),
+            operational_certificate: value
+                .operational_certificate
+                .map(|op_cert| (op_cert.try_into().unwrap())),
+            kes_period: value.kes_period,
+        }
+    }
+}
+
 impl Debug for SignerWithStakeMessagePart {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         let should_be_exhaustive = f.alternate();