Skip to content

Commit 1796fdb

Browse files
committed
fix: address PR reviews comments
- fix: synchronizer create the open messages based on the first certificate of the last epoch instead of the latest certificate - move `MockCertificateVerifier` definition to `tools::mocks` - fix spelling issues
1 parent 9bebed2 commit 1796fdb

File tree

15 files changed

+118
-112
lines changed

15 files changed

+118
-112
lines changed

mithril-aggregator/src/dependency_injection/builder/mod.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,8 @@ pub struct DependenciesBuilder {
185185
/// Genesis signature verifier service.
186186
pub genesis_verifier: Option<Arc<ProtocolGenesisVerifier>>,
187187

188-
/// Certificate chain synchroniser service
189-
pub certificate_chain_synchroniser: Option<Arc<dyn CertificateChainSynchronizer>>,
188+
/// Certificate chain synchronizer service
189+
pub certificate_chain_synchronizer: Option<Arc<dyn CertificateChainSynchronizer>>,
190190

191191
/// Mithril signer registration leader service
192192
pub mithril_signer_registration_leader: Option<Arc<MithrilSignerRegistrationLeader>>,
@@ -315,7 +315,7 @@ impl DependenciesBuilder {
315315
snapshotter: None,
316316
certificate_verifier: None,
317317
genesis_verifier: None,
318-
certificate_chain_synchroniser: None,
318+
certificate_chain_synchronizer: None,
319319
mithril_signer_registration_leader: None,
320320
mithril_signer_registration_follower: None,
321321
signer_registerer: None,
@@ -378,7 +378,7 @@ impl DependenciesBuilder {
378378
verification_key_store: self.get_verification_key_store().await?,
379379
epoch_settings_storer: self.get_epoch_settings_store().await?,
380380
chain_observer: self.get_chain_observer().await?,
381-
certificate_chain_synchroniser: self.get_certificate_chain_synchroniser().await?,
381+
certificate_chain_synchronizer: self.get_certificate_chain_synchronizer().await?,
382382
signer_registerer: self.get_signer_registerer().await?,
383383
signer_synchronizer: self.get_signer_synchronizer().await?,
384384
signer_registration_round_opener: self.get_signer_registration_round_opener().await?,

mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use crate::dependency_injection::{DependenciesBuilder, DependenciesBuilderError,
1111
use crate::get_dependency;
1212
use crate::services::{
1313
BufferedCertifierService, CertificateChainSynchronizer, CertifierService,
14-
MithrilCertificateChainSynchroniserNoop, MithrilCertificateChainSynchronizer,
14+
MithrilCertificateChainSynchronizer, MithrilCertificateChainSynchronizerNoop,
1515
MithrilCertifierService, MithrilSignerRegistrationFollower, SignerSynchronizer,
1616
};
1717
use crate::{
@@ -85,10 +85,10 @@ impl DependenciesBuilder {
8585
get_dependency!(self.multi_signer)
8686
}
8787

88-
async fn build_certificate_chain_synchroniser(
88+
async fn build_certificate_chain_synchronizer(
8989
&mut self,
9090
) -> Result<Arc<dyn CertificateChainSynchronizer>> {
91-
let synchroniser: Arc<dyn CertificateChainSynchronizer> =
91+
let synchronizer: Arc<dyn CertificateChainSynchronizer> =
9292
if self.configuration.is_follower_aggregator() {
9393
let leader_aggregator_client = self.get_leader_aggregator_client().await?;
9494
let verifier = Arc::new(MithrilCertificateVerifier::new(
@@ -105,17 +105,17 @@ impl DependenciesBuilder {
105105
self.root_logger(),
106106
))
107107
} else {
108-
Arc::new(MithrilCertificateChainSynchroniserNoop)
108+
Arc::new(MithrilCertificateChainSynchronizerNoop)
109109
};
110110

111-
Ok(synchroniser)
111+
Ok(synchronizer)
112112
}
113113

114114
/// [CertificateChainSynchronizer] service
115-
pub async fn get_certificate_chain_synchroniser(
115+
pub async fn get_certificate_chain_synchronizer(
116116
&mut self,
117117
) -> Result<Arc<dyn CertificateChainSynchronizer>> {
118-
get_dependency!(self.certificate_chain_synchroniser)
118+
get_dependency!(self.certificate_chain_synchronizer)
119119
}
120120

121121
async fn build_certificate_verifier(&mut self) -> Result<Arc<dyn CertificateVerifier>> {

mithril-aggregator/src/dependency_injection/containers/serve.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ pub struct ServeCommandDependenciesContainer {
5858
/// Chain observer service.
5959
pub(crate) chain_observer: Arc<dyn ChainObserver>,
6060

61-
/// Certificate chain synchroniser service
62-
pub(crate) certificate_chain_synchroniser: Arc<dyn CertificateChainSynchronizer>,
61+
/// Certificate chain synchronizer service
62+
pub(crate) certificate_chain_synchronizer: Arc<dyn CertificateChainSynchronizer>,
6363

6464
/// Signer registerer service
6565
pub signer_registerer: Arc<dyn SignerRegisterer>,

mithril-aggregator/src/runtime/runner.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ pub trait AggregatorRunnerTrait: Sync + Send {
7272
/// Synchronize the follower aggregator signer registration.
7373
async fn synchronize_follower_aggregator_signer_registration(&self) -> StdResult<()>;
7474

75-
/// Synchronise the follower aggregator certificate chain
75+
/// Synchronize the follower aggregator certificate chain
7676
async fn synchronize_follower_aggregator_certificate_chain(
7777
&self,
7878
force_sync: bool,
@@ -522,7 +522,7 @@ impl AggregatorRunnerTrait for AggregatorRunner {
522522
">> synchronize_follower_aggregator_certificate_chain(force_sync:{force_sync})"
523523
);
524524
self.dependencies
525-
.certificate_chain_synchroniser
525+
.certificate_chain_synchronizer
526526
.synchronize_certificate_chain(force_sync)
527527
.await
528528
}

mithril-aggregator/src/services/aggregator_client.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ impl AggregatorHTTPClient {
293293
}
294294
}
295295

296-
async fn certificates_details(
296+
async fn certificate_details(
297297
&self,
298298
certificate_hash: &str,
299299
) -> Result<Option<CertificateMessage>, AggregatorClientError> {
@@ -323,7 +323,7 @@ impl AggregatorHTTPClient {
323323
async fn latest_genesis_certificate(
324324
&self,
325325
) -> Result<Option<CertificateMessage>, AggregatorClientError> {
326-
self.certificates_details("genesis").await
326+
self.certificate_details("genesis").await
327327
}
328328
}
329329

@@ -342,7 +342,7 @@ impl CertificateRetriever for AggregatorHTTPClient {
342342
certificate_hash: &str,
343343
) -> Result<Certificate, CertificateRetrieverError> {
344344
let message = self
345-
.certificates_details(certificate_hash)
345+
.certificate_details(certificate_hash)
346346
.await
347347
.with_context(|| {
348348
format!("Failed to retrieve certificate with hash: '{certificate_hash}'")
@@ -365,7 +365,7 @@ impl RemoteCertificateRetriever for AggregatorHTTPClient {
365365
None => Ok(None),
366366
Some(latest_certificate_list_item) => {
367367
let latest_certificate_message =
368-
self.certificates_details(&latest_certificate_list_item.hash).await?;
368+
self.certificate_details(&latest_certificate_list_item.hash).await?;
369369
latest_certificate_message.map(TryInto::try_into).transpose()
370370
}
371371
}
@@ -586,7 +586,7 @@ mod tests {
586586
then.status(200).body(json!(expected_message).to_string());
587587
});
588588

589-
let fetched_message = client.certificates_details(&expected_message.hash).await.unwrap();
589+
let fetched_message = client.certificate_details(&expected_message.hash).await.unwrap();
590590

591591
assert_eq!(Some(expected_message), fetched_message);
592592
}
@@ -612,7 +612,7 @@ mod tests {
612612
then.status(500).body("an error occurred");
613613
});
614614

615-
match client.certificates_details("whatever").await.unwrap_err() {
615+
match client.certificate_details("whatever").await.unwrap_err() {
616616
AggregatorClientError::RemoteServerTechnical(_) => (),
617617
e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."),
618618
};
@@ -628,7 +628,7 @@ mod tests {
628628
});
629629

630630
let error = client
631-
.certificates_details("whatever")
631+
.certificate_details("whatever")
632632
.await
633633
.expect_err("retrieve_epoch_settings should fail");
634634

Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
mod interface;
22
mod noop;
3-
mod synchroniser_service;
3+
mod synchronizer_service;
44

55
pub use interface::*;
66
pub use noop::*;
7-
pub use synchroniser_service::*;
7+
pub use synchronizer_service::*;

mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs renamed to mithril-aggregator/src/services/certificate_chain_synchronizer/noop.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ use mithril_common::StdResult;
33
use crate::services::CertificateChainSynchronizer;
44

55
/// A noop [CertificateChainSynchronizer] for leader aggregators
6-
pub struct MithrilCertificateChainSynchroniserNoop;
6+
pub struct MithrilCertificateChainSynchronizerNoop;
77

88
#[async_trait::async_trait]
9-
impl CertificateChainSynchronizer for MithrilCertificateChainSynchroniserNoop {
9+
impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizerNoop {
1010
async fn synchronize_certificate_chain(&self, _force: bool) -> StdResult<()> {
1111
Ok(())
1212
}

0 commit comments

Comments
 (0)