Skip to content

Commit e67b9db

Browse files
Alenarjpraynaud
authored andcommitted
Multiple fixes to make aggregator handle multiple signed entity types:
* MithrilStakeDistribution now have beacon to handle the case when the epoch changes but not the stake distribution. * Master certificate retrieval fix: before we always retrieved the first certificate of the previous epoch even if there was one available in the current epoch. * OpenMessage repository fix: Only one openMessage per signed entity type could be recorded per epoch since the get query did not discriminate with the signed entity type beacon, only by its id. * Adapted integration tests to the fact that now the state machine signs two types of data (adding MithrilStakeDistribution to the already signed Cardano Immutables db).
1 parent c314dc6 commit e67b9db

File tree

12 files changed

+469
-145
lines changed

12 files changed

+469
-145
lines changed

mithril-aggregator/src/artifact_builder/artifact_builder_service.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use async_trait::async_trait;
22
use chrono::Utc;
3-
3+
use slog_scope::info;
44
use std::sync::Arc;
55

66
use mithril_common::{
@@ -84,6 +84,12 @@ impl ArtifactBuilderService for MithrilArtifactBuilderService {
8484
signed_entity_type: SignedEntityType,
8585
certificate: &Certificate,
8686
) -> StdResult<()> {
87+
info!(
88+
"MithrilArtifactBuilderService::create_artifact";
89+
"signed_entity_type" => ?signed_entity_type,
90+
"certificate_hash" => &certificate.hash
91+
);
92+
8793
let artifact = self
8894
.compute_artifact(signed_entity_type.clone(), certificate)
8995
.await?;
@@ -117,7 +123,8 @@ mod tests {
117123
async fn build_mithril_stake_distribution_artifact_when_given_mithril_stake_distribution_entity_type(
118124
) {
119125
let signers_with_stake = fake_data::signers_with_stakes(5);
120-
let mithril_stake_distribution_expected = MithrilStakeDistribution::new(signers_with_stake);
126+
let mithril_stake_distribution_expected =
127+
MithrilStakeDistribution::new(Epoch(1), signers_with_stake);
121128
let mithril_stake_distribution_clone = mithril_stake_distribution_expected.clone();
122129

123130
let mock_signed_entity_storer = MockSignedEntityStorer::new();

mithril-aggregator/src/artifact_builder/mithril_stake_distribution.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,18 @@ use mithril_common::{
1515
/// Mithril Stake Distribution
1616
#[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
1717
pub struct MithrilStakeDistribution {
18+
epoch: Epoch,
1819
signers_with_stake: Vec<SignerWithStake>,
1920
hash: String,
2021
}
2122

2223
impl MithrilStakeDistribution {
2324
/// MithrilStakeDistribution artifact factory
24-
pub fn new(signers_with_stake: Vec<SignerWithStake>) -> Self {
25+
pub fn new(epoch: Epoch, signers_with_stake: Vec<SignerWithStake>) -> Self {
2526
let mut signers_with_stake_sorted = signers_with_stake;
2627
signers_with_stake_sorted.sort();
2728
let mut mithril_stake_distribution = Self {
29+
epoch,
2830
signers_with_stake: signers_with_stake_sorted,
2931
hash: "".to_string(),
3032
};
@@ -34,6 +36,7 @@ impl MithrilStakeDistribution {
3436

3537
fn compute_hash(&self) -> String {
3638
let mut hasher = Sha256::new();
39+
hasher.update(self.epoch.0.to_be_bytes());
3740
for signer_with_stake in &self.signers_with_stake {
3841
hasher.update(signer_with_stake.compute_hash().as_bytes());
3942
}
@@ -64,11 +67,12 @@ impl MithrilStakeDistributionArtifactBuilder {
6467
impl ArtifactBuilder<Epoch, MithrilStakeDistribution> for MithrilStakeDistributionArtifactBuilder {
6568
async fn compute_artifact(
6669
&self,
67-
_beacon: Epoch,
70+
beacon: Epoch,
6871
_certificate: &Certificate,
6972
) -> StdResult<MithrilStakeDistribution> {
7073
let multi_signer = self.multi_signer.read().await;
7174
Ok(MithrilStakeDistribution::new(
75+
beacon,
7276
multi_signer.get_next_signers_with_stake().await?,
7377
))
7478
}
@@ -97,7 +101,7 @@ mod tests {
97101
.compute_artifact(Epoch(1), &certificate)
98102
.await
99103
.unwrap();
100-
let artifact_expected = MithrilStakeDistribution::new(signers_with_stake);
104+
let artifact_expected = MithrilStakeDistribution::new(Epoch(1), signers_with_stake);
101105
assert_eq!(artifact_expected, artifact);
102106
}
103107

@@ -106,17 +110,18 @@ mod tests {
106110
let signers_with_stake = fake_data::signers_with_stakes(5);
107111

108112
assert_eq!(
109-
MithrilStakeDistribution::new(signers_with_stake.clone()),
110-
MithrilStakeDistribution::new(signers_with_stake.into_iter().rev().collect())
113+
MithrilStakeDistribution::new(Epoch(1), signers_with_stake.clone()),
114+
MithrilStakeDistribution::new(Epoch(1), signers_with_stake.into_iter().rev().collect())
111115
);
112116
}
113117

114118
#[test]
115119
fn hash_value_doesnt_change_if_signers_order_change() {
116120
let signers_with_stake = fake_data::signers_with_stakes(5);
117121

118-
let sd = MithrilStakeDistribution::new(signers_with_stake.clone());
119-
let sd2 = MithrilStakeDistribution::new(signers_with_stake.into_iter().rev().collect());
122+
let sd = MithrilStakeDistribution::new(Epoch(1), signers_with_stake.clone());
123+
let sd2 =
124+
MithrilStakeDistribution::new(Epoch(1), signers_with_stake.into_iter().rev().collect());
120125

121126
assert_eq!(sd.hash, sd2.hash);
122127
}

mithril-aggregator/src/database/provider/certificate.rs

Lines changed: 198 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,34 @@ pub struct CertificateRecord {
6666
pub sealed_at: String,
6767
}
6868

69+
impl CertificateRecord {
70+
#[cfg(test)]
71+
pub fn dummy_genesis(id: &str, beacon: Beacon) -> Self {
72+
let mut record = Self::dummy(id, "", beacon);
73+
record.parent_certificate_id = None;
74+
record
75+
}
76+
77+
#[cfg(test)]
78+
pub fn dummy(id: &str, parent_id: &str, beacon: Beacon) -> Self {
79+
Self {
80+
certificate_id: id.to_string(),
81+
parent_certificate_id: Some(parent_id.to_string()),
82+
message: "message".to_string(),
83+
signature: "signature".to_string(),
84+
aggregate_verification_key: "avk".to_string(),
85+
epoch: beacon.epoch,
86+
beacon,
87+
protocol_version: "protocol_version".to_string(),
88+
protocol_parameters: Default::default(),
89+
protocol_message: Default::default(),
90+
signers: vec![],
91+
initiated_at: "???".to_string(),
92+
sealed_at: "???".to_string(),
93+
}
94+
}
95+
}
96+
6997
impl From<Certificate> for CertificateRecord {
7098
fn from(other: Certificate) -> Self {
7199
if !other.genesis_signature.is_empty() {
@@ -401,17 +429,11 @@ impl<'conn> MasterCertificateProvider<'conn> {
401429
}
402430

403431
pub fn get_master_certificate_condition(&self, epoch: Epoch) -> WhereCondition {
404-
let condition =
405-
WhereCondition::new("certificate.parent_certificate_id is null", Vec::new()).or_where(
406-
WhereCondition::new("parent_certificate.epoch != certificate.epoch", Vec::new()),
407-
);
408-
409432
let epoch_i64: i64 = epoch.0.try_into().unwrap();
410433
WhereCondition::new(
411434
"certificate.epoch between ?* and ?*",
412435
vec![Value::Integer(epoch_i64 - 1), Value::Integer(epoch_i64)],
413436
)
414-
.and_where(condition)
415437
}
416438
}
417439

@@ -425,18 +447,15 @@ impl<'conn> Provider<'conn> for MasterCertificateProvider<'conn> {
425447
fn get_definition(&self, condition: &str) -> String {
426448
// it is important to alias the fields with the same name as the table
427449
// since the table cannot be aliased in a RETURNING statement in SQLite.
428-
let projection = Self::Entity::get_projection().expand(SourceAlias::new(&[
429-
("{:certificate:}", "certificate"),
430-
("{:parent_certificate:}", "parent_certificate"),
431-
]));
450+
let projection = Self::Entity::get_projection()
451+
.expand(SourceAlias::new(&[("{:certificate:}", "certificate")]));
432452

433453
format!(
434454
r#"
435455
select {projection}
436456
from certificate
437-
left join certificate as parent_certificate
438-
on certificate.parent_certificate_id = parent_certificate.certificate_id
439-
where {condition}"#
457+
where {condition}
458+
group by certificate.epoch order by certificate.epoch desc, certificate.ROWID asc"#
440459
)
441460
}
442461
}
@@ -872,7 +891,10 @@ mod tests {
872891
let condition = provider.get_master_certificate_condition(Epoch(10));
873892
let (condition_str, parameters) = condition.expand();
874893

875-
assert_eq!("certificate.epoch between ?1 and ?2 and (certificate.parent_certificate_id is null or parent_certificate.epoch != certificate.epoch)".to_string(), condition_str);
894+
assert_eq!(
895+
"certificate.epoch between ?1 and ?2".to_string(),
896+
condition_str
897+
);
876898
assert_eq!(vec![Value::Integer(9), Value::Integer(10)], parameters);
877899
}
878900

@@ -904,6 +926,168 @@ mod tests {
904926
assert_eq!(expected_hash, certificate.hash);
905927
}
906928

929+
async fn insert_certificate_records(
930+
connection: Arc<Mutex<Connection>>,
931+
records: Vec<CertificateRecord>,
932+
) {
933+
let lock = connection.lock().await;
934+
let provider = InsertCertificateRecordProvider::new(&lock);
935+
936+
for certificate in records {
937+
provider.persist(certificate).unwrap();
938+
}
939+
}
940+
941+
#[tokio::test]
942+
async fn get_master_certificate_no_certificate_recorded_returns_none() {
943+
let mut deps = DependenciesBuilder::new(Configuration::new_sample());
944+
let connection = deps.get_sqlite_connection().await.unwrap();
945+
let certificates = vec![];
946+
insert_certificate_records(connection.clone(), certificates).await;
947+
948+
let repository = CertificateRepository::new(connection);
949+
let certificate = repository
950+
.get_master_certificate_for_epoch(Epoch(1))
951+
.await
952+
.unwrap();
953+
954+
assert_eq!(None, certificate);
955+
}
956+
957+
#[tokio::test]
958+
async fn get_master_certificate_one_cert_in_current_epoch_recorded_returns_that_one() {
959+
let mut deps = DependenciesBuilder::new(Configuration::new_sample());
960+
let connection = deps.get_sqlite_connection().await.unwrap();
961+
let certificate = CertificateRecord::dummy_genesis("1", Beacon::new(String::new(), 1, 1));
962+
let expected_certificate: Certificate = certificate.clone().into();
963+
insert_certificate_records(connection.clone(), vec![certificate]).await;
964+
965+
let repository = CertificateRepository::new(connection);
966+
let certificate = repository
967+
.get_master_certificate_for_epoch(Epoch(1))
968+
.await
969+
.unwrap()
970+
.expect("This should return a certificate.");
971+
972+
assert_eq!(expected_certificate, certificate);
973+
}
974+
975+
#[tokio::test]
976+
async fn get_master_certificate_multiple_cert_in_current_epoch_returns_first_of_current_epoch()
977+
{
978+
let mut deps = DependenciesBuilder::new(Configuration::new_sample());
979+
let connection = deps.get_sqlite_connection().await.unwrap();
980+
let certificates = vec![
981+
CertificateRecord::dummy_genesis("1", Beacon::new(String::new(), 1, 1)),
982+
CertificateRecord::dummy("2", "1", Beacon::new(String::new(), 1, 2)),
983+
CertificateRecord::dummy("3", "1", Beacon::new(String::new(), 1, 3)),
984+
];
985+
let expected_certificate: Certificate = certificates.first().unwrap().clone().into();
986+
insert_certificate_records(connection.clone(), certificates).await;
987+
988+
let repository = CertificateRepository::new(connection);
989+
let certificate = repository
990+
.get_master_certificate_for_epoch(Epoch(1))
991+
.await
992+
.unwrap()
993+
.expect("This should return a certificate.");
994+
995+
assert_eq!(expected_certificate, certificate);
996+
}
997+
998+
#[tokio::test]
999+
async fn get_master_certificate_multiple_cert_in_previous_epoch_none_in_the_current_returns_first_of_previous_epoch(
1000+
) {
1001+
let mut deps = DependenciesBuilder::new(Configuration::new_sample());
1002+
let connection = deps.get_sqlite_connection().await.unwrap();
1003+
let certificates = vec![
1004+
CertificateRecord::dummy_genesis("1", Beacon::new(String::new(), 1, 1)),
1005+
CertificateRecord::dummy("2", "1", Beacon::new(String::new(), 1, 2)),
1006+
CertificateRecord::dummy("3", "1", Beacon::new(String::new(), 1, 3)),
1007+
];
1008+
let expected_certificate: Certificate = certificates.first().unwrap().clone().into();
1009+
insert_certificate_records(connection.clone(), certificates).await;
1010+
1011+
let repository = CertificateRepository::new(connection);
1012+
let certificate = repository
1013+
.get_master_certificate_for_epoch(Epoch(2))
1014+
.await
1015+
.unwrap()
1016+
.expect("This should return a certificate.");
1017+
1018+
assert_eq!(expected_certificate, certificate);
1019+
}
1020+
1021+
#[tokio::test]
1022+
async fn get_master_certificate_multiple_cert_in_previous_one_cert_in_current_epoch_returns_one_in_current_epoch(
1023+
) {
1024+
let mut deps = DependenciesBuilder::new(Configuration::new_sample());
1025+
let connection = deps.get_sqlite_connection().await.unwrap();
1026+
let certificates = vec![
1027+
CertificateRecord::dummy_genesis("1", Beacon::new(String::new(), 1, 1)),
1028+
CertificateRecord::dummy("2", "1", Beacon::new(String::new(), 1, 2)),
1029+
CertificateRecord::dummy("3", "1", Beacon::new(String::new(), 1, 3)),
1030+
CertificateRecord::dummy("4", "1", Beacon::new(String::new(), 2, 4)),
1031+
];
1032+
let expected_certificate: Certificate = certificates.last().unwrap().clone().into();
1033+
insert_certificate_records(connection.clone(), certificates).await;
1034+
1035+
let repository = CertificateRepository::new(connection);
1036+
let certificate = repository
1037+
.get_master_certificate_for_epoch(Epoch(2))
1038+
.await
1039+
.unwrap()
1040+
.expect("This should return a certificate.");
1041+
1042+
assert_eq!(expected_certificate, certificate);
1043+
}
1044+
1045+
#[tokio::test]
1046+
async fn get_master_certificate_multiple_cert_in_previous_multiple_in_current_epoch_returns_first_of_current_epoch(
1047+
) {
1048+
let mut deps = DependenciesBuilder::new(Configuration::new_sample());
1049+
let connection = deps.get_sqlite_connection().await.unwrap();
1050+
let certificates = vec![
1051+
CertificateRecord::dummy_genesis("1", Beacon::new(String::new(), 1, 1)),
1052+
CertificateRecord::dummy("2", "1", Beacon::new(String::new(), 1, 2)),
1053+
CertificateRecord::dummy("3", "1", Beacon::new(String::new(), 1, 3)),
1054+
CertificateRecord::dummy("4", "1", Beacon::new(String::new(), 2, 4)),
1055+
CertificateRecord::dummy("5", "4", Beacon::new(String::new(), 2, 5)),
1056+
CertificateRecord::dummy("6", "4", Beacon::new(String::new(), 2, 6)),
1057+
];
1058+
let expected_certificate: Certificate = certificates.get(3).unwrap().clone().into();
1059+
insert_certificate_records(connection.clone(), certificates).await;
1060+
1061+
let repository = CertificateRepository::new(connection);
1062+
let certificate = repository
1063+
.get_master_certificate_for_epoch(Epoch(2))
1064+
.await
1065+
.unwrap()
1066+
.expect("This should return a certificate.");
1067+
assert_eq!(expected_certificate, certificate);
1068+
}
1069+
1070+
#[tokio::test]
1071+
async fn get_master_certificate_multiple_cert_in_penultimate_epoch_none_in_previous_returns_none(
1072+
) {
1073+
let mut deps = DependenciesBuilder::new(Configuration::new_sample());
1074+
let connection = deps.get_sqlite_connection().await.unwrap();
1075+
let certificates = vec![
1076+
CertificateRecord::dummy_genesis("1", Beacon::new(String::new(), 1, 1)),
1077+
CertificateRecord::dummy("2", "1", Beacon::new(String::new(), 1, 2)),
1078+
CertificateRecord::dummy("3", "1", Beacon::new(String::new(), 1, 3)),
1079+
];
1080+
insert_certificate_records(connection.clone(), certificates).await;
1081+
1082+
let repository = CertificateRepository::new(connection);
1083+
let certificate = repository
1084+
.get_master_certificate_for_epoch(Epoch(3))
1085+
.await
1086+
.unwrap();
1087+
1088+
assert_eq!(None, certificate);
1089+
}
1090+
9071091
#[tokio::test]
9081092
async fn get_master_certificate_for_epoch() {
9091093
let (certificates, _) = setup_certificate_chain(3, 1);

0 commit comments

Comments
 (0)