refactor(signer): move AggregatorClient::register_signer to a new SignerRegistrationPublisher trait

- Based on the same architecture than the `SignaturePublisher`
- with an http implementation directly on the shared `AggregatorHttpClient`
- add a spy implementation for the tests needs

Author: Alenar

mithril-signer/src/dependency_injection/builder.rs

@@ -513,7 +513,7 @@ impl<'a> DependenciesBuilder<'a> {
 
         let services = SignerDependencyContainer {
             ticker_service,
-            certificate_handler: aggregator_client,
+            certificate_handler: aggregator_client.clone(),
             chain_observer,
             digester,
             single_signer,
@@ -529,6 +529,7 @@ impl<'a> DependenciesBuilder<'a> {
             upkeep_service,
             epoch_service,
             certifier,
+            signer_registration_publisher: aggregator_client,
             kes_signer,
             network_configuration_service,
         };

mithril-signer/src/dependency_injection/containers.rs

@@ -16,7 +16,8 @@ use mithril_ticker::TickerService;
 
 use crate::MetricsService;
 use crate::services::{
-    AggregatorClient, CertifierService, EpochService, SingleSigner, UpkeepService,
+    AggregatorClient, CertifierService, EpochService, SignerRegistrationPublisher, SingleSigner,
+    UpkeepService,
 };
 use crate::store::ProtocolInitializerStorer;
 
@@ -85,6 +86,9 @@ pub struct SignerDependencyContainer {
     /// Certifier service
     pub certifier: Arc<dyn CertifierService>,
 
+    /// Signer registration publisher
+    pub signer_registration_publisher: Arc<dyn SignerRegistrationPublisher>,
+
     /// Kes signer service
     pub kes_signer: Option<Arc<dyn KesSigner>>,
 

mithril-signer/src/runtime/runner.rs

@@ -233,7 +233,7 @@ impl Runner for SignerRunner {
                 kes_period,
             );
             self.services
-                .certificate_handler
+                .signer_registration_publisher
                 .register_signer(epoch_offset_to_recording_epoch, &signer)
                 .await?;
 
@@ -417,6 +417,7 @@ mod tests {
         CardanoTransactionsImporter, DumbAggregatorClient, MithrilEpochService,
         MithrilSingleSigner, MockTransactionStore, MockUpkeepService, SignaturePublisherNoop,
         SignerCertifierService, SignerSignableSeedBuilder, SignerSignedEntityConfigProvider,
+        SpySignerRegistrationPublisher,
     };
     use crate::test_tools::TestLogger;
 
@@ -585,6 +586,7 @@ mod tests {
             upkeep_service,
             epoch_service,
             certifier,
+            signer_registration_publisher: Arc::new(SpySignerRegistrationPublisher::default()),
             kes_signer,
             network_configuration_service,
         }
@@ -659,8 +661,8 @@ mod tests {
     async fn test_register_signer_to_aggregator() {
         let mut services = init_services().await;
         let fixture = MithrilFixtureBuilder::default().with_signers(5).build();
-        let certificate_handler = Arc::new(DumbAggregatorClient::default());
-        services.certificate_handler = certificate_handler.clone();
+        let registration_publisher_spy = Arc::new(SpySignerRegistrationPublisher::default());
+        services.signer_registration_publisher = registration_publisher_spy.clone();
         let protocol_initializer_store = services.protocol_initializer_store.clone();
         let current_epoch = services.ticker_service.get_current_epoch().await.unwrap();
 
@@ -699,7 +701,7 @@ mod tests {
             .expect("registering a signer to the aggregator should not fail");
 
         let last_registered_signer_first_registration =
-            certificate_handler.get_last_registered_signer().await.unwrap();
+            registration_publisher_spy.get_last_registered_signer().await.unwrap();
         let maybe_protocol_initializer_first_registration = protocol_initializer_store
             .get_protocol_initializer(current_epoch.offset_to_recording_epoch())
             .await
@@ -709,7 +711,8 @@ mod tests {
             "A protocol initializer should have been registered at the 'Recording' epoch"
         );
 
-        let total_registered_signers = certificate_handler.get_total_registered_signers().await;
+        let total_registered_signers =
+            registration_publisher_spy.get_total_registered_signers().await;
         assert_eq!(1, total_registered_signers);
 
         runner
@@ -718,7 +721,7 @@ mod tests {
             .expect("registering a signer to the aggregator should not fail");
 
         let last_registered_signer_second_registration =
-            certificate_handler.get_last_registered_signer().await.unwrap();
+            registration_publisher_spy.get_last_registered_signer().await.unwrap();
         let maybe_protocol_initializer_second_registration = protocol_initializer_store
             .get_protocol_initializer(current_epoch.offset_to_recording_epoch())
             .await
@@ -733,7 +736,8 @@ mod tests {
             "The signer registration should be the same and should have been registered twice"
         );
 
-        let total_registered_signers = certificate_handler.get_total_registered_signers().await;
+        let total_registered_signers =
+            registration_publisher_spy.get_total_registered_signers().await;
         assert_eq!(1, total_registered_signers);
     }
 

mithril-signer/src/services/aggregator_client.rs

@@ -1,17 +1,14 @@
 use async_trait::async_trait;
 
 use mithril_aggregator_client::AggregatorHttpClient;
-use mithril_aggregator_client::query::{
-    GetAggregatorFeaturesQuery, GetEpochSettingsQuery, PostRegisterSignerQuery,
-};
+use mithril_aggregator_client::query::{GetAggregatorFeaturesQuery, GetEpochSettingsQuery};
 use mithril_common::{
     StdResult,
-    entities::{Epoch, Signer},
-    messages::{AggregatorFeaturesMessage, TryFromMessageAdapter, TryToMessageAdapter},
+    messages::{AggregatorFeaturesMessage, TryFromMessageAdapter},
 };
 
 use crate::entities::SignerEpochSettings;
-use crate::message_adapters::{FromEpochSettingsAdapter, ToRegisterSignerMessageAdapter};
+use crate::message_adapters::FromEpochSettingsAdapter;
 
 /// Trait for mocking and testing a `AggregatorClient`
 #[cfg_attr(test, mockall::automock)]
@@ -20,9 +17,6 @@ pub trait AggregatorClient: Sync + Send {
     /// Retrieves epoch settings from the aggregator
     async fn retrieve_epoch_settings(&self) -> StdResult<Option<SignerEpochSettings>>;
 
-    /// Registers signer with the aggregator.
-    async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()>;
-
     /// Retrieves aggregator features message from the aggregator
     async fn retrieve_aggregator_features(&self) -> StdResult<AggregatorFeaturesMessage>;
 }
@@ -36,16 +30,6 @@ impl AggregatorClient for AggregatorHttpClient {
         Ok(Some(epoch_settings))
     }
 
-    async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()> {
-        let register_signer_message =
-            ToRegisterSignerMessageAdapter::try_adapt((epoch, signer.to_owned()))?;
-
-        self.send(PostRegisterSignerQuery::new(register_signer_message))
-            .await?;
-
-        Ok(())
-    }
-
     async fn retrieve_aggregator_features(&self) -> StdResult<AggregatorFeaturesMessage> {
         let aggregator_features = self.send(GetAggregatorFeaturesQuery::current()).await?;
         Ok(aggregator_features)
@@ -64,30 +48,14 @@ pub(crate) mod dumb {
     /// It is driven by a Tester that controls the data it can return, and it can return its internal state for testing.
     pub struct DumbAggregatorClient {
         epoch_settings: RwLock<Option<SignerEpochSettings>>,
-        last_registered_signer: RwLock<Option<Signer>>,
         aggregator_features: RwLock<AggregatorFeaturesMessage>,
-        total_registered_signers: RwLock<u32>,
-    }
-
-    impl DumbAggregatorClient {
-        /// Return the last signer that called with the `register` method.
-        pub async fn get_last_registered_signer(&self) -> Option<Signer> {
-            self.last_registered_signer.read().await.clone()
-        }
-
-        /// Return the total number of signers that called with the `register` method.
-        pub async fn get_total_registered_signers(&self) -> u32 {
-            *self.total_registered_signers.read().await
-        }
     }
 
     impl Default for DumbAggregatorClient {
         fn default() -> Self {
             Self {
                 epoch_settings: RwLock::new(Some(SignerEpochSettings::dummy())),
-                last_registered_signer: RwLock::new(None),
                 aggregator_features: RwLock::new(AggregatorFeaturesMessage::dummy()),
-                total_registered_signers: RwLock::new(0),
             }
         }
     }
@@ -100,18 +68,6 @@ pub(crate) mod dumb {
             Ok(epoch_settings)
         }
 
-        /// Registers signer with the aggregator
-        async fn register_signer(&self, _epoch: Epoch, signer: &Signer) -> StdResult<()> {
-            let mut last_registered_signer = self.last_registered_signer.write().await;
-            let signer = signer.clone();
-            *last_registered_signer = Some(signer);
-
-            let mut total_registered_signers = self.total_registered_signers.write().await;
-            *total_registered_signers += 1;
-
-            Ok(())
-        }
-
         async fn retrieve_aggregator_features(&self) -> StdResult<AggregatorFeaturesMessage> {
             let aggregator_features = self.aggregator_features.read().await;
             Ok(aggregator_features.clone())

mithril-signer/src/services/mod.rs

@@ -15,6 +15,7 @@ mod certifier;
 mod epoch_service;
 mod signable_builder;
 mod signature_publisher;
+mod signer_registration;
 mod single_signer;
 mod upkeep_service;
 
@@ -26,5 +27,6 @@ pub use certifier::*;
 pub use epoch_service::*;
 pub use signable_builder::*;
 pub use signature_publisher::*;
+pub use signer_registration::*;
 pub use single_signer::*;
 pub use upkeep_service::*;

mithril-signer/src/services/signer_registration/http.rs

@@ -0,0 +1,25 @@
+use anyhow::Context;
+
+use mithril_aggregator_client::{AggregatorHttpClient, query::PostRegisterSignerQuery};
+use mithril_common::{
+    StdResult,
+    entities::{Epoch, Signer},
+    messages::TryToMessageAdapter,
+};
+
+use crate::ToRegisterSignerMessageAdapter;
+use crate::services::SignerRegistrationPublisher;
+
+#[async_trait::async_trait]
+impl SignerRegistrationPublisher for AggregatorHttpClient {
+    async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()> {
+        let register_signer_message =
+            ToRegisterSignerMessageAdapter::try_adapt((epoch, signer.to_owned()))
+                .with_context(|| "Failed to adapt message to register signer message")?;
+
+        self.send(PostRegisterSignerQuery::new(register_signer_message))
+            .await?;
+
+        Ok(())
+    }
+}

mithril-signer/src/services/signer_registration/interface.rs

@@ -0,0 +1,10 @@
+use mithril_common::StdResult;
+use mithril_common::entities::{Epoch, Signer};
+
+/// Publishes a signer registration to a third party.
+#[cfg_attr(test, mockall::automock)]
+#[async_trait::async_trait]
+pub trait SignerRegistrationPublisher: Send + Sync {
+    /// Registers signer with the aggregator.
+    async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()>;
+}

mithril-signer/src/services/signer_registration/mod.rs

@@ -0,0 +1,8 @@
+mod http;
+mod interface;
+#[cfg(test)]
+mod spy;
+
+pub use interface::*;
+#[cfg(test)]
+pub use spy::*;

mithril-signer/src/services/signer_registration/spy.rs

@@ -0,0 +1,47 @@
+use tokio::sync::RwLock;
+
+use mithril_common::StdResult;
+use mithril_common::entities::{Epoch, Signer};
+
+use crate::services::SignerRegistrationPublisher;
+
+/// A spy implementation of the `SignerRegistrationPublisher` trait for testing
+pub struct SpySignerRegistrationPublisher {
+    last_registered_signer: RwLock<Option<Signer>>,
+    total_registered_signers: RwLock<u32>,
+}
+
+impl SpySignerRegistrationPublisher {
+    /// Return the last signer that called with the `register` method.
+    pub async fn get_last_registered_signer(&self) -> Option<Signer> {
+        self.last_registered_signer.read().await.clone()
+    }
+
+    /// Return the total number of signers that called with the `register` method.
+    pub async fn get_total_registered_signers(&self) -> u32 {
+        *self.total_registered_signers.read().await
+    }
+}
+
+impl Default for SpySignerRegistrationPublisher {
+    fn default() -> Self {
+        Self {
+            last_registered_signer: RwLock::new(None),
+            total_registered_signers: RwLock::new(0),
+        }
+    }
+}
+
+#[async_trait::async_trait]
+impl SignerRegistrationPublisher for SpySignerRegistrationPublisher {
+    async fn register_signer(&self, _epoch: Epoch, signer: &Signer) -> StdResult<()> {
+        let mut last_registered_signer = self.last_registered_signer.write().await;
+        let signer = signer.clone();
+        *last_registered_signer = Some(signer);
+
+        let mut total_registered_signers = self.total_registered_signers.write().await;
+        *total_registered_signers += 1;
+
+        Ok(())
+    }
+}

mithril-signer/tests/test_extensions/certificate_handler.rs

@@ -14,7 +14,7 @@ use mithril_common::{
 };
 use mithril_ticker::{MithrilTickerService, TickerService};
 
-use mithril_signer::services::SignaturePublisher;
+use mithril_signer::services::{SignaturePublisher, SignerRegistrationPublisher};
 use mithril_signer::{entities::SignerEpochSettings, services::AggregatorClient};
 
 pub struct FakeAggregator {
@@ -93,6 +93,18 @@ impl SignaturePublisher for FakeAggregator {
     }
 }
 
+#[async_trait]
+impl SignerRegistrationPublisher for FakeAggregator {
+    async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()> {
+        let mut store = self.registered_signers.write().await;
+        let mut signers = store.get(&epoch).cloned().unwrap_or_default();
+        signers.push(signer.clone());
+        let _ = store.insert(epoch, signers);
+
+        Ok(())
+    }
+}
+
 #[async_trait]
 impl AggregatorClient for FakeAggregator {
     async fn retrieve_epoch_settings(&self) -> StdResult<Option<SignerEpochSettings>> {
@@ -112,16 +124,6 @@ impl AggregatorClient for FakeAggregator {
         }
     }
 
-    /// Registers signer with the aggregator
-    async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()> {
-        let mut store = self.registered_signers.write().await;
-        let mut signers = store.get(&epoch).cloned().unwrap_or_default();
-        signers.push(signer.clone());
-        let _ = store.insert(epoch, signers);
-
-        Ok(())
-    }
-
     async fn retrieve_aggregator_features(&self) -> StdResult<AggregatorFeaturesMessage> {
         let signed_entity_config = self.signed_entity_config.read().await;