Skip to content

Commit 1069937

Browse files
dlachaumedlachaume
authored
Merge pull request #2652 from input-output-hk/dlachaume/2651/impl-try-from-certificate-and-certificate-record
refactor: use `TryFrom` for `Certificate` and `CertificateRecord` conversions
2 parents cecb76b + dc4c8a5 commit 1069937

File tree

11 files changed

+122
-80
lines changed

11 files changed

+122
-80
lines changed

‎Cargo.lock

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎mithril-aggregator/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "mithril-aggregator"
3-
version = "0.7.76"
3+
version = "0.7.77"
44
description = "A Mithril Aggregator server"
55
authors = { workspace = true }
66
edition = { workspace = true }

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

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ mod tests {
8282
let expected_certificate_records: Vec<CertificateRecord> = certificates
8383
.reversed_chain()
8484
.into_iter()
85-
.filter_map(|c| (c.epoch == Epoch(1)).then_some(c.to_owned().into()))
85+
.filter_map(|c| (c.epoch == Epoch(1)).then_some(c.to_owned().try_into().unwrap()))
8686
.collect();
8787
assert_eq!(expected_certificate_records, certificate_records);
8888

@@ -92,7 +92,7 @@ mod tests {
9292
let expected_certificate_records: Vec<CertificateRecord> = certificates
9393
.reversed_chain()
9494
.into_iter()
95-
.filter_map(|c| (c.epoch == Epoch(3)).then_some(c.to_owned().into()))
95+
.filter_map(|c| (c.epoch == Epoch(3)).then_some(c.to_owned().try_into().unwrap()))
9696
.collect();
9797
assert_eq!(expected_certificate_records, certificate_records);
9898

@@ -105,8 +105,11 @@ mod tests {
105105
#[test]
106106
fn test_get_all_certificate_records() {
107107
let certificates = setup_certificate_chain(5, 2);
108-
let expected_certificate_records: Vec<CertificateRecord> =
109-
certificates.reversed_chain().into_iter().map(Into::into).collect();
108+
let expected_certificate_records: Vec<CertificateRecord> = certificates
109+
.reversed_chain()
110+
.into_iter()
111+
.map(|c| c.try_into().unwrap())
112+
.collect();
110113

111114
let connection = main_db_connection().unwrap();
112115
insert_certificate_records(&connection, certificates.certificates_chained.clone());
@@ -127,8 +130,11 @@ mod tests {
127130
phi_f: 0.65,
128131
})
129132
.build();
130-
let first_chain_genesis: CertificateRecord =
131-
first_certificates_chain.genesis_certificate().clone().into();
133+
let first_chain_genesis: CertificateRecord = first_certificates_chain
134+
.genesis_certificate()
135+
.clone()
136+
.try_into()
137+
.unwrap();
132138
let second_certificates_chain = CertificateChainBuilder::new()
133139
.with_total_certificates(2)
134140
.with_protocol_parameters(ProtocolParameters {
@@ -137,8 +143,11 @@ mod tests {
137143
phi_f: 0.65,
138144
})
139145
.build();
140-
let second_chain_genesis: CertificateRecord =
141-
second_certificates_chain.genesis_certificate().clone().into();
146+
let second_chain_genesis: CertificateRecord = second_certificates_chain
147+
.genesis_certificate()
148+
.clone()
149+
.try_into()
150+
.unwrap();
142151
assert_ne!(first_chain_genesis, second_chain_genesis);
143152

144153
let connection = main_db_connection().unwrap();

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ mod tests {
5454
let connection = main_db_connection().unwrap();
5555

5656
for certificate in certificates.certificates_chained {
57-
let certificate_record: CertificateRecord = certificate.into();
57+
let certificate_record: CertificateRecord = certificate.try_into().unwrap();
5858
let certificate_record_saved = connection
5959
.fetch_first(InsertCertificateRecordQuery::one(
6060
certificate_record.clone(),
@@ -67,7 +67,7 @@ mod tests {
6767
#[test]
6868
fn test_insert_many_certificates_records() {
6969
let certificates = setup_certificate_chain(5, 2);
70-
let certificates_records: Vec<CertificateRecord> = certificates.into();
70+
let certificates_records: Vec<CertificateRecord> = certificates.try_into().unwrap();
7171

7272
let connection = main_db_connection().unwrap();
7373

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ mod tests {
5050
#[test]
5151
fn test_insert_many_certificates_records_in_empty_db() {
5252
let certificates = setup_certificate_chain(5, 2);
53-
let certificates_records: Vec<CertificateRecord> = certificates.into();
53+
let certificates_records: Vec<CertificateRecord> = certificates.try_into().unwrap();
5454

5555
let connection = main_db_connection().unwrap();
5656

@@ -75,7 +75,7 @@ mod tests {
7575
fn test_replace_one_certificate_record() {
7676
let certificate_record = CertificateRecord {
7777
epoch: Epoch(12),
78-
..fake_data::certificate("hash").into()
78+
..fake_data::certificate("hash").try_into().unwrap()
7979
};
8080

8181
let connection = main_db_connection().unwrap();
@@ -106,26 +106,26 @@ mod tests {
106106
let tested_records: HashMap<_, CertificateRecord> = HashMap::from([
107107
(
108108
"cert1-genesis",
109-
fake_data::genesis_certificate("genesis").into(),
109+
fake_data::genesis_certificate("genesis").try_into().unwrap(),
110110
),
111-
("cert2", fake_data::certificate("cert2").into()),
111+
("cert2", fake_data::certificate("cert2").try_into().unwrap()),
112112
(
113113
"cert2-modified",
114114
CertificateRecord {
115115
epoch: Epoch(14),
116-
..fake_data::certificate("cert2").into()
116+
..fake_data::certificate("cert2").try_into().unwrap()
117117
},
118118
),
119-
("cert3", fake_data::certificate("cert3").into()),
120-
("cert4", fake_data::certificate("cert4").into()),
119+
("cert3", fake_data::certificate("cert3").try_into().unwrap()),
120+
("cert4", fake_data::certificate("cert4").try_into().unwrap()),
121121
(
122122
"cert4-modified",
123123
CertificateRecord {
124124
epoch: Epoch(32),
125-
..fake_data::certificate("cert4").into()
125+
..fake_data::certificate("cert4").try_into().unwrap()
126126
},
127127
),
128-
("cert5", fake_data::certificate("cert5").into()),
128+
("cert5", fake_data::certificate("cert5").try_into().unwrap()),
129129
]);
130130
let connection = main_db_connection().unwrap();
131131

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

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use chrono::{DateTime, Utc};
22

3+
use mithril_common::StdError;
34
use mithril_common::entities::{
45
Certificate, CertificateMetadata, CertificateSignature, Epoch,
56
HexEncodedAggregateVerificationKey, HexEncodedKey, ProtocolMessage, ProtocolParameters,
@@ -127,24 +128,24 @@ impl CertificateRecord {
127128
}
128129
}
129130

130-
impl From<Certificate> for CertificateRecord {
131-
fn from(other: Certificate) -> Self {
131+
impl TryFrom<Certificate> for CertificateRecord {
132+
type Error = StdError;
133+
134+
fn try_from(other: Certificate) -> Result<Self, Self::Error> {
132135
let signed_entity_type = other.signed_entity_type();
133136
let (signature, parent_certificate_id) = match other.signature {
134-
CertificateSignature::GenesisSignature(signature) => {
135-
(signature.to_bytes_hex().unwrap(), None)
136-
}
137+
CertificateSignature::GenesisSignature(signature) => (signature.to_bytes_hex()?, None),
137138
CertificateSignature::MultiSignature(_, signature) => {
138-
(signature.to_json_hex().unwrap(), Some(other.previous_hash))
139+
(signature.to_json_hex()?, Some(other.previous_hash))
139140
}
140141
};
141142

142-
CertificateRecord {
143+
let certificate_record = CertificateRecord {
143144
certificate_id: other.hash,
144145
parent_certificate_id,
145146
message: other.signed_message,
146147
signature,
147-
aggregate_verification_key: other.aggregate_verification_key.to_json_hex().unwrap(),
148+
aggregate_verification_key: other.aggregate_verification_key.to_json_hex()?,
148149
epoch: other.epoch,
149150
network: other.metadata.network,
150151
signed_entity_type,
@@ -154,12 +155,16 @@ impl From<Certificate> for CertificateRecord {
154155
signers: other.metadata.signers,
155156
initiated_at: other.metadata.initiated_at,
156157
sealed_at: other.metadata.sealed_at,
157-
}
158+
};
159+
160+
Ok(certificate_record)
158161
}
159162
}
160163

161-
impl From<CertificateRecord> for Certificate {
162-
fn from(other: CertificateRecord) -> Self {
164+
impl TryFrom<CertificateRecord> for Certificate {
165+
type Error = StdError;
166+
167+
fn try_from(other: CertificateRecord) -> Result<Self, Self::Error> {
163168
let certificate_metadata = CertificateMetadata::new(
164169
other.network,
165170
other.protocol_version,
@@ -171,27 +176,29 @@ impl From<CertificateRecord> for Certificate {
171176
let (previous_hash, signature) = match other.parent_certificate_id {
172177
None => (
173178
String::new(),
174-
CertificateSignature::GenesisSignature(other.signature.try_into().unwrap()),
179+
CertificateSignature::GenesisSignature(other.signature.try_into()?),
175180
),
176181
Some(parent_certificate_id) => (
177182
parent_certificate_id,
178183
CertificateSignature::MultiSignature(
179184
other.signed_entity_type,
180-
other.signature.try_into().unwrap(),
185+
other.signature.try_into()?,
181186
),
182187
),
183188
};
184189

185-
Certificate {
190+
let certificate = Certificate {
186191
hash: other.certificate_id,
187192
previous_hash,
188193
epoch: other.epoch,
189194
metadata: certificate_metadata,
190195
signed_message: other.protocol_message.compute_hash(),
191196
protocol_message: other.protocol_message,
192-
aggregate_verification_key: other.aggregate_verification_key.try_into().unwrap(),
197+
aggregate_verification_key: other.aggregate_verification_key.try_into()?,
193198
signature,
194-
}
199+
};
200+
201+
Ok(certificate)
195202
}
196203
}
197204

@@ -393,11 +400,11 @@ mod tests {
393400
let certificates = setup_certificate_chain(20, 3);
394401
let mut certificate_records: Vec<CertificateRecord> = Vec::new();
395402
for certificate in certificates.certificates_chained.clone() {
396-
certificate_records.push(certificate.into());
403+
certificate_records.push(certificate.try_into().unwrap());
397404
}
398405
let mut certificates_new: Vec<Certificate> = Vec::new();
399406
for certificate_record in certificate_records {
400-
certificates_new.push(certificate_record.into());
407+
certificates_new.push(certificate_record.try_into().unwrap());
401408
}
402409
assert_eq!(certificates.certificates_chained, certificates_new);
403410
}
@@ -406,7 +413,7 @@ mod tests {
406413
fn converting_certificate_record_to_certificate_should_not_recompute_hash() {
407414
let expected_hash = "my_hash";
408415
let record = CertificateRecord::dummy_genesis(expected_hash, Epoch(1));
409-
let certificate: Certificate = record.into();
416+
let certificate: Certificate = record.try_into().unwrap();
410417

411418
assert_eq!(expected_hash, &certificate.hash);
412419
}

0 commit comments

Comments
 (0)