From 8e1e22719d2659639ea1f776c4d421b8e8f707af Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Wed, 7 Jun 2023 11:36:06 +0200 Subject: [PATCH 1/5] Fix get open message form correct epoch Align the queries to retrieve open message with/without single signatures in Open Message Repository. --- .../src/database/provider/open_message.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/mithril-aggregator/src/database/provider/open_message.rs b/mithril-aggregator/src/database/provider/open_message.rs index 0074d469719..e94b82e1314 100644 --- a/mithril-aggregator/src/database/provider/open_message.rs +++ b/mithril-aggregator/src/database/provider/open_message.rs @@ -421,6 +421,13 @@ impl<'client> OpenMessageWithSingleSignaturesProvider<'client> { Self { connection } } + fn get_epoch_condition(&self, epoch: Epoch) -> WhereCondition { + WhereCondition::new( + "epoch_setting_id = ?*", + vec![Value::Integer(epoch.0 as i64)], + ) + } + fn get_signed_entity_type_condition( &self, signed_entity_type: &SignedEntityType, @@ -493,14 +500,16 @@ impl OpenMessageRepository { Ok(messages.next()) } - /// Return an open message with its associated single signatures if any. + /// Return an open message with its associated single signatures for the given Epoch and [SignedEntityType]. pub async fn get_open_message_with_single_signatures( &self, signed_entity_type: &SignedEntityType, ) -> StdResult> { let lock = self.connection.lock().await; let provider = OpenMessageWithSingleSignaturesProvider::new(&lock); - let filters = provider.get_signed_entity_type_condition(signed_entity_type); + let filters = provider + .get_epoch_condition(signed_entity_type.get_epoch()) + .and_where(provider.get_signed_entity_type_condition(signed_entity_type)); let mut messages = provider.find(filters)?; Ok(messages.next()) From 83c19fc54d31d7915b4fde9df35da144fb63ac51 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Wed, 7 Jun 2023 14:40:20 +0200 Subject: [PATCH 2/5] Fix certifier service open message creation --- mithril-aggregator/src/certifier_service.rs | 7 +++++-- mithril-signer/src/runtime/runner.rs | 6 +----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/mithril-aggregator/src/certifier_service.rs b/mithril-aggregator/src/certifier_service.rs index 81707fcfbf0..f0629ac5e56 100644 --- a/mithril-aggregator/src/certifier_service.rs +++ b/mithril-aggregator/src/certifier_service.rs @@ -231,10 +231,13 @@ impl CertifierService for MithrilCertifierService { protocol_message: &ProtocolMessage, ) -> StdResult { debug!("CertifierService::create_open_message(signed_entity_type: {signed_entity_type:?}, protocol_message: {protocol_message:?})"); - let current_epoch = self.current_epoch.read().await; let open_message = self .open_message_repository - .create_open_message(*current_epoch, signed_entity_type, protocol_message) + .create_open_message( + signed_entity_type.get_epoch(), + signed_entity_type, + protocol_message, + ) .await?; info!("CertifierService::create_open_message: created open message for {signed_entity_type:?}"); debug!( diff --git a/mithril-signer/src/runtime/runner.rs b/mithril-signer/src/runtime/runner.rs index 3e5ee37f212..9272d6ecbd2 100644 --- a/mithril-signer/src/runtime/runner.rs +++ b/mithril-signer/src/runtime/runner.rs @@ -366,11 +366,7 @@ impl Runner for SignerRunner { .await?; // 2 set the next signers keys and stakes in the message - let epoch = match signed_entity_type { - SignedEntityType::MithrilStakeDistribution(epoch) => epoch, - SignedEntityType::CardanoImmutableFilesFull(beacon) => &beacon.epoch, - SignedEntityType::CardanoStakeDistribution(epoch) => epoch, - }; + let epoch = signed_entity_type.get_epoch(); let next_signer_retrieval_epoch = epoch.offset_to_next_signer_retrieval_epoch(); let next_protocol_initializer = self .services From a8896f78c2e90b5bf5e3f2f4d1a266591dd4f9a1 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Thu, 8 Jun 2023 14:52:46 +0200 Subject: [PATCH 3/5] Fix some flakiness in the end to end test This was due to the protocol parameters that were too tight from time to time, were there was e.g. 148 signatures gathered instead of 150 required. --- mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs index ddd28400990..9536e57b19a 100644 --- a/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs +++ b/mithril-test-lab/mithril-end-to-end/src/end_to_end_spec.rs @@ -255,7 +255,7 @@ async fn update_protocol_parameters(aggregator: &mut Aggregator) -> Result<(), S let protocol_parameters_new = ProtocolParameters { k: 150, m: 200, - phi_f: 0.80, + phi_f: 0.85, }; info!( "> updating protocol parameters to {:?}...", From 287f12baa75b3938f6b99c4a084209f342a76b0e Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Thu, 8 Jun 2023 15:05:04 +0200 Subject: [PATCH 4/5] Remove current epoch from Certifier Service --- mithril-aggregator/src/certifier_service.rs | 71 +++---------------- .../src/dependency_injection/builder.rs | 5 +- 2 files changed, 12 insertions(+), 64 deletions(-) diff --git a/mithril-aggregator/src/certifier_service.rs b/mithril-aggregator/src/certifier_service.rs index f0629ac5e56..41dc306ce0c 100644 --- a/mithril-aggregator/src/certifier_service.rs +++ b/mithril-aggregator/src/certifier_service.rs @@ -126,7 +126,6 @@ pub struct MithrilCertifierService { certificate_verifier: Arc, genesis_verifier: Arc, multi_signer: Arc>, - current_epoch: Arc>, _logger: Logger, } @@ -140,7 +139,6 @@ impl MithrilCertifierService { certificate_verifier: Arc, genesis_verifier: Arc, multi_signer: Arc>, - current_epoch: Epoch, logger: Logger, ) -> Self { Self { @@ -150,7 +148,6 @@ impl MithrilCertifierService { multi_signer, certificate_verifier, genesis_verifier, - current_epoch: Arc::new(RwLock::new(current_epoch)), _logger: logger, } } @@ -173,19 +170,8 @@ impl MithrilCertifierService { impl CertifierService for MithrilCertifierService { async fn inform_epoch(&self, epoch: Epoch) -> StdResult<()> { debug!("CertifierService::inform_epoch(epoch: {epoch:?})"); - let mut current_epoch = self.current_epoch.write().await; - - if epoch <= *current_epoch { - debug!("CertifierService::inform_epoch: given epoch ({epoch:?}) is older than current epoch ({}), ignoring", *current_epoch); - - return Ok(()); - } - let nb = self - .open_message_repository - .clean_epoch(*current_epoch) - .await?; - info!("MithrilCertifierService: Got a new Epoch: {epoch:?}. Cleaned {nb} open messages along with their single signatures."); - *current_epoch = epoch; + let nb = self.open_message_repository.clean_epoch(epoch).await?; + info!("MithrilCertifierService: Informed of a new Epoch: {epoch:?}. Cleaned {nb} open messages along with their single signatures."); Ok(()) } @@ -417,43 +403,19 @@ mod tests { certificate_verifier, genesis_verifier, multi_signer, - Epoch(0), logger, ) } #[tokio::test] - async fn should_not_clean_epoch_when_inform_same_epoch() { - let beacon = Beacon::new("devnet".to_string(), 1, 1); - let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); - let protocol_message = ProtocolMessage::new(); - let epoch = beacon.epoch; - let epochs_with_signers = (1..=5).map(Epoch).collect::>(); - let fixture = MithrilFixtureBuilder::default().with_signers(5).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); - certifier_service - .create_open_message(&signed_entity_type, &protocol_message) - .await - .unwrap(); - certifier_service.inform_epoch(epoch).await.unwrap(); - let open_message = certifier_service - .get_open_message(&signed_entity_type) - .await - .unwrap(); - assert!(open_message.is_some()); - } - - #[tokio::test] - async fn should_clean_epoch_when_inform_new_epoch() { + async fn should_clean_epoch_when_inform_epoch() { let beacon = Beacon::new("devnet".to_string(), 1, 1); let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); let protocol_message = ProtocolMessage::new(); let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(5).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); + let certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; certifier_service .create_open_message(&signed_entity_type, &protocol_message) .await @@ -471,11 +433,9 @@ mod tests { let beacon = Beacon::new("devnet".to_string(), 3, 1); let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); let protocol_message = ProtocolMessage::new(); - let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(1).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); + let certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; certifier_service .multi_signer .write() @@ -512,11 +472,9 @@ mod tests { let beacon = Beacon::new("devnet".to_string(), 3, 1); let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); let mut protocol_message = ProtocolMessage::new(); - let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(1).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); + let certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; certifier_service .multi_signer .write() @@ -552,11 +510,9 @@ mod tests { let beacon = Beacon::new("devnet".to_string(), 3, 1); let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); let protocol_message = ProtocolMessage::new(); - let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(1).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); + let certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; let mut open_message = certifier_service .open_message_repository .create_open_message(beacon.epoch, &signed_entity_type, &protocol_message) @@ -586,11 +542,9 @@ mod tests { let beacon = Beacon::new("devnet".to_string(), 3, 1); let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); let protocol_message = ProtocolMessage::new(); - let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(3).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); + let certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; certifier_service .multi_signer .write() @@ -667,11 +621,9 @@ mod tests { async fn should_not_create_certificate_for_open_message_not_created() { let beacon = Beacon::new("devnet".to_string(), 1, 1); let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); - let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(5).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); + let certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; certifier_service .create_certificate(&signed_entity_type) .await @@ -686,8 +638,7 @@ mod tests { let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(5).build(); - let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); + let certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; certifier_service .open_message_repository .create_open_message(epoch, &signed_entity_type, &protocol_message) @@ -708,11 +659,9 @@ mod tests { let beacon = Beacon::new("devnet".to_string(), 1, 1); let signed_entity_type = SignedEntityType::CardanoImmutableFilesFull(beacon.clone()); let protocol_message = ProtocolMessage::new(); - let epoch = beacon.epoch; let epochs_with_signers = (1..=5).map(Epoch).collect::>(); let fixture = MithrilFixtureBuilder::default().with_signers(5).build(); let mut certifier_service = setup_certifier_service(&fixture, &epochs_with_signers).await; - certifier_service.current_epoch = Arc::new(RwLock::new(epoch)); certifier_service.multi_signer = Arc::new(RwLock::new(mock_multi_signer)); certifier_service .create_open_message(&signed_entity_type, &protocol_message) diff --git a/mithril-aggregator/src/dependency_injection/builder.rs b/mithril-aggregator/src/dependency_injection/builder.rs index d5635a992ac..23fb3914554 100644 --- a/mithril-aggregator/src/dependency_injection/builder.rs +++ b/mithril-aggregator/src/dependency_injection/builder.rs @@ -1104,7 +1104,7 @@ impl DependenciesBuilder { )); let certificate_verifier = self.get_certificate_verifier().await?; let genesis_verifier = self.get_genesis_verifier().await?; - let multisigner = self.get_multi_signer().await?; + let multi_signer = self.get_multi_signer().await?; let logger = self.get_logger().await?; Ok(Arc::new(MithrilCertifierService::new( @@ -1113,8 +1113,7 @@ impl DependenciesBuilder { certificate_repository, certificate_verifier, genesis_verifier, - multisigner, - Epoch(0), + multi_signer, logger, ))) } From bd89a07e32e4168c284bb6691f5a6057a8043fe1 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Thu, 8 Jun 2023 15:05:24 +0200 Subject: [PATCH 5/5] Upgrade crates versions --- Cargo.lock | 6 +++--- mithril-aggregator/Cargo.toml | 2 +- mithril-signer/Cargo.toml | 2 +- mithril-test-lab/mithril-end-to-end/Cargo.toml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d255144e16e..bb0c4c813c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2030,7 +2030,7 @@ dependencies = [ [[package]] name = "mithril-aggregator" -version = "0.3.25" +version = "0.3.26" dependencies = [ "async-trait", "chrono", @@ -2142,7 +2142,7 @@ dependencies = [ [[package]] name = "mithril-end-to-end" -version = "0.1.19" +version = "0.1.20" dependencies = [ "async-trait", "clap 4.3.0", @@ -2164,7 +2164,7 @@ dependencies = [ [[package]] name = "mithril-signer" -version = "0.2.47" +version = "0.2.48" dependencies = [ "async-trait", "clap 4.3.0", diff --git a/mithril-aggregator/Cargo.toml b/mithril-aggregator/Cargo.toml index 2dde8e42b9b..6bcb61ea07e 100644 --- a/mithril-aggregator/Cargo.toml +++ b/mithril-aggregator/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-aggregator" -version = "0.3.25" +version = "0.3.26" description = "A Mithril Aggregator server" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-signer/Cargo.toml b/mithril-signer/Cargo.toml index d72ac2c4795..7cc597bf618 100644 --- a/mithril-signer/Cargo.toml +++ b/mithril-signer/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-signer" -version = "0.2.47" +version = "0.2.48" description = "A Mithril Signer" authors = { workspace = true } edition = { workspace = true } diff --git a/mithril-test-lab/mithril-end-to-end/Cargo.toml b/mithril-test-lab/mithril-end-to-end/Cargo.toml index 677a2923dc5..32d3f66a928 100644 --- a/mithril-test-lab/mithril-end-to-end/Cargo.toml +++ b/mithril-test-lab/mithril-end-to-end/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-end-to-end" -version = "0.1.19" +version = "0.1.20" authors = { workspace = true } edition = { workspace = true } documentation = { workspace = true }