Refactor register single signature

jpraynaud · jpraynaud · commit 0b32e3f93f6c · 2023-03-27T10:19:55.000+02:00
And make it use a message argument.
diff --git a/mithril-aggregator/src/http_server/routes/signatures_routes.rs b/mithril-aggregator/src/http_server/routes/signatures_routes.rs
@@ -26,6 +26,7 @@ mod handlers {
     use crate::{
         dependency::MultiSignerWrapper, message_adapters::FromRegisterSingleSignatureAdapter,
     };
+    use mithril_common::entities::SingleSignatures;
     use mithril_common::messages::RegisterSignatureMessage;
     use slog_scope::{debug, warn};
     use std::convert::Infallible;
@@ -37,10 +38,23 @@ mod handlers {
         multi_signer: MultiSignerWrapper,
     ) -> Result<impl warp::Reply, Infallible> {
         debug!("⇄ HTTP SERVER: register_signatures/{:?}", message);
-        let signature = FromRegisterSingleSignatureAdapter::adapt(message);
 
-        let mut multi_signer = multi_signer.write().await;
-        match multi_signer.register_single_signature(&signature).await {
+        async fn register_single_signature(
+            multi_signer: MultiSignerWrapper,
+            signature: SingleSignatures,
+        ) -> Result<(), ProtocolError> {
+            let multi_signer = multi_signer.write().await;
+            let message = multi_signer
+                .get_current_message()
+                .await
+                .ok_or_else(ProtocolError::UnavailableMessage)?;
+            multi_signer
+                .register_single_signature(&message, &signature)
+                .await
+        }
+
+        let signature = FromRegisterSingleSignatureAdapter::adapt(message);
+        match register_single_signature(multi_signer, signature).await {
             Err(ProtocolError::ExistingSingleSignature(party_id)) => {
                 debug!("register_signatures::already_exist"; "party_id" => ?party_id);
                 Ok(reply::empty(StatusCode::CONFLICT))
@@ -58,6 +72,7 @@ mod handlers {
 mod tests {
 
     use crate::http_server::SERVER_BASE_PATH;
+    use mithril_common::entities::ProtocolMessage;
     use mithril_common::messages::RegisterSignatureMessage;
     use mithril_common::test_utils::apispec::APISpec;
     use tokio::sync::RwLock;
@@ -85,11 +100,11 @@ mod tests {
     async fn test_register_signatures_post_ok() {
         let mut mock_multi_signer = MockMultiSigner::new();
         mock_multi_signer
-            .expect_update_current_message()
-            .return_once(|_| Ok(()));
+            .expect_get_current_message()
+            .return_once(|| Some(ProtocolMessage::new()));
         mock_multi_signer
             .expect_register_single_signature()
-            .return_once(|_| Ok(()));
+            .return_once(|_, _| Ok(()));
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.multi_signer = Arc::new(RwLock::new(mock_multi_signer));
 
@@ -118,9 +133,12 @@ mod tests {
     #[tokio::test]
     async fn test_register_signatures_post_ko_400() {
         let mut mock_multi_signer = MockMultiSigner::new();
+        mock_multi_signer
+            .expect_get_current_message()
+            .return_once(|| Some(ProtocolMessage::new()));
         mock_multi_signer
             .expect_register_single_signature()
-            .return_once(|_| Ok(()));
+            .return_once(|_, _| Ok(()));
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.multi_signer = Arc::new(RwLock::new(mock_multi_signer));
 
@@ -153,11 +171,11 @@ mod tests {
         let party_id = message.party_id.clone();
         let mut mock_multi_signer = MockMultiSigner::new();
         mock_multi_signer
-            .expect_update_current_message()
-            .return_once(|_| Ok(()));
+            .expect_get_current_message()
+            .return_once(|| Some(ProtocolMessage::new()));
         mock_multi_signer
             .expect_register_single_signature()
-            .return_once(move |_| Err(ProtocolError::ExistingSingleSignature(party_id)));
+            .return_once(move |_, _| Err(ProtocolError::ExistingSingleSignature(party_id)));
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.multi_signer = Arc::new(RwLock::new(mock_multi_signer));
 
@@ -185,11 +203,11 @@ mod tests {
     async fn test_register_signatures_post_ko_500() {
         let mut mock_multi_signer = MockMultiSigner::new();
         mock_multi_signer
-            .expect_update_current_message()
-            .return_once(|_| Ok(()));
+            .expect_get_current_message()
+            .return_once(|| Some(ProtocolMessage::new()));
         mock_multi_signer
             .expect_register_single_signature()
-            .return_once(|_| Err(ProtocolError::Core("an error occurred".to_string())));
+            .return_once(|_, _| Err(ProtocolError::Core("an error occurred".to_string())));
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.multi_signer = Arc::new(RwLock::new(mock_multi_signer));
 
diff --git a/mithril-aggregator/src/multi_signer.rs b/mithril-aggregator/src/multi_signer.rs
@@ -186,7 +186,8 @@ pub trait MultiSigner: Sync + Send {
 
     /// Registers a single signature
     async fn register_single_signature(
-        &mut self,
+        &self,
+        message: &entities::ProtocolMessage,
         signatures: &entities::SingleSignatures,
     ) -> Result<(), ProtocolError>;
 
@@ -525,18 +526,15 @@ impl MultiSigner for MultiSignerImpl {
 
     /// Registers a single signature
     async fn register_single_signature(
-        &mut self,
+        &self,
+        message: &entities::ProtocolMessage,
         signatures: &entities::SingleSignatures,
     ) -> Result<(), ProtocolError> {
         debug!(
-            "Register single signature from {} at indexes {:?}",
-            signatures.party_id, signatures.won_indexes
+            "Register single signature from {} at indexes {:?} for message {:?}",
+            signatures.party_id, signatures.won_indexes, message
         );
 
-        let message = &self
-            .get_current_message()
-            .await
-            .ok_or_else(ProtocolError::UnavailableMessage)?;
         let protocol_parameters = self
             .get_protocol_parameters()
             .await?
@@ -902,7 +900,7 @@ mod tests {
         // Add some signatures but not enough to reach the quorum: multi-signer should not create the multi-signature
         for signature in signatures_to_almost_reach_quorum {
             multi_signer
-                .register_single_signature(&signature)
+                .register_single_signature(&message, &signature)
                 .await
                 .expect("register single signature should not fail");
         }
@@ -915,7 +913,7 @@ mod tests {
         // Add the remaining signatures to reach the quorum: multi-signer should create a multi-signature
         for signature in &signatures {
             multi_signer
-                .register_single_signature(signature)
+                .register_single_signature(&message, signature)
                 .await
                 .expect("register single signature should not fail");
         }
diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs
@@ -37,7 +37,7 @@ macro_rules! cycle_err {
         $tester
             .cycle()
             .await
-            .expect_err("cycle tick shoudl have returned an error");
+            .expect_err("cycle tick should have returned an error");
         assert_eq!($expected_state, $tester.runtime.get_state());
     }};
 }
@@ -240,7 +240,7 @@ impl RuntimeTester {
 
     /// "Send", actually register, the given single signatures in the multi-signers
     pub async fn send_single_signatures(&self, signers: &[SignerFixture]) -> Result<(), String> {
-        let mut multisigner = self.deps.multi_signer.write().await;
+        let multisigner = self.deps.multi_signer.read().await;
         let message = multisigner
             .get_current_message()
             .await
@@ -258,7 +258,7 @@ impl RuntimeTester {
                 );
 
                 multisigner
-                    .register_single_signature(&single_signatures)
+                    .register_single_signature(&message, &single_signatures)
                     .await
                     .map_err(|e| {
                         format!("registering a winning lottery signature should not fail: {e:?}")
