Skip to content

Commit 6638f0f

Browse files
AlenarAlenar
committed
refactor(aggregator): make ctx config field optional in AggregatorEpochSettings
Note: this commit compiles, but break aggregator tests as init of test data in db will fails because of missing config when inserting to `epoch_setting` table.
1 parent 7d3e5e2 commit 6638f0f

File tree

9 files changed

+152
-55
lines changed

9 files changed

+152
-55
lines changed

‎mithril-aggregator/src/configuration.rs‎

Lines changed: 90 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -382,15 +382,25 @@ pub trait ConfigurationSource {
382382
fn get_leader_aggregator_epoch_settings_configuration(
383383
&self,
384384
) -> StdResult<AggregatorEpochSettings> {
385+
let allowed_discriminants = self.compute_allowed_signed_entity_types_discriminants()?;
386+
387+
let cardano_transactions_signing_config = if allowed_discriminants
388+
.contains(&SignedEntityTypeDiscriminants::CardanoTransactions)
389+
{
390+
let cardano_transactions_signing_config =
391+
self.cardano_transactions_signing_config().with_context(
392+
|| "Configuration `cardano_transactions_signing_config` is mandatory for a Leader Aggregator when `CardanoTransactions` is enabled in `signed_entity_types`"
393+
)?;
394+
Some(cardano_transactions_signing_config)
395+
} else {
396+
None
397+
};
398+
385399
Ok(AggregatorEpochSettings {
386400
protocol_parameters: self.protocol_parameters().with_context(
387401
|| "Configuration `protocol_parameters` is mandatory for a Leader Aggregator",
388402
)?,
389-
cardano_transactions_signing_config: self
390-
.cardano_transactions_signing_config()
391-
.with_context(
392-
|| "Configuration `cardano_transactions_signing_config` is mandatory for a Leader Aggregator",
393-
)?,
403+
cardano_transactions_signing_config,
394404
})
395405
}
396406

@@ -1152,6 +1162,7 @@ impl Source for DefaultConfiguration {
11521162
#[cfg(test)]
11531163
mod test {
11541164
use mithril_common::temp_dir;
1165+
use mithril_common::test::double::fake_data;
11551166

11561167
use super::*;
11571168

@@ -1315,6 +1326,80 @@ mod test {
13151326
assert!(!config.is_follower_aggregator());
13161327
}
13171328

1329+
mod get_leader_aggregator_epoch_settings_configuration {
1330+
use super::*;
1331+
1332+
#[test]
1333+
fn succeed_when_cardano_transactions_is_disabled_and_cardano_transactions_signing_config_is_not_set()
1334+
{
1335+
let epoch_settings = ServeCommandConfiguration {
1336+
signed_entity_types: None,
1337+
cardano_transactions_signing_config: None,
1338+
protocol_parameters: Some(ProtocolParameters::new(1, 2, 3.1)),
1339+
..ServeCommandConfiguration::new_sample(temp_dir!())
1340+
}
1341+
.get_leader_aggregator_epoch_settings_configuration()
1342+
.unwrap();
1343+
1344+
assert_eq!(
1345+
AggregatorEpochSettings {
1346+
protocol_parameters: ProtocolParameters::new(1, 2, 3.1),
1347+
cardano_transactions_signing_config: None
1348+
},
1349+
epoch_settings
1350+
);
1351+
}
1352+
1353+
#[test]
1354+
fn succeed_when_cardano_transactions_is_enabled_and_cardano_transactions_signing_config_is_set()
1355+
{
1356+
let epoch_settings = ServeCommandConfiguration {
1357+
signed_entity_types: Some(
1358+
SignedEntityTypeDiscriminants::CardanoTransactions.to_string(),
1359+
),
1360+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig {
1361+
security_parameter: BlockNumber(10),
1362+
step: BlockNumber(30),
1363+
}),
1364+
protocol_parameters: Some(ProtocolParameters::new(2, 3, 4.1)),
1365+
..ServeCommandConfiguration::new_sample(temp_dir!())
1366+
}
1367+
.get_leader_aggregator_epoch_settings_configuration()
1368+
.unwrap();
1369+
1370+
assert_eq!(
1371+
AggregatorEpochSettings {
1372+
protocol_parameters: ProtocolParameters::new(2, 3, 4.1),
1373+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig {
1374+
security_parameter: BlockNumber(10),
1375+
step: BlockNumber(30),
1376+
},)
1377+
},
1378+
epoch_settings
1379+
);
1380+
}
1381+
1382+
#[test]
1383+
fn fails_when_cardano_transactions_is_enabled_without_associated_config() {
1384+
let error = ServeCommandConfiguration {
1385+
cardano_transactions_signing_config: None,
1386+
signed_entity_types: Some(
1387+
SignedEntityTypeDiscriminants::CardanoTransactions.to_string(),
1388+
),
1389+
protocol_parameters: Some(fake_data::protocol_parameters()),
1390+
..ServeCommandConfiguration::new_sample(temp_dir!())
1391+
}
1392+
.get_leader_aggregator_epoch_settings_configuration()
1393+
.unwrap_err();
1394+
1395+
assert!(
1396+
error
1397+
.to_string()
1398+
.contains("Configuration `cardano_transactions_signing_config` is mandatory")
1399+
);
1400+
}
1401+
}
1402+
13181403
#[test]
13191404
fn serialized_ancillary_files_signer_config_use_snake_case_for_keys_and_kebab_case_for_type_value()
13201405
{

‎mithril-aggregator/src/database/record/epoch_settings.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ impl From<EpochSettingsRecord> for AggregatorEpochSettings {
2020
fn from(other: EpochSettingsRecord) -> Self {
2121
Self {
2222
protocol_parameters: other.protocol_parameters,
23-
cardano_transactions_signing_config: other.cardano_transactions_signing_config,
23+
cardano_transactions_signing_config: Some(other.cardano_transactions_signing_config),
2424
}
2525
}
2626
}

‎mithril-aggregator/src/database/repository/epoch_settings_store.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,10 @@ impl EpochSettingsStorer for EpochSettingsStore {
5353
) -> StdResult<Option<AggregatorEpochSettings>> {
5454
let record_to_insert = EpochSettingsRecord {
5555
epoch_settings_id: epoch,
56-
cardano_transactions_signing_config: epoch_settings.cardano_transactions_signing_config,
56+
// Todo: make this field optional allowing removal of the unwrap()
57+
cardano_transactions_signing_config: epoch_settings
58+
.cardano_transactions_signing_config
59+
.unwrap(),
5760
protocol_parameters: epoch_settings.protocol_parameters,
5861
};
5962
let epoch_settings_record = self

‎mithril-aggregator/src/entities/aggregator_epoch_settings.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ pub struct AggregatorEpochSettings {
77
pub protocol_parameters: ProtocolParameters,
88

99
/// Cardano transactions signing configuration
10-
pub cardano_transactions_signing_config: CardanoTransactionsSigningConfig,
10+
pub cardano_transactions_signing_config: Option<CardanoTransactionsSigningConfig>,
1111
}
1212

1313
#[cfg(test)]
@@ -31,7 +31,7 @@ mod test_utils {
3131
protocol_parameters: self.protocol_parameters,
3232
enabled_signed_entity_types,
3333
signed_entity_types_config: SignedEntityTypeConfiguration {
34-
cardano_transactions: Some(self.cardano_transactions_signing_config),
34+
cardano_transactions: self.cardano_transactions_signing_config,
3535
},
3636
}
3737
}

‎mithril-aggregator/src/services/epoch_service.rs‎

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -316,10 +316,7 @@ impl EpochService for MithrilEpochService {
316316
.configuration_for_registration
317317
.signed_entity_types_config
318318
.cardano_transactions
319-
.clone()
320-
.ok_or(anyhow!(
321-
"Missing cardano transactions signing config for registration epoch {epoch:?}"
322-
))?,
319+
.clone(),
323320
};
324321
self.insert_epoch_settings(
325322
signer_registration_epoch,
@@ -635,15 +632,15 @@ impl FakeEpochService {
635632

636633
let current_epoch_settings = AggregatorEpochSettings {
637634
protocol_parameters: fixture.protocol_parameters(),
638-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(),
635+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig::dummy()),
639636
};
640637
let next_epoch_settings = AggregatorEpochSettings {
641638
protocol_parameters: fixture.protocol_parameters(),
642-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(),
639+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig::dummy()),
643640
};
644641
let signer_registration_epoch_settings = AggregatorEpochSettings {
645642
protocol_parameters: fixture.protocol_parameters(),
646-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(),
643+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig::dummy()),
647644
};
648645

649646
FakeEpochServiceBuilder {
@@ -908,15 +905,21 @@ mod tests {
908905
next_signers_with_stake: epoch_fixture.signers_with_stake(),
909906
stored_current_epoch_settings: AggregatorEpochSettings {
910907
protocol_parameters: epoch_fixture.protocol_parameters(),
911-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(),
908+
cardano_transactions_signing_config: Some(
909+
CardanoTransactionsSigningConfig::dummy(),
910+
),
912911
},
913912
stored_next_epoch_settings: AggregatorEpochSettings {
914913
protocol_parameters: epoch_fixture.protocol_parameters(),
915-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(),
914+
cardano_transactions_signing_config: Some(
915+
CardanoTransactionsSigningConfig::dummy(),
916+
),
916917
},
917918
stored_signer_registration_epoch_settings: AggregatorEpochSettings {
918919
protocol_parameters: epoch_fixture.protocol_parameters(),
919-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(),
920+
cardano_transactions_signing_config: Some(
921+
CardanoTransactionsSigningConfig::dummy(),
922+
),
920923
},
921924
total_spo: 1,
922925
total_stake: 0,
@@ -1082,7 +1085,9 @@ mod tests {
10821085
let mut service = EpochServiceBuilder {
10831086
allowed_discriminants: allowed_discriminants.clone(),
10841087
stored_current_epoch_settings: AggregatorEpochSettings {
1085-
cardano_transactions_signing_config: cardano_transactions_signing_config.clone(),
1088+
cardano_transactions_signing_config: Some(
1089+
cardano_transactions_signing_config.clone(),
1090+
),
10861091
..AggregatorEpochSettings::dummy()
10871092
},
10881093
..EpochServiceBuilder::new(epoch, MithrilFixtureBuilder::default().build())
@@ -1169,16 +1174,19 @@ mod tests {
11691174
.build();
11701175

11711176
let epoch = Epoch(5);
1172-
let mut service = EpochServiceBuilder {
1173-
stored_next_epoch_settings: AggregatorEpochSettings {
1174-
protocol_parameters: next_epoch_fixture.protocol_parameters(),
1175-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig::dummy(),
1176-
},
1177-
next_signers_with_stake: next_epoch_fixture.signers_with_stake().clone(),
1178-
..EpochServiceBuilder::new(epoch, current_epoch_fixture.clone())
1179-
}
1180-
.build()
1181-
.await;
1177+
let mut service =
1178+
EpochServiceBuilder {
1179+
stored_next_epoch_settings: AggregatorEpochSettings {
1180+
protocol_parameters: next_epoch_fixture.protocol_parameters(),
1181+
cardano_transactions_signing_config: Some(
1182+
CardanoTransactionsSigningConfig::dummy(),
1183+
),
1184+
},
1185+
next_signers_with_stake: next_epoch_fixture.signers_with_stake().clone(),
1186+
..EpochServiceBuilder::new(epoch, current_epoch_fixture.clone())
1187+
}
1188+
.build()
1189+
.await;
11821190

11831191
service
11841192
.inform_epoch(epoch)
@@ -1271,10 +1279,10 @@ mod tests {
12711279
async fn inform_epoch_insert_registration_epoch_settings_in_the_store() {
12721280
let expected_epoch_settings = AggregatorEpochSettings {
12731281
protocol_parameters: ProtocolParameters::new(6, 89, 0.124),
1274-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig {
1282+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig {
12751283
security_parameter: BlockNumber(10),
12761284
step: BlockNumber(15),
1277-
},
1285+
}),
12781286
};
12791287

12801288
let epoch = Epoch(4);
@@ -1286,9 +1294,9 @@ mod tests {
12861294
MithrilNetworkConfigurationForEpoch {
12871295
protocol_parameters: expected_epoch_settings.protocol_parameters.clone(),
12881296
signed_entity_types_config: SignedEntityTypeConfiguration {
1289-
cardano_transactions: Some(
1290-
expected_epoch_settings.cardano_transactions_signing_config.clone(),
1291-
),
1297+
cardano_transactions: expected_epoch_settings
1298+
.cardano_transactions_signing_config
1299+
.clone(),
12921300
},
12931301
..Dummy::dummy()
12941302
},

‎mithril-aggregator/src/services/message.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ impl MessageService for MithrilMessageService {
210210
enabled_discriminants.get(&SignedEntityTypeDiscriminants::CardanoTransactions);
211211

212212
let cardano_transactions_signing_config = cardano_transactions_discriminant
213-
.map(|_| epoch_settings.cardano_transactions_signing_config);
213+
.and(epoch_settings.cardano_transactions_signing_config);
214214

215215
let protocol_configuration_message = ProtocolConfigurationMessage {
216216
protocol_parameters: epoch_settings.protocol_parameters,
@@ -670,10 +670,10 @@ mod tests {
670670
let epoch = Epoch(4);
671671
let aggregator_epoch_settings = AggregatorEpochSettings {
672672
protocol_parameters: ProtocolParameters::new(5, 100, 0.65),
673-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig {
673+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig {
674674
security_parameter: BlockNumber(0),
675675
step: BlockNumber(15),
676-
},
676+
}),
677677
};
678678
let message_service = MessageServiceBuilder::new()
679679
.with_epoch_settings(BTreeMap::from([(epoch, aggregator_epoch_settings)]))

‎mithril-aggregator/src/services/network_configuration_provider.rs‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ impl LocalMithrilNetworkConfigurationProvider {
5757
enabled_signed_entity_types: self.allowed_discriminants.clone(),
5858
protocol_parameters: epoch_settings.protocol_parameters,
5959
signed_entity_types_config: SignedEntityTypeConfiguration {
60-
cardano_transactions: Some(epoch_settings.cardano_transactions_signing_config),
60+
cardano_transactions: epoch_settings.cardano_transactions_signing_config,
6161
},
6262
})
6363
}
@@ -165,10 +165,10 @@ mod tests {
165165
{
166166
let local_configuration_epoch_settings = AggregatorEpochSettings {
167167
protocol_parameters: ProtocolParameters::new(3000, 300, 0.3),
168-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig {
168+
cardano_transactions_signing_config: Some(CardanoTransactionsSigningConfig {
169169
security_parameter: BlockNumber(3),
170170
step: BlockNumber(30),
171-
},
171+
}),
172172
};
173173

174174
// Nothing stored at 44, should fallback to configuration
@@ -180,20 +180,24 @@ mod tests {
180180
Epoch(42),
181181
AggregatorEpochSettings {
182182
protocol_parameters: ProtocolParameters::new(1000, 100, 0.1),
183-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig {
184-
security_parameter: BlockNumber(1),
185-
step: BlockNumber(10),
186-
},
183+
cardano_transactions_signing_config: Some(
184+
CardanoTransactionsSigningConfig {
185+
security_parameter: BlockNumber(1),
186+
step: BlockNumber(10),
187+
},
188+
),
187189
},
188190
),
189191
(
190192
Epoch(43),
191193
AggregatorEpochSettings {
192194
protocol_parameters: ProtocolParameters::new(2000, 200, 0.2),
193-
cardano_transactions_signing_config: CardanoTransactionsSigningConfig {
194-
security_parameter: BlockNumber(2),
195-
step: BlockNumber(20),
196-
},
195+
cardano_transactions_signing_config: Some(
196+
CardanoTransactionsSigningConfig {
197+
security_parameter: BlockNumber(2),
198+
step: BlockNumber(20),
199+
},
200+
),
197201
},
198202
),
199203
])),

‎mithril-aggregator/src/store/epoch_settings_storer.rs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
use anyhow::anyhow;
21
use async_trait::async_trait;
32
#[cfg(test)]
43
use std::collections::HashMap;
@@ -68,10 +67,7 @@ pub trait EpochSettingsStorer:
6867
cardano_transactions_signing_config: epoch_configuration
6968
.signed_entity_types_config
7069
.cardano_transactions
71-
.clone()
72-
.ok_or(anyhow!(
73-
"Missing cardano transactions signing config for epoch {epoch}"
74-
))?,
70+
.clone(),
7571
},
7672
)
7773
.await?;

‎mithril-aggregator/src/test/double/dummies.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ mod entities {
5454
/// Create a dummy `AggregatorEpochSettings`
5555
fn dummy() -> Self {
5656
let protocol_parameters = fake_data::protocol_parameters();
57-
let cardano_transactions_signing_config = CardanoTransactionsSigningConfig::dummy();
57+
let cardano_transactions_signing_config =
58+
Some(CardanoTransactionsSigningConfig::dummy());
5859

5960
// Aggregator Epoch settings
6061
AggregatorEpochSettings {

0 commit comments

Comments
 (0)