diff --git a/Cargo.lock b/Cargo.lock index 33e0e4569a7..15953ca692e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3928,7 +3928,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.7.76" +version = "0.7.77" dependencies = [ "anyhow", "async-trait", @@ -4180,7 +4180,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.6.11" +version = "0.6.12" dependencies = [ "anyhow", "async-trait", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 6e54759ea8f..c00570a88d7 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.7.76" +version = "0.7.77" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-aggregator/src/database/query/certificate/get_certificate.rs b/mithril-aggregator/src/database/query/certificate/get_certificate.rs index f3343da48b5..92816b37330 100644 --- a/mithril-aggregator/src/database/query/certificate/get_certificate.rs +++ b/mithril-aggregator/src/database/query/certificate/get_certificate.rs @@ -82,7 +82,7 @@ mod tests { let expected_certificate_records: Vec = 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 = 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 = - certificates.reversed_chain().into_iter().map(Into::into).collect(); + let expected_certificate_records: Vec = 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(); diff --git a/mithril-aggregator/src/database/query/certificate/insert_certificate.rs b/mithril-aggregator/src/database/query/certificate/insert_certificate.rs index 93d49eba4ce..d288d94d2e5 100644 --- a/mithril-aggregator/src/database/query/certificate/insert_certificate.rs +++ b/mithril-aggregator/src/database/query/certificate/insert_certificate.rs @@ -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,7 @@ mod tests { #[test] fn test_insert_many_certificates_records() { let certificates = setup_certificate_chain(5, 2); - let certificates_records: Vec = certificates.into(); + let certificates_records: Vec = certificates.try_into().unwrap(); let connection = main_db_connection().unwrap(); diff --git a/mithril-aggregator/src/database/query/certificate/insert_or_replace_certificate.rs b/mithril-aggregator/src/database/query/certificate/insert_or_replace_certificate.rs index e604f165286..8433813400d 100644 --- a/mithril-aggregator/src/database/query/certificate/insert_or_replace_certificate.rs +++ b/mithril-aggregator/src/database/query/certificate/insert_or_replace_certificate.rs @@ -50,7 +50,7 @@ mod tests { #[test] fn test_insert_many_certificates_records_in_empty_db() { let certificates = setup_certificate_chain(5, 2); - let certificates_records: Vec = certificates.into(); + let certificates_records: Vec = certificates.try_into().unwrap(); let connection = main_db_connection().unwrap(); @@ -75,7 +75,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 +106,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(); diff --git a/mithril-aggregator/src/database/record/certificate.rs b/mithril-aggregator/src/database/record/certificate.rs index e6f517565c9..b55d31c93c1 100644 --- a/mithril-aggregator/src/database/record/certificate.rs +++ b/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 for CertificateRecord { - fn from(other: Certificate) -> Self { +impl TryFrom for CertificateRecord { + type Error = StdError; + + fn try_from(other: Certificate) -> Result { 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,12 +155,16 @@ impl From for CertificateRecord { signers: other.metadata.signers, initiated_at: other.metadata.initiated_at, sealed_at: other.metadata.sealed_at, - } + }; + + Ok(certificate_record) } } -impl From for Certificate { - fn from(other: CertificateRecord) -> Self { +impl TryFrom for Certificate { + type Error = StdError; + + fn try_from(other: CertificateRecord) -> Result { let certificate_metadata = CertificateMetadata::new( other.network, other.protocol_version, @@ -171,27 +176,29 @@ impl From for Certificate { let (previous_hash, signature) = match other.parent_certificate_id { None => ( String::new(), - CertificateSignature::GenesisSignature(other.signature.try_into().unwrap()), + CertificateSignature::GenesisSignature(other.signature.try_into()?), ), Some(parent_certificate_id) => ( parent_certificate_id, CertificateSignature::MultiSignature( other.signed_entity_type, - other.signature.try_into().unwrap(), + other.signature.try_into()?, ), ), }; - Certificate { + let certificate = Certificate { hash: other.certificate_id, previous_hash, epoch: other.epoch, metadata: certificate_metadata, signed_message: other.protocol_message.compute_hash(), protocol_message: other.protocol_message, - aggregate_verification_key: other.aggregate_verification_key.try_into().unwrap(), + aggregate_verification_key: other.aggregate_verification_key.try_into()?, signature, - } + }; + + Ok(certificate) } } @@ -393,11 +400,11 @@ mod tests { let certificates = setup_certificate_chain(20, 3); let mut certificate_records: Vec = 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 = Vec::new(); for certificate_record in certificate_records { - certificates_new.push(certificate_record.into()); + certificates_new.push(certificate_record.try_into().unwrap()); } assert_eq!(certificates.certificates_chained, certificates_new); } @@ -406,7 +413,7 @@ mod tests { fn converting_certificate_record_to_certificate_should_not_recompute_hash() { let expected_hash = "my_hash"; let record = CertificateRecord::dummy_genesis(expected_hash, Epoch(1)); - let certificate: Certificate = record.into(); + let certificate: Certificate = record.try_into().unwrap(); assert_eq!(expected_hash, &certificate.hash); } diff --git a/mithril-aggregator/src/database/repository/certificate_repository.rs b/mithril-aggregator/src/database/repository/certificate_repository.rs index f1d2c4572f5..13fd968346d 100644 --- a/mithril-aggregator/src/database/repository/certificate_repository.rs +++ b/mithril-aggregator/src/database/repository/certificate_repository.rs @@ -4,9 +4,9 @@ use anyhow::anyhow; use async_trait::async_trait; use sqlite::ConnectionThreadSafe; -use mithril_common::StdResult; use mithril_common::certificate_chain::{CertificateRetriever, CertificateRetrieverError}; use mithril_common::entities::{Certificate, Epoch}; +use mithril_common::{StdError, StdResult}; use mithril_persistence::sqlite::ConnectionExtensions; use crate::database::query::{ @@ -30,35 +30,41 @@ impl CertificateRepository { /// Return the certificate corresponding to the given hash if any. pub async fn get_certificate(&self, hash: &str) -> StdResult> where - T: From, + T: TryFrom, + T::Error: Into, { let record = self .connection .fetch_first(GetCertificateRecordQuery::by_certificate_id(hash))?; - Ok(record.map(|c| c.into())) + record.map(|c| c.try_into().map_err(Into::into)).transpose() } /// Return the latest certificates. pub async fn get_latest_certificates(&self, last_n: usize) -> StdResult> where - T: From, + T: TryFrom, + T::Error: Into, { let cursor = self.connection.fetch(GetCertificateRecordQuery::all())?; - Ok(cursor.take(last_n).map(|v| v.into()).collect()) + cursor + .take(last_n) + .map(|c| c.try_into().map_err(Into::into)) + .collect() } /// Return the latest genesis certificate. pub async fn get_latest_genesis_certificate(&self) -> StdResult> where - T: From, + T: TryFrom, + T::Error: Into, { let record = self .connection .fetch_first(GetCertificateRecordQuery::all_genesis())?; - Ok(record.map(|c| c.into())) + record.map(|c| c.try_into().map_err(Into::into)).transpose() } /// Return the first certificate signed per epoch as the reference @@ -66,13 +72,14 @@ impl CertificateRepository { /// other certificates issued within this Epoch. pub async fn get_master_certificate_for_epoch(&self, epoch: Epoch) -> StdResult> where - T: From, + T: TryFrom, + T::Error: Into, { let record = self .connection .fetch_first(MasterCertificateQuery::for_epoch(epoch))?; - Ok(record.map(|c| c.into())) + record.map(|c| c.try_into().map_err(Into::into)).transpose() } /// Create a new certificate in the database. @@ -80,13 +87,13 @@ 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:#?}") }); - Ok(record.into()) + record.try_into() } /// Create many certificates at once in the database. @@ -98,12 +105,14 @@ impl CertificateRepository { return Ok(vec![]); } - let records: Vec = - certificates.into_iter().map(|cert| cert.into()).collect(); + let records: Vec = certificates + .into_iter() + .map(|cert| cert.try_into()) + .collect::>()?; let new_certificates = self.connection.fetch(InsertCertificateRecordQuery::many(records))?; - Ok(new_certificates.map(|cert| cert.into()).collect()) + new_certificates.map(|cert| cert.try_into()).collect::>() } /// Create, or replace if they already exist, many certificates at once in the database. @@ -115,13 +124,15 @@ impl CertificateRepository { return Ok(vec![]); } - let records: Vec = - certificates.into_iter().map(|cert| cert.into()).collect(); + let records: Vec = certificates + .into_iter() + .map(|cert| cert.try_into()) + .collect::>()?; let new_certificates = self .connection .fetch(InsertOrReplaceCertificateRecordQuery::many(records))?; - Ok(new_certificates.map(|cert| cert.into()).collect()) + new_certificates.map(|cert| cert.try_into()).collect::>() } /// Delete all the given certificates from the database @@ -331,7 +342,7 @@ mod tests { async fn get_master_certificate_one_cert_in_current_epoch_recorded_returns_that_one() { let connection = Arc::new(main_db_connection().unwrap()); let certificate = CertificateRecord::dummy_genesis("1", Epoch(1)); - let expected_certificate: Certificate = certificate.clone().into(); + let expected_certificate: Certificate = certificate.clone().try_into().unwrap(); insert_certificate_records(&connection, vec![certificate]); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -353,7 +364,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("2", "1", Epoch(1), 2), CertificateRecord::dummy_db_snapshot("3", "1", Epoch(1), 3), ]; - let expected_certificate: Certificate = certificates.first().unwrap().clone().into(); + let expected_certificate: Certificate = + certificates.first().unwrap().clone().try_into().unwrap(); insert_certificate_records(&connection, certificates); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -375,7 +387,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("2", "1", Epoch(1), 2), CertificateRecord::dummy_db_snapshot("3", "1", Epoch(1), 3), ]; - let expected_certificate: Certificate = certificates.first().unwrap().clone().into(); + let expected_certificate: Certificate = + certificates.first().unwrap().clone().try_into().unwrap(); insert_certificate_records(&connection, certificates); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -398,7 +411,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("3", "1", Epoch(1), 3), CertificateRecord::dummy_db_snapshot("4", "1", Epoch(2), 4), ]; - let expected_certificate: Certificate = certificates.last().unwrap().clone().into(); + let expected_certificate: Certificate = + certificates.last().unwrap().clone().try_into().unwrap(); insert_certificate_records(&connection, certificates); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -423,7 +437,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("5", "4", Epoch(2), 5), CertificateRecord::dummy_db_snapshot("6", "4", Epoch(2), 6), ]; - let expected_certificate: Certificate = certificates.get(3).unwrap().clone().into(); + let expected_certificate: Certificate = + certificates.get(3).unwrap().clone().try_into().unwrap(); insert_certificate_records(&connection, certificates); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -465,7 +480,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("3", "1", Epoch(1), 3), CertificateRecord::dummy_genesis("4", Epoch(1)), ]; - let expected_certificate: Certificate = certificates.last().unwrap().clone().into(); + let expected_certificate: Certificate = + certificates.last().unwrap().clone().try_into().unwrap(); insert_certificate_records(&connection, certificates); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -490,7 +506,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("5", "1", Epoch(2), 5), CertificateRecord::dummy_genesis("6", Epoch(2)), ]; - let expected_certificate: Certificate = certificates.last().unwrap().clone().into(); + let expected_certificate: Certificate = + certificates.last().unwrap().clone().try_into().unwrap(); insert_certificate_records(&connection, certificates); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -513,7 +530,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("3", "1", Epoch(1), 3), CertificateRecord::dummy_genesis("4", Epoch(2)), ]; - let expected_certificate: Certificate = certificates.last().unwrap().clone().into(); + let expected_certificate: Certificate = + certificates.last().unwrap().clone().try_into().unwrap(); insert_certificate_records(&connection, certificates); let repository: CertificateRepository = CertificateRepository::new(connection); @@ -574,7 +592,8 @@ mod tests { CertificateRecord::dummy_db_snapshot("3", "1", Epoch(1), 3), ]; insert_certificate_records(&connection, records.clone()); - let certificates: Vec = records.into_iter().map(|c| c.into()).collect(); + let certificates: Vec = + records.into_iter().map(|c| c.try_into().unwrap()).collect(); // Delete all records except the first repository diff --git a/mithril-aggregator/src/database/test_helper.rs b/mithril-aggregator/src/database/test_helper.rs index 9d3ed820bc4..73992afc682 100644 --- a/mithril-aggregator/src/database/test_helper.rs +++ b/mithril-aggregator/src/database/test_helper.rs @@ -183,14 +183,19 @@ pub fn insert_buffered_single_signatures( Ok(()) } -pub fn insert_certificate_records>( - connection: &ConnectionThreadSafe, - records: Vec, -) { +pub fn insert_certificate_records(connection: &ConnectionThreadSafe, records: Vec) +where + T: TryInto, + T::Error: Into, +{ + let records: Vec = records + .into_iter() + .map(|c| c.try_into().map_err(Into::into)) + .collect::>() + .unwrap(); + let _ = connection - .fetch_first(InsertCertificateRecordQuery::many( - records.into_iter().map(Into::into).collect(), - )) + .fetch_first(InsertCertificateRecordQuery::many(records)) .unwrap(); } diff --git a/mithril-aggregator/src/tools/certificates_hash_migrator.rs b/mithril-aggregator/src/tools/certificates_hash_migrator.rs index 6b477687e73..a2b8df45a0f 100644 --- a/mithril-aggregator/src/tools/certificates_hash_migrator.rs +++ b/mithril-aggregator/src/tools/certificates_hash_migrator.rs @@ -254,7 +254,7 @@ mod test { fn dummy_genesis(certificate_hash: &str, time_point: TimePoint) -> Certificate { let certificate = CertificateRecord::dummy_genesis(certificate_hash, time_point.epoch); - certificate.into() + certificate.try_into().unwrap() } fn dummy_certificate( @@ -272,7 +272,7 @@ mod test { .unwrap(), ); - certificate.into() + certificate.try_into().unwrap() } fn signed_entity_for_certificate(certificate: &Certificate) -> Option { diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index accbd6eb4e9..105bcdafa63 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.6.11" +version = "0.6.12" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-common/src/test/builder/certificate_chain_builder.rs b/mithril-common/src/test/builder/certificate_chain_builder.rs index fcd6f15c8ec..d580bfb9254 100644 --- a/mithril-common/src/test/builder/certificate_chain_builder.rs +++ b/mithril-common/src/test/builder/certificate_chain_builder.rs @@ -123,9 +123,11 @@ impl DerefMut for CertificateChainFixture { } } -impl> From for Vec { - fn from(fixture: CertificateChainFixture) -> Self { - fixture.certificates_chained.into_iter().map(C::from).collect() +impl> TryFrom for Vec { + type Error = C::Error; + + fn try_from(fixture: CertificateChainFixture) -> Result { + fixture.certificates_chained.into_iter().map(C::try_from).collect() } }