refactor: renaming functions and add logs

Author: sfauvel

docs/website/root/mithril/mithril-network/signer.md

@@ -56,7 +56,7 @@ In its initial version, the **Mithril signer** consists of a primary component:
 
 - A runtime powered by a state machine:
   - The runtime operates synchronously and is programmed to run at consistent intervals
-  - Four potential states exist: **INIT**, **UNREGISTERED**, **REGISTERED NOT ABLE TO SIGN** and **READY TO SIGN**.
+  - Four potential states exist: **INIT**, **UNREGISTERED**, **READY TO SIGN** and **REGISTERED NOT ABLE TO SIGN**.
   - The runtime effectively manages state transitions
   - The runtime's framework is depicted in the diagram below:
 

mithril-signer/src/runtime/runner.rs

@@ -268,10 +268,14 @@ impl Runner for SignerRunner {
             .await?
         {
             debug!(" > got protocol initializer for this epoch ({epoch})");
-            return epoch_service.can_signer_sign_current_epoch(
+            if epoch_service.is_signer_included_in_current_stake_distribution(
                 self.services.single_signer.get_party_id(),
                 protocol_initializer,
-            );
+            )? {
+                return Ok(true);
+            } else {
+                debug!(" > Signer not in current stake distribution. Can NOT sign");
+            }
         } else {
             warn!(" > NO protocol initializer found for this epoch ({epoch})",);
         }
@@ -280,11 +284,17 @@ impl Runner for SignerRunner {
     }
 
     async fn can_sign_signed_entity_type(&self, signed_entity_type: &SignedEntityType) -> bool {
-        !self
+        if self
             .services
             .signed_entity_type_lock
             .is_locked(signed_entity_type)
             .await
+        {
+            debug!(" > signed entity type is locked, can NOT sign");
+            false
+        } else {
+            true
+        }
     }
 
     async fn inform_epoch_settings(&self, epoch_settings: EpochSettings) -> StdResult<()> {
@@ -537,7 +547,7 @@ mod tests {
             Ok(vec![])
         }
 
-        fn can_signer_sign_current_epoch(
+        fn is_signer_included_in_current_stake_distribution(
             &self,
             _party_id: PartyId,
             _protocol_initializer: ProtocolInitializer,

mithril-signer/src/runtime/state_machine.rs

@@ -27,7 +27,7 @@ pub enum SignerState {
     ReadyToSign {
         /// Epoch when signer transited to the state.
         epoch: Epoch,
-        /// Last signed entity type that the signer signed since beginning of this state.
+        /// Last signed entity type that the signer signed on this epoch.
         last_signed_entity_type: Option<SignedEntityType>,
     },
 
@@ -92,6 +92,13 @@ pub struct StateMachine {
     metrics_service: Arc<MetricsService>,
 }
 
+enum ReadyToSignTransition {
+    ToReadyToSign(CertificatePending),
+    NoTransitionAlreadySigned,
+    NoTransitionCannotSignPendingCertificate,
+    NoTransitionNoPendingCertificate,
+}
+
 impl StateMachine {
     /// Create a new StateMachine instance.
     pub fn new(
@@ -213,15 +220,16 @@ impl StateMachine {
                                 nested_error: Some(e),
                             }
                         })?;
-                    if let Some(new_state) = self
-                        .ready_to_sign_certificate_next_state(
+                    if let ReadyToSignTransition::ToReadyToSign(certificate) = self
+                        .ready_to_sign_certificate_next_transition(
                             pending_certificate,
                             last_signed_entity_type,
-                            epoch,
                         )
-                        .await?
+                        .await
                     {
-                        *state = new_state
+                        *state = self
+                            .transition_from_ready_to_sign_to_ready_to_sign(*epoch, &certificate)
+                            .await?;
                     }
                 }
             },
@@ -233,12 +241,11 @@ impl StateMachine {
         Ok(())
     }
 
-    async fn ready_to_sign_certificate_next_state(
+    async fn ready_to_sign_certificate_next_transition(
         &self,
         certificate_pending: Option<CertificatePending>,
         last_signed_entity: &Option<SignedEntityType>,
-        epoch: &Epoch,
-    ) -> Result<Option<SignerState>, RuntimeError> {
+    ) -> ReadyToSignTransition {
         fn is_same_signed_entity_type(
             signed_entity_type: &Option<SignedEntityType>,
             certificate: &CertificatePending,
@@ -257,31 +264,26 @@ impl StateMachine {
 
         match certificate_pending {
             Some(certificate) if is_same_signed_entity_type(last_signed_entity, &certificate) => {
-                info!(" ⋅ same entity type, cannot sign pending certificate, waiting…");
-                Ok(None)
+                info!(" ⋅ same entity type, already signed the pending certificate, waiting…");
+                ReadyToSignTransition::NoTransitionAlreadySigned
             }
             Some(certificate) if can_sign_signed_entity_type(self, &certificate).await => {
                 info!(
-                    " ⋅ Epoch has NOT changed but there is a pending certificate";
+                    " ⋅ Epoch has NOT changed we can sign this certificate, transiting to READY_TO_SIGN";
                     "pending_certificate" => ?certificate,
                 );
-                info!(" → we can sign this certificate, transiting to READY_TO_SIGN");
-                Ok(Some(
-                    self.transition_from_ready_to_sign_to_ready_to_sign(*epoch, &certificate)
-                        .await?,
-                ))
+                ReadyToSignTransition::ToReadyToSign(certificate)
             }
             Some(certificate) => {
                 info!(
-                    " ⋅ Epoch has NOT changed but there is a pending certificate";
+                    " ⋅ Epoch has NOT changed but cannot sign this pending certificate, waiting…";
                     "pending_certificate" => ?certificate
                 );
-                info!(" ⋅ cannot sign this pending certificate, waiting…");
-                Ok(None)
+                ReadyToSignTransition::NoTransitionCannotSignPendingCertificate
             }
             None => {
                 info!(" ⋅ no pending certificate, waiting…");
-                Ok(None)
+                ReadyToSignTransition::NoTransitionNoPendingCertificate
             }
         }
     }

mithril-signer/src/services/epoch_service.rs

@@ -49,8 +49,8 @@ pub trait EpochService: Sync + Send {
     /// Get signers with stake for the next epoch
     async fn next_signers_with_stake(&self) -> StdResult<Vec<SignerWithStake>>;
 
-    /// Check if a signer can sign the current epoch
-    fn can_signer_sign_current_epoch(
+    /// Check if a signer is included in the current stake distribution
+    fn is_signer_included_in_current_stake_distribution(
         &self,
         party_id: PartyId,
         protocol_initializer: ProtocolInitializer,
@@ -177,7 +177,7 @@ impl EpochService for MithrilEpochService {
         .await
     }
 
-    fn can_signer_sign_current_epoch(
+    fn is_signer_included_in_current_stake_distribution(
         &self,
         party_id: PartyId,
         protocol_initializer: ProtocolInitializer,
@@ -210,7 +210,8 @@ mod tests {
     };
 
     #[test]
-    fn test_can_signer_sign_returns_error_when_epoch_settings_is_not_set() {
+    fn test_is_signer_included_in_current_stake_distribution_returns_error_when_epoch_settings_is_not_set(
+    ) {
         let party_id = "party_id".to_string();
         let protocol_initializer = MithrilProtocolInitializerBuilder::build(
             &100,
@@ -223,12 +224,13 @@ mod tests {
         let service = MithrilEpochService::new(stake_store);
 
         service
-            .can_signer_sign_current_epoch(party_id, protocol_initializer)
+            .is_signer_included_in_current_stake_distribution(party_id, protocol_initializer)
             .expect_err("can_signer_sign should return error when epoch settings is not set");
     }
 
     #[test]
-    fn test_can_signer_sign_returns_true_when_signer_verification_key_and_pool_id_found() {
+    fn test_is_signer_included_in_current_stake_distribution_returns_true_when_signer_verification_key_and_pool_id_found(
+    ) {
         let fixtures = MithrilFixtureBuilder::default().with_signers(10).build();
         let protocol_initializer = fixtures.signers_fixture()[0]
             .protocol_initializer
@@ -249,19 +251,28 @@ mod tests {
 
         let party_id = fixtures.signers_fixture()[0].party_id();
         assert!(service
-            .can_signer_sign_current_epoch(party_id.clone(), protocol_initializer.clone())
+            .is_signer_included_in_current_stake_distribution(
+                party_id.clone(),
+                protocol_initializer.clone()
+            )
             .unwrap());
 
         let party_id_not_included = fixtures.signers_fixture()[6].party_id();
         assert!(!service
-            .can_signer_sign_current_epoch(party_id_not_included, protocol_initializer)
+            .is_signer_included_in_current_stake_distribution(
+                party_id_not_included,
+                protocol_initializer
+            )
             .unwrap());
 
         let protocol_initializer_not_included = fixtures.signers_fixture()[6]
             .protocol_initializer
             .to_owned();
         assert!(!service
-            .can_signer_sign_current_epoch(party_id, protocol_initializer_not_included)
+            .is_signer_included_in_current_stake_distribution(
+                party_id,
+                protocol_initializer_not_included
+            )
             .unwrap());
     }