From b307ab0f208f661a681f2b6ddf1e5c829e5f273a Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 1 Oct 2024 10:39:22 +0200 Subject: [PATCH 01/15] feat: add `network` and `allowed_discriminants` as parameters for `MithrilEpochService` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- mithril-aggregator/src/configuration.rs | 1 + .../src/dependency_injection/builder.rs | 7 ++++++- mithril-aggregator/src/services/epoch_service.rs | 14 ++++++++++++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index 5b40873eea8..7940f97c04c 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -289,6 +289,7 @@ impl Configuration { .map(|limit| if limit > 3 { limit as u64 } else { 3 }) } + // TODO: Maybe remove this function, signing configuration in SignedEntityConfig is stored in DB /// Compute a [SignedEntityConfig] based on this configuration. pub fn compute_signed_entity_config(&self) -> StdResult { let network = self.get_network()?; diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index 552bfcee96f..ceb6d22d7d0 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -1220,13 +1220,18 @@ impl DependenciesBuilder { async fn build_epoch_service(&mut self) -> Result { let verification_key_store = self.get_verification_key_store().await?; let epoch_settings_storer = self.get_epoch_settings_storer().await?; - let epoch_settings = self.get_epoch_settings_configuration()?; + let network = self.configuration.get_network()?; + let allowed_discriminants = self + .get_signed_entity_config()? + .list_allowed_signed_entity_types_discriminants(); let epoch_service = Arc::new(RwLock::new(MithrilEpochService::new( epoch_settings, epoch_settings_storer, verification_key_store, + network, + allowed_discriminants, ))); Ok(epoch_service) diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index fab60ac3c54..995d7089ee8 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -1,15 +1,17 @@ use anyhow::Context; use async_trait::async_trait; use slog_scope::debug; +use std::collections::BTreeSet; use std::sync::Arc; use thiserror::Error; use mithril_common::crypto_helper::ProtocolAggregateVerificationKey; use mithril_common::entities::{ - CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, Signer, SignerWithStake, + CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityTypeDiscriminants, + Signer, SignerWithStake, }; use mithril_common::protocol::{MultiSigner as ProtocolMultiSigner, SignerBuilder}; -use mithril_common::StdResult; +use mithril_common::{CardanoNetwork, StdResult}; use crate::entities::AggregatorEpochSettings; use crate::{EpochSettingsStorer, VerificationKeyStorer}; @@ -122,6 +124,8 @@ pub struct MithrilEpochService { computed_epoch_data: Option, epoch_settings_storer: Arc, verification_key_store: Arc, + network: CardanoNetwork, + allowed_discriminants: BTreeSet, } impl MithrilEpochService { @@ -130,6 +134,8 @@ impl MithrilEpochService { future_epoch_settings: AggregatorEpochSettings, epoch_settings_storer: Arc, verification_key_store: Arc, + network: CardanoNetwork, + allowed_discriminants: BTreeSet, ) -> Self { Self { future_epoch_settings, @@ -137,6 +143,8 @@ impl MithrilEpochService { computed_epoch_data: None, epoch_settings_storer, verification_key_store, + network, + allowed_discriminants, } } @@ -746,6 +754,8 @@ mod tests { }, Arc::new(epoch_settings_storer), Arc::new(vkey_store), + CardanoNetwork::TestNet(27), + BTreeSet::new(), ) } From ac100a6f61b384f232dece99121da165e5abebcd Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:12:50 +0200 Subject: [PATCH 02/15] feat: implement Signed entity configuration in Epoch Service --- .../src/services/epoch_service.rs | 266 +++++++++++++----- 1 file changed, 192 insertions(+), 74 deletions(-) diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index 995d7089ee8..90752a0f473 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -7,8 +7,8 @@ use thiserror::Error; use mithril_common::crypto_helper::ProtocolAggregateVerificationKey; use mithril_common::entities::{ - CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityTypeDiscriminants, - Signer, SignerWithStake, + BlockNumber, CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityConfig, + SignedEntityTypeDiscriminants, Signer, SignerWithStake, }; use mithril_common::protocol::{MultiSigner as ProtocolMultiSigner, SignerBuilder}; use mithril_common::{CardanoNetwork, StdResult}; @@ -96,6 +96,9 @@ pub trait EpochService: Sync + Send { /// Get the [protocol multi signer][ProtocolMultiSigner] for the next epoch fn next_protocol_multi_signer(&self) -> StdResult<&ProtocolMultiSigner>; + + /// Get the [SignedEntityConfig] for the current epoch. + fn signed_entity_config(&self) -> StdResult<&SignedEntityConfig>; } struct EpochData { @@ -107,6 +110,7 @@ struct EpochData { next_signers_with_stake: Vec, current_signers: Vec, next_signers: Vec, + signed_entity_config: SignedEntityConfig, } struct ComputedEpochData { @@ -245,6 +249,14 @@ impl EpochService for MithrilEpochService { let current_signers = Signer::vec_from(current_signers_with_stake.clone()); let next_signers = Signer::vec_from(next_signers_with_stake.clone()); + let signed_entity_config = SignedEntityConfig { + allowed_discriminants: self.allowed_discriminants.clone(), + network: self.network, + cardano_transactions_signing_config: epoch_settings + .cardano_transactions_signing_config + .clone(), + }; + self.epoch_data = Some(EpochData { epoch, epoch_settings, @@ -254,6 +266,7 @@ impl EpochService for MithrilEpochService { next_signers_with_stake, current_signers, next_signers, + signed_entity_config, }); self.computed_epoch_data = None; @@ -370,6 +383,10 @@ impl EpochService for MithrilEpochService { fn next_protocol_multi_signer(&self) -> StdResult<&ProtocolMultiSigner> { Ok(&self.unwrap_computed_data()?.next_protocol_multi_signer) } + + fn signed_entity_config(&self) -> StdResult<&SignedEntityConfig> { + Ok(&self.unwrap_data()?.signed_entity_config) + } } #[cfg(test)] @@ -423,6 +440,7 @@ impl FakeEpochService { next_signers_with_stake, current_signers, next_signers, + signed_entity_config: SignedEntityConfig::dummy(), }), computed_epoch_data: Some(ComputedEpochData { aggregate_verification_key: protocol_multi_signer @@ -596,17 +614,21 @@ impl EpochService for FakeEpochService { fn next_protocol_multi_signer(&self) -> StdResult<&ProtocolMultiSigner> { Ok(&self.unwrap_computed_data()?.next_protocol_multi_signer) } + + fn signed_entity_config(&self) -> StdResult<&SignedEntityConfig> { + Ok(&self.unwrap_data()?.signed_entity_config) + } } #[cfg(test)] mod tests { - use mithril_common::entities::{CardanoTransactionsSigningConfig, PartyId}; + use mithril_common::entities::{CardanoTransactionsSigningConfig, PartyId, SignedEntityType}; use mithril_common::test_utils::{fake_data, MithrilFixture, MithrilFixtureBuilder}; use mithril_persistence::store::adapter::MemoryAdapter; use std::collections::{BTreeSet, HashMap}; use crate::services::epoch_service::tests::ServiceBuilderParameters::WithFutureProtocolParameters; - use crate::store::FakeEpochSettingsStorer; + use crate::store::{FakeEpochSettingsStorer, MockVerificationKeyStorer}; use crate::VerificationKeyStore; use super::*; @@ -623,6 +645,7 @@ mod tests { next_signers_with_stake: BTreeSet, current_signers: BTreeSet, next_signers: BTreeSet, + signed_entity_config: SignedEntityConfig, } #[derive(Debug, Clone, PartialEq)] @@ -656,6 +679,7 @@ mod tests { .collect(), current_signers: service.current_signers()?.clone().into_iter().collect(), next_signers: service.next_signers()?.clone().into_iter().collect(), + signed_entity_config: service.signed_entity_config()?.clone(), }) } } @@ -679,12 +703,97 @@ mod tests { .collect() } + struct EpochServiceBuilder { + cardano_transactions_signing_config: CardanoTransactionsSigningConfig, + future_protocol_parameters: ProtocolParameters, + network: CardanoNetwork, + allowed_discriminants: BTreeSet, + signer_retrieval_epoch: Epoch, + next_signer_retrieval_epoch: Epoch, + signers_with_stake: Vec, + next_signers_with_stake: Vec, + stored_epoch_settings: AggregatorEpochSettings, + stored_next_epoch_settings: AggregatorEpochSettings, + stored_upcoming_epoch_settings: AggregatorEpochSettings, + } + + impl EpochServiceBuilder { + fn new(epoch: Epoch, current_epoch_fixture: MithrilFixture) -> Self { + let next_epoch_fixture = current_epoch_fixture.clone(); + Self { + // Aggregator configuration + cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), + future_protocol_parameters: current_epoch_fixture.protocol_parameters(), + network: CardanoNetwork::TestNet(0), + allowed_discriminants: BTreeSet::new(), + // Epoch used for verification keys and database + signer_retrieval_epoch: epoch.offset_to_signer_retrieval_epoch().unwrap(), + next_signer_retrieval_epoch: epoch.offset_to_next_signer_retrieval_epoch(), + // Signers in verification key store + signers_with_stake: current_epoch_fixture.signers_with_stake(), + next_signers_with_stake: next_epoch_fixture.signers_with_stake(), + // Database data + stored_epoch_settings: AggregatorEpochSettings { + protocol_parameters: current_epoch_fixture.protocol_parameters(), + cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), + }, + stored_next_epoch_settings: AggregatorEpochSettings { + protocol_parameters: current_epoch_fixture.protocol_parameters(), + cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), + }, + stored_upcoming_epoch_settings: AggregatorEpochSettings { + protocol_parameters: current_epoch_fixture.protocol_parameters(), + cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), + }, + } + } + + fn build(self) -> MithrilEpochService { + let epoch_settings_storer = FakeEpochSettingsStorer::new(vec![ + (self.signer_retrieval_epoch, self.stored_epoch_settings), + ( + self.next_signer_retrieval_epoch, + self.stored_next_epoch_settings.clone(), + ), + ( + self.next_signer_retrieval_epoch.next(), + self.stored_upcoming_epoch_settings.clone(), + ), + ]); + let vkey_store = VerificationKeyStore::new(Box::new( + MemoryAdapter::new(Some(vec![ + ( + self.signer_retrieval_epoch, + map_signers_for_vkey_store(&self.signers_with_stake), + ), + ( + self.next_signer_retrieval_epoch, + map_signers_for_vkey_store(&self.next_signers_with_stake), + ), + ])) + .unwrap(), + )); + MithrilEpochService::new( + AggregatorEpochSettings { + protocol_parameters: self.future_protocol_parameters, + cardano_transactions_signing_config: self.cardano_transactions_signing_config, + }, + Arc::new(epoch_settings_storer), + Arc::new(vkey_store), + self.network, + self.allowed_discriminants, + ) + } + } + + // TODO: Use EpochServiceBuilder and remove ServiceBuilderParameters enum ServiceBuilderParameters { DifferentFixtureForSecondEpoch(MithrilFixture), UpcomingProtocolParameters(ProtocolParameters), WithFutureProtocolParameters(ProtocolParameters), } + // TODO: Use EpochServiceBuilder and remove build_service /// By default will copy data from the given fixture for all epochs, can be fined tuned /// with the [ServiceBuilderParameters]. async fn build_service( @@ -692,71 +801,31 @@ mod tests { current_epoch_fixture: &MithrilFixture, additional_params: &[ServiceBuilderParameters], ) -> MithrilEpochService { - let signer_retrieval_epoch = epoch.offset_to_signer_retrieval_epoch().unwrap(); - let next_signer_retrieval_epoch = epoch.offset_to_next_signer_retrieval_epoch(); - let mut next_epoch_fixture = current_epoch_fixture; - let mut upcoming_protocol_parameters = current_epoch_fixture.protocol_parameters(); - let mut future_protocol_parameters = current_epoch_fixture.protocol_parameters(); - + let mut builder = EpochServiceBuilder::new(epoch, current_epoch_fixture.clone()); for params in additional_params { match params { ServiceBuilderParameters::DifferentFixtureForSecondEpoch(fixture) => { - next_epoch_fixture = fixture + builder.stored_next_epoch_settings = AggregatorEpochSettings { + protocol_parameters: fixture.protocol_parameters(), + cardano_transactions_signing_config: + CardanoTransactionsSigningConfig::dummy(), + }; + builder.next_signers_with_stake = fixture.signers_with_stake().clone(); } ServiceBuilderParameters::UpcomingProtocolParameters(params) => { - upcoming_protocol_parameters = params.clone() + builder.stored_upcoming_epoch_settings = AggregatorEpochSettings { + protocol_parameters: params.clone(), + cardano_transactions_signing_config: + CardanoTransactionsSigningConfig::dummy(), + }; + } + ServiceBuilderParameters::WithFutureProtocolParameters(params) => { + builder.future_protocol_parameters = params.clone(); } - WithFutureProtocolParameters(params) => future_protocol_parameters = params.clone(), } } - let epoch_settings_storer = FakeEpochSettingsStorer::new(vec![ - ( - signer_retrieval_epoch, - AggregatorEpochSettings { - protocol_parameters: current_epoch_fixture.protocol_parameters(), - cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), - }, - ), - ( - next_signer_retrieval_epoch, - AggregatorEpochSettings { - protocol_parameters: next_epoch_fixture.protocol_parameters(), - cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), - }, - ), - ( - next_signer_retrieval_epoch.next(), - AggregatorEpochSettings { - protocol_parameters: upcoming_protocol_parameters.clone(), - cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), - }, - ), - ]); - let vkey_store = VerificationKeyStore::new(Box::new( - MemoryAdapter::new(Some(vec![ - ( - signer_retrieval_epoch, - map_signers_for_vkey_store(¤t_epoch_fixture.signers_with_stake()), - ), - ( - next_signer_retrieval_epoch, - map_signers_for_vkey_store(&next_epoch_fixture.signers_with_stake()), - ), - ])) - .unwrap(), - )); - - MithrilEpochService::new( - AggregatorEpochSettings { - protocol_parameters: future_protocol_parameters, - cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), - }, - Arc::new(epoch_settings_storer), - Arc::new(vkey_store), - CardanoNetwork::TestNet(27), - BTreeSet::new(), - ) + builder.build() } #[tokio::test] @@ -769,19 +838,22 @@ mod tests { let upcoming_protocol_parameters = fake_data::protocol_parameters(); let epoch = Epoch(5); - let mut service = build_service( - epoch, - ¤t_epoch_fixture, - &[ - ServiceBuilderParameters::DifferentFixtureForSecondEpoch( - next_epoch_fixture.clone(), - ), - ServiceBuilderParameters::UpcomingProtocolParameters( - upcoming_protocol_parameters.clone(), - ), - ], - ) - .await; + let builder = EpochServiceBuilder { + next_signers_with_stake: next_epoch_fixture.signers_with_stake(), + stored_next_epoch_settings: AggregatorEpochSettings { + protocol_parameters: next_epoch_fixture.protocol_parameters(), + ..AggregatorEpochSettings::dummy() + }, + stored_upcoming_epoch_settings: AggregatorEpochSettings { + protocol_parameters: upcoming_protocol_parameters.clone(), + ..AggregatorEpochSettings::dummy() + }, + network: SignedEntityConfig::dummy().network, + allowed_discriminants: SignedEntityConfig::dummy().allowed_discriminants, + ..EpochServiceBuilder::new(epoch, current_epoch_fixture.clone()) + }; + + let mut service = builder.build(); service .inform_epoch(epoch) @@ -793,7 +865,7 @@ mod tests { .expect("extracting data from service should not fail"); assert_eq!( - data, + data.clone(), ExpectedEpochData { epoch, protocol_parameters: current_epoch_fixture.protocol_parameters(), @@ -811,6 +883,50 @@ mod tests { .collect(), current_signers: current_epoch_fixture.signers().into_iter().collect(), next_signers: next_epoch_fixture.signers().into_iter().collect(), + signed_entity_config: SignedEntityConfig::dummy(), + } + ); + } + + #[tokio::test] + async fn inform_epoch_get_signed_entity_config_from_its_dependencies_and_store() { + let epoch = Epoch(5); + + let cardano_transactions_signing_config = + CardanoTransactionsSigningConfig::new(BlockNumber(29), BlockNumber(986)); + let network = CardanoNetwork::TestNet(27); + let allowed_discriminants = BTreeSet::from([ + SignedEntityTypeDiscriminants::CardanoTransactions, + SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, + ]); + + let builder = EpochServiceBuilder { + network, + allowed_discriminants: allowed_discriminants.clone(), + stored_epoch_settings: AggregatorEpochSettings { + cardano_transactions_signing_config: cardano_transactions_signing_config.clone(), + ..AggregatorEpochSettings::dummy() + }, + ..EpochServiceBuilder::new(epoch, MithrilFixtureBuilder::default().build()) + }; + + let mut service = builder.build(); + + service + .inform_epoch(epoch) + .await + .expect("inform_epoch should not fail"); + + let signed_entity_config = service + .signed_entity_config() + .expect("extracting data from service should not fail"); + + assert_eq!( + signed_entity_config.clone(), + SignedEntityConfig { + allowed_discriminants, + network, + cardano_transactions_signing_config, } ); } @@ -978,6 +1094,7 @@ mod tests { "next_protocol_multi_signer", service.next_protocol_multi_signer().err(), ), + ("signed_entity_config", service.signed_entity_config().err()), ] { let error = res.unwrap_or_else(|| panic!("getting {name} should have returned an error")); @@ -1008,6 +1125,7 @@ mod tests { assert!(service.next_signers_with_stake().is_ok()); assert!(service.current_signers().is_ok()); assert!(service.next_signers().is_ok()); + assert!(service.signed_entity_config().is_ok()); for (name, res) in [ ( From e54cd9a94cd5cb00f887e0821350415ccc72eef8 Mon Sep 17 00:00:00 2001 From: sfauvel Date: Thu, 3 Oct 2024 17:44:08 +0200 Subject: [PATCH 03/15] Move outdated computation from state_machine to the runner --- mithril-aggregator/src/runtime/runner.rs | 104 +++++++++++++++++- .../src/runtime/state_machine.rs | 62 ++++------- .../src/services/epoch_service.rs | 6 +- 3 files changed, 127 insertions(+), 45 deletions(-) diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index c3735f7ce63..2fc7ec9efb6 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -130,6 +130,13 @@ pub trait AggregatorRunnerTrait: Sync + Send { signed_entity_type: &SignedEntityType, protocol_message: &ProtocolMessage, ) -> StdResult; + + /// Checks if the open message is considered outdated. + async fn is_outdated( + &self, + open_message_signed_entity_type: SignedEntityType, + last_time_point: &TimePoint, + ) -> StdResult; } /// The runner responsibility is to expose a code API for the state machine. It @@ -486,6 +493,33 @@ impl AggregatorRunnerTrait for AggregatorRunner { .create_open_message(signed_entity_type, protocol_message) .await } + + async fn is_outdated( + &self, + open_message_signed_entity_type: SignedEntityType, + last_time_point: &TimePoint, + ) -> StdResult { + let current_open_message = self + .get_current_open_message_for_signed_entity_type( + &open_message_signed_entity_type, + ) + .await + .with_context(|| format!("AggregatorRuntime can not get the current open message for signed entity type: '{}'", &open_message_signed_entity_type))?; + let is_expired_open_message = current_open_message + .as_ref() + .map(|om| om.is_expired) + .unwrap_or(false); + + let exists_newer_open_message = { + let new_signed_entity_type = self + .dependencies + .signed_entity_config + .time_point_to_signed_entity(&open_message_signed_entity_type, last_time_point)?; + new_signed_entity_type != open_message_signed_entity_type + }; + + Ok(exists_newer_open_message || is_expired_open_message) + } } #[cfg(test)] @@ -502,7 +536,7 @@ pub mod tests { use async_trait::async_trait; use chrono::{DateTime, Utc}; use mithril_common::entities::{ - CardanoTransactionsSigningConfig, ChainPoint, SignedEntityTypeDiscriminants, + CardanoTransactionsSigningConfig, ChainPoint, Epoch, SignedEntityTypeDiscriminants, }; use mithril_common::signed_entity_type_lock::SignedEntityTypeLock; use mithril_common::{ @@ -1236,4 +1270,72 @@ pub mod tests { assert!(!signed_entities.is_empty()); assert!(!signed_entities.contains(&SignedEntityTypeDiscriminants::CardanoTransactions)); } + + #[tokio::test] + async fn is_outdated_return_false_when_message_is_not_expired_and_has_same_signed_entity_type() + { + assert!(!is_outdated_returned_when(IsExpired::No, true).await); + } + + #[tokio::test] + async fn is_outdated_return_true_when_message_is_expired_and_has_same_signed_entity_type() { + assert!(is_outdated_returned_when(IsExpired::Yes, true).await); + } + + #[tokio::test] + async fn is_outdated_return_true_when_message_is_not_expired_and_has_not_the_same_signed_entity_type( + ) { + assert!(is_outdated_returned_when(IsExpired::No, false).await); + } + + #[tokio::test] + async fn is_outdated_return_true_when_message_is_expired_and_has_not_the_same_signed_entity_type( + ) { + assert!(is_outdated_returned_when(IsExpired::Yes, false).await); + } + + async fn is_outdated_returned_when( + is_expired: IsExpired, + same_signed_entity_type: bool, + ) -> bool { + let current_time_point = TimePoint { + epoch: Epoch(2), + immutable_file_number: 1, + ..TimePoint::dummy() + }; + + let message_epoch = if same_signed_entity_type { + current_time_point.epoch + } else { + current_time_point.epoch - 1 + }; + let open_message = OpenMessage { + signed_entity_type: SignedEntityType::MithrilStakeDistribution(message_epoch), + is_expired: is_expired == IsExpired::Yes, + ..OpenMessage::dummy() + }; + + let runner = { + let mut deps = initialize_dependencies().await; + let mut mock_certifier_service = MockCertifierService::new(); + + let open_message_cloned = open_message.clone(); + mock_certifier_service + .expect_get_open_message() + .times(1) + .return_once(|_| Ok(Some(open_message_cloned))); + mock_certifier_service + .expect_mark_open_message_if_expired() + .returning(|_| Ok(None)); + + deps.certifier_service = Arc::new(mock_certifier_service); + + build_runner_with_fixture_data(deps).await + }; + + runner + .is_outdated(open_message.signed_entity_type, ¤t_time_point) + .await + .unwrap() + } } diff --git a/mithril-aggregator/src/runtime/state_machine.rs b/mithril-aggregator/src/runtime/state_machine.rs index 8da6cf40b09..ebbaa82f0e8 100644 --- a/mithril-aggregator/src/runtime/state_machine.rs +++ b/mithril-aggregator/src/runtime/state_machine.rs @@ -222,34 +222,21 @@ impl AggregatorRuntime { self.runner.get_time_point_from_chain().await.with_context(|| { "AggregatorRuntime in the state SIGNING can not get current time point from chain" })?; - let current_open_message = self + + let is_outdated = self .runner - .get_current_open_message_for_signed_entity_type( - &state.open_message.signed_entity_type, + .is_outdated( + state.open_message.signed_entity_type.clone(), + &last_time_point, ) - .await - .with_context(|| format!("AggregatorRuntime can not get the current open message for signed entity type: '{}'", &state.open_message.signed_entity_type))?; - let is_expired_open_message = current_open_message - .as_ref() - .map(|om| om.is_expired) - .unwrap_or(false); - let exists_newer_open_message = { - let new_signed_entity_type = self - .config - .signed_entity_config - .time_point_to_signed_entity( - &state.open_message.signed_entity_type, - &last_time_point, - )?; - new_signed_entity_type != state.open_message.signed_entity_type - }; + .await?; if state.current_time_point.epoch < last_time_point.epoch { // SIGNING > IDLE info!("→ Epoch changed, transitioning to IDLE"); let new_state = self.transition_from_signing_to_idle(state).await?; self.state = AggregatorState::Idle(new_state); - } else if exists_newer_open_message || is_expired_open_message { + } else if is_outdated { // SIGNING > READY info!("→ Open message changed, transitioning to READY"); let new_state = self @@ -409,7 +396,7 @@ mod tests { use mockall::predicate; use std::time::Duration; - use mithril_common::entities::{Epoch, SignedEntityConfig, SignedEntityType}; + use mithril_common::entities::SignedEntityConfig; use mithril_common::test_utils::fake_data; use super::super::runner::MockAggregatorRunner; @@ -646,27 +633,20 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_get_current_open_message_for_signed_entity_type() + .expect_is_outdated() .once() - .returning(|_| { - Ok(Some(OpenMessage { - signed_entity_type: SignedEntityType::MithrilStakeDistribution(Epoch(1)), - ..OpenMessage::dummy() - })) - }); + .returning(|_, _| Ok(true)); runner .expect_drop_pending_certificate() .once() .returning(|| Ok(Some(fake_data::certificate_pending()))); - let state = SigningState { + let initial_state = AggregatorState::Signing(SigningState { current_time_point: TimePoint::dummy(), - open_message: OpenMessage { - signed_entity_type: SignedEntityType::MithrilStakeDistribution(Epoch(2)), - ..OpenMessage::dummy() - }, - }; - let mut runtime = init_runtime(Some(AggregatorState::Signing(state)), runner).await; + open_message: OpenMessage::dummy(), + }); + + let mut runtime = init_runtime(Some(initial_state), runner).await; runtime.cycle().await.unwrap(); assert_eq!("ready".to_string(), runtime.get_state()); @@ -680,9 +660,9 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_get_current_open_message_for_signed_entity_type() + .expect_is_outdated() .once() - .returning(|_| Ok(Some(OpenMessage::dummy()))); + .returning(|_, _| Ok(false)); runner .expect_create_certificate() .once() @@ -713,9 +693,9 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_get_current_open_message_for_signed_entity_type() + .expect_is_outdated() .once() - .returning(|_| Ok(Some(OpenMessage::dummy()))); + .returning(|_, _| Ok(false)); runner .expect_create_certificate() .return_once(move |_| Ok(Some(fake_data::certificate("whatever".to_string())))); @@ -753,9 +733,9 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_get_current_open_message_for_signed_entity_type() + .expect_is_outdated() .once() - .returning(|_| Ok(Some(OpenMessage::dummy()))); + .returning(|_, _| Ok(false)); runner .expect_create_certificate() .return_once(move |_| Ok(Some(fake_data::certificate("whatever".to_string())))); diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index 90752a0f473..eae5893f17f 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -7,7 +7,7 @@ use thiserror::Error; use mithril_common::crypto_helper::ProtocolAggregateVerificationKey; use mithril_common::entities::{ - BlockNumber, CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityConfig, + CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityConfig, SignedEntityTypeDiscriminants, Signer, SignerWithStake, }; use mithril_common::protocol::{MultiSigner as ProtocolMultiSigner, SignerBuilder}; @@ -622,13 +622,13 @@ impl EpochService for FakeEpochService { #[cfg(test)] mod tests { - use mithril_common::entities::{CardanoTransactionsSigningConfig, PartyId, SignedEntityType}; + use mithril_common::entities::{BlockNumber, CardanoTransactionsSigningConfig, PartyId}; use mithril_common::test_utils::{fake_data, MithrilFixture, MithrilFixtureBuilder}; use mithril_persistence::store::adapter::MemoryAdapter; use std::collections::{BTreeSet, HashMap}; use crate::services::epoch_service::tests::ServiceBuilderParameters::WithFutureProtocolParameters; - use crate::store::{FakeEpochSettingsStorer, MockVerificationKeyStorer}; + use crate::store::FakeEpochSettingsStorer; use crate::VerificationKeyStore; use super::*; From a678a92d1250a1f53f8043fe38dd8c058ea1316f Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 7 Oct 2024 17:08:52 +0200 Subject: [PATCH 04/15] Remove cardano signing configuration in `init_state_from_fixture` parameters. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead, retrieve it from the aggregator configuration. Co-authored-by: Sébastien Fauvel --- .../src/dependency_injection/containers.rs | 11 ++++------- .../src/services/certifier/certifier_service.rs | 8 ++------ .../tests/test_extensions/runtime_tester.rs | 7 +------ 3 files changed, 7 insertions(+), 19 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/containers.rs b/mithril-aggregator/src/dependency_injection/containers.rs index 73fac4805ff..08b5227f2ca 100644 --- a/mithril-aggregator/src/dependency_injection/containers.rs +++ b/mithril-aggregator/src/dependency_injection/containers.rs @@ -194,19 +194,16 @@ impl DependencyContainer { /// /// Fill the stores of a [DependencyManager] in a way to simulate an aggregator state /// using the data from a precomputed fixture. - pub async fn init_state_from_fixture( - &self, - fixture: &MithrilFixture, - cardano_transactions_signing_config: &CardanoTransactionsSigningConfig, - target_epochs: &[Epoch], - ) { + pub async fn init_state_from_fixture(&self, fixture: &MithrilFixture, target_epochs: &[Epoch]) { for epoch in target_epochs { self.epoch_settings_storer .save_epoch_settings( *epoch, AggregatorEpochSettings { protocol_parameters: fixture.protocol_parameters(), - cardano_transactions_signing_config: cardano_transactions_signing_config + cardano_transactions_signing_config: self + .config + .cardano_transactions_signing_config .clone(), }, ) diff --git a/mithril-aggregator/src/services/certifier/certifier_service.rs b/mithril-aggregator/src/services/certifier/certifier_service.rs index a5f25ac3f8e..a2c26097cfc 100644 --- a/mithril-aggregator/src/services/certifier/certifier_service.rs +++ b/mithril-aggregator/src/services/certifier/certifier_service.rs @@ -382,7 +382,7 @@ mod tests { }; use chrono::{DateTime, Days}; use mithril_common::{ - entities::{CardanoDbBeacon, CardanoTransactionsSigningConfig, ProtocolMessagePartKey}, + entities::{CardanoDbBeacon, ProtocolMessagePartKey}, test_utils::{fake_data, MithrilFixture, MithrilFixtureBuilder}, }; use tokio::sync::RwLock; @@ -442,11 +442,7 @@ mod tests { .await .unwrap(); dependency_manager - .init_state_from_fixture( - fixture, - &CardanoTransactionsSigningConfig::dummy(), - epochs_with_signers, - ) + .init_state_from_fixture(fixture, epochs_with_signers) .await; MithrilCertifierService::from_deps(network, dependency_builder).await diff --git a/mithril-aggregator/tests/test_extensions/runtime_tester.rs b/mithril-aggregator/tests/test_extensions/runtime_tester.rs index 896a5222fce..40b403f9fd0 100644 --- a/mithril-aggregator/tests/test_extensions/runtime_tester.rs +++ b/mithril-aggregator/tests/test_extensions/runtime_tester.rs @@ -9,7 +9,6 @@ use mithril_aggregator::{ AggregatorRuntime, Configuration, DependencyContainer, DumbSnapshotUploader, DumbSnapshotter, SignerRegistrationError, }; -use mithril_common::entities::CardanoTransactionsSigningConfig; use mithril_common::{ cardano_block_scanner::{DumbBlockScanner, ScannedBlock}, chain_observer::{ChainObserver, FakeObserver}, @@ -191,11 +190,7 @@ impl RuntimeTester { // Init the stores needed for a genesis certificate let genesis_epochs = self.dependencies.get_genesis_epochs().await; self.dependencies - .init_state_from_fixture( - fixture, - &CardanoTransactionsSigningConfig::dummy(), - &[genesis_epochs.0, genesis_epochs.1], - ) + .init_state_from_fixture(fixture, &[genesis_epochs.0, genesis_epochs.1]) .await; Ok(()) } From 329486484bb175115fc8810cbe4bdf7dd9875374 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 7 Oct 2024 17:12:08 +0200 Subject: [PATCH 05/15] Use `SignedEntityConfig` from the `EpochService` to create the signed entity type --- .../mithril_stake_distribution.rs | 5 ++- .../src/http_server/routes/epoch_routes.rs | 2 + mithril-aggregator/src/multi_signer.rs | 2 + mithril-aggregator/src/runtime/runner.rs | 43 +++++++++++++++---- .../src/services/epoch_service.rs | 4 +- .../signable_builder/signable_seed_builder.rs | 3 +- 6 files changed, 48 insertions(+), 11 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs b/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs index 6ac599fdc71..081396005f6 100644 --- a/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs +++ b/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs @@ -38,7 +38,9 @@ impl ArtifactBuilder for MithrilStakeDistributi #[cfg(test)] mod tests { - use mithril_common::{crypto_helper::ProtocolParameters, test_utils::fake_data}; + use mithril_common::{ + crypto_helper::ProtocolParameters, entities::SignedEntityConfig, test_utils::fake_data, + }; use std::sync::Arc; use tokio::sync::RwLock; @@ -61,6 +63,7 @@ mod tests { &epoch_settings, &signers_with_stake, &signers_with_stake, + SignedEntityConfig::dummy(), ); let mithril_stake_distribution_artifact_builder = MithrilStakeDistributionArtifactBuilder::new(Arc::new(RwLock::new(epoch_service))); diff --git a/mithril-aggregator/src/http_server/routes/epoch_routes.rs b/mithril-aggregator/src/http_server/routes/epoch_routes.rs index 77463cef2c0..38f6a0010c4 100644 --- a/mithril-aggregator/src/http_server/routes/epoch_routes.rs +++ b/mithril-aggregator/src/http_server/routes/epoch_routes.rs @@ -202,6 +202,7 @@ mod tests { &upcoming_epoch_settings, &fake_data::signers_with_stakes(5), &fake_data::signers_with_stakes(3), + SignedEntityConfig::dummy(), ); let message = get_epoch_settings_message( @@ -245,6 +246,7 @@ mod tests { &AggregatorEpochSettings::dummy(), &fake_data::signers_with_stakes(5), &fake_data::signers_with_stakes(3), + SignedEntityConfig::dummy(), ); let message = get_epoch_settings_message( diff --git a/mithril-aggregator/src/multi_signer.rs b/mithril-aggregator/src/multi_signer.rs index 586259403dc..bade2b3e7a2 100644 --- a/mithril-aggregator/src/multi_signer.rs +++ b/mithril-aggregator/src/multi_signer.rs @@ -134,6 +134,7 @@ impl MultiSigner for MultiSignerImpl { #[cfg(test)] mod tests { + use entities::SignedEntityConfig; use std::sync::Arc; use tokio::sync::RwLock; @@ -189,6 +190,7 @@ mod tests { }, &fixture.signers_with_stake(), &next_fixture.signers_with_stake(), + SignedEntityConfig::dummy(), )))); { diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index 2fc7ec9efb6..5f765023c5c 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -513,7 +513,10 @@ impl AggregatorRunnerTrait for AggregatorRunner { let exists_newer_open_message = { let new_signed_entity_type = self .dependencies - .signed_entity_config + .epoch_service + .read() + .await + .signed_entity_config()? .time_point_to_signed_entity(&open_message_signed_entity_type, last_time_point)?; new_signed_entity_type != open_message_signed_entity_type }; @@ -536,9 +539,11 @@ pub mod tests { use async_trait::async_trait; use chrono::{DateTime, Utc}; use mithril_common::entities::{ - CardanoTransactionsSigningConfig, ChainPoint, Epoch, SignedEntityTypeDiscriminants, + CardanoDbBeacon, CardanoTransactionsSigningConfig, ChainPoint, Epoch, SignedEntityConfig, + SignedEntityTypeDiscriminants, }; use mithril_common::signed_entity_type_lock::SignedEntityTypeLock; + use mithril_common::CardanoNetwork; use mithril_common::{ chain_observer::FakeObserver, digesters::DumbImmutableFileObserver, @@ -580,7 +585,6 @@ pub mod tests { .unwrap(); deps.init_state_from_fixture( &fixture, - &CardanoTransactionsSigningConfig::dummy(), &[ current_epoch.offset_to_signer_retrieval_epoch().unwrap(), current_epoch, @@ -1300,17 +1304,28 @@ pub mod tests { ) -> bool { let current_time_point = TimePoint { epoch: Epoch(2), - immutable_file_number: 1, + immutable_file_number: 12, ..TimePoint::dummy() }; - let message_epoch = if same_signed_entity_type { - current_time_point.epoch + let message_network = CardanoNetwork::MainNet; + let epoch_service_network = if same_signed_entity_type { + message_network } else { - current_time_point.epoch - 1 + CardanoNetwork::DevNet(412) + }; + + let epoch_service_signed_entity_config = SignedEntityConfig { + network: epoch_service_network, + ..SignedEntityConfig::dummy() + }; + let cardano_db_beacon = CardanoDbBeacon { + network: message_network.to_string(), + epoch: current_time_point.epoch, + immutable_file_number: current_time_point.immutable_file_number, }; let open_message = OpenMessage { - signed_entity_type: SignedEntityType::MithrilStakeDistribution(message_epoch), + signed_entity_type: SignedEntityType::CardanoImmutableFilesFull(cardano_db_beacon), is_expired: is_expired == IsExpired::Yes, ..OpenMessage::dummy() }; @@ -1330,6 +1345,18 @@ pub mod tests { deps.certifier_service = Arc::new(mock_certifier_service); + let epoch_settings = AggregatorEpochSettings::dummy(); + let epoch_service = FakeEpochService::with_data( + current_time_point.epoch, + &epoch_settings, + &epoch_settings, + &epoch_settings, + &fake_data::signers_with_stakes(1), + &fake_data::signers_with_stakes(1), + epoch_service_signed_entity_config, + ); + deps.epoch_service = Arc::new(RwLock::new(epoch_service)); + build_runner_with_fixture_data(deps).await }; diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index eae5893f17f..a1a824ebdef 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -409,6 +409,7 @@ impl FakeEpochService { upcoming_epoch_settings: &AggregatorEpochSettings, current_signers_with_stake: &[SignerWithStake], next_signers_with_stake: &[SignerWithStake], + signed_entity_config: SignedEntityConfig, ) -> Self { let protocol_multi_signer = SignerBuilder::new( current_signers_with_stake, @@ -440,7 +441,7 @@ impl FakeEpochService { next_signers_with_stake, current_signers, next_signers, - signed_entity_config: SignedEntityConfig::dummy(), + signed_entity_config, }), computed_epoch_data: Some(ComputedEpochData { aggregate_verification_key: protocol_multi_signer @@ -481,6 +482,7 @@ impl FakeEpochService { &upcoming_epoch_settings, &fixture.signers_with_stake(), &fixture.signers_with_stake(), + SignedEntityConfig::dummy(), ) } diff --git a/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs b/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs index a845acb54e3..8082c34351c 100644 --- a/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs +++ b/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs @@ -57,7 +57,7 @@ impl SignableSeedBuilder for AggregatorSignableSeedBuilder { #[cfg(test)] mod tests { use mithril_common::{ - entities::Epoch, + entities::{Epoch, SignedEntityConfig}, test_utils::{MithrilFixture, MithrilFixtureBuilder}, }; @@ -86,6 +86,7 @@ mod tests { }, &fixture.signers_with_stake(), &next_fixture.signers_with_stake(), + SignedEntityConfig::dummy(), ))); AggregatorSignableSeedBuilder::new(epoch_service) From 477f84ee34a62e14bcc116a1433ad034f7289d46 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Mon, 7 Oct 2024 18:00:30 +0200 Subject: [PATCH 06/15] Remove the signed entity config in the dependency container of the aggregator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add `allowed_discriminants` in the dependencies builder - Remove `signed_entity_config` in `AggregatorConfig` Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- mithril-aggregator/src/configuration.rs | 36 ++++--- .../src/dependency_injection/builder.rs | 52 +++++------ .../src/dependency_injection/containers.rs | 14 +-- .../src/http_server/routes/epoch_routes.rs | 51 ++++------ .../src/http_server/routes/middlewares.rs | 11 ++- .../src/http_server/routes/root_routes.rs | 69 ++++++++++---- mithril-aggregator/src/runtime/runner.rs | 47 ++++++---- .../src/runtime/state_machine.rs | 3 +- .../src/services/epoch_service.rs | 93 +++++++++++++------ .../test_extensions/aggregator_observer.rs | 18 ++-- .../src/entities/signed_entity_config.rs | 12 ++- 11 files changed, 247 insertions(+), 159 deletions(-) diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index 7940f97c04c..38ea004eb37 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -5,7 +5,7 @@ use mithril_common::crypto_helper::ProtocolGenesisSigner; use mithril_common::era::adapters::EraReaderAdapterType; use mithril_doc::{Documenter, DocumenterDefault, StructDoc}; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{BTreeSet, HashMap}; use std::path::PathBuf; use std::str::FromStr; @@ -289,23 +289,22 @@ impl Configuration { .map(|limit| if limit > 3 { limit as u64 } else { 3 }) } - // TODO: Maybe remove this function, signing configuration in SignedEntityConfig is stored in DB - /// Compute a [SignedEntityConfig] based on this configuration. - pub fn compute_signed_entity_config(&self) -> StdResult { - let network = self.get_network()?; - let allowed_discriminants = self + /// Compute the list of signed entity discriminants that are allowed to be processed based in this configuration. + pub fn compute_allowed_signed_entity_types_discriminants( + &self, + ) -> StdResult> { + let mut allowed_discriminants = self .signed_entity_types .as_ref() .map(SignedEntityTypeDiscriminants::parse_list) .transpose() .with_context(|| "Invalid 'signed_entity_types' configuration")? .unwrap_or_default(); + SignedEntityConfig::append_allowed_signed_entity_types_discriminants( + &mut allowed_discriminants, + ); - Ok(SignedEntityConfig { - allowed_discriminants, - network, - cardano_transactions_signing_config: self.cardano_transactions_signing_config.clone(), - }) + Ok(allowed_discriminants) } } @@ -541,4 +540,19 @@ mod test { DefaultConfiguration::default().cardano_transactions_signing_config ); } + + #[test] + fn compute_allowed_signed_entity_types_discriminants_append_default_discriminants() { + let config = Configuration { + signed_entity_types: None, + ..Configuration::new_sample() + }; + + assert_eq!( + config + .compute_allowed_signed_entity_types_discriminants() + .unwrap(), + BTreeSet::from(SignedEntityConfig::DEFAULT_ALLOWED_DISCRIMINANTS) + ); + } } diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index ceb6d22d7d0..d06715e284d 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -1,7 +1,7 @@ use anyhow::Context; use semver::Version; use slog::Logger; -use std::sync::Arc; +use std::{collections::BTreeSet, sync::Arc}; use tokio::{ sync::{ mpsc::{UnboundedReceiver, UnboundedSender}, @@ -29,10 +29,7 @@ use mithril_common::{ CardanoImmutableDigester, DumbImmutableFileObserver, ImmutableDigester, ImmutableFileObserver, ImmutableFileSystemObserver, }, - entities::{ - CertificatePending, CompressionAlgorithm, Epoch, SignedEntityConfig, - SignedEntityTypeDiscriminants, - }, + entities::{CertificatePending, CompressionAlgorithm, Epoch, SignedEntityTypeDiscriminants}, era::{ adapters::{EraReaderAdapterBuilder, EraReaderDummyAdapter}, EraChecker, EraMarker, EraReader, EraReaderAdapter, SupportedEra, @@ -101,8 +98,8 @@ pub struct DependenciesBuilder { /// Configuration parameters pub configuration: Configuration, - /// Signed entity configuration - pub signed_entity_config: Option, + /// List of signed entity discriminants that are allowed to be processed + pub allowed_discriminants: Option>, /// SQLite database connection pub sqlite_connection: Option>, @@ -246,7 +243,7 @@ impl DependenciesBuilder { pub fn new(configuration: Configuration) -> Self { Self { configuration, - signed_entity_config: None, + allowed_discriminants: None, sqlite_connection: None, sqlite_connection_cardano_transaction_pool: None, stake_store: None, @@ -294,13 +291,18 @@ impl DependenciesBuilder { } } - /// Get the signed entity configuration - pub fn get_signed_entity_config(&mut self) -> Result { - if self.signed_entity_config.is_none() { - self.signed_entity_config = Some(self.configuration.compute_signed_entity_config()?); + /// Get the allowed signed entity types discriminants + pub fn get_allowed_signed_entity_types_discriminants( + &mut self, + ) -> Result> { + if self.allowed_discriminants.is_none() { + self.allowed_discriminants = Some( + self.configuration + .compute_allowed_signed_entity_types_discriminants()?, + ); } - Ok(self.signed_entity_config.clone().unwrap()) + Ok(self.allowed_discriminants.clone().unwrap()) } fn build_sqlite_connection( @@ -584,8 +586,9 @@ impl DependenciesBuilder { // Temporary fix, should be removed // Replace empty JSON values '{}' injected with Migration #28 let cardano_signing_config = self - .get_signed_entity_config()? - .cardano_transactions_signing_config; + .configuration + .cardano_transactions_signing_config + .clone(); #[allow(deprecated)] epoch_settings_store .replace_cardano_signing_config_empty_values(cardano_signing_config)?; @@ -1222,9 +1225,7 @@ impl DependenciesBuilder { let epoch_settings_storer = self.get_epoch_settings_storer().await?; let epoch_settings = self.get_epoch_settings_configuration()?; let network = self.configuration.get_network()?; - let allowed_discriminants = self - .get_signed_entity_config()? - .list_allowed_signed_entity_types_discriminants(); + let allowed_discriminants = self.get_allowed_signed_entity_types_discriminants()?; let epoch_service = Arc::new(RwLock::new(MithrilEpochService::new( epoch_settings, @@ -1338,8 +1339,9 @@ impl DependenciesBuilder { let epoch_settings = AggregatorEpochSettings { protocol_parameters: self.configuration.protocol_parameters.clone(), cardano_transactions_signing_config: self - .get_signed_entity_config()? - .cardano_transactions_signing_config, + .configuration + .cardano_transactions_signing_config + .clone(), }; Ok(epoch_settings) } @@ -1348,7 +1350,7 @@ impl DependenciesBuilder { pub async fn build_dependency_container(&mut self) -> Result { let dependency_manager = DependencyContainer { config: self.configuration.clone(), - signed_entity_config: self.get_signed_entity_config()?, + allowed_discriminants: self.get_allowed_signed_entity_types_discriminants()?, sqlite_connection: self.get_sqlite_connection().await?, sqlite_connection_cardano_transaction_pool: self .get_sqlite_connection_cardano_transaction_pool() @@ -1405,10 +1407,7 @@ impl DependenciesBuilder { pub async fn create_aggregator_runner(&mut self) -> Result { let dependency_container = Arc::new(self.build_dependency_container().await?); - let config = AggregatorConfig::new( - Duration::from_millis(self.configuration.run_interval), - self.get_signed_entity_config()?, - ); + let config = AggregatorConfig::new(Duration::from_millis(self.configuration.run_interval)); let runtime = AggregatorRuntime::new( config, None, @@ -1437,8 +1436,7 @@ impl DependenciesBuilder { &mut self, ) -> Result> { let activation = self - .get_signed_entity_config()? - .list_allowed_signed_entity_types_discriminants() + .get_allowed_signed_entity_types_discriminants()? .contains(&SignedEntityTypeDiscriminants::CardanoTransactions); let cardano_transactions_preloader = CardanoTransactionsPreloader::new( self.get_signed_entity_lock().await?, diff --git a/mithril-aggregator/src/dependency_injection/containers.rs b/mithril-aggregator/src/dependency_injection/containers.rs index 08b5227f2ca..c23ca6d58e2 100644 --- a/mithril-aggregator/src/dependency_injection/containers.rs +++ b/mithril-aggregator/src/dependency_injection/containers.rs @@ -1,5 +1,4 @@ -use mithril_persistence::sqlite::SqliteConnectionPool; -use std::sync::Arc; +use std::{collections::BTreeSet, sync::Arc}; use tokio::sync::RwLock; use mithril_common::{ @@ -10,7 +9,7 @@ use mithril_common::{ crypto_helper::ProtocolGenesisVerifier, digesters::{ImmutableDigester, ImmutableFileObserver}, entities::{ - CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityConfig, + CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, SignedEntityTypeDiscriminants, SignerWithStake, StakeDistribution, }, era::{EraChecker, EraReader}, @@ -19,7 +18,10 @@ use mithril_common::{ test_utils::MithrilFixture, TickerService, }; -use mithril_persistence::{sqlite::SqliteConnection, store::StakeStorer}; +use mithril_persistence::{ + sqlite::{SqliteConnection, SqliteConnectionPool}, + store::StakeStorer, +}; use crate::{ configuration::*, @@ -48,8 +50,8 @@ pub struct DependencyContainer { /// Configuration structure. pub config: Configuration, - /// Signed entity configuration. - pub signed_entity_config: SignedEntityConfig, + /// List of signed entity discriminants that are allowed to be processed + pub allowed_discriminants: BTreeSet, /// SQLite database connection /// diff --git a/mithril-aggregator/src/http_server/routes/epoch_routes.rs b/mithril-aggregator/src/http_server/routes/epoch_routes.rs index 38f6a0010c4..3ec8f60e904 100644 --- a/mithril-aggregator/src/http_server/routes/epoch_routes.rs +++ b/mithril-aggregator/src/http_server/routes/epoch_routes.rs @@ -1,8 +1,8 @@ -use std::sync::Arc; +use std::{collections::BTreeSet, sync::Arc}; use warp::Filter; use mithril_common::{ - entities::{SignedEntityConfig, SignedEntityTypeDiscriminants}, + entities::SignedEntityTypeDiscriminants, messages::{EpochSettingsMessage, SignerMessagePart}, StdResult, }; @@ -24,13 +24,13 @@ fn epoch_settings( warp::path!("epoch-settings") .and(warp::get()) .and(middlewares::with_epoch_service(dependency_manager.clone())) - .and(middlewares::with_signed_entity_config(dependency_manager)) + .and(middlewares::with_allowed_discriminants(dependency_manager)) .and_then(handlers::epoch_settings) } async fn get_epoch_settings_message( epoch_service: EpochServiceWrapper, - signed_entity_config: SignedEntityConfig, + allowed_discriminants: BTreeSet, ) -> StdResult { let epoch_service = epoch_service.read().await; @@ -40,9 +40,8 @@ async fn get_epoch_settings_message( let current_signers = epoch_service.current_signers()?; let next_signers = epoch_service.next_signers()?; - let allowed_types = signed_entity_config.list_allowed_signed_entity_types_discriminants(); let cardano_transactions_discriminant = - allowed_types.get(&SignedEntityTypeDiscriminants::CardanoTransactions); + allowed_discriminants.get(&SignedEntityTypeDiscriminants::CardanoTransactions); let cardano_transactions_signing_config = cardano_transactions_discriminant .map(|_| epoch_service.current_cardano_transactions_signing_config()) @@ -68,10 +67,11 @@ async fn get_epoch_settings_message( mod handlers { use slog_scope::debug; + use std::collections::BTreeSet; use std::convert::Infallible; use warp::http::StatusCode; - use mithril_common::entities::SignedEntityConfig; + use mithril_common::entities::SignedEntityTypeDiscriminants; use crate::dependency_injection::EpochServiceWrapper; use crate::http_server::routes::epoch_routes::get_epoch_settings_message; @@ -80,11 +80,11 @@ mod handlers { /// Epoch Settings pub async fn epoch_settings( epoch_service: EpochServiceWrapper, - signed_entity_config: SignedEntityConfig, + allowed_discriminants: BTreeSet, ) -> Result { debug!("⇄ HTTP SERVER: epoch_settings"); let epoch_settings_message = - get_epoch_settings_message(epoch_service, signed_entity_config).await; + get_epoch_settings_message(epoch_service, allowed_discriminants).await; match epoch_settings_message { Ok(message) => Ok(reply::json(&message, StatusCode::OK)), @@ -135,20 +135,10 @@ mod tests { let fixture = MithrilFixtureBuilder::default().with_signers(3).build(); let epoch_service = FakeEpochService::from_fixture(Epoch(4), &fixture); let epoch_service = Arc::new(RwLock::new(epoch_service)); + let allowed_discriminants = + BTreeSet::from([SignedEntityTypeDiscriminants::CardanoTransactions]); - let cardano_transactions_signing_config = CardanoTransactionsSigningConfig { - security_parameter: BlockNumber(70), - step: BlockNumber(15), - }; - let signed_entity_config = SignedEntityConfig { - cardano_transactions_signing_config: cardano_transactions_signing_config.clone(), - allowed_discriminants: BTreeSet::from([ - SignedEntityTypeDiscriminants::CardanoTransactions, - ]), - ..SignedEntityConfig::dummy() - }; - - let message = get_epoch_settings_message(epoch_service, signed_entity_config) + let message = get_epoch_settings_message(epoch_service, allowed_discriminants) .await .unwrap(); @@ -162,17 +152,8 @@ mod tests { let epoch_service = FakeEpochService::from_fixture(Epoch(4), &fixture); let epoch_service = Arc::new(RwLock::new(epoch_service)); - let cardano_transactions_signing_config = CardanoTransactionsSigningConfig { - security_parameter: BlockNumber(70), - step: BlockNumber(15), - }; - let signed_entity_config = SignedEntityConfig { - cardano_transactions_signing_config, - allowed_discriminants: BTreeSet::new(), - ..SignedEntityConfig::dummy() - }; - - let message = get_epoch_settings_message(epoch_service, signed_entity_config) + let allowed_discriminants = BTreeSet::new(); + let message = get_epoch_settings_message(epoch_service, allowed_discriminants) .await .unwrap(); @@ -207,7 +188,7 @@ mod tests { let message = get_epoch_settings_message( Arc::new(RwLock::new(epoch_service)), - SignedEntityConfig::dummy(), + SignedEntityTypeDiscriminants::all(), ) .await .unwrap(); @@ -251,7 +232,7 @@ mod tests { let message = get_epoch_settings_message( Arc::new(RwLock::new(epoch_service)), - SignedEntityConfig::dummy(), + SignedEntityTypeDiscriminants::all(), ) .await .unwrap(); diff --git a/mithril-aggregator/src/http_server/routes/middlewares.rs b/mithril-aggregator/src/http_server/routes/middlewares.rs index 89d0f842b70..5cce8484d5f 100644 --- a/mithril-aggregator/src/http_server/routes/middlewares.rs +++ b/mithril-aggregator/src/http_server/routes/middlewares.rs @@ -1,10 +1,11 @@ +use std::collections::BTreeSet; use std::convert::Infallible; use std::sync::Arc; use warp::Filter; use mithril_common::api_version::APIVersionProvider; -use mithril_common::entities::SignedEntityConfig; +use mithril_common::entities::SignedEntityTypeDiscriminants; use crate::database::repository::SignerGetter; use crate::dependency_injection::EpochServiceWrapper; @@ -43,11 +44,11 @@ pub fn with_config( warp::any().map(move || dependency_manager.config.clone()) } -/// With signed entity config middleware -pub fn with_signed_entity_config( +/// With allowed discriminants config middleware +pub fn with_allowed_discriminants( dependency_manager: Arc, -) -> impl Filter + Clone { - warp::any().map(move || dependency_manager.signed_entity_config.clone()) +) -> impl Filter,), Error = Infallible> + Clone { + warp::any().map(move || dependency_manager.allowed_discriminants.clone()) } /// With Event transmitter middleware diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index 94333bef1fc..dd38bbbe7a1 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -18,7 +18,7 @@ fn root( .and(middlewares::with_api_version_provider( dependency_manager.clone(), )) - .and(middlewares::with_signed_entity_config( + .and(middlewares::with_allowed_discriminants( dependency_manager.clone(), )) .and(middlewares::with_config(dependency_manager)) @@ -26,13 +26,14 @@ fn root( } mod handlers { + use std::collections::BTreeSet; use std::{convert::Infallible, sync::Arc}; use slog_scope::{debug, warn}; use warp::http::StatusCode; use mithril_common::api_version::APIVersionProvider; - use mithril_common::entities::{SignedEntityConfig, SignedEntityTypeDiscriminants}; + use mithril_common::entities::SignedEntityTypeDiscriminants; use mithril_common::messages::{ AggregatorCapabilities, AggregatorFeaturesMessage, CardanoTransactionsProverCapabilities, }; @@ -43,7 +44,7 @@ mod handlers { /// Root pub async fn root( api_version_provider: Arc, - signed_entity_config: SignedEntityConfig, + allowed_discriminants: BTreeSet, configuration: Configuration, ) -> Result { debug!("⇄ HTTP SERVER: root"); @@ -54,8 +55,7 @@ mod handlers { ); let mut capabilities = AggregatorCapabilities { - signed_entity_types: signed_entity_config - .list_allowed_signed_entity_types_discriminants(), + signed_entity_types: allowed_discriminants, cardano_transactions_prover: None, cardano_transactions_signing_config: None, }; @@ -87,8 +87,9 @@ mod handlers { #[cfg(test)] mod tests { + use crate::dependency_injection::DependenciesBuilder; use crate::http_server::SERVER_BASE_PATH; - use crate::{initialize_dependencies, DependencyContainer}; + use crate::{Configuration, DependencyContainer}; use mithril_common::entities::{ BlockNumber, CardanoTransactionsSigningConfig, SignedEntityTypeDiscriminants, }; @@ -123,14 +124,31 @@ mod tests { async fn test_root_route_ok() { let method = Method::GET.as_str(); let path = "/"; - let mut dependency_manager = initialize_dependencies().await; - dependency_manager - .signed_entity_config - .allowed_discriminants = BTreeSet::from([ - SignedEntityTypeDiscriminants::MithrilStakeDistribution, - SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, - SignedEntityTypeDiscriminants::CardanoStakeDistribution, - ]); + let config = Configuration { + signed_entity_types: Some( + BTreeSet::from([ + SignedEntityTypeDiscriminants::CardanoStakeDistribution, + SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, + SignedEntityTypeDiscriminants::MithrilStakeDistribution, + ]) + .iter() + .map(SignedEntityTypeDiscriminants::to_string) + .collect::>() + .join(","), + ), + ..Configuration::new_sample() + }; + let mut builder = DependenciesBuilder::new(config); + let dependency_manager = builder.build_dependency_container().await.unwrap(); + + // let mut dependency_manager = initialize_dependencies().await; + // dependency_manager + // .signed_entity_config + // .allowed_discriminants = BTreeSet::from([ + // SignedEntityTypeDiscriminants::MithrilStakeDistribution, + // SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, + // SignedEntityTypeDiscriminants::CardanoStakeDistribution, + // ]); let expected_open_api_version = dependency_manager .api_version_provider .clone() @@ -182,11 +200,24 @@ mod tests { async fn test_root_route_ok_with_cardano_transactions_enabled() { let method = Method::GET.as_str(); let path = "/"; - let mut dependency_manager = initialize_dependencies().await; - dependency_manager - .signed_entity_config - .allowed_discriminants = - BTreeSet::from([SignedEntityTypeDiscriminants::CardanoTransactions]); + let config = Configuration { + signed_entity_types: Some( + BTreeSet::from([SignedEntityTypeDiscriminants::CardanoTransactions]) + .iter() + .map(SignedEntityTypeDiscriminants::to_string) + .collect::>() + .join(","), + ), + ..Configuration::new_sample() + }; + let mut builder = DependenciesBuilder::new(config); + let mut dependency_manager = builder.build_dependency_container().await.unwrap(); + + // let mut dependency_manager = initialize_dependencies().await; + // dependency_manager + // .signed_entity_config + // .allowed_discriminants = + // BTreeSet::from([SignedEntityTypeDiscriminants::CardanoTransactions]); dependency_manager .config .cardano_transactions_prover_max_hashes_allowed_by_request = 99; diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index 5f765023c5c..ed3f4fd8012 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -5,8 +5,7 @@ use std::sync::Arc; use std::time::Duration; use mithril_common::entities::{ - Certificate, CertificatePending, Epoch, ProtocolMessage, SignedEntityConfig, SignedEntityType, - Signer, TimePoint, + Certificate, CertificatePending, Epoch, ProtocolMessage, SignedEntityType, Signer, TimePoint, }; use mithril_common::StdResult; use mithril_persistence::store::StakeStorer; @@ -22,18 +21,12 @@ use mockall::automock; pub struct AggregatorConfig { /// Interval between each snapshot, in ms pub interval: Duration, - - /// Signed entity configuration. - pub signed_entity_config: SignedEntityConfig, } impl AggregatorConfig { /// Create a new instance of AggregatorConfig. - pub fn new(interval: Duration, signed_entity_config: SignedEntityConfig) -> Self { - Self { - interval, - signed_entity_config, - } + pub fn new(interval: Duration) -> Self { + Self { interval } } } @@ -157,7 +150,10 @@ impl AggregatorRunner { ) -> StdResult> { let signed_entity_types = self .dependencies - .signed_entity_config + .epoch_service + .read() + .await + .signed_entity_config()? .list_allowed_signed_entity_types(time_point)?; let unlocked_signed_entities = self .dependencies @@ -528,7 +524,7 @@ impl AggregatorRunnerTrait for AggregatorRunner { #[cfg(test)] pub mod tests { use crate::entities::AggregatorEpochSettings; - use crate::services::{FakeEpochService, MockUpkeepService}; + use crate::services::{FakeEpochService, FakeEpochServiceData, MockUpkeepService}; use crate::{ entities::OpenMessage, initialize_dependencies, @@ -977,9 +973,13 @@ pub mod tests { let expected_epoch_settings = AggregatorEpochSettings { protocol_parameters: deps.config.protocol_parameters.clone(), cardano_transactions_signing_config: deps - .signed_entity_config + .config .cardano_transactions_signing_config .clone(), + // cardano_transactions_signing_config: deps + // .signed_entity_config + // .cardano_transactions_signing_config + // .clone(), }; let current_epoch = deps.ticker_service.get_current_epoch().await.unwrap(); let insert_epoch = current_epoch.offset_to_epoch_settings_recording_epoch(); @@ -1224,8 +1224,14 @@ pub mod tests { async fn list_available_signed_entity_types_list_all_configured_entities_if_none_are_locked() { let runner = { let mut dependencies = initialize_dependencies().await; - dependencies.signed_entity_config.allowed_discriminants = - SignedEntityTypeDiscriminants::all(); + let epoch_service = FakeEpochService::new(FakeEpochServiceData { + signed_entity_config: SignedEntityConfig { + allowed_discriminants: SignedEntityTypeDiscriminants::all(), + ..SignedEntityConfig::dummy() + }, + ..FakeEpochServiceData::dummy(Epoch(32)) + }); + dependencies.epoch_service = Arc::new(RwLock::new(epoch_service)); dependencies.signed_entity_type_lock = Arc::new(SignedEntityTypeLock::default()); AggregatorRunner::new(Arc::new(dependencies)) }; @@ -1252,9 +1258,16 @@ pub mod tests { let signed_entity_type_lock = Arc::new(SignedEntityTypeLock::default()); let runner = { let mut dependencies = initialize_dependencies().await; - dependencies.signed_entity_config.allowed_discriminants = - SignedEntityTypeDiscriminants::all(); dependencies.signed_entity_type_lock = signed_entity_type_lock.clone(); + let epoch_service = FakeEpochService::new(FakeEpochServiceData { + signed_entity_config: SignedEntityConfig { + allowed_discriminants: SignedEntityTypeDiscriminants::all(), + ..SignedEntityConfig::dummy() + }, + ..FakeEpochServiceData::dummy(Epoch(32)) + }); + dependencies.epoch_service = Arc::new(RwLock::new(epoch_service)); + AggregatorRunner::new(Arc::new(dependencies)) }; diff --git a/mithril-aggregator/src/runtime/state_machine.rs b/mithril-aggregator/src/runtime/state_machine.rs index ebbaa82f0e8..b4b09e2fc09 100644 --- a/mithril-aggregator/src/runtime/state_machine.rs +++ b/mithril-aggregator/src/runtime/state_machine.rs @@ -396,7 +396,6 @@ mod tests { use mockall::predicate; use std::time::Duration; - use mithril_common::entities::SignedEntityConfig; use mithril_common::test_utils::fake_data; use super::super::runner::MockAggregatorRunner; @@ -407,7 +406,7 @@ mod tests { runner: MockAggregatorRunner, ) -> AggregatorRuntime { AggregatorRuntime::new( - AggregatorConfig::new(Duration::from_millis(20), SignedEntityConfig::dummy()), + AggregatorConfig::new(Duration::from_millis(20)), init_state, Arc::new(runner), ) diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index a1a824ebdef..d7c8a33aa66 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -398,50 +398,67 @@ pub struct FakeEpochService { precompute_epoch_data_error: bool, } +#[cfg(test)] +pub struct FakeEpochServiceData { + pub epoch: Epoch, + pub epoch_settings: AggregatorEpochSettings, + pub next_epoch_settings: AggregatorEpochSettings, + pub upcoming_epoch_settings: AggregatorEpochSettings, + pub current_signers_with_stake: Vec, + pub next_signers_with_stake: Vec, + pub signed_entity_config: SignedEntityConfig, +} + +#[cfg(test)] +impl FakeEpochServiceData { + pub fn dummy(epoch: Epoch) -> Self { + use mithril_common::test_utils::fake_data; + let signers = fake_data::signers_with_stakes(3); + + Self { + epoch, + epoch_settings: AggregatorEpochSettings::dummy(), + next_epoch_settings: AggregatorEpochSettings::dummy(), + upcoming_epoch_settings: AggregatorEpochSettings::dummy(), + current_signers_with_stake: signers.clone(), + next_signers_with_stake: signers, + signed_entity_config: SignedEntityConfig::dummy(), + } + } +} + #[cfg(test)] impl FakeEpochService { - /// Note: protocol multi signers and current/next avk will be computed using the given protocol - /// parameters and signers. - pub fn with_data( - epoch: Epoch, - epoch_settings: &AggregatorEpochSettings, - next_epoch_settings: &AggregatorEpochSettings, - upcoming_epoch_settings: &AggregatorEpochSettings, - current_signers_with_stake: &[SignerWithStake], - next_signers_with_stake: &[SignerWithStake], - signed_entity_config: SignedEntityConfig, - ) -> Self { + pub fn new(data: FakeEpochServiceData) -> Self { + let current_signers = Signer::vec_from(data.current_signers_with_stake.clone()); + let next_signers = Signer::vec_from(data.next_signers_with_stake.clone()); + let protocol_multi_signer = SignerBuilder::new( - current_signers_with_stake, - &epoch_settings.protocol_parameters, + &data.current_signers_with_stake, + &data.epoch_settings.protocol_parameters, ) .with_context(|| "Could not build protocol_multi_signer for epoch service") .unwrap() .build_multi_signer(); let next_protocol_multi_signer = SignerBuilder::new( - next_signers_with_stake, - &next_epoch_settings.protocol_parameters, + &data.next_signers_with_stake, + &data.next_epoch_settings.protocol_parameters, ) .with_context(|| "Could not build protocol_multi_signer for epoch service") .unwrap() .build_multi_signer(); - let current_signers_with_stake = current_signers_with_stake.to_vec(); - let next_signers_with_stake = next_signers_with_stake.to_vec(); - let current_signers = Signer::vec_from(current_signers_with_stake.clone()); - let next_signers = Signer::vec_from(next_signers_with_stake.clone()); - Self { epoch_data: Some(EpochData { - epoch, - epoch_settings: epoch_settings.clone(), - next_epoch_settings: next_epoch_settings.clone(), - upcoming_epoch_settings: upcoming_epoch_settings.clone(), - current_signers_with_stake, - next_signers_with_stake, + epoch: data.epoch, + epoch_settings: data.epoch_settings, + next_epoch_settings: data.next_epoch_settings, + upcoming_epoch_settings: data.upcoming_epoch_settings, + current_signers_with_stake: data.current_signers_with_stake, + next_signers_with_stake: data.next_signers_with_stake, current_signers, next_signers, - signed_entity_config, + signed_entity_config: data.signed_entity_config, }), computed_epoch_data: Some(ComputedEpochData { aggregate_verification_key: protocol_multi_signer @@ -457,6 +474,28 @@ impl FakeEpochService { } } + /// Note: protocol multi signers and current/next avk will be computed using the given protocol + /// parameters and signers. + pub fn with_data( + epoch: Epoch, + epoch_settings: &AggregatorEpochSettings, + next_epoch_settings: &AggregatorEpochSettings, + upcoming_epoch_settings: &AggregatorEpochSettings, + current_signers_with_stake: &[SignerWithStake], + next_signers_with_stake: &[SignerWithStake], + signed_entity_config: SignedEntityConfig, + ) -> Self { + Self::new(FakeEpochServiceData { + epoch, + epoch_settings: epoch_settings.clone(), + next_epoch_settings: next_epoch_settings.clone(), + upcoming_epoch_settings: upcoming_epoch_settings.clone(), + current_signers_with_stake: current_signers_with_stake.to_vec(), + next_signers_with_stake: next_signers_with_stake.to_vec(), + signed_entity_config, + }) + } + pub fn from_fixture( epoch: Epoch, fixture: &mithril_common::test_utils::MithrilFixture, diff --git a/mithril-aggregator/tests/test_extensions/aggregator_observer.rs b/mithril-aggregator/tests/test_extensions/aggregator_observer.rs index b633f43286b..67dd62585be 100644 --- a/mithril-aggregator/tests/test_extensions/aggregator_observer.rs +++ b/mithril-aggregator/tests/test_extensions/aggregator_observer.rs @@ -1,13 +1,12 @@ use anyhow::{anyhow, Context}; -use mithril_aggregator::services::SignedEntityService; use mithril_aggregator::{ - dependency_injection::DependenciesBuilder, entities::OpenMessage, services::CertifierService, + dependency_injection::{DependenciesBuilder, EpochServiceWrapper}, + entities::OpenMessage, + services::{CertifierService, SignedEntityService}, }; use mithril_common::entities::{CardanoTransactionsSnapshot, Certificate, SignedEntity}; use mithril_common::{ - entities::{ - Epoch, SignedEntityConfig, SignedEntityType, SignedEntityTypeDiscriminants, TimePoint, - }, + entities::{Epoch, SignedEntityType, SignedEntityTypeDiscriminants, TimePoint}, CardanoNetwork, StdResult, TickerService, }; use std::sync::Arc; @@ -18,7 +17,7 @@ pub struct AggregatorObserver { certifier_service: Arc, signed_entity_service: Arc, ticker_service: Arc, - signed_entity_config: SignedEntityConfig, + epoch_service: EpochServiceWrapper, } impl AggregatorObserver { @@ -29,7 +28,7 @@ impl AggregatorObserver { certifier_service: deps_builder.get_certifier_service().await.unwrap(), signed_entity_service: deps_builder.get_signed_entity_service().await.unwrap(), ticker_service: deps_builder.get_ticker_service().await.unwrap(), - signed_entity_config: deps_builder.get_signed_entity_config().unwrap(), + epoch_service: deps_builder.get_epoch_service().await.unwrap(), } } @@ -72,7 +71,10 @@ impl AggregatorObserver { .await .with_context(|| "Querying the current beacon should not fail")?; - self.signed_entity_config + self.epoch_service + .read() + .await + .signed_entity_config()? .time_point_to_signed_entity(discriminant, &time_point) } diff --git a/mithril-common/src/entities/signed_entity_config.rs b/mithril-common/src/entities/signed_entity_config.rs index 635eeafd461..6ede65f07fe 100644 --- a/mithril-common/src/entities/signed_entity_config.rs +++ b/mithril-common/src/entities/signed_entity_config.rs @@ -40,6 +40,14 @@ impl SignedEntityConfig { SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, ]; + /// Append to the given list of allowed signed entity types discriminants the [Self::DEFAULT_ALLOWED_DISCRIMINANTS] + /// if not already present. + pub fn append_allowed_signed_entity_types_discriminants( + discriminants: &mut BTreeSet, + ) { + discriminants.append(&mut BTreeSet::from(Self::DEFAULT_ALLOWED_DISCRIMINANTS)); + } + /// Create the deduplicated list of allowed signed entity types discriminants. /// /// The list is the aggregation of [Self::DEFAULT_ALLOWED_DISCRIMINANTS] and @@ -47,8 +55,8 @@ impl SignedEntityConfig { pub fn list_allowed_signed_entity_types_discriminants( &self, ) -> BTreeSet { - let mut discriminants = BTreeSet::from(Self::DEFAULT_ALLOWED_DISCRIMINANTS); - discriminants.append(&mut self.allowed_discriminants.clone()); + let mut discriminants = self.allowed_discriminants.clone(); + Self::append_allowed_signed_entity_types_discriminants(&mut discriminants); discriminants } From 8f5ca53a7cf71f575ae07f5faa83a77c26afee6c Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 8 Oct 2024 14:54:42 +0200 Subject: [PATCH 07/15] Remove test function `build_service` in favor of `EpochServiceBuilder` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel Co-authored-by: DJO --- .../src/services/epoch_service.rs | 87 +++++-------------- 1 file changed, 20 insertions(+), 67 deletions(-) diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index d7c8a33aa66..4f4d41569a8 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -668,7 +668,6 @@ mod tests { use mithril_persistence::store::adapter::MemoryAdapter; use std::collections::{BTreeSet, HashMap}; - use crate::services::epoch_service::tests::ServiceBuilderParameters::WithFutureProtocolParameters; use crate::store::FakeEpochSettingsStorer; use crate::VerificationKeyStore; @@ -827,48 +826,6 @@ mod tests { } } - // TODO: Use EpochServiceBuilder and remove ServiceBuilderParameters - enum ServiceBuilderParameters { - DifferentFixtureForSecondEpoch(MithrilFixture), - UpcomingProtocolParameters(ProtocolParameters), - WithFutureProtocolParameters(ProtocolParameters), - } - - // TODO: Use EpochServiceBuilder and remove build_service - /// By default will copy data from the given fixture for all epochs, can be fined tuned - /// with the [ServiceBuilderParameters]. - async fn build_service( - epoch: Epoch, - current_epoch_fixture: &MithrilFixture, - additional_params: &[ServiceBuilderParameters], - ) -> MithrilEpochService { - let mut builder = EpochServiceBuilder::new(epoch, current_epoch_fixture.clone()); - for params in additional_params { - match params { - ServiceBuilderParameters::DifferentFixtureForSecondEpoch(fixture) => { - builder.stored_next_epoch_settings = AggregatorEpochSettings { - protocol_parameters: fixture.protocol_parameters(), - cardano_transactions_signing_config: - CardanoTransactionsSigningConfig::dummy(), - }; - builder.next_signers_with_stake = fixture.signers_with_stake().clone(); - } - ServiceBuilderParameters::UpcomingProtocolParameters(params) => { - builder.stored_upcoming_epoch_settings = AggregatorEpochSettings { - protocol_parameters: params.clone(), - cardano_transactions_signing_config: - CardanoTransactionsSigningConfig::dummy(), - }; - } - ServiceBuilderParameters::WithFutureProtocolParameters(params) => { - builder.future_protocol_parameters = params.clone(); - } - } - } - - builder.build() - } - #[tokio::test] async fn inform_epoch_get_data_from_its_dependencies() { let current_epoch_fixture = MithrilFixtureBuilder::default().with_signers(3).build(); @@ -941,7 +898,7 @@ mod tests { SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, ]); - let builder = EpochServiceBuilder { + let mut service = EpochServiceBuilder { network, allowed_discriminants: allowed_discriminants.clone(), stored_epoch_settings: AggregatorEpochSettings { @@ -949,9 +906,8 @@ mod tests { ..AggregatorEpochSettings::dummy() }, ..EpochServiceBuilder::new(epoch, MithrilFixtureBuilder::default().build()) - }; - - let mut service = builder.build(); + } + .build(); service .inform_epoch(epoch) @@ -981,14 +937,15 @@ mod tests { .build(); let epoch = Epoch(5); - let mut service = build_service( - epoch, - ¤t_epoch_fixture, - &[ServiceBuilderParameters::DifferentFixtureForSecondEpoch( - next_epoch_fixture.clone(), - )], - ) - .await; + let mut service = EpochServiceBuilder { + stored_next_epoch_settings: AggregatorEpochSettings { + protocol_parameters: next_epoch_fixture.protocol_parameters(), + cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), + }, + next_signers_with_stake: next_epoch_fixture.signers_with_stake().clone(), + ..EpochServiceBuilder::new(epoch, current_epoch_fixture.clone()) + } + .build(); service .inform_epoch(epoch) @@ -1017,7 +974,7 @@ mod tests { let fixture = MithrilFixtureBuilder::default().with_signers(3).build(); let avk = fixture.compute_avk(); let epoch = Epoch(4); - let mut service = build_service(epoch, &fixture, &[]).await; + let mut service = EpochServiceBuilder::new(epoch, fixture.clone()).build(); let signer_builder = SignerBuilder::new( &fixture.signers_with_stake(), &fixture.protocol_parameters(), @@ -1040,17 +997,13 @@ mod tests { #[tokio::test] async fn update_epoch_settings_insert_future_epoch_settings_in_the_store() { - let fixture = MithrilFixtureBuilder::default().with_signers(3).build(); let future_protocol_parameters = ProtocolParameters::new(6, 89, 0.124); let epoch = Epoch(4); - let mut service = build_service( - epoch, - &fixture, - &[WithFutureProtocolParameters( - future_protocol_parameters.clone(), - )], - ) - .await; + let mut service = EpochServiceBuilder { + future_protocol_parameters: future_protocol_parameters.clone(), + ..EpochServiceBuilder::new(epoch, MithrilFixtureBuilder::default().build()) + } + .build(); service .inform_epoch(epoch) @@ -1082,7 +1035,7 @@ mod tests { #[tokio::test] async fn cant_get_data_if_inform_epoch_has_not_been_called() { let fixture = MithrilFixtureBuilder::default().with_signers(3).build(); - let service = build_service(Epoch(4), &fixture, &[]).await; + let service = EpochServiceBuilder::new(Epoch(4), fixture.clone()).build(); for (name, res) in [ ( @@ -1151,7 +1104,7 @@ mod tests { async fn can_only_get_non_computed_data_if_inform_epoch_has_been_called_but_not_precompute_epoch_data( ) { let fixture = MithrilFixtureBuilder::default().with_signers(3).build(); - let mut service = build_service(Epoch(4), &fixture, &[]).await; + let mut service = EpochServiceBuilder::new(Epoch(4), fixture.clone()).build(); service.inform_epoch(Epoch(4)).await.unwrap(); assert!(service.epoch_of_current_data().is_ok()); From 26be0d640575317b9c5eb9aeafd1b086db4e9a4f Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Tue, 8 Oct 2024 15:54:34 +0200 Subject: [PATCH 08/15] Refactor epoch service test tools --- .../mithril_stake_distribution.rs | 22 +++-- mithril-aggregator/src/configuration.rs | 2 +- .../src/http_server/routes/epoch_routes.rs | 40 ++++----- .../src/http_server/routes/root_routes.rs | 14 ---- mithril-aggregator/src/multi_signer.rs | 24 +++--- mithril-aggregator/src/runtime/runner.rs | 39 +++++---- .../src/services/epoch_service.rs | 81 +++++++------------ .../signable_builder/signable_seed_builder.rs | 42 +++++----- 8 files changed, 112 insertions(+), 152 deletions(-) diff --git a/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs b/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs index 081396005f6..5ab3bb5fdeb 100644 --- a/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs +++ b/mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs @@ -38,15 +38,13 @@ impl ArtifactBuilder for MithrilStakeDistributi #[cfg(test)] mod tests { - use mithril_common::{ - crypto_helper::ProtocolParameters, entities::SignedEntityConfig, test_utils::fake_data, - }; + use mithril_common::{crypto_helper::ProtocolParameters, test_utils::fake_data}; use std::sync::Arc; use tokio::sync::RwLock; use super::*; - use crate::{entities::AggregatorEpochSettings, services::FakeEpochService}; + use crate::{entities::AggregatorEpochSettings, services::FakeEpochServiceBuilder}; #[tokio::test] async fn should_compute_valid_artifact() { @@ -56,15 +54,13 @@ mod tests { protocol_parameters: fake_data::protocol_parameters(), ..AggregatorEpochSettings::dummy() }; - let epoch_service = FakeEpochService::with_data( - Epoch(1), - &epoch_settings, - &epoch_settings, - &epoch_settings, - &signers_with_stake, - &signers_with_stake, - SignedEntityConfig::dummy(), - ); + let epoch_service = FakeEpochServiceBuilder { + epoch_settings: epoch_settings.clone(), + current_signers_with_stake: signers_with_stake.clone(), + next_signers_with_stake: signers_with_stake.clone(), + ..FakeEpochServiceBuilder::dummy(Epoch(1)) + } + .build(); let mithril_stake_distribution_artifact_builder = MithrilStakeDistributionArtifactBuilder::new(Arc::new(RwLock::new(epoch_service))); let artifact = mithril_stake_distribution_artifact_builder diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index 38ea004eb37..af4c00b3b37 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -289,7 +289,7 @@ impl Configuration { .map(|limit| if limit > 3 { limit as u64 } else { 3 }) } - /// Compute the list of signed entity discriminants that are allowed to be processed based in this configuration. + /// Compute the list of signed entity discriminants that are allowed to be processed based on this configuration. pub fn compute_allowed_signed_entity_types_discriminants( &self, ) -> StdResult> { diff --git a/mithril-aggregator/src/http_server/routes/epoch_routes.rs b/mithril-aggregator/src/http_server/routes/epoch_routes.rs index 3ec8f60e904..a6faaf5e98d 100644 --- a/mithril-aggregator/src/http_server/routes/epoch_routes.rs +++ b/mithril-aggregator/src/http_server/routes/epoch_routes.rs @@ -106,14 +106,14 @@ mod tests { use mithril_common::{ entities::{ BlockNumber, CardanoTransactionsSigningConfig, Epoch, ProtocolParameters, - SignedEntityConfig, SignedEntityTypeDiscriminants, + SignedEntityTypeDiscriminants, }, test_utils::{apispec::APISpec, fake_data, MithrilFixtureBuilder}, }; - use crate::initialize_dependencies; use crate::services::FakeEpochService; use crate::{entities::AggregatorEpochSettings, http_server::SERVER_BASE_PATH}; + use crate::{initialize_dependencies, services::FakeEpochServiceBuilder}; use super::*; @@ -176,15 +176,15 @@ mod tests { ..AggregatorEpochSettings::dummy() }; - let epoch_service = FakeEpochService::with_data( - Epoch(1), - ¤t_epoch_settings, - &next_epoch_settings, - &upcoming_epoch_settings, - &fake_data::signers_with_stakes(5), - &fake_data::signers_with_stakes(3), - SignedEntityConfig::dummy(), - ); + let epoch_service = FakeEpochServiceBuilder { + epoch_settings: current_epoch_settings, + next_epoch_settings: next_epoch_settings.clone(), + upcoming_epoch_settings: upcoming_epoch_settings.clone(), + current_signers_with_stake: fake_data::signers_with_stakes(5), + next_signers_with_stake: fake_data::signers_with_stakes(3), + ..FakeEpochServiceBuilder::dummy(Epoch(1)) + } + .build(); let message = get_epoch_settings_message( Arc::new(RwLock::new(epoch_service)), @@ -220,15 +220,15 @@ mod tests { ..AggregatorEpochSettings::dummy() }; - let epoch_service = FakeEpochService::with_data( - Epoch(1), - ¤t_epoch_settings, - &next_epoch_settings, - &AggregatorEpochSettings::dummy(), - &fake_data::signers_with_stakes(5), - &fake_data::signers_with_stakes(3), - SignedEntityConfig::dummy(), - ); + let epoch_service = FakeEpochServiceBuilder { + epoch_settings: current_epoch_settings.clone(), + next_epoch_settings: next_epoch_settings.clone(), + upcoming_epoch_settings: AggregatorEpochSettings::dummy(), + current_signers_with_stake: fake_data::signers_with_stakes(5), + next_signers_with_stake: fake_data::signers_with_stakes(3), + ..FakeEpochServiceBuilder::dummy(Epoch(1)) + } + .build(); let message = get_epoch_settings_message( Arc::new(RwLock::new(epoch_service)), diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index dd38bbbe7a1..4f029615b18 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -141,14 +141,6 @@ mod tests { let mut builder = DependenciesBuilder::new(config); let dependency_manager = builder.build_dependency_container().await.unwrap(); - // let mut dependency_manager = initialize_dependencies().await; - // dependency_manager - // .signed_entity_config - // .allowed_discriminants = BTreeSet::from([ - // SignedEntityTypeDiscriminants::MithrilStakeDistribution, - // SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, - // SignedEntityTypeDiscriminants::CardanoStakeDistribution, - // ]); let expected_open_api_version = dependency_manager .api_version_provider .clone() @@ -212,12 +204,6 @@ mod tests { }; let mut builder = DependenciesBuilder::new(config); let mut dependency_manager = builder.build_dependency_container().await.unwrap(); - - // let mut dependency_manager = initialize_dependencies().await; - // dependency_manager - // .signed_entity_config - // .allowed_discriminants = - // BTreeSet::from([SignedEntityTypeDiscriminants::CardanoTransactions]); dependency_manager .config .cardano_transactions_prover_max_hashes_allowed_by_request = 99; diff --git a/mithril-aggregator/src/multi_signer.rs b/mithril-aggregator/src/multi_signer.rs index bade2b3e7a2..5f7baaa9bd3 100644 --- a/mithril-aggregator/src/multi_signer.rs +++ b/mithril-aggregator/src/multi_signer.rs @@ -134,7 +134,6 @@ impl MultiSigner for MultiSignerImpl { #[cfg(test)] mod tests { - use entities::SignedEntityConfig; use std::sync::Arc; use tokio::sync::RwLock; @@ -144,7 +143,7 @@ mod tests { use mithril_common::test_utils::{fake_data, MithrilFixtureBuilder}; use crate::entities::AggregatorEpochSettings; - use crate::services::FakeEpochService; + use crate::services::{FakeEpochService, FakeEpochServiceBuilder}; use super::*; @@ -173,25 +172,26 @@ mod tests { let epoch = Epoch(5); let fixture = MithrilFixtureBuilder::default().with_signers(5).build(); let next_fixture = MithrilFixtureBuilder::default().with_signers(4).build(); - let multi_signer = - MultiSignerImpl::new(Arc::new(RwLock::new(FakeEpochService::with_data( - epoch, - &AggregatorEpochSettings { + let multi_signer = MultiSignerImpl::new(Arc::new(RwLock::new( + FakeEpochServiceBuilder { + epoch_settings: AggregatorEpochSettings { protocol_parameters: fixture.protocol_parameters(), ..AggregatorEpochSettings::dummy() }, - &AggregatorEpochSettings { + next_epoch_settings: AggregatorEpochSettings { protocol_parameters: next_fixture.protocol_parameters(), ..AggregatorEpochSettings::dummy() }, - &AggregatorEpochSettings { + upcoming_epoch_settings: AggregatorEpochSettings { protocol_parameters: next_fixture.protocol_parameters(), ..AggregatorEpochSettings::dummy() }, - &fixture.signers_with_stake(), - &next_fixture.signers_with_stake(), - SignedEntityConfig::dummy(), - )))); + current_signers_with_stake: fixture.signers_with_stake(), + next_signers_with_stake: next_fixture.signers_with_stake(), + ..FakeEpochServiceBuilder::dummy(epoch) + } + .build(), + ))); { let message = setup_message(); diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index ed3f4fd8012..df5ad55a320 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -524,7 +524,7 @@ impl AggregatorRunnerTrait for AggregatorRunner { #[cfg(test)] pub mod tests { use crate::entities::AggregatorEpochSettings; - use crate::services::{FakeEpochService, FakeEpochServiceData, MockUpkeepService}; + use crate::services::{FakeEpochService, FakeEpochServiceBuilder, MockUpkeepService}; use crate::{ entities::OpenMessage, initialize_dependencies, @@ -976,10 +976,6 @@ pub mod tests { .config .cardano_transactions_signing_config .clone(), - // cardano_transactions_signing_config: deps - // .signed_entity_config - // .cardano_transactions_signing_config - // .clone(), }; let current_epoch = deps.ticker_service.get_current_epoch().await.unwrap(); let insert_epoch = current_epoch.offset_to_epoch_settings_recording_epoch(); @@ -1224,13 +1220,14 @@ pub mod tests { async fn list_available_signed_entity_types_list_all_configured_entities_if_none_are_locked() { let runner = { let mut dependencies = initialize_dependencies().await; - let epoch_service = FakeEpochService::new(FakeEpochServiceData { + let epoch_service = FakeEpochServiceBuilder { signed_entity_config: SignedEntityConfig { allowed_discriminants: SignedEntityTypeDiscriminants::all(), ..SignedEntityConfig::dummy() }, - ..FakeEpochServiceData::dummy(Epoch(32)) - }); + ..FakeEpochServiceBuilder::dummy(Epoch(32)) + } + .build(); dependencies.epoch_service = Arc::new(RwLock::new(epoch_service)); dependencies.signed_entity_type_lock = Arc::new(SignedEntityTypeLock::default()); AggregatorRunner::new(Arc::new(dependencies)) @@ -1259,13 +1256,14 @@ pub mod tests { let runner = { let mut dependencies = initialize_dependencies().await; dependencies.signed_entity_type_lock = signed_entity_type_lock.clone(); - let epoch_service = FakeEpochService::new(FakeEpochServiceData { + let epoch_service = FakeEpochServiceBuilder { signed_entity_config: SignedEntityConfig { allowed_discriminants: SignedEntityTypeDiscriminants::all(), ..SignedEntityConfig::dummy() }, - ..FakeEpochServiceData::dummy(Epoch(32)) - }); + ..FakeEpochServiceBuilder::dummy(Epoch(32)) + } + .build(); dependencies.epoch_service = Arc::new(RwLock::new(epoch_service)); AggregatorRunner::new(Arc::new(dependencies)) @@ -1311,6 +1309,10 @@ pub mod tests { assert!(is_outdated_returned_when(IsExpired::Yes, false).await); } + // The network field in the signed entity configuration is modified to test the scenario + // where the computed signed entity type differs from the one specified in the open message. + // This is the simplest attribute to manipulate, compared to the protocol parameters, + // even though it may not be a real use case. async fn is_outdated_returned_when( is_expired: IsExpired, same_signed_entity_type: bool, @@ -1358,16 +1360,11 @@ pub mod tests { deps.certifier_service = Arc::new(mock_certifier_service); - let epoch_settings = AggregatorEpochSettings::dummy(); - let epoch_service = FakeEpochService::with_data( - current_time_point.epoch, - &epoch_settings, - &epoch_settings, - &epoch_settings, - &fake_data::signers_with_stakes(1), - &fake_data::signers_with_stakes(1), - epoch_service_signed_entity_config, - ); + let epoch_service = FakeEpochServiceBuilder { + signed_entity_config: epoch_service_signed_entity_config, + ..FakeEpochServiceBuilder::dummy(current_time_point.epoch) + } + .build(); deps.epoch_service = Arc::new(RwLock::new(epoch_service)); build_runner_with_fixture_data(deps).await diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index 4f4d41569a8..577e06d9222 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -399,7 +399,7 @@ pub struct FakeEpochService { } #[cfg(test)] -pub struct FakeEpochServiceData { +pub struct FakeEpochServiceBuilder { pub epoch: Epoch, pub epoch_settings: AggregatorEpochSettings, pub next_epoch_settings: AggregatorEpochSettings, @@ -410,7 +410,7 @@ pub struct FakeEpochServiceData { } #[cfg(test)] -impl FakeEpochServiceData { +impl FakeEpochServiceBuilder { pub fn dummy(epoch: Epoch) -> Self { use mithril_common::test_utils::fake_data; let signers = fake_data::signers_with_stakes(3); @@ -425,40 +425,37 @@ impl FakeEpochServiceData { signed_entity_config: SignedEntityConfig::dummy(), } } -} -#[cfg(test)] -impl FakeEpochService { - pub fn new(data: FakeEpochServiceData) -> Self { - let current_signers = Signer::vec_from(data.current_signers_with_stake.clone()); - let next_signers = Signer::vec_from(data.next_signers_with_stake.clone()); + pub fn build(self) -> FakeEpochService { + let current_signers = Signer::vec_from(self.current_signers_with_stake.clone()); + let next_signers = Signer::vec_from(self.next_signers_with_stake.clone()); let protocol_multi_signer = SignerBuilder::new( - &data.current_signers_with_stake, - &data.epoch_settings.protocol_parameters, + &self.current_signers_with_stake, + &self.epoch_settings.protocol_parameters, ) .with_context(|| "Could not build protocol_multi_signer for epoch service") .unwrap() .build_multi_signer(); let next_protocol_multi_signer = SignerBuilder::new( - &data.next_signers_with_stake, - &data.next_epoch_settings.protocol_parameters, + &self.next_signers_with_stake, + &self.next_epoch_settings.protocol_parameters, ) .with_context(|| "Could not build protocol_multi_signer for epoch service") .unwrap() .build_multi_signer(); - Self { + FakeEpochService { epoch_data: Some(EpochData { - epoch: data.epoch, - epoch_settings: data.epoch_settings, - next_epoch_settings: data.next_epoch_settings, - upcoming_epoch_settings: data.upcoming_epoch_settings, - current_signers_with_stake: data.current_signers_with_stake, - next_signers_with_stake: data.next_signers_with_stake, + epoch: self.epoch, + epoch_settings: self.epoch_settings, + next_epoch_settings: self.next_epoch_settings, + upcoming_epoch_settings: self.upcoming_epoch_settings, + current_signers_with_stake: self.current_signers_with_stake, + next_signers_with_stake: self.next_signers_with_stake, current_signers, next_signers, - signed_entity_config: data.signed_entity_config, + signed_entity_config: self.signed_entity_config, }), computed_epoch_data: Some(ComputedEpochData { aggregate_verification_key: protocol_multi_signer @@ -473,29 +470,10 @@ impl FakeEpochService { precompute_epoch_data_error: false, } } +} - /// Note: protocol multi signers and current/next avk will be computed using the given protocol - /// parameters and signers. - pub fn with_data( - epoch: Epoch, - epoch_settings: &AggregatorEpochSettings, - next_epoch_settings: &AggregatorEpochSettings, - upcoming_epoch_settings: &AggregatorEpochSettings, - current_signers_with_stake: &[SignerWithStake], - next_signers_with_stake: &[SignerWithStake], - signed_entity_config: SignedEntityConfig, - ) -> Self { - Self::new(FakeEpochServiceData { - epoch, - epoch_settings: epoch_settings.clone(), - next_epoch_settings: next_epoch_settings.clone(), - upcoming_epoch_settings: upcoming_epoch_settings.clone(), - current_signers_with_stake: current_signers_with_stake.to_vec(), - next_signers_with_stake: next_signers_with_stake.to_vec(), - signed_entity_config, - }) - } - +#[cfg(test)] +impl FakeEpochService { pub fn from_fixture( epoch: Epoch, fixture: &mithril_common::test_utils::MithrilFixture, @@ -514,15 +492,16 @@ impl FakeEpochService { protocol_parameters: fixture.protocol_parameters(), cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), }; - Self::with_data( - epoch, - &epoch_settings, - &next_epoch_settings, - &upcoming_epoch_settings, - &fixture.signers_with_stake(), - &fixture.signers_with_stake(), - SignedEntityConfig::dummy(), - ) + + FakeEpochServiceBuilder { + epoch_settings, + next_epoch_settings, + upcoming_epoch_settings, + current_signers_with_stake: fixture.signers_with_stake(), + next_signers_with_stake: fixture.signers_with_stake(), + ..FakeEpochServiceBuilder::dummy(epoch) + } + .build() } /// Note: using this will make all 'get' method from [EpochService] trait diff --git a/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs b/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs index 8082c34351c..ea1f1130f70 100644 --- a/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs +++ b/mithril-aggregator/src/services/signable_builder/signable_seed_builder.rs @@ -57,11 +57,11 @@ impl SignableSeedBuilder for AggregatorSignableSeedBuilder { #[cfg(test)] mod tests { use mithril_common::{ - entities::{Epoch, SignedEntityConfig}, + entities::Epoch, test_utils::{MithrilFixture, MithrilFixtureBuilder}, }; - use crate::{entities::AggregatorEpochSettings, services::FakeEpochService}; + use crate::{entities::AggregatorEpochSettings, services::FakeEpochServiceBuilder}; use super::*; @@ -70,24 +70,26 @@ mod tests { fixture: &MithrilFixture, next_fixture: &MithrilFixture, ) -> AggregatorSignableSeedBuilder { - let epoch_service = Arc::new(RwLock::new(FakeEpochService::with_data( - epoch, - &AggregatorEpochSettings { - protocol_parameters: fixture.protocol_parameters(), - ..AggregatorEpochSettings::dummy() - }, - &AggregatorEpochSettings { - protocol_parameters: next_fixture.protocol_parameters(), - ..AggregatorEpochSettings::dummy() - }, - &AggregatorEpochSettings { - protocol_parameters: next_fixture.protocol_parameters(), - ..AggregatorEpochSettings::dummy() - }, - &fixture.signers_with_stake(), - &next_fixture.signers_with_stake(), - SignedEntityConfig::dummy(), - ))); + let epoch_service = Arc::new(RwLock::new( + FakeEpochServiceBuilder { + epoch_settings: AggregatorEpochSettings { + protocol_parameters: fixture.protocol_parameters(), + ..AggregatorEpochSettings::dummy() + }, + next_epoch_settings: AggregatorEpochSettings { + protocol_parameters: next_fixture.protocol_parameters(), + ..AggregatorEpochSettings::dummy() + }, + upcoming_epoch_settings: AggregatorEpochSettings { + protocol_parameters: next_fixture.protocol_parameters(), + ..AggregatorEpochSettings::dummy() + }, + current_signers_with_stake: fixture.signers_with_stake(), + next_signers_with_stake: next_fixture.signers_with_stake(), + ..FakeEpochServiceBuilder::dummy(epoch) + } + .build(), + )); AggregatorSignableSeedBuilder::new(epoch_service) } From 65e8a7dfc641fc2dd087abebb6d1bd86ff425f3a Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 9 Oct 2024 15:40:04 +0200 Subject: [PATCH 09/15] Renaming and remove comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Sébastien Fauvel --- .../src/http_server/routes/epoch_routes.rs | 4 ++- .../src/http_server/routes/middlewares.rs | 4 +-- .../src/http_server/routes/root_routes.rs | 6 ++-- mithril-aggregator/src/runtime/runner.rs | 36 ++++++++----------- .../src/runtime/state_machine.rs | 10 +++--- .../src/services/epoch_service.rs | 10 ++---- 6 files changed, 31 insertions(+), 39 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/epoch_routes.rs b/mithril-aggregator/src/http_server/routes/epoch_routes.rs index a6faaf5e98d..0cd7040dea7 100644 --- a/mithril-aggregator/src/http_server/routes/epoch_routes.rs +++ b/mithril-aggregator/src/http_server/routes/epoch_routes.rs @@ -24,7 +24,9 @@ fn epoch_settings( warp::path!("epoch-settings") .and(warp::get()) .and(middlewares::with_epoch_service(dependency_manager.clone())) - .and(middlewares::with_allowed_discriminants(dependency_manager)) + .and(middlewares::with_allowed_signed_entity_type_discriminants( + dependency_manager, + )) .and_then(handlers::epoch_settings) } diff --git a/mithril-aggregator/src/http_server/routes/middlewares.rs b/mithril-aggregator/src/http_server/routes/middlewares.rs index 5cce8484d5f..0dd3039c867 100644 --- a/mithril-aggregator/src/http_server/routes/middlewares.rs +++ b/mithril-aggregator/src/http_server/routes/middlewares.rs @@ -44,8 +44,8 @@ pub fn with_config( warp::any().map(move || dependency_manager.config.clone()) } -/// With allowed discriminants config middleware -pub fn with_allowed_discriminants( +/// With allowed signed entity discriminants middleware +pub fn with_allowed_signed_entity_type_discriminants( dependency_manager: Arc, ) -> impl Filter,), Error = Infallible> + Clone { warp::any().map(move || dependency_manager.allowed_discriminants.clone()) diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index 4f029615b18..12cb259a2a0 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -18,7 +18,7 @@ fn root( .and(middlewares::with_api_version_provider( dependency_manager.clone(), )) - .and(middlewares::with_allowed_discriminants( + .and(middlewares::with_allowed_signed_entity_type_discriminants( dependency_manager.clone(), )) .and(middlewares::with_config(dependency_manager)) @@ -44,7 +44,7 @@ mod handlers { /// Root pub async fn root( api_version_provider: Arc, - allowed_discriminants: BTreeSet, + allowed_signed_entity_type_discriminants: BTreeSet, configuration: Configuration, ) -> Result { debug!("⇄ HTTP SERVER: root"); @@ -55,7 +55,7 @@ mod handlers { ); let mut capabilities = AggregatorCapabilities { - signed_entity_types: allowed_discriminants, + signed_entity_types: allowed_signed_entity_type_discriminants, cardano_transactions_prover: None, cardano_transactions_signing_config: None, }; diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index df5ad55a320..7030de7b72e 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -125,7 +125,7 @@ pub trait AggregatorRunnerTrait: Sync + Send { ) -> StdResult; /// Checks if the open message is considered outdated. - async fn is_outdated( + async fn is_open_message_outdated( &self, open_message_signed_entity_type: SignedEntityType, last_time_point: &TimePoint, @@ -490,7 +490,7 @@ impl AggregatorRunnerTrait for AggregatorRunner { .await } - async fn is_outdated( + async fn is_open_message_outdated( &self, open_message_signed_entity_type: SignedEntityType, last_time_point: &TimePoint, @@ -1287,36 +1287,30 @@ pub mod tests { } #[tokio::test] - async fn is_outdated_return_false_when_message_is_not_expired_and_has_same_signed_entity_type() - { - assert!(!is_outdated_returned_when(IsExpired::No, true).await); + async fn is_outdated_return_false_when_message_is_not_expired_and_no_newer_open_message() { + assert!(!is_outdated_returned_when(IsExpired::No, false).await); } #[tokio::test] - async fn is_outdated_return_true_when_message_is_expired_and_has_same_signed_entity_type() { - assert!(is_outdated_returned_when(IsExpired::Yes, true).await); + async fn is_outdated_return_true_when_message_is_expired_and_no_newer_open_message() { + assert!(is_outdated_returned_when(IsExpired::Yes, false).await); } #[tokio::test] - async fn is_outdated_return_true_when_message_is_not_expired_and_has_not_the_same_signed_entity_type( - ) { - assert!(is_outdated_returned_when(IsExpired::No, false).await); + async fn is_outdated_return_true_when_message_is_not_expired_and_exists_newer_open_message() { + assert!(is_outdated_returned_when(IsExpired::No, true).await); } #[tokio::test] - async fn is_outdated_return_true_when_message_is_expired_and_has_not_the_same_signed_entity_type( - ) { - assert!(is_outdated_returned_when(IsExpired::Yes, false).await); + async fn is_outdated_return_true_when_message_is_expired_and_exists_newer_open_message() { + assert!(is_outdated_returned_when(IsExpired::Yes, true).await); } // The network field in the signed entity configuration is modified to test the scenario // where the computed signed entity type differs from the one specified in the open message. // This is the simplest attribute to manipulate, compared to the protocol parameters, // even though it may not be a real use case. - async fn is_outdated_returned_when( - is_expired: IsExpired, - same_signed_entity_type: bool, - ) -> bool { + async fn is_outdated_returned_when(is_expired: IsExpired, newer_open_message: bool) -> bool { let current_time_point = TimePoint { epoch: Epoch(2), immutable_file_number: 12, @@ -1324,10 +1318,10 @@ pub mod tests { }; let message_network = CardanoNetwork::MainNet; - let epoch_service_network = if same_signed_entity_type { - message_network - } else { + let epoch_service_network = if newer_open_message { CardanoNetwork::DevNet(412) + } else { + message_network }; let epoch_service_signed_entity_config = SignedEntityConfig { @@ -1371,7 +1365,7 @@ pub mod tests { }; runner - .is_outdated(open_message.signed_entity_type, ¤t_time_point) + .is_open_message_outdated(open_message.signed_entity_type, ¤t_time_point) .await .unwrap() } diff --git a/mithril-aggregator/src/runtime/state_machine.rs b/mithril-aggregator/src/runtime/state_machine.rs index b4b09e2fc09..19f8d04721a 100644 --- a/mithril-aggregator/src/runtime/state_machine.rs +++ b/mithril-aggregator/src/runtime/state_machine.rs @@ -225,7 +225,7 @@ impl AggregatorRuntime { let is_outdated = self .runner - .is_outdated( + .is_open_message_outdated( state.open_message.signed_entity_type.clone(), &last_time_point, ) @@ -632,7 +632,7 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_is_outdated() + .expect_is_open_message_outdated() .once() .returning(|_, _| Ok(true)); runner @@ -659,7 +659,7 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_is_outdated() + .expect_is_open_message_outdated() .once() .returning(|_, _| Ok(false)); runner @@ -692,7 +692,7 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_is_outdated() + .expect_is_open_message_outdated() .once() .returning(|_, _| Ok(false)); runner @@ -732,7 +732,7 @@ mod tests { .once() .returning(|| Ok(TimePoint::dummy())); runner - .expect_is_outdated() + .expect_is_open_message_outdated() .once() .returning(|_, _| Ok(false)); runner diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index 577e06d9222..1a91715f34e 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -129,7 +129,7 @@ pub struct MithrilEpochService { epoch_settings_storer: Arc, verification_key_store: Arc, network: CardanoNetwork, - allowed_discriminants: BTreeSet, + allowed_signed_entity_discriminants: BTreeSet, } impl MithrilEpochService { @@ -148,7 +148,7 @@ impl MithrilEpochService { epoch_settings_storer, verification_key_store, network, - allowed_discriminants, + allowed_signed_entity_discriminants: allowed_discriminants, } } @@ -250,7 +250,7 @@ impl EpochService for MithrilEpochService { let next_signers = Signer::vec_from(next_signers_with_stake.clone()); let signed_entity_config = SignedEntityConfig { - allowed_discriminants: self.allowed_discriminants.clone(), + allowed_discriminants: self.allowed_signed_entity_discriminants.clone(), network: self.network, cardano_transactions_signing_config: epoch_settings .cardano_transactions_signing_config @@ -740,18 +740,14 @@ mod tests { fn new(epoch: Epoch, current_epoch_fixture: MithrilFixture) -> Self { let next_epoch_fixture = current_epoch_fixture.clone(); Self { - // Aggregator configuration cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), future_protocol_parameters: current_epoch_fixture.protocol_parameters(), network: CardanoNetwork::TestNet(0), allowed_discriminants: BTreeSet::new(), - // Epoch used for verification keys and database signer_retrieval_epoch: epoch.offset_to_signer_retrieval_epoch().unwrap(), next_signer_retrieval_epoch: epoch.offset_to_next_signer_retrieval_epoch(), - // Signers in verification key store signers_with_stake: current_epoch_fixture.signers_with_stake(), next_signers_with_stake: next_epoch_fixture.signers_with_stake(), - // Database data stored_epoch_settings: AggregatorEpochSettings { protocol_parameters: current_epoch_fixture.protocol_parameters(), cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), From 0244426f7e7065bc20350d4c5c437e5ceb66095b Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 9 Oct 2024 16:46:24 +0200 Subject: [PATCH 10/15] Refactor signed entity types construction in route route tests --- .../src/http_server/routes/root_routes.rs | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/mithril-aggregator/src/http_server/routes/root_routes.rs b/mithril-aggregator/src/http_server/routes/root_routes.rs index 12cb259a2a0..781a0f165e2 100644 --- a/mithril-aggregator/src/http_server/routes/root_routes.rs +++ b/mithril-aggregator/src/http_server/routes/root_routes.rs @@ -125,17 +125,12 @@ mod tests { let method = Method::GET.as_str(); let path = "/"; let config = Configuration { - signed_entity_types: Some( - BTreeSet::from([ - SignedEntityTypeDiscriminants::CardanoStakeDistribution, - SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, - SignedEntityTypeDiscriminants::MithrilStakeDistribution, - ]) - .iter() - .map(SignedEntityTypeDiscriminants::to_string) - .collect::>() - .join(","), - ), + signed_entity_types: Some(format!( + "{}, {}, {}", + SignedEntityTypeDiscriminants::CardanoStakeDistribution, + SignedEntityTypeDiscriminants::CardanoImmutableFilesFull, + SignedEntityTypeDiscriminants::MithrilStakeDistribution, + )), ..Configuration::new_sample() }; let mut builder = DependenciesBuilder::new(config); @@ -193,13 +188,10 @@ mod tests { let method = Method::GET.as_str(); let path = "/"; let config = Configuration { - signed_entity_types: Some( - BTreeSet::from([SignedEntityTypeDiscriminants::CardanoTransactions]) - .iter() - .map(SignedEntityTypeDiscriminants::to_string) - .collect::>() - .join(","), - ), + signed_entity_types: Some(format!( + "{}", + SignedEntityTypeDiscriminants::CardanoTransactions + )), ..Configuration::new_sample() }; let mut builder = DependenciesBuilder::new(config); From 8d1d4e498e927a5e3070eb6e86fb60413e91ef7a Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 9 Oct 2024 17:02:07 +0200 Subject: [PATCH 11/15] `append_allowed_signed_entity_types_discriminants` returns a new instance of the list instead of updating the given one --- mithril-aggregator/src/configuration.rs | 9 +++++---- mithril-common/src/entities/signed_entity_config.rs | 11 ++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/mithril-aggregator/src/configuration.rs b/mithril-aggregator/src/configuration.rs index af4c00b3b37..de92ee316a2 100644 --- a/mithril-aggregator/src/configuration.rs +++ b/mithril-aggregator/src/configuration.rs @@ -293,16 +293,17 @@ impl Configuration { pub fn compute_allowed_signed_entity_types_discriminants( &self, ) -> StdResult> { - let mut allowed_discriminants = self + let allowed_discriminants = self .signed_entity_types .as_ref() .map(SignedEntityTypeDiscriminants::parse_list) .transpose() .with_context(|| "Invalid 'signed_entity_types' configuration")? .unwrap_or_default(); - SignedEntityConfig::append_allowed_signed_entity_types_discriminants( - &mut allowed_discriminants, - ); + let allowed_discriminants = + SignedEntityConfig::append_allowed_signed_entity_types_discriminants( + allowed_discriminants, + ); Ok(allowed_discriminants) } diff --git a/mithril-common/src/entities/signed_entity_config.rs b/mithril-common/src/entities/signed_entity_config.rs index 6ede65f07fe..0fe5604a1d5 100644 --- a/mithril-common/src/entities/signed_entity_config.rs +++ b/mithril-common/src/entities/signed_entity_config.rs @@ -43,9 +43,11 @@ impl SignedEntityConfig { /// Append to the given list of allowed signed entity types discriminants the [Self::DEFAULT_ALLOWED_DISCRIMINANTS] /// if not already present. pub fn append_allowed_signed_entity_types_discriminants( - discriminants: &mut BTreeSet, - ) { + discriminants: BTreeSet, + ) -> BTreeSet { + let mut discriminants = discriminants; discriminants.append(&mut BTreeSet::from(Self::DEFAULT_ALLOWED_DISCRIMINANTS)); + discriminants } /// Create the deduplicated list of allowed signed entity types discriminants. @@ -55,9 +57,8 @@ impl SignedEntityConfig { pub fn list_allowed_signed_entity_types_discriminants( &self, ) -> BTreeSet { - let mut discriminants = self.allowed_discriminants.clone(); - Self::append_allowed_signed_entity_types_discriminants(&mut discriminants); - discriminants + let discriminants = self.allowed_discriminants.clone(); + Self::append_allowed_signed_entity_types_discriminants(discriminants) } /// Convert this time point to a signed entity type based on the given discriminant. From 61c48e8ed616e8f33937aca7b01bd1f463fc8cec Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 9 Oct 2024 17:10:18 +0200 Subject: [PATCH 12/15] Remove `allowed_discriminants` from dependencies builder --- .../src/dependency_injection/builder.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index d06715e284d..7a9ce67f483 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -98,9 +98,6 @@ pub struct DependenciesBuilder { /// Configuration parameters pub configuration: Configuration, - /// List of signed entity discriminants that are allowed to be processed - pub allowed_discriminants: Option>, - /// SQLite database connection pub sqlite_connection: Option>, @@ -243,7 +240,6 @@ impl DependenciesBuilder { pub fn new(configuration: Configuration) -> Self { Self { configuration, - allowed_discriminants: None, sqlite_connection: None, sqlite_connection_cardano_transaction_pool: None, stake_store: None, @@ -292,17 +288,14 @@ impl DependenciesBuilder { } /// Get the allowed signed entity types discriminants - pub fn get_allowed_signed_entity_types_discriminants( - &mut self, + fn get_allowed_signed_entity_types_discriminants( + &self, ) -> Result> { - if self.allowed_discriminants.is_none() { - self.allowed_discriminants = Some( - self.configuration - .compute_allowed_signed_entity_types_discriminants()?, - ); - } + let allowed_discriminants = self + .configuration + .compute_allowed_signed_entity_types_discriminants()?; - Ok(self.allowed_discriminants.clone().unwrap()) + Ok(allowed_discriminants) } fn build_sqlite_connection( From 2def8ad1ac787751d9dca1409323308456007ba8 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:01:12 +0200 Subject: [PATCH 13/15] Change the way to test `is_open_message_outdated` cases when there is a newer open message. Now modifying the epoch message instead of the signed entity config parameter. We are not testing anymore that the function calls the epoch service. --- mithril-aggregator/src/runtime/runner.rs | 56 +++++++++--------------- 1 file changed, 21 insertions(+), 35 deletions(-) diff --git a/mithril-aggregator/src/runtime/runner.rs b/mithril-aggregator/src/runtime/runner.rs index 7030de7b72e..ddce241007a 100644 --- a/mithril-aggregator/src/runtime/runner.rs +++ b/mithril-aggregator/src/runtime/runner.rs @@ -535,11 +535,10 @@ pub mod tests { use async_trait::async_trait; use chrono::{DateTime, Utc}; use mithril_common::entities::{ - CardanoDbBeacon, CardanoTransactionsSigningConfig, ChainPoint, Epoch, SignedEntityConfig, + CardanoTransactionsSigningConfig, ChainPoint, Epoch, SignedEntityConfig, SignedEntityTypeDiscriminants, }; use mithril_common::signed_entity_type_lock::SignedEntityTypeLock; - use mithril_common::CardanoNetwork; use mithril_common::{ chain_observer::FakeObserver, digesters::DumbImmutableFileObserver, @@ -1287,54 +1286,42 @@ pub mod tests { } #[tokio::test] - async fn is_outdated_return_false_when_message_is_not_expired_and_no_newer_open_message() { + async fn is_open_message_outdated_return_false_when_message_is_not_expired_and_no_newer_open_message( + ) { assert!(!is_outdated_returned_when(IsExpired::No, false).await); } #[tokio::test] - async fn is_outdated_return_true_when_message_is_expired_and_no_newer_open_message() { + async fn is_open_message_outdated_return_true_when_message_is_expired_and_no_newer_open_message( + ) { assert!(is_outdated_returned_when(IsExpired::Yes, false).await); } #[tokio::test] - async fn is_outdated_return_true_when_message_is_not_expired_and_exists_newer_open_message() { + async fn is_open_message_outdated_return_true_when_message_is_not_expired_and_exists_newer_open_message( + ) { assert!(is_outdated_returned_when(IsExpired::No, true).await); } #[tokio::test] - async fn is_outdated_return_true_when_message_is_expired_and_exists_newer_open_message() { + async fn is_open_message_outdated_return_true_when_message_is_expired_and_exists_newer_open_message( + ) { assert!(is_outdated_returned_when(IsExpired::Yes, true).await); } - // The network field in the signed entity configuration is modified to test the scenario - // where the computed signed entity type differs from the one specified in the open message. - // This is the simplest attribute to manipulate, compared to the protocol parameters, - // even though it may not be a real use case. async fn is_outdated_returned_when(is_expired: IsExpired, newer_open_message: bool) -> bool { let current_time_point = TimePoint { epoch: Epoch(2), - immutable_file_number: 12, ..TimePoint::dummy() }; - let message_network = CardanoNetwork::MainNet; - let epoch_service_network = if newer_open_message { - CardanoNetwork::DevNet(412) + let message_epoch = if newer_open_message { + current_time_point.epoch + 54 } else { - message_network - }; - - let epoch_service_signed_entity_config = SignedEntityConfig { - network: epoch_service_network, - ..SignedEntityConfig::dummy() + current_time_point.epoch }; - let cardano_db_beacon = CardanoDbBeacon { - network: message_network.to_string(), - epoch: current_time_point.epoch, - immutable_file_number: current_time_point.immutable_file_number, - }; - let open_message = OpenMessage { - signed_entity_type: SignedEntityType::CardanoImmutableFilesFull(cardano_db_beacon), + let open_message_to_verify = OpenMessage { + signed_entity_type: SignedEntityType::MithrilStakeDistribution(message_epoch), is_expired: is_expired == IsExpired::Yes, ..OpenMessage::dummy() }; @@ -1343,29 +1330,28 @@ pub mod tests { let mut deps = initialize_dependencies().await; let mut mock_certifier_service = MockCertifierService::new(); - let open_message_cloned = open_message.clone(); + let open_message_current = open_message_to_verify.clone(); mock_certifier_service .expect_get_open_message() .times(1) - .return_once(|_| Ok(Some(open_message_cloned))); + .return_once(|_| Ok(Some(open_message_current))); mock_certifier_service .expect_mark_open_message_if_expired() .returning(|_| Ok(None)); deps.certifier_service = Arc::new(mock_certifier_service); - let epoch_service = FakeEpochServiceBuilder { - signed_entity_config: epoch_service_signed_entity_config, - ..FakeEpochServiceBuilder::dummy(current_time_point.epoch) - } - .build(); + let epoch_service = FakeEpochServiceBuilder::dummy(current_time_point.epoch).build(); deps.epoch_service = Arc::new(RwLock::new(epoch_service)); build_runner_with_fixture_data(deps).await }; runner - .is_open_message_outdated(open_message.signed_entity_type, ¤t_time_point) + .is_open_message_outdated( + open_message_to_verify.signed_entity_type, + ¤t_time_point, + ) .await .unwrap() } From 85fa75b9e93f986d386e87d321eb7e900b1820c7 Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:08:17 +0200 Subject: [PATCH 14/15] Refactor `EpochServiceBuilder` simplifying its inner parameters --- .../src/services/epoch_service.rs | 38 ++++++++++--------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/mithril-aggregator/src/services/epoch_service.rs b/mithril-aggregator/src/services/epoch_service.rs index 1a91715f34e..8183e6bbb41 100644 --- a/mithril-aggregator/src/services/epoch_service.rs +++ b/mithril-aggregator/src/services/epoch_service.rs @@ -727,8 +727,7 @@ mod tests { future_protocol_parameters: ProtocolParameters, network: CardanoNetwork, allowed_discriminants: BTreeSet, - signer_retrieval_epoch: Epoch, - next_signer_retrieval_epoch: Epoch, + current_epoch: Epoch, signers_with_stake: Vec, next_signers_with_stake: Vec, stored_epoch_settings: AggregatorEpochSettings, @@ -737,52 +736,57 @@ mod tests { } impl EpochServiceBuilder { - fn new(epoch: Epoch, current_epoch_fixture: MithrilFixture) -> Self { - let next_epoch_fixture = current_epoch_fixture.clone(); + fn new(epoch: Epoch, epoch_fixture: MithrilFixture) -> Self { Self { cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), - future_protocol_parameters: current_epoch_fixture.protocol_parameters(), + future_protocol_parameters: epoch_fixture.protocol_parameters(), network: CardanoNetwork::TestNet(0), allowed_discriminants: BTreeSet::new(), - signer_retrieval_epoch: epoch.offset_to_signer_retrieval_epoch().unwrap(), - next_signer_retrieval_epoch: epoch.offset_to_next_signer_retrieval_epoch(), - signers_with_stake: current_epoch_fixture.signers_with_stake(), - next_signers_with_stake: next_epoch_fixture.signers_with_stake(), + current_epoch: epoch, + signers_with_stake: epoch_fixture.signers_with_stake(), + next_signers_with_stake: epoch_fixture.signers_with_stake(), stored_epoch_settings: AggregatorEpochSettings { - protocol_parameters: current_epoch_fixture.protocol_parameters(), + protocol_parameters: epoch_fixture.protocol_parameters(), cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), }, stored_next_epoch_settings: AggregatorEpochSettings { - protocol_parameters: current_epoch_fixture.protocol_parameters(), + protocol_parameters: epoch_fixture.protocol_parameters(), cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), }, stored_upcoming_epoch_settings: AggregatorEpochSettings { - protocol_parameters: current_epoch_fixture.protocol_parameters(), + protocol_parameters: epoch_fixture.protocol_parameters(), cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(), }, } } fn build(self) -> MithrilEpochService { + let signer_retrieval_epoch = self + .current_epoch + .offset_to_signer_retrieval_epoch() + .unwrap(); + let next_signer_retrieval_epoch = + self.current_epoch.offset_to_next_signer_retrieval_epoch(); + let epoch_settings_storer = FakeEpochSettingsStorer::new(vec![ - (self.signer_retrieval_epoch, self.stored_epoch_settings), + (signer_retrieval_epoch, self.stored_epoch_settings), ( - self.next_signer_retrieval_epoch, + next_signer_retrieval_epoch, self.stored_next_epoch_settings.clone(), ), ( - self.next_signer_retrieval_epoch.next(), + next_signer_retrieval_epoch.next(), self.stored_upcoming_epoch_settings.clone(), ), ]); let vkey_store = VerificationKeyStore::new(Box::new( MemoryAdapter::new(Some(vec![ ( - self.signer_retrieval_epoch, + signer_retrieval_epoch, map_signers_for_vkey_store(&self.signers_with_stake), ), ( - self.next_signer_retrieval_epoch, + next_signer_retrieval_epoch, map_signers_for_vkey_store(&self.next_signers_with_stake), ), ])) From 91f748e7330d1ebfb83862e9bdc4b75d621072df Mon Sep 17 00:00:00 2001 From: Damien Lachaume <135982616+dlachaume@users.noreply.github.com> Date: Wed, 9 Oct 2024 18:15:25 +0200 Subject: [PATCH 15/15] chore: upgrade crate versions * mithril-aggregator from `0.5.77` to `0.5.78` * mithril-common from `0.4.66` to `0.4.67` --- Cargo.lock | 4 ++-- mithril-aggregator/Cargo.toml | 2 +- mithril-common/Cargo.toml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 146e7a9e7f9..9e211be853f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3403,7 +3403,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.5.77" +version = "0.5.78" dependencies = [ "anyhow", "async-trait", @@ -3559,7 +3559,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.4.66" +version = "0.4.67" dependencies = [ "anyhow", "async-trait", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index e86aa3620c0..7e7e094db63 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.5.77" +version = "0.5.78" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index 51fa28491e9..4ea4e38c8bc 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.4.66" +version = "0.4.67" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true }