refactor(aggregator): implement TryFrom<Certificate> for CertificateRecord

Author: dlachaume

mithril-aggregator/src/database/query/certificate/get_certificate.rs

@@ -82,7 +82,7 @@ mod tests {
         let expected_certificate_records: Vec<CertificateRecord> = certificates
             .reversed_chain()
             .into_iter()
-            .filter_map(|c| (c.epoch == Epoch(1)).then_some(c.to_owned().into()))
+            .filter_map(|c| (c.epoch == Epoch(1)).then_some(c.to_owned().try_into().unwrap()))
             .collect();
         assert_eq!(expected_certificate_records, certificate_records);
 
@@ -92,7 +92,7 @@ mod tests {
         let expected_certificate_records: Vec<CertificateRecord> = certificates
             .reversed_chain()
             .into_iter()
-            .filter_map(|c| (c.epoch == Epoch(3)).then_some(c.to_owned().into()))
+            .filter_map(|c| (c.epoch == Epoch(3)).then_some(c.to_owned().try_into().unwrap()))
             .collect();
         assert_eq!(expected_certificate_records, certificate_records);
 
@@ -105,8 +105,11 @@ mod tests {
     #[test]
     fn test_get_all_certificate_records() {
         let certificates = setup_certificate_chain(5, 2);
-        let expected_certificate_records: Vec<CertificateRecord> =
-            certificates.reversed_chain().into_iter().map(Into::into).collect();
+        let expected_certificate_records: Vec<CertificateRecord> = certificates
+            .reversed_chain()
+            .into_iter()
+            .map(|c| c.try_into().unwrap())
+            .collect();
 
         let connection = main_db_connection().unwrap();
         insert_certificate_records(&connection, certificates.certificates_chained.clone());
@@ -127,8 +130,11 @@ mod tests {
                 phi_f: 0.65,
             })
             .build();
-        let first_chain_genesis: CertificateRecord =
-            first_certificates_chain.genesis_certificate().clone().into();
+        let first_chain_genesis: CertificateRecord = first_certificates_chain
+            .genesis_certificate()
+            .clone()
+            .try_into()
+            .unwrap();
         let second_certificates_chain = CertificateChainBuilder::new()
             .with_total_certificates(2)
             .with_protocol_parameters(ProtocolParameters {
@@ -137,8 +143,11 @@ mod tests {
                 phi_f: 0.65,
             })
             .build();
-        let second_chain_genesis: CertificateRecord =
-            second_certificates_chain.genesis_certificate().clone().into();
+        let second_chain_genesis: CertificateRecord = second_certificates_chain
+            .genesis_certificate()
+            .clone()
+            .try_into()
+            .unwrap();
         assert_ne!(first_chain_genesis, second_chain_genesis);
 
         let connection = main_db_connection().unwrap();

mithril-aggregator/src/database/query/certificate/insert_certificate.rs

@@ -40,7 +40,7 @@ impl Query for InsertCertificateRecordQuery {
 
 #[cfg(test)]
 mod tests {
-    use mithril_common::test::crypto_helper::setup_certificate_chain;
+    use mithril_common::{entities::Certificate, test::crypto_helper::setup_certificate_chain};
     use mithril_persistence::sqlite::ConnectionExtensions;
 
     use crate::database::test_helper::main_db_connection;
@@ -54,7 +54,7 @@ mod tests {
         let connection = main_db_connection().unwrap();
 
         for certificate in certificates.certificates_chained {
-            let certificate_record: CertificateRecord = certificate.into();
+            let certificate_record: CertificateRecord = certificate.try_into().unwrap();
             let certificate_record_saved = connection
                 .fetch_first(InsertCertificateRecordQuery::one(
                     certificate_record.clone(),
@@ -67,7 +67,9 @@ mod tests {
     #[test]
     fn test_insert_many_certificates_records() {
         let certificates = setup_certificate_chain(5, 2);
-        let certificates_records: Vec<CertificateRecord> = certificates.into();
+        let certificates: Vec<Certificate> = certificates.into();
+        let certificates_records: Vec<CertificateRecord> =
+            certificates.into_iter().map(|c| c.try_into().unwrap()).collect();
 
         let connection = main_db_connection().unwrap();
 

mithril-aggregator/src/database/query/certificate/insert_or_replace_certificate.rs

@@ -38,7 +38,7 @@ impl Query for InsertOrReplaceCertificateRecordQuery {
 mod tests {
     use std::collections::HashMap;
 
-    use mithril_common::entities::Epoch;
+    use mithril_common::entities::{Certificate, Epoch};
     use mithril_common::test::{crypto_helper::setup_certificate_chain, double::fake_data};
     use mithril_persistence::sqlite::ConnectionExtensions;
 
@@ -50,7 +50,9 @@ mod tests {
     #[test]
     fn test_insert_many_certificates_records_in_empty_db() {
         let certificates = setup_certificate_chain(5, 2);
-        let certificates_records: Vec<CertificateRecord> = certificates.into();
+        let certificates: Vec<Certificate> = certificates.into();
+        let certificates_records: Vec<CertificateRecord> =
+            certificates.into_iter().map(|c| c.try_into().unwrap()).collect();
 
         let connection = main_db_connection().unwrap();
 
@@ -75,7 +77,7 @@ mod tests {
     fn test_replace_one_certificate_record() {
         let certificate_record = CertificateRecord {
             epoch: Epoch(12),
-            ..fake_data::certificate("hash").into()
+            ..fake_data::certificate("hash").try_into().unwrap()
         };
 
         let connection = main_db_connection().unwrap();
@@ -106,26 +108,26 @@ mod tests {
         let tested_records: HashMap<_, CertificateRecord> = HashMap::from([
             (
                 "cert1-genesis",
-                fake_data::genesis_certificate("genesis").into(),
+                fake_data::genesis_certificate("genesis").try_into().unwrap(),
             ),
-            ("cert2", fake_data::certificate("cert2").into()),
+            ("cert2", fake_data::certificate("cert2").try_into().unwrap()),
             (
                 "cert2-modified",
                 CertificateRecord {
                     epoch: Epoch(14),
-                    ..fake_data::certificate("cert2").into()
+                    ..fake_data::certificate("cert2").try_into().unwrap()
                 },
             ),
-            ("cert3", fake_data::certificate("cert3").into()),
-            ("cert4", fake_data::certificate("cert4").into()),
+            ("cert3", fake_data::certificate("cert3").try_into().unwrap()),
+            ("cert4", fake_data::certificate("cert4").try_into().unwrap()),
             (
                 "cert4-modified",
                 CertificateRecord {
                     epoch: Epoch(32),
-                    ..fake_data::certificate("cert4").into()
+                    ..fake_data::certificate("cert4").try_into().unwrap()
                 },
             ),
-            ("cert5", fake_data::certificate("cert5").into()),
+            ("cert5", fake_data::certificate("cert5").try_into().unwrap()),
         ]);
         let connection = main_db_connection().unwrap();
 

mithril-aggregator/src/database/record/certificate.rs

@@ -1,5 +1,6 @@
 use chrono::{DateTime, Utc};
 
+use mithril_common::StdError;
 use mithril_common::entities::{
     Certificate, CertificateMetadata, CertificateSignature, Epoch,
     HexEncodedAggregateVerificationKey, HexEncodedKey, ProtocolMessage, ProtocolParameters,
@@ -127,24 +128,24 @@ impl CertificateRecord {
     }
 }
 
-impl From<Certificate> for CertificateRecord {
-    fn from(other: Certificate) -> Self {
+impl TryFrom<Certificate> for CertificateRecord {
+    type Error = StdError;
+
+    fn try_from(other: Certificate) -> Result<Self, Self::Error> {
         let signed_entity_type = other.signed_entity_type();
         let (signature, parent_certificate_id) = match other.signature {
-            CertificateSignature::GenesisSignature(signature) => {
-                (signature.to_bytes_hex().unwrap(), None)
-            }
+            CertificateSignature::GenesisSignature(signature) => (signature.to_bytes_hex()?, None),
             CertificateSignature::MultiSignature(_, signature) => {
-                (signature.to_json_hex().unwrap(), Some(other.previous_hash))
+                (signature.to_json_hex()?, Some(other.previous_hash))
             }
         };
 
-        CertificateRecord {
+        let certificate_record = CertificateRecord {
             certificate_id: other.hash,
             parent_certificate_id,
             message: other.signed_message,
             signature,
-            aggregate_verification_key: other.aggregate_verification_key.to_json_hex().unwrap(),
+            aggregate_verification_key: other.aggregate_verification_key.to_json_hex()?,
             epoch: other.epoch,
             network: other.metadata.network,
             signed_entity_type,
@@ -154,7 +155,9 @@ impl From<Certificate> for CertificateRecord {
             signers: other.metadata.signers,
             initiated_at: other.metadata.initiated_at,
             sealed_at: other.metadata.sealed_at,
-        }
+        };
+
+        Ok(certificate_record)
     }
 }
 
@@ -393,7 +396,7 @@ mod tests {
         let certificates = setup_certificate_chain(20, 3);
         let mut certificate_records: Vec<CertificateRecord> = Vec::new();
         for certificate in certificates.certificates_chained.clone() {
-            certificate_records.push(certificate.into());
+            certificate_records.push(certificate.try_into().unwrap());
         }
         let mut certificates_new: Vec<Certificate> = Vec::new();
         for certificate_record in certificate_records {

mithril-aggregator/src/database/repository/certificate_repository.rs

@@ -80,7 +80,7 @@ impl CertificateRepository {
         let record = self
             .connection
             .fetch_first(InsertCertificateRecordQuery::one(
-                certificate.clone().into(),
+                certificate.clone().try_into()?,
             ))?
             .unwrap_or_else(|| {
                 panic!("No entity returned by the persister, certificate = {certificate:#?}")
@@ -98,8 +98,10 @@ impl CertificateRepository {
             return Ok(vec![]);
         }
 
-        let records: Vec<CertificateRecord> =
-            certificates.into_iter().map(|cert| cert.into()).collect();
+        let records: Vec<CertificateRecord> = certificates
+            .into_iter()
+            .map(|cert| cert.try_into())
+            .collect::<StdResult<_>>()?;
         let new_certificates =
             self.connection.fetch(InsertCertificateRecordQuery::many(records))?;
 
@@ -115,8 +117,10 @@ impl CertificateRepository {
             return Ok(vec![]);
         }
 
-        let records: Vec<CertificateRecord> =
-            certificates.into_iter().map(|cert| cert.into()).collect();
+        let records: Vec<CertificateRecord> = certificates
+            .into_iter()
+            .map(|cert| cert.try_into())
+            .collect::<StdResult<_>>()?;
         let new_certificates = self
             .connection
             .fetch(InsertOrReplaceCertificateRecordQuery::many(records))?;

mithril-aggregator/src/database/test_helper.rs

@@ -183,14 +183,19 @@ pub fn insert_buffered_single_signatures(
     Ok(())
 }
 
-pub fn insert_certificate_records<T: Into<CertificateRecord>>(
-    connection: &ConnectionThreadSafe,
-    records: Vec<T>,
-) {
+pub fn insert_certificate_records<T>(connection: &ConnectionThreadSafe, records: Vec<T>)
+where
+    T: TryInto<CertificateRecord>,
+    T::Error: Into<StdError>,
+{
+    let records: Vec<CertificateRecord> = records
+        .into_iter()
+        .map(|c| c.try_into().map_err(Into::into))
+        .collect::<StdResult<_>>()
+        .unwrap();
+
     let _ = connection
-        .fetch_first(InsertCertificateRecordQuery::many(
-            records.into_iter().map(Into::into).collect(),
-        ))
+        .fetch_first(InsertCertificateRecordQuery::many(records))
         .unwrap();
 }