Use a Protocol Avk instead of string in certificate entity

Alenar · Alenar · commit 8c803c3eaf23 · 2023-08-22T16:45:28.000+02:00
diff --git a/mithril-aggregator/src/database/provider/certificate.rs b/mithril-aggregator/src/database/provider/certificate.rs
@@ -113,7 +113,7 @@ impl From<Certificate> for CertificateRecord {
             parent_certificate_id,
             message: other.signed_message,
             signature,
-            aggregate_verification_key: other.aggregate_verification_key,
+            aggregate_verification_key: other.aggregate_verification_key.to_json_hex().unwrap(),
             epoch: other.beacon.epoch,
             beacon: other.beacon,
             protocol_version: other.metadata.protocol_version,
@@ -153,7 +153,7 @@ impl From<CertificateRecord> for Certificate {
             metadata: certificate_metadata,
             signed_message: other.protocol_message.compute_hash(),
             protocol_message: other.protocol_message,
-            aggregate_verification_key: other.aggregate_verification_key,
+            aggregate_verification_key: other.aggregate_verification_key.try_into().unwrap(),
             signature,
         }
     }
diff --git a/mithril-aggregator/src/message_adapters/to_certificate_list_message.rs b/mithril-aggregator/src/message_adapters/to_certificate_list_message.rs
@@ -27,7 +27,10 @@ impl ToMessageAdapter<Vec<Certificate>, CertificateListMessage>
                 },
                 protocol_message: certificate.protocol_message,
                 signed_message: certificate.signed_message,
-                aggregate_verification_key: certificate.aggregate_verification_key,
+                aggregate_verification_key: certificate
+                    .aggregate_verification_key
+                    .to_json_hex()
+                    .unwrap(),
             })
             .collect()
     }
@@ -58,7 +61,10 @@ mod tests {
             },
             protocol_message: certificate.protocol_message,
             signed_message: certificate.signed_message,
-            aggregate_verification_key: certificate.aggregate_verification_key,
+            aggregate_verification_key: certificate
+                .aggregate_verification_key
+                .to_json_hex()
+                .unwrap(),
         }];
 
         assert_eq!(certificate_list_message_expected, certificate_list_message);
diff --git a/mithril-aggregator/src/message_adapters/to_certificate_message.rs b/mithril-aggregator/src/message_adapters/to_certificate_message.rs
@@ -34,7 +34,10 @@ impl ToMessageAdapter<Certificate, CertificateMessage> for ToCertificateMessageA
             metadata,
             protocol_message: certificate.protocol_message,
             signed_message: certificate.signed_message,
-            aggregate_verification_key: certificate.aggregate_verification_key,
+            aggregate_verification_key: certificate
+                .aggregate_verification_key
+                .to_json_hex()
+                .unwrap(),
             multi_signature,
             genesis_signature,
         }
diff --git a/mithril-aggregator/src/multi_signer.rs b/mithril-aggregator/src/multi_signer.rs
@@ -11,7 +11,6 @@ use mithril_common::{
     entities::{self, Epoch, SignerWithStake, StakeDistribution},
     protocol::{MultiSigner as ProtocolMultiSigner, SignerBuilder},
     store::{StakeStorer, StoreError},
-    StdError,
 };
 
 use crate::{entities::OpenMessage, store::VerificationKeyStorer, ProtocolParametersStorer};
@@ -38,10 +37,6 @@ pub enum ProtocolError {
     #[error("single signature already recorded")]
     ExistingSingleSignature(entities::PartyId),
 
-    /// Codec error.
-    #[error("codec error: '{0:?}'")]
-    Codec(StdError),
-
     /// Mithril STM library returned an error.
     #[error("core error: '{0}'")]
     Core(String),
@@ -122,31 +117,29 @@ pub trait MultiSigner: Sync + Send {
     /// Compute stake distribution aggregate verification key
     async fn compute_stake_distribution_aggregate_verification_key(
         &self,
-    ) -> Result<String, ProtocolError> {
+    ) -> Result<ProtocolAggregateVerificationKey, ProtocolError> {
         let signers_with_stake = self.get_signers_with_stake().await?;
         let protocol_parameters = self
             .get_protocol_parameters()
             .await?
             .ok_or_else(ProtocolError::UnavailableProtocolParameters)?;
-        let avk = self
+        Ok(self
             .compute_aggregate_verification_key(&signers_with_stake, &protocol_parameters)
-            .await?;
-        Ok(avk.to_json_hex().map_err(ProtocolError::Codec)?)
+            .await?)
     }
 
     /// Compute next stake distribution aggregate verification key
     async fn compute_next_stake_distribution_aggregate_verification_key(
         &self,
-    ) -> Result<String, ProtocolError> {
+    ) -> Result<ProtocolAggregateVerificationKey, ProtocolError> {
         let next_signers_with_stake = self.get_next_signers_with_stake().await?;
         let protocol_parameters = self
             .get_next_protocol_parameters()
             .await?
             .ok_or_else(ProtocolError::UnavailableProtocolParameters)?;
-        let next_avk = self
+        Ok(self
             .compute_aggregate_verification_key(&next_signers_with_stake, &protocol_parameters)
-            .await?;
-        Ok(next_avk.to_json_hex().map_err(ProtocolError::Codec)?)
+            .await?)
     }
 
     /// Get signers
diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs
@@ -1,3 +1,4 @@
+use anyhow::Context;
 use async_trait::async_trait;
 use slog_scope::{debug, info, warn};
 use std::{path::Path, path::PathBuf, sync::Arc};
@@ -369,7 +370,9 @@ impl AggregatorRunnerTrait for AggregatorRunner {
             ProtocolMessagePartKey::NextAggregateVerificationKey,
             multi_signer
                 .compute_next_stake_distribution_aggregate_verification_key()
-                .await?,
+                .await?
+                .to_json_hex()
+                .with_context(|| "convert next avk to json hex failure")?,
         );
 
         Ok(protocol_message)
diff --git a/mithril-aggregator/src/tools/genesis.rs b/mithril-aggregator/src/tools/genesis.rs
@@ -82,8 +82,6 @@ impl GenesisTools {
         let genesis_avk = multi_signer
             .compute_next_stake_distribution_aggregate_verification_key()
             .await?;
-        let genesis_avk = ProtocolAggregateVerificationKey::from_json_hex(&genesis_avk)
-            .with_context(|| "Aggregate verification key decode error")?;
 
         Ok(Self::new(
             protocol_parameters,
diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs
@@ -416,7 +416,7 @@ impl RuntimeTester {
         let expected_certificate = match signed_entity_record {
             None if certificate.is_genesis() => ExpectedCertificate::new_genesis(
                 certificate.beacon,
-                certificate.aggregate_verification_key,
+                certificate.aggregate_verification_key.try_into().unwrap(),
             ),
             None => {
                 panic!("A certificate should always have a SignedEntity if it's not a genesis certificate");
@@ -429,7 +429,7 @@ impl RuntimeTester {
                 ExpectedCertificate::new(
                     certificate.beacon,
                     &certificate.metadata.signers,
-                    certificate.aggregate_verification_key,
+                    certificate.aggregate_verification_key.try_into().unwrap(),
                     record.signed_entity_type,
                     previous_cert_identifier,
                 )
diff --git a/mithril-client/src/aggregator_client/certificate_client.rs b/mithril-client/src/aggregator_client/certificate_client.rs
@@ -108,7 +108,10 @@ mod tests {
                     },
                     protocol_message: certificate.protocol_message.clone(),
                     signed_message: certificate.signed_message.clone(),
-                    aggregate_verification_key: certificate.aggregate_verification_key.clone(),
+                    aggregate_verification_key: certificate
+                        .aggregate_verification_key
+                        .try_into()
+                        .unwrap(),
                     multi_signature,
                     genesis_signature,
                 };
diff --git a/mithril-client/src/message_adapters/from_certificate_message_adapter.rs b/mithril-client/src/message_adapters/from_certificate_message_adapter.rs
@@ -27,7 +27,9 @@ impl TryFromMessageAdapter<CertificateMessage, Certificate> for FromCertificateM
             metadata,
             protocol_message: certificate_message.protocol_message,
             signed_message: certificate_message.signed_message,
-            aggregate_verification_key: certificate_message.aggregate_verification_key,
+            aggregate_verification_key: certificate_message
+                .aggregate_verification_key
+                .try_into()?,
             signature: if certificate_message.genesis_signature.is_empty() {
                 CertificateSignature::MultiSignature(
                     certificate_message.multi_signature.try_into()?,
diff --git a/mithril-common/src/certificate_chain/certificate_genesis.rs b/mithril-common/src/certificate_chain/certificate_genesis.rs
@@ -88,9 +88,6 @@ impl CertificateGenesisProducer {
         );
         let previous_hash = "".to_string();
         let genesis_protocol_message = Self::create_genesis_protocol_message(&genesis_avk)?;
-        let genesis_avk = genesis_avk
-            .to_json_hex()
-            .map_err(CertificateGenesisProducerError::Codec)?;
         Ok(Certificate::new(
             previous_hash,
             beacon,
diff --git a/mithril-common/src/certificate_chain/certificate_verifier.rs b/mithril-common/src/certificate_chain/certificate_verifier.rs
@@ -1,5 +1,6 @@
 //! A module used to validate the Certificate Chain created by an aggregator
 //!
+use anyhow::Context;
 use async_trait::async_trait;
 use hex::ToHex;
 use slog::{debug, Logger};
@@ -144,22 +145,19 @@ impl MithrilCertificateVerifier {
         &self,
         message: &[u8],
         multi_signature: &ProtocolMultiSignature,
-        aggregate_verification_key: &str,
+        aggregate_verification_key: &ProtocolAggregateVerificationKey,
         protocol_parameters: &ProtocolParameters,
     ) -> Result<(), CertificateVerifierError> {
         debug!(
             self.logger,
             "Verify multi signature for {:?}",
             message.encode_hex::<String>()
         );
-        let aggregate_verification_key =
-            ProtocolAggregateVerificationKey::from_json_hex(aggregate_verification_key)
-                .map_err(CertificateVerifierError::Codec)?;
 
         multi_signature
             .verify(
                 message,
-                &aggregate_verification_key,
+                aggregate_verification_key,
                 &protocol_parameters.to_owned().into(),
             )
             .map_err(|e| CertificateVerifierError::VerifyMultiSignature(e.to_string()))
@@ -181,21 +179,44 @@ impl MithrilCertificateVerifier {
         let previous_certificate = certificate_retriever
             .get_certificate_details(&certificate.previous_hash)
             .await?;
+
+        if previous_certificate.hash != certificate.previous_hash {
+            return Err(CertificateVerifierError::CertificateChainPreviousHashUnmatch);
+        }
+
+        let current_certificate_avk: String = certificate
+            .aggregate_verification_key
+            .to_json_hex()
+            .with_context(|| {
+                format!(
+                    "avk to string conversion error for certificate: `{}`",
+                    certificate.hash
+                )
+            })
+            .map_err(CertificateVerifierError::Codec)?;
+
+        let previous_certificate_avk: String = previous_certificate
+            .aggregate_verification_key
+            .to_json_hex()
+            .with_context(|| {
+                format!(
+                    "avk to string conversion error for previous certificate: `{}`",
+                    certificate.hash
+                )
+            })
+            .map_err(CertificateVerifierError::Codec)?;
+
         let valid_certificate_has_different_epoch_as_previous =
-            |next_aggregate_verification_key: &String| -> bool {
-                next_aggregate_verification_key == &certificate.aggregate_verification_key
+            |next_aggregate_verification_key: &str| -> bool {
+                next_aggregate_verification_key == current_certificate_avk
                     && previous_certificate.beacon.epoch != certificate.beacon.epoch
             };
         let valid_certificate_has_same_epoch_as_previous = || -> bool {
-            previous_certificate.aggregate_verification_key
-                == certificate.aggregate_verification_key
+            previous_certificate_avk == current_certificate_avk
                 && previous_certificate.beacon.epoch == certificate.beacon.epoch
         };
-        if previous_certificate.hash != certificate.previous_hash {
-            return Err(CertificateVerifierError::CertificateChainPreviousHashUnmatch);
-        }
 
-        match &previous_certificate
+        match previous_certificate
             .protocol_message
             .get_message_part(&ProtocolMessagePartKey::NextAggregateVerificationKey)
         {
@@ -336,7 +357,7 @@ mod tests {
                 .verify_multi_signature(
                     &message_tampered,
                     &multi_signature,
-                    &aggregate_verification_key.to_json_hex().unwrap(),
+                    &aggregate_verification_key,
                     &fixture.protocol_parameters(),
                 )
                 .is_err(),
@@ -346,7 +367,7 @@ mod tests {
             .verify_multi_signature(
                 &message_hash,
                 &multi_signature,
-                &aggregate_verification_key.to_json_hex().unwrap(),
+                &aggregate_verification_key,
                 &fixture.protocol_parameters(),
             )
             .expect("multi signature verification should have succeeded");
diff --git a/mithril-common/src/crypto_helper/tests_setup.rs b/mithril-common/src/crypto_helper/tests_setup.rs
@@ -204,8 +204,6 @@ pub fn setup_certificate_chain(
         let clerk = clerk_for_signers(signers);
         clerk.compute_avk().into()
     };
-    let avk_encode =
-        |avk: &ProtocolAggregateVerificationKey| -> String { avk.to_json_hex().unwrap() };
     epochs.pop();
     let certificates = epochs
         .into_iter()
@@ -226,9 +224,9 @@ pub fn setup_certificate_chain(
                 .set_message_part(ProtocolMessagePartKey::SnapshotDigest, digest);
             fake_certificate.protocol_message.set_message_part(
                 ProtocolMessagePartKey::NextAggregateVerificationKey,
-                avk_encode(&next_avk),
+                next_avk.to_json_hex().unwrap(),
             );
-            fake_certificate.aggregate_verification_key = avk_encode(&avk);
+            fake_certificate.aggregate_verification_key = avk;
             fake_certificate.signed_message = fake_certificate.protocol_message.compute_hash();
             fake_certificate.previous_hash = "".to_string();
             match i {
diff --git a/mithril-common/src/entities/certificate.rs b/mithril-common/src/entities/certificate.rs
@@ -1,7 +1,7 @@
-use crate::crypto_helper::{ProtocolGenesisSignature, ProtocolMultiSignature};
-use crate::entities::{
-    Beacon, CertificateMetadata, HexEncodedAgregateVerificationKey, ProtocolMessage,
+use crate::crypto_helper::{
+    ProtocolAggregateVerificationKey, ProtocolGenesisSignature, ProtocolMultiSignature,
 };
+use crate::entities::{Beacon, CertificateMetadata, ProtocolMessage};
 use std::cmp::Ordering;
 use std::fmt::{Debug, Formatter};
 
@@ -52,7 +52,7 @@ pub struct Certificate {
     /// Aggregate verification key
     /// The AVK used to sign during the current epoch
     /// aka AVK(n-2)
-    pub aggregate_verification_key: HexEncodedAgregateVerificationKey,
+    pub aggregate_verification_key: ProtocolAggregateVerificationKey,
 
     /// Certificate signature
     pub signature: CertificateSignature,
@@ -65,7 +65,7 @@ impl Certificate {
         beacon: Beacon,
         metadata: CertificateMetadata,
         protocol_message: ProtocolMessage,
-        aggregate_verification_key: HexEncodedAgregateVerificationKey,
+        aggregate_verification_key: ProtocolAggregateVerificationKey,
         signature: CertificateSignature,
     ) -> Certificate {
         let signed_message = protocol_message.compute_hash();
@@ -91,7 +91,12 @@ impl Certificate {
         hasher.update(self.metadata.compute_hash().as_bytes());
         hasher.update(self.protocol_message.compute_hash().as_bytes());
         hasher.update(self.signed_message.as_bytes());
-        hasher.update(self.aggregate_verification_key.as_bytes());
+        hasher.update(
+            self.aggregate_verification_key
+                .to_json_hex()
+                .unwrap()
+                .as_bytes(),
+        );
         match &self.signature {
             CertificateSignature::GenesisSignature(signature) => {
                 hasher.update(signature.to_bytes_hex());
@@ -227,7 +232,9 @@ mod tests {
                 get_signers_with_stake(),
             ),
             get_protocol_message(),
-            fake_keys::aggregate_verification_key()[0].to_owned(),
+            fake_keys::aggregate_verification_key()[0]
+                .try_into()
+                .unwrap(),
             CertificateSignature::MultiSignature(
                 fake_keys::multi_signature()[0].try_into().unwrap(),
             ),
@@ -328,7 +335,9 @@ mod tests {
                 get_signers_with_stake(),
             ),
             get_protocol_message(),
-            fake_keys::aggregate_verification_key()[1].to_owned(),
+            fake_keys::aggregate_verification_key()[1]
+                .try_into()
+                .unwrap(),
             CertificateSignature::GenesisSignature(
                 fake_keys::genesis_signature()[0].try_into().unwrap(),
             ),
diff --git a/mithril-common/src/test_utils/fake_data.rs b/mithril-common/src/test_utils/fake_data.rs
@@ -134,7 +134,9 @@ pub fn certificate(certificate_hash: String) -> entities::Certificate {
 
     // Certificate
     let previous_hash = format!("{certificate_hash}0");
-    let aggregate_verification_key = fake_keys::aggregate_verification_key()[1].to_owned();
+    let aggregate_verification_key = fake_keys::aggregate_verification_key()[1]
+        .try_into()
+        .unwrap();
     let multi_signature = fake_keys::multi_signature()[0].try_into().unwrap();
 
     entities::Certificate {
