From facae4fbedafaf8e13671c4ec3468edde91d8f11 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Fri, 11 Oct 2024 15:28:22 +0200 Subject: [PATCH 01/16] feat: implement a certificate chain builder for tests --- mithril-common/src/crypto_helper/genesis.rs | 2 +- .../test_utils/certificate_chain_builder.rs | 289 ++++++++++++++++++ mithril-common/src/test_utils/mod.rs | 2 + 3 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 mithril-common/src/test_utils/certificate_chain_builder.rs diff --git a/mithril-common/src/crypto_helper/genesis.rs b/mithril-common/src/crypto_helper/genesis.rs index 6d35510d90b..198f1174b9e 100644 --- a/mithril-common/src/crypto_helper/genesis.rs +++ b/mithril-common/src/crypto_helper/genesis.rs @@ -18,7 +18,7 @@ pub struct ProtocolGenesisError(#[source] StdError); /// A protocol Genesis Signer that is responsible for signing the /// [Genesis Certificate](https://mithril.network/doc/mithril/mithril-protocol/certificates#the-certificate-chain-design) -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct ProtocolGenesisSigner { /// Protocol Genesis secret key pub(crate) secret_key: ProtocolGenesisSecretKey, diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs new file mode 100644 index 00000000000..b5b0bce7ad3 --- /dev/null +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -0,0 +1,289 @@ +use std::{cmp::min, collections::HashMap, sync::Arc}; + +use crate::{ + certificate_chain::CertificateGenesisProducer, + crypto_helper::{ + ProtocolAggregateVerificationKey, ProtocolClerk, ProtocolGenesisSigner, + ProtocolGenesisVerifier, ProtocolParameters, + }, + entities::{ + Certificate, CertificateMetadata, CertificateSignature, Epoch, ProtocolMessagePartKey, + }, + test_utils::{fake_data, MithrilFixture, MithrilFixtureBuilder, SignerFixture}, +}; + +/// Context used while building a certificate chain. For tests only. +pub struct CertificateChainBuilderContext<'a> { + pub index_certificate: usize, + #[allow(dead_code)] + pub total_certificates: usize, + pub epoch: Epoch, + pub fixture: &'a MithrilFixture, + pub next_fixture: &'a MithrilFixture, +} + +/// A builder for creating a certificate chain. For tests only. +/// +/// # Example usage +/// +/// ``` +/// use mithril_common::crypto_helper::ProtocolParameters; +/// use mithril_common::test_utils::CertificateChainBuilder; +/// +/// let (certificate_chain, _protocol_genesis_verifier) = CertificateChainBuilder::new() +/// .with_total_certificates(5) +/// .with_certificates_per_epoch(2) +/// .with_protocol_parameters(ProtocolParameters { +/// m: 100, +/// k: 5, +/// phi_f: 0.65, +/// }).build(); +/// +/// assert_eq!(5, certificate_chain.len()); +/// ``` +pub struct CertificateChainBuilder { + total_certificates: u64, + certificates_per_epoch: u64, + protocol_parameters: ProtocolParameters, +} + +impl CertificateChainBuilder { + /// Create a new [CertificateChainBuilder] instance. + pub fn new() -> Self { + let protocol_parameters = ProtocolParameters { + m: 100, + k: 5, + phi_f: 0.65, + }; + Self { + total_certificates: 5, + certificates_per_epoch: 1, + protocol_parameters, + } + } + + /// Set the total number of certificates to generate. + pub fn with_total_certificates(mut self, total_certificates: u64) -> Self { + self.total_certificates = total_certificates; + + self + } + + /// Set the number of certificates per epoch. + pub fn with_certificates_per_epoch(mut self, certificates_per_epoch: u64) -> Self { + self.certificates_per_epoch = certificates_per_epoch; + + self + } + + /// Set the protocol parameters. + pub fn with_protocol_parameters(mut self, protocol_parameters: ProtocolParameters) -> Self { + self.protocol_parameters = protocol_parameters; + + self + } + + /// Build the certificate chain. + pub fn build(self) -> (Vec, ProtocolGenesisVerifier) { + let (genesis_signer, genesis_verifier) = CertificateChainBuilder::setup_genesis(); + let epochs = Self::setup_epochs(self.total_certificates, self.certificates_per_epoch); + let fixtures_per_epoch = + Self::setup_fixtures_for_epochs(&epochs, &self.protocol_parameters); + let certificate_chain_length = epochs.len() - 1; + let certificates = epochs + .iter() + .take(certificate_chain_length) + .enumerate() + .map(|(i, epoch)| { + let fixture = fixtures_per_epoch.get(epoch).unwrap(); + let next_fixture = fixtures_per_epoch.get(&(*epoch + 1)).unwrap(); + let context = CertificateChainBuilderContext { + index_certificate: i, + total_certificates: certificate_chain_length, + epoch: *epoch, + fixture, + next_fixture, + }; + match i { + 0 => Self::create_genesis_certificate(&context, &genesis_signer), + _ => Self::create_standard_certificate(&context), + } + }) + .collect::>(); + let certificates_chained = Self::compute_chained_certificates(certificates); + + (certificates_chained, genesis_verifier) + } + + fn compute_clerk_for_signers(signers: &[SignerFixture]) -> ProtocolClerk { + let first_signer = &signers[0].protocol_signer; + + ProtocolClerk::from_signer(first_signer) + } + + fn compute_avk_for_signers(signers: &[SignerFixture]) -> ProtocolAggregateVerificationKey { + let clerk = Self::compute_clerk_for_signers(signers); + + clerk.compute_avk().into() + } + + fn setup_genesis() -> (ProtocolGenesisSigner, ProtocolGenesisVerifier) { + let genesis_signer = ProtocolGenesisSigner::create_deterministic_genesis_signer(); + let genesis_verifier = genesis_signer.create_genesis_verifier(); + + (genesis_signer, genesis_verifier) + } + + fn setup_epochs(total_certificates: u64, certificates_per_epoch: u64) -> Vec { + (1..total_certificates + 2) + .map(|i| match certificates_per_epoch { + 0 => panic!("expected at least 1 certificate per epoch"), + 1 => Epoch(i), + _ => Epoch(i / certificates_per_epoch + 1), + }) + .collect::>() + } + + fn setup_fixtures_for_epochs( + epochs: &[Epoch], + protocol_parameters: &ProtocolParameters, + ) -> HashMap { + let fixtures_per_epoch = epochs + .iter() + .map(|epoch| { + // TODO: set that in the builder configuration? + let total_signers = min(2 + **epoch as usize, 5); + ( + *epoch, + MithrilFixtureBuilder::default() + .with_protocol_parameters(protocol_parameters.to_owned().into()) + .with_signers(total_signers) + .build(), + ) + }) + .collect::>(); + + fixtures_per_epoch + } + + fn create_base_certificate(context: &CertificateChainBuilderContext) -> Certificate { + let index = context.index_certificate; + let epoch = context.epoch; + let immutable_file_number = index as u64 * 10; + let digest = format!("digest{index}"); + let certificate_hash = format!("certificate_hash-{index}"); + let avk = Self::compute_avk_for_signers(&context.fixture.signers_fixture()); + let next_avk = Self::compute_avk_for_signers(&context.next_fixture.signers_fixture()); + let next_protocol_parameters = &context.next_fixture.protocol_parameters(); + let mut base_certificate = fake_data::certificate(certificate_hash); + base_certificate + .protocol_message + .set_message_part(ProtocolMessagePartKey::SnapshotDigest, digest); + base_certificate.protocol_message.set_message_part( + ProtocolMessagePartKey::NextAggregateVerificationKey, + next_avk.to_json_hex().unwrap(), + ); + base_certificate.protocol_message.set_message_part( + ProtocolMessagePartKey::NextProtocolParameters, + next_protocol_parameters.compute_hash(), + ); + base_certificate + .protocol_message + .set_message_part(ProtocolMessagePartKey::CurrentEpoch, epoch.to_string()); + + Certificate { + epoch, + aggregate_verification_key: avk.to_owned(), + previous_hash: "".to_string(), + signed_message: base_certificate.protocol_message.compute_hash(), + #[allow(deprecated)] + metadata: CertificateMetadata { + immutable_file_number, + ..base_certificate.metadata + }, + ..base_certificate + } + } + + fn create_genesis_certificate( + context: &CertificateChainBuilderContext, + genesis_signer: &ProtocolGenesisSigner, + ) -> Certificate { + let epoch = context.epoch; + let certificate = Self::create_base_certificate(context); + let next_avk = Self::compute_avk_for_signers(&context.next_fixture.signers_fixture()); + let next_protocol_parameters = &context.next_fixture.protocol_parameters(); + let genesis_producer = + CertificateGenesisProducer::new(Some(Arc::new(genesis_signer.to_owned()))); + let genesis_protocol_message = CertificateGenesisProducer::create_genesis_protocol_message( + next_protocol_parameters, + &next_avk, + &epoch, + ) + .unwrap(); + let genesis_signature = genesis_producer + .sign_genesis_protocol_message(genesis_protocol_message) + .unwrap(); + + CertificateGenesisProducer::create_genesis_certificate( + certificate.metadata.protocol_parameters, + certificate.metadata.network, + certificate.epoch, + #[allow(deprecated)] + certificate.metadata.immutable_file_number, + next_avk, + genesis_signature, + ) + .unwrap() + } + + fn create_standard_certificate(context: &CertificateChainBuilderContext) -> Certificate { + let fixture = context.fixture; + let mut certificate = Self::create_base_certificate(context); + certificate.metadata.signers = fixture.stake_distribution_parties(); + let single_signatures = fixture + .signers_fixture() + .iter() + .filter_map(|s| { + s.protocol_signer + .sign(certificate.signed_message.as_bytes()) + }) + .collect::>(); + let clerk = CertificateChainBuilder::compute_clerk_for_signers(&fixture.signers_fixture()); + let multi_signature = clerk + .aggregate(&single_signatures, certificate.signed_message.as_bytes()) + .unwrap(); + certificate.signature = CertificateSignature::MultiSignature( + certificate.signed_entity_type(), + multi_signature.into(), + ); + + certificate + } + + fn compute_chained_certificates(certificates: Vec) -> Vec { + let mut certificates_chained: Vec = Vec::new(); + certificates + .iter() + .enumerate() + .for_each(|(i, certificate)| { + let mut certificate_new = certificate.clone(); + if i > 0 { + if let Some(previous_certificate) = certificates_chained.get(i - 1) { + certificate_new.previous_hash = previous_certificate.compute_hash(); + } + } + certificate_new.hash = certificate_new.compute_hash(); + certificates_chained.push(certificate_new); + }); + certificates_chained.reverse(); + + certificates_chained + } +} + +impl Default for CertificateChainBuilder { + fn default() -> Self { + Self::new() + } +} diff --git a/mithril-common/src/test_utils/mod.rs b/mithril-common/src/test_utils/mod.rs index 2a4aeede2ac..e616a18af7a 100644 --- a/mithril-common/src/test_utils/mod.rs +++ b/mithril-common/src/test_utils/mod.rs @@ -14,6 +14,7 @@ pub mod fake_data; pub mod fake_keys; mod cardano_transactions_builder; +mod certificate_chain_builder; mod fixture_builder; mod mithril_fixture; @@ -24,6 +25,7 @@ mod temp_dir; pub mod test_http_server; pub use cardano_transactions_builder::CardanoTransactionsBuilder; +pub use certificate_chain_builder::CertificateChainBuilder; pub use fixture_builder::{MithrilFixtureBuilder, StakeDistributionGenerationMethod}; pub use mithril_fixture::{MithrilFixture, SignerFixture}; pub use temp_dir::*; From 0d19dddf662cab6edcacf59188a00de886150e08 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Fri, 11 Oct 2024 15:29:06 +0200 Subject: [PATCH 02/16] refactor: use CertificateChainBuilder in crypto helper tests setup --- .../src/crypto_helper/tests_setup.rs | 156 +----------------- 1 file changed, 8 insertions(+), 148 deletions(-) diff --git a/mithril-common/src/crypto_helper/tests_setup.rs b/mithril-common/src/crypto_helper/tests_setup.rs index 096d9813ffe..71bb06aa6f3 100644 --- a/mithril-common/src/crypto_helper/tests_setup.rs +++ b/mithril-common/src/crypto_helper/tests_setup.rs @@ -1,18 +1,13 @@ //! Test data builders for Mithril STM types, for testing purpose. use super::{genesis::*, types::*, OpCert, SerDeShelleyFileFormat}; use crate::{ - certificate_chain::CertificateGenesisProducer, - entities::{ - Certificate, CertificateSignature, Epoch, ProtocolMessage, ProtocolMessagePartKey, - SignerWithStake, Stake, - }, - test_utils::{fake_data, MithrilFixtureBuilder, SignerFixture}, + entities::{Certificate, ProtocolMessage, ProtocolMessagePartKey, SignerWithStake, Stake}, + test_utils::{CertificateChainBuilder, SignerFixture}, }; -use crate::entities::{CertificateMetadata, SignedEntityType}; use rand_chacha::ChaCha20Rng; use rand_core::SeedableRng; -use std::{cmp::min, collections::HashMap, fs, path::PathBuf, sync::Arc}; +use std::{fs, path::PathBuf}; /// Create or retrieve a temporary directory for storing cryptographic material for a signer, use this for tests only. pub fn setup_temp_directory_for_signer( @@ -163,150 +158,15 @@ pub fn setup_signers_from_stake_distribution( .collect::<_>() } -/// Instantiate a Genesis Signer and its associated Verifier -pub fn setup_genesis() -> (ProtocolGenesisSigner, ProtocolGenesisVerifier) { - let genesis_signer = ProtocolGenesisSigner::create_deterministic_genesis_signer(); - let genesis_verifier = genesis_signer.create_genesis_verifier(); - (genesis_signer, genesis_verifier) -} - /// Instantiate a certificate chain, use this for tests only. pub fn setup_certificate_chain( total_certificates: u64, certificates_per_epoch: u64, ) -> (Vec, ProtocolGenesisVerifier) { - let genesis_signer = ProtocolGenesisSigner::create_deterministic_genesis_signer(); - let genesis_verifier = genesis_signer.create_genesis_verifier(); - let genesis_producer = CertificateGenesisProducer::new(Some(Arc::new(genesis_signer))); - let protocol_parameters = setup_protocol_parameters(); - let genesis_epoch = Epoch(1); - let mut epochs = (genesis_epoch.0..total_certificates + 2) - .map(|i| match certificates_per_epoch { - 0 => panic!("expected at least 1 certificate per epoch"), - 1 => Epoch(i), - _ => Epoch(i / certificates_per_epoch + 1), - }) - .collect::>(); - let fixture_per_epoch = epochs - .clone() - .into_iter() - .map(|epoch| { - ( - epoch, - MithrilFixtureBuilder::default() - .with_protocol_parameters(protocol_parameters.into()) - .with_signers(min(2 + *epoch as usize, 5)) - .build(), - ) - }) - .collect::>(); - let clerk_for_signers = |signers: &[SignerFixture]| -> ProtocolClerk { - let first_signer = &signers[0].protocol_signer; - ProtocolClerk::from_signer(first_signer) - }; - let avk_for_signers = |signers: &[SignerFixture]| -> ProtocolAggregateVerificationKey { - let clerk = clerk_for_signers(signers); - clerk.compute_avk().into() - }; - epochs.pop(); - let certificates = epochs - .into_iter() - .enumerate() - .map(|(i, epoch)| { - let immutable_file_number = i as u64 * 10; - let digest = format!("digest{i}"); - let certificate_hash = format!("certificate_hash-{i}"); - let fixture = fixture_per_epoch.get(&epoch).unwrap(); - let next_fixture = fixture_per_epoch.get(&(epoch + 1)).unwrap(); - let avk = avk_for_signers(&fixture.signers_fixture()); - let next_avk = avk_for_signers(&next_fixture.signers_fixture()); - let next_protocol_parameters = &next_fixture.protocol_parameters(); - let mut fake_certificate = { - let mut base_certificate = fake_data::certificate(certificate_hash); - base_certificate - .protocol_message - .set_message_part(ProtocolMessagePartKey::SnapshotDigest, digest); - base_certificate.protocol_message.set_message_part( - ProtocolMessagePartKey::NextAggregateVerificationKey, - next_avk.to_json_hex().unwrap(), - ); - Certificate { - epoch, - aggregate_verification_key: avk, - previous_hash: "".to_string(), - signed_message: base_certificate.protocol_message.compute_hash(), - #[allow(deprecated)] - metadata: CertificateMetadata { - immutable_file_number, - ..base_certificate.metadata - }, - ..base_certificate - } - }; + let certificate_chain_builder = CertificateChainBuilder::new() + .with_total_certificates(total_certificates) + .with_certificates_per_epoch(certificates_per_epoch) + .with_protocol_parameters(setup_protocol_parameters()); - let beacon = fake_certificate.as_cardano_db_beacon(); - match i { - 0 => { - let genesis_protocol_message = - CertificateGenesisProducer::create_genesis_protocol_message( - next_protocol_parameters, - &next_avk, - &genesis_epoch, - ) - .unwrap(); - let genesis_signature = genesis_producer - .sign_genesis_protocol_message(genesis_protocol_message) - .unwrap(); - fake_certificate = CertificateGenesisProducer::create_genesis_certificate( - fake_certificate.metadata.protocol_parameters, - beacon.network, - beacon.epoch, - beacon.immutable_file_number, - next_avk, - genesis_signature, - ) - .unwrap() - } - _ => { - fake_certificate.metadata.signers = fixture.stake_distribution_parties(); - let single_signatures = fixture - .signers_fixture() - .iter() - .filter_map(|s| { - s.protocol_signer - .sign(fake_certificate.signed_message.as_bytes()) - }) - .collect::>(); - let clerk = clerk_for_signers(&fixture.signers_fixture()); - let multi_signature = clerk - .aggregate( - &single_signatures, - fake_certificate.signed_message.as_bytes(), - ) - .unwrap(); - fake_certificate.signature = CertificateSignature::MultiSignature( - SignedEntityType::CardanoImmutableFilesFull(beacon), - multi_signature.into(), - ); - } - } - fake_certificate - }) - .collect::>(); - let mut certificates_new: Vec = Vec::new(); - certificates - .iter() - .enumerate() - .for_each(|(i, certificate)| { - let mut certificate_new = certificate.clone(); - if i > 0 { - if let Some(previous_certificate) = certificates_new.get(i - 1) { - certificate_new.previous_hash = previous_certificate.compute_hash(); - } - } - certificate_new.hash = certificate_new.compute_hash(); - certificates_new.push(certificate_new); - }); - certificates_new.reverse(); - (certificates_new, genesis_verifier) + certificate_chain_builder.build() } From c6b5bda3f2bc07d4074e060aa0878d07711bab6a Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Fri, 11 Oct 2024 15:46:23 +0200 Subject: [PATCH 03/16] feat: add support for certificate chain alteration in CertificateChainBuilder --- .../test_utils/certificate_chain_builder.rs | 101 +++++++++++++++--- 1 file changed, 89 insertions(+), 12 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index b5b0bce7ad3..09942d77c73 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -1,17 +1,24 @@ -use std::{cmp::min, collections::HashMap, sync::Arc}; - use crate::{ certificate_chain::CertificateGenesisProducer, crypto_helper::{ ProtocolAggregateVerificationKey, ProtocolClerk, ProtocolGenesisSigner, ProtocolGenesisVerifier, ProtocolParameters, }, - entities::{ - Certificate, CertificateMetadata, CertificateSignature, Epoch, ProtocolMessagePartKey, - }, + entities::{Certificate, CertificateSignature, Epoch, ProtocolMessagePartKey}, test_utils::{fake_data, MithrilFixture, MithrilFixtureBuilder, SignerFixture}, }; +use crate::entities::CertificateMetadata; +use std::{cmp::min, collections::HashMap, sync::Arc}; + +/// Genesis certificate post builder function type. For tests only. +pub type GenesisCertificatePostBuilderFunc = + dyn Fn(Certificate, &CertificateChainBuilderContext, &ProtocolGenesisSigner) -> Certificate; + +/// Standard certificate post builder function type. For tests only. +pub type StandardCertificatePostBuilderFunc = + dyn Fn(Certificate, &CertificateChainBuilderContext) -> Certificate; + /// Context used while building a certificate chain. For tests only. pub struct CertificateChainBuilderContext<'a> { pub index_certificate: usize, @@ -24,9 +31,24 @@ pub struct CertificateChainBuilderContext<'a> { /// A builder for creating a certificate chain. For tests only. /// -/// # Example usage +/// # Example usage for building a fully valid certificate chain +/// +/// ``` +/// use mithril_common::crypto_helper::ProtocolParameters; +/// use mithril_common::test_utils::CertificateChainBuilder; +/// +/// let (certificate_chain, _protocol_genesis_verifier) = CertificateChainBuilder::new() +/// .with_total_certificates(5) +/// .with_certificates_per_epoch(2) +/// .build(); +/// +/// assert_eq!(5, certificate_chain.len()); +/// ``` +/// +/// # Example usage for building an adversarial certificate chain /// /// ``` +/// use mithril_common::entities::Epoch; /// use mithril_common::crypto_helper::ProtocolParameters; /// use mithril_common::test_utils::CertificateChainBuilder; /// @@ -37,17 +59,27 @@ pub struct CertificateChainBuilderContext<'a> { /// m: 100, /// k: 5, /// phi_f: 0.65, -/// }).build(); +/// }).with_standard_certificate_post_builder(&|certificate, context| { +/// let mut certificate = certificate; +/// // Tamper the epoch of the last certificate +/// if context.index_certificate == context.total_certificates - 1 { +/// certificate.epoch = Epoch(123); +/// } +/// +/// certificate +/// }).build(); /// /// assert_eq!(5, certificate_chain.len()); /// ``` -pub struct CertificateChainBuilder { +pub struct CertificateChainBuilder<'a> { total_certificates: u64, certificates_per_epoch: u64, protocol_parameters: ProtocolParameters, + genesis_certificate_post_builder: Option<&'a GenesisCertificatePostBuilderFunc>, + standard_certificate_post_builder: Option<&'a StandardCertificatePostBuilderFunc>, } -impl CertificateChainBuilder { +impl<'a> CertificateChainBuilder<'a> { /// Create a new [CertificateChainBuilder] instance. pub fn new() -> Self { let protocol_parameters = ProtocolParameters { @@ -59,6 +91,8 @@ impl CertificateChainBuilder { total_certificates: 5, certificates_per_epoch: 1, protocol_parameters, + genesis_certificate_post_builder: None, + standard_certificate_post_builder: None, } } @@ -83,6 +117,26 @@ impl CertificateChainBuilder { self } + /// Set the genesis certificate post builder. + pub fn with_genesis_certificate_post_builder( + mut self, + genesis_certificate_post_builder: &'a GenesisCertificatePostBuilderFunc, + ) -> Self { + self.genesis_certificate_post_builder = Some(genesis_certificate_post_builder); + + self + } + + /// Set the standard certificate post builder. + pub fn with_standard_certificate_post_builder( + mut self, + standard_certificate_post_builder: &'a StandardCertificatePostBuilderFunc, + ) -> Self { + self.standard_certificate_post_builder = Some(standard_certificate_post_builder); + + self + } + /// Build the certificate chain. pub fn build(self) -> (Vec, ProtocolGenesisVerifier) { let (genesis_signer, genesis_verifier) = CertificateChainBuilder::setup_genesis(); @@ -105,8 +159,31 @@ impl CertificateChainBuilder { next_fixture, }; match i { - 0 => Self::create_genesis_certificate(&context, &genesis_signer), - _ => Self::create_standard_certificate(&context), + 0 => { + let mut certificate = + Self::create_genesis_certificate(&context, &genesis_signer); + if let Some(genesis_certificate_post_builder) = + self.genesis_certificate_post_builder + { + certificate = genesis_certificate_post_builder( + certificate, + &context, + &genesis_signer, + ); + } + + certificate + } + _ => { + let mut certificate = Self::create_standard_certificate(&context); + if let Some(standard_certificate_post_builder) = + self.standard_certificate_post_builder + { + certificate = standard_certificate_post_builder(certificate, &context); + } + + certificate + } } }) .collect::>(); @@ -282,7 +359,7 @@ impl CertificateChainBuilder { } } -impl Default for CertificateChainBuilder { +impl<'a> Default for CertificateChainBuilder<'a> { fn default() -> Self { Self::new() } From 381975965163b807a1f3367d90282a500503ec67 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Fri, 11 Oct 2024 16:03:59 +0200 Subject: [PATCH 04/16] refactor: simplify chain alteration processor in CertificateChainBuilder --- .../test_utils/certificate_chain_builder.rs | 85 ++++++++++--------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 09942d77c73..4a66a5efc1b 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -11,12 +11,12 @@ use crate::{ use crate::entities::CertificateMetadata; use std::{cmp::min, collections::HashMap, sync::Arc}; -/// Genesis certificate post builder function type. For tests only. -pub type GenesisCertificatePostBuilderFunc = +/// Genesis certificate processor function type. For tests only. +pub type GenesisCertificateProcessorFunc = dyn Fn(Certificate, &CertificateChainBuilderContext, &ProtocolGenesisSigner) -> Certificate; -/// Standard certificate post builder function type. For tests only. -pub type StandardCertificatePostBuilderFunc = +/// Standard certificate processor function type. For tests only. +pub type StandardCertificateProcessorFunc = dyn Fn(Certificate, &CertificateChainBuilderContext) -> Certificate; /// Context used while building a certificate chain. For tests only. @@ -59,7 +59,7 @@ pub struct CertificateChainBuilderContext<'a> { /// m: 100, /// k: 5, /// phi_f: 0.65, -/// }).with_standard_certificate_post_builder(&|certificate, context| { +/// }).with_standard_certificate_processor(&|certificate, context| { /// let mut certificate = certificate; /// // Tamper the epoch of the last certificate /// if context.index_certificate == context.total_certificates - 1 { @@ -75,8 +75,8 @@ pub struct CertificateChainBuilder<'a> { total_certificates: u64, certificates_per_epoch: u64, protocol_parameters: ProtocolParameters, - genesis_certificate_post_builder: Option<&'a GenesisCertificatePostBuilderFunc>, - standard_certificate_post_builder: Option<&'a StandardCertificatePostBuilderFunc>, + genesis_certificate_processor: &'a GenesisCertificateProcessorFunc, + standard_certificate_processor: &'a StandardCertificateProcessorFunc, } impl<'a> CertificateChainBuilder<'a> { @@ -91,8 +91,8 @@ impl<'a> CertificateChainBuilder<'a> { total_certificates: 5, certificates_per_epoch: 1, protocol_parameters, - genesis_certificate_post_builder: None, - standard_certificate_post_builder: None, + genesis_certificate_processor: &|certificate, _, _| certificate, + standard_certificate_processor: &|certificate, _| certificate, } } @@ -117,22 +117,22 @@ impl<'a> CertificateChainBuilder<'a> { self } - /// Set the genesis certificate post builder. - pub fn with_genesis_certificate_post_builder( + /// Set the genesis certificate processor. + pub fn with_genesis_certificate_processor( mut self, - genesis_certificate_post_builder: &'a GenesisCertificatePostBuilderFunc, + genesis_certificate_processor: &'a GenesisCertificateProcessorFunc, ) -> Self { - self.genesis_certificate_post_builder = Some(genesis_certificate_post_builder); + self.genesis_certificate_processor = genesis_certificate_processor; self } - /// Set the standard certificate post builder. - pub fn with_standard_certificate_post_builder( + /// Set the standard certificate processor. + pub fn with_standard_certificate_processor( mut self, - standard_certificate_post_builder: &'a StandardCertificatePostBuilderFunc, + standard_certificate_processor: &'a StandardCertificateProcessorFunc, ) -> Self { - self.standard_certificate_post_builder = Some(standard_certificate_post_builder); + self.standard_certificate_processor = standard_certificate_processor; self } @@ -143,6 +143,8 @@ impl<'a> CertificateChainBuilder<'a> { let epochs = Self::setup_epochs(self.total_certificates, self.certificates_per_epoch); let fixtures_per_epoch = Self::setup_fixtures_for_epochs(&epochs, &self.protocol_parameters); + let genesis_certificate_processor = self.genesis_certificate_processor; + let standard_certificate_processor = self.standard_certificate_processor; let certificate_chain_length = epochs.len() - 1; let certificates = epochs .iter() @@ -160,29 +162,15 @@ impl<'a> CertificateChainBuilder<'a> { }; match i { 0 => { - let mut certificate = + let certificate = Self::create_genesis_certificate(&context, &genesis_signer); - if let Some(genesis_certificate_post_builder) = - self.genesis_certificate_post_builder - { - certificate = genesis_certificate_post_builder( - certificate, - &context, - &genesis_signer, - ); - } - - certificate + + genesis_certificate_processor(certificate, &context, &genesis_signer) } _ => { - let mut certificate = Self::create_standard_certificate(&context); - if let Some(standard_certificate_post_builder) = - self.standard_certificate_post_builder - { - certificate = standard_certificate_post_builder(certificate, &context); - } - - certificate + let certificate = Self::create_standard_certificate(&context); + + standard_certificate_processor(certificate, &context) } } }) @@ -244,11 +232,11 @@ impl<'a> CertificateChainBuilder<'a> { } fn create_base_certificate(context: &CertificateChainBuilderContext) -> Certificate { - let index = context.index_certificate; + let index_certificate = context.index_certificate; let epoch = context.epoch; - let immutable_file_number = index as u64 * 10; - let digest = format!("digest{index}"); - let certificate_hash = format!("certificate_hash-{index}"); + let immutable_file_number = index_certificate as u64 * 10; + let digest = format!("digest{index_certificate}"); + let certificate_hash = format!("certificate_hash-{index_certificate}"); let avk = Self::compute_avk_for_signers(&context.fixture.signers_fixture()); let next_avk = Self::compute_avk_for_signers(&context.next_fixture.signers_fixture()); let next_protocol_parameters = &context.next_fixture.protocol_parameters(); @@ -364,3 +352,18 @@ impl<'a> Default for CertificateChainBuilder<'a> { Self::new() } } + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn certificate_chain_has_correct_length() { + let (certificate_chain, _) = CertificateChainBuilder::new() + .with_total_certificates(5) + .with_certificates_per_epoch(2) + .build(); + + assert_eq!(5, certificate_chain.len()); + } +} From d255561a323b8292a6c93fc65f6d64d861771a2c Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Fri, 11 Oct 2024 16:24:00 +0200 Subject: [PATCH 05/16] refactor: enhance CertificateChainBuilder implementation --- .../test_utils/certificate_chain_builder.rs | 83 ++++++++++--------- 1 file changed, 42 insertions(+), 41 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 4a66a5efc1b..5c7b2e53ac7 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -140,42 +140,39 @@ impl<'a> CertificateChainBuilder<'a> { /// Build the certificate chain. pub fn build(self) -> (Vec, ProtocolGenesisVerifier) { let (genesis_signer, genesis_verifier) = CertificateChainBuilder::setup_genesis(); - let epochs = Self::setup_epochs(self.total_certificates, self.certificates_per_epoch); - let fixtures_per_epoch = - Self::setup_fixtures_for_epochs(&epochs, &self.protocol_parameters); + let epochs = self.build_epochs(); + let fixtures_per_epoch = self.build_fixtures_for_epochs(&epochs); let genesis_certificate_processor = self.genesis_certificate_processor; let standard_certificate_processor = self.standard_certificate_processor; let certificate_chain_length = epochs.len() - 1; let certificates = epochs - .iter() + .into_iter() .take(certificate_chain_length) .enumerate() .map(|(i, epoch)| { - let fixture = fixtures_per_epoch.get(epoch).unwrap(); - let next_fixture = fixtures_per_epoch.get(&(*epoch + 1)).unwrap(); + let fixture = fixtures_per_epoch.get(&epoch).unwrap(); + let next_fixture = fixtures_per_epoch.get(&epoch.next()).unwrap(); let context = CertificateChainBuilderContext { index_certificate: i, total_certificates: certificate_chain_length, - epoch: *epoch, + epoch, fixture, next_fixture, }; match i { - 0 => { - let certificate = - Self::create_genesis_certificate(&context, &genesis_signer); - - genesis_certificate_processor(certificate, &context, &genesis_signer) - } - _ => { - let certificate = Self::create_standard_certificate(&context); - - standard_certificate_processor(certificate, &context) - } + 0 => genesis_certificate_processor( + self.build_genesis_certificate(&context, &genesis_signer), + &context, + &genesis_signer, + ), + _ => standard_certificate_processor( + self.build_standard_certificate(&context), + &context, + ), } }) .collect::>(); - let certificates_chained = Self::compute_chained_certificates(certificates); + let certificates_chained = self.compute_chained_certificates(certificates); (certificates_chained, genesis_verifier) } @@ -199,7 +196,9 @@ impl<'a> CertificateChainBuilder<'a> { (genesis_signer, genesis_verifier) } - fn setup_epochs(total_certificates: u64, certificates_per_epoch: u64) -> Vec { + fn build_epochs(&self) -> Vec { + let total_certificates = self.total_certificates; + let certificates_per_epoch = self.certificates_per_epoch; (1..total_certificates + 2) .map(|i| match certificates_per_epoch { 0 => panic!("expected at least 1 certificate per epoch"), @@ -209,19 +208,17 @@ impl<'a> CertificateChainBuilder<'a> { .collect::>() } - fn setup_fixtures_for_epochs( - epochs: &[Epoch], - protocol_parameters: &ProtocolParameters, - ) -> HashMap { + fn build_fixtures_for_epochs(&self, epochs: &[Epoch]) -> HashMap { let fixtures_per_epoch = epochs .iter() .map(|epoch| { // TODO: set that in the builder configuration? let total_signers = min(2 + **epoch as usize, 5); + let protocol_parameters = self.protocol_parameters.to_owned().into(); ( *epoch, MithrilFixtureBuilder::default() - .with_protocol_parameters(protocol_parameters.to_owned().into()) + .with_protocol_parameters(protocol_parameters) .with_signers(total_signers) .build(), ) @@ -231,7 +228,7 @@ impl<'a> CertificateChainBuilder<'a> { fixtures_per_epoch } - fn create_base_certificate(context: &CertificateChainBuilderContext) -> Certificate { + fn build_base_certificate(&self, context: &CertificateChainBuilderContext) -> Certificate { let index_certificate = context.index_certificate; let epoch = context.epoch; let immutable_file_number = index_certificate as u64 * 10; @@ -239,43 +236,45 @@ impl<'a> CertificateChainBuilder<'a> { let certificate_hash = format!("certificate_hash-{index_certificate}"); let avk = Self::compute_avk_for_signers(&context.fixture.signers_fixture()); let next_avk = Self::compute_avk_for_signers(&context.next_fixture.signers_fixture()); + let protocol_parameters = context.fixture.protocol_parameters().to_owned(); let next_protocol_parameters = &context.next_fixture.protocol_parameters(); - let mut base_certificate = fake_data::certificate(certificate_hash); - base_certificate - .protocol_message - .set_message_part(ProtocolMessagePartKey::SnapshotDigest, digest); - base_certificate.protocol_message.set_message_part( + let base_certificate = fake_data::certificate(certificate_hash); + let mut protocol_message = base_certificate.protocol_message; + protocol_message.set_message_part(ProtocolMessagePartKey::SnapshotDigest, digest); + protocol_message.set_message_part( ProtocolMessagePartKey::NextAggregateVerificationKey, next_avk.to_json_hex().unwrap(), ); - base_certificate.protocol_message.set_message_part( + protocol_message.set_message_part( ProtocolMessagePartKey::NextProtocolParameters, next_protocol_parameters.compute_hash(), ); - base_certificate - .protocol_message - .set_message_part(ProtocolMessagePartKey::CurrentEpoch, epoch.to_string()); + protocol_message.set_message_part(ProtocolMessagePartKey::CurrentEpoch, epoch.to_string()); + let signed_message = protocol_message.compute_hash(); Certificate { epoch, aggregate_verification_key: avk.to_owned(), previous_hash: "".to_string(), - signed_message: base_certificate.protocol_message.compute_hash(), + protocol_message, + signed_message, #[allow(deprecated)] metadata: CertificateMetadata { immutable_file_number, + protocol_parameters, ..base_certificate.metadata }, ..base_certificate } } - fn create_genesis_certificate( + fn build_genesis_certificate( + &self, context: &CertificateChainBuilderContext, genesis_signer: &ProtocolGenesisSigner, ) -> Certificate { let epoch = context.epoch; - let certificate = Self::create_base_certificate(context); + let certificate = self.build_base_certificate(context); let next_avk = Self::compute_avk_for_signers(&context.next_fixture.signers_fixture()); let next_protocol_parameters = &context.next_fixture.protocol_parameters(); let genesis_producer = @@ -302,9 +301,9 @@ impl<'a> CertificateChainBuilder<'a> { .unwrap() } - fn create_standard_certificate(context: &CertificateChainBuilderContext) -> Certificate { + fn build_standard_certificate(&self, context: &CertificateChainBuilderContext) -> Certificate { let fixture = context.fixture; - let mut certificate = Self::create_base_certificate(context); + let mut certificate = self.build_base_certificate(context); certificate.metadata.signers = fixture.stake_distribution_parties(); let single_signatures = fixture .signers_fixture() @@ -326,7 +325,7 @@ impl<'a> CertificateChainBuilder<'a> { certificate } - fn compute_chained_certificates(certificates: Vec) -> Vec { + fn compute_chained_certificates(&self, certificates: Vec) -> Vec { let mut certificates_chained: Vec = Vec::new(); certificates .iter() @@ -337,6 +336,8 @@ impl<'a> CertificateChainBuilder<'a> { if let Some(previous_certificate) = certificates_chained.get(i - 1) { certificate_new.previous_hash = previous_certificate.compute_hash(); } + } else { + certificate_new.previous_hash = "".to_string(); } certificate_new.hash = certificate_new.compute_hash(); certificates_chained.push(certificate_new); From a944266bb02f8886b00a0caecca518e2d7e9d8cd Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Fri, 11 Oct 2024 16:44:12 +0200 Subject: [PATCH 06/16] feat: add support for number of signers per epoch processor in CertificateChainBuilder --- .../test_utils/certificate_chain_builder.rs | 58 ++++++++++++++----- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 5c7b2e53ac7..a71023e754e 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -19,6 +19,9 @@ pub type GenesisCertificateProcessorFunc = pub type StandardCertificateProcessorFunc = dyn Fn(Certificate, &CertificateChainBuilderContext) -> Certificate; +/// Total signers per epoch processor function type. For tests only. +pub type TotalSignersPerEpochProcessorFunc = dyn Fn(Epoch) -> usize; + /// Context used while building a certificate chain. For tests only. pub struct CertificateChainBuilderContext<'a> { pub index_certificate: usize, @@ -31,7 +34,7 @@ pub struct CertificateChainBuilderContext<'a> { /// A builder for creating a certificate chain. For tests only. /// -/// # Example usage for building a fully valid certificate chain +/// # Simple example usage for building a fully valid certificate chain /// /// ``` /// use mithril_common::crypto_helper::ProtocolParameters; @@ -45,10 +48,10 @@ pub struct CertificateChainBuilderContext<'a> { /// assert_eq!(5, certificate_chain.len()); /// ``` /// -/// # Example usage for building an adversarial certificate chain +/// # More complex example usage for building a fully valid certificate chain /// /// ``` -/// use mithril_common::entities::Epoch; +/// use std::cmp::min; /// use mithril_common::crypto_helper::ProtocolParameters; /// use mithril_common::test_utils::CertificateChainBuilder; /// @@ -59,7 +62,24 @@ pub struct CertificateChainBuilderContext<'a> { /// m: 100, /// k: 5, /// phi_f: 0.65, -/// }).with_standard_certificate_processor(&|certificate, context| { +/// }) +/// .with_total_signers_per_epoch_processor(&|epoch| min(1 + *epoch as usize, 10)) +/// .build(); +/// +/// assert_eq!(5, certificate_chain.len()); +/// ``` +/// +/// # Advanced example usage for building an adversarial certificate chain +/// +/// ``` +/// use mithril_common::entities::Epoch; +/// use mithril_common::crypto_helper::ProtocolParameters; +/// use mithril_common::test_utils::CertificateChainBuilder; +/// +/// let (certificate_chain, _protocol_genesis_verifier) = CertificateChainBuilder::new() +/// .with_total_certificates(5) +/// .with_certificates_per_epoch(2) +/// .with_standard_certificate_processor(&|certificate, context| { /// let mut certificate = certificate; /// // Tamper the epoch of the last certificate /// if context.index_certificate == context.total_certificates - 1 { @@ -67,7 +87,8 @@ pub struct CertificateChainBuilderContext<'a> { /// } /// /// certificate -/// }).build(); +/// }) +/// .build(); /// /// assert_eq!(5, certificate_chain.len()); /// ``` @@ -75,6 +96,7 @@ pub struct CertificateChainBuilder<'a> { total_certificates: u64, certificates_per_epoch: u64, protocol_parameters: ProtocolParameters, + total_signers_per_epoch_processor: &'a TotalSignersPerEpochProcessorFunc, genesis_certificate_processor: &'a GenesisCertificateProcessorFunc, standard_certificate_processor: &'a StandardCertificateProcessorFunc, } @@ -91,6 +113,7 @@ impl<'a> CertificateChainBuilder<'a> { total_certificates: 5, certificates_per_epoch: 1, protocol_parameters, + total_signers_per_epoch_processor: &|epoch| min(2 + *epoch as usize, 5), genesis_certificate_processor: &|certificate, _, _| certificate, standard_certificate_processor: &|certificate, _| certificate, } @@ -117,6 +140,16 @@ impl<'a> CertificateChainBuilder<'a> { self } + /// Set the total signers per epoch processor. + pub fn with_total_signers_per_epoch_processor( + mut self, + total_signers_per_epoch_processor: &'a TotalSignersPerEpochProcessorFunc, + ) -> Self { + self.total_signers_per_epoch_processor = total_signers_per_epoch_processor; + + self + } + /// Set the genesis certificate processor. pub fn with_genesis_certificate_processor( mut self, @@ -144,22 +177,22 @@ impl<'a> CertificateChainBuilder<'a> { let fixtures_per_epoch = self.build_fixtures_for_epochs(&epochs); let genesis_certificate_processor = self.genesis_certificate_processor; let standard_certificate_processor = self.standard_certificate_processor; - let certificate_chain_length = epochs.len() - 1; + let total_certificates = epochs.len() - 1; let certificates = epochs .into_iter() - .take(certificate_chain_length) + .take(total_certificates) .enumerate() - .map(|(i, epoch)| { + .map(|(index_certificate, epoch)| { let fixture = fixtures_per_epoch.get(&epoch).unwrap(); let next_fixture = fixtures_per_epoch.get(&epoch.next()).unwrap(); let context = CertificateChainBuilderContext { - index_certificate: i, - total_certificates: certificate_chain_length, + index_certificate, + total_certificates, epoch, fixture, next_fixture, }; - match i { + match index_certificate { 0 => genesis_certificate_processor( self.build_genesis_certificate(&context, &genesis_signer), &context, @@ -212,8 +245,7 @@ impl<'a> CertificateChainBuilder<'a> { let fixtures_per_epoch = epochs .iter() .map(|epoch| { - // TODO: set that in the builder configuration? - let total_signers = min(2 + **epoch as usize, 5); + let total_signers = (self.total_signers_per_epoch_processor)(*epoch); let protocol_parameters = self.protocol_parameters.to_owned().into(); ( *epoch, From 91ce6a516461b158a27b86003c541c6e91180422 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Fri, 11 Oct 2024 18:37:18 +0200 Subject: [PATCH 07/16] feat: enhance tests for CertificateChainBuilder --- .../test_utils/certificate_chain_builder.rs | 224 +++++++++++++++++- 1 file changed, 223 insertions(+), 1 deletion(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index a71023e754e..8f6e1d21650 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -388,10 +388,14 @@ impl<'a> Default for CertificateChainBuilder<'a> { #[cfg(test)] mod test { + use std::collections::BTreeMap; + + use crate::entities::{self, ProtocolMessage}; + use super::*; #[test] - fn certificate_chain_has_correct_length() { + fn builds_certificate_chain_with_correct_length() { let (certificate_chain, _) = CertificateChainBuilder::new() .with_total_certificates(5) .with_certificates_per_epoch(2) @@ -399,4 +403,222 @@ mod test { assert_eq!(5, certificate_chain.len()); } + + #[test] + fn builds_valid_epochs() { + let expected_epochs = vec![Epoch(1), Epoch(2), Epoch(2), Epoch(3), Epoch(3), Epoch(4)]; + + let epochs = CertificateChainBuilder::default() + .with_total_certificates(5) + .with_certificates_per_epoch(2) + .build_epochs(); + + assert_eq!(expected_epochs, epochs); + } + + #[test] + fn builds_valid_fixtures_per_epochs() { + let expected_total_signers = (1..=6).collect::>(); + let certificate_chain_builder = CertificateChainBuilder::default() + .with_total_certificates(5) + .with_certificates_per_epoch(1) + .with_total_signers_per_epoch_processor(&|epoch| *epoch as usize); + let epochs = certificate_chain_builder.build_epochs(); + + let epoch_fixtures = + BTreeMap::from_iter(certificate_chain_builder.build_fixtures_for_epochs(&epochs)); + + let total_signers = epoch_fixtures + .into_values() + .map(|fixture| fixture.signers().len()) + .collect::>(); + assert_eq!(expected_total_signers, total_signers); + } + + #[test] + fn builds_valid_genesis_certificate() { + let expected_protocol_parameters = ProtocolParameters { + m: 123, + k: 45, + phi_f: 0.67, + }; + let fixture = MithrilFixtureBuilder::default() + .with_protocol_parameters(expected_protocol_parameters.into()) + .with_signers(2) + .build(); + let next_fixture = MithrilFixtureBuilder::default() + .with_protocol_parameters(expected_protocol_parameters.into()) + .with_signers(3) + .build(); + let next_avk = next_fixture.compute_and_encode_avk(); + let context = CertificateChainBuilderContext { + index_certificate: 0, + total_certificates: 5, + epoch: Epoch(1), + fixture: &fixture, + next_fixture: &next_fixture, + }; + let mut expected_protocol_message = ProtocolMessage::new(); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::NextAggregateVerificationKey, + next_avk, + ); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::NextProtocolParameters, + Into::::into(expected_protocol_parameters).compute_hash(), + ); + expected_protocol_message + .set_message_part(ProtocolMessagePartKey::CurrentEpoch, "1".to_string()); + let expected_signed_message = expected_protocol_message.compute_hash(); + let (protocol_genesis_signer, _) = CertificateChainBuilder::setup_genesis(); + + let genesis_certificate = CertificateChainBuilder::default() + .with_protocol_parameters(expected_protocol_parameters) + .build_genesis_certificate(&context, &protocol_genesis_signer); + + assert!(genesis_certificate.is_genesis()); + assert_eq!(Epoch(1), genesis_certificate.epoch); + assert_eq!( + expected_protocol_parameters, + genesis_certificate.metadata.protocol_parameters.into() + ); + assert_eq!(0, genesis_certificate.metadata.signers.len()); + assert_eq!( + expected_protocol_message, + genesis_certificate.protocol_message + ); + assert_eq!(expected_signed_message, genesis_certificate.signed_message); + } + + #[test] + fn builds_valid_standard_certificate() { + let expected_protocol_parameters = ProtocolParameters { + m: 123, + k: 45, + phi_f: 0.67, + }; + let fixture = MithrilFixtureBuilder::default() + .with_protocol_parameters(expected_protocol_parameters.into()) + .with_signers(2) + .build(); + let next_fixture = MithrilFixtureBuilder::default() + .with_protocol_parameters(expected_protocol_parameters.into()) + .with_signers(3) + .build(); + let avk = fixture.compute_and_encode_avk(); + let next_avk = next_fixture.compute_and_encode_avk(); + let context = CertificateChainBuilderContext { + index_certificate: 0, + total_certificates: 5, + epoch: Epoch(1), + fixture: &fixture, + next_fixture: &next_fixture, + }; + let mut expected_protocol_message = ProtocolMessage::new(); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::SnapshotDigest, + "digest0".to_string(), + ); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::NextAggregateVerificationKey, + next_avk, + ); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::NextProtocolParameters, + Into::::into(expected_protocol_parameters).compute_hash(), + ); + expected_protocol_message + .set_message_part(ProtocolMessagePartKey::CurrentEpoch, "1".to_string()); + let expected_signed_message = expected_protocol_message.compute_hash(); + + let standard_certificate = CertificateChainBuilder::default() + .with_protocol_parameters(expected_protocol_parameters) + .build_standard_certificate(&context); + + assert!(!standard_certificate.is_genesis()); + assert_eq!(Epoch(1), standard_certificate.epoch); + assert_eq!( + expected_protocol_parameters, + standard_certificate.metadata.protocol_parameters.into() + ); + assert_eq!(2, standard_certificate.metadata.signers.len()); + assert_eq!( + expected_protocol_message, + standard_certificate.protocol_message + ); + assert_eq!(expected_signed_message, standard_certificate.signed_message); + assert_eq!( + avk, + standard_certificate + .aggregate_verification_key + .to_json_hex() + .unwrap() + ); + } + + #[test] + fn builds_certificate_chain_correctly_chained() { + let certificates = vec![ + fake_data::certificate("cert-1".to_string()), + fake_data::certificate("cert-2".to_string()), + fake_data::certificate("cert-3".to_string()), + ]; + + let certificates_chained = + CertificateChainBuilder::default().compute_chained_certificates(certificates); + + assert_eq!("", certificates_chained[2].previous_hash); + assert_eq!( + certificates_chained[2].hash, + certificates_chained[1].previous_hash + ); + assert_eq!( + certificates_chained[1].hash, + certificates_chained[0].previous_hash + ); + } + + #[test] + fn builds_certificate_chain_with_alteration_on_genesis_certificate() { + let (certificates, _) = CertificateChainBuilder::new() + .with_total_certificates(5) + .with_genesis_certificate_processor(&|certificate, _, _| { + let mut certificate = certificate; + certificate.signed_message = "altered_msg".to_string(); + + certificate + }) + .build(); + + assert_eq!( + "altered_msg".to_string(), + certificates.last().unwrap().signed_message + ); + } + + #[test] + fn builds_certificate_chain_with_alteration_on_standard_certificates() { + let total_certificates = 5; + let expected_signed_messages = (1..total_certificates) + .rev() + .map(|i| format!("altered-msg-{}", i)) + .collect::>(); + + let (certificates, _) = CertificateChainBuilder::new() + .with_total_certificates(total_certificates) + .with_standard_certificate_processor(&|certificate, context| { + let mut certificate = certificate; + certificate.signed_message = format!("altered-msg-{}", context.index_certificate); + + certificate + }) + .build(); + + let signed_message = certificates + .into_iter() + .take(total_certificates as usize - 1) + .map(|certificate| certificate.signed_message) + .collect::>(); + assert_eq!(expected_signed_messages, signed_message); + } } From 391ac54ba29e72f237914389b3b641360e3225a6 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Mon, 14 Oct 2024 17:53:48 +0200 Subject: [PATCH 08/16] refactor: make CertificateChainBuilderContext compute seed protocol message --- .../test_utils/certificate_chain_builder.rs | 127 ++++++++++++------ 1 file changed, 84 insertions(+), 43 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 8f6e1d21650..7cb183c694d 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -4,7 +4,7 @@ use crate::{ ProtocolAggregateVerificationKey, ProtocolClerk, ProtocolGenesisSigner, ProtocolGenesisVerifier, ProtocolParameters, }, - entities::{Certificate, CertificateSignature, Epoch, ProtocolMessagePartKey}, + entities::{Certificate, CertificateSignature, Epoch, ProtocolMessage, ProtocolMessagePartKey}, test_utils::{fake_data, MithrilFixture, MithrilFixtureBuilder, SignerFixture}, }; @@ -32,6 +32,25 @@ pub struct CertificateChainBuilderContext<'a> { pub next_fixture: &'a MithrilFixture, } +impl CertificateChainBuilderContext<'_> { + /// Computes the protocol message seed. + pub fn compute_protocol_message_seed(&self) -> ProtocolMessage { + let mut protocol_message = ProtocolMessage::new(); + protocol_message.set_message_part( + ProtocolMessagePartKey::NextAggregateVerificationKey, + self.next_fixture.compute_and_encode_avk(), + ); + protocol_message.set_message_part( + ProtocolMessagePartKey::NextProtocolParameters, + self.next_fixture.protocol_parameters().compute_hash(), + ); + protocol_message + .set_message_part(ProtocolMessagePartKey::CurrentEpoch, self.epoch.to_string()); + + protocol_message + } +} + /// A builder for creating a certificate chain. For tests only. /// /// # Simple example usage for building a fully valid certificate chain @@ -264,24 +283,11 @@ impl<'a> CertificateChainBuilder<'a> { let index_certificate = context.index_certificate; let epoch = context.epoch; let immutable_file_number = index_certificate as u64 * 10; - let digest = format!("digest{index_certificate}"); let certificate_hash = format!("certificate_hash-{index_certificate}"); let avk = Self::compute_avk_for_signers(&context.fixture.signers_fixture()); - let next_avk = Self::compute_avk_for_signers(&context.next_fixture.signers_fixture()); let protocol_parameters = context.fixture.protocol_parameters().to_owned(); - let next_protocol_parameters = &context.next_fixture.protocol_parameters(); let base_certificate = fake_data::certificate(certificate_hash); - let mut protocol_message = base_certificate.protocol_message; - protocol_message.set_message_part(ProtocolMessagePartKey::SnapshotDigest, digest); - protocol_message.set_message_part( - ProtocolMessagePartKey::NextAggregateVerificationKey, - next_avk.to_json_hex().unwrap(), - ); - protocol_message.set_message_part( - ProtocolMessagePartKey::NextProtocolParameters, - next_protocol_parameters.compute_hash(), - ); - protocol_message.set_message_part(ProtocolMessagePartKey::CurrentEpoch, epoch.to_string()); + let protocol_message = context.compute_protocol_message_seed(); let signed_message = protocol_message.compute_hash(); Certificate { @@ -337,6 +343,13 @@ impl<'a> CertificateChainBuilder<'a> { let fixture = context.fixture; let mut certificate = self.build_base_certificate(context); certificate.metadata.signers = fixture.stake_distribution_parties(); + let mut protocol_message = certificate.protocol_message.clone(); + protocol_message.set_message_part( + ProtocolMessagePartKey::SnapshotDigest, + format!("digest-{}", context.index_certificate), + ); + certificate.protocol_message = protocol_message; + certificate.signed_message = certificate.protocol_message.compute_hash(); let single_signatures = fixture .signers_fixture() .iter() @@ -390,10 +403,60 @@ impl<'a> Default for CertificateChainBuilder<'a> { mod test { use std::collections::BTreeMap; - use crate::entities::{self, ProtocolMessage}; - use super::*; + #[test] + fn certificate_chain_builder_context_computes_correct_protocol_message_seed() { + let protocol_parameters = ProtocolParameters { + m: 123, + k: 45, + phi_f: 0.67, + }; + let next_protocol_parameters = ProtocolParameters { + m: 100, + k: 10, + phi_f: 0.10, + }; + let fixture = MithrilFixtureBuilder::default() + .with_protocol_parameters(protocol_parameters.into()) + .with_signers(2) + .build(); + let next_fixture = MithrilFixtureBuilder::default() + .with_protocol_parameters(next_protocol_parameters.into()) + .with_signers(3) + .build(); + let context = CertificateChainBuilderContext { + index_certificate: 2, + total_certificates: 5, + epoch: Epoch(1), + fixture: &fixture, + next_fixture: &next_fixture, + }; + let expected_next_avk_part_value = next_fixture.compute_and_encode_avk(); + let expected_next_protocol_parameters_part_value = + next_fixture.protocol_parameters().compute_hash(); + + let expected_current_epoch_part_value = context.epoch.to_string(); + + let protocol_message = context.compute_protocol_message_seed(); + + let mut expected_protocol_message = ProtocolMessage::new(); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::NextAggregateVerificationKey, + expected_next_avk_part_value, + ); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::NextProtocolParameters, + expected_next_protocol_parameters_part_value, + ); + expected_protocol_message.set_message_part( + ProtocolMessagePartKey::CurrentEpoch, + expected_current_epoch_part_value, + ); + + assert_eq!(expected_protocol_message, protocol_message); + } + #[test] fn builds_certificate_chain_with_correct_length() { let (certificate_chain, _) = CertificateChainBuilder::new() @@ -450,7 +513,6 @@ mod test { .with_protocol_parameters(expected_protocol_parameters.into()) .with_signers(3) .build(); - let next_avk = next_fixture.compute_and_encode_avk(); let context = CertificateChainBuilderContext { index_certificate: 0, total_certificates: 5, @@ -458,17 +520,7 @@ mod test { fixture: &fixture, next_fixture: &next_fixture, }; - let mut expected_protocol_message = ProtocolMessage::new(); - expected_protocol_message.set_message_part( - ProtocolMessagePartKey::NextAggregateVerificationKey, - next_avk, - ); - expected_protocol_message.set_message_part( - ProtocolMessagePartKey::NextProtocolParameters, - Into::::into(expected_protocol_parameters).compute_hash(), - ); - expected_protocol_message - .set_message_part(ProtocolMessagePartKey::CurrentEpoch, "1".to_string()); + let expected_protocol_message = context.compute_protocol_message_seed(); let expected_signed_message = expected_protocol_message.compute_hash(); let (protocol_genesis_signer, _) = CertificateChainBuilder::setup_genesis(); @@ -506,29 +558,18 @@ mod test { .with_signers(3) .build(); let avk = fixture.compute_and_encode_avk(); - let next_avk = next_fixture.compute_and_encode_avk(); let context = CertificateChainBuilderContext { - index_certificate: 0, + index_certificate: 2, total_certificates: 5, epoch: Epoch(1), fixture: &fixture, next_fixture: &next_fixture, }; - let mut expected_protocol_message = ProtocolMessage::new(); + let mut expected_protocol_message = context.compute_protocol_message_seed(); expected_protocol_message.set_message_part( ProtocolMessagePartKey::SnapshotDigest, - "digest0".to_string(), - ); - expected_protocol_message.set_message_part( - ProtocolMessagePartKey::NextAggregateVerificationKey, - next_avk, - ); - expected_protocol_message.set_message_part( - ProtocolMessagePartKey::NextProtocolParameters, - Into::::into(expected_protocol_parameters).compute_hash(), + format!("digest-{}", context.index_certificate), ); - expected_protocol_message - .set_message_part(ProtocolMessagePartKey::CurrentEpoch, "1".to_string()); let expected_signed_message = expected_protocol_message.compute_hash(); let standard_certificate = CertificateChainBuilder::default() From e013fcddec6c57a8615a44e1282ee7c298fcf741 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Mon, 14 Oct 2024 17:58:53 +0200 Subject: [PATCH 09/16] feat: CertificateChainBuilderContext checks if current certificate is last in chain --- .../test_utils/certificate_chain_builder.rs | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 7cb183c694d..b34dfeaf625 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -49,6 +49,11 @@ impl CertificateChainBuilderContext<'_> { protocol_message } + + /// Checks if the current certificate is the last one. + pub fn is_last_certificate(&self) -> bool { + self.index_certificate == self.total_certificates - 1 + } } /// A builder for creating a certificate chain. For tests only. @@ -100,8 +105,8 @@ impl CertificateChainBuilderContext<'_> { /// .with_certificates_per_epoch(2) /// .with_standard_certificate_processor(&|certificate, context| { /// let mut certificate = certificate; -/// // Tamper the epoch of the last certificate -/// if context.index_certificate == context.total_certificates - 1 { +/// // Alter the epoch of the last certificate +/// if context.is_last_certificate() { /// certificate.epoch = Epoch(123); /// } /// @@ -457,6 +462,20 @@ mod test { assert_eq!(expected_protocol_message, protocol_message); } + #[test] + fn certificate_chain_builder_context_checks_correctly_if_certificate_is_last() { + let fixture = MithrilFixtureBuilder::default().with_signers(2).build(); + let context = CertificateChainBuilderContext { + index_certificate: 4, + total_certificates: 5, + epoch: Epoch(1), + fixture: &fixture, + next_fixture: &fixture, + }; + + assert!(context.is_last_certificate()); + } + #[test] fn builds_certificate_chain_with_correct_length() { let (certificate_chain, _) = CertificateChainBuilder::new() From fb13267aeff968d92d0325eca599b5754f774329 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Mon, 14 Oct 2024 19:07:28 +0200 Subject: [PATCH 10/16] fix: avoid unneeded computation of fixtures in CertificateChainBuilder --- .../src/test_utils/certificate_chain_builder.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index b34dfeaf625..0a9f0338bda 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -9,7 +9,11 @@ use crate::{ }; use crate::entities::CertificateMetadata; -use std::{cmp::min, collections::HashMap, sync::Arc}; +use std::{ + cmp::min, + collections::{BTreeSet, HashMap}, + sync::Arc, +}; /// Genesis certificate processor function type. For tests only. pub type GenesisCertificateProcessorFunc = @@ -268,6 +272,8 @@ impl<'a> CertificateChainBuilder<'a> { fn build_fixtures_for_epochs(&self, epochs: &[Epoch]) -> HashMap { let fixtures_per_epoch = epochs .iter() + .collect::>() + .into_iter() .map(|epoch| { let total_signers = (self.total_signers_per_epoch_processor)(*epoch); let protocol_parameters = self.protocol_parameters.to_owned().into(); From f6ed39b8d423db87291677f32a08e460e97f27a5 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Mon, 14 Oct 2024 19:07:59 +0200 Subject: [PATCH 11/16] refactor: epochs sequence computation in CertificateChainBuilder --- .../test_utils/certificate_chain_builder.rs | 114 ++++++++++++++---- 1 file changed, 88 insertions(+), 26 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 0a9f0338bda..e0b8154e107 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -12,6 +12,7 @@ use crate::entities::CertificateMetadata; use std::{ cmp::min, collections::{BTreeSet, HashMap}, + iter::repeat, sync::Arc, }; @@ -201,18 +202,18 @@ impl<'a> CertificateChainBuilder<'a> { /// Build the certificate chain. pub fn build(self) -> (Vec, ProtocolGenesisVerifier) { let (genesis_signer, genesis_verifier) = CertificateChainBuilder::setup_genesis(); - let epochs = self.build_epochs(); + let epochs = self.build_epochs_sequence().collect::>(); let fixtures_per_epoch = self.build_fixtures_for_epochs(&epochs); let genesis_certificate_processor = self.genesis_certificate_processor; let standard_certificate_processor = self.standard_certificate_processor; - let total_certificates = epochs.len() - 1; + let total_certificates = self.total_certificates as usize; let certificates = epochs .into_iter() .take(total_certificates) .enumerate() .map(|(index_certificate, epoch)| { - let fixture = fixtures_per_epoch.get(&epoch).unwrap(); - let next_fixture = fixtures_per_epoch.get(&epoch.next()).unwrap(); + let fixture = fixtures_per_epoch.get(&epoch).unwrap_or_else(|| panic!("Fixture not found at epoch {epoch:?} with {} total certificates and {} certificates per epoch", self.total_certificates, self.certificates_per_epoch)); + let next_fixture = fixtures_per_epoch.get(&epoch.next()).unwrap_or_else(|| panic!("Next fixture not found at epoch {epoch:?} with {} total certificates and {} certificates per epoch", self.total_certificates, self.certificates_per_epoch)); let context = CertificateChainBuilderContext { index_certificate, total_certificates, @@ -257,16 +258,30 @@ impl<'a> CertificateChainBuilder<'a> { (genesis_signer, genesis_verifier) } - fn build_epochs(&self) -> Vec { + fn build_epochs_sequence(&self) -> impl Iterator { let total_certificates = self.total_certificates; let certificates_per_epoch = self.certificates_per_epoch; - (1..total_certificates + 2) - .map(|i| match certificates_per_epoch { - 0 => panic!("expected at least 1 certificate per epoch"), - 1 => Epoch(i), - _ => Epoch(i / certificates_per_epoch + 1), - }) - .collect::>() + assert!( + certificates_per_epoch > 0, + "Certificates per epoch must be greater than 0" + ); + assert!( + total_certificates >= certificates_per_epoch, + "Total certificates must be greater or equal to certificates per epoch" + ); + // The total number of epochs in the sequence is the total number of certificates divided by + // the number of certificates per epoch plus two (one for the genesis epoch and one to compute the next fixtures of the last epoch) + let total_epochs_in_sequence = + (total_certificates - 1).div_ceil(certificates_per_epoch) + 2; + (1..=total_epochs_in_sequence).flat_map(move |epoch| { + let repeat_epoch = if epoch == 1 || epoch == total_epochs_in_sequence { + // We need only one occurence of the first and the last epochs in the sequence + 1 + } else { + certificates_per_epoch as usize + }; + repeat(Epoch(epoch)).take(repeat_epoch) + }) } fn build_fixtures_for_epochs(&self, epochs: &[Epoch]) -> HashMap { @@ -381,6 +396,8 @@ impl<'a> CertificateChainBuilder<'a> { certificate } + // Returns the chained certificates in reverse order + // The latest certificate of the chain is the first in the vector fn compute_chained_certificates(&self, certificates: Vec) -> Vec { let mut certificates_chained: Vec = Vec::new(); certificates @@ -416,6 +433,30 @@ mod test { use super::*; + fn build_epoch_numbers_sequence( + total_certificates: u64, + certificates_per_epoch: u64, + ) -> Vec { + CertificateChainBuilder::default() + .with_total_certificates(total_certificates) + .with_certificates_per_epoch(certificates_per_epoch) + .build_epochs_sequence() + .map(|epoch| epoch.0) + .collect::>() + } + + fn build_certificate_chain( + total_certificates: u64, + certificates_per_epoch: u64, + ) -> Vec { + let (certificate_chain, _) = CertificateChainBuilder::default() + .with_total_certificates(total_certificates) + .with_certificates_per_epoch(certificates_per_epoch) + .build(); + + certificate_chain + } + #[test] fn certificate_chain_builder_context_computes_correct_protocol_message_seed() { let protocol_parameters = ProtocolParameters { @@ -484,24 +525,43 @@ mod test { #[test] fn builds_certificate_chain_with_correct_length() { - let (certificate_chain, _) = CertificateChainBuilder::new() - .with_total_certificates(5) - .with_certificates_per_epoch(2) - .build(); - - assert_eq!(5, certificate_chain.len()); + assert_eq!(4, build_certificate_chain(4, 1).len()); + assert_eq!(4, build_certificate_chain(4, 2).len()); + assert_eq!(4, build_certificate_chain(4, 3).len()); + assert_eq!(4, build_certificate_chain(4, 4).len()); + assert_eq!(5, build_certificate_chain(5, 1).len()); + assert_eq!(5, build_certificate_chain(5, 2).len()); + assert_eq!(5, build_certificate_chain(5, 3).len()); + assert_eq!(7, build_certificate_chain(7, 3).len()); + assert_eq!(15, build_certificate_chain(15, 3).len()); } #[test] - fn builds_valid_epochs() { - let expected_epochs = vec![Epoch(1), Epoch(2), Epoch(2), Epoch(3), Epoch(3), Epoch(4)]; + fn builds_valid_epochs_sequence() { + assert_eq!(vec![1, 2, 3, 4], build_epoch_numbers_sequence(3, 1)); + assert_eq!(vec![1, 2, 2, 3, 3, 4], build_epoch_numbers_sequence(4, 2)); + assert_eq!(vec![1, 2, 2, 3, 3, 4], build_epoch_numbers_sequence(5, 2)); + assert_eq!( + vec![1, 2, 2, 2, 3, 3, 3, 4], + build_epoch_numbers_sequence(7, 3), + ); + assert_eq!( + vec![1, 2, 2, 2, 3, 3, 3, 4, 4, 4, 5, 5, 5, 6, 6, 6, 7], + build_epoch_numbers_sequence(15, 3), + ); + } - let epochs = CertificateChainBuilder::default() - .with_total_certificates(5) - .with_certificates_per_epoch(2) - .build_epochs(); + #[test] + #[should_panic] + fn panics_building_invalid_epochs_sequence_no_certificates_per_epoch() { + build_epoch_numbers_sequence(3, 0); + } - assert_eq!(expected_epochs, epochs); + #[test] + #[should_panic] + fn panics_building_invalid_epochs_sequence_less_total_certificates_than_certificates_per_epoch() + { + build_epoch_numbers_sequence(3, 5); } #[test] @@ -511,7 +571,9 @@ mod test { .with_total_certificates(5) .with_certificates_per_epoch(1) .with_total_signers_per_epoch_processor(&|epoch| *epoch as usize); - let epochs = certificate_chain_builder.build_epochs(); + let epochs = certificate_chain_builder + .build_epochs_sequence() + .collect::>(); let epoch_fixtures = BTreeMap::from_iter(certificate_chain_builder.build_fixtures_for_epochs(&epochs)); From 9b63c01e6bd1a9cd572bd736dc87c59467e53a7f Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Wed, 16 Oct 2024 15:49:07 +0200 Subject: [PATCH 12/16] refactor: make epochs and certificate index sequences easier to understand By splitting the epochs sequence in: - (epoch) sequence which is the list of epochs used when creating internal artifacts - (certificate_index, epoch) sequence which is explicitely used to create the final certificate chain. --- .../test_utils/certificate_chain_builder.rs | 122 ++++++++++++------ 1 file changed, 84 insertions(+), 38 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index e0b8154e107..605c8f2dac7 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -202,15 +202,11 @@ impl<'a> CertificateChainBuilder<'a> { /// Build the certificate chain. pub fn build(self) -> (Vec, ProtocolGenesisVerifier) { let (genesis_signer, genesis_verifier) = CertificateChainBuilder::setup_genesis(); - let epochs = self.build_epochs_sequence().collect::>(); - let fixtures_per_epoch = self.build_fixtures_for_epochs(&epochs); let genesis_certificate_processor = self.genesis_certificate_processor; let standard_certificate_processor = self.standard_certificate_processor; let total_certificates = self.total_certificates as usize; - let certificates = epochs - .into_iter() - .take(total_certificates) - .enumerate() + let fixtures_per_epoch = self.build_fixtures_for_epochs(); + let certificates = self.build_certificate_index_and_epoch_sequence() .map(|(index_certificate, epoch)| { let fixture = fixtures_per_epoch.get(&epoch).unwrap_or_else(|| panic!("Fixture not found at epoch {epoch:?} with {} total certificates and {} certificates per epoch", self.total_certificates, self.certificates_per_epoch)); let next_fixture = fixtures_per_epoch.get(&epoch.next()).unwrap_or_else(|| panic!("Next fixture not found at epoch {epoch:?} with {} total certificates and {} certificates per epoch", self.total_certificates, self.certificates_per_epoch)); @@ -269,40 +265,47 @@ impl<'a> CertificateChainBuilder<'a> { total_certificates >= certificates_per_epoch, "Total certificates must be greater or equal to certificates per epoch" ); - // The total number of epochs in the sequence is the total number of certificates divided by - // the number of certificates per epoch plus two (one for the genesis epoch and one to compute the next fixtures of the last epoch) + // The total number of epochs in the sequence is the total number of standard certificates (total number of certificates minus one genesis certificate) + // divided by the number of certificates per epoch plus two (one for the genesis epoch and one to compute the next fixtures of the last epoch) let total_epochs_in_sequence = (total_certificates - 1).div_ceil(certificates_per_epoch) + 2; - (1..=total_epochs_in_sequence).flat_map(move |epoch| { - let repeat_epoch = if epoch == 1 || epoch == total_epochs_in_sequence { - // We need only one occurence of the first and the last epochs in the sequence - 1 - } else { - certificates_per_epoch as usize - }; - repeat(Epoch(epoch)).take(repeat_epoch) - }) - } - - fn build_fixtures_for_epochs(&self, epochs: &[Epoch]) -> HashMap { - let fixtures_per_epoch = epochs - .iter() + (1..=total_epochs_in_sequence).map(Epoch) + } + + fn build_certificate_index_and_epoch_sequence(&self) -> impl Iterator { + let total_certificates = self.total_certificates as usize; + let certificates_per_epoch = self.certificates_per_epoch as usize; + + self.build_epochs_sequence() + .flat_map(move |epoch| { + let repeat_epoch = if epoch == 1 { + // No need to repeat with the genesis epoch + 1 + } else { + certificates_per_epoch + }; + repeat(Epoch(*epoch)).take(repeat_epoch) + }) + .take(total_certificates) + .enumerate() + } + + fn build_fixtures_for_epochs(&self) -> HashMap { + self.build_epochs_sequence() .collect::>() .into_iter() .map(|epoch| { - let total_signers = (self.total_signers_per_epoch_processor)(*epoch); + let total_signers = (self.total_signers_per_epoch_processor)(epoch); let protocol_parameters = self.protocol_parameters.to_owned().into(); ( - *epoch, + epoch, MithrilFixtureBuilder::default() .with_protocol_parameters(protocol_parameters) .with_signers(total_signers) .build(), ) }) - .collect::>(); - - fixtures_per_epoch + .collect::>() } fn build_base_certificate(&self, context: &CertificateChainBuilderContext) -> Certificate { @@ -441,10 +444,35 @@ mod test { .with_total_certificates(total_certificates) .with_certificates_per_epoch(certificates_per_epoch) .build_epochs_sequence() - .map(|epoch| epoch.0) + .map(|epoch| *epoch) + .collect::>() + } + + fn build_certificate_index_and_epoch_numbers_sequence( + total_certificates: u64, + certificates_per_epoch: u64, + ) -> Vec<(usize, u64)> { + CertificateChainBuilder::default() + .with_total_certificates(total_certificates) + .with_certificates_per_epoch(certificates_per_epoch) + .build_certificate_index_and_epoch_sequence() + .map(|(certificate_index, epoch)| (certificate_index, *epoch)) .collect::>() } + fn build_epoch_numbers_sequence_in_certificate_chain( + total_certificates: u64, + certificates_per_epoch: u64, + ) -> Vec { + build_certificate_index_and_epoch_numbers_sequence( + total_certificates, + certificates_per_epoch, + ) + .iter() + .map(|(_certificate_index, epoch)| *epoch) + .collect::>() + } + fn build_certificate_chain( total_certificates: u64, certificates_per_epoch: u64, @@ -539,15 +567,36 @@ mod test { #[test] fn builds_valid_epochs_sequence() { assert_eq!(vec![1, 2, 3, 4], build_epoch_numbers_sequence(3, 1)); - assert_eq!(vec![1, 2, 2, 3, 3, 4], build_epoch_numbers_sequence(4, 2)); - assert_eq!(vec![1, 2, 2, 3, 3, 4], build_epoch_numbers_sequence(5, 2)); + assert_eq!(vec![1, 2, 3, 4], build_epoch_numbers_sequence(4, 2)); + assert_eq!(vec![1, 2, 3, 4], build_epoch_numbers_sequence(5, 2)); + assert_eq!(vec![1, 2, 3, 4], build_epoch_numbers_sequence(7, 3),); assert_eq!( - vec![1, 2, 2, 2, 3, 3, 3, 4], - build_epoch_numbers_sequence(7, 3), + vec![1, 2, 3, 4, 5, 6, 7], + build_epoch_numbers_sequence(15, 3), ); + } + + #[test] + fn builds_valid_certificate_index_and_epoch_numbers_sequence() { assert_eq!( - vec![1, 2, 2, 2, 3, 3, 3, 4, 4, 4, 5, 5, 5, 6, 6, 6, 7], - build_epoch_numbers_sequence(15, 3), + vec![1, 2, 3], + build_epoch_numbers_sequence_in_certificate_chain(3, 1) + ); + assert_eq!( + vec![1, 2, 2, 3], + build_epoch_numbers_sequence_in_certificate_chain(4, 2) + ); + assert_eq!( + vec![1, 2, 2, 3, 3], + build_epoch_numbers_sequence_in_certificate_chain(5, 2) + ); + assert_eq!( + vec![1, 2, 2, 2, 3, 3, 3], + build_epoch_numbers_sequence_in_certificate_chain(7, 3), + ); + assert_eq!( + vec![1, 2, 2, 2, 3, 3, 3, 4, 4, 4, 5, 5, 5, 6, 6], + build_epoch_numbers_sequence_in_certificate_chain(15, 3), ); } @@ -571,12 +620,9 @@ mod test { .with_total_certificates(5) .with_certificates_per_epoch(1) .with_total_signers_per_epoch_processor(&|epoch| *epoch as usize); - let epochs = certificate_chain_builder - .build_epochs_sequence() - .collect::>(); let epoch_fixtures = - BTreeMap::from_iter(certificate_chain_builder.build_fixtures_for_epochs(&epochs)); + BTreeMap::from_iter(certificate_chain_builder.build_fixtures_for_epochs()); let total_signers = epoch_fixtures .into_values() From bcd03dde5dea5f27065550322438eaad9a9981bf Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Wed, 16 Oct 2024 16:10:21 +0200 Subject: [PATCH 13/16] refactor: clarify computation of the number of epochs in sequence --- .../src/test_utils/certificate_chain_builder.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 605c8f2dac7..fc5b3f6b8c7 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -267,8 +267,12 @@ impl<'a> CertificateChainBuilder<'a> { ); // The total number of epochs in the sequence is the total number of standard certificates (total number of certificates minus one genesis certificate) // divided by the number of certificates per epoch plus two (one for the genesis epoch and one to compute the next fixtures of the last epoch) - let total_epochs_in_sequence = - (total_certificates - 1).div_ceil(certificates_per_epoch) + 2; + const TOTAL_GENESIS_CERTIFICATES: u64 = 1; + const TOTAL_EXTRA_EPOCHS_FOR_FIXTURES_COMPUTATION: u64 = 1; + let total_epochs_in_sequence = (total_certificates - TOTAL_GENESIS_CERTIFICATES) + .div_ceil(certificates_per_epoch) + + TOTAL_GENESIS_CERTIFICATES + + TOTAL_EXTRA_EPOCHS_FOR_FIXTURES_COMPUTATION; (1..=total_epochs_in_sequence).map(Epoch) } From 22c9b78824168ddb3b6ee0b732665820dc017725 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Wed, 16 Oct 2024 16:28:06 +0200 Subject: [PATCH 14/16] refactor: change visibility of processor function types And add a factory for the 'CertificateChainBuilderContext'. --- .../test_utils/certificate_chain_builder.rs | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index fc5b3f6b8c7..404ae688d80 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -17,15 +17,15 @@ use std::{ }; /// Genesis certificate processor function type. For tests only. -pub type GenesisCertificateProcessorFunc = +type GenesisCertificateProcessorFunc = dyn Fn(Certificate, &CertificateChainBuilderContext, &ProtocolGenesisSigner) -> Certificate; /// Standard certificate processor function type. For tests only. -pub type StandardCertificateProcessorFunc = +type StandardCertificateProcessorFunc = dyn Fn(Certificate, &CertificateChainBuilderContext) -> Certificate; /// Total signers per epoch processor function type. For tests only. -pub type TotalSignersPerEpochProcessorFunc = dyn Fn(Epoch) -> usize; +type TotalSignersPerEpochProcessorFunc = dyn Fn(Epoch) -> usize; /// Context used while building a certificate chain. For tests only. pub struct CertificateChainBuilderContext<'a> { @@ -37,7 +37,23 @@ pub struct CertificateChainBuilderContext<'a> { pub next_fixture: &'a MithrilFixture, } -impl CertificateChainBuilderContext<'_> { +impl<'a> CertificateChainBuilderContext<'a> { + fn new( + index_certificate: usize, + total_certificates: usize, + epoch: Epoch, + fixture: &'a MithrilFixture, + next_fixture: &'a MithrilFixture, + ) -> Self { + Self { + index_certificate, + total_certificates, + epoch, + fixture, + next_fixture, + } + } + /// Computes the protocol message seed. pub fn compute_protocol_message_seed(&self) -> ProtocolMessage { let mut protocol_message = ProtocolMessage::new(); @@ -210,13 +226,13 @@ impl<'a> CertificateChainBuilder<'a> { .map(|(index_certificate, epoch)| { let fixture = fixtures_per_epoch.get(&epoch).unwrap_or_else(|| panic!("Fixture not found at epoch {epoch:?} with {} total certificates and {} certificates per epoch", self.total_certificates, self.certificates_per_epoch)); let next_fixture = fixtures_per_epoch.get(&epoch.next()).unwrap_or_else(|| panic!("Next fixture not found at epoch {epoch:?} with {} total certificates and {} certificates per epoch", self.total_certificates, self.certificates_per_epoch)); - let context = CertificateChainBuilderContext { + let context = CertificateChainBuilderContext::new( index_certificate, total_certificates, epoch, fixture, next_fixture, - }; + ); match index_certificate { 0 => genesis_certificate_processor( self.build_genesis_certificate(&context, &genesis_signer), From c223548733d72af7094486246d299e7f63e3d5ee Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Wed, 16 Oct 2024 17:39:36 +0200 Subject: [PATCH 15/16] refactor: simplify computation of chained certificate chain --- .../test_utils/certificate_chain_builder.rs | 55 ++++++++++++------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/mithril-common/src/test_utils/certificate_chain_builder.rs b/mithril-common/src/test_utils/certificate_chain_builder.rs index 404ae688d80..832b316da5c 100644 --- a/mithril-common/src/test_utils/certificate_chain_builder.rs +++ b/mithril-common/src/test_utils/certificate_chain_builder.rs @@ -422,22 +422,30 @@ impl<'a> CertificateChainBuilder<'a> { // Returns the chained certificates in reverse order // The latest certificate of the chain is the first in the vector fn compute_chained_certificates(&self, certificates: Vec) -> Vec { - let mut certificates_chained: Vec = Vec::new(); - certificates - .iter() - .enumerate() - .for_each(|(i, certificate)| { - let mut certificate_new = certificate.clone(); - if i > 0 { - if let Some(previous_certificate) = certificates_chained.get(i - 1) { - certificate_new.previous_hash = previous_certificate.compute_hash(); - } - } else { - certificate_new.previous_hash = "".to_string(); - } - certificate_new.hash = certificate_new.compute_hash(); - certificates_chained.push(certificate_new); - }); + fn update_certificate_previous_hash( + certificate: Certificate, + previous_certificate: Option<&Certificate>, + ) -> Certificate { + let mut certificate = certificate; + certificate.previous_hash = previous_certificate + .map(|c| c.hash.to_string()) + .unwrap_or_default(); + certificate.hash = certificate.compute_hash(); + + certificate + } + + let mut certificates_chained: Vec = + certificates + .into_iter() + .fold(Vec::new(), |mut certificates_chained, certificate| { + let previous_certificate_maybe = certificates_chained.last(); + let certificate = + update_certificate_previous_hash(certificate, previous_certificate_maybe); + certificates_chained.push(certificate); + + certificates_chained + }); certificates_chained.reverse(); certificates_chained @@ -753,9 +761,18 @@ mod test { #[test] fn builds_certificate_chain_correctly_chained() { let certificates = vec![ - fake_data::certificate("cert-1".to_string()), - fake_data::certificate("cert-2".to_string()), - fake_data::certificate("cert-3".to_string()), + Certificate { + epoch: Epoch(1), + ..fake_data::certificate("cert-1".to_string()) + }, + Certificate { + epoch: Epoch(2), + ..fake_data::certificate("cert-2".to_string()) + }, + Certificate { + epoch: Epoch(3), + ..fake_data::certificate("cert-3".to_string()) + }, ]; let certificates_chained = From 335ee13663b1b91b146fc3d9ee617e8e7ce1ebc7 Mon Sep 17 00:00:00 2001 From: Jean-Philippe Raynaud Date: Wed, 16 Oct 2024 19:08:41 +0200 Subject: [PATCH 16/16] chore: bump crates versions - 'mithril-common' from '0.4.71' to '0.4.72'. --- Cargo.lock | 2 +- mithril-common/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1a39f2d741f..aaa3aeee4f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3708,7 +3708,7 @@ dependencies = [ [[package]] name = "mithril-common" -version = "0.4.71" +version = "0.4.72" dependencies = [ "anyhow", "async-trait", diff --git a/mithril-common/Cargo.toml b/mithril-common/Cargo.toml index a0ac19208fa..34b836aac66 100644 --- a/mithril-common/Cargo.toml +++ b/mithril-common/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mithril-common" -version = "0.4.71" +version = "0.4.72" description = "Common types, interfaces, and utilities for Mithril nodes." authors = { workspace = true } edition = { workspace = true }