From cd3d8e2f05023880b97be7909beec018174a7326 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Fri, 4 Jul 2025 18:20:53 +0200 Subject: [PATCH 01/28] feat(aggregator): make leader url withstand trailing slash if the leader aggregator url was specified with a trailing slash, request to it would fails because joined url would have two slashes, i.e. `http://aggregator//route`. This align with the behavior of the aggregator client of the mithril-client lib. --- .../builder/enablers/misc.rs | 14 +++-- .../src/services/aggregator_client.rs | 54 ++++++++++++++----- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs b/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs index e4874a3131d..2fcc78b4f66 100644 --- a/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs +++ b/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs @@ -4,6 +4,8 @@ //! - group these enablers into more logical categories //! - redefine the actual categories so those miscellaneous enablers fit into them +use anyhow::{Context, anyhow}; +use reqwest::Url; use std::sync::Arc; use std::time::Duration; @@ -58,10 +60,16 @@ impl DependenciesBuilder { /// Builds an [AggregatorClient] pub async fn build_leader_aggregator_client(&mut self) -> Result> { - let leader_aggregator_endpoint = - self.configuration.leader_aggregator_endpoint().unwrap_or_default(); + let leader_aggregator_endpoint = self.configuration.leader_aggregator_endpoint().ok_or( + anyhow!("Leader Aggregator endpoint is mandatory for follower Aggregator"), + )?; + let aggregator_client = AggregatorHTTPClient::new( - leader_aggregator_endpoint, + Url::parse(&leader_aggregator_endpoint).with_context(|| { + format!( + "Failed to parse leader aggregator endpoint: '{leader_aggregator_endpoint}'" + ) + })?, None, self.get_api_version_provider().await?, Some(Duration::from_secs(30)), diff --git a/mithril-aggregator/src/services/aggregator_client.rs b/mithril-aggregator/src/services/aggregator_client.rs index 6e3b657226c..d485b9eea48 100644 --- a/mithril-aggregator/src/services/aggregator_client.rs +++ b/mithril-aggregator/src/services/aggregator_client.rs @@ -1,8 +1,9 @@ -use anyhow::anyhow; +use anyhow::{Context, anyhow}; use async_trait::async_trait; use mithril_common::messages::TryFromMessageAdapter; use reqwest::header::{self, HeaderValue}; -use reqwest::{self, Client, Proxy, RequestBuilder, Response, StatusCode}; +use reqwest::{self, Client, Proxy, RequestBuilder, Response, StatusCode, Url}; + use semver::Version; use slog::{Logger, debug, error, warn}; use std::{io, sync::Arc, time::Duration}; @@ -129,7 +130,7 @@ pub trait AggregatorClient: Sync + Send { /// AggregatorHTTPClient is a http client for an aggregator pub struct AggregatorHTTPClient { - aggregator_endpoint: String, + aggregator_endpoint: Url, relay_endpoint: Option, api_version_provider: Arc, timeout_duration: Option, @@ -139,7 +140,7 @@ pub struct AggregatorHTTPClient { impl AggregatorHTTPClient { /// AggregatorHTTPClient factory pub fn new( - aggregator_endpoint: String, + aggregator_endpoint: Url, relay_endpoint: Option, api_version_provider: Arc, timeout_duration: Option, @@ -147,6 +148,18 @@ impl AggregatorHTTPClient { ) -> Self { let logger = logger.new_with_component_name::(); debug!(logger, "New AggregatorHTTPClient created"); + + // Trailing slash is significant because url::join + // (https://docs.rs/url/latest/url/struct.Url.html#method.join) will remove + // the 'path' part of the url if it doesn't end with a trailing slash. + let aggregator_endpoint = if aggregator_endpoint.as_str().ends_with('/') { + aggregator_endpoint + } else { + let mut url = aggregator_endpoint.clone(); + url.set_path(&format!("{}/", aggregator_endpoint.path())); + url + }; + Self { aggregator_endpoint, relay_endpoint, @@ -156,6 +169,18 @@ impl AggregatorHTTPClient { } } + fn join_aggregator_endpoint(&self, endpoint: &str) -> Result { + self.aggregator_endpoint + .join(endpoint) + .with_context(|| { + format!( + "Invalid url when joining given endpoint, '{endpoint}', to aggregator url '{}'", + self.aggregator_endpoint + ) + }) + .map_err(AggregatorClientError::HTTPClientCreation) + } + fn prepare_http_client(&self) -> Result { let client = match &self.relay_endpoint { Some(relay_endpoint) => Client::builder() @@ -225,9 +250,9 @@ impl AggregatorClient for AggregatorHTTPClient { &self, ) -> Result, AggregatorClientError> { debug!(self.logger, "Retrieve epoch settings"); - let url = format!("{}/epoch-settings", self.aggregator_endpoint); + let url = self.join_aggregator_endpoint("epoch-settings")?; let response = self - .prepare_request_builder(self.prepare_http_client()?.get(url.clone())) + .prepare_request_builder(self.prepare_http_client()?.get(url)) .send() .await; @@ -288,6 +313,7 @@ pub(crate) mod dumb { mod tests { use http::response::Builder as HttpResponseBuilder; use httpmock::prelude::*; + use reqwest::IntoUrl; use serde_json::json; use mithril_common::api_version::DummyApiVersionDiscriminantSource; @@ -296,12 +322,12 @@ mod tests { use super::*; - fn setup_client>(server_url: U) -> AggregatorHTTPClient { + fn setup_client(server_url: U) -> AggregatorHTTPClient { let discriminant_source = DummyApiVersionDiscriminantSource::default(); let api_version_provider = APIVersionProvider::new(Arc::new(discriminant_source)); AggregatorHTTPClient::new( - server_url.into(), + server_url.into_url().unwrap(), None, Arc::new(api_version_provider), None, @@ -567,7 +593,7 @@ mod tests { let aggregator_version = "1.0.0"; let (logger, log_inspector) = TestLogger::memory(); let version_provider = version_provider_with_open_api_version(aggregator_version); - let mut client = setup_client("whatever"); + let mut client = setup_client("http://whatever"); client.api_version_provider = Arc::new(version_provider); client.logger = logger; let response = build_fake_response_with_header( @@ -594,7 +620,7 @@ mod tests { let version = "1.0.0"; let (logger, log_inspector) = TestLogger::memory(); let version_provider = version_provider_with_open_api_version(version); - let mut client = setup_client("whatever"); + let mut client = setup_client("http://whatever"); client.api_version_provider = Arc::new(version_provider); client.logger = logger; let response = build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, version); @@ -610,7 +636,7 @@ mod tests { let aggregator_version = "2.0.0"; let (logger, log_inspector) = TestLogger::memory(); let version_provider = version_provider_with_open_api_version(aggregator_version); - let mut client = setup_client("whatever"); + let mut client = setup_client("http://whatever"); client.api_version_provider = Arc::new(version_provider); client.logger = logger; let response = build_fake_response_with_header( @@ -631,7 +657,7 @@ mod tests { #[test] fn test_does_not_log_or_fail_when_header_is_missing() { let (logger, log_inspector) = TestLogger::memory(); - let mut client = setup_client("whatever"); + let mut client = setup_client("http://whatever"); client.logger = logger; let response = build_fake_response_with_header("NotMithrilAPIVersionHeader", "whatever"); @@ -644,7 +670,7 @@ mod tests { #[test] fn test_does_not_log_or_fail_when_header_is_not_a_version() { let (logger, log_inspector) = TestLogger::memory(); - let mut client = setup_client("whatever"); + let mut client = setup_client("http://whatever"); client.logger = logger; let response = build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, "not_a_version"); @@ -658,7 +684,7 @@ mod tests { fn test_logs_error_when_aggregator_version_cannot_be_computed() { let (logger, log_inspector) = TestLogger::memory(); let version_provider = version_provider_without_open_api_version(); - let mut client = setup_client("whatever"); + let mut client = setup_client("http://whatever"); client.api_version_provider = Arc::new(version_provider); client.logger = logger; let response = build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, "1.0.0"); From be30da54bb225fa76d5d6af3845ac86ec1f5899e Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Fri, 4 Jul 2025 20:15:40 +0200 Subject: [PATCH 02/28] refactor(aggregator): invert dependency between aggregator client and signer registration follower several benefits: - cleaner cut between signer registration business layer and its infrastructure - allow to use the concrete `AggregatorHTTPClient` type in dependency injection, allowing to implement multiple traits on it and still pass it to the multiple depenedencies that use that trait --- .../builder/enablers/misc.rs | 12 ++--- .../src/dependency_injection/builder/mod.rs | 4 +- .../src/services/aggregator_client.rs | 35 ++++++------ .../src/services/signer_registration/api.rs | 10 ++++ .../services/signer_registration/follower.rs | 53 +++++++------------ .../src/services/signer_registration/mod.rs | 9 ++-- 6 files changed, 61 insertions(+), 62 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs b/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs index 2fcc78b4f66..497f3262ecb 100644 --- a/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs +++ b/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs @@ -21,8 +21,8 @@ use crate::get_dependency; #[cfg(feature = "future_dmq")] use crate::services::SignatureConsumerDmq; use crate::services::{ - AggregatorClient, AggregatorHTTPClient, MessageService, MithrilMessageService, - SequentialSignatureProcessor, SignatureConsumer, SignatureConsumerNoop, SignatureProcessor, + AggregatorHTTPClient, MessageService, MithrilMessageService, SequentialSignatureProcessor, + SignatureConsumer, SignatureConsumerNoop, SignatureProcessor, }; impl DependenciesBuilder { async fn build_signed_entity_type_lock(&mut self) -> Result> { @@ -58,8 +58,8 @@ impl DependenciesBuilder { get_dependency!(self.message_service) } - /// Builds an [AggregatorClient] - pub async fn build_leader_aggregator_client(&mut self) -> Result> { + /// Builds an [AggregatorHTTPClient] + pub async fn build_leader_aggregator_client(&mut self) -> Result> { let leader_aggregator_endpoint = self.configuration.leader_aggregator_endpoint().ok_or( anyhow!("Leader Aggregator endpoint is mandatory for follower Aggregator"), )?; @@ -79,8 +79,8 @@ impl DependenciesBuilder { Ok(Arc::new(aggregator_client)) } - /// Returns a leader [AggregatorClient] - pub async fn get_leader_aggregator_client(&mut self) -> Result> { + /// Returns a leader [AggregatorHTTPClient] + pub async fn get_leader_aggregator_client(&mut self) -> Result> { get_dependency!(self.leader_aggregator_client) } diff --git a/mithril-aggregator/src/dependency_injection/builder/mod.rs b/mithril-aggregator/src/dependency_injection/builder/mod.rs index 9befa7d5472..1654ba8682b 100644 --- a/mithril-aggregator/src/dependency_injection/builder/mod.rs +++ b/mithril-aggregator/src/dependency_injection/builder/mod.rs @@ -56,7 +56,7 @@ use crate::{ file_uploaders::FileUploader, http_server::routes::router::{self, RouterConfig, RouterState}, services::{ - AggregatorClient, CertifierService, MessageService, MithrilSignerRegistrationFollower, + AggregatorHTTPClient, CertifierService, MessageService, MithrilSignerRegistrationFollower, ProverService, SignedEntityService, SignerSynchronizer, Snapshotter, StakeDistributionService, UpkeepService, }, @@ -273,7 +273,7 @@ pub struct DependenciesBuilder { pub metrics_service: Option>, /// Leader aggregator client - pub leader_aggregator_client: Option>, + pub leader_aggregator_client: Option>, /// Protocol parameters retriever pub protocol_parameters_retriever: Option>, diff --git a/mithril-aggregator/src/services/aggregator_client.rs b/mithril-aggregator/src/services/aggregator_client.rs index d485b9eea48..4ce4014fef7 100644 --- a/mithril-aggregator/src/services/aggregator_client.rs +++ b/mithril-aggregator/src/services/aggregator_client.rs @@ -10,7 +10,7 @@ use std::{io, sync::Arc, time::Duration}; use thiserror::Error; use mithril_common::{ - MITHRIL_AGGREGATOR_VERSION_HEADER, MITHRIL_API_VERSION_HEADER, StdError, + MITHRIL_AGGREGATOR_VERSION_HEADER, MITHRIL_API_VERSION_HEADER, StdError, StdResult, api_version::APIVersionProvider, entities::{ClientError, ServerError}, logging::LoggerExtensions, @@ -19,6 +19,7 @@ use mithril_common::{ use crate::entities::LeaderAggregatorEpochSettings; use crate::message_adapters::FromEpochSettingsAdapter; +use crate::services::LeaderAggregatorClient; const JSON_CONTENT_TYPE: HeaderValue = HeaderValue::from_static("application/json"); @@ -118,16 +119,6 @@ impl AggregatorClientError { } } -/// Trait for mocking and testing a `AggregatorClient` -#[cfg_attr(test, mockall::automock)] -#[async_trait] -pub trait AggregatorClient: Sync + Send { - /// Retrieves epoch settings from the aggregator - async fn retrieve_epoch_settings( - &self, - ) -> Result, AggregatorClientError>; -} - /// AggregatorHTTPClient is a http client for an aggregator pub struct AggregatorHTTPClient { aggregator_endpoint: Url, @@ -244,9 +235,9 @@ impl AggregatorHTTPClient { } } -#[async_trait] -impl AggregatorClient for AggregatorHTTPClient { - async fn retrieve_epoch_settings( +// Route specifics methods +impl AggregatorHTTPClient { + async fn epoch_settings( &self, ) -> Result, AggregatorClientError> { debug!(self.logger, "Retrieve epoch settings"); @@ -276,6 +267,14 @@ impl AggregatorClient for AggregatorHTTPClient { } } +#[async_trait] +impl LeaderAggregatorClient for AggregatorHTTPClient { + async fn retrieve_epoch_settings(&self) -> StdResult> { + let epoch_settings = self.epoch_settings().await?; + Ok(epoch_settings) + } +} + #[cfg(test)] pub(crate) mod dumb { use tokio::sync::RwLock; @@ -298,10 +297,10 @@ pub(crate) mod dumb { } #[async_trait] - impl AggregatorClient for DumbAggregatorClient { + impl LeaderAggregatorClient for DumbAggregatorClient { async fn retrieve_epoch_settings( &self, - ) -> Result, AggregatorClientError> { + ) -> StdResult> { let epoch_settings = self.epoch_settings.read().await.clone(); Ok(epoch_settings) @@ -396,7 +395,7 @@ mod tests { then.status(500).body("an error occurred"); }); - match client.retrieve_epoch_settings().await.unwrap_err() { + match client.epoch_settings().await.unwrap_err() { AggregatorClientError::RemoteServerTechnical(_) => (), e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), }; @@ -412,7 +411,7 @@ mod tests { }); let error = client - .retrieve_epoch_settings() + .epoch_settings() .await .expect_err("retrieve_epoch_settings should fail"); diff --git a/mithril-aggregator/src/services/signer_registration/api.rs b/mithril-aggregator/src/services/signer_registration/api.rs index f5b88959080..a2697455b81 100644 --- a/mithril-aggregator/src/services/signer_registration/api.rs +++ b/mithril-aggregator/src/services/signer_registration/api.rs @@ -5,6 +5,8 @@ use mithril_common::{ entities::{Epoch, Signer, SignerWithStake, StakeDistribution}, }; +use crate::entities::LeaderAggregatorEpochSettings; + use super::SignerRegistrationError; /// Represents the information needed to handle a signer registration round @@ -87,3 +89,11 @@ pub trait SignerRegistrationVerifier: Send + Sync { stake_distribution: &StakeDistribution, ) -> StdResult; } + +/// Define how data are retrieved from a leader aggregator +#[cfg_attr(test, mockall::automock)] +#[async_trait] +pub trait LeaderAggregatorClient: Sync + Send { + /// Retrieves epoch settings from the aggregator + async fn retrieve_epoch_settings(&self) -> StdResult>; +} diff --git a/mithril-aggregator/src/services/signer_registration/follower.rs b/mithril-aggregator/src/services/signer_registration/follower.rs index 6daaa821b14..78f4a0b6ce5 100644 --- a/mithril-aggregator/src/services/signer_registration/follower.rs +++ b/mithril-aggregator/src/services/signer_registration/follower.rs @@ -11,12 +11,11 @@ use mithril_persistence::store::StakeStorer; use crate::{ SignerRegistrationVerifier, VerificationKeyStorer, dependency_injection::EpochServiceWrapper, - services::AggregatorClient, }; use super::{ - SignerRecorder, SignerRegisterer, SignerRegistrationError, SignerRegistrationRound, - SignerRegistrationRoundOpener, SignerSynchronizer, + LeaderAggregatorClient, SignerRecorder, SignerRegisterer, SignerRegistrationError, + SignerRegistrationRound, SignerRegistrationRoundOpener, SignerSynchronizer, }; /// A [MithrilSignerRegistrationFollower] supports signer registrations in a follower aggregator @@ -34,7 +33,7 @@ pub struct MithrilSignerRegistrationFollower { signer_registration_verifier: Arc, /// Leader aggregator client - leader_aggregator_client: Arc, + leader_aggregator_client: Arc, /// Stake store stake_store: Arc, @@ -47,7 +46,7 @@ impl MithrilSignerRegistrationFollower { verification_key_store: Arc, signer_recorder: Arc, signer_registration_verifier: Arc, - leader_aggregator_client: Arc, + leader_aggregator_client: Arc, stake_store: Arc, ) -> Self { Self { @@ -178,36 +177,28 @@ impl SignerRegistrationRoundOpener for MithrilSignerRegistrationFollower { #[cfg(test)] mod tests { - use std::sync::Arc; - - use anyhow::anyhow; - use mithril_persistence::store::StakeStorer; - - use mithril_common::{ - entities::{Epoch, Signer, SignerWithStake}, - messages::{EpochSettingsMessage, SignerMessagePart, TryFromMessageAdapter}, - test_utils::MithrilFixtureBuilder, + use mithril_common::messages::{ + EpochSettingsMessage, SignerMessagePart, TryFromMessageAdapter, }; + use mithril_common::test_utils::MithrilFixtureBuilder; use crate::{ - MithrilSignerRegistrationFollower, SignerRecorder, SignerRegisterer, - SignerRegistrationRoundOpener, SignerRegistrationVerifier, VerificationKeyStorer, database::{repository::SignerRegistrationStore, test_helper::main_db_connection}, message_adapters::FromEpochSettingsAdapter, services::{ - AggregatorClient, AggregatorClientError, FakeEpochService, MockAggregatorClient, - MockSignerRecorder, MockSignerRegistrationVerifier, SignerSynchronizer, + FakeEpochService, MockLeaderAggregatorClient, MockSignerRecorder, + MockSignerRegistrationVerifier, }, tools::mocks::MockStakeStore, }; + use super::*; + use test_utils::*; mod test_utils { use tokio::sync::RwLock; - use crate::{dependency_injection::EpochServiceWrapper, services::FakeEpochService}; - use super::*; /// MithrilSignerRegistrationFollowerBuilder is a test builder for [MithrilSignerRegistrationFollower] @@ -215,7 +206,7 @@ mod tests { epoch_service: EpochServiceWrapper, signer_recorder: Arc, signer_registration_verifier: Arc, - leader_aggregator_client: Arc, + leader_aggregator_client: Arc, stake_store: Arc, verification_key_store: Arc, } @@ -226,7 +217,7 @@ mod tests { epoch_service: Arc::new(RwLock::new(FakeEpochService::without_data())), signer_recorder: Arc::new(MockSignerRecorder::new()), signer_registration_verifier: Arc::new(MockSignerRegistrationVerifier::new()), - leader_aggregator_client: Arc::new(MockAggregatorClient::new()), + leader_aggregator_client: Arc::new(MockLeaderAggregatorClient::new()), stake_store: Arc::new(MockStakeStore::new()), verification_key_store: Arc::new(SignerRegistrationStore::new( Arc::new(main_db_connection().unwrap()), @@ -263,7 +254,7 @@ mod tests { pub fn with_leader_aggregator_client( self, - leader_aggregator_client: Arc, + leader_aggregator_client: Arc, ) -> Self { Self { leader_aggregator_client, @@ -359,7 +350,7 @@ mod tests { Arc::new(signer_registration_verifier) }) .with_leader_aggregator_client({ - let mut aggregator_client = MockAggregatorClient::new(); + let mut aggregator_client = MockLeaderAggregatorClient::new(); aggregator_client .expect_retrieve_epoch_settings() .returning(move || Ok(Some(epoch_settings_message.clone()))) @@ -421,7 +412,7 @@ mod tests { Arc::new(signer_registration_verifier) }) .with_leader_aggregator_client({ - let mut aggregator_client = MockAggregatorClient::new(); + let mut aggregator_client = MockLeaderAggregatorClient::new(); aggregator_client .expect_retrieve_epoch_settings() .returning(move || Ok(Some(epoch_settings_message.clone()))) @@ -488,7 +479,7 @@ mod tests { Arc::new(signer_registration_verifier) }) .with_leader_aggregator_client({ - let mut aggregator_client = MockAggregatorClient::new(); + let mut aggregator_client = MockLeaderAggregatorClient::new(); aggregator_client .expect_retrieve_epoch_settings() .returning(move || Ok(Some(epoch_settings_message.clone()))) @@ -517,14 +508,10 @@ mod tests { async fn synchronize_all_signers_fails_if_fetching_epoch_settings_fails() { let signer_registration_follower = MithrilSignerRegistrationFollowerBuilder::default() .with_leader_aggregator_client({ - let mut aggregator_client = MockAggregatorClient::new(); + let mut aggregator_client = MockLeaderAggregatorClient::new(); aggregator_client .expect_retrieve_epoch_settings() - .returning(move || { - Err(AggregatorClientError::RemoteServerTechnical(anyhow!( - "an error" - ))) - }) + .returning(move || Err(anyhow!("an error"))) .times(1); Arc::new(aggregator_client) @@ -553,7 +540,7 @@ mod tests { .unwrap(); let signer_registration_follower = MithrilSignerRegistrationFollowerBuilder::default() .with_leader_aggregator_client({ - let mut aggregator_client = MockAggregatorClient::new(); + let mut aggregator_client = MockLeaderAggregatorClient::new(); aggregator_client .expect_retrieve_epoch_settings() .returning(move || Ok(Some(epoch_settings_message.clone()))) diff --git a/mithril-aggregator/src/services/signer_registration/mod.rs b/mithril-aggregator/src/services/signer_registration/mod.rs index ac7aa7aa442..8dfa1ac55c7 100644 --- a/mithril-aggregator/src/services/signer_registration/mod.rs +++ b/mithril-aggregator/src/services/signer_registration/mod.rs @@ -5,8 +5,8 @@ mod leader; mod verifier; pub use api::{ - SignerRecorder, SignerRegisterer, SignerRegistrationRound, SignerRegistrationRoundOpener, - SignerRegistrationVerifier, SignerSynchronizer, + LeaderAggregatorClient, SignerRecorder, SignerRegisterer, SignerRegistrationRound, + SignerRegistrationRoundOpener, SignerRegistrationVerifier, SignerSynchronizer, }; pub use error::SignerRegistrationError; pub use follower::MithrilSignerRegistrationFollower; @@ -14,4 +14,7 @@ pub use leader::MithrilSignerRegistrationLeader; pub use verifier::MithrilSignerRegistrationVerifier; #[cfg(test)] -pub use api::{MockSignerRecorder, MockSignerRegisterer, MockSignerRegistrationVerifier}; +pub use api::{ + MockLeaderAggregatorClient, MockSignerRecorder, MockSignerRegisterer, + MockSignerRegistrationVerifier, +}; From 71635790125294c7df53ee244e3df7fbc29bdacf Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Fri, 4 Jul 2025 16:42:07 +0200 Subject: [PATCH 03/28] refactor(aggregator-test): extract leader follower test http server to a module In integration tests --- .../leader_aggregator_http_server.rs | 48 +++++++++++++++++++ .../tests/test_extensions/mod.rs | 1 + .../tests/test_extensions/runtime_tester.rs | 40 ++-------------- 3 files changed, 53 insertions(+), 36 deletions(-) create mode 100644 mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs diff --git a/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs b/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs new file mode 100644 index 00000000000..695bc2660bc --- /dev/null +++ b/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs @@ -0,0 +1,48 @@ +use std::convert::Infallible; +use std::sync::Arc; + +use serde_json::json; +use warp::Filter; +use warp::http::StatusCode; + +use mithril_common::StdResult; +use mithril_common::entities::SignedEntityTypeDiscriminants; +use mithril_test_http_server::{TestHttpServer, test_http_server}; + +use crate::test_extensions::{AggregatorObserver, RuntimeTester}; + +pub struct LeaderAggregatorHttpServer {} + +impl LeaderAggregatorHttpServer { + pub fn spawn(runtime_tester: &RuntimeTester) -> StdResult { + let routes = warp::path("epoch-settings") + .and(with_observer(runtime_tester)) + .and_then(epoch_settings_handler); + + Ok(test_http_server(routes)) + } +} + +fn with_observer( + runtime_tester: &RuntimeTester, +) -> impl Filter,), Error = Infallible> + Clone + use<> { + let observer = runtime_tester.observer.clone(); + warp::any().map(move || observer.clone()) +} + +async fn epoch_settings_handler( + observer: Arc, +) -> Result { + let allowed_discriminants = SignedEntityTypeDiscriminants::all(); + let epoch_settings_message = observer.get_epoch_settings(allowed_discriminants).await; + match epoch_settings_message { + Ok(message) => Ok(Box::new(warp::reply::with_status( + warp::reply::json(&message), + StatusCode::OK, + ))), + Err(err) => Ok(Box::new(warp::reply::with_status( + warp::reply::json(&json!(err.to_string())), + StatusCode::INTERNAL_SERVER_ERROR, + ))), + } +} diff --git a/mithril-aggregator/tests/test_extensions/mod.rs b/mithril-aggregator/tests/test_extensions/mod.rs index 4ac0f630b04..7d90a61aa2f 100644 --- a/mithril-aggregator/tests/test_extensions/mod.rs +++ b/mithril-aggregator/tests/test_extensions/mod.rs @@ -8,6 +8,7 @@ pub mod runtime_tester; pub mod utilities; pub mod aggregator_observer; mod expected_certificate; +mod leader_aggregator_http_server; mod metrics_tester; pub use aggregator_observer::AggregatorObserver; diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index 49447ac5c2c..18cc1c8e196 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -1,13 +1,9 @@ use anyhow::{Context, anyhow}; use chrono::Utc; -use serde_json::json; use slog::Drain; use slog_scope::debug; -use std::convert::Infallible; use std::sync::Arc; use std::time::Duration; -use warp::Filter; -use warp::http::StatusCode; use mithril_aggregator::{ AggregatorRuntime, ConfigurationSource, DumbUploader, MetricsService, @@ -38,8 +34,9 @@ use mithril_common::{ }, }; use mithril_era::{EraMarker, EraReader, adapters::EraReaderDummyAdapter}; -use mithril_test_http_server::{TestHttpServer, test_http_server}; +use mithril_test_http_server::TestHttpServer; +use crate::test_extensions::leader_aggregator_http_server::LeaderAggregatorHttpServer; use crate::test_extensions::utilities::tx_hash; use crate::test_extensions::{AggregatorObserver, ExpectedCertificate, MetricsVerifier}; @@ -240,37 +237,8 @@ impl RuntimeTester { Ok(()) } - pub async fn expose_epoch_settings(&mut self) -> StdResult { - fn with_observer( - runtime_tester: &RuntimeTester, - ) -> impl Filter,), Error = Infallible> + Clone + use<> - { - let observer = runtime_tester.observer.clone(); - warp::any().map(move || observer.clone()) - } - - async fn epoch_settings_handler( - observer: Arc, - ) -> Result { - let allowed_discriminants = SignedEntityTypeDiscriminants::all(); - let epoch_settings_message = observer.get_epoch_settings(allowed_discriminants).await; - match epoch_settings_message { - Ok(message) => Ok(Box::new(warp::reply::with_status( - warp::reply::json(&message), - StatusCode::OK, - ))), - Err(err) => Ok(Box::new(warp::reply::with_status( - warp::reply::json(&json!(err.to_string())), - StatusCode::INTERNAL_SERVER_ERROR, - ))), - } - } - - let routes = warp::path("epoch-settings") - .and(with_observer(self)) - .and_then(epoch_settings_handler); - - Ok(test_http_server(routes)) + pub async fn expose_epoch_settings(&self) -> StdResult { + LeaderAggregatorHttpServer::spawn(self) } /// Increase the immutable file number of the simulated db, returns the new number. From a22eedc052be08338587d205bd46be86a04f2926 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Fri, 4 Jul 2025 18:44:42 +0200 Subject: [PATCH 04/28] refactor(aggregator-test): use axum test for leader aggregator http server Instead of warp based `mithril-test-http-server`, as this allow to write simpler, more readable, code. --- Cargo.lock | 2 + mithril-aggregator/Cargo.toml | 2 + .../tests/create_certificate_follower.rs | 5 +- .../test_extensions/aggregator_observer.rs | 18 +---- .../leader_aggregator_http_server.rs | 80 ++++++++++++------- .../tests/test_extensions/runtime_tester.rs | 5 +- 6 files changed, 62 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af75e05e994..eec4bbe8368 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3914,6 +3914,8 @@ version = "0.7.72" dependencies = [ "anyhow", "async-trait", + "axum", + "axum-test", "chrono", "clap", "config", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 5446f43aef0..c1457b1f2df 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -76,6 +76,8 @@ zstd = { version = "0.13.3", features = ["zstdmt"] } tikv-jemallocator = { version = "0.6.0", optional = true } [dev-dependencies] +axum = { version = "0.8.4", features = ["json"] } +axum-test = "17.3.0" criterion = { version = "0.6.0", features = ["html_reports", "async_tokio"] } http = "1.3.1" httpmock = "0.7.0" diff --git a/mithril-aggregator/tests/create_certificate_follower.rs b/mithril-aggregator/tests/create_certificate_follower.rs index 4148519e10a..548009cd272 100644 --- a/mithril-aggregator/tests/create_certificate_follower.rs +++ b/mithril-aggregator/tests/create_certificate_follower.rs @@ -109,7 +109,8 @@ async fn create_certificate_follower() { }; let mut leader_tester = RuntimeTester::build(start_time_point.clone(), leader_configuration.clone()).await; - let leader_aggregator_http_server = leader_tester.expose_epoch_settings().await.unwrap(); + let leader_aggregator_http_server = + leader_tester.spawn_leader_aggregator_http_server().await.unwrap(); let follower_configuration = ServeCommandConfiguration { data_stores_directory: get_test_dir("create_certificate_follower"), @@ -117,7 +118,7 @@ async fn create_certificate_follower() { "aggregator-integration", "create_certificate_follower", ), - leader_aggregator_endpoint: Some(leader_aggregator_http_server.url()), + leader_aggregator_endpoint: Some(leader_aggregator_http_server.url().to_string()), ..leader_configuration }; let mut follower_tester = RuntimeTester::build(start_time_point, follower_configuration).await; diff --git a/mithril-aggregator/tests/test_extensions/aggregator_observer.rs b/mithril-aggregator/tests/test_extensions/aggregator_observer.rs index a2b2a2980df..407f9687ee7 100644 --- a/mithril-aggregator/tests/test_extensions/aggregator_observer.rs +++ b/mithril-aggregator/tests/test_extensions/aggregator_observer.rs @@ -1,10 +1,10 @@ use anyhow::{Context, anyhow}; -use std::{collections::BTreeSet, sync::Arc}; +use std::sync::Arc; use mithril_aggregator::{ dependency_injection::{DependenciesBuilder, EpochServiceWrapper}, entities::OpenMessage, - services::{CertifierService, MessageService, SignedEntityService}, + services::{CertifierService, SignedEntityService}, }; use mithril_common::{ StdResult, @@ -12,7 +12,6 @@ use mithril_common::{ CardanoTransactionsSnapshot, Certificate, Epoch, SignedEntityType, SignedEntityTypeDiscriminants, TimePoint, }, - messages::EpochSettingsMessage, signable_builder::SignedEntity, }; use mithril_ticker::TickerService; @@ -23,7 +22,6 @@ pub struct AggregatorObserver { signed_entity_service: Arc, ticker_service: Arc, epoch_service: EpochServiceWrapper, - message_service: Arc, } impl AggregatorObserver { @@ -34,7 +32,6 @@ impl AggregatorObserver { signed_entity_service: deps_builder.get_signed_entity_service().await.unwrap(), ticker_service: deps_builder.get_ticker_service().await.unwrap(), epoch_service: deps_builder.get_epoch_service().await.unwrap(), - message_service: deps_builder.get_message_service().await.unwrap(), } } @@ -84,17 +81,6 @@ impl AggregatorObserver { .time_point_to_signed_entity(discriminant, &time_point) } - /// Get the current [EpochSettingsMessage] of the aggregator - pub async fn get_epoch_settings( - &self, - allowed_discriminants: BTreeSet, - ) -> StdResult { - self.message_service - .get_epoch_settings_message(allowed_discriminants) - .await - .with_context(|| "Querying the current epoch settings should not fail") - } - /// Get the last certificate produced by the aggregator pub async fn get_last_certificate(&self) -> StdResult { let certificate = self diff --git a/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs b/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs index 695bc2660bc..f128e539df2 100644 --- a/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs +++ b/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs @@ -1,48 +1,68 @@ -use std::convert::Infallible; use std::sync::Arc; -use serde_json::json; -use warp::Filter; -use warp::http::StatusCode; +use axum::{ + Json, Router, + extract::State, + http::StatusCode, + response::{IntoResponse, Response}, + routing::get, +}; +use axum_test::TestServer; +use reqwest::Url; -use mithril_common::StdResult; +use mithril_aggregator::services::MessageService; use mithril_common::entities::SignedEntityTypeDiscriminants; -use mithril_test_http_server::{TestHttpServer, test_http_server}; +use mithril_common::logging::LoggerExtensions; +use mithril_common::{StdError, StdResult}; -use crate::test_extensions::{AggregatorObserver, RuntimeTester}; +use crate::test_extensions::RuntimeTester; -pub struct LeaderAggregatorHttpServer {} +pub struct LeaderAggregatorHttpServer { + server: TestServer, + url: Url, +} impl LeaderAggregatorHttpServer { - pub fn spawn(runtime_tester: &RuntimeTester) -> StdResult { - let routes = warp::path("epoch-settings") - .and(with_observer(runtime_tester)) - .and_then(epoch_settings_handler); + pub fn spawn(runtime_tester: &RuntimeTester) -> StdResult { + let state = LeaderAggregatorRoutesState { + message_service: runtime_tester.dependencies.message_service.clone(), + logger: slog_scope::logger().new_with_component_name::(), + }; + let router = Router::new() + .route("/epoch-settings", get(epoch_settings)) + .with_state(state); + + let server = TestServer::builder().http_transport().build(router)?; + let url = server.server_address().unwrap(); + + Ok(Self { server, url }) + } - Ok(test_http_server(routes)) + pub fn url(&self) -> &Url { + &self.url } } -fn with_observer( - runtime_tester: &RuntimeTester, -) -> impl Filter,), Error = Infallible> + Clone + use<> { - let observer = runtime_tester.observer.clone(); - warp::any().map(move || observer.clone()) +#[derive(Clone)] +struct LeaderAggregatorRoutesState { + message_service: Arc, + logger: slog::Logger, } -async fn epoch_settings_handler( - observer: Arc, -) -> Result { +fn internal_server_error(err: StdError) -> impl IntoResponse { + (StatusCode::INTERNAL_SERVER_ERROR, Json(err.to_string())) +} + +async fn epoch_settings(state: State) -> Response { + slog::debug!(state.logger, "/epoch-settings"); let allowed_discriminants = SignedEntityTypeDiscriminants::all(); - let epoch_settings_message = observer.get_epoch_settings(allowed_discriminants).await; + let epoch_settings_message = state + .message_service + .get_epoch_settings_message(allowed_discriminants) + .await; + match epoch_settings_message { - Ok(message) => Ok(Box::new(warp::reply::with_status( - warp::reply::json(&message), - StatusCode::OK, - ))), - Err(err) => Ok(Box::new(warp::reply::with_status( - warp::reply::json(&json!(err.to_string())), - StatusCode::INTERNAL_SERVER_ERROR, - ))), + Ok(message) => (StatusCode::OK, Json(message)).into_response(), + Err(err) => internal_server_error(err).into_response(), } } diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index 18cc1c8e196..322873af477 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -34,7 +34,6 @@ use mithril_common::{ }, }; use mithril_era::{EraMarker, EraReader, adapters::EraReaderDummyAdapter}; -use mithril_test_http_server::TestHttpServer; use crate::test_extensions::leader_aggregator_http_server::LeaderAggregatorHttpServer; use crate::test_extensions::utilities::tx_hash; @@ -237,7 +236,9 @@ impl RuntimeTester { Ok(()) } - pub async fn expose_epoch_settings(&self) -> StdResult { + pub async fn spawn_leader_aggregator_http_server( + &self, + ) -> StdResult { LeaderAggregatorHttpServer::spawn(self) } From f31bbb8936b2fa42cb5efc187a7e6431c3579545 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 24 Jun 2025 10:22:06 +0200 Subject: [PATCH 05/28] feat(aggregator): scaffold `CertificateChainSynchronizer` --- .../interface.rs | 53 ++++++++++++ .../certificate_chain_synchroniser/mod.rs | 5 ++ .../synchroniser_service.rs | 84 +++++++++++++++++++ mithril-aggregator/src/services/mod.rs | 2 + 4 files changed, 144 insertions(+) create mode 100644 mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs create mode 100644 mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs create mode 100644 mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs new file mode 100644 index 00000000000..6e4fc6c5620 --- /dev/null +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs @@ -0,0 +1,53 @@ +use async_trait::async_trait; + +use mithril_common::StdResult; +use mithril_common::certificate_chain::CertificateRetriever; +#[cfg(test)] +use mithril_common::certificate_chain::CertificateRetrieverError; +use mithril_common::entities::Certificate; + +/// Define how to synchronize the certificate chain with a remote source +#[cfg_attr(test, mockall::automock)] +#[async_trait::async_trait] +pub trait CertificateChainSynchronizer: Send + Sync { + /// Synchronize the certificate chain with a remote source + /// + /// If `force` is true, the chain will always be synchronized, else it will only synchronize + /// if the remote source has started a new chain with a new Genesis. + async fn synchronize_certificate_chain(&self, force: bool) -> StdResult<()>; +} + +/// Define how to retrieve remote certificate details +#[async_trait] +pub trait RemoteCertificateRetriever: CertificateRetriever + Sync + Send { + /// Get latest certificate + async fn get_latest_certificate_details(&self) -> StdResult>; + + /// Get genesis certificate + async fn get_genesis_certificate_details(&self) -> StdResult>; +} + +// Note: we can't use mockall::automock here because RemoteCertificateRetriever have a supertrait +#[cfg(test)] +mockall::mock! { + pub(crate) RemoteCertificateRetriever {} + #[async_trait] + impl RemoteCertificateRetriever for RemoteCertificateRetriever { + async fn get_latest_certificate_details(&self) -> StdResult>; + async fn get_genesis_certificate_details(&self) -> StdResult>; + } + #[async_trait] + impl CertificateRetriever for RemoteCertificateRetriever { + async fn get_certificate_details(&self, certificate_hash: &str, ) -> Result; + } +} + +/// Define how to store the synchronized certificate and retrieve details about the actual local chain +#[cfg_attr(test, mockall::automock)] +#[async_trait] +pub trait SynchronizedCertificateStorer: Send + Sync { + /// Insert a Certificate in the database, if it already exists, it will be deleted before inserting + async fn insert_or_replace(&self, certificate: &Certificate) -> StdResult<()>; + /// Get the latest genesis Certificate + async fn get_latest_genesis(&self) -> StdResult>; +} diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs new file mode 100644 index 00000000000..d55885013c3 --- /dev/null +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs @@ -0,0 +1,5 @@ +mod interface; +mod synchroniser_service; + +pub use interface::*; +pub use synchroniser_service::*; diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs new file mode 100644 index 00000000000..28fff0e8876 --- /dev/null +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -0,0 +1,84 @@ +//! # Certificate chain synchronizer +//! +//! Behavior: +//! 1. Check force: +//! - If false, fetch the latest local genesis certificate in database +//! - If it's found, fetch the remote Genesis certificate +//! - If it's different from the local genesis, continue synchronization +//! - If it's the same, abort with an `Ok` +//! - If it's not found, continue synchronization +//! - If true, skip the remote Genesis certificate check and synchronize +//! 2. Fetch then validate the latest remote certificate +//! - if valid, store it in an in-memory FIFO list +//! - if invalid, abort with an `Err` +//! 3. Repeat step 2. with each parent of the certificate until the genesis certificate is reached +//! 4. Store the fetched certificates in the database, for each certificate: +//! - if it exists in the database, it is replaced +//! - if it doesn't exist, it is inserted +//! 5. End +//! + +use async_trait::async_trait; +use slog::Logger; +use std::sync::Arc; + +use mithril_common::StdResult; +use mithril_common::entities::Certificate; +use mithril_common::logging::LoggerExtensions; + +use super::{ + CertificateChainSynchronizer, RemoteCertificateRetriever, SynchronizedCertificateStorer, +}; + +/// Service that synchronizes the certificate chain with a remote aggregator +pub struct MithrilCertificateChainSynchronizer { + remote_certificate_retriever: Arc, + certificate_storer: Arc, + logger: Logger, +} + +impl MithrilCertificateChainSynchronizer { + /// Create a new `MithrilCertificateChainSynchronizer` instance + pub fn new( + remote_certificate_retriever: Arc, + certificate_storer: Arc, + logger: Logger, + ) -> Self { + Self { + remote_certificate_retriever, + certificate_storer, + logger: logger.new_with_component_name::(), + } + } + + async fn should_sync(&self, force: bool) -> StdResult { + Ok(force) + } + + async fn retrieve_remote_certificate_chain(&self) -> StdResult> { + Ok(Vec::new()) + } + + async fn store_certificate_chain(&self, certificate_chain: Vec) -> StdResult<()> { + Ok(()) + } +} + +#[async_trait] +impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { + async fn synchronize_certificate_chain(&self, force: bool) -> StdResult<()> { + if !self.should_sync(force).await? { + return Ok(()); + } + + let remote_certificate_chain = self.retrieve_remote_certificate_chain().await?; + self.store_certificate_chain(remote_certificate_chain).await?; + + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; +} diff --git a/mithril-aggregator/src/services/mod.rs b/mithril-aggregator/src/services/mod.rs index d4649849fde..307b7c062e1 100644 --- a/mithril-aggregator/src/services/mod.rs +++ b/mithril-aggregator/src/services/mod.rs @@ -11,6 +11,7 @@ mod aggregator_client; mod cardano_transactions_importer; +mod certificate_chain_synchroniser; mod certifier; mod epoch_service; mod message; @@ -27,6 +28,7 @@ mod usage_reporter; pub use aggregator_client::*; pub use cardano_transactions_importer::*; +pub use certificate_chain_synchroniser::*; pub use certifier::*; pub use epoch_service::*; pub use message::*; From d97c731918a0c0b6fcd5bcf834ec419a489b1b79 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 24 Jun 2025 19:39:22 +0200 Subject: [PATCH 06/28] feat(aggregator): impl logic that check if sync is needed --- .../synchroniser_service.rs | 182 +++++++++++++++++- 1 file changed, 180 insertions(+), 2 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 28fff0e8876..492bdeeb906 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -17,7 +17,7 @@ //! - if it doesn't exist, it is inserted //! 5. End //! - +use anyhow::anyhow; use async_trait::async_trait; use slog::Logger; use std::sync::Arc; @@ -52,7 +52,25 @@ impl MithrilCertificateChainSynchronizer { } async fn should_sync(&self, force: bool) -> StdResult { - Ok(force) + if force { + return Ok(true); + } + + match self.certificate_storer.get_latest_genesis().await? { + Some(local_genesis) => { + match self + .remote_certificate_retriever + .get_genesis_certificate_details() + .await? + { + Some(remote_genesis) => Ok(local_genesis != remote_genesis), + // The remote aggregator doesn't have a chain yet, we can't sync + None => Err(anyhow!("Remote aggregator doesn't have a chain yet")), + } + } + // No local genesis certificate found, always sync + None => Ok(true), + } } async fn retrieve_remote_certificate_chain(&self) -> StdResult> { @@ -80,5 +98,165 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { #[cfg(test)] mod tests { + use anyhow::anyhow; + + use mithril_common::test_utils::fake_data; + + use crate::services::{MockRemoteCertificateRetriever, MockSynchronizedCertificateStorer}; + use crate::test_tools::TestLogger; + use super::*; + + impl MithrilCertificateChainSynchronizer { + fn new_for_test( + remote_certificate_retriever_mock_config: impl FnOnce(&mut MockRemoteCertificateRetriever), + certificate_storer_mock_config: impl FnOnce(&mut MockSynchronizedCertificateStorer), + ) -> Self { + let mut remote_certificate_retriever_mock = MockRemoteCertificateRetriever::new(); + remote_certificate_retriever_mock_config(&mut remote_certificate_retriever_mock); + + let mut certificate_storer_mock = MockSynchronizedCertificateStorer::new(); + certificate_storer_mock_config(&mut certificate_storer_mock); + + Self { + remote_certificate_retriever: Arc::new(remote_certificate_retriever_mock), + certificate_storer: Arc::new(certificate_storer_mock), + logger: TestLogger::stdout(), + } + } + } + + macro_rules! mocked_synchroniser { + () => { + MithrilCertificateChainSynchronizer::new_for_test(|_| {}, |_| {}) + }; + (with_remote_genesis: $remote_genesis_result:expr) => { + MithrilCertificateChainSynchronizer::new_for_test( + move |retriever| { + retriever + .expect_get_genesis_certificate_details() + .return_once(move || $remote_genesis_result); + }, + |_| {}, + ) + }; + (with_local_genesis: $local_genesis_result:expr) => { + MithrilCertificateChainSynchronizer::new_for_test( + |_| {}, + move |storer| { + storer + .expect_get_latest_genesis() + .return_once(move || $local_genesis_result); + }, + ) + }; + (with_remote_genesis: $remote_genesis_result:expr, with_local_genesis: $local_genesis_result:expr) => { + MithrilCertificateChainSynchronizer::new_for_test( + move |retriever| { + retriever + .expect_get_genesis_certificate_details() + .return_once(move || $remote_genesis_result); + }, + move |storer| { + storer + .expect_get_latest_genesis() + .return_once(move || $local_genesis_result); + }, + ) + }; + } + + mod should_sync { + use super::*; + + #[tokio::test] + async fn should_sync_if_force_true() { + let synchroniser = mocked_synchroniser!(); + + let should_sync = synchroniser.should_sync(true).await.unwrap(); + assert!(should_sync); + } + + #[tokio::test] + async fn should_sync_if_force_true_without_checking_genesis_certificate() { + let synchroniser = mocked_synchroniser!(with_remote_genesis: Err(anyhow!( + "should not fetch genesis" + ))); + + let should_sync = synchroniser.should_sync(true).await.unwrap(); + assert!(should_sync); + } + + #[tokio::test] + async fn should_sync_if_false_and_no_local_genesis_certificate_found() { + let synchroniser = mocked_synchroniser!(with_local_genesis: Ok(None)); + + let should_sync = synchroniser.should_sync(false).await.unwrap(); + assert!(should_sync); + } + + #[tokio::test] + async fn should_abort_with_error_if_force_false_and_fails_to_retrieve_local_genesis() { + let synchroniser = mocked_synchroniser!(with_local_genesis: Err(anyhow!("failure"))); + synchroniser + .should_sync(false) + .await + .expect_err("Expected an error but was:"); + } + + #[tokio::test] + async fn should_abort_with_error_if_force_false_and_fails_to_retrieve_remote_genesis() { + let synchroniser = mocked_synchroniser!( + with_remote_genesis: Err(anyhow!("failure")), + with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) + ); + synchroniser + .should_sync(false) + .await + .expect_err("Expected an error but was:"); + } + + #[tokio::test] + async fn should_abort_with_error_if_force_false_and_remote_genesis_is_none() { + let synchroniser = mocked_synchroniser!( + with_remote_genesis: Ok(None), + with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) + ); + let error = synchroniser + .should_sync(false) + .await + .expect_err("Expected an error but was:"); + + assert!( + error + .to_string() + .contains("Remote aggregator doesn't have a chain yet"), + "Unexpected error:\n{error:?}" + ); + } + + #[tokio::test] + async fn should_sync_if_force_false_and_remote_genesis_dont_matches_local_genesis() { + let synchroniser = mocked_synchroniser!( + with_remote_genesis: Ok(Some(fake_data::genesis_certificate("remote_genesis"))), + with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) + ); + + let should_sync = synchroniser.should_sync(false).await.unwrap(); + assert!(should_sync); + } + + #[tokio::test] + async fn should_not_sync_if_force_false_and_remote_genesis_matches_local_genesis() { + let remote_genesis = fake_data::genesis_certificate("genesis"); + let local_genesis = remote_genesis.clone(); + let synchroniser = mocked_synchroniser!( + with_remote_genesis: Ok(Some(remote_genesis)), + with_local_genesis: Ok(Some(local_genesis)) + ); + + let should_sync = synchroniser.should_sync(false).await.unwrap(); + assert!(!should_sync); + } + } } From 053ba8cf5ddcb233122afc39d9124ade8e8aa811 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Thu, 26 Jun 2025 11:39:54 +0200 Subject: [PATCH 07/28] test(aggregator): rework synchroniser test tools & prepare remote chain retrieval tests --- .../interface.rs | 21 +- .../synchroniser_service.rs | 191 +++++++++++++----- 2 files changed, 143 insertions(+), 69 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs index 6e4fc6c5620..24f998fa1fc 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs @@ -1,9 +1,6 @@ use async_trait::async_trait; use mithril_common::StdResult; -use mithril_common::certificate_chain::CertificateRetriever; -#[cfg(test)] -use mithril_common::certificate_chain::CertificateRetrieverError; use mithril_common::entities::Certificate; /// Define how to synchronize the certificate chain with a remote source @@ -18,8 +15,9 @@ pub trait CertificateChainSynchronizer: Send + Sync { } /// Define how to retrieve remote certificate details +#[cfg_attr(test, mockall::automock)] #[async_trait] -pub trait RemoteCertificateRetriever: CertificateRetriever + Sync + Send { +pub trait RemoteCertificateRetriever: Sync + Send { /// Get latest certificate async fn get_latest_certificate_details(&self) -> StdResult>; @@ -27,21 +25,6 @@ pub trait RemoteCertificateRetriever: CertificateRetriever + Sync + Send { async fn get_genesis_certificate_details(&self) -> StdResult>; } -// Note: we can't use mockall::automock here because RemoteCertificateRetriever have a supertrait -#[cfg(test)] -mockall::mock! { - pub(crate) RemoteCertificateRetriever {} - #[async_trait] - impl RemoteCertificateRetriever for RemoteCertificateRetriever { - async fn get_latest_certificate_details(&self) -> StdResult>; - async fn get_genesis_certificate_details(&self) -> StdResult>; - } - #[async_trait] - impl CertificateRetriever for RemoteCertificateRetriever { - async fn get_certificate_details(&self, certificate_hash: &str, ) -> Result; - } -} - /// Define how to store the synchronized certificate and retrieve details about the actual local chain #[cfg_attr(test, mockall::automock)] #[async_trait] diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 492bdeeb906..cc060b0c8a6 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -23,6 +23,8 @@ use slog::Logger; use std::sync::Arc; use mithril_common::StdResult; +use mithril_common::certificate_chain::CertificateVerifier; +use mithril_common::crypto_helper::ProtocolGenesisVerifier; use mithril_common::entities::Certificate; use mithril_common::logging::LoggerExtensions; @@ -34,6 +36,8 @@ use super::{ pub struct MithrilCertificateChainSynchronizer { remote_certificate_retriever: Arc, certificate_storer: Arc, + certificate_verifier: Arc, + genesis_verifier: Arc, logger: Logger, } @@ -42,11 +46,15 @@ impl MithrilCertificateChainSynchronizer { pub fn new( remote_certificate_retriever: Arc, certificate_storer: Arc, + certificate_verifier: Arc, + genesis_verifier: Arc, logger: Logger, ) -> Self { Self { remote_certificate_retriever, certificate_storer, + certificate_verifier, + genesis_verifier, logger: logger.new_with_component_name::(), } } @@ -73,7 +81,10 @@ impl MithrilCertificateChainSynchronizer { } } - async fn retrieve_remote_certificate_chain(&self) -> StdResult> { + async fn retrieve_and_validate_remote_certificate_chain( + &self, + starting_point: Certificate, + ) -> StdResult> { Ok(Vec::new()) } @@ -89,7 +100,14 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { return Ok(()); } - let remote_certificate_chain = self.retrieve_remote_certificate_chain().await?; + let starting_point = self + .remote_certificate_retriever + .get_latest_certificate_details() + .await? + .ok_or(anyhow!("Remote aggregator doesn't have a chain yet"))?; + let remote_certificate_chain = self + .retrieve_and_validate_remote_certificate_chain(starting_point) + .await?; self.store_certificate_chain(remote_certificate_chain).await?; Ok(()) @@ -100,69 +118,113 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { mod tests { use anyhow::anyhow; - use mithril_common::test_utils::fake_data; + use mithril_common::crypto_helper::ProtocolGenesisVerificationKey; + use mithril_common::test_utils::{fake_data, fake_keys, mock_extensions::MockBuilder}; use crate::services::{MockRemoteCertificateRetriever, MockSynchronizedCertificateStorer}; use crate::test_tools::TestLogger; use super::*; + mockall::mock! { + CertificateVerifier {} + #[async_trait] + impl CertificateVerifier for CertificateVerifier { + async fn verify_genesis_certificate( + &self, + genesis_certificate: &Certificate, + genesis_verification_key: &ProtocolGenesisVerificationKey, + ) -> StdResult<()>; + + async fn verify_standard_certificate( + &self, + certificate: &Certificate, + previous_certificate: &Certificate, + ) -> StdResult<()>; + + async fn verify_certificate( + &self, + certificate: &Certificate, + genesis_verification_key: &ProtocolGenesisVerificationKey, + ) -> StdResult>; + + async fn verify_certificate_chain( + &self, + certificate: Certificate, + genesis_verification_key: &ProtocolGenesisVerificationKey, + ) -> StdResult<()>; + } + } + impl MithrilCertificateChainSynchronizer { - fn new_for_test( - remote_certificate_retriever_mock_config: impl FnOnce(&mut MockRemoteCertificateRetriever), - certificate_storer_mock_config: impl FnOnce(&mut MockSynchronizedCertificateStorer), - ) -> Self { - let mut remote_certificate_retriever_mock = MockRemoteCertificateRetriever::new(); - remote_certificate_retriever_mock_config(&mut remote_certificate_retriever_mock); - - let mut certificate_storer_mock = MockSynchronizedCertificateStorer::new(); - certificate_storer_mock_config(&mut certificate_storer_mock); - - Self { - remote_certificate_retriever: Arc::new(remote_certificate_retriever_mock), - certificate_storer: Arc::new(certificate_storer_mock), - logger: TestLogger::stdout(), - } + fn default_for_test() -> Self { + let genesis_verification_key = + fake_keys::genesis_verification_key()[0].try_into().unwrap(); + Self::new( + Arc::new(MockRemoteCertificateRetriever::new()), + Arc::new(MockSynchronizedCertificateStorer::new()), + Arc::new(MockCertificateVerifier::new()), + Arc::new(ProtocolGenesisVerifier::from_verification_key( + genesis_verification_key, + )), + TestLogger::stdout(), + ) } } macro_rules! mocked_synchroniser { - () => { - MithrilCertificateChainSynchronizer::new_for_test(|_| {}, |_| {}) - }; (with_remote_genesis: $remote_genesis_result:expr) => { - MithrilCertificateChainSynchronizer::new_for_test( - move |retriever| { - retriever - .expect_get_genesis_certificate_details() - .return_once(move || $remote_genesis_result); - }, - |_| {}, - ) + MithrilCertificateChainSynchronizer { + remote_certificate_retriever: + MockBuilder::::configure(|retriever| { + retriever + .expect_get_genesis_certificate_details() + .return_once(move || $remote_genesis_result); + }), + ..MithrilCertificateChainSynchronizer::default_for_test() + } }; (with_local_genesis: $local_genesis_result:expr) => { - MithrilCertificateChainSynchronizer::new_for_test( - |_| {}, - move |storer| { - storer - .expect_get_latest_genesis() - .return_once(move || $local_genesis_result); - }, - ) + MithrilCertificateChainSynchronizer { + certificate_storer: MockBuilder::::configure( + |storer| { + storer + .expect_get_latest_genesis() + .return_once(move || $local_genesis_result); + }, + ), + ..MithrilCertificateChainSynchronizer::default_for_test() + } }; (with_remote_genesis: $remote_genesis_result:expr, with_local_genesis: $local_genesis_result:expr) => { - MithrilCertificateChainSynchronizer::new_for_test( - move |retriever| { - retriever - .expect_get_genesis_certificate_details() - .return_once(move || $remote_genesis_result); - }, - move |storer| { - storer - .expect_get_latest_genesis() - .return_once(move || $local_genesis_result); - }, - ) + MithrilCertificateChainSynchronizer { + remote_certificate_retriever: + MockBuilder::::configure(|retriever| { + retriever + .expect_get_genesis_certificate_details() + .return_once(move || $remote_genesis_result); + }), + certificate_storer: MockBuilder::::configure( + |storer| { + storer + .expect_get_latest_genesis() + .return_once(move || $local_genesis_result); + }, + ), + ..MithrilCertificateChainSynchronizer::default_for_test() + } + }; + (with_verify_certificate_result: $verify_certificate_result:expr) => { + MithrilCertificateChainSynchronizer { + certificate_verifier: MockBuilder::::configure( + |verifier| { + verifier + .expect_verify_certificate() + .return_once(move |_, _| $verify_certificate_result); + }, + ), + ..MithrilCertificateChainSynchronizer::default_for_test() + } }; } @@ -171,7 +233,7 @@ mod tests { #[tokio::test] async fn should_sync_if_force_true() { - let synchroniser = mocked_synchroniser!(); + let synchroniser = MithrilCertificateChainSynchronizer::default_for_test(); let should_sync = synchroniser.should_sync(true).await.unwrap(); assert!(should_sync); @@ -259,4 +321,33 @@ mod tests { assert!(!should_sync); } } + + mod retrieve_validate_remote_certificate_chain { + use super::*; + use mithril_common::certificate_chain::{ + FakeCertificaterRetriever, MithrilCertificateVerifier, + }; + + fn fake_verifier(remote_certificate_chain: &[Certificate]) -> Arc { + let verifier = MithrilCertificateVerifier::new( + TestLogger::stdout(), + Arc::new(FakeCertificaterRetriever::from_certificates( + remote_certificate_chain, + )), + ); + Arc::new(verifier) + } + + #[tokio::test] + async fn succeed_if_the_remote_chain_only_contains_a_genesis_certificate() {} + + #[tokio::test] + async fn abort_with_error_if_a_certificate_is_invalid() {} + + #[tokio::test] + async fn abort_with_error_if_remote_retrieval_fails() {} + + #[tokio::test] + async fn succeed_with_a_valid_certificate_chain() {} + } } From 34b578bb981c2bdb5a227b07d9328a6acf2d7be5 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Thu, 26 Jun 2025 18:16:41 +0200 Subject: [PATCH 08/28] feat(aggregator): implement logic to retrieve/validate remote chain Nothing is stored yet, this will come afterward --- .../synchroniser_service.rs | 72 +++++++++++++++++-- 1 file changed, 66 insertions(+), 6 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index cc060b0c8a6..78b85b5df7a 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -85,7 +85,28 @@ impl MithrilCertificateChainSynchronizer { &self, starting_point: Certificate, ) -> StdResult> { - Ok(Vec::new()) + let mut validated_certificates = Vec::new(); + let mut certificate = starting_point; + + loop { + let parent_certificate = self + .certificate_verifier + .verify_certificate(&certificate, &self.genesis_verifier.to_verification_key()) + .await + .with_context( + || format!("Failed to verify certificate: `{}`", certificate.hash,), + )?; + validated_certificates.push(certificate); + + match parent_certificate { + None => break, + Some(parent) => { + certificate = parent; + } + } + } + + Ok(validated_certificates) } async fn store_certificate_chain(&self, certificate_chain: Vec) -> StdResult<()> { @@ -339,15 +360,54 @@ mod tests { } #[tokio::test] - async fn succeed_if_the_remote_chain_only_contains_a_genesis_certificate() {} + async fn succeed_if_the_remote_chain_only_contains_a_genesis_certificate() { + let chain = CertificateChainBuilder::new().with_total_certificates(1).build(); + let synchroniser = MithrilCertificateChainSynchronizer { + certificate_verifier: fake_verifier(&chain), + genesis_verifier: Arc::new(chain.genesis_verifier.clone()), + ..MithrilCertificateChainSynchronizer::default_for_test() + }; - #[tokio::test] - async fn abort_with_error_if_a_certificate_is_invalid() {} + let starting_point = chain[0].clone(); + let remote_certificate_chain = synchroniser + .retrieve_and_validate_remote_certificate_chain(starting_point) + .await + .unwrap(); + + assert_eq!(remote_certificate_chain, chain.certificates_chained); + } #[tokio::test] - async fn abort_with_error_if_remote_retrieval_fails() {} + async fn abort_with_error_if_a_certificate_is_invalid() { + let synchroniser = mocked_synchroniser!(with_verify_certificate_result: Err(anyhow!("invalid certificate"))); + + let starting_point = fake_data::certificate("certificate"); + synchroniser + .retrieve_and_validate_remote_certificate_chain(starting_point) + .await + .expect_err("Expected an error but was:"); + } #[tokio::test] - async fn succeed_with_a_valid_certificate_chain() {} + async fn succeed_with_a_valid_certificate_chain() { + let chain = CertificateChainBuilder::new() + .with_total_certificates(10) + .with_certificates_per_epoch(3) + .build(); + let synchroniser = MithrilCertificateChainSynchronizer { + certificate_verifier: fake_verifier(&chain), + genesis_verifier: Arc::new(chain.genesis_verifier.clone()), + ..MithrilCertificateChainSynchronizer::default_for_test() + }; + + let starting_point = chain[0].clone(); + let remote_certificate_chain = synchroniser + .retrieve_and_validate_remote_certificate_chain(starting_point.clone()) + .await + .unwrap(); + + let expected = chain.certificate_path_to_genesis(&starting_point.hash); + assert_eq!(remote_certificate_chain, expected); + } } } From ee585f7c72e4ff516dfd8e18bddbe6da11683874 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 1 Jul 2025 12:37:40 +0200 Subject: [PATCH 09/28] feat(aggregator): implement logic to store retrieved chain --- .../synchroniser_service.rs | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 78b85b5df7a..6dc86757158 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -110,6 +110,11 @@ impl MithrilCertificateChainSynchronizer { } async fn store_certificate_chain(&self, certificate_chain: Vec) -> StdResult<()> { + for certificate in certificate_chain { + self.certificate_storer + .insert_or_replace(&certificate) + .await?; + } Ok(()) } } @@ -410,4 +415,75 @@ mod tests { assert_eq!(remote_certificate_chain, expected); } } + + mod store_remote_certificate_chain { + use std::sync::RwLock; + + use super::*; + + #[derive(Default)] + struct DumbCertificateStorer { + certificates: RwLock>, + } + + impl DumbCertificateStorer { + fn stored_certificates(&self) -> Vec { + self.certificates.read().unwrap().clone() + } + } + + #[async_trait] + impl SynchronizedCertificateStorer for DumbCertificateStorer { + async fn insert_or_replace(&self, certificate: &Certificate) -> StdResult<()> { + let mut certificates = self.certificates.write().unwrap(); + certificates.push(certificate.clone()); + Ok(()) + } + + async fn get_latest_genesis(&self) -> StdResult> { + unimplemented!("not needed in store_remote_certificate_chain tests") + } + } + + #[tokio::test] + async fn do_store_given_certificates() { + let certificates_chain = vec![ + fake_data::genesis_certificate("genesis"), + fake_data::certificate("certificate1"), + fake_data::certificate("certificate2"), + ]; + let storer = Arc::new(DumbCertificateStorer::default()); + let synchroniser = MithrilCertificateChainSynchronizer { + certificate_storer: storer.clone(), + ..MithrilCertificateChainSynchronizer::default_for_test() + }; + + assert_eq!(Vec::::new(), storer.stored_certificates()); + + synchroniser + .store_certificate_chain(certificates_chain.clone()) + .await + .unwrap(); + + assert_eq!(certificates_chain, storer.stored_certificates()); + } + + #[tokio::test] + async fn fail_on_storer_error() { + let synchroniser = MithrilCertificateChainSynchronizer { + certificate_storer: MockBuilder::::configure( + |mock| { + mock.expect_insert_or_replace() + .return_once(move |_| Err(anyhow!("failure"))); + }, + ), + ..MithrilCertificateChainSynchronizer::default_for_test() + }; + + synchroniser + .store_certificate_chain(vec![fake_data::certificate("certificate")]) + .await + .unwrap_err(); + } + } } From 8be4f82bcafa87b622a7a6087a4e2f03bdaa7fe5 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 1 Jul 2025 12:45:32 +0200 Subject: [PATCH 10/28] feat(aggregator): add logs & error context to `CertificateChainSynchronizer` --- .../synchroniser_service.rs | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 6dc86757158..3ea3860e757 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -17,9 +17,9 @@ //! - if it doesn't exist, it is inserted //! 5. End //! -use anyhow::anyhow; +use anyhow::{Context, anyhow}; use async_trait::async_trait; -use slog::Logger; +use slog::{Logger, debug, info}; use std::sync::Arc; use mithril_common::StdResult; @@ -111,9 +111,7 @@ impl MithrilCertificateChainSynchronizer { async fn store_certificate_chain(&self, certificate_chain: Vec) -> StdResult<()> { for certificate in certificate_chain { - self.certificate_storer - .insert_or_replace(&certificate) - .await?; + self.certificate_storer.insert_or_replace(&certificate).await?; } Ok(()) } @@ -122,20 +120,34 @@ impl MithrilCertificateChainSynchronizer { #[async_trait] impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { async fn synchronize_certificate_chain(&self, force: bool) -> StdResult<()> { - if !self.should_sync(force).await? { + debug!(self.logger, ">> synchronize_certificate_chain"; "force" => force); + if !self.should_sync(force).await.with_context(|| { + format!("Failed to check if certificate chain should be sync (force: `{force}`)") + })? { return Ok(()); } + info!(self.logger, "Start synchronizing certificate chain"); let starting_point = self .remote_certificate_retriever .get_latest_certificate_details() .await? - .ok_or(anyhow!("Remote aggregator doesn't have a chain yet"))?; + .ok_or( + anyhow!("Remote aggregator doesn't have a chain yet") + .context("Failed to retrieve latest remote certificate details"), + )?; let remote_certificate_chain = self .retrieve_and_validate_remote_certificate_chain(starting_point) - .await?; - self.store_certificate_chain(remote_certificate_chain).await?; - + .await + .with_context(|| "Failed to retrieve and validate remote certificate chain")?; + self.store_certificate_chain(remote_certificate_chain) + .await + .with_context(|| "Failed to store remote retrieved certificate chain")?; + + info!( + self.logger, + "Certificate chain synchronized with remote source" + ); Ok(()) } } @@ -145,7 +157,9 @@ mod tests { use anyhow::anyhow; use mithril_common::crypto_helper::ProtocolGenesisVerificationKey; - use mithril_common::test_utils::{fake_data, fake_keys, mock_extensions::MockBuilder}; + use mithril_common::test_utils::{ + CertificateChainBuilder, fake_data, fake_keys, mock_extensions::MockBuilder, + }; use crate::services::{MockRemoteCertificateRetriever, MockSynchronizedCertificateStorer}; use crate::test_tools::TestLogger; @@ -349,11 +363,12 @@ mod tests { } mod retrieve_validate_remote_certificate_chain { - use super::*; use mithril_common::certificate_chain::{ FakeCertificaterRetriever, MithrilCertificateVerifier, }; + use super::*; + fn fake_verifier(remote_certificate_chain: &[Certificate]) -> Arc { let verifier = MithrilCertificateVerifier::new( TestLogger::stdout(), From 4ec737835c7e3430f8298c8cfadf6ad79cc2c14a Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 1 Jul 2025 17:33:17 +0200 Subject: [PATCH 11/28] refactor(aggregator): add a enum to make sync reason explicit in synchroniser --- .../synchroniser_service.rs | 130 +++++++++++------- 1 file changed, 81 insertions(+), 49 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 3ea3860e757..63391f4b98a 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -41,6 +41,25 @@ pub struct MithrilCertificateChainSynchronizer { logger: Logger, } +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum SyncStatus { + Forced, + NoLocalGenesis, + RemoteGenesisMatchesLocalGenesis, + RemoteGenesisDontMatchesLocalGenesis, +} + +impl SyncStatus { + fn should_sync(&self) -> bool { + match self { + SyncStatus::Forced => true, + SyncStatus::NoLocalGenesis => true, + SyncStatus::RemoteGenesisMatchesLocalGenesis => false, + SyncStatus::RemoteGenesisDontMatchesLocalGenesis => true, + } + } +} + impl MithrilCertificateChainSynchronizer { /// Create a new `MithrilCertificateChainSynchronizer` instance pub fn new( @@ -59,9 +78,9 @@ impl MithrilCertificateChainSynchronizer { } } - async fn should_sync(&self, force: bool) -> StdResult { + async fn check_sync_state(&self, force: bool) -> StdResult { if force { - return Ok(true); + return Ok(SyncStatus::Forced); } match self.certificate_storer.get_latest_genesis().await? { @@ -71,13 +90,15 @@ impl MithrilCertificateChainSynchronizer { .get_genesis_certificate_details() .await? { - Some(remote_genesis) => Ok(local_genesis != remote_genesis), + Some(remote_genesis) if (local_genesis == remote_genesis) => { + Ok(SyncStatus::RemoteGenesisMatchesLocalGenesis) + } + Some(_) => Ok(SyncStatus::RemoteGenesisDontMatchesLocalGenesis), // The remote aggregator doesn't have a chain yet, we can't sync None => Err(anyhow!("Remote aggregator doesn't have a chain yet")), } } - // No local genesis certificate found, always sync - None => Ok(true), + None => Ok(SyncStatus::NoLocalGenesis), } } @@ -121,12 +142,16 @@ impl MithrilCertificateChainSynchronizer { impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { async fn synchronize_certificate_chain(&self, force: bool) -> StdResult<()> { debug!(self.logger, ">> synchronize_certificate_chain"; "force" => force); - if !self.should_sync(force).await.with_context(|| { + + let sync_state = self.check_sync_state(force).await.with_context(|| { format!("Failed to check if certificate chain should be sync (force: `{force}`)") - })? { + })?; + if sync_state.should_sync() { + info!(self.logger, "Start synchronizing certificate chain"; "sync_state" => ?sync_state); + } else { + info!(self.logger, "No need to synchronize certificate chain"; "sync_state" => ?sync_state); return Ok(()); } - info!(self.logger, "Start synchronizing certificate chain"); let starting_point = self .remote_certificate_retriever @@ -268,40 +293,71 @@ mod tests { }; } - mod should_sync { + mod check_sync_state { use super::*; + #[test] + fn sync_state_should_sync() { + assert!(SyncStatus::Forced.should_sync()); + assert!(!SyncStatus::RemoteGenesisMatchesLocalGenesis.should_sync()); + assert!(SyncStatus::RemoteGenesisDontMatchesLocalGenesis.should_sync()); + assert!(SyncStatus::NoLocalGenesis.should_sync()); + } + #[tokio::test] - async fn should_sync_if_force_true() { + async fn state_when_force_true() { let synchroniser = MithrilCertificateChainSynchronizer::default_for_test(); - let should_sync = synchroniser.should_sync(true).await.unwrap(); - assert!(should_sync); + let sync_state = synchroniser.check_sync_state(true).await.unwrap(); + assert_eq!(SyncStatus::Forced, sync_state); } #[tokio::test] - async fn should_sync_if_force_true_without_checking_genesis_certificate() { - let synchroniser = mocked_synchroniser!(with_remote_genesis: Err(anyhow!( - "should not fetch genesis" - ))); + async fn state_when_force_false_and_no_local_genesis_certificate_found() { + let synchroniser = mocked_synchroniser!(with_local_genesis: Ok(None)); - let should_sync = synchroniser.should_sync(true).await.unwrap(); - assert!(should_sync); + let sync_state = synchroniser.check_sync_state(false).await.unwrap(); + assert_eq!(SyncStatus::NoLocalGenesis, sync_state); } #[tokio::test] - async fn should_sync_if_false_and_no_local_genesis_certificate_found() { - let synchroniser = mocked_synchroniser!(with_local_genesis: Ok(None)); + async fn state_when_force_false_and_remote_genesis_dont_matches_local_genesis() { + let synchroniser = mocked_synchroniser!( + with_remote_genesis: Ok(Some(fake_data::genesis_certificate("remote_genesis"))), + with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) + ); - let should_sync = synchroniser.should_sync(false).await.unwrap(); - assert!(should_sync); + let sync_state = synchroniser.check_sync_state(false).await.unwrap(); + assert_eq!(SyncStatus::RemoteGenesisDontMatchesLocalGenesis, sync_state); + } + + #[tokio::test] + async fn state_when_force_false_and_remote_genesis_matches_local_genesis() { + let remote_genesis = fake_data::genesis_certificate("genesis"); + let local_genesis = remote_genesis.clone(); + let synchroniser = mocked_synchroniser!( + with_remote_genesis: Ok(Some(remote_genesis)), + with_local_genesis: Ok(Some(local_genesis)) + ); + + let sync_state = synchroniser.check_sync_state(false).await.unwrap(); + assert_eq!(SyncStatus::RemoteGenesisMatchesLocalGenesis, sync_state); + } + + #[tokio::test] + async fn if_force_true_it_should_not_fetch_remote_genesis_certificate() { + let synchroniser = mocked_synchroniser!(with_remote_genesis: Err(anyhow!( + "should not fetch genesis" + ))); + + synchroniser.check_sync_state(true).await.unwrap(); } #[tokio::test] async fn should_abort_with_error_if_force_false_and_fails_to_retrieve_local_genesis() { let synchroniser = mocked_synchroniser!(with_local_genesis: Err(anyhow!("failure"))); synchroniser - .should_sync(false) + .check_sync_state(false) .await .expect_err("Expected an error but was:"); } @@ -313,7 +369,7 @@ mod tests { with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) ); synchroniser - .should_sync(false) + .check_sync_state(false) .await .expect_err("Expected an error but was:"); } @@ -325,7 +381,7 @@ mod tests { with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) ); let error = synchroniser - .should_sync(false) + .check_sync_state(false) .await .expect_err("Expected an error but was:"); @@ -336,30 +392,6 @@ mod tests { "Unexpected error:\n{error:?}" ); } - - #[tokio::test] - async fn should_sync_if_force_false_and_remote_genesis_dont_matches_local_genesis() { - let synchroniser = mocked_synchroniser!( - with_remote_genesis: Ok(Some(fake_data::genesis_certificate("remote_genesis"))), - with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) - ); - - let should_sync = synchroniser.should_sync(false).await.unwrap(); - assert!(should_sync); - } - - #[tokio::test] - async fn should_not_sync_if_force_false_and_remote_genesis_matches_local_genesis() { - let remote_genesis = fake_data::genesis_certificate("genesis"); - let local_genesis = remote_genesis.clone(); - let synchroniser = mocked_synchroniser!( - with_remote_genesis: Ok(Some(remote_genesis)), - with_local_genesis: Ok(Some(local_genesis)) - ); - - let should_sync = synchroniser.should_sync(false).await.unwrap(); - assert!(!should_sync); - } } mod retrieve_validate_remote_certificate_chain { From f48f558f1c64e6cb70fa2c86227536a315390708 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 1 Jul 2025 15:11:17 +0200 Subject: [PATCH 12/28] refactor(aggregator): make storage of sync certificates works on a batch This allow implementor to use transactions if needed. --- .../certificate_chain_synchroniser/interface.rs | 5 +++-- .../synchroniser_service.rs | 15 +++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs index 24f998fa1fc..b53e747502b 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs @@ -29,8 +29,9 @@ pub trait RemoteCertificateRetriever: Sync + Send { #[cfg_attr(test, mockall::automock)] #[async_trait] pub trait SynchronizedCertificateStorer: Send + Sync { - /// Insert a Certificate in the database, if it already exists, it will be deleted before inserting - async fn insert_or_replace(&self, certificate: &Certificate) -> StdResult<()>; + /// Insert a list of Certificates in the database, if some already exists, they will be deleted before inserting + async fn insert_or_replace_many(&self, certificates: Vec) -> StdResult<()>; + /// Get the latest genesis Certificate async fn get_latest_genesis(&self) -> StdResult>; } diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 63391f4b98a..60e642c61fa 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -131,9 +131,9 @@ impl MithrilCertificateChainSynchronizer { } async fn store_certificate_chain(&self, certificate_chain: Vec) -> StdResult<()> { - for certificate in certificate_chain { - self.certificate_storer.insert_or_replace(&certificate).await?; - } + self.certificate_storer + .insert_or_replace_many(certificate_chain) + .await?; Ok(()) } } @@ -481,9 +481,12 @@ mod tests { #[async_trait] impl SynchronizedCertificateStorer for DumbCertificateStorer { - async fn insert_or_replace(&self, certificate: &Certificate) -> StdResult<()> { + async fn insert_or_replace_many( + &self, + certificates_chain: Vec, + ) -> StdResult<()> { let mut certificates = self.certificates.write().unwrap(); - certificates.push(certificate.clone()); + *certificates = certificates_chain; Ok(()) } @@ -520,7 +523,7 @@ mod tests { let synchroniser = MithrilCertificateChainSynchronizer { certificate_storer: MockBuilder::::configure( |mock| { - mock.expect_insert_or_replace() + mock.expect_insert_or_replace_many() .return_once(move |_| Err(anyhow!("failure"))); }, ), From 7ccd88c5a47e11d095c6d1361a02875db501446f Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 1 Jul 2025 16:56:57 +0200 Subject: [PATCH 13/28] test(common): add `latest_certificate` to `CertificateChainFixture` --- .../test_utils/certificate_chain_builder.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 6bf1beea035..9e121a84538 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -132,6 +132,11 @@ impl CertificateChainFixture { &self.certificates_chained[self.certificates_chained.len() - 1] } + /// Return the latest certificate of this chain + pub fn latest_certificate(&self) -> &Certificate { + &self.certificates_chained[0] + } + /// Return a copy of the chain but with reversed order (from genesis to last) pub fn reversed_chain(&self) -> Vec { self.certificates_chained.iter().rev().cloned().collect() @@ -1065,6 +1070,22 @@ mod test { assert!(chain_with_multiple_certificates.genesis_certificate().is_genesis()); } + #[test] + fn get_latest_certificate() { + let chain_with_only_a_genesis = CertificateChainBuilder::new() + .with_total_certificates(1) + .build(); + assert!(chain_with_only_a_genesis.latest_certificate().is_genesis()); + + let chain_with_multiple_certificates = CertificateChainBuilder::new() + .with_total_certificates(10) + .build(); + assert_eq!( + chain_with_multiple_certificates.latest_certificate(), + chain_with_multiple_certificates.first().unwrap() + ); + } + #[test] fn path_to_genesis_from_a_chain_with_one_certificate_per_epoch() { let chain = CertificateChainBuilder::new() From 6a4c9c0eb763cf288cfa463339c851436e616372 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 1 Jul 2025 16:57:41 +0200 Subject: [PATCH 14/28] test(aggregator): add 'e2e' tests to synchroniser --- .../synchroniser_service.rs | 165 +++++++++++++----- 1 file changed, 121 insertions(+), 44 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 60e642c61fa..e16c78e1931 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -180,10 +180,15 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { #[cfg(test)] mod tests { use anyhow::anyhow; + use std::sync::RwLock; + use mithril_common::certificate_chain::{ + FakeCertificaterRetriever, MithrilCertificateVerifier, + }; use mithril_common::crypto_helper::ProtocolGenesisVerificationKey; use mithril_common::test_utils::{ - CertificateChainBuilder, fake_data, fake_keys, mock_extensions::MockBuilder, + CertificateChainBuilder, CertificateChainFixture, fake_data, fake_keys, + mock_extensions::MockBuilder, }; use crate::services::{MockRemoteCertificateRetriever, MockSynchronizedCertificateStorer}; @@ -293,6 +298,51 @@ mod tests { }; } + fn fake_verifier(remote_certificate_chain: &[Certificate]) -> Arc { + let verifier = MithrilCertificateVerifier::new( + TestLogger::stdout(), + Arc::new(FakeCertificaterRetriever::from_certificates( + remote_certificate_chain, + )), + ); + Arc::new(verifier) + } + + #[derive(Default)] + struct DumbCertificateStorer { + certificates: RwLock>, + genesis_certificate: Option, + } + + impl DumbCertificateStorer { + fn new(genesis: Certificate, already_stored: Vec) -> Self { + Self { + certificates: RwLock::new(already_stored), + genesis_certificate: Some(genesis), + } + } + + fn stored_certificates(&self) -> Vec { + self.certificates.read().unwrap().clone() + } + } + + #[async_trait] + impl SynchronizedCertificateStorer for DumbCertificateStorer { + async fn insert_or_replace_many( + &self, + certificates_chain: Vec, + ) -> StdResult<()> { + let mut certificates = self.certificates.write().unwrap(); + *certificates = certificates_chain; + Ok(()) + } + + async fn get_latest_genesis(&self) -> StdResult> { + Ok(self.genesis_certificate.clone()) + } + } + mod check_sync_state { use super::*; @@ -395,22 +445,8 @@ mod tests { } mod retrieve_validate_remote_certificate_chain { - use mithril_common::certificate_chain::{ - FakeCertificaterRetriever, MithrilCertificateVerifier, - }; - use super::*; - fn fake_verifier(remote_certificate_chain: &[Certificate]) -> Arc { - let verifier = MithrilCertificateVerifier::new( - TestLogger::stdout(), - Arc::new(FakeCertificaterRetriever::from_certificates( - remote_certificate_chain, - )), - ); - Arc::new(verifier) - } - #[tokio::test] async fn succeed_if_the_remote_chain_only_contains_a_genesis_certificate() { let chain = CertificateChainBuilder::new().with_total_certificates(1).build(); @@ -464,37 +500,8 @@ mod tests { } mod store_remote_certificate_chain { - use std::sync::RwLock; - use super::*; - #[derive(Default)] - struct DumbCertificateStorer { - certificates: RwLock>, - } - - impl DumbCertificateStorer { - fn stored_certificates(&self) -> Vec { - self.certificates.read().unwrap().clone() - } - } - - #[async_trait] - impl SynchronizedCertificateStorer for DumbCertificateStorer { - async fn insert_or_replace_many( - &self, - certificates_chain: Vec, - ) -> StdResult<()> { - let mut certificates = self.certificates.write().unwrap(); - *certificates = certificates_chain; - Ok(()) - } - - async fn get_latest_genesis(&self) -> StdResult> { - unimplemented!("not needed in store_remote_certificate_chain tests") - } - } - #[tokio::test] async fn do_store_given_certificates() { let certificates_chain = vec![ @@ -536,4 +543,74 @@ mod tests { .unwrap_err(); } } + + mod synchronize_certificate_chain { + use super::*; + + fn build_synchroniser( + remote_chain: &CertificateChainFixture, + storer: Arc, + ) -> MithrilCertificateChainSynchronizer { + MithrilCertificateChainSynchronizer { + certificate_storer: storer.clone(), + remote_certificate_retriever: + MockBuilder::::configure(|mock| { + let genesis = remote_chain.genesis_certificate().clone(); + mock.expect_get_genesis_certificate_details() + .return_once(move || Ok(Some(genesis))); + let latest = remote_chain.latest_certificate().clone(); + mock.expect_get_latest_certificate_details() + .return_once(move || Ok(Some(latest))); + }), + certificate_verifier: fake_verifier(remote_chain), + ..MithrilCertificateChainSynchronizer::default_for_test() + } + } + + #[tokio::test] + async fn store_all() { + let remote_chain = CertificateChainBuilder::default() + .with_certificates_per_epoch(3) + .with_total_certificates(8) + .build(); + let storer = Arc::new(DumbCertificateStorer::default()); + let synchroniser = build_synchroniser(&remote_chain, storer.clone()); + + // Will sync even if force is false + synchroniser.synchronize_certificate_chain(false).await.unwrap(); + + assert_eq!( + remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash), + storer.stored_certificates() + ); + } + + #[tokio::test] + async fn store_partial() { + let remote_chain = CertificateChainBuilder::default() + .with_certificates_per_epoch(1) + .with_total_certificates(8) + .build(); + let existing_certificates = + remote_chain.certificate_path_to_genesis(&remote_chain[5].hash); + let storer = Arc::new(DumbCertificateStorer::new( + remote_chain.genesis_certificate().clone(), + existing_certificates.clone(), + )); + let synchroniser = build_synchroniser(&remote_chain, storer.clone()); + + // Force false - won't sync + synchroniser.synchronize_certificate_chain(false).await.unwrap(); + + assert_eq!(&existing_certificates, &storer.stored_certificates()); + + // Force true - will sync + synchroniser.synchronize_certificate_chain(true).await.unwrap(); + + assert_eq!( + remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash), + storer.stored_certificates() + ); + } + } } From b855ab38fd38cb2ad093e2c53d5dfa772984f229 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 1 Jul 2025 19:31:11 +0200 Subject: [PATCH 15/28] feat(aggregator): add `RemoteCertificateRetriever` to `AggregatorHTTPClient` --- .../src/services/aggregator_client.rs | 312 +++++++++++++++++- 1 file changed, 308 insertions(+), 4 deletions(-) diff --git a/mithril-aggregator/src/services/aggregator_client.rs b/mithril-aggregator/src/services/aggregator_client.rs index 4ce4014fef7..43443990623 100644 --- a/mithril-aggregator/src/services/aggregator_client.rs +++ b/mithril-aggregator/src/services/aggregator_client.rs @@ -1,6 +1,5 @@ use anyhow::{Context, anyhow}; use async_trait::async_trait; -use mithril_common::messages::TryFromMessageAdapter; use reqwest::header::{self, HeaderValue}; use reqwest::{self, Client, Proxy, RequestBuilder, Response, StatusCode, Url}; @@ -12,14 +11,16 @@ use thiserror::Error; use mithril_common::{ MITHRIL_AGGREGATOR_VERSION_HEADER, MITHRIL_API_VERSION_HEADER, StdError, StdResult, api_version::APIVersionProvider, - entities::{ClientError, ServerError}, + entities::{Certificate, ClientError, ServerError}, logging::LoggerExtensions, - messages::EpochSettingsMessage, + messages::{ + CertificateListMessage, CertificateMessage, EpochSettingsMessage, TryFromMessageAdapter, + }, }; use crate::entities::LeaderAggregatorEpochSettings; use crate::message_adapters::FromEpochSettingsAdapter; -use crate::services::LeaderAggregatorClient; +use crate::services::{LeaderAggregatorClient, RemoteCertificateRetriever}; const JSON_CONTENT_TYPE: HeaderValue = HeaderValue::from_static("application/json"); @@ -265,6 +266,64 @@ impl AggregatorHTTPClient { Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), } } + + async fn latest_certificates_list( + &self, + ) -> Result { + debug!(self.logger, "Retrieve latest certificates list"); + let url = self.join_aggregator_endpoint("certificates")?; + let response = self + .prepare_request_builder(self.prepare_http_client()?.get(url)) + .send() + .await; + + match response { + Ok(response) => match response.status() { + StatusCode::OK => { + self.warn_if_api_version_mismatch(&response); + match response.json::().await { + Ok(message) => Ok(message), + Err(err) => Err(AggregatorClientError::JsonParseFailed(anyhow!(err))), + } + } + _ => Err(AggregatorClientError::from_response(response).await), + }, + Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), + } + } + + async fn certificates_details( + &self, + certificate_hash: &str, + ) -> Result, AggregatorClientError> { + debug!(self.logger, "Retrieve certificate details"; "certificate_hash" => %certificate_hash); + let url = self.join_aggregator_endpoint(&format!("certificate/{certificate_hash}"))?; + let response = self + .prepare_request_builder(self.prepare_http_client()?.get(url)) + .send() + .await; + + match response { + Ok(response) => match response.status() { + StatusCode::OK => { + self.warn_if_api_version_mismatch(&response); + match response.json::().await { + Ok(message) => Ok(Some(message)), + Err(err) => Err(AggregatorClientError::JsonParseFailed(anyhow!(err))), + } + } + StatusCode::NOT_FOUND => Ok(None), + _ => Err(AggregatorClientError::from_response(response).await), + }, + Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), + } + } + + async fn latest_genesis_certificate( + &self, + ) -> Result, AggregatorClientError> { + self.certificates_details("genesis").await + } } #[async_trait] @@ -275,6 +334,29 @@ impl LeaderAggregatorClient for AggregatorHTTPClient { } } +#[async_trait] +impl RemoteCertificateRetriever for AggregatorHTTPClient { + async fn get_latest_certificate_details(&self) -> StdResult> { + let latest_certificates_list = self.latest_certificates_list().await?; + + match latest_certificates_list.first() { + None => Ok(None), + Some(latest_certificate_list_item) => { + let latest_certificate_message = + self.certificates_details(&latest_certificate_list_item.hash).await?; + latest_certificate_message.map(TryInto::try_into).transpose() + } + } + } + + async fn get_genesis_certificate_details(&self) -> StdResult> { + match self.latest_genesis_certificate().await? { + Some(message) => Ok(Some(message.try_into()?)), + None => Ok(None), + } + } +} + #[cfg(test)] pub(crate) mod dumb { use tokio::sync::RwLock; @@ -316,6 +398,7 @@ mod tests { use serde_json::json; use mithril_common::api_version::DummyApiVersionDiscriminantSource; + use mithril_common::messages::CertificateListItemMessage; use crate::test_tools::TestLogger; @@ -421,6 +504,178 @@ mod tests { ); } + #[tokio::test] + async fn test_latest_certificates_list_ok_200() { + let (server, client) = setup_server_and_client(); + let expected_list = vec![ + CertificateListItemMessage::dummy(), + CertificateListItemMessage::dummy(), + ]; + let _server_mock = server.mock(|when, then| { + when.path("/certificates"); + then.status(200).body(json!(expected_list).to_string()); + }); + + let fetched_list = client.latest_certificates_list().await.unwrap(); + + assert_eq!(expected_list, fetched_list); + } + + #[tokio::test] + async fn test_latest_certificates_list_ko_500() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/certificates"); + then.status(500).body("an error occurred"); + }); + + match client.latest_certificates_list().await.unwrap_err() { + AggregatorClientError::RemoteServerTechnical(_) => (), + e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), + }; + } + + #[tokio::test] + async fn test_latest_certificates_list_timeout() { + let (server, mut client) = setup_server_and_client(); + client.timeout_duration = Some(Duration::from_millis(10)); + let _server_mock = server.mock(|when, then| { + when.path("/certificates"); + then.delay(Duration::from_millis(100)); + }); + + let error = client + .latest_certificates_list() + .await + .expect_err("retrieve_epoch_settings should fail"); + + assert!( + matches!(error, AggregatorClientError::RemoteServerUnreachable(_)), + "unexpected error type: {error:?}" + ); + } + + #[tokio::test] + async fn test_certificates_details_ok_200() { + let (server, client) = setup_server_and_client(); + let expected_message = CertificateMessage::dummy(); + let _server_mock = server.mock(|when, then| { + when.path(format!("/certificate/{}", expected_message.hash)); + then.status(200).body(json!(expected_message).to_string()); + }); + + let fetched_message = client.certificates_details(&expected_message.hash).await.unwrap(); + + assert_eq!(Some(expected_message), fetched_message); + } + + #[tokio::test] + async fn test_certificates_details_ok_404() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/not-found"); + then.status(404); + }); + + let fetched_message = client.latest_genesis_certificate().await.unwrap(); + + assert_eq!(None, fetched_message); + } + + #[tokio::test] + async fn test_certificates_details_ko_500() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/whatever"); + then.status(500).body("an error occurred"); + }); + + match client.certificates_details("whatever").await.unwrap_err() { + AggregatorClientError::RemoteServerTechnical(_) => (), + e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), + }; + } + + #[tokio::test] + async fn test_certificates_details_timeout() { + let (server, mut client) = setup_server_and_client(); + client.timeout_duration = Some(Duration::from_millis(10)); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/whatever"); + then.delay(Duration::from_millis(100)); + }); + + let error = client + .certificates_details("whatever") + .await + .expect_err("retrieve_epoch_settings should fail"); + + assert!( + matches!(error, AggregatorClientError::RemoteServerUnreachable(_)), + "unexpected error type: {error:?}" + ); + } + + #[tokio::test] + async fn test_latest_genesis_ok_200() { + let (server, client) = setup_server_and_client(); + let genesis_message = CertificateMessage::dummy(); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/genesis"); + then.status(200).body(json!(genesis_message).to_string()); + }); + + let fetched = client.latest_genesis_certificate().await.unwrap(); + + assert_eq!(Some(genesis_message), fetched); + } + + #[tokio::test] + async fn test_latest_genesis_ok_404() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/genesis"); + then.status(404); + }); + + let fetched = client.latest_genesis_certificate().await.unwrap(); + + assert_eq!(None, fetched); + } + + #[tokio::test] + async fn test_latest_genesis_ko_500() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/genesis"); + then.status(500).body("an error occurred"); + }); + + let error = client.latest_genesis_certificate().await.unwrap_err(); + + assert!( + matches!(error, AggregatorClientError::RemoteServerTechnical(_)), + "Expected Aggregator::RemoteServerTechnical error, got {error:?}" + ); + } + + #[tokio::test] + async fn test_latest_genesis_timeout() { + let (server, mut client) = setup_server_and_client(); + client.timeout_duration = Some(Duration::from_millis(10)); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/genesis"); + then.delay(Duration::from_millis(100)); + }); + + let error = client.latest_genesis_certificate().await.unwrap_err(); + + assert!( + matches!(error, AggregatorClientError::RemoteServerUnreachable(_)), + "unexpected error type: {error:?}" + ); + } + #[tokio::test] async fn test_4xx_errors_are_handled_as_remote_server_logical() { let response = build_text_response(StatusCode::BAD_REQUEST, "error text"); @@ -724,4 +979,53 @@ mod tests { ); } } + + mod remote_certificate_retriever { + use mithril_common::test_utils::fake_data; + + use super::*; + + #[tokio::test] + async fn test_get_latest_certificate_details() { + let (server, client) = setup_server_and_client(); + let expected_certificate = fake_data::certificate("expected"); + let latest_message: CertificateMessage = + expected_certificate.clone().try_into().unwrap(); + let latest_certificates = vec![ + CertificateListItemMessage { + hash: expected_certificate.hash.clone(), + ..CertificateListItemMessage::dummy() + }, + CertificateListItemMessage::dummy(), + CertificateListItemMessage::dummy(), + ]; + let _server_mock = server.mock(|when, then| { + when.path("/certificates"); + then.status(200).body(json!(latest_certificates).to_string()); + }); + let _server_mock = server.mock(|when, then| { + when.path(format!("/certificate/{}", latest_message.hash)); + then.status(200).body(json!(latest_message).to_string()); + }); + + let fetched_certificate = client.get_latest_certificate_details().await.unwrap(); + + assert_eq!(Some(expected_certificate), fetched_certificate); + } + + #[tokio::test] + async fn test_get_latest_genesis_certificate() { + let (server, client) = setup_server_and_client(); + let genesis_message = CertificateMessage::dummy(); + let expected_genesis: Certificate = genesis_message.clone().try_into().unwrap(); + let _server_mock = server.mock(|when, then| { + when.path("/certificate/genesis"); + then.status(200).body(json!(genesis_message).to_string()); + }); + + let fetched = client.get_genesis_certificate_details().await.unwrap(); + + assert_eq!(Some(expected_genesis), fetched); + } + } } From d63b82b00ad2ad162dc7b4307ae0b065beff3ec4 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Thu, 3 Jul 2025 17:53:27 +0200 Subject: [PATCH 16/28] feat(aggregator): ensure synced data is stored from genesis to latest This is critical since rows are returned from the database by insertion order reversed. --- .../synchroniser_service.rs | 80 ++++++++++++++++--- 1 file changed, 67 insertions(+), 13 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index e16c78e1931..677ac8e81a7 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -12,7 +12,7 @@ //! - if valid, store it in an in-memory FIFO list //! - if invalid, abort with an `Err` //! 3. Repeat step 2. with each parent of the certificate until the genesis certificate is reached -//! 4. Store the fetched certificates in the database, for each certificate: +//! 4. Store the fetched certificates in the database, from genesis to latest, for each certificate: //! - if it exists in the database, it is replaced //! - if it doesn't exist, it is inserted //! 5. End @@ -20,6 +20,7 @@ use anyhow::{Context, anyhow}; use async_trait::async_trait; use slog::{Logger, debug, info}; +use std::collections::VecDeque; use std::sync::Arc; use mithril_common::StdResult; @@ -106,7 +107,9 @@ impl MithrilCertificateChainSynchronizer { &self, starting_point: Certificate, ) -> StdResult> { - let mut validated_certificates = Vec::new(); + // IMPORTANT: Order matters, certificates must be ordered from genesis to latest + // (fetched database data is returned from last inserted to oldest) + let mut validated_certificates = VecDeque::new(); let mut certificate = starting_point; loop { @@ -117,7 +120,7 @@ impl MithrilCertificateChainSynchronizer { .with_context( || format!("Failed to verify certificate: `{}`", certificate.hash,), )?; - validated_certificates.push(certificate); + validated_certificates.push_front(certificate); match parent_certificate { None => break, @@ -127,7 +130,7 @@ impl MithrilCertificateChainSynchronizer { } } - Ok(validated_certificates) + Ok(validated_certificates.into()) } async fn store_certificate_chain(&self, certificate_chain: Vec) -> StdResult<()> { @@ -445,6 +448,8 @@ mod tests { } mod retrieve_validate_remote_certificate_chain { + use mockall::predicate::{always, eq}; + use super::*; #[tokio::test] @@ -494,9 +499,58 @@ mod tests { .await .unwrap(); - let expected = chain.certificate_path_to_genesis(&starting_point.hash); + let mut expected = chain.certificate_path_to_genesis(&starting_point.hash); + expected.reverse(); assert_eq!(remote_certificate_chain, expected); } + + #[tokio::test] + async fn return_chain_ordered_from_genesis_to_latest() { + let base_certificate = fake_data::certificate("whatever"); + let chain = vec![ + fake_data::genesis_certificate("genesis"), + Certificate { + hash: "hash1".to_string(), + previous_hash: "genesis".to_string(), + ..base_certificate.clone() + }, + Certificate { + hash: "hash2".to_string(), + previous_hash: "hash1".to_string(), + ..base_certificate + }, + ]; + let synchroniser = MithrilCertificateChainSynchronizer { + certificate_verifier: MockBuilder::::configure(|mock| { + let cert_1 = chain[1].clone(); + mock.expect_verify_certificate() + .with(eq(chain[2].clone()), always()) + .return_once(move |_, _| Ok(Some(cert_1))); + let genesis = chain[0].clone(); + mock.expect_verify_certificate() + .with(eq(chain[1].clone()), always()) + .return_once(move |_, _| Ok(Some(genesis))); + mock.expect_verify_certificate() + .with(eq(chain[0].clone()), always()) + .return_once(move |_, _| Ok(None)); + }), + ..MithrilCertificateChainSynchronizer::default_for_test() + }; + + let starting_point = chain[2].clone(); + let remote_certificate_chain = synchroniser + .retrieve_and_validate_remote_certificate_chain(starting_point.clone()) + .await + .unwrap(); + + assert_eq!( + remote_certificate_chain + .into_iter() + .map(|c| c.hash) + .collect::>(), + vec!["genesis".to_string(), "hash1".to_string(), "hash2".to_string()] + ); + } } mod store_remote_certificate_chain { @@ -579,10 +633,10 @@ mod tests { // Will sync even if force is false synchroniser.synchronize_certificate_chain(false).await.unwrap(); - assert_eq!( - remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash), - storer.stored_certificates() - ); + let mut expected = + remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash); + expected.reverse(); + assert_eq!(expected, storer.stored_certificates()); } #[tokio::test] @@ -607,10 +661,10 @@ mod tests { // Force true - will sync synchroniser.synchronize_certificate_chain(true).await.unwrap(); - assert_eq!( - remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash), - storer.stored_certificates() - ); + let mut expected = + remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash); + expected.reverse(); + assert_eq!(expected, storer.stored_certificates()); } } } From 70dcce9c343346ce01981649798b7a33cef3430f Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Thu, 3 Jul 2025 19:28:54 +0200 Subject: [PATCH 17/28] feat(aggregator): make synchroniser add an open message at process end --- .../interface.rs | 10 ++++ .../synchroniser_service.rs | 51 +++++++++++++++++-- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs index b53e747502b..3f0ca2a50cf 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs @@ -3,6 +3,8 @@ use async_trait::async_trait; use mithril_common::StdResult; use mithril_common::entities::Certificate; +use crate::entities::OpenMessage; + /// Define how to synchronize the certificate chain with a remote source #[cfg_attr(test, mockall::automock)] #[async_trait::async_trait] @@ -35,3 +37,11 @@ pub trait SynchronizedCertificateStorer: Send + Sync { /// Get the latest genesis Certificate async fn get_latest_genesis(&self) -> StdResult>; } + +/// Define how to store the open message created at the end of the synchronization process +#[cfg_attr(test, mockall::automock)] +#[async_trait] +pub trait OpenMessageStorer: Send + Sync { + /// Store an open_message in the database + async fn insert_or_replace_open_message(&self, open_message: OpenMessage) -> StdResult<()>; +} diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 677ac8e81a7..63e9a6a79d3 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -15,10 +15,13 @@ //! 4. Store the fetched certificates in the database, from genesis to latest, for each certificate: //! - if it exists in the database, it is replaced //! - if it doesn't exist, it is inserted -//! 5. End +//! 5. Create a certified open message in the database, based on the latest certificate and with a +//! MithrilStakeDistribution signed entity type +//! 6. End //! use anyhow::{Context, anyhow}; use async_trait::async_trait; +use chrono::Utc; use slog::{Logger, debug, info}; use std::collections::VecDeque; use std::sync::Arc; @@ -26,11 +29,14 @@ use std::sync::Arc; use mithril_common::StdResult; use mithril_common::certificate_chain::CertificateVerifier; use mithril_common::crypto_helper::ProtocolGenesisVerifier; -use mithril_common::entities::Certificate; +use mithril_common::entities::{Certificate, SignedEntityType}; use mithril_common::logging::LoggerExtensions; +use crate::entities::OpenMessage; + use super::{ - CertificateChainSynchronizer, RemoteCertificateRetriever, SynchronizedCertificateStorer, + CertificateChainSynchronizer, OpenMessageStorer, RemoteCertificateRetriever, + SynchronizedCertificateStorer, }; /// Service that synchronizes the certificate chain with a remote aggregator @@ -39,6 +45,7 @@ pub struct MithrilCertificateChainSynchronizer { certificate_storer: Arc, certificate_verifier: Arc, genesis_verifier: Arc, + open_message_storer: Arc, logger: Logger, } @@ -68,6 +75,7 @@ impl MithrilCertificateChainSynchronizer { certificate_storer: Arc, certificate_verifier: Arc, genesis_verifier: Arc, + open_message_storer: Arc, logger: Logger, ) -> Self { Self { @@ -75,6 +83,7 @@ impl MithrilCertificateChainSynchronizer { certificate_storer, certificate_verifier, genesis_verifier, + open_message_storer, logger: logger.new_with_component_name::(), } } @@ -164,6 +173,7 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { anyhow!("Remote aggregator doesn't have a chain yet") .context("Failed to retrieve latest remote certificate details"), )?; + let open_message = prepare_open_message_to_store(&starting_point); let remote_certificate_chain = self .retrieve_and_validate_remote_certificate_chain(starting_point) .await @@ -171,6 +181,10 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { self.store_certificate_chain(remote_certificate_chain) .await .with_context(|| "Failed to store remote retrieved certificate chain")?; + self.open_message_storer + .insert_or_replace_open_message(open_message) + .await + .with_context(|| "Failed to store open message when synchronizing certificate chain")?; info!( self.logger, @@ -180,6 +194,19 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { } } +fn prepare_open_message_to_store(latest_certificate: &Certificate) -> OpenMessage { + OpenMessage { + epoch: latest_certificate.epoch, + signed_entity_type: SignedEntityType::MithrilStakeDistribution(latest_certificate.epoch), + protocol_message: latest_certificate.protocol_message.clone(), + is_certified: true, + is_expired: false, + single_signatures: Vec::new(), + created_at: Utc::now(), + expires_at: None, + } +} + #[cfg(test)] mod tests { use anyhow::anyhow; @@ -194,7 +221,9 @@ mod tests { mock_extensions::MockBuilder, }; - use crate::services::{MockRemoteCertificateRetriever, MockSynchronizedCertificateStorer}; + use crate::services::{ + MockOpenMessageStorer, MockRemoteCertificateRetriever, MockSynchronizedCertificateStorer, + }; use crate::test_tools::TestLogger; use super::*; @@ -240,6 +269,7 @@ mod tests { Arc::new(ProtocolGenesisVerifier::from_verification_key( genesis_verification_key, )), + Arc::new(MockOpenMessageStorer::new()), TestLogger::stdout(), ) } @@ -599,6 +629,8 @@ mod tests { } mod synchronize_certificate_chain { + use mockall::predicate::function; + use super::*; fn build_synchroniser( @@ -617,6 +649,17 @@ mod tests { .return_once(move || Ok(Some(latest))); }), certificate_verifier: fake_verifier(remote_chain), + open_message_storer: MockBuilder::::configure(|mock| { + // Ensure that `store_open_message` is called + let expected_msd_epoch = remote_chain.latest_certificate().epoch; + mock.expect_insert_or_replace_open_message() + .with(function(move |open_message: &OpenMessage| { + open_message.signed_entity_type + == SignedEntityType::MithrilStakeDistribution(expected_msd_epoch) + })) + .times(1..) + .returning(|_| Ok(())); + }), ..MithrilCertificateChainSynchronizer::default_for_test() } } From c45076c5b76c43e5fff29a3eae961eccef460d09 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 8 Jul 2025 16:54:57 +0200 Subject: [PATCH 18/28] feat(aggregator): make synchroniser only persist first cert of each epoch plus the genesis. --- .../synchroniser_service.rs | 34 +++++++++++++++---- 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs index 63e9a6a79d3..9c3f2e61e72 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs @@ -129,11 +129,19 @@ impl MithrilCertificateChainSynchronizer { .with_context( || format!("Failed to verify certificate: `{}`", certificate.hash,), )?; - validated_certificates.push_front(certificate); match parent_certificate { - None => break, + None => { + validated_certificates.push_front(certificate); + break; + } Some(parent) => { + // At the start of the retrieval the first certificate may not be the first of + // its epoch, filter them out since we only need one certificate per epoch + if !validated_certificates.is_empty() || parent.epoch != certificate.epoch { + validated_certificates.push_front(certificate); + } + certificate = parent; } } @@ -480,6 +488,8 @@ mod tests { mod retrieve_validate_remote_certificate_chain { use mockall::predicate::{always, eq}; + use mithril_common::entities::Epoch; + use super::*; #[tokio::test] @@ -512,10 +522,14 @@ mod tests { } #[tokio::test] - async fn succeed_with_a_valid_certificate_chain() { + async fn succeed_with_a_valid_certificate_chain_and_only_get_first_certificate_of_each_epoch_plus_genesis() + { + // Note: the `CertificateChainBuilder` use one epoch for the genesis only, so in order + // for the last epoch to have two certificates when `certificates_per_epoch` is an *even* + // number, we need to set `total_certificates` to an *odd* number let chain = CertificateChainBuilder::new() - .with_total_certificates(10) - .with_certificates_per_epoch(3) + .with_total_certificates(9) + .with_certificates_per_epoch(2) .build(); let synchroniser = MithrilCertificateChainSynchronizer { certificate_verifier: fake_verifier(&chain), @@ -530,7 +544,10 @@ mod tests { .unwrap(); let mut expected = chain.certificate_path_to_genesis(&starting_point.hash); + // Remote certificate chain is returned ordered from genesis to latest expected.reverse(); + // Remove the latest certificate has it's not the first of its epoch + expected.pop(); assert_eq!(remote_certificate_chain, expected); } @@ -538,13 +555,18 @@ mod tests { async fn return_chain_ordered_from_genesis_to_latest() { let base_certificate = fake_data::certificate("whatever"); let chain = vec![ - fake_data::genesis_certificate("genesis"), Certificate { + epoch: Epoch(2), + ..fake_data::genesis_certificate("genesis") + }, + Certificate { + epoch: Epoch(3), hash: "hash1".to_string(), previous_hash: "genesis".to_string(), ..base_certificate.clone() }, Certificate { + epoch: Epoch(4), hash: "hash2".to_string(), previous_hash: "hash1".to_string(), ..base_certificate From 55d7d320a44167aa0ad2d2809040e5fb74e8ae99 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Thu, 3 Jul 2025 14:41:42 +0200 Subject: [PATCH 19/28] feat(aggregator): implement persistence of synchronised certificates --- .../database/query/certificate/conditions.rs | 65 +++++++ .../query/certificate/insert_certificate.rs | 67 +------ .../insert_or_replace_certificate.rs | 169 ++++++++++++++++++ .../src/database/query/certificate/mod.rs | 3 + .../repository/certificate_repository.rs | 33 +++- 5 files changed, 274 insertions(+), 63 deletions(-) create mode 100644 mithril-aggregator/src/database/query/certificate/conditions.rs create mode 100644 mithril-aggregator/src/database/query/certificate/insert_or_replace_certificate.rs diff --git a/mithril-aggregator/src/database/query/certificate/conditions.rs b/mithril-aggregator/src/database/query/certificate/conditions.rs new file mode 100644 index 00000000000..c6687ffc7f5 --- /dev/null +++ b/mithril-aggregator/src/database/query/certificate/conditions.rs @@ -0,0 +1,65 @@ +//! Shared `WhereCondition` across certificates queries + +use sqlite::Value; +use std::iter::repeat_n; + +use mithril_persistence::sqlite::WhereCondition; + +use crate::database::record::CertificateRecord; + +pub(super) fn insert_many(certificates_records: Vec) -> WhereCondition { + let columns = "(\ + certificate_id, \ + parent_certificate_id, \ + message, \ + signature, \ + aggregate_verification_key, \ + epoch, \ + network, \ + signed_entity_type_id, \ + signed_entity_beacon, \ + protocol_version, \ + protocol_parameters, \ + protocol_message, \ + signers, \ + initiated_at, \ + sealed_at)"; + let values_columns: Vec<&str> = repeat_n( + "(?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*)", + certificates_records.len(), + ) + .collect(); + + let values: Vec = certificates_records + .into_iter() + .flat_map(|certificate_record| { + vec![ + Value::String(certificate_record.certificate_id), + match certificate_record.parent_certificate_id { + Some(parent_certificate_id) => Value::String(parent_certificate_id), + None => Value::Null, + }, + Value::String(certificate_record.message), + Value::String(certificate_record.signature), + Value::String(certificate_record.aggregate_verification_key), + Value::Integer(certificate_record.epoch.try_into().unwrap()), + Value::String(certificate_record.network), + Value::Integer(certificate_record.signed_entity_type.index() as i64), + Value::String(certificate_record.signed_entity_type.get_json_beacon().unwrap()), + Value::String(certificate_record.protocol_version), + Value::String( + serde_json::to_string(&certificate_record.protocol_parameters).unwrap(), + ), + Value::String(serde_json::to_string(&certificate_record.protocol_message).unwrap()), + Value::String(serde_json::to_string(&certificate_record.signers).unwrap()), + Value::String(certificate_record.initiated_at.to_rfc3339()), + Value::String(certificate_record.sealed_at.to_rfc3339()), + ] + }) + .collect(); + + WhereCondition::new( + format!("{columns} values {}", values_columns.join(", ")).as_str(), + values, + ) +} diff --git a/mithril-aggregator/src/database/query/certificate/insert_certificate.rs b/mithril-aggregator/src/database/query/certificate/insert_certificate.rs index 82ea9b34364..381e0321e23 100644 --- a/mithril-aggregator/src/database/query/certificate/insert_certificate.rs +++ b/mithril-aggregator/src/database/query/certificate/insert_certificate.rs @@ -1,11 +1,9 @@ -use std::iter::repeat_n; - -use sqlite::Value; - use mithril_persistence::sqlite::{Query, SourceAlias, SqLiteEntity, WhereCondition}; use crate::database::record::CertificateRecord; +use super::conditions; + /// Query to insert [CertificateRecord] in the sqlite database pub struct InsertCertificateRecordQuery { condition: WhereCondition, @@ -17,64 +15,9 @@ impl InsertCertificateRecordQuery { } pub fn many(certificates_records: Vec) -> Self { - let columns = "(\ - certificate_id, \ - parent_certificate_id, \ - message, \ - signature, \ - aggregate_verification_key, \ - epoch, \ - network, \ - signed_entity_type_id, \ - signed_entity_beacon, \ - protocol_version, \ - protocol_parameters, \ - protocol_message, \ - signers, \ - initiated_at, \ - sealed_at)"; - let values_columns: Vec<&str> = repeat_n( - "(?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*)", - certificates_records.len(), - ) - .collect(); - - let values: Vec = certificates_records - .into_iter() - .flat_map(|certificate_record| { - vec![ - Value::String(certificate_record.certificate_id), - match certificate_record.parent_certificate_id { - Some(parent_certificate_id) => Value::String(parent_certificate_id), - None => Value::Null, - }, - Value::String(certificate_record.message), - Value::String(certificate_record.signature), - Value::String(certificate_record.aggregate_verification_key), - Value::Integer(certificate_record.epoch.try_into().unwrap()), - Value::String(certificate_record.network), - Value::Integer(certificate_record.signed_entity_type.index() as i64), - Value::String(certificate_record.signed_entity_type.get_json_beacon().unwrap()), - Value::String(certificate_record.protocol_version), - Value::String( - serde_json::to_string(&certificate_record.protocol_parameters).unwrap(), - ), - Value::String( - serde_json::to_string(&certificate_record.protocol_message).unwrap(), - ), - Value::String(serde_json::to_string(&certificate_record.signers).unwrap()), - Value::String(certificate_record.initiated_at.to_rfc3339()), - Value::String(certificate_record.sealed_at.to_rfc3339()), - ] - }) - .collect(); - - let condition = WhereCondition::new( - format!("{columns} values {}", values_columns.join(", ")).as_str(), - values, - ); - - Self { condition } + Self { + condition: conditions::insert_many(certificates_records), + } } } 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 new file mode 100644 index 00000000000..fa34bb91588 --- /dev/null +++ b/mithril-aggregator/src/database/query/certificate/insert_or_replace_certificate.rs @@ -0,0 +1,169 @@ +use mithril_persistence::sqlite::{Query, SourceAlias, SqLiteEntity, WhereCondition}; + +use crate::database::record::CertificateRecord; + +use super::conditions; + +/// Query to insert or replace [CertificateRecord] in the sqlite database +pub struct InsertOrReplaceCertificateRecordQuery { + condition: WhereCondition, +} + +impl InsertOrReplaceCertificateRecordQuery { + pub fn many(certificates_records: Vec) -> Self { + Self { + condition: conditions::insert_many(certificates_records), + } + } +} + +impl Query for InsertOrReplaceCertificateRecordQuery { + type Entity = CertificateRecord; + + fn filters(&self) -> WhereCondition { + self.condition.clone() + } + + fn get_definition(&self, condition: &str) -> String { + // it is important to alias the fields with the same name as the table + // since the table cannot be aliased in a RETURNING statement in SQLite. + let projection = Self::Entity::get_projection() + .expand(SourceAlias::new(&[("{:certificate:}", "certificate")])); + + format!("insert or replace into certificate {condition} returning {projection}") + } +} + +#[cfg(test)] +mod tests { + use std::collections::HashMap; + + use mithril_common::crypto_helper::tests_setup::setup_certificate_chain; + use mithril_common::entities::Epoch; + use mithril_common::test_utils::fake_data; + use mithril_persistence::sqlite::ConnectionExtensions; + + use crate::database::query::{GetCertificateRecordQuery, InsertCertificateRecordQuery}; + use crate::database::test_helper::main_db_connection; + + use super::*; + + #[test] + fn test_insert_many_certificates_records_in_empty_db() { + let certificates = setup_certificate_chain(5, 2); + let certificates_records: Vec = certificates.into(); + + let connection = main_db_connection().unwrap(); + + let certificates_records_saved: Vec = connection + .fetch_collect(InsertOrReplaceCertificateRecordQuery::many( + certificates_records.clone(), + )) + .expect("saving many records should not fail"); + + assert_eq!(certificates_records, certificates_records_saved); + + // Check insertion order + let all_records: Vec = + connection.fetch_collect(GetCertificateRecordQuery::all()).unwrap(); + assert_eq!( + certificates_records.into_iter().rev().collect::>(), + all_records + ); + } + + #[test] + fn test_replace_one_certificate_record() { + let certificate_record = CertificateRecord { + epoch: Epoch(12), + ..fake_data::certificate("hash").into() + }; + + let connection = main_db_connection().unwrap(); + let certificate_record_saved = connection + .fetch_first(InsertCertificateRecordQuery::one( + certificate_record.clone(), + )) + .unwrap(); + assert_eq!(Some(Epoch(12)), certificate_record_saved.map(|r| r.epoch)); + + let modified_certificate_record = CertificateRecord { + epoch: Epoch(23), + ..certificate_record + }; + let certificate_record_saved = connection + .fetch_first(InsertOrReplaceCertificateRecordQuery::many(vec![ + modified_certificate_record.clone(), + ])) + .unwrap(); + assert_eq!(Some(Epoch(23)), certificate_record_saved.map(|r| r.epoch)); + + let all_records_cursor = connection.fetch(GetCertificateRecordQuery::all()).unwrap(); + assert_eq!(1, all_records_cursor.count()); + } + + #[test] + fn test_insert_and_replace_many_certificate_record() { + let tested_records: HashMap<_, CertificateRecord> = HashMap::from([ + ( + "cert1-genesis", + fake_data::genesis_certificate("genesis").into(), + ), + ("cert2", fake_data::certificate("cert2").into()), + ( + "cert2-modified", + CertificateRecord { + epoch: Epoch(14), + ..fake_data::certificate("cert2").into() + }, + ), + ("cert3", fake_data::certificate("cert3").into()), + ("cert4", fake_data::certificate("cert4").into()), + ( + "cert4-modified", + CertificateRecord { + epoch: Epoch(32), + ..fake_data::certificate("cert4").into() + }, + ), + ("cert5", fake_data::certificate("cert5").into()), + ]); + let connection = main_db_connection().unwrap(); + + let cursor = connection + .fetch(InsertCertificateRecordQuery::many(vec![ + tested_records["cert1-genesis"].clone(), + tested_records["cert2"].clone(), + tested_records["cert3"].clone(), + tested_records["cert4"].clone(), + tested_records["cert5"].clone(), + ])) + .unwrap(); + assert_eq!(5, cursor.count()); + + let cursor = connection + .fetch(InsertOrReplaceCertificateRecordQuery::many(vec![ + tested_records["cert1-genesis"].clone(), + tested_records["cert2-modified"].clone(), + tested_records["cert3"].clone(), + tested_records["cert4-modified"].clone(), + ])) + .unwrap(); + assert_eq!(4, cursor.count()); + + let all_records: Vec = + connection.fetch_collect(GetCertificateRecordQuery::all()).unwrap(); + assert_eq!(5, all_records.len()); + assert_eq!( + all_records, + vec![ + tested_records["cert4-modified"].clone(), + tested_records["cert3"].clone(), + tested_records["cert2-modified"].clone(), + tested_records["cert1-genesis"].clone(), + // Since the cert5 was not in the Insert/replace query, it now has a lower rowid and shows first + tested_records["cert5"].clone(), + ] + ); + } +} diff --git a/mithril-aggregator/src/database/query/certificate/mod.rs b/mithril-aggregator/src/database/query/certificate/mod.rs index 05f5ed5e073..9d1cc2c8c98 100644 --- a/mithril-aggregator/src/database/query/certificate/mod.rs +++ b/mithril-aggregator/src/database/query/certificate/mod.rs @@ -1,9 +1,12 @@ +mod conditions; mod delete_certificate; mod get_certificate; mod get_master_certificate; mod insert_certificate; +mod insert_or_replace_certificate; pub use delete_certificate::*; pub use get_certificate::*; pub use get_master_certificate::*; pub use insert_certificate::*; +pub use insert_or_replace_certificate::*; diff --git a/mithril-aggregator/src/database/repository/certificate_repository.rs b/mithril-aggregator/src/database/repository/certificate_repository.rs index 18ecd0a27b1..ebb985ee5a3 100644 --- a/mithril-aggregator/src/database/repository/certificate_repository.rs +++ b/mithril-aggregator/src/database/repository/certificate_repository.rs @@ -11,9 +11,10 @@ use mithril_persistence::sqlite::ConnectionExtensions; use crate::database::query::{ DeleteCertificateQuery, GetCertificateRecordQuery, InsertCertificateRecordQuery, - MasterCertificateQuery, + InsertOrReplaceCertificateRecordQuery, MasterCertificateQuery, }; use crate::database::record::CertificateRecord; +use crate::services::SynchronizedCertificateStorer; /// Database frontend API for Certificate queries. pub struct CertificateRepository { @@ -105,6 +106,24 @@ impl CertificateRepository { Ok(new_certificates.map(|cert| cert.into()).collect()) } + /// Create, or replace if they already exist, many certificates at once in the database. + pub async fn create_or_replace_many_certificates( + &self, + certificates: Vec, + ) -> StdResult> { + if certificates.is_empty() { + return Ok(vec![]); + } + + let records: Vec = + certificates.into_iter().map(|cert| cert.into()).collect(); + let new_certificates = self + .connection + .fetch(InsertOrReplaceCertificateRecordQuery::many(records))?; + + Ok(new_certificates.map(|cert| cert.into()).collect()) + } + /// Delete all the given certificates from the database pub async fn delete_certificates(&self, certificates: &[&Certificate]) -> StdResult<()> { let ids = certificates.iter().map(|c| c.hash.as_str()).collect::>(); @@ -131,6 +150,18 @@ impl CertificateRetriever for CertificateRepository { } } +#[async_trait] +impl SynchronizedCertificateStorer for CertificateRepository { + async fn insert_or_replace_many(&self, certificates: Vec) -> StdResult<()> { + self.create_or_replace_many_certificates(certificates).await?; + Ok(()) + } + + async fn get_latest_genesis(&self) -> StdResult> { + self.get_latest_genesis_certificate().await + } +} + #[cfg(test)] mod tests { use mithril_common::crypto_helper::tests_setup::setup_certificate_chain; From 8931cb8b40a05ce62660dc599eb3f7530cb04b25 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Fri, 4 Jul 2025 15:25:37 +0200 Subject: [PATCH 20/28] feat(aggregator): implement persistence of openmessage created post sync --- .../database/query/open_message/conditions.rs | 32 +++++++ .../query/open_message/get_open_message.rs | 7 ++ .../query/open_message/insert_open_message.rs | 96 +++++++++++++++---- .../insert_or_replace_open_message.rs | 92 ++++++++++++++++++ .../src/database/query/open_message/mod.rs | 3 + .../src/database/record/open_message.rs | 5 + .../repository/open_message_repository.rs | 63 +++++++++++- .../src/entities/open_message.rs | 2 +- 8 files changed, 279 insertions(+), 21 deletions(-) create mode 100644 mithril-aggregator/src/database/query/open_message/conditions.rs create mode 100644 mithril-aggregator/src/database/query/open_message/insert_or_replace_open_message.rs diff --git a/mithril-aggregator/src/database/query/open_message/conditions.rs b/mithril-aggregator/src/database/query/open_message/conditions.rs new file mode 100644 index 00000000000..393c8a1a36f --- /dev/null +++ b/mithril-aggregator/src/database/query/open_message/conditions.rs @@ -0,0 +1,32 @@ +//! Shared `WhereCondition` across open message queries + +use sqlite::Value; + +use mithril_common::StdResult; +use mithril_persistence::sqlite::WhereCondition; + +use crate::database::record::OpenMessageRecord; + +pub(crate) fn insert_one(record: OpenMessageRecord) -> StdResult { + let expression = "(\ + open_message_id, epoch_setting_id, beacon, signed_entity_type_id, protocol_message, \ + expires_at, created_at, is_certified, is_expired\ + ) values (?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*, ?*)"; + let beacon_str = record.signed_entity_type.get_json_beacon()?; + let parameters = vec![ + Value::String(record.open_message_id.to_string()), + Value::Integer(record.epoch.try_into()?), + Value::String(beacon_str), + Value::Integer(record.signed_entity_type.index() as i64), + Value::String(serde_json::to_string(&record.protocol_message)?), + record + .expires_at + .map(|t| Value::String(t.to_rfc3339())) + .unwrap_or(Value::Null), + Value::String(record.created_at.to_rfc3339()), + Value::Integer(record.is_certified as i64), + Value::Integer(record.is_expired as i64), + ]; + + Ok(WhereCondition::new(expression, parameters)) +} diff --git a/mithril-aggregator/src/database/query/open_message/get_open_message.rs b/mithril-aggregator/src/database/query/open_message/get_open_message.rs index 85cbe1a4f5a..3a16b52fd00 100644 --- a/mithril-aggregator/src/database/query/open_message/get_open_message.rs +++ b/mithril-aggregator/src/database/query/open_message/get_open_message.rs @@ -15,6 +15,13 @@ pub struct GetOpenMessageQuery { } impl GetOpenMessageQuery { + #[cfg(test)] + pub fn all() -> Self { + Self { + condition: WhereCondition::default(), + } + } + pub fn by_epoch_and_signed_entity_type( epoch: Epoch, signed_entity_type: &SignedEntityType, diff --git a/mithril-aggregator/src/database/query/open_message/insert_open_message.rs b/mithril-aggregator/src/database/query/open_message/insert_open_message.rs index a3663ce3c65..df60e6a29ec 100644 --- a/mithril-aggregator/src/database/query/open_message/insert_open_message.rs +++ b/mithril-aggregator/src/database/query/open_message/insert_open_message.rs @@ -1,11 +1,10 @@ use chrono::Utc; -use sqlite::Value; -use uuid::Uuid; use mithril_common::StdResult; use mithril_common::entities::{Epoch, ProtocolMessage, SignedEntityType}; use mithril_persistence::sqlite::{Query, SourceAlias, SqLiteEntity, WhereCondition}; +use crate::database::query::open_message::conditions; use crate::database::record::OpenMessageRecord; /// Query to insert [OpenMessageRecord] in the sqlite database @@ -19,23 +18,20 @@ impl InsertOpenMessageQuery { signed_entity_type: &SignedEntityType, protocol_message: &ProtocolMessage, ) -> StdResult { - let expression = "(open_message_id, epoch_setting_id, beacon, signed_entity_type_id, protocol_message, expires_at, created_at) values (?*, ?*, ?*, ?*, ?*, ?*, ?*)"; - let beacon_str = signed_entity_type.get_json_beacon()?; - let parameters = vec![ - Value::String(Uuid::new_v4().to_string()), - Value::Integer(epoch.try_into()?), - Value::String(beacon_str), - Value::Integer(signed_entity_type.index() as i64), - Value::String(serde_json::to_string(protocol_message)?), - signed_entity_type - .get_open_message_timeout() - .map(|t| Value::String((Utc::now() + t).to_rfc3339())) - .unwrap_or(Value::Null), - Value::String(Utc::now().to_rfc3339()), - ]; + let now = Utc::now(); + let record = OpenMessageRecord { + open_message_id: OpenMessageRecord::new_id(), + epoch, + signed_entity_type: signed_entity_type.clone(), + protocol_message: protocol_message.clone(), + is_certified: false, + is_expired: false, + created_at: now, + expires_at: signed_entity_type.get_open_message_timeout().map(|t| now + t), + }; Ok(Self { - condition: WhereCondition::new(expression, parameters), + condition: conditions::insert_one(record)?, }) } } @@ -54,3 +50,69 @@ impl Query for InsertOpenMessageQuery { format!("insert into open_message {condition} returning {projection}") } } + +#[cfg(test)] +mod tests { + use mithril_common::entities::ProtocolMessagePartKey; + use mithril_persistence::sqlite::ConnectionExtensions; + + use crate::database::query::GetOpenMessageQuery; + use crate::database::test_helper::main_db_connection; + + use super::*; + + #[test] + fn test_insert_one() { + let connection = main_db_connection().unwrap(); + let epoch = Epoch(5); + let signed_entity_type = SignedEntityType::CardanoStakeDistribution(Epoch(10)); + let mut protocol_message = ProtocolMessage::new(); + protocol_message.set_message_part( + ProtocolMessagePartKey::CardanoStakeDistributionEpoch, + "value".to_string(), + ); + + connection + .fetch_first( + InsertOpenMessageQuery::one(epoch, &signed_entity_type, &protocol_message).unwrap(), + ) + .unwrap(); + let records: Vec = + connection.fetch_collect(GetOpenMessageQuery::all()).unwrap(); + + assert_eq!(1, records.len()); + assert_eq!( + OpenMessageRecord { + open_message_id: records[0].open_message_id, + epoch, + signed_entity_type, + protocol_message, + is_certified: false, + is_expired: false, + created_at: records[0].created_at, + expires_at: records[0].expires_at, + }, + records[0] + ); + } + + #[should_panic] + #[test] + fn test_insert_two_entries_with_same_signed_entity_violate_unique_constraint() { + let connection = main_db_connection().unwrap(); + let epoch = Epoch(5); + let signed_entity_type = SignedEntityType::MithrilStakeDistribution(Epoch(10)); + + connection + .fetch_first( + InsertOpenMessageQuery::one(epoch, &signed_entity_type, &ProtocolMessage::new()) + .unwrap(), + ) + .unwrap(); + + let _ = connection.fetch_first( + InsertOpenMessageQuery::one(epoch + 10, &signed_entity_type, &ProtocolMessage::new()) + .unwrap(), + ); + } +} diff --git a/mithril-aggregator/src/database/query/open_message/insert_or_replace_open_message.rs b/mithril-aggregator/src/database/query/open_message/insert_or_replace_open_message.rs new file mode 100644 index 00000000000..358fed71d42 --- /dev/null +++ b/mithril-aggregator/src/database/query/open_message/insert_or_replace_open_message.rs @@ -0,0 +1,92 @@ +use mithril_common::StdResult; +use mithril_persistence::sqlite::{Query, SourceAlias, SqLiteEntity, WhereCondition}; + +use crate::database::query::open_message::conditions; +use crate::database::record::OpenMessageRecord; + +/// Query to insert [OpenMessageRecord] in the sqlite database +pub struct InsertOrReplaceOpenMessageQuery { + condition: WhereCondition, +} + +impl InsertOrReplaceOpenMessageQuery { + pub fn one(record: OpenMessageRecord) -> StdResult { + Ok(Self { + condition: conditions::insert_one(record)?, + }) + } +} + +impl Query for InsertOrReplaceOpenMessageQuery { + type Entity = OpenMessageRecord; + + fn filters(&self) -> WhereCondition { + self.condition.clone() + } + + fn get_definition(&self, condition: &str) -> String { + let aliases = SourceAlias::new(&[("{:open_message:}", "open_message")]); + let projection = Self::Entity::get_projection().expand(aliases); + + format!("insert or replace into open_message {condition} returning {projection}") + } +} + +#[cfg(test)] +mod tests { + use mithril_persistence::sqlite::ConnectionExtensions; + + use crate::database::query::GetOpenMessageQuery; + use crate::database::test_helper::main_db_connection; + + use super::*; + + #[test] + fn test_insert_one() { + let connection = main_db_connection().unwrap(); + let record = OpenMessageRecord::dummy(); + + connection + .fetch_first(InsertOrReplaceOpenMessageQuery::one(record.clone()).unwrap()) + .unwrap(); + let records: Vec = connection + .fetch_collect( + GetOpenMessageQuery::by_epoch_and_signed_entity_type( + record.epoch, + &record.signed_entity_type, + ) + .unwrap(), + ) + .unwrap(); + + assert_eq!(1, records.len()); + assert_eq!(record, records[0]); + } + + #[test] + fn test_insert_record_for_existing_signed_entity_type_replaces_it() { + let connection = main_db_connection().unwrap(); + let record = OpenMessageRecord { + is_expired: false, + ..OpenMessageRecord::dummy() + }; + + connection + .fetch_first(InsertOrReplaceOpenMessageQuery::one(record.clone()).unwrap()) + .unwrap(); + + let replaced_record = connection + .fetch_first( + InsertOrReplaceOpenMessageQuery::one(OpenMessageRecord { + is_expired: true, + ..record.clone() + }) + .unwrap(), + ) + .unwrap(); + let count = connection.fetch(GetOpenMessageQuery::all()).unwrap().count(); + + assert_eq!(1, count); + assert_eq!(Some(true), replaced_record.map(|r| r.is_expired)); + } +} diff --git a/mithril-aggregator/src/database/query/open_message/mod.rs b/mithril-aggregator/src/database/query/open_message/mod.rs index f964c018855..9580befce44 100644 --- a/mithril-aggregator/src/database/query/open_message/mod.rs +++ b/mithril-aggregator/src/database/query/open_message/mod.rs @@ -1,11 +1,14 @@ +mod conditions; mod delete_open_message; mod get_open_message; mod get_open_message_with_single_signatures; mod insert_open_message; +mod insert_or_replace_open_message; mod update_open_message; pub use delete_open_message::*; pub use get_open_message::*; pub use get_open_message_with_single_signatures::*; pub use insert_open_message::*; +pub use insert_or_replace_open_message::*; pub use update_open_message::*; diff --git a/mithril-aggregator/src/database/record/open_message.rs b/mithril-aggregator/src/database/record/open_message.rs index 993ead563e5..40b0159dc75 100644 --- a/mithril-aggregator/src/database/record/open_message.rs +++ b/mithril-aggregator/src/database/record/open_message.rs @@ -39,6 +39,11 @@ pub struct OpenMessageRecord { } impl OpenMessageRecord { + /// Creates a new random id that can be used for a new record + pub fn new_id() -> Uuid { + Uuid::new_v4() + } + #[cfg(test)] /// Create a dumb OpenMessage instance mainly for test purposes pub fn dummy() -> Self { diff --git a/mithril-aggregator/src/database/repository/open_message_repository.rs b/mithril-aggregator/src/database/repository/open_message_repository.rs index 66edbdcba85..57f36635ee8 100644 --- a/mithril-aggregator/src/database/repository/open_message_repository.rs +++ b/mithril-aggregator/src/database/repository/open_message_repository.rs @@ -1,6 +1,6 @@ -use std::sync::Arc; - +use async_trait::async_trait; use chrono::Utc; +use std::sync::Arc; use mithril_common::StdResult; use mithril_common::entities::{Epoch, ProtocolMessage, SignedEntityType}; @@ -8,9 +8,11 @@ use mithril_persistence::sqlite::{ConnectionExtensions, SqliteConnection}; use crate::database::query::{ DeleteOpenMessageQuery, GetOpenMessageQuery, GetOpenMessageWithSingleSignaturesQuery, - InsertOpenMessageQuery, UpdateOpenMessageQuery, + InsertOpenMessageQuery, InsertOrReplaceOpenMessageQuery, UpdateOpenMessageQuery, }; use crate::database::record::{OpenMessageRecord, OpenMessageWithSingleSignaturesRecord}; +use crate::entities::OpenMessage; +use crate::services::OpenMessageStorer; /// ## Open message repository /// @@ -79,6 +81,21 @@ impl OpenMessageRepository { message.ok_or_else(|| panic!("Inserting an open_message should not return nothing.")) } + /// Create, or replace if one with the same [SignedEntityType] they already exist, a + /// [OpenMessageRecord] in the database. + pub async fn create_or_replace_open_message( + &self, + record: OpenMessageRecord, + ) -> StdResult { + let message = self + .connection + .fetch_first(InsertOrReplaceOpenMessageQuery::one(record)?)?; + + message.ok_or_else(|| { + panic!("Inserting or replacing an open_message should not return nothing.") + }) + } + /// Updates an [OpenMessageRecord] in the database. pub async fn update_open_message( &self, @@ -102,6 +119,24 @@ impl OpenMessageRepository { } } +#[async_trait] +impl OpenMessageStorer for OpenMessageRepository { + async fn insert_or_replace_open_message(&self, open_message: OpenMessage) -> StdResult<()> { + let record = OpenMessageRecord { + open_message_id: OpenMessageRecord::new_id(), + epoch: open_message.epoch, + signed_entity_type: open_message.signed_entity_type, + protocol_message: open_message.protocol_message, + is_certified: open_message.is_certified, + is_expired: open_message.is_expired, + created_at: open_message.created_at, + expires_at: open_message.expires_at, + }; + self.create_or_replace_open_message(record).await?; + Ok(()) + } +} + #[cfg(test)] mod tests { use mithril_common::entities::{BlockNumber, CardanoDbBeacon}; @@ -261,6 +296,28 @@ mod tests { assert_eq!(open_message.epoch, message.epoch); } + #[tokio::test] + async fn repository_create_or_replace_open_message() { + let connection = get_connection().await; + let repository = OpenMessageRepository::new(connection.clone()); + let mut inserted_record = repository + .create_or_replace_open_message(OpenMessageRecord { + epoch: Epoch(5), + signed_entity_type: SignedEntityType::MithrilStakeDistribution(Epoch(6)), + ..OpenMessageRecord::dummy() + }) + .await + .unwrap(); + assert_eq!(Epoch(5), inserted_record.epoch); + + inserted_record.epoch = Epoch(32); + let replaced_record = repository + .create_or_replace_open_message(inserted_record) + .await + .unwrap(); + assert_eq!(Epoch(32), replaced_record.epoch); + } + #[tokio::test] async fn repository_update_open_message() { let connection = get_connection().await; diff --git a/mithril-aggregator/src/entities/open_message.rs b/mithril-aggregator/src/entities/open_message.rs index e3b8f98e1cb..813ba0648b3 100644 --- a/mithril-aggregator/src/entities/open_message.rs +++ b/mithril-aggregator/src/entities/open_message.rs @@ -120,7 +120,7 @@ mod test { fn test_from_record() { let created_at = Utc::now(); let record = OpenMessageRecord { - open_message_id: Uuid::new_v4(), + open_message_id: OpenMessageRecord::new_id(), epoch: Epoch(1), signed_entity_type: SignedEntityType::dummy(), protocol_message: ProtocolMessage::default(), From 930ee6e82dcf7ceeadb5db12648c8d02a21d8ec8 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Fri, 4 Jul 2025 20:28:21 +0200 Subject: [PATCH 21/28] feat(aggregator): add follower sync in state machine + wire synchroniser --- .../src/dependency_injection/builder/mod.rs | 11 ++++-- .../builder/protocol/certificates.rs | 38 ++++++++++++++++++- .../dependency_injection/containers/serve.rs | 8 +++- mithril-aggregator/src/runtime/runner.rs | 20 ++++++++++ .../src/runtime/state_machine.rs | 19 +++++++++- .../src/services/aggregator_client.rs | 22 +++++++++++ .../certificate_chain_synchroniser/mod.rs | 2 + .../certificate_chain_synchroniser/noop.rs | 13 +++++++ 8 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs diff --git a/mithril-aggregator/src/dependency_injection/builder/mod.rs b/mithril-aggregator/src/dependency_injection/builder/mod.rs index 1654ba8682b..f0d5a7083f0 100644 --- a/mithril-aggregator/src/dependency_injection/builder/mod.rs +++ b/mithril-aggregator/src/dependency_injection/builder/mod.rs @@ -56,9 +56,9 @@ use crate::{ file_uploaders::FileUploader, http_server::routes::router::{self, RouterConfig, RouterState}, services::{ - AggregatorHTTPClient, CertifierService, MessageService, MithrilSignerRegistrationFollower, - ProverService, SignedEntityService, SignerSynchronizer, Snapshotter, - StakeDistributionService, UpkeepService, + AggregatorHTTPClient, CertificateChainSynchronizer, CertifierService, MessageService, + MithrilSignerRegistrationFollower, ProverService, SignedEntityService, SignerSynchronizer, + Snapshotter, StakeDistributionService, UpkeepService, }, tools::file_archiver::FileArchiver, }; @@ -185,6 +185,9 @@ pub struct DependenciesBuilder { /// Genesis signature verifier service. pub genesis_verifier: Option>, + /// Certificate chain synchroniser service + pub certificate_chain_synchroniser: Option>, + /// Mithril signer registration leader service pub mithril_signer_registration_leader: Option>, @@ -312,6 +315,7 @@ impl DependenciesBuilder { snapshotter: None, certificate_verifier: None, genesis_verifier: None, + certificate_chain_synchroniser: None, mithril_signer_registration_leader: None, mithril_signer_registration_follower: None, signer_registerer: None, @@ -374,6 +378,7 @@ impl DependenciesBuilder { verification_key_store: self.get_verification_key_store().await?, epoch_settings_storer: self.get_epoch_settings_store().await?, chain_observer: self.get_chain_observer().await?, + certificate_chain_synchroniser: self.get_certificate_chain_synchroniser().await?, signer_registerer: self.get_signer_registerer().await?, signer_synchronizer: self.get_signer_synchronizer().await?, signer_registration_round_opener: self.get_signer_registration_round_opener().await?, diff --git a/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs b/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs index 1bbb754cb91..8ed9e2bdde2 100644 --- a/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs +++ b/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs @@ -10,8 +10,9 @@ use crate::database::repository::{BufferedSingleSignatureRepository, SingleSigna use crate::dependency_injection::{DependenciesBuilder, DependenciesBuilderError, Result}; use crate::get_dependency; use crate::services::{ - BufferedCertifierService, CertifierService, MithrilCertifierService, - MithrilSignerRegistrationFollower, SignerSynchronizer, + BufferedCertifierService, CertificateChainSynchronizer, CertifierService, + MithrilCertificateChainSynchroniserNoop, MithrilCertificateChainSynchronizer, + MithrilCertifierService, MithrilSignerRegistrationFollower, SignerSynchronizer, }; use crate::{ ExecutionEnvironment, MithrilSignerRegistrationLeader, MithrilSignerRegistrationVerifier, @@ -84,6 +85,39 @@ impl DependenciesBuilder { get_dependency!(self.multi_signer) } + async fn build_certificate_chain_synchroniser( + &mut self, + ) -> Result> { + let synchroniser: Arc = + if self.configuration.is_follower_aggregator() { + let leader_aggregator_client = self.get_leader_aggregator_client().await?; + let verifier = Arc::new(MithrilCertificateVerifier::new( + self.root_logger(), + leader_aggregator_client.clone(), + )); + + Arc::new(MithrilCertificateChainSynchronizer::new( + leader_aggregator_client, + self.get_certificate_repository().await?, + verifier, + self.get_genesis_verifier().await?, + self.get_open_message_repository().await?, + self.root_logger(), + )) + } else { + Arc::new(MithrilCertificateChainSynchroniserNoop) + }; + + Ok(synchroniser) + } + + /// [CertificateChainSynchronizer] service + pub async fn get_certificate_chain_synchroniser( + &mut self, + ) -> Result> { + get_dependency!(self.certificate_chain_synchroniser) + } + async fn build_certificate_verifier(&mut self) -> Result> { let verifier = Arc::new(MithrilCertificateVerifier::new( self.root_logger(), diff --git a/mithril-aggregator/src/dependency_injection/containers/serve.rs b/mithril-aggregator/src/dependency_injection/containers/serve.rs index ecf68d4dc78..63fb9231f7c 100644 --- a/mithril-aggregator/src/dependency_injection/containers/serve.rs +++ b/mithril-aggregator/src/dependency_injection/containers/serve.rs @@ -27,8 +27,9 @@ use crate::{ entities::AggregatorEpochSettings, event_store::{EventMessage, TransmitterService}, services::{ - CertifierService, EpochService, MessageService, ProverService, SignedEntityService, - SignerRecorder, SignerSynchronizer, StakeDistributionService, UpkeepService, + CertificateChainSynchronizer, CertifierService, EpochService, MessageService, + ProverService, SignedEntityService, SignerRecorder, SignerSynchronizer, + StakeDistributionService, UpkeepService, }, }; @@ -57,6 +58,9 @@ pub struct ServeCommandDependenciesContainer { /// Chain observer service. pub(crate) chain_observer: Arc, + /// Certificate chain synchroniser service + pub(crate) certificate_chain_synchroniser: Arc, + /// Signer registerer service pub signer_registerer: Arc, diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index 933e68ac1bf..5e6bc388691 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -72,6 +72,12 @@ pub trait AggregatorRunnerTrait: Sync + Send { /// Synchronize the follower aggregator signer registration. async fn synchronize_follower_aggregator_signer_registration(&self) -> StdResult<()>; + /// Synchronise the follower aggregator certificate chain + async fn synchronize_follower_aggregator_certificate_chain( + &self, + force_sync: bool, + ) -> StdResult<()>; + /// Ask the EpochService to update the epoch settings. async fn update_epoch_settings(&self) -> StdResult<()>; @@ -506,6 +512,20 @@ impl AggregatorRunnerTrait for AggregatorRunner { .get_runtime_cycle_total_since_startup() .increment(); } + + async fn synchronize_follower_aggregator_certificate_chain( + &self, + force_sync: bool, + ) -> StdResult<()> { + debug!( + self.logger, + ">> synchronize_follower_aggregator_certificate_chain(force_sync:{force_sync})" + ); + self.dependencies + .certificate_chain_synchroniser + .synchronize_certificate_chain(force_sync) + .await + } } #[cfg(test)] diff --git a/mithril-aggregator/src/runtime/state_machine.rs b/mithril-aggregator/src/runtime/state_machine.rs index 6add3afab29..3b5453011d4 100644 --- a/mithril-aggregator/src/runtime/state_machine.rs +++ b/mithril-aggregator/src/runtime/state_machine.rs @@ -277,13 +277,21 @@ impl AggregatorRuntime { self.runner.precompute_epoch_data().await?; } - self.runner + let chain_validity_result = self + .runner .is_certificate_chain_valid(&new_time_point) .await .map_err(|e| RuntimeError::KeepState { message: "certificate chain is invalid".to_string(), nested_error: e.into(), - })?; + }); + if self.config.is_follower { + let force_sync = chain_validity_result.is_err(); + self.runner + .synchronize_follower_aggregator_certificate_chain(force_sync) + .await?; + } + chain_validity_result?; Ok(()) } @@ -834,6 +842,8 @@ mod tests { } mod follower { + use mockall::predicate::eq; + use super::*; #[tokio::test] @@ -911,6 +921,11 @@ mod tests { .expect_is_certificate_chain_valid() .once() .returning(|_| Ok(())); + runner + .expect_synchronize_follower_aggregator_certificate_chain() + .once() + .with(eq(false)) // Certificate chain valid so force_sync must be false + .returning(|_| Ok(())); runner .expect_update_era_checker() .with(predicate::eq(new_time_point_clone.clone().epoch)) diff --git a/mithril-aggregator/src/services/aggregator_client.rs b/mithril-aggregator/src/services/aggregator_client.rs index 43443990623..547b0d81257 100644 --- a/mithril-aggregator/src/services/aggregator_client.rs +++ b/mithril-aggregator/src/services/aggregator_client.rs @@ -11,6 +11,7 @@ use thiserror::Error; use mithril_common::{ MITHRIL_AGGREGATOR_VERSION_HEADER, MITHRIL_API_VERSION_HEADER, StdError, StdResult, api_version::APIVersionProvider, + certificate_chain::{CertificateRetriever, CertificateRetrieverError}, entities::{Certificate, ClientError, ServerError}, logging::LoggerExtensions, messages::{ @@ -334,6 +335,27 @@ impl LeaderAggregatorClient for AggregatorHTTPClient { } } +#[async_trait] +impl CertificateRetriever for AggregatorHTTPClient { + async fn get_certificate_details( + &self, + certificate_hash: &str, + ) -> Result { + let message = self + .certificates_details(certificate_hash) + .await + .with_context(|| { + format!("Failed to retrieve certificate with hash: '{certificate_hash}'") + }) + .map_err(CertificateRetrieverError)? + .ok_or(CertificateRetrieverError(anyhow!( + "Certificate does not exist: '{certificate_hash}'" + )))?; + + message.try_into().map_err(CertificateRetrieverError) + } +} + #[async_trait] impl RemoteCertificateRetriever for AggregatorHTTPClient { async fn get_latest_certificate_details(&self) -> StdResult> { diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs index d55885013c3..cde74808358 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs @@ -1,5 +1,7 @@ mod interface; +mod noop; mod synchroniser_service; pub use interface::*; +pub use noop::*; pub use synchroniser_service::*; diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs b/mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs new file mode 100644 index 00000000000..076bc6e2ae1 --- /dev/null +++ b/mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs @@ -0,0 +1,13 @@ +use mithril_common::StdResult; + +use crate::services::CertificateChainSynchronizer; + +/// A noop [CertificateChainSynchronizer] for leader aggregators +pub struct MithrilCertificateChainSynchroniserNoop; + +#[async_trait::async_trait] +impl CertificateChainSynchronizer for MithrilCertificateChainSynchroniserNoop { + async fn synchronize_certificate_chain(&self, _force: bool) -> StdResult<()> { + Ok(()) + } +} From c5fa04057e3e8fb66ffd9b426d85ccfa4e8ffb4d Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Mon, 7 Jul 2025 17:11:53 +0200 Subject: [PATCH 22/28] test(aggregator): update tooling to check cert have associated signed entity only if not synchronized Has synchronised certificate are created without a signed entity, previouysly it was only the case for genesis certificates. --- .../tests/test_extensions/runtime_tester.rs | 156 ++++++++++-------- .../tests/test_extensions/utilities.rs | 18 ++ 2 files changed, 104 insertions(+), 70 deletions(-) diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index 322873af477..f45d2de5217 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -36,7 +36,7 @@ use mithril_common::{ use mithril_era::{EraMarker, EraReader, adapters::EraReaderDummyAdapter}; use crate::test_extensions::leader_aggregator_http_server::LeaderAggregatorHttpServer; -use crate::test_extensions::utilities::tx_hash; +use crate::test_extensions::utilities::{async_wait, tx_hash}; use crate::test_extensions::{AggregatorObserver, ExpectedCertificate, MetricsVerifier}; #[macro_export] @@ -67,9 +67,10 @@ macro_rules! cycle_err { let (runtime_cycle_success, runtime_cycle_total) = $tester.get_runtime_cycle_success_and_total_since_startup_metrics(); - RuntimeTester::cycle(&mut $tester) + let err = RuntimeTester::cycle(&mut $tester) .await .expect_err("cycle tick should have returned an error"); + slog_scope::info!("cycle_err result: {err:?}"); assert_eq!($expected_state, $tester.runtime.get_state()); assert_metrics_eq!( @@ -85,12 +86,30 @@ macro_rules! cycle_err { macro_rules! assert_last_certificate_eq { ( $tester:expr, $expected_certificate:expr ) => {{ if let Some(signed_type) = $expected_certificate.get_signed_type() { - $tester.wait_until_signed_entity(&signed_type).await.unwrap(); + RuntimeTester::wait_until_signed_entity(&$tester, &signed_type) + .await + .unwrap(); } - let last_certificate = RuntimeTester::get_last_expected_certificate(&mut $tester) - .await - .unwrap(); + let is_synchronized_from_leader = false; + let last_certificate = + RuntimeTester::get_last_expected_certificate(&mut $tester, is_synchronized_from_leader) + .await + .unwrap(); + assert_eq!($expected_certificate, last_certificate); + }}; + ( $tester:expr, synchronised_from_leader => $expected_certificate:expr ) => {{ + if let Some(signed_type) = $expected_certificate.get_signed_type() { + RuntimeTester::wait_until_certificate(&$tester, &signed_type) + .await + .unwrap(); + } + + let is_synchronized_from_leader = true; + let last_certificate = + RuntimeTester::get_last_expected_certificate(&mut $tester, is_synchronized_from_leader) + .await + .unwrap(); assert_eq!($expected_certificate, last_certificate); }}; } @@ -538,12 +557,11 @@ impl RuntimeTester { Ok(()) } - /// Get the last produced certificate with its signed entity if it's not a genesis certificate - pub async fn get_last_certificate_with_signed_entity( + /// Get the last produced signed entity + async fn get_last_signed_entity( &mut self, - ) -> StdResult<(Certificate, Option)> { - let certificate = self.observer.get_last_certificate().await?; - + certificate: &Certificate, + ) -> StdResult> { let signed_entity = match &certificate.signature { CertificateSignature::GenesisSignature(..) => None, CertificateSignature::MultiSignature(..) => { @@ -560,37 +578,40 @@ impl RuntimeTester { } }; - Ok((certificate, signed_entity)) + Ok(signed_entity) } /// Get the last produced certificate and transform it to a [ExpectedCertificate] - pub async fn get_last_expected_certificate(&mut self) -> StdResult { - let (certificate, signed_entity_record) = - self.get_last_certificate_with_signed_entity().await?; + pub async fn get_last_expected_certificate( + &mut self, + is_synchronized_from_leader: bool, + ) -> StdResult { + let certificate = self.observer.get_last_certificate().await?; - let expected_certificate = match signed_entity_record { - None if certificate.is_genesis() => ExpectedCertificate::new_genesis( + let expected_certificate = if certificate.is_genesis() { + ExpectedCertificate::new_genesis( certificate.epoch, certificate.aggregate_verification_key.try_into().unwrap(), - ), - None => { - panic!( + ) + } else { + let signed_entity_type = certificate.signed_entity_type(); + let previous_cert_identifier = self + .get_expected_certificate_identifier(&certificate.previous_hash) + .await?; + + if !is_synchronized_from_leader && !certificate.is_genesis() { + self.get_last_signed_entity(&certificate).await?.ok_or(anyhow!( "A certificate should always have a SignedEntity if it's not a genesis certificate" - ); - } - Some(record) => { - let previous_cert_identifier = self - .get_expected_certificate_identifier(&certificate.previous_hash) - .await?; - - ExpectedCertificate::new( - certificate.epoch, - certificate.metadata.signers.as_slice(), - certificate.aggregate_verification_key.try_into().unwrap(), - record.signed_entity_type, - previous_cert_identifier, - ) + ))?; } + + ExpectedCertificate::new( + certificate.epoch, + certificate.metadata.signers.as_slice(), + certificate.aggregate_verification_key.try_into().unwrap(), + signed_entity_type, + previous_cert_identifier, + ) }; Ok(expected_certificate) @@ -601,30 +622,22 @@ impl RuntimeTester { &mut self, certificate_hash: &str, ) -> StdResult { - let cert_identifier = match self + let certificate = self .dependencies - .signed_entity_storer - .get_signed_entity_by_certificate_id(certificate_hash) + .certificate_repository + .get_certificate::(certificate_hash) .await - .with_context(|| "Querying signed entity should not fail")? - { - Some(record) => ExpectedCertificate::identifier(&record.signed_entity_type), - None => { - // Certificate is a genesis certificate - let genesis_certificate = self - .dependencies - .certifier_service - .get_certificate_by_hash(certificate_hash) - .await - .with_context(|| "Querying genesis certificate should not fail")? - .ok_or(anyhow!( - "A genesis certificate should exist with hash {}", - certificate_hash - ))?; - ExpectedCertificate::genesis_identifier(genesis_certificate.epoch) - } - }; + .with_context(|| format!("Failed to query certificate with hash {certificate_hash}"))? + .ok_or(anyhow!( + "A certificate should exist with hash {}", + certificate_hash + ))?; + let cert_identifier = if certificate.is_genesis() { + ExpectedCertificate::genesis_identifier(certificate.epoch) + } else { + ExpectedCertificate::identifier(&certificate.signed_entity_type()) + }; Ok(cert_identifier) } @@ -634,22 +647,25 @@ impl RuntimeTester { &self, signed_entity_type_expected: &SignedEntityType, ) -> StdResult<()> { - let mut max_iteration = 100; - while !self - .observer - .is_last_signed_entity(signed_entity_type_expected) - .await? - { - max_iteration -= 1; - if max_iteration <= 0 { - return Err(anyhow!( - "Signed entity not found: {signed_entity_type_expected}" - )); - } - tokio::time::sleep(Duration::from_millis(1)).await; - } + async_wait!( + max_iter:100, sleep_ms:1, + condition: !self.observer.is_last_signed_entity(signed_entity_type_expected).await?, + error_msg: "Signed entity not found: {signed_entity_type_expected}" + ) + } - Ok(()) + /// Wait until the last stored certificate of the given signed entity type + /// corresponds to the expected signed entity type + pub async fn wait_until_certificate( + &self, + certificate_signed_entity_type: &SignedEntityType, + ) -> StdResult<()> { + async_wait!( + max_iter:100, sleep_ms:1, + condition: self.observer.get_last_certificate().await?.signed_entity_type() + != *certificate_signed_entity_type, + error_msg: "Certificate not found for signed entity: {certificate_signed_entity_type}" + ) } /// Returns the runtime cycle success and total metrics since startup diff --git a/mithril-aggregator/tests/test_extensions/utilities.rs b/mithril-aggregator/tests/test_extensions/utilities.rs index 3ac8c00b95d..c6921470b6a 100644 --- a/mithril-aggregator/tests/test_extensions/utilities.rs +++ b/mithril-aggregator/tests/test_extensions/utilities.rs @@ -29,3 +29,21 @@ macro_rules! comment { test_extensions::utilities::comment(format!($($comment)*)); }}; } + +#[macro_export] +macro_rules! async_wait { + ( max_iter:$max_iter:expr, sleep_ms:$sleep_ms:expr, condition:$condition:expr, error_msg:$($error_msg:tt)* + ) => {{ + let mut max_iteration: usize = $max_iter; + while $condition { + max_iteration -= 1; + if max_iteration == 0 { + return Err(anyhow::anyhow!($($error_msg)*)); + } + tokio::time::sleep(Duration::from_millis($sleep_ms)).await; + } + + Ok(()) + }}; +} +pub use async_wait; From 3d4734d76a01c38e6c64d6fb0be25c1e5f170728 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Fri, 4 Jul 2025 19:20:32 +0200 Subject: [PATCH 23/28] test(aggregator): update `create_certificate_follower` integration test --- .../tests/create_certificate_follower.rs | 171 ++++++------------ .../leader_aggregator_http_server.rs | 34 +++- 2 files changed, 86 insertions(+), 119 deletions(-) diff --git a/mithril-aggregator/tests/create_certificate_follower.rs b/mithril-aggregator/tests/create_certificate_follower.rs index 548009cd272..60f2562ddb3 100644 --- a/mithril-aggregator/tests/create_certificate_follower.rs +++ b/mithril-aggregator/tests/create_certificate_follower.rs @@ -5,9 +5,8 @@ use std::{collections::HashMap, ops::Range}; use mithril_aggregator::ServeCommandConfiguration; use mithril_common::{ entities::{ - BlockNumber, CardanoTransactionsSigningConfig, ChainPoint, Epoch, ProtocolParameters, - SignedEntityType, SignedEntityTypeDiscriminants, SlotNumber, StakeDistributionParty, - TimePoint, + BlockNumber, ChainPoint, Epoch, ProtocolParameters, SignedEntityType, + SignedEntityTypeDiscriminants, SlotNumber, StakeDistributionParty, TimePoint, }, temp_dir, test_utils::{ @@ -86,7 +85,7 @@ async fn create_certificate_follower() { phi_f: 0.95, }; let fixtures = - EpochFixturesMapBuilder::build_fixtures_sequence(1..10, protocol_parameters.clone()); + EpochFixturesMapBuilder::build_fixtures_sequence(1..6, protocol_parameters.clone()); let epoch_fixtures_map = EpochFixturesMapBuilder::build_epoch_fixtures_map(&fixtures); let start_time_point = TimePoint { epoch: Epoch(1), @@ -100,11 +99,9 @@ async fn create_certificate_follower() { let leader_configuration = ServeCommandConfiguration { protocol_parameters: protocol_parameters.clone(), data_stores_directory: get_test_dir("create_certificate_leader"), - cardano_transactions_signing_config: CardanoTransactionsSigningConfig { - security_parameter: BlockNumber(0), - step: BlockNumber(30), - }, - signed_entity_types: Some(SignedEntityTypeDiscriminants::CardanoDatabase.to_string()), + signed_entity_types: Some( + SignedEntityTypeDiscriminants::CardanoStakeDistribution.to_string(), + ), ..ServeCommandConfiguration::new_sample(temp_dir!()) }; let mut leader_tester = @@ -127,6 +124,7 @@ async fn create_certificate_follower() { "Epoch 1: - the leader aggregator registers the first signers - the leader aggregator can't transition from 'Idle' to 'Ready' + - the follower can't synchronise its chain with the leader aggregator - the follower aggregator can't transition from 'Idle' to 'Ready' " ); @@ -211,7 +209,8 @@ async fn create_certificate_follower() { "Epoch 3: - the leader aggregator produces a new certificate - the follower aggregator synchronizes signers from the leader aggregator - - the follower aggregator can't transition from 'Idle' to 'Ready' + - the follower synchronise its chain with the leader aggregator + - the follower aggregator can transition from 'Idle' to 'Ready' " ); let epoch_fixture = &epoch_fixtures_map[&Epoch(3)]; @@ -233,10 +232,6 @@ async fn create_certificate_follower() { cycle!(leader_tester, "ready"); cycle!(leader_tester, "signing"); - comment!("Follower: change the epoch after leader"); - follower_tester.increase_epoch().await.unwrap(); - cycle_err!(follower_tester, "idle"); - comment!("Leader: register signers"); leader_tester .register_signers(&epoch_fixture.registering.signers_fixture()) @@ -256,27 +251,36 @@ async fn create_certificate_follower() { comment!("Leader: state machine should issue a certificate for the MithrilStakeDistribution"); cycle!(leader_tester, "signing"); - assert_last_certificate_eq!( - leader_tester, - ExpectedCertificate::new( - Epoch(3), - StakeDistributionParty::from_signers( - epoch_fixture.current_signing.unwrap().signers_with_stake() - ) - .as_slice(), - epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), - SignedEntityType::MithrilStakeDistribution(Epoch(3)), - ExpectedCertificate::genesis_identifier(Epoch(2)), + let expected_certificate_on_both_aggregator = ExpectedCertificate::new( + Epoch(3), + StakeDistributionParty::from_signers( + epoch_fixture.current_signing.unwrap().signers_with_stake(), ) + .as_slice(), + epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), + SignedEntityType::MithrilStakeDistribution(Epoch(3)), + ExpectedCertificate::genesis_identifier(Epoch(2)), + ); + assert_last_certificate_eq!(leader_tester, expected_certificate_on_both_aggregator); + cycle_err!(leader_tester, "signing"); + + comment!( + "Follower: change the epoch after leader made a certificate and synchronise certificate chain" + ); + follower_tester.increase_epoch().await.unwrap(); + cycle_err!(follower_tester, "idle"); + assert_last_certificate_eq!( + follower_tester, synchronised_from_leader => expected_certificate_on_both_aggregator ); + cycle!(follower_tester, "ready"); cycle_err!(leader_tester, "signing"); comment!( "Epoch 4: - the leader aggregator produces a new certificate - the follower aggregator synchronizes signers from the leader aggregator - - the follower aggregator bootstraps its genesis certificate - - the follower aggregator can't transition from 'Idle' to 'Ready'" + - the follower aggregator produces a new certificate + - the follower aggregator new certificate uses the same avk as the leader aggregator's new certificate" ); let epoch_fixture = &epoch_fixtures_map[&Epoch(4)]; @@ -299,13 +303,8 @@ async fn create_certificate_follower() { comment!("Follower: change the epoch after leader"); follower_tester.increase_epoch().await.unwrap(); - cycle_err!(follower_tester, "idle"); - - comment!("Follower: bootstrap the genesis certificate"); - follower_tester - .register_genesis_certificate(epoch_fixture.next_signing.unwrap()) - .await - .unwrap(); + cycle!(follower_tester, "idle"); + cycle!(follower_tester, "ready"); comment!("Leader: register signers"); leader_tester @@ -326,69 +325,19 @@ async fn create_certificate_follower() { comment!("Leader: state machine should issue a certificate for the MithrilStakeDistribution"); cycle!(leader_tester, "ready"); - assert_last_certificate_eq!( - leader_tester, - ExpectedCertificate::new( - Epoch(4), - StakeDistributionParty::from_signers( - epoch_fixture.current_signing.unwrap().signers_with_stake() - ) - .as_slice(), - epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), - SignedEntityType::MithrilStakeDistribution(Epoch(4)), - ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(3))), + let leader_expected_certificate = ExpectedCertificate::new( + Epoch(4), + StakeDistributionParty::from_signers( + epoch_fixture.current_signing.unwrap().signers_with_stake(), ) + .as_slice(), + epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), + SignedEntityType::MithrilStakeDistribution(Epoch(4)), + ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(3))), ); + assert_last_certificate_eq!(leader_tester, leader_expected_certificate); cycle!(leader_tester, "signing"); - comment!( - "Epoch 5: - - the leader aggregator produces a new certificate - - the follower aggregator produces a new certificate - - the follower aggregator new certificate uses the same avk as the leader aggregator's new certificate - "); - let epoch_fixture = &epoch_fixtures_map[&Epoch(5)]; - - comment!("Leader: update stake distribution source"); - leader_tester - .update_stake_distribution(epoch_fixture.registering.stake_distribution()) - .await - .unwrap(); - - comment!("Follower: update stake distribution source"); - follower_tester - .update_stake_distribution(epoch_fixture.registering.stake_distribution()) - .await - .unwrap(); - - comment!("Follower: change the epoch before leader"); - follower_tester.increase_epoch().await.unwrap(); - cycle!(follower_tester, "idle"); - - comment!("Leader: change the epoch"); - leader_tester.increase_epoch().await.unwrap(); - cycle!(leader_tester, "idle"); - cycle!(leader_tester, "ready"); - - comment!("Follower: change the epoch after leader"); - cycle!(follower_tester, "ready"); - - comment!("Leader: register signers"); - leader_tester - .register_signers(&epoch_fixture.registering.signers_fixture()) - .await - .unwrap(); - cycle!(leader_tester, "signing"); - - comment!("Leader: signers send their single signature"); - leader_tester - .send_single_signatures( - SignedEntityTypeDiscriminants::MithrilStakeDistribution, - &epoch_fixture.current_signing.unwrap().signers_fixture(), - ) - .await - .unwrap(); - comment!("Follower: signers send their single signature"); cycle!(follower_tester, "signing"); follower_tester @@ -399,31 +348,17 @@ async fn create_certificate_follower() { .await .unwrap(); - comment!("Leader: state machine should issue a certificate for the MithrilStakeDistribution"); - cycle!(leader_tester, "ready"); - let leader_expected_certificate = ExpectedCertificate::new( - Epoch(5), - StakeDistributionParty::from_signers( - epoch_fixture.current_signing.unwrap().signers_with_stake(), - ) - .as_slice(), - epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), - SignedEntityType::MithrilStakeDistribution(Epoch(5)), - ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(4))), - ); - assert_last_certificate_eq!(leader_tester, leader_expected_certificate); - comment!("Follower: state machine should issue a certificate for the MithrilStakeDistribution"); cycle!(follower_tester, "ready"); let follower_expected_certificate = ExpectedCertificate::new( - Epoch(5), + Epoch(4), StakeDistributionParty::from_signers( epoch_fixture.current_signing.unwrap().signers_with_stake(), ) .as_slice(), epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), - SignedEntityType::MithrilStakeDistribution(Epoch(5)), - ExpectedCertificate::genesis_identifier(Epoch(4)), + SignedEntityType::MithrilStakeDistribution(Epoch(4)), + ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(3))), ); assert_last_certificate_eq!(follower_tester, follower_expected_certificate); let expected_avk = epoch_fixture @@ -435,12 +370,12 @@ async fn create_certificate_follower() { assert_eq!(expected_avk, follower_expected_certificate.avk()); comment!( - "Epoch 6: + "Epoch 5: - the leader aggregator produces a new certificate - the follower aggregator produces a new certificate - the follower aggregator new certificate uses the same avk as the leader aggregator's new certificate "); - let epoch_fixture = &epoch_fixtures_map[&Epoch(6)]; + let epoch_fixture = &epoch_fixtures_map[&Epoch(5)]; comment!("Leader: update stake distribution source"); leader_tester @@ -493,28 +428,28 @@ async fn create_certificate_follower() { comment!("Leader: state machine should issue a certificate for the MithrilStakeDistribution"); cycle!(leader_tester, "ready"); let leader_expected_certificate = ExpectedCertificate::new( - Epoch(6), + Epoch(5), StakeDistributionParty::from_signers( epoch_fixture.current_signing.unwrap().signers_with_stake(), ) .as_slice(), epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), - SignedEntityType::MithrilStakeDistribution(Epoch(6)), - ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(5))), + SignedEntityType::MithrilStakeDistribution(Epoch(5)), + ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(4))), ); assert_last_certificate_eq!(leader_tester, leader_expected_certificate); comment!("Follower: state machine should issue a certificate for the MithrilStakeDistribution"); cycle!(follower_tester, "ready"); let follower_expected_certificate = ExpectedCertificate::new( - Epoch(6), + Epoch(5), StakeDistributionParty::from_signers( epoch_fixture.current_signing.unwrap().signers_with_stake(), ) .as_slice(), epoch_fixture.current_signing.unwrap().compute_and_encode_avk(), - SignedEntityType::MithrilStakeDistribution(Epoch(6)), - ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(5))), + SignedEntityType::MithrilStakeDistribution(Epoch(5)), + ExpectedCertificate::identifier(&SignedEntityType::MithrilStakeDistribution(Epoch(4))), ); assert_last_certificate_eq!(follower_tester, follower_expected_certificate); let expected_avk = epoch_fixture diff --git a/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs b/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs index f128e539df2..72a387aa284 100644 --- a/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs +++ b/mithril-aggregator/tests/test_extensions/leader_aggregator_http_server.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use axum::{ Json, Router, - extract::State, + extract::{Path, State}, http::StatusCode, response::{IntoResponse, Response}, routing::get, @@ -30,6 +30,9 @@ impl LeaderAggregatorHttpServer { }; let router = Router::new() .route("/epoch-settings", get(epoch_settings)) + .route("/certificates", get(certificates_list)) + .route("/certificate/genesis", get(certificate_last_genesis)) + .route("/certificate/{hash}", get(certificate_by_hash)) .with_state(state); let server = TestServer::builder().http_transport().build(router)?; @@ -66,3 +69,32 @@ async fn epoch_settings(state: State) -> Response { Err(err) => internal_server_error(err).into_response(), } } + +async fn certificates_list(state: State) -> Response { + slog::debug!(state.logger, "/certificates"); + match state.message_service.get_certificate_list_message(5).await { + Ok(message) => (StatusCode::OK, Json(message)).into_response(), + Err(err) => internal_server_error(err).into_response(), + } +} + +async fn certificate_last_genesis(state: State) -> Response { + slog::debug!(state.logger, "/certificate/genesis"); + match state.message_service.get_latest_genesis_certificate_message().await { + Ok(Some(message)) => (StatusCode::OK, Json(message)).into_response(), + Ok(None) => StatusCode::NOT_FOUND.into_response(), + Err(err) => internal_server_error(err).into_response(), + } +} + +async fn certificate_by_hash( + Path(hash): Path, + state: State, +) -> Response { + slog::debug!(state.logger, "/certificate/{hash}"); + match state.message_service.get_certificate_message(&hash).await { + Ok(Some(message)) => (StatusCode::OK, Json(message)).into_response(), + Ok(None) => StatusCode::NOT_FOUND.into_response(), + Err(err) => internal_server_error(err).into_response(), + } +} From 9bebed2b30c1513a6563af1647011229b20b77ca Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 8 Jul 2025 14:58:27 +0200 Subject: [PATCH 24/28] test: update e2e to follower auto-sync --- .../mithril-end-to-end/src/assertions/exec.rs | 11 -- .../mithril-end-to-end/src/end_to_end_spec.rs | 48 +++-- .../src/mithril/infrastructure.rs | 168 +++++++++++------- .../mithril-end-to-end/src/run_only.rs | 44 ++--- 4 files changed, 155 insertions(+), 116 deletions(-) diff --git a/mithril-test-lab/mithril-end-to-end/src/assertions/exec.rs b/mithril-test-lab/mithril-end-to-end/src/assertions/exec.rs index e0a2c4eced4..3b78bee7ab2 100644 --- a/mithril-test-lab/mithril-end-to-end/src/assertions/exec.rs +++ b/mithril-test-lab/mithril-end-to-end/src/assertions/exec.rs @@ -7,17 +7,6 @@ use slog_scope::info; pub async fn bootstrap_genesis_certificate(aggregator: &Aggregator) -> StdResult<()> { info!("Bootstrap genesis certificate"; "aggregator" => &aggregator.name()); - - // A follower aggregator needs to wait few cycles of the state machine to be able to bootstrap - // This should be removed when the aggregator is able to synchronize its certificate chain from another aggregator - if !aggregator.is_first() { - const CYCLES_WAIT_FOLLOWER: u64 = 3; - tokio::time::sleep(std::time::Duration::from_millis( - CYCLES_WAIT_FOLLOWER * aggregator.mithril_run_interval() as u64, - )) - .await; - } - info!("> stopping aggregator"; "aggregator" => &aggregator.name()); aggregator.stop().await?; info!("> bootstrapping genesis using signers registered two epochs ago..."; "aggregator" => &aggregator.name()); diff --git a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs index bbc65aab9c6..cc7298a5b5b 100644 --- a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs +++ b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs @@ -1,5 +1,6 @@ use std::sync::Arc; +use slog_scope::info; use tokio::task::JoinSet; use mithril_common::{ @@ -54,6 +55,15 @@ impl Spec { // As we get closer to the tip of the chain when signing, we'll be able to relax this constraint. assertions::transfer_funds(spec.infrastructure.devnet()).await?; + info!("bootstrapping leader aggregator"); + spec.bootstrap_leader_aggregator(&spec.infrastructure).await?; + + info!("Starting followers"); + for follower_aggregator in spec.infrastructure.follower_aggregators() { + follower_aggregator.serve().await?; + } + + info!("Running scenarios"); for index in 0..spec.infrastructure.aggregators().len() { let spec_clone = spec.clone(); join_set.spawn(async move { @@ -72,44 +82,54 @@ impl Spec { Ok(()) } - pub async fn run_scenario( + pub async fn bootstrap_leader_aggregator( &self, - aggregator: &Aggregator, infrastructure: &MithrilInfrastructure, ) -> StdResult<()> { - assertions::wait_for_enough_immutable(aggregator).await?; - let chain_observer = aggregator.chain_observer(); + let leader_aggregator = infrastructure.leader_aggregator(); + + assertions::wait_for_enough_immutable(leader_aggregator).await?; + let chain_observer = leader_aggregator.chain_observer(); let start_epoch = chain_observer.get_current_epoch().await?.unwrap_or_default(); // Wait 4 epochs after start epoch for the aggregator to be able to bootstrap a genesis certificate let mut target_epoch = start_epoch + 4; assertions::wait_for_aggregator_at_target_epoch( - aggregator, + leader_aggregator, target_epoch, "minimal epoch for the aggregator to be able to bootstrap genesis certificate" .to_string(), ) .await?; - assertions::bootstrap_genesis_certificate(aggregator).await?; - assertions::wait_for_epoch_settings(aggregator).await?; + assertions::bootstrap_genesis_certificate(leader_aggregator).await?; + assertions::wait_for_epoch_settings(leader_aggregator).await?; // Wait 2 epochs before changing stake distribution, so that we use at least one original stake distribution target_epoch += 2; assertions::wait_for_aggregator_at_target_epoch( - aggregator, + leader_aggregator, target_epoch, "epoch after which the stake distribution will change".to_string(), ) .await?; - if aggregator.is_first() { - // Delegate some stakes to pools - let delegation_round = 1; - assertions::delegate_stakes_to_pools(infrastructure.devnet(), delegation_round).await?; - } + // Delegate some stakes to pools + let delegation_round = 1; + assertions::delegate_stakes_to_pools(infrastructure.devnet(), delegation_round).await?; + + Ok(()) + } + + pub async fn run_scenario( + &self, + aggregator: &Aggregator, + infrastructure: &MithrilInfrastructure, + ) -> StdResult<()> { + let chain_observer = aggregator.chain_observer(); + let start_epoch = chain_observer.get_current_epoch().await?.unwrap_or_default(); // Wait 2 epochs before changing protocol parameters - target_epoch += 2; + let mut target_epoch = start_epoch + 2; assertions::wait_for_aggregator_at_target_epoch( aggregator, target_epoch, diff --git a/mithril-test-lab/mithril-end-to-end/src/mithril/infrastructure.rs b/mithril-test-lab/mithril-end-to-end/src/mithril/infrastructure.rs index f8d5df5ec86..5db25573fe2 100644 --- a/mithril-test-lab/mithril-end-to-end/src/mithril/infrastructure.rs +++ b/mithril-test-lab/mithril-end-to-end/src/mithril/infrastructure.rs @@ -103,17 +103,22 @@ impl MithrilInfrastructure { let relay_signer_registration_mode = &config.relay_signer_registration_mode; let relay_signature_registration_mode = &config.relay_signature_registration_mode; - let aggregators = - Self::start_aggregators(config, aggregator_cardano_nodes, chain_observer_type).await?; - let aggregator_endpoints = aggregators + let (leader_aggregator, follower_aggregators) = + Self::prepare_aggregators(config, aggregator_cardano_nodes, chain_observer_type) + .await?; + + Self::register_startup_era(&leader_aggregator, config).await?; + leader_aggregator.serve().await?; + + let follower_aggregator_endpoints = follower_aggregators .iter() .map(|aggregator| aggregator.endpoint()) .collect::>(); - let leader_aggregator_endpoint = aggregator_endpoints[0].to_owned(); let (relay_aggregators, relay_signers, relay_passives) = Self::start_relays( config, - &aggregator_endpoints, + leader_aggregator.endpoint(), + &follower_aggregator_endpoints, &signer_party_ids, relay_signer_registration_mode.to_owned(), relay_signature_registration_mode.to_owned(), @@ -121,7 +126,7 @@ impl MithrilInfrastructure { let signers = Self::start_signers( config, - leader_aggregator_endpoint, + leader_aggregator.endpoint(), signer_cardano_nodes, &relay_signers, ) @@ -132,11 +137,14 @@ impl MithrilInfrastructure { CardanoNetwork::DevNet(DEVNET_MAGIC_ID), )); + let mut all_aggregators = vec![leader_aggregator]; + all_aggregators.extend(follower_aggregators); + Ok(Self { bin_dir: config.bin_dir.to_path_buf(), artifacts_dir: config.artifacts_dir.to_path_buf(), devnet: config.devnet.clone(), - aggregators, + aggregators: all_aggregators, signers, relay_aggregators, relay_signers, @@ -176,8 +184,13 @@ impl MithrilInfrastructure { + 1; if self.era_reader_adapter == "cardano-chain" { let devnet = self.devnet.clone(); - assertions::register_era_marker(self.aggregator(0), &devnet, next_era, next_era_epoch) - .await?; + assertions::register_era_marker( + self.leader_aggregator(), + &devnet, + next_era, + next_era_epoch, + ) + .await?; } let mut current_era = self.current_era.write().await; *current_era = next_era.to_owned(); @@ -185,69 +198,80 @@ impl MithrilInfrastructure { Ok(()) } - async fn start_aggregators( + async fn prepare_aggregators( config: &MithrilInfrastructureConfig, - pool_nodes: &[FullNode], + full_nodes: &[FullNode], chain_observer_type: &str, - ) -> StdResult> { - let mut aggregators = vec![]; - let mut leader_aggregator_endpoint: Option = None; - for (index, full_node) in pool_nodes.iter().enumerate() { - let aggregator_name = Aggregator::name_suffix(index); - let aggregator_artifacts_dir = config - .artifacts_dir - .join(format!("mithril-aggregator-{aggregator_name}")); - let aggregator_store_dir = - config.store_dir.join(format!("aggregator-{aggregator_name}")); - let aggregator = Aggregator::new(&AggregatorConfig { - index, - name: &aggregator_name, - server_port: config.server_port + index as u64, + ) -> StdResult<(Aggregator, Vec)> { + let [leader_node, follower_nodes @ ..] = full_nodes else { + panic!("Can't prepare Aggregators: No full nodes found"); + }; + let leader_aggregator = + Self::prepare_aggregator(0, leader_node, config, chain_observer_type, None).await?; + + let mut follower_aggregators = vec![]; + for (index, full_node) in follower_nodes.iter().enumerate() { + let aggregator = Self::prepare_aggregator( + index + 1, full_node, - cardano_cli_path: &config.devnet.cardano_cli_path(), - work_dir: &config.work_dir, - store_dir: &aggregator_store_dir, - artifacts_dir: &aggregator_artifacts_dir, - bin_dir: &config.bin_dir, - cardano_node_version: &config.cardano_node_version, - mithril_run_interval: config.mithril_run_interval, - mithril_era: &config.mithril_era, - mithril_era_reader_adapter: &config.mithril_era_reader_adapter, - mithril_era_marker_address: &config.devnet.mithril_era_marker_address()?, - signed_entity_types: &config.signed_entity_types, + config, chain_observer_type, - leader_aggregator_endpoint: &leader_aggregator_endpoint.clone(), - })?; - - aggregator - .set_protocol_parameters(&ProtocolParameters { - k: 70, - m: 105, - phi_f: 0.95, - }) - .await; - - if leader_aggregator_endpoint.is_none() - && config.has_leader_follower_signer_registration() - { - leader_aggregator_endpoint = Some(aggregator.endpoint()); - } - - aggregators.push(aggregator); + Some(leader_aggregator.endpoint()), + ) + .await?; + follower_aggregators.push(aggregator); } - Self::register_startup_era(&aggregators[0], config).await?; - - for aggregator in &aggregators { - aggregator.serve().await?; - } + Ok((leader_aggregator, follower_aggregators)) + } - Ok(aggregators) + async fn prepare_aggregator( + index: usize, + full_node: &FullNode, + config: &MithrilInfrastructureConfig, + chain_observer_type: &str, + leader_aggregator_endpoint: Option, + ) -> StdResult { + let aggregator_name = Aggregator::name_suffix(index); + let aggregator_artifacts_dir = config + .artifacts_dir + .join(format!("mithril-aggregator-{aggregator_name}")); + let aggregator_store_dir = config.store_dir.join(format!("aggregator-{aggregator_name}")); + let aggregator = Aggregator::new(&AggregatorConfig { + index, + name: &aggregator_name, + server_port: config.server_port + index as u64, + full_node, + cardano_cli_path: &config.devnet.cardano_cli_path(), + work_dir: &config.work_dir, + store_dir: &aggregator_store_dir, + artifacts_dir: &aggregator_artifacts_dir, + bin_dir: &config.bin_dir, + cardano_node_version: &config.cardano_node_version, + mithril_run_interval: config.mithril_run_interval, + mithril_era: &config.mithril_era, + mithril_era_reader_adapter: &config.mithril_era_reader_adapter, + mithril_era_marker_address: &config.devnet.mithril_era_marker_address()?, + signed_entity_types: &config.signed_entity_types, + chain_observer_type, + leader_aggregator_endpoint: &leader_aggregator_endpoint, + })?; + + aggregator + .set_protocol_parameters(&ProtocolParameters { + k: 70, + m: 105, + phi_f: 0.95, + }) + .await; + + Ok(aggregator) } fn start_relays( config: &MithrilInfrastructureConfig, - aggregator_endpoints: &[String], + leader_aggregator_endpoint: String, + follower_aggregator_endpoints: &[String], signers_party_ids: &[PartyId], relay_signer_registration_mode: String, relay_signature_registration_mode: String, @@ -255,11 +279,15 @@ impl MithrilInfrastructure { if !config.use_relays { return Ok((vec![], vec![], vec![])); } + let aggregator_endpoints = [ + vec![leader_aggregator_endpoint.clone()], + follower_aggregator_endpoints.to_vec(), + ] + .concat(); let mut relay_aggregators: Vec = vec![]; let mut relay_signers: Vec = vec![]; let mut relay_passives: Vec = vec![]; - let leader_aggregator_endpoint = &aggregator_endpoints[0]; info!("Starting the Mithril infrastructure in P2P mode (experimental)"); @@ -287,7 +315,7 @@ impl MithrilInfrastructure { dial_to: bootstrap_peer_addr.clone(), relay_signer_registration_mode: relay_signer_registration_mode.clone(), relay_signature_registration_mode: relay_signature_registration_mode.clone(), - aggregator_endpoint: leader_aggregator_endpoint, + aggregator_endpoint: &leader_aggregator_endpoint, party_id: party_id.clone(), work_dir: &config.work_dir, bin_dir: &config.bin_dir, @@ -377,7 +405,7 @@ impl MithrilInfrastructure { signer.stop().await?; } - for aggregator in &self.aggregators { + for aggregator in self.aggregators() { aggregator.stop().await?; } @@ -396,6 +424,18 @@ impl MithrilInfrastructure { &self.aggregators[index] } + pub fn leader_aggregator(&self) -> &Aggregator { + &self.aggregators[0] + } + + pub fn follower_aggregators(&self) -> &[Aggregator] { + &self.aggregators[1..] + } + + pub fn follower_aggregator(&self, index: usize) -> &Aggregator { + &self.aggregators[index + 1] + } + pub fn signers(&self) -> &[Signer] { &self.signers } diff --git a/mithril-test-lab/mithril-end-to-end/src/run_only.rs b/mithril-test-lab/mithril-end-to-end/src/run_only.rs index 3774a7f8739..8c327d39054 100644 --- a/mithril-test-lab/mithril-end-to-end/src/run_only.rs +++ b/mithril-test-lab/mithril-end-to-end/src/run_only.rs @@ -1,10 +1,10 @@ use std::sync::Arc; -use tokio::task::JoinSet; +use slog_scope::info; use mithril_common::StdResult; -use crate::{Aggregator, MithrilInfrastructure, assertions}; +use crate::{MithrilInfrastructure, assertions}; pub struct RunOnly { pub infrastructure: Arc, @@ -17,51 +17,41 @@ impl RunOnly { pub async fn run(self) -> StdResult<()> { let run_only = Arc::new(self); - let mut join_set = JoinSet::new(); + info!("bootstrapping leader aggregator"); + run_only.bootstrap_leader_aggregator(&run_only.infrastructure).await?; - for index in 0..run_only.infrastructure.aggregators().len() { - let run_only_clone = run_only.clone(); - join_set.spawn(async move { - let infrastructure = &run_only_clone.infrastructure; - - run_only_clone - .bootstrap_aggregator(infrastructure.aggregator(index), infrastructure) - .await - }); - } - - while let Some(res) = join_set.join_next().await { - res??; + info!("Starting followers"); + for follower_aggregator in run_only.infrastructure.follower_aggregators() { + follower_aggregator.serve().await?; } Ok(()) } - pub async fn bootstrap_aggregator( + pub async fn bootstrap_leader_aggregator( &self, - aggregator: &Aggregator, infrastructure: &MithrilInfrastructure, ) -> StdResult<()> { - assertions::wait_for_enough_immutable(aggregator).await?; - let chain_observer = aggregator.chain_observer(); + let leader_aggregator = infrastructure.leader_aggregator(); + + assertions::wait_for_enough_immutable(leader_aggregator).await?; + let chain_observer = leader_aggregator.chain_observer(); let start_epoch = chain_observer.get_current_epoch().await?.unwrap_or_default(); // Wait 3 epochs after start epoch for the aggregator to be able to bootstrap a genesis certificate let target_epoch = start_epoch + 3; assertions::wait_for_aggregator_at_target_epoch( - aggregator, + leader_aggregator, target_epoch, "minimal epoch for the aggregator to be able to bootstrap genesis certificate" .to_string(), ) .await?; - assertions::bootstrap_genesis_certificate(aggregator).await?; - assertions::wait_for_epoch_settings(aggregator).await?; + assertions::bootstrap_genesis_certificate(leader_aggregator).await?; + assertions::wait_for_epoch_settings(leader_aggregator).await?; - if aggregator.is_first() { - // Transfer some funds on the devnet to have some Cardano transactions to sign - assertions::transfer_funds(infrastructure.devnet()).await?; - } + // Transfer some funds on the devnet to have some Cardano transactions to sign + assertions::transfer_funds(infrastructure.devnet()).await?; Ok(()) } From 1796fdbcfa03c7bd8fd7b009e52af05e08cc3a82 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 9 Jul 2025 17:34:49 +0200 Subject: [PATCH 25/28] 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 --- .../src/dependency_injection/builder/mod.rs | 8 +- .../builder/protocol/certificates.rs | 14 +- .../dependency_injection/containers/serve.rs | 4 +- mithril-aggregator/src/runtime/runner.rs | 4 +- .../src/services/aggregator_client.rs | 14 +- .../interface.rs | 0 .../mod.rs | 4 +- .../noop.rs | 4 +- .../synchronizer_service.rs} | 124 +++++++----------- mithril-aggregator/src/services/mod.rs | 4 +- mithril-aggregator/src/tools/mocks.rs | 36 ++++- .../tests/create_certificate_follower.rs | 8 +- .../tests/test_extensions/runtime_tester.rs | 2 +- .../mithril-end-to-end/src/end_to_end_spec.rs | 2 +- .../mithril-end-to-end/src/run_only.rs | 2 +- 15 files changed, 118 insertions(+), 112 deletions(-) rename mithril-aggregator/src/services/{certificate_chain_synchroniser => certificate_chain_synchronizer}/interface.rs (100%) rename mithril-aggregator/src/services/{certificate_chain_synchroniser => certificate_chain_synchronizer}/mod.rs (52%) rename mithril-aggregator/src/services/{certificate_chain_synchroniser => certificate_chain_synchronizer}/noop.rs (83%) rename mithril-aggregator/src/services/{certificate_chain_synchroniser/synchroniser_service.rs => certificate_chain_synchronizer/synchronizer_service.rs} (88%) diff --git a/mithril-aggregator/src/dependency_injection/builder/mod.rs b/mithril-aggregator/src/dependency_injection/builder/mod.rs index f0d5a7083f0..d14398c7d1d 100644 --- a/mithril-aggregator/src/dependency_injection/builder/mod.rs +++ b/mithril-aggregator/src/dependency_injection/builder/mod.rs @@ -185,8 +185,8 @@ pub struct DependenciesBuilder { /// Genesis signature verifier service. pub genesis_verifier: Option>, - /// Certificate chain synchroniser service - pub certificate_chain_synchroniser: Option>, + /// Certificate chain synchronizer service + pub certificate_chain_synchronizer: Option>, /// Mithril signer registration leader service pub mithril_signer_registration_leader: Option>, @@ -315,7 +315,7 @@ impl DependenciesBuilder { snapshotter: None, certificate_verifier: None, genesis_verifier: None, - certificate_chain_synchroniser: None, + certificate_chain_synchronizer: None, mithril_signer_registration_leader: None, mithril_signer_registration_follower: None, signer_registerer: None, @@ -378,7 +378,7 @@ impl DependenciesBuilder { verification_key_store: self.get_verification_key_store().await?, epoch_settings_storer: self.get_epoch_settings_store().await?, chain_observer: self.get_chain_observer().await?, - certificate_chain_synchroniser: self.get_certificate_chain_synchroniser().await?, + certificate_chain_synchronizer: self.get_certificate_chain_synchronizer().await?, signer_registerer: self.get_signer_registerer().await?, signer_synchronizer: self.get_signer_synchronizer().await?, signer_registration_round_opener: self.get_signer_registration_round_opener().await?, diff --git a/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs b/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs index 8ed9e2bdde2..aa8ec7851a4 100644 --- a/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs +++ b/mithril-aggregator/src/dependency_injection/builder/protocol/certificates.rs @@ -11,7 +11,7 @@ use crate::dependency_injection::{DependenciesBuilder, DependenciesBuilderError, use crate::get_dependency; use crate::services::{ BufferedCertifierService, CertificateChainSynchronizer, CertifierService, - MithrilCertificateChainSynchroniserNoop, MithrilCertificateChainSynchronizer, + MithrilCertificateChainSynchronizer, MithrilCertificateChainSynchronizerNoop, MithrilCertifierService, MithrilSignerRegistrationFollower, SignerSynchronizer, }; use crate::{ @@ -85,10 +85,10 @@ impl DependenciesBuilder { get_dependency!(self.multi_signer) } - async fn build_certificate_chain_synchroniser( + async fn build_certificate_chain_synchronizer( &mut self, ) -> Result> { - let synchroniser: Arc = + let synchronizer: Arc = if self.configuration.is_follower_aggregator() { let leader_aggregator_client = self.get_leader_aggregator_client().await?; let verifier = Arc::new(MithrilCertificateVerifier::new( @@ -105,17 +105,17 @@ impl DependenciesBuilder { self.root_logger(), )) } else { - Arc::new(MithrilCertificateChainSynchroniserNoop) + Arc::new(MithrilCertificateChainSynchronizerNoop) }; - Ok(synchroniser) + Ok(synchronizer) } /// [CertificateChainSynchronizer] service - pub async fn get_certificate_chain_synchroniser( + pub async fn get_certificate_chain_synchronizer( &mut self, ) -> Result> { - get_dependency!(self.certificate_chain_synchroniser) + get_dependency!(self.certificate_chain_synchronizer) } async fn build_certificate_verifier(&mut self) -> Result> { diff --git a/mithril-aggregator/src/dependency_injection/containers/serve.rs b/mithril-aggregator/src/dependency_injection/containers/serve.rs index 63fb9231f7c..000902c84ca 100644 --- a/mithril-aggregator/src/dependency_injection/containers/serve.rs +++ b/mithril-aggregator/src/dependency_injection/containers/serve.rs @@ -58,8 +58,8 @@ pub struct ServeCommandDependenciesContainer { /// Chain observer service. pub(crate) chain_observer: Arc, - /// Certificate chain synchroniser service - pub(crate) certificate_chain_synchroniser: Arc, + /// Certificate chain synchronizer service + pub(crate) certificate_chain_synchronizer: Arc, /// Signer registerer service pub signer_registerer: Arc, diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index 5e6bc388691..affffc98395 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -72,7 +72,7 @@ pub trait AggregatorRunnerTrait: Sync + Send { /// Synchronize the follower aggregator signer registration. async fn synchronize_follower_aggregator_signer_registration(&self) -> StdResult<()>; - /// Synchronise the follower aggregator certificate chain + /// Synchronize the follower aggregator certificate chain async fn synchronize_follower_aggregator_certificate_chain( &self, force_sync: bool, @@ -522,7 +522,7 @@ impl AggregatorRunnerTrait for AggregatorRunner { ">> synchronize_follower_aggregator_certificate_chain(force_sync:{force_sync})" ); self.dependencies - .certificate_chain_synchroniser + .certificate_chain_synchronizer .synchronize_certificate_chain(force_sync) .await } diff --git a/mithril-aggregator/src/services/aggregator_client.rs b/mithril-aggregator/src/services/aggregator_client.rs index 547b0d81257..a81f42928cf 100644 --- a/mithril-aggregator/src/services/aggregator_client.rs +++ b/mithril-aggregator/src/services/aggregator_client.rs @@ -293,7 +293,7 @@ impl AggregatorHTTPClient { } } - async fn certificates_details( + async fn certificate_details( &self, certificate_hash: &str, ) -> Result, AggregatorClientError> { @@ -323,7 +323,7 @@ impl AggregatorHTTPClient { async fn latest_genesis_certificate( &self, ) -> Result, AggregatorClientError> { - self.certificates_details("genesis").await + self.certificate_details("genesis").await } } @@ -342,7 +342,7 @@ impl CertificateRetriever for AggregatorHTTPClient { certificate_hash: &str, ) -> Result { let message = self - .certificates_details(certificate_hash) + .certificate_details(certificate_hash) .await .with_context(|| { format!("Failed to retrieve certificate with hash: '{certificate_hash}'") @@ -365,7 +365,7 @@ impl RemoteCertificateRetriever for AggregatorHTTPClient { None => Ok(None), Some(latest_certificate_list_item) => { let latest_certificate_message = - self.certificates_details(&latest_certificate_list_item.hash).await?; + self.certificate_details(&latest_certificate_list_item.hash).await?; latest_certificate_message.map(TryInto::try_into).transpose() } } @@ -586,7 +586,7 @@ mod tests { then.status(200).body(json!(expected_message).to_string()); }); - let fetched_message = client.certificates_details(&expected_message.hash).await.unwrap(); + let fetched_message = client.certificate_details(&expected_message.hash).await.unwrap(); assert_eq!(Some(expected_message), fetched_message); } @@ -612,7 +612,7 @@ mod tests { then.status(500).body("an error occurred"); }); - match client.certificates_details("whatever").await.unwrap_err() { + match client.certificate_details("whatever").await.unwrap_err() { AggregatorClientError::RemoteServerTechnical(_) => (), e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), }; @@ -628,7 +628,7 @@ mod tests { }); let error = client - .certificates_details("whatever") + .certificate_details("whatever") .await .expect_err("retrieve_epoch_settings should fail"); diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs b/mithril-aggregator/src/services/certificate_chain_synchronizer/interface.rs similarity index 100% rename from mithril-aggregator/src/services/certificate_chain_synchroniser/interface.rs rename to mithril-aggregator/src/services/certificate_chain_synchronizer/interface.rs diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs b/mithril-aggregator/src/services/certificate_chain_synchronizer/mod.rs similarity index 52% rename from mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs rename to mithril-aggregator/src/services/certificate_chain_synchronizer/mod.rs index cde74808358..83562263901 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/mod.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchronizer/mod.rs @@ -1,7 +1,7 @@ mod interface; mod noop; -mod synchroniser_service; +mod synchronizer_service; pub use interface::*; pub use noop::*; -pub use synchroniser_service::*; +pub use synchronizer_service::*; diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs b/mithril-aggregator/src/services/certificate_chain_synchronizer/noop.rs similarity index 83% rename from mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs rename to mithril-aggregator/src/services/certificate_chain_synchronizer/noop.rs index 076bc6e2ae1..2d02ba51b19 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/noop.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchronizer/noop.rs @@ -3,10 +3,10 @@ use mithril_common::StdResult; use crate::services::CertificateChainSynchronizer; /// A noop [CertificateChainSynchronizer] for leader aggregators -pub struct MithrilCertificateChainSynchroniserNoop; +pub struct MithrilCertificateChainSynchronizerNoop; #[async_trait::async_trait] -impl CertificateChainSynchronizer for MithrilCertificateChainSynchroniserNoop { +impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizerNoop { async fn synchronize_certificate_chain(&self, _force: bool) -> StdResult<()> { Ok(()) } diff --git a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs b/mithril-aggregator/src/services/certificate_chain_synchronizer/synchronizer_service.rs similarity index 88% rename from mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs rename to mithril-aggregator/src/services/certificate_chain_synchronizer/synchronizer_service.rs index 9c3f2e61e72..7342e9c3f6b 100644 --- a/mithril-aggregator/src/services/certificate_chain_synchroniser/synchroniser_service.rs +++ b/mithril-aggregator/src/services/certificate_chain_synchronizer/synchronizer_service.rs @@ -15,8 +15,8 @@ //! 4. Store the fetched certificates in the database, from genesis to latest, for each certificate: //! - if it exists in the database, it is replaced //! - if it doesn't exist, it is inserted -//! 5. Create a certified open message in the database, based on the latest certificate and with a -//! MithrilStakeDistribution signed entity type +//! 5. Create a certified open message in the database, based on the latest certificate (the first +//! of the last epoch synchronized) //! 6. End //! use anyhow::{Context, anyhow}; @@ -54,7 +54,7 @@ enum SyncStatus { Forced, NoLocalGenesis, RemoteGenesisMatchesLocalGenesis, - RemoteGenesisDontMatchesLocalGenesis, + RemoteGenesisDoesntMatchLocalGenesis, } impl SyncStatus { @@ -63,7 +63,7 @@ impl SyncStatus { SyncStatus::Forced => true, SyncStatus::NoLocalGenesis => true, SyncStatus::RemoteGenesisMatchesLocalGenesis => false, - SyncStatus::RemoteGenesisDontMatchesLocalGenesis => true, + SyncStatus::RemoteGenesisDoesntMatchLocalGenesis => true, } } } @@ -103,7 +103,7 @@ impl MithrilCertificateChainSynchronizer { Some(remote_genesis) if (local_genesis == remote_genesis) => { Ok(SyncStatus::RemoteGenesisMatchesLocalGenesis) } - Some(_) => Ok(SyncStatus::RemoteGenesisDontMatchesLocalGenesis), + Some(_) => Ok(SyncStatus::RemoteGenesisDoesntMatchLocalGenesis), // The remote aggregator doesn't have a chain yet, we can't sync None => Err(anyhow!("Remote aggregator doesn't have a chain yet")), } @@ -116,7 +116,7 @@ impl MithrilCertificateChainSynchronizer { &self, starting_point: Certificate, ) -> StdResult> { - // IMPORTANT: Order matters, certificates must be ordered from genesis to latest + // IMPORTANT: Order matters, returned certificates must be ordered from genesis to latest // (fetched database data is returned from last inserted to oldest) let mut validated_certificates = VecDeque::new(); let mut certificate = starting_point; @@ -181,11 +181,15 @@ impl CertificateChainSynchronizer for MithrilCertificateChainSynchronizer { anyhow!("Remote aggregator doesn't have a chain yet") .context("Failed to retrieve latest remote certificate details"), )?; - let open_message = prepare_open_message_to_store(&starting_point); let remote_certificate_chain = self .retrieve_and_validate_remote_certificate_chain(starting_point) .await .with_context(|| "Failed to retrieve and validate remote certificate chain")?; + let open_message = prepare_open_message_to_store( + remote_certificate_chain + .last() + .ok_or(anyhow!("Retrieved certificate chain is empty"))?, + ); self.store_certificate_chain(remote_certificate_chain) .await .with_context(|| "Failed to store remote retrieved certificate chain")?; @@ -223,7 +227,6 @@ mod tests { use mithril_common::certificate_chain::{ FakeCertificaterRetriever, MithrilCertificateVerifier, }; - use mithril_common::crypto_helper::ProtocolGenesisVerificationKey; use mithril_common::test_utils::{ CertificateChainBuilder, CertificateChainFixture, fake_data, fake_keys, mock_extensions::MockBuilder, @@ -233,39 +236,10 @@ mod tests { MockOpenMessageStorer, MockRemoteCertificateRetriever, MockSynchronizedCertificateStorer, }; use crate::test_tools::TestLogger; + use crate::tools::mocks::MockCertificateVerifier; use super::*; - mockall::mock! { - CertificateVerifier {} - #[async_trait] - impl CertificateVerifier for CertificateVerifier { - async fn verify_genesis_certificate( - &self, - genesis_certificate: &Certificate, - genesis_verification_key: &ProtocolGenesisVerificationKey, - ) -> StdResult<()>; - - async fn verify_standard_certificate( - &self, - certificate: &Certificate, - previous_certificate: &Certificate, - ) -> StdResult<()>; - - async fn verify_certificate( - &self, - certificate: &Certificate, - genesis_verification_key: &ProtocolGenesisVerificationKey, - ) -> StdResult>; - - async fn verify_certificate_chain( - &self, - certificate: Certificate, - genesis_verification_key: &ProtocolGenesisVerificationKey, - ) -> StdResult<()>; - } - } - impl MithrilCertificateChainSynchronizer { fn default_for_test() -> Self { let genesis_verification_key = @@ -283,7 +257,7 @@ mod tests { } } - macro_rules! mocked_synchroniser { + macro_rules! mocked_synchronizer { (with_remote_genesis: $remote_genesis_result:expr) => { MithrilCertificateChainSynchronizer { remote_certificate_retriever: @@ -391,63 +365,63 @@ mod tests { fn sync_state_should_sync() { assert!(SyncStatus::Forced.should_sync()); assert!(!SyncStatus::RemoteGenesisMatchesLocalGenesis.should_sync()); - assert!(SyncStatus::RemoteGenesisDontMatchesLocalGenesis.should_sync()); + assert!(SyncStatus::RemoteGenesisDoesntMatchLocalGenesis.should_sync()); assert!(SyncStatus::NoLocalGenesis.should_sync()); } #[tokio::test] async fn state_when_force_true() { - let synchroniser = MithrilCertificateChainSynchronizer::default_for_test(); + let synchronizer = MithrilCertificateChainSynchronizer::default_for_test(); - let sync_state = synchroniser.check_sync_state(true).await.unwrap(); + let sync_state = synchronizer.check_sync_state(true).await.unwrap(); assert_eq!(SyncStatus::Forced, sync_state); } #[tokio::test] async fn state_when_force_false_and_no_local_genesis_certificate_found() { - let synchroniser = mocked_synchroniser!(with_local_genesis: Ok(None)); + let synchronizer = mocked_synchronizer!(with_local_genesis: Ok(None)); - let sync_state = synchroniser.check_sync_state(false).await.unwrap(); + let sync_state = synchronizer.check_sync_state(false).await.unwrap(); assert_eq!(SyncStatus::NoLocalGenesis, sync_state); } #[tokio::test] async fn state_when_force_false_and_remote_genesis_dont_matches_local_genesis() { - let synchroniser = mocked_synchroniser!( + let synchronizer = mocked_synchronizer!( with_remote_genesis: Ok(Some(fake_data::genesis_certificate("remote_genesis"))), with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) ); - let sync_state = synchroniser.check_sync_state(false).await.unwrap(); - assert_eq!(SyncStatus::RemoteGenesisDontMatchesLocalGenesis, sync_state); + let sync_state = synchronizer.check_sync_state(false).await.unwrap(); + assert_eq!(SyncStatus::RemoteGenesisDoesntMatchLocalGenesis, sync_state); } #[tokio::test] async fn state_when_force_false_and_remote_genesis_matches_local_genesis() { let remote_genesis = fake_data::genesis_certificate("genesis"); let local_genesis = remote_genesis.clone(); - let synchroniser = mocked_synchroniser!( + let synchronizer = mocked_synchronizer!( with_remote_genesis: Ok(Some(remote_genesis)), with_local_genesis: Ok(Some(local_genesis)) ); - let sync_state = synchroniser.check_sync_state(false).await.unwrap(); + let sync_state = synchronizer.check_sync_state(false).await.unwrap(); assert_eq!(SyncStatus::RemoteGenesisMatchesLocalGenesis, sync_state); } #[tokio::test] async fn if_force_true_it_should_not_fetch_remote_genesis_certificate() { - let synchroniser = mocked_synchroniser!(with_remote_genesis: Err(anyhow!( + let synchronizer = mocked_synchronizer!(with_remote_genesis: Err(anyhow!( "should not fetch genesis" ))); - synchroniser.check_sync_state(true).await.unwrap(); + synchronizer.check_sync_state(true).await.unwrap(); } #[tokio::test] async fn should_abort_with_error_if_force_false_and_fails_to_retrieve_local_genesis() { - let synchroniser = mocked_synchroniser!(with_local_genesis: Err(anyhow!("failure"))); - synchroniser + let synchronizer = mocked_synchronizer!(with_local_genesis: Err(anyhow!("failure"))); + synchronizer .check_sync_state(false) .await .expect_err("Expected an error but was:"); @@ -455,11 +429,11 @@ mod tests { #[tokio::test] async fn should_abort_with_error_if_force_false_and_fails_to_retrieve_remote_genesis() { - let synchroniser = mocked_synchroniser!( + let synchronizer = mocked_synchronizer!( with_remote_genesis: Err(anyhow!("failure")), with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) ); - synchroniser + synchronizer .check_sync_state(false) .await .expect_err("Expected an error but was:"); @@ -467,11 +441,11 @@ mod tests { #[tokio::test] async fn should_abort_with_error_if_force_false_and_remote_genesis_is_none() { - let synchroniser = mocked_synchroniser!( + let synchronizer = mocked_synchronizer!( with_remote_genesis: Ok(None), with_local_genesis: Ok(Some(fake_data::genesis_certificate("local_genesis"))) ); - let error = synchroniser + let error = synchronizer .check_sync_state(false) .await .expect_err("Expected an error but was:"); @@ -495,14 +469,14 @@ mod tests { #[tokio::test] async fn succeed_if_the_remote_chain_only_contains_a_genesis_certificate() { let chain = CertificateChainBuilder::new().with_total_certificates(1).build(); - let synchroniser = MithrilCertificateChainSynchronizer { + let synchronizer = MithrilCertificateChainSynchronizer { certificate_verifier: fake_verifier(&chain), genesis_verifier: Arc::new(chain.genesis_verifier.clone()), ..MithrilCertificateChainSynchronizer::default_for_test() }; let starting_point = chain[0].clone(); - let remote_certificate_chain = synchroniser + let remote_certificate_chain = synchronizer .retrieve_and_validate_remote_certificate_chain(starting_point) .await .unwrap(); @@ -512,10 +486,10 @@ mod tests { #[tokio::test] async fn abort_with_error_if_a_certificate_is_invalid() { - let synchroniser = mocked_synchroniser!(with_verify_certificate_result: Err(anyhow!("invalid certificate"))); + let synchronizer = mocked_synchronizer!(with_verify_certificate_result: Err(anyhow!("invalid certificate"))); let starting_point = fake_data::certificate("certificate"); - synchroniser + synchronizer .retrieve_and_validate_remote_certificate_chain(starting_point) .await .expect_err("Expected an error but was:"); @@ -531,14 +505,14 @@ mod tests { .with_total_certificates(9) .with_certificates_per_epoch(2) .build(); - let synchroniser = MithrilCertificateChainSynchronizer { + let synchronizer = MithrilCertificateChainSynchronizer { certificate_verifier: fake_verifier(&chain), genesis_verifier: Arc::new(chain.genesis_verifier.clone()), ..MithrilCertificateChainSynchronizer::default_for_test() }; let starting_point = chain[0].clone(); - let remote_certificate_chain = synchroniser + let remote_certificate_chain = synchronizer .retrieve_and_validate_remote_certificate_chain(starting_point.clone()) .await .unwrap(); @@ -572,7 +546,7 @@ mod tests { ..base_certificate }, ]; - let synchroniser = MithrilCertificateChainSynchronizer { + let synchronizer = MithrilCertificateChainSynchronizer { certificate_verifier: MockBuilder::::configure(|mock| { let cert_1 = chain[1].clone(); mock.expect_verify_certificate() @@ -590,7 +564,7 @@ mod tests { }; let starting_point = chain[2].clone(); - let remote_certificate_chain = synchroniser + let remote_certificate_chain = synchronizer .retrieve_and_validate_remote_certificate_chain(starting_point.clone()) .await .unwrap(); @@ -616,14 +590,14 @@ mod tests { fake_data::certificate("certificate2"), ]; let storer = Arc::new(DumbCertificateStorer::default()); - let synchroniser = MithrilCertificateChainSynchronizer { + let synchronizer = MithrilCertificateChainSynchronizer { certificate_storer: storer.clone(), ..MithrilCertificateChainSynchronizer::default_for_test() }; assert_eq!(Vec::::new(), storer.stored_certificates()); - synchroniser + synchronizer .store_certificate_chain(certificates_chain.clone()) .await .unwrap(); @@ -633,7 +607,7 @@ mod tests { #[tokio::test] async fn fail_on_storer_error() { - let synchroniser = MithrilCertificateChainSynchronizer { + let synchronizer = MithrilCertificateChainSynchronizer { certificate_storer: MockBuilder::::configure( |mock| { mock.expect_insert_or_replace_many() @@ -643,7 +617,7 @@ mod tests { ..MithrilCertificateChainSynchronizer::default_for_test() }; - synchroniser + synchronizer .store_certificate_chain(vec![fake_data::certificate("certificate")]) .await .unwrap_err(); @@ -655,7 +629,7 @@ mod tests { use super::*; - fn build_synchroniser( + fn build_synchronizer( remote_chain: &CertificateChainFixture, storer: Arc, ) -> MithrilCertificateChainSynchronizer { @@ -693,10 +667,10 @@ mod tests { .with_total_certificates(8) .build(); let storer = Arc::new(DumbCertificateStorer::default()); - let synchroniser = build_synchroniser(&remote_chain, storer.clone()); + let synchronizer = build_synchronizer(&remote_chain, storer.clone()); // Will sync even if force is false - synchroniser.synchronize_certificate_chain(false).await.unwrap(); + synchronizer.synchronize_certificate_chain(false).await.unwrap(); let mut expected = remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash); @@ -716,15 +690,15 @@ mod tests { remote_chain.genesis_certificate().clone(), existing_certificates.clone(), )); - let synchroniser = build_synchroniser(&remote_chain, storer.clone()); + let synchronizer = build_synchronizer(&remote_chain, storer.clone()); // Force false - won't sync - synchroniser.synchronize_certificate_chain(false).await.unwrap(); + synchronizer.synchronize_certificate_chain(false).await.unwrap(); assert_eq!(&existing_certificates, &storer.stored_certificates()); // Force true - will sync - synchroniser.synchronize_certificate_chain(true).await.unwrap(); + synchronizer.synchronize_certificate_chain(true).await.unwrap(); let mut expected = remote_chain.certificate_path_to_genesis(&remote_chain.latest_certificate().hash); diff --git a/mithril-aggregator/src/services/mod.rs b/mithril-aggregator/src/services/mod.rs index 307b7c062e1..d8443da5f65 100644 --- a/mithril-aggregator/src/services/mod.rs +++ b/mithril-aggregator/src/services/mod.rs @@ -11,7 +11,7 @@ mod aggregator_client; mod cardano_transactions_importer; -mod certificate_chain_synchroniser; +mod certificate_chain_synchronizer; mod certifier; mod epoch_service; mod message; @@ -28,7 +28,7 @@ mod usage_reporter; pub use aggregator_client::*; pub use cardano_transactions_importer::*; -pub use certificate_chain_synchroniser::*; +pub use certificate_chain_synchronizer::*; pub use certifier::*; pub use epoch_service::*; pub use message::*; diff --git a/mithril-aggregator/src/tools/mocks.rs b/mithril-aggregator/src/tools/mocks.rs index f3b315de936..ceac43a6d2c 100644 --- a/mithril-aggregator/src/tools/mocks.rs +++ b/mithril-aggregator/src/tools/mocks.rs @@ -2,13 +2,45 @@ use async_trait::async_trait; use mithril_cardano_node_chain::chain_observer::{ChainObserver, ChainObserverError}; use mithril_cardano_node_chain::entities::{ChainAddress, TxDatum}; -use mithril_common::crypto_helper::{KesPeriod, OpCert}; -use mithril_common::entities::{ChainPoint, Epoch, StakeDistribution}; +use mithril_common::certificate_chain::CertificateVerifier; +use mithril_common::crypto_helper::{KesPeriod, OpCert, ProtocolGenesisVerificationKey}; +use mithril_common::entities::{Certificate, ChainPoint, Epoch, StakeDistribution}; use mithril_persistence::store::StakeStorer; use mithril_common::StdResult; use mockall::mock; +mock! { + pub CertificateVerifier {} + + #[async_trait] + impl CertificateVerifier for CertificateVerifier { + async fn verify_genesis_certificate( + &self, + genesis_certificate: &Certificate, + genesis_verification_key: &ProtocolGenesisVerificationKey, + ) -> StdResult<()>; + + async fn verify_standard_certificate( + &self, + certificate: &Certificate, + previous_certificate: &Certificate, + ) -> StdResult<()>; + + async fn verify_certificate( + &self, + certificate: &Certificate, + genesis_verification_key: &ProtocolGenesisVerificationKey, + ) -> StdResult>; + + async fn verify_certificate_chain( + &self, + certificate: Certificate, + genesis_verification_key: &ProtocolGenesisVerificationKey, + ) -> StdResult<()>; + } +} + mock! { pub ChainObserver {} diff --git a/mithril-aggregator/tests/create_certificate_follower.rs b/mithril-aggregator/tests/create_certificate_follower.rs index 60f2562ddb3..b0c152bd63f 100644 --- a/mithril-aggregator/tests/create_certificate_follower.rs +++ b/mithril-aggregator/tests/create_certificate_follower.rs @@ -124,7 +124,7 @@ async fn create_certificate_follower() { "Epoch 1: - the leader aggregator registers the first signers - the leader aggregator can't transition from 'Idle' to 'Ready' - - the follower can't synchronise its chain with the leader aggregator + - the follower can't synchronize its chain with the leader aggregator - the follower aggregator can't transition from 'Idle' to 'Ready' " ); @@ -209,7 +209,7 @@ async fn create_certificate_follower() { "Epoch 3: - the leader aggregator produces a new certificate - the follower aggregator synchronizes signers from the leader aggregator - - the follower synchronise its chain with the leader aggregator + - the follower synchronizes its chain with the leader aggregator - the follower aggregator can transition from 'Idle' to 'Ready' " ); @@ -265,12 +265,12 @@ async fn create_certificate_follower() { cycle_err!(leader_tester, "signing"); comment!( - "Follower: change the epoch after leader made a certificate and synchronise certificate chain" + "Follower: change the epoch after leader created a certificate and synchronize certificate chain" ); follower_tester.increase_epoch().await.unwrap(); cycle_err!(follower_tester, "idle"); assert_last_certificate_eq!( - follower_tester, synchronised_from_leader => expected_certificate_on_both_aggregator + follower_tester, synchronized_from_leader => expected_certificate_on_both_aggregator ); cycle!(follower_tester, "ready"); cycle_err!(leader_tester, "signing"); diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index f45d2de5217..d5b4f91cd1b 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -98,7 +98,7 @@ macro_rules! assert_last_certificate_eq { .unwrap(); assert_eq!($expected_certificate, last_certificate); }}; - ( $tester:expr, synchronised_from_leader => $expected_certificate:expr ) => {{ + ( $tester:expr, synchronized_from_leader => $expected_certificate:expr ) => {{ if let Some(signed_type) = $expected_certificate.get_signed_type() { RuntimeTester::wait_until_certificate(&$tester, &signed_type) .await diff --git a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs index cc7298a5b5b..1a3aa4078a0 100644 --- a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs +++ b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs @@ -55,7 +55,7 @@ impl Spec { // As we get closer to the tip of the chain when signing, we'll be able to relax this constraint. assertions::transfer_funds(spec.infrastructure.devnet()).await?; - info!("bootstrapping leader aggregator"); + info!("Bootstrapping leader aggregator"); spec.bootstrap_leader_aggregator(&spec.infrastructure).await?; info!("Starting followers"); diff --git a/mithril-test-lab/mithril-end-to-end/src/run_only.rs b/mithril-test-lab/mithril-end-to-end/src/run_only.rs index 8c327d39054..abcedfa59c2 100644 --- a/mithril-test-lab/mithril-end-to-end/src/run_only.rs +++ b/mithril-test-lab/mithril-end-to-end/src/run_only.rs @@ -17,7 +17,7 @@ impl RunOnly { pub async fn run(self) -> StdResult<()> { let run_only = Arc::new(self); - info!("bootstrapping leader aggregator"); + info!("Bootstrapping leader aggregator"); run_only.bootstrap_leader_aggregator(&run_only.infrastructure).await?; info!("Starting followers"); From a54c3bed6f47c2d88829f6a94c4d655d15279c67 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 9 Jul 2025 17:37:01 +0200 Subject: [PATCH 26/28] test(aggregato): in `create_certificate_follower` check open messages after sync This ensure that the synchronizer create an open message for MSD and that the state machine then proceed to the next signed entity type. --- .../tests/create_certificate_follower.rs | 31 ++++++++++++- .../tests/test_extensions/runtime_tester.rs | 45 ++++++++++--------- 2 files changed, 53 insertions(+), 23 deletions(-) diff --git a/mithril-aggregator/tests/create_certificate_follower.rs b/mithril-aggregator/tests/create_certificate_follower.rs index b0c152bd63f..3ded9df4bed 100644 --- a/mithril-aggregator/tests/create_certificate_follower.rs +++ b/mithril-aggregator/tests/create_certificate_follower.rs @@ -211,6 +211,8 @@ async fn create_certificate_follower() { - the follower aggregator synchronizes signers from the leader aggregator - the follower synchronizes its chain with the leader aggregator - the follower aggregator can transition from 'Idle' to 'Ready' + - the follower aggregator transition from 'Ready' to 'Signing(CardanoStakeDistribution)', skipping + MithrilStakeDistribution " ); let epoch_fixture = &epoch_fixtures_map[&Epoch(3)]; @@ -272,8 +274,35 @@ async fn create_certificate_follower() { assert_last_certificate_eq!( follower_tester, synchronized_from_leader => expected_certificate_on_both_aggregator ); + let current_msd_open_message = follower_tester + .observer + .get_current_open_message(SignedEntityTypeDiscriminants::MithrilStakeDistribution) + .await + .unwrap(); + assert!( + current_msd_open_message + .as_ref() + .is_some_and(|m| m.epoch == 3 && m.is_certified), + "Expected a certified OpenMessage for MithrilStakeDistribution, got:\n{current_msd_open_message:#?}" + ); + + comment!( + "Follower: transition to 'Signing(CardanoStakeDistribution)' directly since a\ + OpenMessage for MithrilStakeDistribution was stored by the synchronizer" + ); cycle!(follower_tester, "ready"); - cycle_err!(leader_tester, "signing"); + cycle!(follower_tester, "signing"); + let current_csd_open_message = follower_tester + .observer + .get_current_open_message(SignedEntityTypeDiscriminants::CardanoStakeDistribution) + .await + .unwrap(); + assert!( + current_csd_open_message + .as_ref() + .is_some_and(|m| m.epoch == 3 && !m.is_certified), + "Expected a non-certified OpenMessage for CardanoStakeDistribution, got:\n{current_csd_open_message:#?}" + ); comment!( "Epoch 4: diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index d5b4f91cd1b..5cae914f313 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -44,14 +44,15 @@ macro_rules! cycle { ( $tester:expr, $expected_state:expr ) => {{ use $crate::test_extensions::ExpectedMetrics; + let runtime_tester: &mut RuntimeTester = &mut $tester; let (runtime_cycle_success, runtime_cycle_total) = - $tester.get_runtime_cycle_success_and_total_since_startup_metrics(); + runtime_tester.get_runtime_cycle_success_and_total_since_startup_metrics(); - RuntimeTester::cycle(&mut $tester).await.unwrap(); - assert_eq!($expected_state, $tester.runtime.get_state()); + runtime_tester.cycle().await.unwrap(); + assert_eq!($expected_state, runtime_tester.runtime.get_state()); assert_metrics_eq!( - $tester.metrics_verifier, + runtime_tester.metrics_verifier, ExpectedMetrics::new() .runtime_cycle_success(runtime_cycle_success + 1) .runtime_cycle_total(runtime_cycle_total + 1) @@ -64,17 +65,19 @@ macro_rules! cycle_err { ( $tester:expr, $expected_state:expr ) => {{ use $crate::test_extensions::ExpectedMetrics; + let runtime_tester: &mut RuntimeTester = &mut $tester; let (runtime_cycle_success, runtime_cycle_total) = - $tester.get_runtime_cycle_success_and_total_since_startup_metrics(); + runtime_tester.get_runtime_cycle_success_and_total_since_startup_metrics(); - let err = RuntimeTester::cycle(&mut $tester) + let err = runtime_tester + .cycle() .await .expect_err("cycle tick should have returned an error"); slog_scope::info!("cycle_err result: {err:?}"); - assert_eq!($expected_state, $tester.runtime.get_state()); + assert_eq!($expected_state, runtime_tester.runtime.get_state()); assert_metrics_eq!( - $tester.metrics_verifier, + runtime_tester.metrics_verifier, ExpectedMetrics::new() .runtime_cycle_success(runtime_cycle_success) .runtime_cycle_total(runtime_cycle_total + 1) @@ -85,31 +88,29 @@ macro_rules! cycle_err { #[macro_export] macro_rules! assert_last_certificate_eq { ( $tester:expr, $expected_certificate:expr ) => {{ + let runtime_tester: &mut RuntimeTester = &mut $tester; if let Some(signed_type) = $expected_certificate.get_signed_type() { - RuntimeTester::wait_until_signed_entity(&$tester, &signed_type) - .await - .unwrap(); + runtime_tester.wait_until_signed_entity(&signed_type).await.unwrap(); } let is_synchronized_from_leader = false; - let last_certificate = - RuntimeTester::get_last_expected_certificate(&mut $tester, is_synchronized_from_leader) - .await - .unwrap(); + let last_certificate = runtime_tester + .get_last_expected_certificate(is_synchronized_from_leader) + .await + .unwrap(); assert_eq!($expected_certificate, last_certificate); }}; ( $tester:expr, synchronized_from_leader => $expected_certificate:expr ) => {{ + let runtime_tester: &mut RuntimeTester = &mut $tester; if let Some(signed_type) = $expected_certificate.get_signed_type() { - RuntimeTester::wait_until_certificate(&$tester, &signed_type) - .await - .unwrap(); + runtime_tester.wait_until_certificate(&signed_type).await.unwrap(); } let is_synchronized_from_leader = true; - let last_certificate = - RuntimeTester::get_last_expected_certificate(&mut $tester, is_synchronized_from_leader) - .await - .unwrap(); + let last_certificate = runtime_tester + .get_last_expected_certificate(is_synchronized_from_leader) + .await + .unwrap(); assert_eq!($expected_certificate, last_certificate); }}; } From a9d24d83ebb25fdb02a47d83a5f38b519c319671 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 9 Jul 2025 17:57:37 +0200 Subject: [PATCH 27/28] chore: update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b9396942682..6a3735cc08c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ As a minor extension, we have adopted a slightly different versioning convention - **UNSTABLE** : - Support for DMQ signature publisher in the signer and signature consumer in the aggregator. + - Implement automatic certificates chain synchronization between leader/follower aggregators. + - Crates versions: | Crate | Version | From 11c8aec9f92d911a3c622445e095688040c0bbd1 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 9 Jul 2025 17:52:35 +0200 Subject: [PATCH 28/28] chore: upgrade crate versions * mithril-aggregator from `0.7.72` to `0.7.73` * mithril-common from `0.6.7` to `0.6.8` * mithril-end-to-end from `0.4.95` to `0.4.96` --- Cargo.lock | 6 +++--- mithril-aggregator/Cargo.toml | 2 +- mithril-common/Cargo.toml | 2 +- mithril-test-lab/mithril-end-to-end/Cargo.toml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eec4bbe8368..f28604c31c8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3910,7 +3910,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.7.72" +version = "0.7.73" dependencies = [ "anyhow", "async-trait", @@ -4162,7 +4162,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.6.7" +version = "0.6.8" dependencies = [ "anyhow", "async-trait", @@ -4240,7 +4240,7 @@ dependencies = [ [[package]] name = "mithril-end-to-end" -version = "0.4.95" +version = "0.4.96" dependencies = [ "anyhow", "async-recursion", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index c1457b1f2df..713c5f5023d 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.7.72" +version = "0.7.73" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index fcc1e647c44..0f5a23f20cb 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.6.7" +version = "0.6.8" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-test-lab/mithril-end-to-end/Cargo.toml b/mithril-test-lab/mithril-end-to-end/Cargo.toml index 19198097cde..54802e4d462 100644 --- a/mithril-test-lab/mithril-end-to-end/Cargo.toml +++ b/mithril-test-lab/mithril-end-to-end/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-end-to-end" -version = "0.4.95" +version = "0.4.96" authors = { workspace = true } edition = { workspace = true } documentation = { workspace = true }