resolve issues

consensus/src/alerts/mod.rs

@@ -106,7 +106,7 @@ pub enum AlertMessage<H: Hasher, D: Data, S: Signature, MS: PartialMultisignatur
     /// Alert regarding forks, signed by the person claiming misconduct.
     ForkAlert(UncheckedSigned<Alert<H, D, S>, S>),
     /// An internal RMC message, together with the id of the sender.
-    RmcHash(NodeIndex, RmcMessage<H::Hash, S, MS>),
+    RmcMessage(NodeIndex, RmcMessage<H::Hash, S, MS>),
     /// A request by a node for a fork alert identified by the given hash.
     AlertRequest(NodeIndex, H::Hash),
 }
@@ -115,7 +115,7 @@ impl<H: Hasher, D: Data, S: Signature, MS: PartialMultisignature> AlertMessage<H
     pub fn included_data(&self) -> Vec<D> {
         match self {
             Self::ForkAlert(unchecked_alert) => unchecked_alert.as_signable().included_data(),
-            Self::RmcHash(_, _) => Vec::new(),
+            Self::RmcMessage(_, _) => Vec::new(),
             Self::AlertRequest(_, _) => Vec::new(),
         }
     }

consensus/src/alerts/service.rs

@@ -80,7 +80,7 @@ impl<H: Hasher, D: Data, MK: MultiKeychain> Service<H, D, MK> {
         message: RmcMessage<H::Hash, MK::Signature, MK::PartialMultisignature>,
     ) {
         self.send_message_for_network(
-            AlertMessage::RmcHash(self.node_index, message),
+            AlertMessage::RmcMessage(self.node_index, message),
             Recipient::Everyone,
         );
     }
@@ -142,7 +142,7 @@ impl<H: Hasher, D: Data, MK: MultiKeychain> Service<H, D, MK> {
                 }
                 Err(error) => debug!(target: LOG_TARGET, "{}", error),
             },
-            AlertMessage::RmcHash(sender, message) => {
+            AlertMessage::RmcMessage(sender, message) => {
                 match self.handler.on_rmc_message(sender, message) {
                     RmcResponse::RmcMessage(message) => {
                         if let Some(multisigned) = self.rmc_service.process_message(message) {
@@ -274,6 +274,7 @@ impl<H: Hasher, D: Data, MK: MultiKeychain> Service<H, D, MK> {
                 },
             }
             if self.exiting {
+                debug!(target: LOG_TARGET, "Alerter decided to exit.");
                 terminator.terminate_sync().await;
                 break;
             }

consensus/src/testing/alerts.rs

@@ -312,7 +312,7 @@ async fn reacts_to_correctly_incoming_alert() {
         .incoming_message(AlertMessage::ForkAlert(signed_alert))
         .outgoing_notification(ForkingNotification::Forker(fork_proof));
     test_case.outgoing_message(
-        AlertMessage::RmcHash(own_index, RmcMessage::SignedHash(signed_alert_hash.clone())),
+        AlertMessage::RmcMessage(own_index, RmcMessage::SignedHash(signed_alert_hash.clone())),
         Recipient::Everyone,
     );
     test_case.run(own_index).await;
@@ -336,7 +336,7 @@ async fn notifies_about_finished_alert() {
     for i in 1..n_members.0 - 1 {
         let node_id = NodeIndex(i);
         let signed_alert_hash = test_case.indexed_unchecked_signed(alert_hash, node_id);
-        test_case.incoming_message(AlertMessage::RmcHash(
+        test_case.incoming_message(AlertMessage::RmcMessage(
             node_id,
             RmcMessage::SignedHash(signed_alert_hash),
         ));
@@ -357,7 +357,7 @@ async fn asks_about_unknown_alert() {
     let alert_hash = Signable::hash(&alert);
     let signed_alert_hash = test_case.indexed_unchecked_signed(alert_hash, alerter_index);
     test_case
-        .incoming_message(AlertMessage::RmcHash(
+        .incoming_message(AlertMessage::RmcMessage(
             alerter_index,
             RmcMessage::SignedHash(signed_alert_hash),
         ))
@@ -386,7 +386,7 @@ async fn ignores_wrong_alert() {
         .unexpected_notification(ForkingNotification::Forker(wrong_fork_proof));
     for i in 1..n_members.0 {
         test_case.unexpected_message(
-            AlertMessage::RmcHash(
+            AlertMessage::RmcMessage(
                 own_index,
                 RmcMessage::SignedHash(signed_wrong_alert_hash.clone()),
             ),
@@ -421,7 +421,7 @@ async fn responds_to_alert_queries() {
             Recipient::Everyone,
         )
         .outgoing_message(
-            AlertMessage::RmcHash(own_index, RmcMessage::SignedHash(signed_alert_hash.clone())),
+            AlertMessage::RmcMessage(own_index, RmcMessage::SignedHash(signed_alert_hash.clone())),
             Recipient::Everyone,
         )
         .wait()
@@ -458,7 +458,7 @@ async fn notifies_only_about_multisigned_alert() {
         test_case.indexed_unchecked_signed(empty_alert_hash, double_committer);
     test_case
         .incoming_message(AlertMessage::ForkAlert(signed_empty_alert))
-        .incoming_message(AlertMessage::RmcHash(
+        .incoming_message(AlertMessage::RmcMessage(
             double_committer,
             RmcMessage::SignedHash(signed_empty_alert_hash),
         ))
@@ -496,7 +496,7 @@ async fn notifies_only_about_multisigned_alert() {
         multisigned_nonempty_alert_hash.into_unchecked();
     test_case
         .incoming_message(AlertMessage::ForkAlert(signed_nonempty_alert))
-        .incoming_message(AlertMessage::RmcHash(
+        .incoming_message(AlertMessage::RmcMessage(
             other_honest_node,
             RmcMessage::MultisignedHash(unchecked_multisigned_nonempty_alert_hash),
         ))

rmc/src/handler.rs

@@ -24,7 +24,11 @@ impl Display for Error {
     }
 }
 
-type OnStartRmcResponse<H, MK> = (Signed<Indexed<H>, MK>, Option<Multisigned<H, MK>>);
+pub enum OnStartRmcResponse<H: Signable, MK: MultiKeychain> {
+    SignedHash(Signed<Indexed<H>, MK>),
+    MultisignedHash(Multisigned<H, MK>),
+    Noop,
+}
 
 pub struct Handler<H: Signable + Hash, MK: MultiKeychain> {
     keychain: MK,
@@ -43,12 +47,15 @@ impl<H: Signable + Hash + Eq + Clone + Debug, MK: MultiKeychain> Handler<H, MK>
     /// version of the hash for broadcast. Should be called at most once for a particular hash.
     pub fn on_start_rmc(&mut self, hash: H) -> OnStartRmcResponse<H, MK> {
         let signed_hash = Signed::sign_with_index(hash, &self.keychain);
-        let mut maybe_multisigned = None;
-        if !self.already_completed(signed_hash.as_signable().as_signable()) {
-            maybe_multisigned = self.handle_signed_hash(signed_hash.clone());
+        if self.already_completed(signed_hash.as_signable().as_signable()) {
+            return OnStartRmcResponse::Noop;
+        }
+        if let Some(multisigned) = self.handle_signed_hash(signed_hash.clone()) {
+            return OnStartRmcResponse::MultisignedHash(multisigned);
         }
-        (signed_hash, maybe_multisigned)
+        OnStartRmcResponse::SignedHash(signed_hash)
     }
+
     /// Update the internal state with the signed hash. If the hash is incorrectly signed then
     /// [`Error::BadSignature`] is returned. If Adding this signature completes a multisignature
     /// then `Ok(multisigned)` is returned. Otherwise `Ok(None)` is returned.

rmc/src/service.rs

@@ -7,6 +7,7 @@ pub use aleph_bft_crypto::{
 use core::fmt::Debug;
 use log::{debug, warn};
 use std::hash::Hash;
+use crate::handler::OnStartRmcResponse;
 
 const LOG_TARGET: &str = "AlephBFT-rmc";
 
@@ -45,19 +46,24 @@ where
         Service { scheduler, handler }
     }
 
-    /// Signs the given `hash` and starts scheduling a message containing the signed hash for
-    /// repeated broadcasts. If the given `hash` completes the multisignature, it is returned.
-    /// Otherwise `None` is returned.
+    /// Signs the given `hash` and adds the signature to the collection. If the given `hash`
+    /// completes the multisignature, it is scheduled for the broadcasts and then returned.
+    /// If the multisignature is not completed, `None` is returned. If the multisignature was
+    /// already completed when starting rmc, no tasks are scheduled. Otherwise the signed hash
+    /// is scheduled for the broadcasts.
     pub fn start_rmc(&mut self, hash: H) -> Option<Multisigned<H, MK>> {
         debug!(target: LOG_TARGET, "starting rmc for {:?}", hash);
-        let (signed_hash, maybe_multisigned) = self.handler.on_start_rmc(hash);
-        self.scheduler
-            .add_task(RmcMessage::SignedHash(signed_hash.into_unchecked()));
-        if let Some(multisigned) = maybe_multisigned.clone() {
-            self.scheduler
-                .add_task(RmcMessage::MultisignedHash(multisigned.into_unchecked()));
+        match self.handler.on_start_rmc(hash) {
+            OnStartRmcResponse::SignedHash(signed_hash) => {
+                self.scheduler.add_task(RmcMessage::SignedHash(signed_hash.into_unchecked()));
+            }
+            OnStartRmcResponse::MultisignedHash(multisigned) => {
+                self.scheduler.add_task(RmcMessage::MultisignedHash(multisigned.clone().into_unchecked()));
+                return Some(multisigned);
+            }
+            OnStartRmcResponse::Noop => {}
         }
-        maybe_multisigned
+        None
     }
 
     /// Processes a message which can be of two types. If the message is a hash signed by one