Merge pull request #877 from input-output-hk/jpraynaud/873-fix-verification-key-discrepancy

Fix Signer Registration Discrepancy

Author: jpraynaud

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-aggregator"
-version = "0.3.1"
+version = "0.3.2"
 description = "A Mithril Aggregator server"
 authors = { workspace = true }
 edition = { workspace = true }

mithril-aggregator/Cargo.toml

@@ -54,11 +54,25 @@ mod handlers {
             "⇄ HTTP SERVER: register_signer/{:?}",
             register_signer_message
         );
+
+        let registration_epoch = match register_signer_message.epoch {
+            Some(epoch) => epoch,
+            None => match signer_registerer.get_current_round().await {
+                Some(round) => round.epoch,
+                None => {
+                    let err = SignerRegistrationError::RegistrationRoundNotYetOpened;
+                    warn!("register_signer::error"; "error" => ?err);
+                    return Ok(reply::internal_server_error(err.to_string()));
+                }
+            },
+        };
+
         let signer = FromRegisterSignerAdapter::adapt(register_signer_message);
         let mut headers: Vec<(&str, &str)> = match signer_node_version.as_ref() {
             Some(version) => vec![("signer-node-version", version)],
             None => Vec::new(),
         };
+
         let epoch_str = match beacon_provider.get_current_beacon().await {
             Ok(beacon) => format!("{}", beacon.epoch),
             Err(e) => {
@@ -67,11 +81,14 @@ mod handlers {
                 String::new()
             }
         };
-
         if !epoch_str.is_empty() {
             headers.push(("epoch", epoch_str.as_str()));
         }
-        match signer_registerer.register_signer(&signer).await {
+
+        match signer_registerer
+            .register_signer(registration_epoch, &signer)
+            .await
+        {
             Ok(signer_with_stake) => {
                 let _ = event_transmitter.send_event_message(
                     "HTTP::signer_register",
@@ -147,7 +164,10 @@ mod tests {
         let mut mock_signer_registerer = MockSignerRegisterer::new();
         mock_signer_registerer
             .expect_register_signer()
-            .return_once(|_| Ok(signer_with_stake));
+            .return_once(|_, _| Ok(signer_with_stake));
+        mock_signer_registerer
+            .expect_get_current_round()
+            .return_once(|| None);
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.signer_registerer = Arc::new(mock_signer_registerer);
 
@@ -179,7 +199,10 @@ mod tests {
         let mut mock_signer_registerer = MockSignerRegisterer::new();
         mock_signer_registerer
             .expect_register_signer()
-            .return_once(|_| Err(SignerRegistrationError::ExistingSigner(signer_with_stake)));
+            .return_once(|_, _| Err(SignerRegistrationError::ExistingSigner(signer_with_stake)));
+        mock_signer_registerer
+            .expect_get_current_round()
+            .return_once(|| None);
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.signer_registerer = Arc::new(mock_signer_registerer);
 
@@ -210,11 +233,14 @@ mod tests {
         let mut mock_signer_registerer = MockSignerRegisterer::new();
         mock_signer_registerer
             .expect_register_signer()
-            .return_once(|_| {
+            .return_once(|_, _| {
                 Err(SignerRegistrationError::FailedSignerRegistration(
                     ProtocolRegistrationError::OpCertInvalid,
                 ))
             });
+        mock_signer_registerer
+            .expect_get_current_round()
+            .return_once(|| None);
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.signer_registerer = Arc::new(mock_signer_registerer);
 
@@ -245,11 +271,14 @@ mod tests {
         let mut mock_signer_registerer = MockSignerRegisterer::new();
         mock_signer_registerer
             .expect_register_signer()
-            .return_once(|_| {
+            .return_once(|_, _| {
                 Err(SignerRegistrationError::ChainObserver(
                     "an error occurred".to_string(),
                 ))
             });
+        mock_signer_registerer
+            .expect_get_current_round()
+            .return_once(|| None);
         let (mut dependency_manager, _) = initialize_dependencies().await;
         dependency_manager.signer_registerer = Arc::new(mock_signer_registerer);
 

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

@@ -24,6 +24,15 @@ pub enum SignerRegistrationError {
     #[error("a signer registration round has not yet to be opened")]
     RegistrationRoundNotYetOpened,
 
+    /// Registration round for unexpected epoch
+    #[error("unexpected signer registration round epoch: current_round_epoch: {current_round_epoch}, received_epoch: {received_epoch}")]
+    RegistrationRoundUnexpectedEpoch {
+        /// Epoch of the current round
+        current_round_epoch: Epoch,
+        /// Epoch of the received signer registration
+        received_epoch: Epoch,
+    },
+
     /// Codec error.
     #[error("codec error: '{0}'")]
     Codec(String),
@@ -52,7 +61,9 @@ pub enum SignerRegistrationError {
 /// Represents the information needed to handle a signer registration round
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct SignerRegistrationRound {
-    epoch: Epoch,
+    /// Registration round epoch
+    pub epoch: Epoch,
+
     stake_distribution: StakeDistribution,
 }
 
@@ -73,8 +84,12 @@ pub trait SignerRegisterer: Sync + Send {
     /// Register a signer
     async fn register_signer(
         &self,
+        epoch: Epoch,
         signer: &Signer,
     ) -> Result<SignerWithStake, SignerRegistrationError>;
+
+    /// Get current open round if exists
+    async fn get_current_round(&self) -> Option<SignerRegistrationRound>;
 }
 
 /// Trait to open a signer registration round
@@ -171,12 +186,19 @@ impl SignerRegistrationRoundOpener for MithrilSignerRegisterer {
 impl SignerRegisterer for MithrilSignerRegisterer {
     async fn register_signer(
         &self,
+        epoch: Epoch,
         signer: &Signer,
     ) -> Result<SignerWithStake, SignerRegistrationError> {
         let registration_round = self.current_round.read().await;
         let registration_round = registration_round
             .as_ref()
             .ok_or(SignerRegistrationError::RegistrationRoundNotYetOpened)?;
+        if registration_round.epoch != epoch {
+            return Err(SignerRegistrationError::RegistrationRoundUnexpectedEpoch {
+                current_round_epoch: registration_round.epoch,
+                received_epoch: epoch,
+            });
+        }
 
         let mut key_registration = ProtocolKeyRegistration::init(
             &registration_round
@@ -245,6 +267,10 @@ impl SignerRegisterer for MithrilSignerRegisterer {
             None => Ok(signer_save),
         }
     }
+
+    async fn get_current_round(&self) -> Option<SignerRegistrationRound> {
+        self.current_round.read().await.as_ref().cloned()
+    }
 }
 
 #[cfg(test)]
@@ -293,7 +319,7 @@ mod tests {
             .expect("signer registration round opening should not fail");
 
         signer_registerer
-            .register_signer(&signer_to_register)
+            .register_signer(registration_epoch, &signer_to_register)
             .await
             .expect("signer registration should not fail");
 
@@ -342,7 +368,7 @@ mod tests {
             .expect("signer registration round opening should not fail");
 
         signer_registerer
-            .register_signer(&signer_to_register)
+            .register_signer(registration_epoch, &signer_to_register)
             .await
             .expect("signer registration should not fail");
 
@@ -373,11 +399,12 @@ mod tests {
             verification_key_store.clone(),
             Arc::new(signer_recorder),
         );
+        let registration_epoch = Epoch(1);
         let fixture = MithrilFixtureBuilder::default().with_signers(5).build();
         let signer_to_register: Signer = fixture.signers()[0].to_owned();
 
         signer_registerer
-            .register_signer(&signer_to_register)
+            .register_signer(registration_epoch, &signer_to_register)
             .await
             .expect_err("signer registration should fail if no round opened");
     }

mithril-aggregator/src/signer_registerer.rs

@@ -193,6 +193,7 @@ async fn certificate_chain() {
     current_epoch = tester.increase_epoch().await.unwrap();
     tester.increase_immutable_number().await.unwrap();
     cycle!(tester, "ready");
+    tester.register_signers(&signers).await.unwrap();
     cycle!(tester, "signing");
 
     let signed_entity_type = tester
@@ -214,6 +215,7 @@ async fn certificate_chain() {
     tester.increase_epoch().await.unwrap();
     tester.increase_immutable_number().await.unwrap();
     cycle!(tester, "ready");
+    tester.register_signers(&signers).await.unwrap();
     cycle!(tester, "signing");
     let signed_entity_type = tester
         .open_message_observer

mithril-aggregator/tests/certificate_chain.rs

@@ -13,7 +13,7 @@ use tokio::sync::mpsc::UnboundedReceiver;
 use crate::test_extensions::open_message_observer::OpenMessageObserver;
 use mithril_aggregator::{
     AggregatorRuntime, Configuration, DumbSnapshotUploader, DumbSnapshotter,
-    ProtocolParametersStorer, SignerRegisterer,
+    ProtocolParametersStorer, SignerRegisterer, SignerRegistrationError,
 };
 use mithril_common::crypto_helper::{ProtocolClerk, ProtocolGenesisSigner};
 use mithril_common::digesters::DumbImmutableFileObserver;
@@ -248,14 +248,27 @@ impl RuntimeTester {
 
     /// Register the given signers in the registerer
     pub async fn register_signers(&mut self, signers: &[SignerFixture]) -> Result<(), String> {
+        let registration_epoch = self
+            .chain_observer
+            .current_beacon
+            .read()
+            .await
+            .as_ref()
+            .unwrap()
+            .epoch
+            .offset_to_recording_epoch();
+        let signer_registerer = self.deps_builder.get_mithril_registerer().await.unwrap();
         for signer_with_stake in signers.iter().map(|f| &f.signer_with_stake) {
-            self.deps_builder
-                .get_mithril_registerer()
+            match signer_registerer
+                .register_signer(registration_epoch, &signer_with_stake.to_owned().into())
                 .await
-                .unwrap()
-                .register_signer(&signer_with_stake.to_owned().into())
-                .await
-                .map_err(|e| format!("Registering a signer should not fail: {e:?}"))?;
+            {
+                Ok(_) => {}
+                Err(SignerRegistrationError::ExistingSigner(_)) => {}
+                Err(e) => {
+                    return Err(format!("Registering a signer should not fail: {e:?}"));
+                }
+            }
         }
 
         Ok(())

mithril-aggregator/tests/test_extensions/runtime_tester.rs

@@ -1,6 +1,6 @@
 [package]
 name = "mithril-common"
-version = "0.2.41"
+version = "0.2.42"
 authors = { workspace = true }
 edition = { workspace = true }
 documentation = { workspace = true }

mithril-common/Cargo.toml

@@ -8,3 +8,9 @@ mod supported_era;
 pub use era_checker::EraChecker;
 pub use era_reader::*;
 pub use supported_era::*;
+
+/// Macro used to mark the code that should be cleaned up when the new era is activated
+#[macro_export]
+macro_rules! era_deprecate {
+    ( $comment:literal ) => {};
+}

mithril-common/src/era/mod.rs

@@ -18,6 +18,7 @@ pub mod crypto_helper;
 pub mod database;
 pub mod digesters;
 pub mod entities;
+#[macro_use]
 pub mod era;
 pub mod messages;
 pub mod signable_builder;

mithril-common/src/lib.rs

@@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize};
 
 use crate::entities::{HexEncodedSingleSignature, LotteryIndex, PartyId, SignedEntityType};
 
+era_deprecate!("make signed_entity_type of RegisterSignatureMessage not optional");
 /// Message structure to register single signature.
 #[derive(Clone, Debug, PartialEq, Eq, Default, Serialize, Deserialize)]
 pub struct RegisterSignatureMessage {