From 7d33e896fb75ef755501ed873c9263610111848e Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 28 Oct 2025 19:47:09 +0100 Subject: [PATCH 01/13] refactor(aggregator-client): rename `query/certificate` mod to `get` Since there will be arround ~20 requests maximum, making a module by subject will result on a lots of modules with one or two files, not that helpful. + rename `CertificateDetailsQuery` to `GetCertificateQuery` to have an uniform convention: `{Verb}{Subject}Query`. --- .../src/query/certificate/mod.rs | 3 -- .../src/query/get/get_aggregator_features.rs | 0 .../get_certificate.rs} | 43 +++++++------------ .../src/query/get/mod.rs | 3 ++ .../src/query/mod.rs | 4 +- .../mithril-aggregator-client/src/test/mod.rs | 14 ++++++ 6 files changed, 35 insertions(+), 32 deletions(-) delete mode 100644 internal/mithril-aggregator-client/src/query/certificate/mod.rs create mode 100644 internal/mithril-aggregator-client/src/query/get/get_aggregator_features.rs rename internal/mithril-aggregator-client/src/query/{certificate/get_certificate_details.rs => get/get_certificate.rs} (77%) create mode 100644 internal/mithril-aggregator-client/src/query/get/mod.rs diff --git a/internal/mithril-aggregator-client/src/query/certificate/mod.rs b/internal/mithril-aggregator-client/src/query/certificate/mod.rs deleted file mode 100644 index c09cabaf25f..00000000000 --- a/internal/mithril-aggregator-client/src/query/certificate/mod.rs +++ /dev/null @@ -1,3 +0,0 @@ -mod get_certificate_details; - -pub use get_certificate_details::*; diff --git a/internal/mithril-aggregator-client/src/query/get/get_aggregator_features.rs b/internal/mithril-aggregator-client/src/query/get/get_aggregator_features.rs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/internal/mithril-aggregator-client/src/query/certificate/get_certificate_details.rs b/internal/mithril-aggregator-client/src/query/get/get_certificate.rs similarity index 77% rename from internal/mithril-aggregator-client/src/query/certificate/get_certificate_details.rs rename to internal/mithril-aggregator-client/src/query/get/get_certificate.rs index 6ae31f44448..169ab754226 100644 --- a/internal/mithril-aggregator-client/src/query/certificate/get_certificate_details.rs +++ b/internal/mithril-aggregator-client/src/query/get/get_certificate.rs @@ -9,12 +9,12 @@ use crate::AggregatorClientResult; use crate::error::AggregatorClientError; use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; -/// Get the details of a certificate -pub struct CertificateDetailsQuery { +/// Query to get the details of a certificate +pub struct GetCertificateQuery { hash: String, } -impl CertificateDetailsQuery { +impl GetCertificateQuery { /// Instantiate a query to get a certificate by hash pub fn by_hash>(hash: H) -> Self { Self { hash: hash.into() } @@ -30,7 +30,7 @@ impl CertificateDetailsQuery { #[cfg_attr(target_family = "wasm", async_trait(?Send))] #[cfg_attr(not(target_family = "wasm"), async_trait)] -impl AggregatorQuery for CertificateDetailsQuery { +impl AggregatorQuery for GetCertificateQuery { type Response = Option; type Body = (); @@ -65,7 +65,7 @@ mod tests { use mithril_common::test::double::Dummy; - use crate::test::setup_server_and_client; + use crate::test::{assert_error_matches, setup_server_and_client}; use super::*; @@ -79,7 +79,7 @@ mod tests { }); let fetched_message = client - .send(CertificateDetailsQuery::by_hash(&expected_message.hash)) + .send(GetCertificateQuery::by_hash(&expected_message.hash)) .await .unwrap(); @@ -94,10 +94,7 @@ mod tests { then.status(404); }); - let fetched_message = client - .send(CertificateDetailsQuery::by_hash("whatever")) - .await - .unwrap(); + let fetched_message = client.send(GetCertificateQuery::by_hash("whatever")).await.unwrap(); assert_eq!(None, fetched_message); } @@ -110,14 +107,12 @@ mod tests { then.status(500).body("an error occurred"); }); - match client - .send(CertificateDetailsQuery::by_hash("whatever")) + let error = client + .send(GetCertificateQuery::by_hash("whatever")) .await - .unwrap_err() - { - AggregatorClientError::RemoteServerTechnical(_) => (), - e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), - }; + .unwrap_err(); + + assert_error_matches!(error, AggregatorClientError::RemoteServerTechnical(_)); } #[tokio::test] @@ -129,7 +124,7 @@ mod tests { then.status(200).body(json!(genesis_message).to_string()); }); - let fetched = client.send(CertificateDetailsQuery::latest_genesis()).await.unwrap(); + let fetched = client.send(GetCertificateQuery::latest_genesis()).await.unwrap(); assert_eq!(Some(genesis_message), fetched); } @@ -142,7 +137,7 @@ mod tests { then.status(404); }); - let fetched = client.send(CertificateDetailsQuery::latest_genesis()).await.unwrap(); + let fetched = client.send(GetCertificateQuery::latest_genesis()).await.unwrap(); assert_eq!(None, fetched); } @@ -155,14 +150,8 @@ mod tests { then.status(500).body("an error occurred"); }); - let error = client - .send(CertificateDetailsQuery::latest_genesis()) - .await - .unwrap_err(); + let error = client.send(GetCertificateQuery::latest_genesis()).await.unwrap_err(); - assert!( - matches!(error, AggregatorClientError::RemoteServerTechnical(_)), - "Expected Aggregator::RemoteServerTechnical error, got {error:?}" - ); + assert_error_matches!(error, AggregatorClientError::RemoteServerTechnical(_)); } } diff --git a/internal/mithril-aggregator-client/src/query/get/mod.rs b/internal/mithril-aggregator-client/src/query/get/mod.rs new file mode 100644 index 00000000000..51c8abee07e --- /dev/null +++ b/internal/mithril-aggregator-client/src/query/get/mod.rs @@ -0,0 +1,3 @@ +mod get_certificate; + +pub use get_certificate::*; diff --git a/internal/mithril-aggregator-client/src/query/mod.rs b/internal/mithril-aggregator-client/src/query/mod.rs index 2cff48897fd..9c9f69aa42b 100644 --- a/internal/mithril-aggregator-client/src/query/mod.rs +++ b/internal/mithril-aggregator-client/src/query/mod.rs @@ -4,7 +4,7 @@ //! - Certificate: Get by hash, get latest genesis certificate //! mod api; -mod certificate; +mod get; pub(crate) use api::*; -pub use certificate::*; +pub use get::*; diff --git a/internal/mithril-aggregator-client/src/test/mod.rs b/internal/mithril-aggregator-client/src/test/mod.rs index 265f3d6d400..08d2abe1edd 100644 --- a/internal/mithril-aggregator-client/src/test/mod.rs +++ b/internal/mithril-aggregator-client/src/test/mod.rs @@ -15,3 +15,17 @@ pub(crate) fn setup_server_and_client() -> (MockServer, AggregatorClient) { (server, client) } + +#[cfg(test)] +macro_rules! assert_error_matches { + ($error:expr, $error_type:pat) => { + assert!( + matches!($error, $error_type), + "Expected {} error, got '{:?}'.", + stringify!($error_type), + $error + ); + }; +} +#[cfg(test)] +pub(crate) use assert_error_matches; From d9cf7a179261c0d9f5b2465bf3e9f606c79b7363 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 28 Oct 2025 20:26:26 +0100 Subject: [PATCH 02/13] refactor(aggregator-client): minor adjustement following first usage in follower aggregator - use an arc for the `ApiVersionProvider` so it fit with our current DI systems --- .../mithril-aggregator-client/src/builder.rs | 12 ++++--- .../mithril-aggregator-client/src/client.rs | 35 ++++++++++--------- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/internal/mithril-aggregator-client/src/builder.rs b/internal/mithril-aggregator-client/src/builder.rs index fe464a73d69..1e14e3fdb3f 100644 --- a/internal/mithril-aggregator-client/src/builder.rs +++ b/internal/mithril-aggregator-client/src/builder.rs @@ -2,6 +2,7 @@ use anyhow::Context; use reqwest::{Client, IntoUrl, Proxy, Url}; use slog::{Logger, o}; use std::collections::HashMap; +use std::sync::Arc; use std::time::Duration; use mithril_common::StdResult; @@ -12,7 +13,7 @@ use crate::client::AggregatorClient; /// A builder of [AggregatorClient] pub struct AggregatorClientBuilder { aggregator_url_result: reqwest::Result, - api_version_provider: Option, + api_version_provider: Option>, additional_headers: Option>, timeout_duration: Option, relay_endpoint: Option, @@ -41,7 +42,10 @@ impl AggregatorClientBuilder { } /// Set the [APIVersionProvider] to use. - pub fn with_api_version_provider(mut self, api_version_provider: APIVersionProvider) -> Self { + pub fn with_api_version_provider( + mut self, + api_version_provider: Arc, + ) -> Self { self.api_version_provider = Some(api_version_provider); self } @@ -59,8 +63,8 @@ impl AggregatorClientBuilder { } /// Set the address of the relay - pub fn with_relay_endpoint(mut self, relay_endpoint: String) -> Self { - self.relay_endpoint = Some(relay_endpoint); + pub fn with_relay_endpoint(mut self, relay_endpoint: Option) -> Self { + self.relay_endpoint = relay_endpoint; self } diff --git a/internal/mithril-aggregator-client/src/client.rs b/internal/mithril-aggregator-client/src/client.rs index 04812231ccb..d51a30df8b4 100644 --- a/internal/mithril-aggregator-client/src/client.rs +++ b/internal/mithril-aggregator-client/src/client.rs @@ -2,6 +2,7 @@ use anyhow::{Context, anyhow}; use reqwest::{IntoUrl, Response, Url, header::HeaderMap}; use semver::Version; use slog::{Logger, error, warn}; +use std::sync::Arc; use std::time::Duration; use mithril_common::MITHRIL_API_VERSION_HEADER; @@ -17,7 +18,7 @@ const API_VERSION_MISMATCH_WARNING_MESSAGE: &str = "OpenAPI version may be incom /// A client to send HTTP requests to a Mithril Aggregator pub struct AggregatorClient { pub(super) aggregator_endpoint: Url, - pub(super) api_version_provider: APIVersionProvider, + pub(super) api_version_provider: Arc, pub(super) additional_headers: HeaderMap, pub(super) timeout_duration: Option, pub(super) client: reqwest::Client, @@ -240,8 +241,9 @@ mod tests { #[tokio::test] async fn test_query_send_mithril_api_version_header() { let (server, mut client) = setup_server_and_client(); - client.api_version_provider = - APIVersionProvider::new_with_default_version(Version::parse("1.2.9").unwrap()); + client.api_version_provider = Arc::new(APIVersionProvider::new_with_default_version( + Version::parse("1.2.9").unwrap(), + )); server.mock(|when, then| { when.method(httpmock::Method::GET) .header(MITHRIL_API_VERSION_HEADER, "1.2.9"); @@ -254,8 +256,9 @@ mod tests { #[tokio::test] async fn test_query_send_additional_header_and_dont_override_mithril_api_version_header() { let (server, mut client) = setup_server_and_client(); - client.api_version_provider = - APIVersionProvider::new_with_default_version(Version::parse("1.2.9").unwrap()); + client.api_version_provider = Arc::new(APIVersionProvider::new_with_default_version( + Version::parse("1.2.9").unwrap(), + )); client.additional_headers = { let mut headers = HeaderMap::new(); headers.insert(MITHRIL_API_VERSION_HEADER, "9.4.5".parse().unwrap()); @@ -336,9 +339,9 @@ mod tests { let (logger, log_inspector) = TestLogger::memory(); let client = AggregatorClient::builder("http://whatever") .with_logger(logger) - .with_api_version_provider(APIVersionProvider::new_with_default_version( + .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version( Version::parse(client_version).unwrap(), - )) + ))) .build() .unwrap(); let response = @@ -360,9 +363,9 @@ mod tests { let (logger, log_inspector) = TestLogger::memory(); let client = AggregatorClient::builder("http://whatever") .with_logger(logger) - .with_api_version_provider(APIVersionProvider::new_with_default_version( + .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version( Version::parse(version).unwrap(), - )) + ))) .build() .unwrap(); let response = build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, version); @@ -379,9 +382,9 @@ mod tests { let (logger, log_inspector) = TestLogger::memory(); let client = AggregatorClient::builder("http://whatever") .with_logger(logger) - .with_api_version_provider(APIVersionProvider::new_with_default_version( + .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version( Version::parse(client_version).unwrap(), - )) + ))) .build() .unwrap(); let response = @@ -402,7 +405,7 @@ mod tests { let (logger, log_inspector) = TestLogger::memory(); let client = AggregatorClient::builder("http://whatever") .with_logger(logger) - .with_api_version_provider(APIVersionProvider::default()) + .with_api_version_provider(Arc::new(APIVersionProvider::default())) .build() .unwrap(); let response = @@ -418,7 +421,7 @@ mod tests { let (logger, log_inspector) = TestLogger::memory(); let client = AggregatorClient::builder("http://whatever") .with_logger(logger) - .with_api_version_provider(APIVersionProvider::default()) + .with_api_version_provider(Arc::new(APIVersionProvider::default())) .build() .unwrap(); let response = @@ -434,7 +437,7 @@ mod tests { let (logger, log_inspector) = TestLogger::memory(); let client = AggregatorClient::builder("http://whatever") .with_logger(logger) - .with_api_version_provider(APIVersionProvider::new_failing()) + .with_api_version_provider(Arc::new(APIVersionProvider::new_failing())) .build() .unwrap(); let response = build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, "1.0.0"); @@ -450,9 +453,9 @@ mod tests { let client_version = "1.0.0"; let (server, mut client) = setup_server_and_client(); let (logger, log_inspector) = TestLogger::memory(); - client.api_version_provider = APIVersionProvider::new_with_default_version( + client.api_version_provider = Arc::new(APIVersionProvider::new_with_default_version( Version::parse(client_version).unwrap(), - ); + )); client.logger = logger; server.mock(|_, then| { then.status(StatusCode::CREATED.as_u16()) From fe9f4791560116b9fd6ffbd832e55b7cf294b38c Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 29 Oct 2025 11:55:20 +0100 Subject: [PATCH 03/13] refactor(aggregator-client): rename client and error types to include `Http` This makes clear that this client is Http focused only and can't be extended as is to support other protocols. --- .../mithril-aggregator-client/src/builder.rs | 10 ++--- .../mithril-aggregator-client/src/client.rs | 41 +++++++++++-------- .../mithril-aggregator-client/src/error.rs | 30 +++++++------- internal/mithril-aggregator-client/src/lib.rs | 6 +-- .../src/query/api.rs | 10 ++--- .../src/query/get/get_certificate.rs | 12 +++--- .../mithril-aggregator-client/src/test/mod.rs | 6 +-- 7 files changed, 60 insertions(+), 55 deletions(-) diff --git a/internal/mithril-aggregator-client/src/builder.rs b/internal/mithril-aggregator-client/src/builder.rs index 1e14e3fdb3f..897a51e3821 100644 --- a/internal/mithril-aggregator-client/src/builder.rs +++ b/internal/mithril-aggregator-client/src/builder.rs @@ -8,9 +8,9 @@ use std::time::Duration; use mithril_common::StdResult; use mithril_common::api_version::APIVersionProvider; -use crate::client::AggregatorClient; +use crate::client::AggregatorHttpClient; -/// A builder of [AggregatorClient] +/// A builder of [AggregatorHttpClient] pub struct AggregatorClientBuilder { aggregator_url_result: reqwest::Result, api_version_provider: Option>, @@ -68,8 +68,8 @@ impl AggregatorClientBuilder { self } - /// Returns an [AggregatorClient] based on the builder configuration - pub fn build(self) -> StdResult { + /// Returns an [AggregatorHttpClient] based on the builder configuration + pub fn build(self) -> StdResult { let aggregator_endpoint = enforce_trailing_slash(self.aggregator_url_result.with_context( || "Invalid aggregator endpoint, it must be a correctly formed url", @@ -84,7 +84,7 @@ impl AggregatorClientBuilder { .proxy(Proxy::all(relay_endpoint).with_context(|| "Relay proxy creation failed")?) } - Ok(AggregatorClient { + Ok(AggregatorHttpClient { aggregator_endpoint, api_version_provider, additional_headers: (&additional_headers) diff --git a/internal/mithril-aggregator-client/src/client.rs b/internal/mithril-aggregator-client/src/client.rs index d51a30df8b4..eef31bbc1a3 100644 --- a/internal/mithril-aggregator-client/src/client.rs +++ b/internal/mithril-aggregator-client/src/client.rs @@ -8,15 +8,15 @@ use std::time::Duration; use mithril_common::MITHRIL_API_VERSION_HEADER; use mithril_common::api_version::APIVersionProvider; -use crate::AggregatorClientResult; +use crate::AggregatorHttpClientResult; use crate::builder::AggregatorClientBuilder; -use crate::error::AggregatorClientError; +use crate::error::AggregatorHttpClientError; use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; const API_VERSION_MISMATCH_WARNING_MESSAGE: &str = "OpenAPI version may be incompatible, please update Mithril client library to the latest version."; /// A client to send HTTP requests to a Mithril Aggregator -pub struct AggregatorClient { +pub struct AggregatorHttpClient { pub(super) aggregator_endpoint: Url, pub(super) api_version_provider: Arc, pub(super) additional_headers: HeaderMap, @@ -25,7 +25,7 @@ pub struct AggregatorClient { pub(super) logger: Logger, } -impl AggregatorClient { +impl AggregatorHttpClient { /// Creates a [AggregatorClientBuilder] to configure a `AggregatorClient`. // // This is the same as `AggregatorClient::builder()`. @@ -34,7 +34,10 @@ impl AggregatorClient { } /// Send the given query to the Mithril Aggregator - pub async fn send(&self, query: Q) -> AggregatorClientResult { + pub async fn send( + &self, + query: Q, + ) -> AggregatorHttpClientResult { // Todo: error handling ? Reuse the version in `warn_if_api_version_mismatch` ? let current_api_version = self.api_version_provider.compute_current_version().unwrap(); let mut request_builder = match Q::method() { @@ -62,11 +65,13 @@ impl AggregatorClient { }; query.handle_response(context).await } - Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), + Err(err) => Err(AggregatorHttpClientError::RemoteServerUnreachable(anyhow!( + err + ))), } } - fn join_aggregator_endpoint(&self, endpoint: &str) -> AggregatorClientResult { + fn join_aggregator_endpoint(&self, endpoint: &str) -> AggregatorHttpClientResult { self.aggregator_endpoint .join(endpoint) .with_context(|| { @@ -75,7 +80,7 @@ impl AggregatorClient { self.aggregator_endpoint ) }) - .map_err(AggregatorClientError::InvalidEndpoint) + .map_err(AggregatorHttpClientError::InvalidEndpoint) } /// Check API version mismatch and log a warning if the aggregator's version is more recent. @@ -141,13 +146,13 @@ mod tests { async fn handle_response( &self, context: QueryContext, - ) -> AggregatorClientResult { + ) -> AggregatorHttpClientResult { match context.response.status() { StatusCode::OK => context .response .json::() .await - .map_err(|err| AggregatorClientError::JsonParseFailed(anyhow!(err))), + .map_err(|err| AggregatorHttpClientError::JsonParseFailed(anyhow!(err))), _ => Err(context.unhandled_status_code().await), } } @@ -192,7 +197,7 @@ mod tests { async fn handle_response( &self, context: QueryContext, - ) -> AggregatorClientResult { + ) -> AggregatorHttpClientResult { match context.response.status() { StatusCode::CREATED => Ok(()), _ => Err(context.unhandled_status_code().await), @@ -293,7 +298,7 @@ mod tests { let error = client.send(TestGetQuery).await.expect_err("should not fail"); assert!( - matches!(error, AggregatorClientError::RemoteServerUnreachable(_)), + matches!(error, AggregatorHttpClientError::RemoteServerUnreachable(_)), "unexpected error type: {error:?}" ); } @@ -337,7 +342,7 @@ mod tests { let aggregator_version = "2.0.0"; let client_version = "1.0.0"; let (logger, log_inspector) = TestLogger::memory(); - let client = AggregatorClient::builder("http://whatever") + let client = AggregatorHttpClient::builder("http://whatever") .with_logger(logger) .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version( Version::parse(client_version).unwrap(), @@ -361,7 +366,7 @@ mod tests { fn test_no_warning_logged_when_versions_match() { let version = "1.0.0"; let (logger, log_inspector) = TestLogger::memory(); - let client = AggregatorClient::builder("http://whatever") + let client = AggregatorHttpClient::builder("http://whatever") .with_logger(logger) .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version( Version::parse(version).unwrap(), @@ -380,7 +385,7 @@ mod tests { let aggregator_version = "1.0.0"; let client_version = "2.0.0"; let (logger, log_inspector) = TestLogger::memory(); - let client = AggregatorClient::builder("http://whatever") + let client = AggregatorHttpClient::builder("http://whatever") .with_logger(logger) .with_api_version_provider(Arc::new(APIVersionProvider::new_with_default_version( Version::parse(client_version).unwrap(), @@ -403,7 +408,7 @@ mod tests { #[test] fn test_does_not_log_or_fail_when_header_is_missing() { let (logger, log_inspector) = TestLogger::memory(); - let client = AggregatorClient::builder("http://whatever") + let client = AggregatorHttpClient::builder("http://whatever") .with_logger(logger) .with_api_version_provider(Arc::new(APIVersionProvider::default())) .build() @@ -419,7 +424,7 @@ mod tests { #[test] fn test_does_not_log_or_fail_when_header_is_not_a_version() { let (logger, log_inspector) = TestLogger::memory(); - let client = AggregatorClient::builder("http://whatever") + let client = AggregatorHttpClient::builder("http://whatever") .with_logger(logger) .with_api_version_provider(Arc::new(APIVersionProvider::default())) .build() @@ -435,7 +440,7 @@ mod tests { #[test] fn test_logs_error_when_client_version_cannot_be_computed() { let (logger, log_inspector) = TestLogger::memory(); - let client = AggregatorClient::builder("http://whatever") + let client = AggregatorHttpClient::builder("http://whatever") .with_logger(logger) .with_api_version_provider(Arc::new(APIVersionProvider::new_failing())) .build() diff --git a/internal/mithril-aggregator-client/src/error.rs b/internal/mithril-aggregator-client/src/error.rs index 4232336107b..822c9cc7b92 100644 --- a/internal/mithril-aggregator-client/src/error.rs +++ b/internal/mithril-aggregator-client/src/error.rs @@ -9,7 +9,7 @@ use crate::JSON_CONTENT_TYPE; /// Error structure for the Aggregator Client. #[derive(Error, Debug)] -pub enum AggregatorClientError { +pub enum AggregatorHttpClientError { /// Error raised when querying the aggregator returned a 5XX error. #[error("Internal error of the Aggregator")] RemoteServerTechnical(#[source] StdError), @@ -39,7 +39,7 @@ pub enum AggregatorClientError { RegistrationRoundNotYetOpened(#[source] StdError), } -impl AggregatorClientError { +impl AggregatorHttpClientError { /// Create an `AggregatorClientError` from a response. /// /// This method is meant to be used after handling domain-specific cases leaving only @@ -133,12 +133,12 @@ mod tests { #[tokio::test] async fn test_4xx_errors_are_handled_as_remote_server_logical() { let response = build_text_response(StatusCode::BAD_REQUEST, "error text"); - let handled_error = AggregatorClientError::from_response(response).await; + let handled_error = AggregatorHttpClientError::from_response(response).await; assert!( matches!( handled_error, - AggregatorClientError::RemoteServerLogical(..) + AggregatorHttpClientError::RemoteServerLogical(..) ), "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'", ); @@ -147,12 +147,12 @@ mod tests { #[tokio::test] async fn test_5xx_errors_are_handled_as_remote_server_technical() { let response = build_text_response(StatusCode::INTERNAL_SERVER_ERROR, "error text"); - let handled_error = AggregatorClientError::from_response(response).await; + let handled_error = AggregatorHttpClientError::from_response(response).await; assert!( matches!( handled_error, - AggregatorClientError::RemoteServerTechnical(..) + AggregatorHttpClientError::RemoteServerTechnical(..) ), "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'", ); @@ -161,12 +161,12 @@ mod tests { #[tokio::test] async fn test_550_error_is_handled_as_registration_round_not_yet_opened() { let response = build_text_response(StatusCode::from_u16(550).unwrap(), "Not yet available"); - let handled_error = AggregatorClientError::from_response(response).await; + let handled_error = AggregatorHttpClientError::from_response(response).await; assert!( matches!( handled_error, - AggregatorClientError::RegistrationRoundNotYetOpened(..) + AggregatorHttpClientError::RegistrationRoundNotYetOpened(..) ), "Expected error to be RegistrationRoundNotYetOpened\ngot '{handled_error:?}'", ); @@ -176,12 +176,12 @@ mod tests { async fn test_non_4xx_or_5xx_errors_are_handled_as_unhandled_status_code_and_contains_response_text() { let response = build_text_response(StatusCode::OK, "ok text"); - let handled_error = AggregatorClientError::from_response(response).await; + let handled_error = AggregatorHttpClientError::from_response(response).await; assert!( matches!( handled_error, - AggregatorClientError::UnhandledStatusCode(..) if format!("{handled_error:?}").contains("ok text") + AggregatorHttpClientError::UnhandledStatusCode(..) if format!("{handled_error:?}").contains("ok text") ), "Expected error to be UnhandledStatusCode with 'ok text' in error text\ngot '{handled_error:?}'", ); @@ -193,7 +193,7 @@ mod tests { let response = build_text_response(StatusCode::EXPECTATION_FAILED, error_text); assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, + AggregatorHttpClientError::get_root_cause(response).await, "expectation failed: An error occurred; please try again later." ); } @@ -205,7 +205,7 @@ mod tests { let response = build_json_response(StatusCode::BAD_REQUEST, &client_error); assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, + AggregatorHttpClientError::get_root_cause(response).await, "bad request: label: message" ); } @@ -217,7 +217,7 @@ mod tests { let response = build_json_response(StatusCode::BAD_REQUEST, &server_error); assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, + AggregatorHttpClientError::get_root_cause(response).await, "bad request: message" ); } @@ -230,7 +230,7 @@ mod tests { ); assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, + AggregatorHttpClientError::get_root_cause(response).await, r#"internal server error: {"first":"foreign","second":"unknown"}"# ); } @@ -244,7 +244,7 @@ mod tests { .unwrap() .into(); - let root_cause = AggregatorClientError::get_root_cause(response).await; + let root_cause = AggregatorHttpClientError::get_root_cause(response).await; assert_error_text_contains!(root_cause, "bad request"); assert!( diff --git a/internal/mithril-aggregator-client/src/lib.rs b/internal/mithril-aggregator-client/src/lib.rs index 2549ef9f302..b60d55d1195 100644 --- a/internal/mithril-aggregator-client/src/lib.rs +++ b/internal/mithril-aggregator-client/src/lib.rs @@ -10,11 +10,11 @@ pub mod query; mod test; pub use builder::AggregatorClientBuilder; -pub use client::AggregatorClient; -pub use error::AggregatorClientError; +pub use client::AggregatorHttpClient; +pub use error::AggregatorHttpClientError; pub(crate) const JSON_CONTENT_TYPE: reqwest::header::HeaderValue = reqwest::header::HeaderValue::from_static("application/json"); /// Aggregator-client result type -pub type AggregatorClientResult = Result; +pub type AggregatorHttpClientResult = Result; diff --git a/internal/mithril-aggregator-client/src/query/api.rs b/internal/mithril-aggregator-client/src/query/api.rs index e083cf54e30..803102c897d 100644 --- a/internal/mithril-aggregator-client/src/query/api.rs +++ b/internal/mithril-aggregator-client/src/query/api.rs @@ -2,8 +2,8 @@ use reqwest::Response; use serde::de::DeserializeOwned; use slog::Logger; -use crate::AggregatorClientResult; -use crate::error::AggregatorClientError; +use crate::AggregatorHttpClientResult; +use crate::error::AggregatorHttpClientError; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum QueryMethod { @@ -28,7 +28,7 @@ pub trait AggregatorQuery { async fn handle_response( &self, context: QueryContext, - ) -> AggregatorClientResult; + ) -> AggregatorHttpClientResult; } pub struct QueryContext { @@ -37,7 +37,7 @@ pub struct QueryContext { } impl QueryContext { - pub async fn unhandled_status_code(self) -> AggregatorClientError { - AggregatorClientError::from_response(self.response).await + pub async fn unhandled_status_code(self) -> AggregatorHttpClientError { + AggregatorHttpClientError::from_response(self.response).await } } diff --git a/internal/mithril-aggregator-client/src/query/get/get_certificate.rs b/internal/mithril-aggregator-client/src/query/get/get_certificate.rs index 169ab754226..1201d90f0b9 100644 --- a/internal/mithril-aggregator-client/src/query/get/get_certificate.rs +++ b/internal/mithril-aggregator-client/src/query/get/get_certificate.rs @@ -5,8 +5,8 @@ use slog::debug; use mithril_common::messages::CertificateMessage; -use crate::AggregatorClientResult; -use crate::error::AggregatorClientError; +use crate::AggregatorHttpClientResult; +use crate::error::AggregatorHttpClientError; use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; /// Query to get the details of a certificate @@ -45,13 +45,13 @@ impl AggregatorQuery for GetCertificateQuery { async fn handle_response( &self, context: QueryContext, - ) -> AggregatorClientResult { + ) -> AggregatorHttpClientResult { debug!(context.logger, "Retrieve certificate details"; "certificate_hash" => %self.hash); match context.response.status() { StatusCode::OK => match context.response.json::().await { Ok(message) => Ok(Some(message)), - Err(err) => Err(AggregatorClientError::JsonParseFailed(anyhow!(err))), + Err(err) => Err(AggregatorHttpClientError::JsonParseFailed(anyhow!(err))), }, StatusCode::NOT_FOUND => Ok(None), _ => Err(context.unhandled_status_code().await), @@ -112,7 +112,7 @@ mod tests { .await .unwrap_err(); - assert_error_matches!(error, AggregatorClientError::RemoteServerTechnical(_)); + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); } #[tokio::test] @@ -152,6 +152,6 @@ mod tests { let error = client.send(GetCertificateQuery::latest_genesis()).await.unwrap_err(); - assert_error_matches!(error, AggregatorClientError::RemoteServerTechnical(_)); + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); } } diff --git a/internal/mithril-aggregator-client/src/test/mod.rs b/internal/mithril-aggregator-client/src/test/mod.rs index 08d2abe1edd..d0010379bfc 100644 --- a/internal/mithril-aggregator-client/src/test/mod.rs +++ b/internal/mithril-aggregator-client/src/test/mod.rs @@ -1,14 +1,14 @@ use httpmock::MockServer; -use crate::AggregatorClient; +use crate::AggregatorHttpClient; #[cfg(test)] mithril_common::define_test_logger!(); #[cfg(test)] -pub(crate) fn setup_server_and_client() -> (MockServer, AggregatorClient) { +pub(crate) fn setup_server_and_client() -> (MockServer, AggregatorHttpClient) { let server = MockServer::start(); - let client = AggregatorClient::builder(server.base_url()) + let client = AggregatorHttpClient::builder(server.base_url()) .with_logger(TestLogger::stdout()) .build() .unwrap(); From 9fe35e438c6d384134c37f157740c4838e60073d Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 28 Oct 2025 19:56:22 +0100 Subject: [PATCH 04/13] feat(aggregator-client): impl `CertificateRetriever` to the shared client --- .../src/external_trait_impls.rs | 93 +++++++++++++++++++ internal/mithril-aggregator-client/src/lib.rs | 1 + 2 files changed, 94 insertions(+) create mode 100644 internal/mithril-aggregator-client/src/external_trait_impls.rs diff --git a/internal/mithril-aggregator-client/src/external_trait_impls.rs b/internal/mithril-aggregator-client/src/external_trait_impls.rs new file mode 100644 index 00000000000..daad400bab8 --- /dev/null +++ b/internal/mithril-aggregator-client/src/external_trait_impls.rs @@ -0,0 +1,93 @@ +use anyhow::{Context, anyhow}; +use async_trait::async_trait; + +use mithril_common::certificate_chain::{CertificateRetriever, CertificateRetrieverError}; +use mithril_common::entities::Certificate; + +use crate::AggregatorHttpClient; +use crate::query::GetCertificateQuery; + +#[cfg_attr(target_family = "wasm", async_trait(?Send))] +#[cfg_attr(not(target_family = "wasm"), async_trait)] +impl CertificateRetriever for AggregatorHttpClient { + async fn get_certificate_details( + &self, + certificate_hash: &str, + ) -> Result { + let message = self + .send(GetCertificateQuery::by_hash(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) + } +} + +#[cfg(test)] +mod tests { + use mithril_common::{ + entities::ServerError, messages::CertificateMessage, test::double::Dummy, + }; + + use super::*; + + #[tokio::test] + async fn test_retrieve_certificate_that_exist() { + let certificate_message = CertificateMessage::dummy(); + let expected_certificate = certificate_message.clone().try_into().unwrap(); + + let (server, client) = crate::test::setup_server_and_client(); + server.mock(|when, then| { + when.method(httpmock::Method::GET) + .path(format!("/certificate/{}", certificate_message.hash)); + then.status(200) + .body(serde_json::to_string(&certificate_message).unwrap()); + }); + + let certificate = client + .get_certificate_details(&certificate_message.hash) + .await + .unwrap(); + + assert_eq!(certificate, expected_certificate); + } + + #[tokio::test] + async fn test_retrieve_certificate_that_does_not_exist() { + let (server, client) = crate::test::setup_server_and_client(); + server.mock(|when, then| { + when.method(httpmock::Method::GET); + then.status(404); + }); + + let error = client.get_certificate_details("whatever").await.unwrap_err(); + + assert!( + format!("{error:?}").contains("Certificate does not exist"), + "Error message should contain 'Certificate does not exist', error:\n{error:?}", + ); + } + + #[tokio::test] + async fn test_retrieve_certificate_when_request_fails() { + let (server, client) = crate::test::setup_server_and_client(); + server.mock(|when, then| { + when.method(httpmock::Method::GET); + then.status(500) + .body(serde_json::to_string(&ServerError::new("an error")).unwrap()); + }); + + let error = client.get_certificate_details("whatever").await.unwrap_err(); + + assert!( + format!("{error:?}").contains("Failed to retrieve certificate with hash"), + "Error message should contain 'Failed to retrieve certificate with hash', error:\n{error:?}", + ); + } +} diff --git a/internal/mithril-aggregator-client/src/lib.rs b/internal/mithril-aggregator-client/src/lib.rs index b60d55d1195..2ae905df9da 100644 --- a/internal/mithril-aggregator-client/src/lib.rs +++ b/internal/mithril-aggregator-client/src/lib.rs @@ -5,6 +5,7 @@ mod builder; mod client; mod error; +mod external_trait_impls; pub mod query; #[cfg(test)] mod test; From a07587de6c58a3171bedb3ecff73c4b276abbbaa Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 28 Oct 2025 19:30:41 +0100 Subject: [PATCH 05/13] feat(aggregator-client): add missing queries needed by a follower aggregator Add `GetCertificatesListQuery` and `GetEpochSettingsQuery` --- .../src/query/get/get_certificates_list.rs | 88 +++++++++++++++++++ .../src/query/get/get_epoch_settings.rs | 83 +++++++++++++++++ .../src/query/get/mod.rs | 4 + 3 files changed, 175 insertions(+) create mode 100644 internal/mithril-aggregator-client/src/query/get/get_certificates_list.rs create mode 100644 internal/mithril-aggregator-client/src/query/get/get_epoch_settings.rs diff --git a/internal/mithril-aggregator-client/src/query/get/get_certificates_list.rs b/internal/mithril-aggregator-client/src/query/get/get_certificates_list.rs new file mode 100644 index 00000000000..9755df37816 --- /dev/null +++ b/internal/mithril-aggregator-client/src/query/get/get_certificates_list.rs @@ -0,0 +1,88 @@ +use anyhow::anyhow; +use async_trait::async_trait; +use reqwest::StatusCode; + +use mithril_common::messages::CertificateListMessage; + +use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; +use crate::{AggregatorHttpClientError, AggregatorHttpClientResult}; + +/// Query to get a list of certificates +pub struct GetCertificatesListQuery {} + +impl GetCertificatesListQuery { + /// Instantiate a query to get the latest certificates list + pub fn latest() -> Self { + Self {} + } +} + +#[cfg_attr(target_family = "wasm", async_trait(?Send))] +#[cfg_attr(not(target_family = "wasm"), async_trait)] +impl AggregatorQuery for GetCertificatesListQuery { + type Response = CertificateListMessage; + type Body = (); + + fn method() -> QueryMethod { + QueryMethod::Get + } + + fn route(&self) -> String { + "certificates".to_string() + } + + async fn handle_response( + &self, + context: QueryContext, + ) -> AggregatorHttpClientResult { + match context.response.status() { + StatusCode::OK => match context.response.json::().await { + Ok(message) => Ok(message), + Err(err) => Err(AggregatorHttpClientError::JsonParseFailed(anyhow!(err))), + }, + _ => Err(context.unhandled_status_code().await), + } + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use mithril_common::messages::CertificateListItemMessage; + use mithril_common::test::double::Dummy; + + use crate::test::{assert_error_matches, setup_server_and_client}; + + use super::*; + + #[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.send(GetCertificatesListQuery::latest()).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"); + }); + + let error = client.send(GetCertificatesListQuery::latest()).await.unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); + } +} diff --git a/internal/mithril-aggregator-client/src/query/get/get_epoch_settings.rs b/internal/mithril-aggregator-client/src/query/get/get_epoch_settings.rs new file mode 100644 index 00000000000..42040c41c1a --- /dev/null +++ b/internal/mithril-aggregator-client/src/query/get/get_epoch_settings.rs @@ -0,0 +1,83 @@ +use anyhow::anyhow; +use async_trait::async_trait; +use reqwest::StatusCode; + +use mithril_common::messages::EpochSettingsMessage; + +use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; +use crate::{AggregatorHttpClientError, AggregatorHttpClientResult}; + +/// Query to get the current epoch settings +pub struct GetEpochSettingsQuery {} + +impl GetEpochSettingsQuery { + /// Instantiate a query to get the current epoch settings + pub fn current() -> Self { + Self {} + } +} + +#[cfg_attr(target_family = "wasm", async_trait(?Send))] +#[cfg_attr(not(target_family = "wasm"), async_trait)] +impl AggregatorQuery for GetEpochSettingsQuery { + type Response = EpochSettingsMessage; + type Body = (); + + fn method() -> QueryMethod { + QueryMethod::Get + } + + fn route(&self) -> String { + "epoch-settings".to_string() + } + + async fn handle_response( + &self, + context: QueryContext, + ) -> AggregatorHttpClientResult { + match context.response.status() { + StatusCode::OK => match context.response.json::().await { + Ok(message) => Ok(message), + Err(err) => Err(AggregatorHttpClientError::JsonParseFailed(anyhow!(err))), + }, + _ => Err(context.unhandled_status_code().await), + } + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use mithril_common::test::double::Dummy; + + use crate::test::{assert_error_matches, setup_server_and_client}; + + use super::*; + + #[tokio::test] + async fn test_epoch_settings_ok_200() { + let (server, client) = setup_server_and_client(); + let epoch_settings_expected = EpochSettingsMessage::dummy(); + let _server_mock = server.mock(|when, then| { + when.path("/epoch-settings"); + then.status(200).body(json!(epoch_settings_expected).to_string()); + }); + + let epoch_settings = client.send(GetEpochSettingsQuery::current()).await.unwrap(); + assert_eq!(epoch_settings_expected, epoch_settings); + } + + #[tokio::test] + async fn test_epoch_settings_ko_500() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/epoch-settings"); + then.status(500).body("an error occurred"); + }); + + let error = client.send(GetEpochSettingsQuery::current()).await.unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); + } +} diff --git a/internal/mithril-aggregator-client/src/query/get/mod.rs b/internal/mithril-aggregator-client/src/query/get/mod.rs index 51c8abee07e..3360631d06c 100644 --- a/internal/mithril-aggregator-client/src/query/get/mod.rs +++ b/internal/mithril-aggregator-client/src/query/get/mod.rs @@ -1,3 +1,7 @@ mod get_certificate; +mod get_certificates_list; +mod get_epoch_settings; pub use get_certificate::*; +pub use get_certificates_list::*; +pub use get_epoch_settings::*; From b6483bd6f34aee6d86694f97ac4d199e13c0fc27 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 29 Oct 2025 16:33:24 +0100 Subject: [PATCH 06/13] feat(aggregator-client): add `GetProtocolConfigurationQuery` --- .../query/get/get_protocol_configuration.rs | 109 ++++++++++++++++++ .../src/query/get/mod.rs | 2 + 2 files changed, 111 insertions(+) create mode 100644 internal/mithril-aggregator-client/src/query/get/get_protocol_configuration.rs diff --git a/internal/mithril-aggregator-client/src/query/get/get_protocol_configuration.rs b/internal/mithril-aggregator-client/src/query/get/get_protocol_configuration.rs new file mode 100644 index 00000000000..5fb02458ce3 --- /dev/null +++ b/internal/mithril-aggregator-client/src/query/get/get_protocol_configuration.rs @@ -0,0 +1,109 @@ +use anyhow::anyhow; +use async_trait::async_trait; +use mithril_common::entities::Epoch; +use mithril_common::messages::ProtocolConfigurationMessage; +use reqwest::StatusCode; + +use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; +use crate::{AggregatorHttpClientError, AggregatorHttpClientResult}; + +/// Query to get the current epoch settings +pub struct GetProtocolConfigurationQuery { + epoch: Epoch, +} + +impl GetProtocolConfigurationQuery { + /// Instantiate a query to get the current epoch settings + pub fn epoch(epoch: Epoch) -> Self { + Self { epoch } + } +} + +#[cfg_attr(target_family = "wasm", async_trait(?Send))] +#[cfg_attr(not(target_family = "wasm"), async_trait)] +impl AggregatorQuery for GetProtocolConfigurationQuery { + type Response = Option; + type Body = (); + + fn method() -> QueryMethod { + QueryMethod::Get + } + + fn route(&self) -> String { + format!("protocol-configuration/{}", self.epoch) + } + + async fn handle_response( + &self, + context: QueryContext, + ) -> AggregatorHttpClientResult { + match context.response.status() { + StatusCode::OK => match context.response.json::().await { + Ok(message) => Ok(Some(message)), + Err(err) => Err(AggregatorHttpClientError::JsonParseFailed(anyhow!(err))), + }, + StatusCode::NOT_FOUND => Ok(None), + _ => Err(context.unhandled_status_code().await), + } + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use mithril_common::test::double::Dummy; + + use crate::test::{assert_error_matches, setup_server_and_client}; + + use super::*; + + #[tokio::test] + async fn test_ok_200() { + let (server, client) = setup_server_and_client(); + let message_expected = ProtocolConfigurationMessage::dummy(); + let _server_mock = server.mock(|when, then| { + when.path("/protocol-configuration/42"); + then.status(200).body(json!(message_expected).to_string()); + }); + + let message = client + .send(GetProtocolConfigurationQuery::epoch(Epoch(42))) + .await + .unwrap(); + + assert_eq!(Some(message_expected), message); + } + + #[tokio::test] + async fn test_ok_404() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/protocol-configuration/42"); + then.status(404); + }); + + let message = client + .send(GetProtocolConfigurationQuery::epoch(Epoch(42))) + .await + .unwrap(); + + assert_eq!(None, message); + } + + #[tokio::test] + async fn test_ko_500() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.path("/protocol-configuration/42"); + then.status(500).body("an error occurred"); + }); + + let error = client + .send(GetProtocolConfigurationQuery::epoch(Epoch(42))) + .await + .expect_err("should throw a error"); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); + } +} diff --git a/internal/mithril-aggregator-client/src/query/get/mod.rs b/internal/mithril-aggregator-client/src/query/get/mod.rs index 3360631d06c..677974d0ccb 100644 --- a/internal/mithril-aggregator-client/src/query/get/mod.rs +++ b/internal/mithril-aggregator-client/src/query/get/mod.rs @@ -1,7 +1,9 @@ mod get_certificate; mod get_certificates_list; mod get_epoch_settings; +mod get_protocol_configuration; pub use get_certificate::*; pub use get_certificates_list::*; pub use get_epoch_settings::*; +pub use get_protocol_configuration::*; From a64620066f462b1712f4e873b0fa669759247fdb Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 28 Oct 2025 22:10:23 +0100 Subject: [PATCH 07/13] feat(aggregator-client): add missing query for use in `mithril-signer` add `GetAggregatorFeatures`, `PostRegisterSignatureQuery`, and `PostRegisterSignerQuery` queries. --- .../mithril-aggregator-client/src/error.rs | 2 +- .../src/query/get/get_aggregator_features.rs | 101 ++++++++++ .../src/query/get/mod.rs | 2 + .../src/query/mod.rs | 2 + .../src/query/post/mod.rs | 5 + .../src/query/post/post_register_signature.rs | 189 ++++++++++++++++++ .../src/query/post/post_register_signer.rs | 117 +++++++++++ 7 files changed, 417 insertions(+), 1 deletion(-) create mode 100644 internal/mithril-aggregator-client/src/query/post/mod.rs create mode 100644 internal/mithril-aggregator-client/src/query/post/post_register_signature.rs create mode 100644 internal/mithril-aggregator-client/src/query/post/post_register_signer.rs diff --git a/internal/mithril-aggregator-client/src/error.rs b/internal/mithril-aggregator-client/src/error.rs index 822c9cc7b92..0d8b2ef7b6c 100644 --- a/internal/mithril-aggregator-client/src/error.rs +++ b/internal/mithril-aggregator-client/src/error.rs @@ -63,7 +63,7 @@ impl AggregatorHttpClientError { } } - async fn get_root_cause(response: Response) -> String { + pub(crate) async fn get_root_cause(response: Response) -> String { let error_code = response.status(); let canonical_reason = error_code.canonical_reason().unwrap_or_default().to_lowercase(); let is_json = response diff --git a/internal/mithril-aggregator-client/src/query/get/get_aggregator_features.rs b/internal/mithril-aggregator-client/src/query/get/get_aggregator_features.rs index e69de29bb2d..50ae26fa504 100644 --- a/internal/mithril-aggregator-client/src/query/get/get_aggregator_features.rs +++ b/internal/mithril-aggregator-client/src/query/get/get_aggregator_features.rs @@ -0,0 +1,101 @@ +use anyhow::anyhow; +use async_trait::async_trait; +use reqwest::StatusCode; +use slog::debug; + +use mithril_common::messages::AggregatorFeaturesMessage; + +use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; +use crate::{AggregatorHttpClientError, AggregatorHttpClientResult}; + +/// Query to get the features of the aggregator +pub struct GetAggregatorFeaturesQuery {} + +impl GetAggregatorFeaturesQuery { + /// Instantiate a query to get the features of the aggregator + pub fn current() -> Self { + Self {} + } +} + +#[cfg_attr(target_family = "wasm", async_trait(?Send))] +#[cfg_attr(not(target_family = "wasm"), async_trait)] +impl AggregatorQuery for GetAggregatorFeaturesQuery { + type Response = AggregatorFeaturesMessage; + type Body = (); + + fn method() -> QueryMethod { + QueryMethod::Get + } + + fn route(&self) -> String { + "/".to_string() + } + + async fn handle_response( + &self, + context: QueryContext, + ) -> AggregatorHttpClientResult { + debug!(context.logger, "Retrieve aggregator features message"); + + match context.response.status() { + StatusCode::OK => Ok(context + .response + .json::() + .await + .map_err(|e| AggregatorHttpClientError::JsonParseFailed(anyhow!(e)))?), + _ => Err(context.unhandled_status_code().await), + } + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use mithril_common::test::double::Dummy; + + use crate::test::{assert_error_matches, setup_server_and_client}; + + use super::*; + + #[tokio::test] + async fn test_aggregator_features_ok_200() { + let (server, client) = setup_server_and_client(); + let message_expected = AggregatorFeaturesMessage::dummy(); + let _server_mock = server.mock(|when, then| { + when.path("/"); + then.status(200).body(json!(message_expected).to_string()); + }); + + let message = client.send(GetAggregatorFeaturesQuery::current()).await.unwrap(); + + assert_eq!(message_expected, message); + } + + #[tokio::test] + async fn test_aggregator_features_ko_500() { + let (server, client) = setup_server_and_client(); + server.mock(|when, then| { + when.any_request(); + then.status(500).body("an error occurred"); + }); + + let error = client.send(GetAggregatorFeaturesQuery::current()).await.unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); + } + + #[tokio::test] + async fn test_aggregator_features_ko_json_serialization() { + let (server, client) = setup_server_and_client(); + server.mock(|when, then| { + when.any_request(); + then.status(200).body("this is not a json"); + }); + + let error = client.send(GetAggregatorFeaturesQuery::current()).await.unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::JsonParseFailed(_)); + } +} diff --git a/internal/mithril-aggregator-client/src/query/get/mod.rs b/internal/mithril-aggregator-client/src/query/get/mod.rs index 677974d0ccb..750cbeb13b0 100644 --- a/internal/mithril-aggregator-client/src/query/get/mod.rs +++ b/internal/mithril-aggregator-client/src/query/get/mod.rs @@ -1,8 +1,10 @@ +mod get_aggregator_features; mod get_certificate; mod get_certificates_list; mod get_epoch_settings; mod get_protocol_configuration; +pub use get_aggregator_features::*; pub use get_certificate::*; pub use get_certificates_list::*; pub use get_epoch_settings::*; diff --git a/internal/mithril-aggregator-client/src/query/mod.rs b/internal/mithril-aggregator-client/src/query/mod.rs index 9c9f69aa42b..1908a99d972 100644 --- a/internal/mithril-aggregator-client/src/query/mod.rs +++ b/internal/mithril-aggregator-client/src/query/mod.rs @@ -5,6 +5,8 @@ //! mod api; mod get; +mod post; pub(crate) use api::*; pub use get::*; +pub use post::*; diff --git a/internal/mithril-aggregator-client/src/query/post/mod.rs b/internal/mithril-aggregator-client/src/query/post/mod.rs new file mode 100644 index 00000000000..cd3e4bbfa82 --- /dev/null +++ b/internal/mithril-aggregator-client/src/query/post/mod.rs @@ -0,0 +1,5 @@ +mod post_register_signature; +mod post_register_signer; + +pub use post_register_signature::*; +pub use post_register_signer::*; diff --git a/internal/mithril-aggregator-client/src/query/post/post_register_signature.rs b/internal/mithril-aggregator-client/src/query/post/post_register_signature.rs new file mode 100644 index 00000000000..2a3d8694f2e --- /dev/null +++ b/internal/mithril-aggregator-client/src/query/post/post_register_signature.rs @@ -0,0 +1,189 @@ +use async_trait::async_trait; +use reqwest::StatusCode; +use slog::debug; + +use mithril_common::messages::RegisterSignatureMessageHttp; + +use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; +use crate::{AggregatorHttpClientError, AggregatorHttpClientResult}; + +/// Query to register a signature to a Mithril Aggregator. +pub struct PostRegisterSignatureQuery { + message: RegisterSignatureMessageHttp, +} + +impl PostRegisterSignatureQuery { + /// Instantiate a new query to register a signature + pub fn new(message: RegisterSignatureMessageHttp) -> Self { + Self { message } + } +} + +#[cfg_attr(target_family = "wasm", async_trait(?Send))] +#[cfg_attr(not(target_family = "wasm"), async_trait)] +impl AggregatorQuery for PostRegisterSignatureQuery { + type Response = (); + type Body = RegisterSignatureMessageHttp; + + fn method() -> QueryMethod { + QueryMethod::Post + } + + fn route(&self) -> String { + "register-signatures".to_string() + } + + fn body(&self) -> Option { + Some(self.message.clone()) + } + + async fn handle_response( + &self, + context: QueryContext, + ) -> AggregatorHttpClientResult { + debug!(context.logger, "Register signature"; "signed_entity" => ?self.message.signed_entity_type); + + match context.response.status() { + StatusCode::CREATED | StatusCode::ACCEPTED => Ok(()), + StatusCode::GONE => { + let root_cause = AggregatorHttpClientError::get_root_cause(context.response).await; + debug!(context.logger, "Message already certified or expired"; "details" => &root_cause); + + Ok(()) + } + _ => Err(context.unhandled_status_code().await), + } + } +} + +#[cfg(test)] +mod tests { + use httpmock::Method::POST; + + use mithril_common::entities::ClientError; + use mithril_common::test::double::Dummy; + + use crate::test::{TestLogger, assert_error_matches, setup_server_and_client}; + + use super::*; + + #[tokio::test] + async fn test_register_signature_ok_201() { + let expected_message = RegisterSignatureMessageHttp::dummy(); + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST) + .path("/register-signatures") + .body(serde_json::to_string(&expected_message).unwrap()); + then.status(201); + }); + + let register_signature = + client.send(PostRegisterSignatureQuery::new(expected_message)).await; + register_signature.expect("unexpected error"); + } + + #[tokio::test] + async fn test_register_signature_ok_202() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST).path("/register-signatures"); + then.status(202); + }); + + let register_signature = client + .send(PostRegisterSignatureQuery::new( + RegisterSignatureMessageHttp::dummy(), + )) + .await; + register_signature.expect("unexpected error"); + } + + #[tokio::test] + async fn test_register_signature_ko_400() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST).path("/register-signatures"); + then.status(400).body( + serde_json::to_vec(&ClientError::new( + "error".to_string(), + "an error".to_string(), + )) + .unwrap(), + ); + }); + + let error = client + .send(PostRegisterSignatureQuery::new( + RegisterSignatureMessageHttp::dummy(), + )) + .await + .unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerLogical(_)); + } + + #[tokio::test] + async fn test_register_signature_ok_410_log_response_body() { + let (logger, log_inspector) = TestLogger::memory(); + + let (server, mut client) = setup_server_and_client(); + client.logger = logger; + let _server_mock = server.mock(|when, then| { + when.method(POST).path("/register-signatures"); + then.status(410).body( + serde_json::to_vec(&ClientError::new( + "already_aggregated".to_string(), + "too late".to_string(), + )) + .unwrap(), + ); + }); + + client + .send(PostRegisterSignatureQuery::new( + RegisterSignatureMessageHttp::dummy(), + )) + .await + .expect("Should not fail when status is 410 (GONE)"); + + assert!(log_inspector.contains_log("already_aggregated")); + assert!(log_inspector.contains_log("too late")); + } + + #[tokio::test] + async fn test_register_signature_ko_409() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST).path("/register-signatures"); + then.status(409); + }); + + let error = client + .send(PostRegisterSignatureQuery::new( + RegisterSignatureMessageHttp::dummy(), + )) + .await + .unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerLogical(_)); + } + + #[tokio::test] + async fn test_register_signature_ko_500() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST).path("/register-signatures"); + then.status(500).body("an error occurred"); + }); + + let error = client + .send(PostRegisterSignatureQuery::new( + RegisterSignatureMessageHttp::dummy(), + )) + .await + .unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); + } +} diff --git a/internal/mithril-aggregator-client/src/query/post/post_register_signer.rs b/internal/mithril-aggregator-client/src/query/post/post_register_signer.rs new file mode 100644 index 00000000000..9dccac1ad98 --- /dev/null +++ b/internal/mithril-aggregator-client/src/query/post/post_register_signer.rs @@ -0,0 +1,117 @@ +use async_trait::async_trait; +use reqwest::StatusCode; +use slog::debug; + +use mithril_common::messages::RegisterSignerMessage; + +use crate::AggregatorHttpClientResult; +use crate::query::{AggregatorQuery, QueryContext, QueryMethod}; + +/// Query to register a signer to a Mithril Aggregator. +pub struct PostRegisterSignerQuery { + message: RegisterSignerMessage, +} + +impl PostRegisterSignerQuery { + /// Instantiate a new query to register a signer + pub fn new(message: RegisterSignerMessage) -> Self { + Self { message } + } +} + +#[cfg_attr(target_family = "wasm", async_trait(?Send))] +#[cfg_attr(not(target_family = "wasm"), async_trait)] +impl AggregatorQuery for PostRegisterSignerQuery { + type Response = (); + type Body = RegisterSignerMessage; + + fn method() -> QueryMethod { + QueryMethod::Post + } + + fn route(&self) -> String { + "register-signer".to_string() + } + + fn body(&self) -> Option { + Some(self.message.clone()) + } + + async fn handle_response( + &self, + context: QueryContext, + ) -> AggregatorHttpClientResult { + debug!(context.logger, "Register signer"; "epoch" => *self.message.epoch, "party_id" => &self.message.party_id); + + match context.response.status() { + StatusCode::CREATED => Ok(()), + _ => Err(context.unhandled_status_code().await), + } + } +} + +#[cfg(test)] +mod tests { + use httpmock::Method::POST; + + use mithril_common::entities::ClientError; + use mithril_common::test::double::Dummy; + + use crate::AggregatorHttpClientError; + use crate::test::{assert_error_matches, setup_server_and_client}; + + use super::*; + + #[tokio::test] + async fn test_register_signer_ok_201() { + let expected_message = RegisterSignerMessage::dummy(); + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST) + .path("/register-signer") + .body(serde_json::to_string(&expected_message).unwrap()); + then.status(201); + }); + + let register_signer = client.send(PostRegisterSignerQuery::new(expected_message)).await; + register_signer.expect("unexpected error"); + } + + #[tokio::test] + async fn test_register_signer_ko_400() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST).path("/register-signer"); + then.status(400).body( + serde_json::to_vec(&ClientError::new( + "error".to_string(), + "an error".to_string(), + )) + .unwrap(), + ); + }); + + let error = client + .send(PostRegisterSignerQuery::new(RegisterSignerMessage::dummy())) + .await + .unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerLogical(_)); + } + + #[tokio::test] + async fn test_register_signer_ko_500() { + let (server, client) = setup_server_and_client(); + let _server_mock = server.mock(|when, then| { + when.method(POST).path("/register-signer"); + then.status(500).body("an error occurred"); + }); + + let error = client + .send(PostRegisterSignerQuery::new(RegisterSignerMessage::dummy())) + .await + .unwrap_err(); + + assert_error_matches!(error, AggregatorHttpClientError::RemoteServerTechnical(_)); + } +} From 11709aac42c6c31399dd568a44cad479f9949806 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 28 Oct 2025 20:32:39 +0100 Subject: [PATCH 08/13] refactor(aggregator): use new shared aggregator client instead of an internal, copied and adapted over the signer, implementation --- Cargo.lock | 1 + mithril-aggregator/Cargo.toml | 1 + .../builder/enablers/misc.rs | 34 +- .../src/dependency_injection/builder/mod.rs | 5 +- .../src/services/aggregator_client.rs | 1006 +---------------- mithril-aggregator/src/services/mod.rs | 1 - 6 files changed, 35 insertions(+), 1013 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17c98d5a82c..b0e92be8070 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3610,6 +3610,7 @@ dependencies = [ "hex", "http", "httpmock", + "mithril-aggregator-client", "mithril-api-spec", "mithril-cardano-node-chain", "mithril-cardano-node-internal-database", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 333b71044aa..24aa0f64047 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -26,6 +26,7 @@ flate2 = "1.1.4" gcloud-kms = { version = "1.3.1", features = ["auth"] } gcloud-storage = { version = "1.1.1", features = ["auth"] } hex = { workspace = true } +mithril-aggregator-client = { path = "../internal/mithril-aggregator-client" } mithril-cardano-node-chain = { path = "../internal/cardano-node/mithril-cardano-node-chain" } mithril-cardano-node-internal-database = { path = "../internal/cardano-node/mithril-cardano-node-internal-database" } mithril-cli-helper = { path = "../internal/mithril-cli-helper" } diff --git a/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs b/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs index 6e8e0dc2780..9d7f8d49c9a 100644 --- a/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs +++ b/mithril-aggregator/src/dependency_injection/builder/enablers/misc.rs @@ -4,11 +4,12 @@ //! - 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 anyhow::anyhow; use std::sync::Arc; use std::time::Duration; +use mithril_aggregator_client::AggregatorHttpClient; +use mithril_common::logging::LoggerExtensions; #[cfg(feature = "future_dmq")] use mithril_common::messages::RegisterSignatureMessageDmq; #[cfg(feature = "future_dmq")] @@ -21,9 +22,10 @@ use crate::get_dependency; #[cfg(feature = "future_dmq")] use crate::services::SignatureConsumerDmq; use crate::services::{ - AggregatorHTTPClient, MessageService, MithrilMessageService, SequentialSignatureProcessor, - SignatureConsumer, SignatureConsumerNoop, SignatureProcessor, + MessageService, MithrilMessageService, SequentialSignatureProcessor, SignatureConsumer, + SignatureConsumerNoop, SignatureProcessor, }; + impl DependenciesBuilder { async fn build_signed_entity_type_lock(&mut self) -> Result> { let signed_entity_lock = Arc::new(SignedEntityTypeLock::default()); @@ -60,29 +62,23 @@ impl DependenciesBuilder { get_dependency!(self.message_service) } - /// Builds an [AggregatorHTTPClient] - 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"), )?; - let aggregator_client = AggregatorHTTPClient::new( - 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)), - self.root_logger(), - ); + let aggregator_client = AggregatorHttpClient::builder(&leader_aggregator_endpoint) + .with_api_version_provider(self.get_api_version_provider().await?) + .with_timeout(Duration::from_secs(30)) + .with_logger(self.root_logger.new_with_name("LeaderAggregatorClient")) + .build()?; Ok(Arc::new(aggregator_client)) } - /// Returns a leader [AggregatorHTTPClient] - 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 6b34125990c..5e84c8a541a 100644 --- a/mithril-aggregator/src/dependency_injection/builder/mod.rs +++ b/mithril-aggregator/src/dependency_injection/builder/mod.rs @@ -15,6 +15,7 @@ use tokio::{ }; use warp::Filter; +use mithril_aggregator_client::AggregatorHttpClient; use mithril_cardano_node_chain::{ chain_observer::{CardanoCliRunner, ChainObserver}, chain_reader::ChainBlockReader, @@ -56,7 +57,7 @@ use crate::{ file_uploaders::FileUploader, http_server::routes::router::{self, RouterConfig, RouterState}, services::{ - AggregatorHTTPClient, CertificateChainSynchronizer, CertifierService, MessageService, + CertificateChainSynchronizer, CertifierService, MessageService, MithrilSignerRegistrationFollower, ProverService, SignedEntityService, SignerSynchronizer, Snapshotter, StakeDistributionService, UpkeepService, }, @@ -278,7 +279,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 1604f920d9e..826a7188ca3 100644 --- a/mithril-aggregator/src/services/aggregator_client.rs +++ b/mithril-aggregator/src/services/aggregator_client.rs @@ -1,1021 +1,45 @@ -use anyhow::{Context, anyhow}; use async_trait::async_trait; -use reqwest::header::{self, HeaderValue}; -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}; -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::{ - CertificateListMessage, CertificateMessage, EpochSettingsMessage, TryFromMessageAdapter, - }, +use mithril_aggregator_client::{ + AggregatorHttpClient, + query::{GetCertificateQuery, GetCertificatesListQuery, GetEpochSettingsQuery}, }; +use mithril_common::{StdResult, entities::Certificate, messages::TryFromMessageAdapter}; use crate::entities::LeaderAggregatorEpochSettings; use crate::message_adapters::FromEpochSettingsAdapter; use crate::services::{LeaderAggregatorClient, RemoteCertificateRetriever}; -const JSON_CONTENT_TYPE: HeaderValue = HeaderValue::from_static("application/json"); - -const API_VERSION_MISMATCH_WARNING_MESSAGE: &str = - "OpenAPI version may be incompatible, please update your Mithril node to the latest version."; - -/// Error structure for the Aggregator Client. -#[derive(Error, Debug)] -pub enum AggregatorClientError { - /// The aggregator host has returned a technical error. - #[error("remote server technical error")] - RemoteServerTechnical(#[source] StdError), - - /// The aggregator host responded it cannot fulfill our request. - #[error("remote server logical error")] - RemoteServerLogical(#[source] StdError), - - /// Could not reach aggregator. - #[error("remote server unreachable")] - RemoteServerUnreachable(#[source] StdError), - - /// Unhandled status code - #[error("unhandled status code: {0}, response text: {1}")] - UnhandledStatusCode(StatusCode, String), - - /// Could not parse response. - #[error("json parsing failed")] - JsonParseFailed(#[source] StdError), - - /// Mostly network errors. - #[error("Input/Output error")] - IOError(#[from] io::Error), - - /// HTTP client creation error - #[error("HTTP client creation failed")] - HTTPClientCreation(#[source] StdError), - - /// Proxy creation error - #[error("proxy creation failed")] - ProxyCreation(#[source] StdError), - - /// Adapter error - #[error("adapter failed")] - Adapter(#[source] StdError), -} - -impl AggregatorClientError { - /// Create an `AggregatorClientError` from a response. - /// - /// This method is meant to be used after handling domain-specific cases leaving only - /// 4xx or 5xx status codes. - /// Otherwise, it will return an `UnhandledStatusCode` error. - pub async fn from_response(response: Response) -> Self { - let error_code = response.status(); - - if error_code.is_client_error() { - let root_cause = Self::get_root_cause(response).await; - Self::RemoteServerLogical(anyhow!(root_cause)) - } else if error_code.is_server_error() { - let root_cause = Self::get_root_cause(response).await; - Self::RemoteServerTechnical(anyhow!(root_cause)) - } else { - let response_text = response.text().await.unwrap_or_default(); - Self::UnhandledStatusCode(error_code, response_text) - } - } - - async fn get_root_cause(response: Response) -> String { - let error_code = response.status(); - let canonical_reason = error_code.canonical_reason().unwrap_or_default().to_lowercase(); - let is_json = response - .headers() - .get(header::CONTENT_TYPE) - .is_some_and(|ct| JSON_CONTENT_TYPE == ct); - - if is_json { - let json_value: serde_json::Value = response.json().await.unwrap_or_default(); - - if let Ok(client_error) = serde_json::from_value::(json_value.clone()) { - format!( - "{}: {}: {}", - canonical_reason, client_error.label, client_error.message - ) - } else if let Ok(server_error) = - serde_json::from_value::(json_value.clone()) - { - format!("{}: {}", canonical_reason, server_error.message) - } else if json_value.is_null() { - canonical_reason.to_string() - } else { - format!("{canonical_reason}: {json_value}") - } - } else { - let response_text = response.text().await.unwrap_or_default(); - format!("{canonical_reason}: {response_text}") - } - } -} - -/// AggregatorHTTPClient is a http client for an aggregator -pub struct AggregatorHTTPClient { - aggregator_endpoint: Url, - relay_endpoint: Option, - api_version_provider: Arc, - timeout_duration: Option, - logger: Logger, -} - -impl AggregatorHTTPClient { - /// AggregatorHTTPClient factory - pub fn new( - aggregator_endpoint: Url, - relay_endpoint: Option, - api_version_provider: Arc, - timeout_duration: Option, - logger: Logger, - ) -> 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, - api_version_provider, - timeout_duration, - logger, - } - } - - 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() - .proxy( - Proxy::all(relay_endpoint) - .map_err(|e| AggregatorClientError::ProxyCreation(anyhow!(e)))?, - ) - .build() - .map_err(|e| AggregatorClientError::HTTPClientCreation(anyhow!(e)))?, - None => Client::new(), - }; - - Ok(client) - } - - /// Forge a client request adding protocol version in the headers. - pub fn prepare_request_builder(&self, request_builder: RequestBuilder) -> RequestBuilder { - let request_builder = request_builder - .header( - MITHRIL_API_VERSION_HEADER, - self.api_version_provider - .compute_current_version() - .unwrap() - .to_string(), - ) - .header(MITHRIL_AGGREGATOR_VERSION_HEADER, env!("CARGO_PKG_VERSION")); - - if let Some(duration) = self.timeout_duration { - request_builder.timeout(duration) - } else { - request_builder - } - } - - /// Check API version mismatch and log a warning if the leader aggregator's version is more recent. - fn warn_if_api_version_mismatch(&self, response: &Response) { - let leader_version = response - .headers() - .get(MITHRIL_API_VERSION_HEADER) - .and_then(|v| v.to_str().ok()) - .and_then(|s| Version::parse(s).ok()); - - let follower_version = self.api_version_provider.compute_current_version(); - - match (leader_version, follower_version) { - (Some(leader), Ok(follower)) if follower < leader => { - warn!(self.logger, "{}", API_VERSION_MISMATCH_WARNING_MESSAGE; - "leader_aggregator_version" => %leader, - "aggregator_version" => %follower, - ); - } - (Some(_), Err(error)) => { - error!( - self.logger, - "Failed to compute the current aggregator API version"; - "error" => error.to_string() - ); - } - _ => {} - } - } -} - -// Route specifics methods -impl AggregatorHTTPClient { - async fn epoch_settings( - &self, - ) -> Result, AggregatorClientError> { - debug!(self.logger, "Retrieve epoch settings"); - let url = self.join_aggregator_endpoint("epoch-settings")?; - 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) => { - let epoch_settings = FromEpochSettingsAdapter::try_adapt(message) - .map_err(|e| AggregatorClientError::Adapter(anyhow!(e)))?; - Ok(Some(epoch_settings)) - } - Err(err) => Err(AggregatorClientError::JsonParseFailed(anyhow!(err))), - } - } - _ => Err(AggregatorClientError::from_response(response).await), - }, - 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 certificate_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.certificate_details("genesis").await - } -} - #[async_trait] -impl LeaderAggregatorClient for AggregatorHTTPClient { +impl LeaderAggregatorClient for AggregatorHttpClient { async fn retrieve_epoch_settings(&self) -> StdResult> { - let epoch_settings = self.epoch_settings().await?; - Ok(epoch_settings) + let epoch_settings = self.send(GetEpochSettingsQuery::current()).await?; + FromEpochSettingsAdapter::try_adapt(epoch_settings).map(Some) } } #[async_trait] -impl CertificateRetriever for AggregatorHTTPClient { - async fn get_certificate_details( - &self, - certificate_hash: &str, - ) -> Result { - let message = self - .certificate_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 { +impl RemoteCertificateRetriever for AggregatorHttpClient { async fn get_latest_certificate_details(&self) -> StdResult> { - let latest_certificates_list = self.latest_certificates_list().await?; + let latest_certificates_list = self.send(GetCertificatesListQuery::latest()).await?; match latest_certificates_list.first() { None => Ok(None), Some(latest_certificate_list_item) => { - let latest_certificate_message = - self.certificate_details(&latest_certificate_list_item.hash).await?; + let latest_certificate_message = self + .send(GetCertificateQuery::by_hash( + &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? { + match self.send(GetCertificateQuery::latest_genesis()).await? { Some(message) => Ok(Some(message.try_into()?)), None => Ok(None), } } } - -#[cfg(test)] -mod tests { - use http::response::Builder as HttpResponseBuilder; - use httpmock::prelude::*; - use reqwest::IntoUrl; - use serde_json::json; - - use mithril_common::messages::CertificateListItemMessage; - use mithril_common::test::double::{Dummy, DummyApiVersionDiscriminantSource}; - - use crate::test::TestLogger; - - use super::*; - - 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_url().unwrap(), - None, - Arc::new(api_version_provider), - None, - TestLogger::stdout(), - ) - } - - fn setup_server_and_client() -> (MockServer, AggregatorHTTPClient) { - let server = MockServer::start(); - let aggregator_endpoint = server.url(""); - let client = setup_client(&aggregator_endpoint); - - (server, client) - } - - fn build_text_response>(status_code: StatusCode, body: T) -> Response { - HttpResponseBuilder::new() - .status(status_code) - .body(body.into()) - .unwrap() - .into() - } - - fn build_json_response(status_code: StatusCode, body: &T) -> Response { - HttpResponseBuilder::new() - .status(status_code) - .header(header::CONTENT_TYPE, JSON_CONTENT_TYPE) - .body(serde_json::to_string(&body).unwrap()) - .unwrap() - .into() - } - - macro_rules! assert_error_text_contains { - ($error: expr, $expect_contains: expr) => { - let error = &$error; - assert!( - error.contains($expect_contains), - "Expected error message to contain '{}'\ngot '{error:?}'", - $expect_contains, - ); - }; - } - - #[tokio::test] - async fn test_epoch_settings_ok_200() { - let (server, client) = setup_server_and_client(); - let epoch_settings_expected = EpochSettingsMessage::dummy(); - let _server_mock = server.mock(|when, then| { - when.path("/epoch-settings"); - then.status(200).body(json!(epoch_settings_expected).to_string()); - }); - - let epoch_settings = client.retrieve_epoch_settings().await; - epoch_settings.as_ref().expect("unexpected error"); - assert_eq!( - FromEpochSettingsAdapter::try_adapt(epoch_settings_expected).unwrap(), - epoch_settings.unwrap().unwrap() - ); - } - - #[tokio::test] - async fn test_epoch_settings_ko_500() { - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.path("/epoch-settings"); - then.status(500).body("an error occurred"); - }); - - match client.epoch_settings().await.unwrap_err() { - AggregatorClientError::RemoteServerTechnical(_) => (), - e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), - }; - } - - #[tokio::test] - async fn test_epoch_settings_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("/epoch-settings"); - then.delay(Duration::from_millis(100)); - }); - - let error = client - .epoch_settings() - .await - .expect_err("retrieve_epoch_settings should fail"); - - assert!( - matches!(error, AggregatorClientError::RemoteServerUnreachable(_)), - "unexpected error type: {error:?}" - ); - } - - #[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.certificate_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.certificate_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 - .certificate_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"); - let handled_error = AggregatorClientError::from_response(response).await; - - assert!( - matches!( - handled_error, - AggregatorClientError::RemoteServerLogical(..) - ), - "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'", - ); - } - - #[tokio::test] - async fn test_5xx_errors_are_handled_as_remote_server_technical() { - let response = build_text_response(StatusCode::INTERNAL_SERVER_ERROR, "error text"); - let handled_error = AggregatorClientError::from_response(response).await; - - assert!( - matches!( - handled_error, - AggregatorClientError::RemoteServerTechnical(..) - ), - "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'", - ); - } - - #[tokio::test] - async fn test_non_4xx_or_5xx_errors_are_handled_as_unhandled_status_code_and_contains_response_text() - { - let response = build_text_response(StatusCode::OK, "ok text"); - let handled_error = AggregatorClientError::from_response(response).await; - - assert!( - matches!( - handled_error, - AggregatorClientError::UnhandledStatusCode(..) if format!("{handled_error:?}").contains("ok text") - ), - "Expected error to be UnhandledStatusCode with 'ok text' in error text\ngot '{handled_error:?}'", - ); - } - - #[tokio::test] - async fn test_root_cause_of_non_json_response_contains_response_plain_text() { - let error_text = "An error occurred; please try again later."; - let response = build_text_response(StatusCode::EXPECTATION_FAILED, error_text); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - "expectation failed: An error occurred; please try again later." - ); - } - - #[tokio::test] - async fn test_root_cause_of_json_formatted_client_error_response_contains_error_label_and_message() - { - let client_error = ClientError::new("label", "message"); - let response = build_json_response(StatusCode::BAD_REQUEST, &client_error); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - "bad request: label: message" - ); - } - - #[tokio::test] - async fn test_root_cause_of_json_formatted_server_error_response_contains_error_label_and_message() - { - let server_error = ServerError::new("message"); - let response = build_json_response(StatusCode::BAD_REQUEST, &server_error); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - "bad request: message" - ); - } - - #[tokio::test] - async fn test_root_cause_of_unknown_formatted_json_response_contains_json_key_value_pairs() { - let response = build_json_response( - StatusCode::INTERNAL_SERVER_ERROR, - &json!({ "second": "unknown", "first": "foreign" }), - ); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - r#"internal server error: {"first":"foreign","second":"unknown"}"# - ); - } - - #[tokio::test] - async fn test_root_cause_with_invalid_json_response_still_contains_response_status_name() { - let response = HttpResponseBuilder::new() - .status(StatusCode::BAD_REQUEST) - .header(header::CONTENT_TYPE, JSON_CONTENT_TYPE) - .body(r#"{"invalid":"unexpected dot", "key": "value".}"#) - .unwrap() - .into(); - - let root_cause = AggregatorClientError::get_root_cause(response).await; - - assert_error_text_contains!(root_cause, "bad request"); - assert!( - !root_cause.contains("bad request: "), - "Expected error message should not contain additional information \ngot '{root_cause:?}'" - ); - } - - mod warn_if_api_version_mismatch { - use std::collections::HashMap; - - use mithril_common::test::api_version_extensions::ApiVersionProviderTestExtension; - use mithril_common::test::logging::MemoryDrainForTestInspector; - - use super::*; - - fn version_provider_with_open_api_version>( - version: V, - ) -> APIVersionProvider { - let mut version_provider = version_provider_without_open_api_version(); - let mut open_api_versions = HashMap::new(); - open_api_versions.insert( - "openapi.yaml".to_string(), - Version::parse(&version.into()).unwrap(), - ); - version_provider.update_open_api_versions(open_api_versions); - - version_provider - } - - fn version_provider_without_open_api_version() -> APIVersionProvider { - let mut version_provider = - APIVersionProvider::new(Arc::new(DummyApiVersionDiscriminantSource::default())); - version_provider.update_open_api_versions(HashMap::new()); - - version_provider - } - - fn build_fake_response_with_header, V: Into>( - key: K, - value: V, - ) -> Response { - HttpResponseBuilder::new() - .header(key.into(), value.into()) - .body("whatever") - .unwrap() - .into() - } - - fn assert_api_version_warning_logged, A: Into>( - log_inspector: &MemoryDrainForTestInspector, - leader_aggregator_version: L, - aggregator_version: A, - ) { - assert!(log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - assert!(log_inspector.contains_log(&format!( - "leader_aggregator_version={}", - leader_aggregator_version.into() - ))); - assert!( - log_inspector - .contains_log(&format!("aggregator_version={}", aggregator_version.into())) - ); - } - - #[test] - fn test_logs_warning_when_leader_aggregator_api_version_is_newer() { - let leader_aggregator_version = "2.0.0"; - 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("http://whatever"); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let response = build_fake_response_with_header( - MITHRIL_API_VERSION_HEADER, - leader_aggregator_version, - ); - - assert!( - Version::parse(leader_aggregator_version).unwrap() - > Version::parse(aggregator_version).unwrap() - ); - - client.warn_if_api_version_mismatch(&response); - - assert_api_version_warning_logged( - &log_inspector, - leader_aggregator_version, - aggregator_version, - ); - } - - #[test] - fn test_no_warning_logged_when_versions_match() { - 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("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); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[test] - fn test_no_warning_logged_when_leader_aggregator_api_version_is_older() { - let leader_aggregator_version = "1.0.0"; - 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("http://whatever"); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let response = build_fake_response_with_header( - MITHRIL_API_VERSION_HEADER, - leader_aggregator_version, - ); - - assert!( - Version::parse(leader_aggregator_version).unwrap() - < Version::parse(aggregator_version).unwrap() - ); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[test] - fn test_does_not_log_or_fail_when_header_is_missing() { - let (logger, log_inspector) = TestLogger::memory(); - let mut client = setup_client("http://whatever"); - client.logger = logger; - let response = - build_fake_response_with_header("NotMithrilAPIVersionHeader", "whatever"); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[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("http://whatever"); - client.logger = logger; - let response = - build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, "not_a_version"); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[test] - 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("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"); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[tokio::test] - async fn test_epoch_settings_ok_200_log_warning_if_api_version_mismatch() { - let leader_aggregator_version = "2.0.0"; - let aggregator_version = "1.0.0"; - let (server, mut client) = setup_server_and_client(); - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(aggregator_version); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let epoch_settings_expected = EpochSettingsMessage::dummy(); - let _server_mock = server.mock(|when, then| { - when.path("/epoch-settings"); - then.status(200) - .body(json!(epoch_settings_expected).to_string()) - .header(MITHRIL_API_VERSION_HEADER, leader_aggregator_version); - }); - - assert!( - Version::parse(leader_aggregator_version).unwrap() - > Version::parse(aggregator_version).unwrap() - ); - - client.retrieve_epoch_settings().await.unwrap(); - - assert_api_version_warning_logged( - &log_inspector, - leader_aggregator_version, - aggregator_version, - ); - } - } - - mod remote_certificate_retriever { - use mithril_common::test::double::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); - } - } -} diff --git a/mithril-aggregator/src/services/mod.rs b/mithril-aggregator/src/services/mod.rs index d8443da5f65..26c053b63e4 100644 --- a/mithril-aggregator/src/services/mod.rs +++ b/mithril-aggregator/src/services/mod.rs @@ -26,7 +26,6 @@ mod stake_distribution; mod upkeep; mod usage_reporter; -pub use aggregator_client::*; pub use cardano_transactions_importer::*; pub use certificate_chain_synchronizer::*; pub use certifier::*; From 464027b81747c037ed2dc9995aeb49708a7ece9a Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Wed, 29 Oct 2025 16:32:52 +0100 Subject: [PATCH 09/13] refactor(protocol-config): use new shared aggregator client --- Cargo.lock | 5 +- internal/mithril-protocol-config/Cargo.toml | 4 +- .../src/{http_client/http_impl.rs => http.rs} | 139 ++++++++---------- internal/mithril-protocol-config/src/lib.rs | 5 +- mithril-signer/Cargo.toml | 1 + .../src/dependency_injection/builder.rs | 13 +- .../src/services/aggregator_client.rs | 70 --------- 7 files changed, 82 insertions(+), 155 deletions(-) rename internal/mithril-protocol-config/src/{http_client/http_impl.rs => http.rs} (50%) diff --git a/Cargo.lock b/Cargo.lock index b0e92be8070..4da117d22e1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4028,10 +4028,10 @@ version = "0.1.1" dependencies = [ "anyhow", "async-trait", - "http", "httpmock", + "mithril-aggregator-client", "mithril-common", - "mockall", + "serde_json", "slog", "slog-async", "slog-term", @@ -4113,6 +4113,7 @@ dependencies = [ "hex", "http", "httpmock", + "mithril-aggregator-client", "mithril-cardano-node-chain", "mithril-cardano-node-internal-database", "mithril-cli-helper", diff --git a/internal/mithril-protocol-config/Cargo.toml b/internal/mithril-protocol-config/Cargo.toml index 58cfb2536de..cacb7d08f24 100644 --- a/internal/mithril-protocol-config/Cargo.toml +++ b/internal/mithril-protocol-config/Cargo.toml @@ -12,13 +12,13 @@ include = ["**/*.rs", "Cargo.toml", "README.md", ".gitignore"] [dependencies] anyhow = { workspace = true } async-trait = { workspace = true } +mithril-aggregator-client = { path = "../mithril-aggregator-client" } mithril-common = { path = "../../mithril-common" } tokio = { workspace = true } [dev-dependencies] -http = "1.3.1" httpmock = "0.8.1" -mockall = { workspace = true } +serde_json = { workspace = true } slog = { workspace = true } slog-async = { workspace = true } slog-term = { workspace = true } diff --git a/internal/mithril-protocol-config/src/http_client/http_impl.rs b/internal/mithril-protocol-config/src/http.rs similarity index 50% rename from internal/mithril-protocol-config/src/http_client/http_impl.rs rename to internal/mithril-protocol-config/src/http.rs index bb00af77752..8d2f80a2cc3 100644 --- a/internal/mithril-protocol-config/src/http_client/http_impl.rs +++ b/internal/mithril-protocol-config/src/http.rs @@ -4,36 +4,24 @@ use anyhow::{Context, anyhow}; use async_trait::async_trait; use std::sync::Arc; +use mithril_aggregator_client::AggregatorHttpClient; +use mithril_aggregator_client::query::GetProtocolConfigurationQuery; use mithril_common::StdResult; use mithril_common::entities::Epoch; -use mithril_common::messages::ProtocolConfigurationMessage; use crate::interface::MithrilNetworkConfigurationProvider; use crate::model::{MithrilNetworkConfiguration, MithrilNetworkConfigurationForEpoch}; -/// Trait to retrieve protocol configuration -#[cfg_attr(test, mockall::automock)] -#[async_trait] -pub trait ProtocolConfigurationRetrieverFromAggregator: Sync + Send { - /// Retrieves protocol configuration for a given epoch from the aggregator - async fn retrieve_protocol_configuration( - &self, - epoch: Epoch, - ) -> StdResult; -} - /// Structure implementing MithrilNetworkConfigurationProvider using HTTP. pub struct HttpMithrilNetworkConfigurationProvider { - protocol_configuration_retriever: Arc, + aggregator_http_client: Arc, } impl HttpMithrilNetworkConfigurationProvider { /// HttpMithrilNetworkConfigurationProvider factory - pub fn new( - protocol_configuration_retriever: Arc, - ) -> Self { + pub fn new(aggregator_http_client: Arc) -> Self { Self { - protocol_configuration_retriever, + aggregator_http_client, } } } @@ -52,21 +40,30 @@ impl MithrilNetworkConfigurationProvider for HttpMithrilNetworkConfigurationProv let registration_epoch = epoch.offset_to_next_signer_retrieval_epoch().next(); let configuration_for_aggregation: MithrilNetworkConfigurationForEpoch = self - .protocol_configuration_retriever - .retrieve_protocol_configuration(aggregation_epoch) + .aggregator_http_client + .send(GetProtocolConfigurationQuery::epoch(aggregation_epoch)) .await? + .ok_or(anyhow!( + "Missing network configuration for aggregation epoch {aggregation_epoch}" + ))? .into(); let configuration_for_next_aggregation = self - .protocol_configuration_retriever - .retrieve_protocol_configuration(next_aggregation_epoch) + .aggregator_http_client + .send(GetProtocolConfigurationQuery::epoch(next_aggregation_epoch)) .await? + .ok_or(anyhow!( + "Missing network configuration for next aggregation epoch {next_aggregation_epoch}" + ))? .into(); let configuration_for_registration = self - .protocol_configuration_retriever - .retrieve_protocol_configuration(registration_epoch) + .aggregator_http_client + .send(GetProtocolConfigurationQuery::epoch(registration_epoch)) .await? + .ok_or(anyhow!( + "Missing network configuration for registration epoch {registration_epoch}" + ))? .into(); configuration_for_aggregation.signed_entity_types_config.cardano_transactions.clone() @@ -85,65 +82,57 @@ impl MithrilNetworkConfigurationProvider for HttpMithrilNetworkConfigurationProv #[cfg(test)] mod tests { - use mockall::predicate::eq; + use httpmock::MockServer; use std::sync::Arc; - use mithril_common::{ - entities::{Epoch, ProtocolParameters}, - messages::ProtocolConfigurationMessage, - test::double::Dummy, - }; + use mithril_common::entities::ProtocolParameters; + use mithril_common::messages::ProtocolConfigurationMessage; + use mithril_common::test::double::Dummy; + + use crate::test::test_tools::TestLogger; - use crate::{ - http_client::http_impl::{ - HttpMithrilNetworkConfigurationProvider, - MockProtocolConfigurationRetrieverFromAggregator, - }, - interface::MithrilNetworkConfigurationProvider, - }; + use super::*; #[tokio::test] async fn test_get_network_configuration_retrieve_configurations_for_aggregation_next_aggregation_and_registration() { - let mut protocol_configuration_retriever = - MockProtocolConfigurationRetrieverFromAggregator::new(); - - protocol_configuration_retriever - .expect_retrieve_protocol_configuration() - .once() - .with(eq(Epoch(41))) - .returning(|_| { - Ok(ProtocolConfigurationMessage { - protocol_parameters: ProtocolParameters::new(1000, 100, 0.1), - ..Dummy::dummy() - }) - }); - - protocol_configuration_retriever - .expect_retrieve_protocol_configuration() - .once() - .with(eq(Epoch(42))) - .returning(|_| { - Ok(ProtocolConfigurationMessage { - protocol_parameters: ProtocolParameters::new(2000, 200, 0.2), - ..Dummy::dummy() - }) - }); - - protocol_configuration_retriever - .expect_retrieve_protocol_configuration() - .once() - .with(eq(Epoch(43))) - .returning(|_| { - Ok(ProtocolConfigurationMessage { - protocol_parameters: ProtocolParameters::new(3000, 300, 0.3), - ..Dummy::dummy() - }) - }); - - let mithril_configuration_provider = HttpMithrilNetworkConfigurationProvider::new( - Arc::new(protocol_configuration_retriever), - ); + let configuration_epoch_41 = ProtocolConfigurationMessage { + protocol_parameters: ProtocolParameters::new(1000, 100, 0.1), + ..Dummy::dummy() + }; + let configuration_epoch_42 = ProtocolConfigurationMessage { + protocol_parameters: ProtocolParameters::new(2000, 200, 0.2), + ..Dummy::dummy() + }; + let configuration_epoch_43 = ProtocolConfigurationMessage { + protocol_parameters: ProtocolParameters::new(3000, 300, 0.3), + ..Dummy::dummy() + }; + + let server = MockServer::start(); + server.mock(|when, then| { + when.path("/protocol-configuration/41"); + then.status(200) + .body(serde_json::to_string(&configuration_epoch_41).unwrap()); + }); + server.mock(|when, then| { + when.path("/protocol-configuration/42"); + then.status(200) + .body(serde_json::to_string(&configuration_epoch_42).unwrap()); + }); + server.mock(|when, then| { + when.path("/protocol-configuration/43"); + then.status(200) + .body(serde_json::to_string(&configuration_epoch_43).unwrap()); + }); + + let mithril_configuration_provider = + HttpMithrilNetworkConfigurationProvider::new(Arc::new( + AggregatorHttpClient::builder(server.base_url()) + .with_logger(TestLogger::stdout()) + .build() + .unwrap(), + )); let configuration = mithril_configuration_provider .get_network_configuration(Epoch(42)) diff --git a/internal/mithril-protocol-config/src/lib.rs b/internal/mithril-protocol-config/src/lib.rs index a9f42fb2097..b03e1691ad0 100644 --- a/internal/mithril-protocol-config/src/lib.rs +++ b/internal/mithril-protocol-config/src/lib.rs @@ -1,10 +1,7 @@ #![warn(missing_docs)] //! This crate provides mechanisms to read and check the configuration parameters of a Mithril network. -/// HTTP client Implementation for interacting with Mithril Network. -pub mod http_client { - pub mod http_impl; -} +pub mod http; pub mod interface; pub mod model; pub mod test; diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index ab9b9b99167..33b5594847c 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -23,6 +23,7 @@ chrono = { workspace = true } clap = { workspace = true } config = { workspace = true } hex = { workspace = true } +mithril-aggregator-client = { path = "../internal/mithril-aggregator-client" } mithril-cardano-node-chain = { path = "../internal/cardano-node/mithril-cardano-node-chain" } mithril-cardano-node-internal-database = { path = "../internal/cardano-node/mithril-cardano-node-internal-database" } mithril-cli-helper = { path = "../internal/mithril-cli-helper" } diff --git a/mithril-signer/src/dependency_injection/builder.rs b/mithril-signer/src/dependency_injection/builder.rs index d5d37687819..e3faa3cf7ff 100644 --- a/mithril-signer/src/dependency_injection/builder.rs +++ b/mithril-signer/src/dependency_injection/builder.rs @@ -6,6 +6,7 @@ use anyhow::{Context, anyhow}; use slog::Logger; use tokio::sync::{Mutex, RwLock}; +use mithril_aggregator_client::AggregatorHttpClient; use mithril_cardano_node_chain::{ chain_observer::{CardanoCliRunner, ChainObserver, ChainObserverBuilder, ChainObserverType}, chain_reader::PallasChainReader, @@ -41,7 +42,7 @@ use mithril_persistence::database::repository::CardanoTransactionRepository; use mithril_persistence::database::{ApplicationNodeType, SqlMigration}; use mithril_persistence::sqlite::{ConnectionBuilder, SqliteConnection, SqliteConnectionPool}; -use mithril_protocol_config::http_client::http_impl::HttpMithrilNetworkConfigurationProvider; +use mithril_protocol_config::http::HttpMithrilNetworkConfigurationProvider; #[cfg(feature = "future_dmq")] use mithril_dmq::{DmqMessageBuilder, DmqPublisherClientPallas}; @@ -269,6 +270,14 @@ impl<'a> DependenciesBuilder<'a> { )); let api_version_provider = Arc::new(APIVersionProvider::new(era_checker.clone())); + let aggregator_client_for_network_config = Arc::new( + AggregatorHttpClient::builder(self.config.aggregator_endpoint.clone()) + .with_relay_endpoint(self.config.relay_endpoint.clone()) + .with_api_version_provider(api_version_provider.clone()) + .with_timeout(Duration::from_millis(HTTP_REQUEST_TIMEOUT_DURATION)) + .with_logger(self.root_logger()) + .build()?, + ); let aggregator_client = Arc::new(AggregatorHTTPClient::new( self.config.aggregator_endpoint.clone(), self.config.relay_endpoint.clone(), @@ -376,7 +385,7 @@ impl<'a> DependenciesBuilder<'a> { )); let metrics_service = Arc::new(MetricsService::new(self.root_logger())?); let network_configuration_service = Arc::new(HttpMithrilNetworkConfigurationProvider::new( - aggregator_client.clone(), + aggregator_client_for_network_config, )); let preloader_activation = CardanoTransactionsPreloaderActivationSigner::new( network_configuration_service.clone(), diff --git a/mithril-signer/src/services/aggregator_client.rs b/mithril-signer/src/services/aggregator_client.rs index e5601b57d3d..99aa4fd6c88 100644 --- a/mithril-signer/src/services/aggregator_client.rs +++ b/mithril-signer/src/services/aggregator_client.rs @@ -1,8 +1,5 @@ use anyhow::anyhow; use async_trait::async_trait; -use mithril_common::StdResult; -use mithril_common::messages::ProtocolConfigurationMessage; -use mithril_protocol_config::http_client::http_impl::ProtocolConfigurationRetrieverFromAggregator; use reqwest::header::{self, HeaderValue}; use reqwest::{self, Client, Proxy, RequestBuilder, Response, StatusCode}; use semver::Version; @@ -381,40 +378,6 @@ impl AggregatorClient for AggregatorHTTPClient { } } -#[async_trait] -impl ProtocolConfigurationRetrieverFromAggregator for AggregatorHTTPClient { - async fn retrieve_protocol_configuration( - &self, - epoch: Epoch, - ) -> StdResult { - debug!(self.logger, "Retrieve protocol configuration"); - let url = format!( - "{}/protocol-configuration/{}", - self.aggregator_endpoint, epoch - ); - let response = self - .prepare_request_builder(self.prepare_http_client()?.get(url.clone())) - .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)).into()) - } - } - } - _ => Err(AggregatorClientError::from_response(response).await.into()), - }, - Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err)).into()), - } - } -} - #[cfg(test)] pub(crate) mod dumb { use mithril_common::test::double::Dummy; @@ -681,39 +644,6 @@ mod tests { ); } - mod protocol_configuration { - - use super::*; - - #[tokio::test] - async fn test_ok_200() { - let (server, client) = setup_server_and_client(); - let message_expected = ProtocolConfigurationMessage::dummy(); - let _server_mock = server.mock(|when, then| { - when.path("/protocol-configuration/42"); - then.status(200).body(json!(message_expected).to_string()); - }); - - let message = client.retrieve_protocol_configuration(Epoch(42)).await.unwrap(); - - assert_eq!(message_expected, message); - } - - #[tokio::test] - async fn test_ko_500() { - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.path("/protocol-configuration/42"); - then.status(500).body("an error occurred"); - }); - - client - .retrieve_protocol_configuration(Epoch(42)) - .await - .expect_err("should throw a error"); - } - } - #[tokio::test] async fn test_register_signer_ok_201() { let epoch = Epoch(1); From 92a808dcf3f1d46fa2462b820be5a5cd9bba5d2f Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Tue, 28 Oct 2025 23:23:51 +0100 Subject: [PATCH 10/13] refactor(signer): use new shared aggregator client instead of an internal implementation --- .../src/dependency_injection/builder.rs | 22 +- mithril-signer/src/runtime/runner.rs | 6 +- mithril-signer/src/runtime/state_machine.rs | 10 +- .../src/services/aggregator_client.rs | 1310 +---------------- .../test_extensions/certificate_handler.rs | 41 +- .../test_extensions/state_machine_tester.rs | 2 +- 6 files changed, 61 insertions(+), 1330 deletions(-) diff --git a/mithril-signer/src/dependency_injection/builder.rs b/mithril-signer/src/dependency_injection/builder.rs index e3faa3cf7ff..ebc9080deda 100644 --- a/mithril-signer/src/dependency_injection/builder.rs +++ b/mithril-signer/src/dependency_injection/builder.rs @@ -51,12 +51,11 @@ use crate::dependency_injection::SignerDependencyContainer; #[cfg(feature = "future_dmq")] use crate::services::SignaturePublisherDmq; use crate::services::{ - AggregatorHTTPClient, CardanoTransactionsImporter, - CardanoTransactionsPreloaderActivationSigner, MithrilEpochService, MithrilSingleSigner, - SignaturePublishRetryPolicy, SignaturePublisherDelayer, SignaturePublisherNoop, - SignaturePublisherRetrier, SignerCertifierService, SignerSignableSeedBuilder, - SignerSignedEntityConfigProvider, SignerUpkeepService, TransactionsImporterByChunk, - TransactionsImporterWithPruner, TransactionsImporterWithVacuum, + CardanoTransactionsImporter, CardanoTransactionsPreloaderActivationSigner, MithrilEpochService, + MithrilSingleSigner, SignaturePublishRetryPolicy, SignaturePublisherDelayer, + SignaturePublisherNoop, SignaturePublisherRetrier, SignerCertifierService, + SignerSignableSeedBuilder, SignerSignedEntityConfigProvider, SignerUpkeepService, + TransactionsImporterByChunk, TransactionsImporterWithPruner, TransactionsImporterWithVacuum, }; use crate::store::MKTreeStoreSqlite; use crate::{ @@ -270,7 +269,7 @@ impl<'a> DependenciesBuilder<'a> { )); let api_version_provider = Arc::new(APIVersionProvider::new(era_checker.clone())); - let aggregator_client_for_network_config = Arc::new( + let aggregator_client = Arc::new( AggregatorHttpClient::builder(self.config.aggregator_endpoint.clone()) .with_relay_endpoint(self.config.relay_endpoint.clone()) .with_api_version_provider(api_version_provider.clone()) @@ -278,13 +277,6 @@ impl<'a> DependenciesBuilder<'a> { .with_logger(self.root_logger()) .build()?, ); - let aggregator_client = Arc::new(AggregatorHTTPClient::new( - self.config.aggregator_endpoint.clone(), - self.config.relay_endpoint.clone(), - api_version_provider.clone(), - Some(Duration::from_millis(HTTP_REQUEST_TIMEOUT_DURATION)), - self.root_logger(), - )); let cardano_immutable_snapshot_builder = Arc::new(CardanoImmutableFilesFullSignableBuilder::new( @@ -385,7 +377,7 @@ impl<'a> DependenciesBuilder<'a> { )); let metrics_service = Arc::new(MetricsService::new(self.root_logger())?); let network_configuration_service = Arc::new(HttpMithrilNetworkConfigurationProvider::new( - aggregator_client_for_network_config, + aggregator_client.clone(), )); let preloader_activation = CardanoTransactionsPreloaderActivationSigner::new( network_configuration_service.clone(), diff --git a/mithril-signer/src/runtime/runner.rs b/mithril-signer/src/runtime/runner.rs index 16789529ef5..11b3bc560ea 100644 --- a/mithril-signer/src/runtime/runner.rs +++ b/mithril-signer/src/runtime/runner.rs @@ -134,11 +134,7 @@ impl Runner for SignerRunner { ) -> StdResult> { debug!(self.logger, ">> get_epoch_settings"); - self.services - .certificate_handler - .retrieve_epoch_settings() - .await - .map_err(|e| e.into()) + self.services.certificate_handler.retrieve_epoch_settings().await } async fn get_beacon_to_sign(&self, time_point: TimePoint) -> StdResult> { diff --git a/mithril-signer/src/runtime/state_machine.rs b/mithril-signer/src/runtime/state_machine.rs index 2fbf27bb943..a800f298523 100644 --- a/mithril-signer/src/runtime/state_machine.rs +++ b/mithril-signer/src/runtime/state_machine.rs @@ -10,9 +10,10 @@ use mithril_common::{ logging::LoggerExtensions, }; +use mithril_aggregator_client::AggregatorHttpClientError; use mithril_protocol_config::model::MithrilNetworkConfiguration; -use crate::{MetricsService, entities::BeaconToSign, services::AggregatorClientError}; +use crate::{MetricsService, entities::BeaconToSign}; use super::{Runner, RuntimeError}; @@ -333,8 +334,8 @@ impl StateMachine { epoch: Epoch, ) -> Result, RuntimeError> { if let Err(e) = register_result { - if let Some(AggregatorClientError::RegistrationRoundNotYetOpened(_)) = - e.downcast_ref::() + if let Some(AggregatorHttpClientError::RegistrationRoundNotYetOpened(_)) = + e.downcast_ref::() { Ok(Some(SignerState::Unregistered { epoch })) } else if e.downcast_ref::().is_some() { @@ -497,7 +498,6 @@ mod tests { use crate::SignerEpochSettings; use crate::runtime::runner::MockSignerRunner; - use crate::services::AggregatorClientError; use crate::test_tools::TestLogger; use super::*; @@ -720,7 +720,7 @@ mod tests { .returning(|| Ok(TimePoint::dummy())); runner.expect_update_stake_distribution().once().returning(|_| Ok(())); runner.expect_register_signer_to_aggregator().once().returning(|| { - Err(AggregatorClientError::RegistrationRoundNotYetOpened( + Err(AggregatorHttpClientError::RegistrationRoundNotYetOpened( anyhow!("Not yet opened"), ))? }); diff --git a/mithril-signer/src/services/aggregator_client.rs b/mithril-signer/src/services/aggregator_client.rs index 99aa4fd6c88..9a77a0db619 100644 --- a/mithril-signer/src/services/aggregator_client.rs +++ b/mithril-signer/src/services/aggregator_client.rs @@ -1,22 +1,15 @@ use anyhow::anyhow; use async_trait::async_trait; -use reqwest::header::{self, HeaderValue}; -use reqwest::{self, Client, Proxy, RequestBuilder, Response, StatusCode}; -use semver::Version; -use slog::{Logger, debug, error, warn}; -use std::{io, sync::Arc, time::Duration}; -use thiserror::Error; +use mithril_aggregator_client::query::{ + GetAggregatorFeaturesQuery, GetEpochSettingsQuery, PostRegisterSignatureQuery, + PostRegisterSignerQuery, +}; +use mithril_aggregator_client::{AggregatorHttpClient, AggregatorHttpClientError}; use mithril_common::{ - MITHRIL_API_VERSION_HEADER, MITHRIL_SIGNER_VERSION_HEADER, StdError, - api_version::APIVersionProvider, - entities::{ - ClientError, Epoch, ProtocolMessage, ServerError, SignedEntityType, Signer, SingleSignature, - }, - logging::LoggerExtensions, - messages::{ - AggregatorFeaturesMessage, EpochSettingsMessage, TryFromMessageAdapter, TryToMessageAdapter, - }, + StdResult, + entities::{Epoch, ProtocolMessage, SignedEntityType, Signer, SingleSignature}, + messages::{AggregatorFeaturesMessage, TryFromMessageAdapter, TryToMessageAdapter}, }; use crate::entities::SignerEpochSettings; @@ -24,126 +17,15 @@ use crate::message_adapters::{ FromEpochSettingsAdapter, ToRegisterSignatureMessageAdapter, ToRegisterSignerMessageAdapter, }; -const JSON_CONTENT_TYPE: HeaderValue = HeaderValue::from_static("application/json"); - -const API_VERSION_MISMATCH_WARNING_MESSAGE: &str = - "OpenAPI version may be incompatible, please update your Mithril node to the latest version."; - -/// Error structure for the Aggregator Client. -#[derive(Error, Debug)] -pub enum AggregatorClientError { - /// The aggregator host has returned a technical error. - #[error("remote server technical error")] - RemoteServerTechnical(#[source] StdError), - - /// The aggregator host responded it cannot fulfill our request. - #[error("remote server logical error")] - RemoteServerLogical(#[source] StdError), - - /// Could not reach aggregator. - #[error("remote server unreachable")] - RemoteServerUnreachable(#[source] StdError), - - /// Unhandled status code - #[error("unhandled status code: {0}, response text: {1}")] - UnhandledStatusCode(StatusCode, String), - - /// Could not parse response. - #[error("json parsing failed")] - JsonParseFailed(#[source] StdError), - - /// Mostly network errors. - #[error("Input/Output error")] - IOError(#[from] io::Error), - - /// HTTP client creation error - #[error("HTTP client creation failed")] - HTTPClientCreation(#[source] StdError), - - /// Proxy creation error - #[error("proxy creation failed")] - ProxyCreation(#[source] StdError), - - /// Adapter error - #[error("adapter failed")] - Adapter(#[source] StdError), - - /// No signer registration round opened yet - #[error("a signer registration round is not opened yet, please try again later")] - RegistrationRoundNotYetOpened(#[source] StdError), -} - -impl AggregatorClientError { - /// Create an `AggregatorClientError` from a response. - /// - /// This method is meant to be used after handling domain-specific cases leaving only - /// 4xx or 5xx status codes. - /// Otherwise, it will return an `UnhandledStatusCode` error. - pub async fn from_response(response: Response) -> Self { - let error_code = response.status(); - - if error_code.is_client_error() { - let root_cause = Self::get_root_cause(response).await; - Self::RemoteServerLogical(anyhow!(root_cause)) - } else if error_code.is_server_error() { - let root_cause = Self::get_root_cause(response).await; - match error_code.as_u16() { - 550 => Self::RegistrationRoundNotYetOpened(anyhow!(root_cause)), - _ => Self::RemoteServerTechnical(anyhow!(root_cause)), - } - } else { - let response_text = response.text().await.unwrap_or_default(); - Self::UnhandledStatusCode(error_code, response_text) - } - } - - async fn get_root_cause(response: Response) -> String { - let error_code = response.status(); - let canonical_reason = error_code.canonical_reason().unwrap_or_default().to_lowercase(); - let is_json = response - .headers() - .get(header::CONTENT_TYPE) - .is_some_and(|ct| JSON_CONTENT_TYPE == ct); - - if is_json { - let json_value: serde_json::Value = response.json().await.unwrap_or_default(); - - if let Ok(client_error) = serde_json::from_value::(json_value.clone()) { - format!( - "{}: {}: {}", - canonical_reason, client_error.label, client_error.message - ) - } else if let Ok(server_error) = - serde_json::from_value::(json_value.clone()) - { - format!("{}: {}", canonical_reason, server_error.message) - } else if json_value.is_null() { - canonical_reason.to_string() - } else { - format!("{canonical_reason}: {json_value}") - } - } else { - let response_text = response.text().await.unwrap_or_default(); - format!("{canonical_reason}: {response_text}") - } - } -} - /// 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>; + async fn retrieve_epoch_settings(&self) -> StdResult>; /// Registers signer with the aggregator. - async fn register_signer( - &self, - epoch: Epoch, - signer: &Signer, - ) -> Result<(), AggregatorClientError>; + async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()>; /// Registers single signature with the aggregator. async fn register_signature( @@ -151,164 +33,29 @@ pub trait AggregatorClient: Sync + Send { signed_entity_type: &SignedEntityType, signature: &SingleSignature, protocol_message: &ProtocolMessage, - ) -> Result<(), AggregatorClientError>; + ) -> StdResult<()>; /// Retrieves aggregator features message from the aggregator - async fn retrieve_aggregator_features( - &self, - ) -> Result; -} - -/// AggregatorHTTPClient is a http client for an aggregator -pub struct AggregatorHTTPClient { - aggregator_endpoint: String, - relay_endpoint: Option, - api_version_provider: Arc, - timeout_duration: Option, - logger: Logger, -} - -impl AggregatorHTTPClient { - /// AggregatorHTTPClient factory - pub fn new( - aggregator_endpoint: String, - relay_endpoint: Option, - api_version_provider: Arc, - timeout_duration: Option, - logger: Logger, - ) -> Self { - let logger = logger.new_with_component_name::(); - debug!(logger, "New AggregatorHTTPClient created"); - Self { - aggregator_endpoint, - relay_endpoint, - api_version_provider, - timeout_duration, - logger, - } - } - - fn prepare_http_client(&self) -> Result { - let client = match &self.relay_endpoint { - Some(relay_endpoint) => Client::builder() - .proxy( - Proxy::all(relay_endpoint) - .map_err(|e| AggregatorClientError::ProxyCreation(anyhow!(e)))?, - ) - .build() - .map_err(|e| AggregatorClientError::HTTPClientCreation(anyhow!(e)))?, - None => Client::new(), - }; - - Ok(client) - } - - /// Forge a client request adding protocol version in the headers. - pub fn prepare_request_builder(&self, request_builder: RequestBuilder) -> RequestBuilder { - let request_builder = request_builder - .header( - MITHRIL_API_VERSION_HEADER, - self.api_version_provider - .compute_current_version() - .unwrap() - .to_string(), - ) - .header(MITHRIL_SIGNER_VERSION_HEADER, env!("CARGO_PKG_VERSION")); - - if let Some(duration) = self.timeout_duration { - request_builder.timeout(duration) - } else { - request_builder - } - } - - /// Check API version mismatch and log a warning if the aggregator's version is more recent. - fn warn_if_api_version_mismatch(&self, response: &Response) { - let aggregator_version = response - .headers() - .get(MITHRIL_API_VERSION_HEADER) - .and_then(|v| v.to_str().ok()) - .and_then(|s| Version::parse(s).ok()); - - let signer_version = self.api_version_provider.compute_current_version(); - - match (aggregator_version, signer_version) { - (Some(aggregator), Ok(signer)) if signer < aggregator => { - warn!(self.logger, "{}", API_VERSION_MISMATCH_WARNING_MESSAGE; - "aggregator_version" => %aggregator, - "signer_version" => %signer, - ); - } - (Some(_), Err(error)) => { - error!( - self.logger, - "Failed to compute the current signer API version"; - "error" => error.to_string() - ); - } - _ => {} - } - } + async fn retrieve_aggregator_features(&self) -> StdResult; } #[async_trait] -impl AggregatorClient for AggregatorHTTPClient { - async fn retrieve_epoch_settings( - &self, - ) -> Result, AggregatorClientError> { - debug!(self.logger, "Retrieve epoch settings"); - let url = format!("{}/epoch-settings", self.aggregator_endpoint); - let response = self - .prepare_request_builder(self.prepare_http_client()?.get(url.clone())) - .send() - .await; +impl AggregatorClient for AggregatorHttpClient { + async fn retrieve_epoch_settings(&self) -> StdResult> { + let message = self.send(GetEpochSettingsQuery::current()).await?; + let epoch_settings = FromEpochSettingsAdapter::try_adapt(message)?; - match response { - Ok(response) => match response.status() { - StatusCode::OK => { - self.warn_if_api_version_mismatch(&response); - match response.json::().await { - Ok(message) => { - let epoch_settings = FromEpochSettingsAdapter::try_adapt(message) - .map_err(|e| AggregatorClientError::Adapter(anyhow!(e)))?; - Ok(Some(epoch_settings)) - } - Err(err) => Err(AggregatorClientError::JsonParseFailed(anyhow!(err))), - } - } - _ => Err(AggregatorClientError::from_response(response).await), - }, - Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), - } + Ok(Some(epoch_settings)) } - async fn register_signer( - &self, - epoch: Epoch, - signer: &Signer, - ) -> Result<(), AggregatorClientError> { - debug!(self.logger, "Register signer"); - let url = format!("{}/register-signer", self.aggregator_endpoint); + async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()> { let register_signer_message = - ToRegisterSignerMessageAdapter::try_adapt((epoch, signer.to_owned())) - .map_err(|e| AggregatorClientError::Adapter(anyhow!(e)))?; - let response = self - .prepare_request_builder(self.prepare_http_client()?.post(url.clone())) - .json(®ister_signer_message) - .send() - .await; + ToRegisterSignerMessageAdapter::try_adapt((epoch, signer.to_owned()))?; - match response { - Ok(response) => match response.status() { - StatusCode::CREATED => { - self.warn_if_api_version_mismatch(&response); + self.send(PostRegisterSignerQuery::new(register_signer_message)) + .await?; - Ok(()) - } - _ => Err(AggregatorClientError::from_response(response).await), - }, - Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), - } + Ok(()) } async fn register_signature( @@ -316,65 +63,25 @@ impl AggregatorClient for AggregatorHTTPClient { signed_entity_type: &SignedEntityType, signature: &SingleSignature, protocol_message: &ProtocolMessage, - ) -> Result<(), AggregatorClientError> { - debug!(self.logger, "Register signature"); - let url = format!("{}/register-signatures", self.aggregator_endpoint); + ) -> StdResult<()> { let register_single_signature_message = ToRegisterSignatureMessageAdapter::try_adapt(( signed_entity_type.to_owned(), signature.to_owned(), protocol_message, )) - .map_err(|e| AggregatorClientError::Adapter(anyhow!(e)))?; - let response = self - .prepare_request_builder(self.prepare_http_client()?.post(url.clone())) - .json(®ister_single_signature_message) - .send() - .await; + .map_err(|e| AggregatorHttpClientError::JsonParseFailed(anyhow!(e)))?; - match response { - Ok(response) => match response.status() { - StatusCode::CREATED | StatusCode::ACCEPTED => { - self.warn_if_api_version_mismatch(&response); - - Ok(()) - } - StatusCode::GONE => { - self.warn_if_api_version_mismatch(&response); - let root_cause = AggregatorClientError::get_root_cause(response).await; - debug!(self.logger, "Message already certified or expired"; "details" => &root_cause); + self.send(PostRegisterSignatureQuery::new( + register_single_signature_message, + )) + .await?; - Ok(()) - } - _ => Err(AggregatorClientError::from_response(response).await), - }, - Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), - } + Ok(()) } - async fn retrieve_aggregator_features( - &self, - ) -> Result { - debug!(self.logger, "Retrieve aggregator features message"); - let url = format!("{}/", self.aggregator_endpoint); - let response = self - .prepare_request_builder(self.prepare_http_client()?.get(url.clone())) - .send() - .await; - - match response { - Ok(response) => match response.status() { - StatusCode::OK => { - self.warn_if_api_version_mismatch(&response); - - Ok(response - .json::() - .await - .map_err(|e| AggregatorClientError::JsonParseFailed(anyhow!(e)))?) - } - _ => Err(AggregatorClientError::from_response(response).await), - }, - Err(err) => Err(AggregatorClientError::RemoteServerUnreachable(anyhow!(err))), - } + async fn retrieve_aggregator_features(&self) -> StdResult { + let aggregator_features = self.send(GetAggregatorFeaturesQuery::current()).await?; + Ok(aggregator_features) } } @@ -413,20 +120,14 @@ pub(crate) mod dumb { #[async_trait] impl AggregatorClient for DumbAggregatorClient { - async fn retrieve_epoch_settings( - &self, - ) -> Result, AggregatorClientError> { + async fn retrieve_epoch_settings(&self) -> StdResult> { let epoch_settings = self.epoch_settings.read().await.clone(); Ok(epoch_settings) } /// Registers signer with the aggregator - async fn register_signer( - &self, - _epoch: Epoch, - signer: &Signer, - ) -> Result<(), AggregatorClientError> { + async fn register_signer(&self, _epoch: Epoch, signer: &Signer) -> StdResult<()> { let mut last_registered_signer = self.last_registered_signer.write().await; let signer = signer.clone(); *last_registered_signer = Some(signer); @@ -440,952 +141,13 @@ pub(crate) mod dumb { _signed_entity_type: &SignedEntityType, _signature: &SingleSignature, _protocol_message: &ProtocolMessage, - ) -> Result<(), AggregatorClientError> { + ) -> StdResult<()> { Ok(()) } - async fn retrieve_aggregator_features( - &self, - ) -> Result { + async fn retrieve_aggregator_features(&self) -> StdResult { let aggregator_features = self.aggregator_features.read().await; Ok(aggregator_features.clone()) } } } - -#[cfg(test)] -mod tests { - use std::collections::HashMap; - - use http::response::Builder as HttpResponseBuilder; - use httpmock::prelude::*; - use semver::Version; - use serde_json::json; - - use mithril_common::entities::Epoch; - use mithril_common::messages::TryFromMessageAdapter; - use mithril_common::test::{ - double::{Dummy, DummyApiVersionDiscriminantSource, fake_data}, - logging::MemoryDrainForTestInspector, - }; - - use crate::test_tools::TestLogger; - - use super::*; - - macro_rules! assert_is_error { - ($error:expr, $error_type:pat) => { - assert!( - matches!($error, $error_type), - "Expected {} error, got '{:?}'.", - stringify!($error_type), - $error - ); - }; - } - - fn setup_client>(server_url: U) -> AggregatorHTTPClient { - let discriminant_source = DummyApiVersionDiscriminantSource::new("dummy"); - let api_version_provider = APIVersionProvider::new(Arc::new(discriminant_source)); - - AggregatorHTTPClient::new( - server_url.into(), - None, - Arc::new(api_version_provider), - None, - TestLogger::stdout(), - ) - } - - fn setup_server_and_client() -> (MockServer, AggregatorHTTPClient) { - let server = MockServer::start(); - let aggregator_endpoint = server.url(""); - let client = setup_client(&aggregator_endpoint); - - (server, client) - } - - fn set_returning_500(server: &MockServer) { - server.mock(|_, then| { - then.status(500).body("an error occurred"); - }); - } - - fn set_unparsable_json(server: &MockServer) { - server.mock(|_, then| { - then.status(200).body("this is not a json"); - }); - } - - fn build_text_response>(status_code: StatusCode, body: T) -> Response { - HttpResponseBuilder::new() - .status(status_code) - .body(body.into()) - .unwrap() - .into() - } - - fn build_json_response(status_code: StatusCode, body: &T) -> Response { - HttpResponseBuilder::new() - .status(status_code) - .header(header::CONTENT_TYPE, JSON_CONTENT_TYPE) - .body(serde_json::to_string(&body).unwrap()) - .unwrap() - .into() - } - - macro_rules! assert_error_text_contains { - ($error: expr, $expect_contains: expr) => { - let error = &$error; - assert!( - error.contains($expect_contains), - "Expected error message to contain '{}'\ngot '{error:?}'", - $expect_contains, - ); - }; - } - - #[tokio::test] - async fn test_aggregator_features_ok_200() { - let (server, client) = setup_server_and_client(); - let message_expected = AggregatorFeaturesMessage::dummy(); - let _server_mock = server.mock(|when, then| { - when.path("/"); - then.status(200).body(json!(message_expected).to_string()); - }); - - let message = client.retrieve_aggregator_features().await.unwrap(); - - assert_eq!(message_expected, message); - } - - #[tokio::test] - async fn test_aggregator_features_ko_500() { - let (server, client) = setup_server_and_client(); - set_returning_500(&server); - - let error = client.retrieve_aggregator_features().await.unwrap_err(); - - assert_is_error!(error, AggregatorClientError::RemoteServerTechnical(_)); - } - - #[tokio::test] - async fn test_aggregator_features_ko_json_serialization() { - let (server, client) = setup_server_and_client(); - set_unparsable_json(&server); - - let error = client.retrieve_aggregator_features().await.unwrap_err(); - - assert_is_error!(error, AggregatorClientError::JsonParseFailed(_)); - } - - #[tokio::test] - async fn test_aggregator_features_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("/"); - then.delay(Duration::from_millis(100)); - }); - - let error = client.retrieve_aggregator_features().await.unwrap_err(); - - assert_is_error!(error, AggregatorClientError::RemoteServerUnreachable(_)); - } - - #[tokio::test] - async fn test_epoch_settings_ok_200() { - let (server, client) = setup_server_and_client(); - let epoch_settings_expected = EpochSettingsMessage::dummy(); - let _server_mock = server.mock(|when, then| { - when.path("/epoch-settings"); - then.status(200).body(json!(epoch_settings_expected).to_string()); - }); - - let epoch_settings = client.retrieve_epoch_settings().await; - epoch_settings.as_ref().expect("unexpected error"); - assert_eq!( - FromEpochSettingsAdapter::try_adapt(epoch_settings_expected).unwrap(), - epoch_settings.unwrap().unwrap() - ); - } - - #[tokio::test] - async fn test_epoch_settings_ko_500() { - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.path("/epoch-settings"); - then.status(500).body("an error occurred"); - }); - - match client.retrieve_epoch_settings().await.unwrap_err() { - AggregatorClientError::RemoteServerTechnical(_) => (), - e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), - }; - } - - #[tokio::test] - async fn test_epoch_settings_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("/epoch-settings"); - then.delay(Duration::from_millis(100)); - }); - - let error = client - .retrieve_epoch_settings() - .await - .expect_err("retrieve_epoch_settings should fail"); - - assert!( - matches!(error, AggregatorClientError::RemoteServerUnreachable(_)), - "unexpected error type: {error:?}" - ); - } - - #[tokio::test] - async fn test_register_signer_ok_201() { - let epoch = Epoch(1); - let single_signers = fake_data::signers(1); - let single_signer = single_signers.first().unwrap(); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signer"); - then.status(201); - }); - - let register_signer = client.register_signer(epoch, single_signer).await; - register_signer.expect("unexpected error"); - } - - #[tokio::test] - async fn test_register_signer_ko_400() { - let epoch = Epoch(1); - let single_signers = fake_data::signers(1); - let single_signer = single_signers.first().unwrap(); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signer"); - then.status(400).body( - serde_json::to_vec(&ClientError::new( - "error".to_string(), - "an error".to_string(), - )) - .unwrap(), - ); - }); - - match client.register_signer(epoch, single_signer).await.unwrap_err() { - AggregatorClientError::RemoteServerLogical(_) => (), - err => { - panic!( - "Expected a AggregatorClientError::RemoteServerLogical error, got '{err:?}'." - ) - } - }; - } - - #[tokio::test] - async fn test_register_signer_ko_500() { - let epoch = Epoch(1); - let single_signers = fake_data::signers(1); - let single_signer = single_signers.first().unwrap(); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signer"); - then.status(500).body("an error occurred"); - }); - - match client.register_signer(epoch, single_signer).await.unwrap_err() { - AggregatorClientError::RemoteServerTechnical(_) => (), - e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), - }; - } - - #[tokio::test] - async fn test_register_signer_timeout() { - let epoch = Epoch(1); - let single_signers = fake_data::signers(1); - let single_signer = single_signers.first().unwrap(); - let (server, mut client) = setup_server_and_client(); - client.timeout_duration = Some(Duration::from_millis(10)); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signer"); - then.delay(Duration::from_millis(100)); - }); - - let error = client - .register_signer(epoch, single_signer) - .await - .expect_err("register_signer should fail"); - - assert!( - matches!(error, AggregatorClientError::RemoteServerUnreachable(_)), - "unexpected error type: {error:?}" - ); - } - - #[tokio::test] - async fn test_register_signature_ok_201() { - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(201); - }); - - let register_signature = client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await; - register_signature.expect("unexpected error"); - } - - #[tokio::test] - async fn test_register_signature_ok_202() { - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(202); - }); - - let register_signature = client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await; - register_signature.expect("unexpected error"); - } - - #[tokio::test] - async fn test_register_signature_ko_400() { - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(400).body( - serde_json::to_vec(&ClientError::new( - "error".to_string(), - "an error".to_string(), - )) - .unwrap(), - ); - }); - - match client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .unwrap_err() - { - AggregatorClientError::RemoteServerLogical(_) => (), - e => panic!("Expected Aggregator::RemoteServerLogical error, got '{e:?}'."), - }; - } - - #[tokio::test] - async fn test_register_signature_ok_410_log_response_body() { - let (logger, log_inspector) = TestLogger::memory(); - - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, mut client) = setup_server_and_client(); - client.logger = logger; - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(410).body( - serde_json::to_vec(&ClientError::new( - "already_aggregated".to_string(), - "too late".to_string(), - )) - .unwrap(), - ); - }); - - client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .expect("Should not fail when status is 410 (GONE)"); - - assert!(log_inspector.contains_log("already_aggregated")); - assert!(log_inspector.contains_log("too late")); - } - - #[tokio::test] - async fn test_register_signature_ko_409() { - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(409); - }); - - match client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .unwrap_err() - { - AggregatorClientError::RemoteServerLogical(_) => (), - e => panic!("Expected Aggregator::RemoteServerLogical error, got '{e:?}'."), - } - } - - #[tokio::test] - async fn test_register_signature_ko_500() { - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, client) = setup_server_and_client(); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(500).body("an error occurred"); - }); - - match client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .unwrap_err() - { - AggregatorClientError::RemoteServerTechnical(_) => (), - e => panic!("Expected Aggregator::RemoteServerTechnical error, got '{e:?}'."), - }; - } - - #[tokio::test] - async fn test_register_signature_timeout() { - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, mut client) = setup_server_and_client(); - client.timeout_duration = Some(Duration::from_millis(10)); - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.delay(Duration::from_millis(100)); - }); - - let error = client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .expect_err("register_signature should fail"); - - 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"); - let handled_error = AggregatorClientError::from_response(response).await; - - assert!( - matches!( - handled_error, - AggregatorClientError::RemoteServerLogical(..) - ), - "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'", - ); - } - - #[tokio::test] - async fn test_5xx_errors_are_handled_as_remote_server_technical() { - let response = build_text_response(StatusCode::INTERNAL_SERVER_ERROR, "error text"); - let handled_error = AggregatorClientError::from_response(response).await; - - assert!( - matches!( - handled_error, - AggregatorClientError::RemoteServerTechnical(..) - ), - "Expected error to be RemoteServerLogical\ngot '{handled_error:?}'", - ); - } - - #[tokio::test] - async fn test_550_error_is_handled_as_registration_round_not_yet_opened() { - let response = build_text_response(StatusCode::from_u16(550).unwrap(), "Not yet available"); - let handled_error = AggregatorClientError::from_response(response).await; - - assert!( - matches!( - handled_error, - AggregatorClientError::RegistrationRoundNotYetOpened(..) - ), - "Expected error to be RegistrationRoundNotYetOpened\ngot '{handled_error:?}'", - ); - } - - #[tokio::test] - async fn test_non_4xx_or_5xx_errors_are_handled_as_unhandled_status_code_and_contains_response_text() - { - let response = build_text_response(StatusCode::OK, "ok text"); - let handled_error = AggregatorClientError::from_response(response).await; - - assert!( - matches!( - handled_error, - AggregatorClientError::UnhandledStatusCode(..) if format!("{handled_error:?}").contains("ok text") - ), - "Expected error to be UnhandledStatusCode with 'ok text' in error text\ngot '{handled_error:?}'", - ); - } - - #[tokio::test] - async fn test_root_cause_of_non_json_response_contains_response_plain_text() { - let error_text = "An error occurred; please try again later."; - let response = build_text_response(StatusCode::EXPECTATION_FAILED, error_text); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - "expectation failed: An error occurred; please try again later." - ); - } - - #[tokio::test] - async fn test_root_cause_of_json_formatted_client_error_response_contains_error_label_and_message() - { - let client_error = ClientError::new("label", "message"); - let response = build_json_response(StatusCode::BAD_REQUEST, &client_error); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - "bad request: label: message" - ); - } - - #[tokio::test] - async fn test_root_cause_of_json_formatted_server_error_response_contains_error_label_and_message() - { - let server_error = ServerError::new("message"); - let response = build_json_response(StatusCode::BAD_REQUEST, &server_error); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - "bad request: message" - ); - } - - #[tokio::test] - async fn test_root_cause_of_unknown_formatted_json_response_contains_json_key_value_pairs() { - let response = build_json_response( - StatusCode::INTERNAL_SERVER_ERROR, - &json!({ "second": "unknown", "first": "foreign" }), - ); - - assert_error_text_contains!( - AggregatorClientError::get_root_cause(response).await, - r#"internal server error: {"first":"foreign","second":"unknown"}"# - ); - } - - #[tokio::test] - async fn test_root_cause_with_invalid_json_response_still_contains_response_status_name() { - let response = HttpResponseBuilder::new() - .status(StatusCode::BAD_REQUEST) - .header(header::CONTENT_TYPE, JSON_CONTENT_TYPE) - .body(r#"{"invalid":"unexpected dot", "key": "value".}"#) - .unwrap() - .into(); - - let root_cause = AggregatorClientError::get_root_cause(response).await; - - assert_error_text_contains!(root_cause, "bad request"); - assert!( - !root_cause.contains("bad request: "), - "Expected error message should not contain additional information \ngot '{root_cause:?}'" - ); - } - - #[tokio::test] - async fn test_sends_accept_encoding_header() { - let (server, client) = setup_server_and_client(); - server.mock(|when, then| { - when.is_true(|req| { - let headers = req.headers(); - let accept_encoding_header = headers - .get("accept-encoding") - .expect("Accept-Encoding header not found"); - - ["gzip", "br", "deflate", "zstd"].iter().all(|&encoding| { - accept_encoding_header.to_str().is_ok_and(|h| h.contains(encoding)) - }) - }); - - then.status(201); - }); - - client - .register_signature( - &SignedEntityType::dummy(), - &fake_data::single_signature((1..5).collect()), - &ProtocolMessage::default(), - ) - .await - .expect("Should succeed with Accept-Encoding header"); - } - - mod warn_if_api_version_mismatch { - use mithril_common::test::api_version_extensions::ApiVersionProviderTestExtension; - - use super::*; - - fn version_provider_with_open_api_version>( - version: V, - ) -> APIVersionProvider { - let mut version_provider = version_provider_without_open_api_version(); - let mut open_api_versions = HashMap::new(); - open_api_versions.insert( - "openapi.yaml".to_string(), - Version::parse(&version.into()).unwrap(), - ); - version_provider.update_open_api_versions(open_api_versions); - - version_provider - } - - fn version_provider_without_open_api_version() -> APIVersionProvider { - let mut version_provider = - APIVersionProvider::new(Arc::new(DummyApiVersionDiscriminantSource::new("dummy"))); - version_provider.update_open_api_versions(HashMap::new()); - - version_provider - } - - fn build_fake_response_with_header, V: Into>( - key: K, - value: V, - ) -> Response { - HttpResponseBuilder::new() - .header(key.into(), value.into()) - .body("whatever") - .unwrap() - .into() - } - - fn assert_api_version_warning_logged, S: Into>( - log_inspector: &MemoryDrainForTestInspector, - aggregator_version: A, - signer_version: S, - ) { - assert!(log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - assert!( - log_inspector - .contains_log(&format!("aggregator_version={}", aggregator_version.into())) - ); - assert!( - log_inspector.contains_log(&format!("signer_version={}", signer_version.into())) - ); - } - - #[test] - fn test_logs_warning_when_aggregator_api_version_is_newer() { - let aggregator_version = "2.0.0"; - let signer_version = "1.0.0"; - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - let mut client = setup_client("whatever"); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let response = - build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, aggregator_version); - - assert!( - Version::parse(aggregator_version).unwrap() - > Version::parse(signer_version).unwrap() - ); - - client.warn_if_api_version_mismatch(&response); - - assert_api_version_warning_logged(&log_inspector, aggregator_version, signer_version); - } - - #[test] - fn test_no_warning_logged_when_versions_match() { - 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"); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let response = build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, version); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[test] - fn test_no_warning_logged_when_aggregator_api_version_is_older() { - let aggregator_version = "1.0.0"; - let signer_version = "2.0.0"; - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - let mut client = setup_client("whatever"); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let response = - build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, aggregator_version); - - assert!( - Version::parse(aggregator_version).unwrap() - < Version::parse(signer_version).unwrap() - ); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[test] - fn test_does_not_log_or_fail_when_header_is_missing() { - let (logger, log_inspector) = TestLogger::memory(); - let mut client = setup_client("whatever"); - client.logger = logger; - let response = - build_fake_response_with_header("NotMithrilAPIVersionHeader", "whatever"); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[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"); - client.logger = logger; - let response = - build_fake_response_with_header(MITHRIL_API_VERSION_HEADER, "not_a_version"); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[test] - fn test_logs_error_when_signer_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"); - 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"); - - client.warn_if_api_version_mismatch(&response); - - assert!(!log_inspector.contains_log(API_VERSION_MISMATCH_WARNING_MESSAGE)); - } - - #[tokio::test] - async fn test_aggregator_features_ok_200_log_warning_if_api_version_mismatch() { - let aggregator_version = "2.0.0"; - let signer_version = "1.0.0"; - let (server, mut client) = setup_server_and_client(); - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - - let message_expected = AggregatorFeaturesMessage::dummy(); - let _server_mock = server.mock(|when, then| { - when.path("/"); - then.status(200) - .header(MITHRIL_API_VERSION_HEADER, aggregator_version) - .body(json!(message_expected).to_string()); - }); - - assert!( - Version::parse(aggregator_version).unwrap() - > Version::parse(signer_version).unwrap() - ); - - client.retrieve_aggregator_features().await.unwrap(); - - assert_api_version_warning_logged(&log_inspector, aggregator_version, signer_version); - } - - #[tokio::test] - async fn test_epoch_settings_ok_200_log_warning_if_api_version_mismatch() { - let aggregator_version = "2.0.0"; - let signer_version = "1.0.0"; - let (server, mut client) = setup_server_and_client(); - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - - let epoch_settings_expected = EpochSettingsMessage::dummy(); - let _server_mock = server.mock(|when, then| { - when.path("/epoch-settings"); - then.status(200) - .header(MITHRIL_API_VERSION_HEADER, aggregator_version) - .body(json!(epoch_settings_expected).to_string()); - }); - - assert!( - Version::parse(aggregator_version).unwrap() - > Version::parse(signer_version).unwrap() - ); - - client.retrieve_epoch_settings().await.unwrap(); - - assert_api_version_warning_logged(&log_inspector, aggregator_version, signer_version); - } - - #[tokio::test] - async fn test_register_signer_ok_201_log_warning_if_api_version_mismatch() { - let aggregator_version = "2.0.0"; - let signer_version = "1.0.0"; - let epoch = Epoch(1); - let single_signers = fake_data::signers(1); - let single_signer = single_signers.first().unwrap(); - let (server, mut client) = setup_server_and_client(); - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signer"); - then.status(201) - .header(MITHRIL_API_VERSION_HEADER, aggregator_version); - }); - - assert!( - Version::parse(aggregator_version).unwrap() - > Version::parse(signer_version).unwrap() - ); - - client.register_signer(epoch, single_signer).await.unwrap(); - - assert_api_version_warning_logged(&log_inspector, aggregator_version, signer_version); - } - - #[tokio::test] - async fn test_register_signature_ok_201_log_warning_if_api_version_mismatch() { - let aggregator_version = "2.0.0"; - let signer_version = "1.0.0"; - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, mut client) = setup_server_and_client(); - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(201) - .header(MITHRIL_API_VERSION_HEADER, aggregator_version); - }); - - assert!( - Version::parse(aggregator_version).unwrap() - > Version::parse(signer_version).unwrap() - ); - - client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .expect("Should not fail"); - - assert_api_version_warning_logged(&log_inspector, aggregator_version, signer_version); - } - - #[tokio::test] - async fn test_register_signature_ok_202_log_warning_if_api_version_mismatch() { - let aggregator_version = "2.0.0"; - let signer_version = "1.0.0"; - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, mut client) = setup_server_and_client(); - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(202) - .header(MITHRIL_API_VERSION_HEADER, aggregator_version); - }); - - assert!( - Version::parse(aggregator_version).unwrap() - > Version::parse(signer_version).unwrap() - ); - - client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .unwrap(); - - assert_api_version_warning_logged(&log_inspector, aggregator_version, signer_version); - } - - #[tokio::test] - async fn test_register_signature_ok_410_log_warning_if_api_version_mismatch() { - let aggregator_version = "2.0.0"; - let signer_version = "1.0.0"; - let single_signature = fake_data::single_signature((1..5).collect()); - let (server, mut client) = setup_server_and_client(); - let (logger, log_inspector) = TestLogger::memory(); - let version_provider = version_provider_with_open_api_version(signer_version); - client.api_version_provider = Arc::new(version_provider); - client.logger = logger; - let _server_mock = server.mock(|when, then| { - when.method(POST).path("/register-signatures"); - then.status(410) - .body( - serde_json::to_vec(&ClientError::new( - "already_aggregated".to_string(), - "too late".to_string(), - )) - .unwrap(), - ) - .header(MITHRIL_API_VERSION_HEADER, aggregator_version); - }); - - assert!( - Version::parse(aggregator_version).unwrap() - > Version::parse(signer_version).unwrap() - ); - - client - .register_signature( - &SignedEntityType::dummy(), - &single_signature, - &ProtocolMessage::default(), - ) - .await - .unwrap(); - - assert_api_version_warning_logged(&log_inspector, aggregator_version, signer_version); - } - } -} diff --git a/mithril-signer/tests/test_extensions/certificate_handler.rs b/mithril-signer/tests/test_extensions/certificate_handler.rs index c693a89e2bc..394346231fb 100644 --- a/mithril-signer/tests/test_extensions/certificate_handler.rs +++ b/mithril-signer/tests/test_extensions/certificate_handler.rs @@ -1,10 +1,10 @@ -use anyhow::anyhow; use async_trait::async_trait; use std::collections::BTreeSet; use std::{collections::HashMap, sync::Arc}; use tokio::sync::RwLock; use mithril_common::{ + StdResult, entities::{ CardanoTransactionsSigningConfig, Epoch, ProtocolMessage, SignedEntityConfig, SignedEntityType, SignedEntityTypeDiscriminants, Signer, SingleSignature, TimePoint, @@ -14,10 +14,7 @@ use mithril_common::{ }; use mithril_ticker::{MithrilTickerService, TickerService}; -use mithril_signer::{ - entities::SignerEpochSettings, - services::{AggregatorClient, AggregatorClientError}, -}; +use mithril_signer::{entities::SignerEpochSettings, services::AggregatorClient}; pub struct FakeAggregator { signed_entity_config: RwLock, @@ -57,25 +54,17 @@ impl FakeAggregator { signed_entity_config.allowed_discriminants = discriminants.clone(); } - async fn get_time_point(&self) -> Result { - let time_point = self - .ticker_service - .get_current_time_point() - .await - .map_err(|e| AggregatorClientError::RemoteServerTechnical(anyhow!(e)))?; - + async fn get_time_point(&self) -> StdResult { + let time_point = self.ticker_service.get_current_time_point().await?; Ok(time_point) } async fn get_current_signers( &self, store: &HashMap>, - ) -> Result, AggregatorClientError> { + ) -> StdResult> { let time_point = self.get_time_point().await?; - let epoch = time_point - .epoch - .offset_to_signer_retrieval_epoch() - .map_err(|e| AggregatorClientError::RemoteServerTechnical(anyhow!(e)))?; + let epoch = time_point.epoch.offset_to_signer_retrieval_epoch()?; Ok(store.get(&epoch).cloned().unwrap_or_default()) } @@ -83,7 +72,7 @@ impl FakeAggregator { async fn get_next_signers( &self, store: &HashMap>, - ) -> Result, AggregatorClientError> { + ) -> StdResult> { let time_point = self.get_time_point().await?; let epoch = time_point.epoch.offset_to_next_signer_retrieval_epoch(); @@ -93,9 +82,7 @@ impl FakeAggregator { #[async_trait] impl AggregatorClient for FakeAggregator { - async fn retrieve_epoch_settings( - &self, - ) -> Result, AggregatorClientError> { + async fn retrieve_epoch_settings(&self) -> StdResult> { if *self.withhold_epoch_settings.read().await { Ok(None) } else { @@ -113,11 +100,7 @@ impl AggregatorClient for FakeAggregator { } /// Registers signer with the aggregator - async fn register_signer( - &self, - epoch: Epoch, - signer: &Signer, - ) -> Result<(), AggregatorClientError> { + async fn register_signer(&self, epoch: Epoch, signer: &Signer) -> StdResult<()> { let mut store = self.registered_signers.write().await; let mut signers = store.get(&epoch).cloned().unwrap_or_default(); signers.push(signer.clone()); @@ -132,13 +115,11 @@ impl AggregatorClient for FakeAggregator { _signed_entity_type: &SignedEntityType, _signature: &SingleSignature, _protocol_message: &ProtocolMessage, - ) -> Result<(), AggregatorClientError> { + ) -> StdResult<()> { Ok(()) } - async fn retrieve_aggregator_features( - &self, - ) -> Result { + async fn retrieve_aggregator_features(&self) -> StdResult { let signed_entity_config = self.signed_entity_config.read().await; let mut message = AggregatorFeaturesMessage::dummy(); diff --git a/mithril-signer/tests/test_extensions/state_machine_tester.rs b/mithril-signer/tests/test_extensions/state_machine_tester.rs index 1435ab6aba3..bcaa1220f71 100644 --- a/mithril-signer/tests/test_extensions/state_machine_tester.rs +++ b/mithril-signer/tests/test_extensions/state_machine_tester.rs @@ -696,7 +696,7 @@ impl StateMachineTester { self.certificate_handler .register_signer(epoch, &signer_with_stake.to_owned().into()) .await - .map_err(|e| TestError::SubsystemError(e.into()))?; + .map_err(TestError::SubsystemError)?; } Ok(self) From 4b3612a8043c22782e2739b025aab06e87d42ba8 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Mon, 3 Nov 2025 11:52:20 +0100 Subject: [PATCH 11/13] test: add a test to check that the compression is enabled in signer and aggregator --- internal/mithril-aggregator-client/src/lib.rs | 1 - .../mithril-aggregator-client/src/test/mod.rs | 35 +++++++++++++++++++ mithril-aggregator/src/lib.rs | 5 +++ mithril-signer/src/lib.rs | 5 +++ 4 files changed, 45 insertions(+), 1 deletion(-) diff --git a/internal/mithril-aggregator-client/src/lib.rs b/internal/mithril-aggregator-client/src/lib.rs index 2ae905df9da..a57b4ab2a2d 100644 --- a/internal/mithril-aggregator-client/src/lib.rs +++ b/internal/mithril-aggregator-client/src/lib.rs @@ -7,7 +7,6 @@ mod client; mod error; mod external_trait_impls; pub mod query; -#[cfg(test)] mod test; pub use builder::AggregatorClientBuilder; diff --git a/internal/mithril-aggregator-client/src/test/mod.rs b/internal/mithril-aggregator-client/src/test/mod.rs index d0010379bfc..7327e753974 100644 --- a/internal/mithril-aggregator-client/src/test/mod.rs +++ b/internal/mithril-aggregator-client/src/test/mod.rs @@ -1,5 +1,7 @@ +#[cfg(test)] use httpmock::MockServer; +#[cfg(test)] use crate::AggregatorHttpClient; #[cfg(test)] @@ -29,3 +31,36 @@ macro_rules! assert_error_matches { } #[cfg(test)] pub(crate) use assert_error_matches; + +/// Define a test that checks that http compression is enabled for the client. +/// +/// Requires `httpmock` in dev-dependencies +#[macro_export] +macro_rules! test_http_compression_is_enabled { + () => { + #[tokio::test] + async fn test_http_compression_is_enabled_and_send_accept_encoding_header_with_correct_values() { + let server = httpmock::MockServer::start(); + server.mock(|when, then| { + when.is_true(|req| { + let headers = req.headers(); + let accept_encoding_header = headers + .get("accept-encoding") + .expect("Accept-Encoding header not found"); + + ["gzip", "br", "deflate", "zstd"].iter().all(|&encoding| { + accept_encoding_header.to_str().is_ok_and(|h| h.contains(encoding)) + }) + }); + + then.status(200).body("[]"); + }); + + let client = $crate::AggregatorHttpClient::builder(server.base_url()).build().unwrap(); + client + .send($crate::query::GetCertificatesListQuery::latest()) + .await + .expect("GET request should succeed with Accept-Encoding header"); + } + }; +} diff --git a/mithril-aggregator/src/lib.rs b/mithril-aggregator/src/lib.rs index 7d9909bff76..1f814d7faeb 100644 --- a/mithril-aggregator/src/lib.rs +++ b/mithril-aggregator/src/lib.rs @@ -69,3 +69,8 @@ use tikv_jemallocator::Jemalloc; #[cfg(all(not(target_env = "msvc"), feature = "jemallocator"))] #[global_allocator] static GLOBAL: Jemalloc = Jemalloc; + +#[cfg(test)] +mod tests { + mithril_aggregator_client::test_http_compression_is_enabled!(); +} diff --git a/mithril-signer/src/lib.rs b/mithril-signer/src/lib.rs index 5a770109531..3631aaf5e16 100644 --- a/mithril-signer/src/lib.rs +++ b/mithril-signer/src/lib.rs @@ -43,3 +43,8 @@ static GLOBAL: Jemalloc = Jemalloc; pub(crate) mod test_tools { mithril_common::define_test_logger!(); } + +#[cfg(test)] +mod tests { + mithril_aggregator_client::test_http_compression_is_enabled!(); +} From 6bda57afff2cee9cba6bb202d989cdea87b31b27 Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Mon, 3 Nov 2025 12:50:37 +0100 Subject: [PATCH 12/13] chore: simplify signer dependency and documentate http features configuration --- Cargo.lock | 2 -- internal/mithril-aggregator-client/README.md | 38 +++++++++++++++++++- mithril-signer/Cargo.toml | 6 ++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4da117d22e1..35ce3641275 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4111,7 +4111,6 @@ dependencies = [ "config", "criterion", "hex", - "http", "httpmock", "mithril-aggregator-client", "mithril-cardano-node-chain", @@ -4131,7 +4130,6 @@ dependencies = [ "prometheus-parse", "rand_core 0.6.4", "reqwest", - "semver", "serde", "serde_json", "slog", diff --git a/internal/mithril-aggregator-client/README.md b/internal/mithril-aggregator-client/README.md index f9a673353ec..31f3bea1734 100644 --- a/internal/mithril-aggregator-client/README.md +++ b/internal/mithril-aggregator-client/README.md @@ -1,3 +1,39 @@ # Mithril-aggregator-client [![CI workflow](https://github.com/input-output-hk/mithril/actions/workflows/ci.yml/badge.svg)](https://github.com/input-output-hk/mithril/actions/workflows/ci.yml) [![License](https://img.shields.io/badge/license-Apache%202.0-blue?style=flat-square)](https://github.com/input-output-hk/mithril/blob/main/LICENSE) [![Discord](https://img.shields.io/discord/500028886025895936.svg?logo=discord&style=flat-square)](https://discord.gg/5kaErDKDRq) -This crate provides a client to request data from a Mithril Aggregator. +This crate provides a client to request data from a Mithril Aggregator over http. + +## Configuration of http features + +Reqwest is used as the backend for the http client. Its default features are disabled to let the user control +which [features to enable](https://docs.rs/reqwest/latest/reqwest/#optional-features). + +To enable a reqwest feature, add the following to your `Cargo.toml`: + +```toml +reqwest = { version = "x.yy", features = ["feature_a", "feature_b"] } +``` + +for example, if reqwest is a workspace dependency, and you want to enable the `default` feature and the compression features: + +```toml +reqwest = { workspace = true, features = [ + "default", + "gzip", + "zstd", + "deflate", + "brotli" +] } +``` + +### Unused dependency warning + +You should add the `reqwest` dependency even if you don't use it directly in your code. +If you are using [`cargo machete`](https://github.com/bnjbvr/cargo-machete) to track unused dependencies, it will raise a warning. + +To avoid this warning, add the following to your `Cargo.toml`: + +```toml +[package.metadata.cargo-machete] +# reqwest: for features configuration, indirect dependency via `mithril-aggregator-client` +ignored = ["reqwest"] +``` diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index 33b5594847c..8e03ad166c5 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -9,6 +9,10 @@ homepage = { workspace = true } license = { workspace = true } repository = { workspace = true } +[package.metadata.cargo-machete] +# reqwest: for features configuration, indirect dependency via `mithril-aggregator-client` +ignored = ["reqwest"] + [features] default = ["jemallocator"] @@ -46,7 +50,6 @@ reqwest = { workspace = true, features = [ "deflate", "brotli" ] } -semver = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } slog = { workspace = true, features = [ @@ -64,7 +67,6 @@ tikv-jemallocator = { version = "0.6.0", optional = true } [dev-dependencies] criterion = { version = "0.7.0", features = ["html_reports", "async_tokio"] } -http = "1.3.1" httpmock = "0.8.1" mockall = { workspace = true } prometheus-parse = "0.2.5" From fcd8891eda19e376de7a11f95f048095867ab4cc Mon Sep 17 00:00:00 2001 From: DJO <790521+Alenar@users.noreply.github.com> Date: Mon, 3 Nov 2025 12:59:59 +0100 Subject: [PATCH 13/13] chore: upgrade crate versions * mithril-aggregator-client from `0.1.1` to `0.1.2` * mithril-protocol-config from `0.1.1` to `0.1.2` * mithril-aggregator from `0.7.92` to `0.7.93` * mithril-signer from `0.2.274` to `0.2.275` --- Cargo.lock | 8 ++++---- internal/mithril-aggregator-client/Cargo.toml | 2 +- internal/mithril-protocol-config/Cargo.toml | 2 +- mithril-aggregator/Cargo.toml | 2 +- mithril-signer/Cargo.toml | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 35ce3641275..1976d4de071 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3594,7 +3594,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.7.92" +version = "0.7.93" dependencies = [ "anyhow", "async-trait", @@ -3654,7 +3654,7 @@ dependencies = [ [[package]] name = "mithril-aggregator-client" -version = "0.1.1" +version = "0.1.2" dependencies = [ "anyhow", "async-trait", @@ -4024,7 +4024,7 @@ dependencies = [ [[package]] name = "mithril-protocol-config" -version = "0.1.1" +version = "0.1.2" dependencies = [ "anyhow", "async-trait", @@ -4102,7 +4102,7 @@ dependencies = [ [[package]] name = "mithril-signer" -version = "0.2.274" +version = "0.2.275" dependencies = [ "anyhow", "async-trait", diff --git a/internal/mithril-aggregator-client/Cargo.toml b/internal/mithril-aggregator-client/Cargo.toml index 937f26ee328..1b4ff98742a 100644 --- a/internal/mithril-aggregator-client/Cargo.toml +++ b/internal/mithril-aggregator-client/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator-client" -version = "0.1.1" +version = "0.1.2" description = "Client to request data from a Mithril Aggregator" authors.workspace = true documentation.workspace = true diff --git a/internal/mithril-protocol-config/Cargo.toml b/internal/mithril-protocol-config/Cargo.toml index cacb7d08f24..4434db76620 100644 --- a/internal/mithril-protocol-config/Cargo.toml +++ b/internal/mithril-protocol-config/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-protocol-config" -version = "0.1.1" +version = "0.1.2" description = "Configuraton parameters for Mithril network" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 24aa0f64047..8390d0136fb 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.7.92" +version = "0.7.93" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index 8e03ad166c5..c5ce5384531 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-signer" -version = "0.2.274" +version = "0.2.275" description = "A Mithril Signer" authors = { workspace = true } edition = { workspace = true }