Skip to content

Commit

Permalink
Taking into account the remarks of the PR
Browse files Browse the repository at this point in the history
  • Loading branch information
sfauvel committed Sep 9, 2024
1 parent f5fcf39 commit 9339988
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 48 deletions.
2 changes: 1 addition & 1 deletion mithril-common/src/messages/epoch_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ mod tests {
}
}

// Test the retro compatibility with previous structure.
// Test the backward compatibility with previous structure.
#[test]
fn test_actual_json_deserialized_into_previous_message() {
let json = ACTUAL_JSON;
Expand Down
65 changes: 38 additions & 27 deletions mithril-common/src/messages/message_parts/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,35 +70,46 @@ impl SignerWithStakeMessagePart {

/// Convert a set of signer message parts into a set of signers with stake
pub fn try_into_signers(messages: Vec<Self>) -> StdResult<Vec<SignerWithStake>> {
let mut signers: Vec<SignerWithStake> = Vec::new();
messages
.into_iter()
.map(SignerWithStakeMessagePart::try_into)
.collect()
}
}

for message in messages {
let verification_key_signature: Option<ProtocolSignerVerificationKeySignature> = message.verification_key_signature
.map(|f| f.try_into())
.transpose()
.with_context(|| format!("Error while parsing verification key signature message, party_id = '{}'", message.party_id))?;
let operational_certificate: Option<ProtocolOpCert> = message
.operational_certificate
.map(|f| f.try_into())
.transpose()
.with_context(|| {
format!(
"Error while parsing operational certificate message, party_id = '{}'.",
message.party_id
)
})?;
let value = SignerWithStake {
party_id: message.party_id,
verification_key: message.verification_key.try_into()?,
verification_key_signature,
kes_period: message.kes_period,
operational_certificate,
stake: message.stake,
};
signers.push(value);
}
impl TryInto<SignerWithStake> for SignerWithStakeMessagePart {
type Error = StdError;

Ok(signers)
fn try_into(self) -> Result<SignerWithStake, Self::Error> {
let verification_key_signature: Option<ProtocolSignerVerificationKeySignature> = self
.verification_key_signature
.map(|f| f.try_into())
.transpose()
.with_context(|| {
format!(
"Error while parsing verification key signature message, party_id = '{}'",
self.party_id
)
})?;
let operational_certificate: Option<ProtocolOpCert> = self
.operational_certificate
.map(|f| f.try_into())
.transpose()
.with_context(|| {
format!(
"Error while parsing operational certificate message, party_id = '{}'.",
self.party_id
)
})?;
let value = SignerWithStake {
party_id: self.party_id,
verification_key: self.verification_key.try_into()?,
verification_key_signature,
kes_period: self.kes_period,
operational_certificate,
stake: self.stake,
};
Ok(value)
}
}

Expand Down
21 changes: 13 additions & 8 deletions mithril-signer/src/message_adapters/from_epoch_settings.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,26 @@
use anyhow::Context;
use mithril_common::{
entities::EpochSettings,
messages::{EpochSettingsMessage, FromMessageAdapter, SignerMessagePart},
messages::{EpochSettingsMessage, SignerMessagePart, TryFromMessageAdapter},
StdResult,
};

/// Adapter to convert [EpochSettingsMessage] to [EpochSettings].
pub struct FromEpochSettingsAdapter;

impl FromMessageAdapter<EpochSettingsMessage, EpochSettings> for FromEpochSettingsAdapter {
impl TryFromMessageAdapter<EpochSettingsMessage, EpochSettings> for FromEpochSettingsAdapter {
/// Method to convert.
fn adapt(message: EpochSettingsMessage) -> EpochSettings {
EpochSettings {
fn try_adapt(message: EpochSettingsMessage) -> StdResult<EpochSettings> {
let epoch_settings = EpochSettings {
epoch: message.epoch,
protocol_parameters: message.protocol_parameters,
next_protocol_parameters: message.next_protocol_parameters,
current_signers: SignerMessagePart::try_into_signers(message.current_signers).unwrap(),
next_signers: SignerMessagePart::try_into_signers(message.next_signers).unwrap(),
}
current_signers: SignerMessagePart::try_into_signers(message.current_signers)
.with_context(|| "'FromMessageAdapter' can not convert the current signers")?,
next_signers: SignerMessagePart::try_into_signers(message.next_signers)
.with_context(|| "'FromMessageAdapter' can not convert the next signers")?,
};
Ok(epoch_settings)
}
}

Expand All @@ -27,7 +32,7 @@ mod tests {
fn test_simple_message() {
let message = EpochSettingsMessage::dummy();
let epoch = message.epoch;
let epoch_settings = FromEpochSettingsAdapter::adapt(message);
let epoch_settings = FromEpochSettingsAdapter::try_adapt(message).unwrap();

assert_eq!(epoch, epoch_settings.epoch);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ impl TryFromMessageAdapter<CertificatePendingMessage, CertificatePending>
signed_entity_type: message.signed_entity_type,
protocol_parameters: message.protocol_parameters,
next_protocol_parameters: message.next_protocol_parameters,
// This field is deprecated and should not be used in Signer.
// This field is deprecated and should not be used by the Mithril signer.
signers: vec![],
// This field is deprecated and should not be used in Signer.
// This field is deprecated and should not be used by the Mithril signer.
next_signers: vec![],
};

Expand Down
19 changes: 13 additions & 6 deletions mithril-signer/src/runtime/runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ impl SignerRunner {
.next_signers_with_stake()
.await
}

/// Get the current signers.
async fn get_current_signers(&self) -> StdResult<Vec<Signer>> {
self.services
.epoch_service
.read()
.await
.current_signers()
.cloned()
}
}

#[cfg_attr(test, automock)]
Expand Down Expand Up @@ -264,11 +274,8 @@ impl Runner for SignerRunner {
return Ok(false);
}

// TODO XXX How handle errors ? It occurs in test_create_immutable_files_full_single_signature integration test (no signers in epoch 1)
// TODO XXX Do we warn and return "can not sign" or return an error ?
let current_signer_with_stake: Option<SignerWithStake> = {
let epoch_service = self.services.epoch_service.read().await;
let current_signers = epoch_service.current_signers_with_stake().await;
let current_signer: Option<Signer> = {
let current_signers = self.get_current_signers().await;
if let Ok(signers) = current_signers {
signers
.iter()
Expand All @@ -280,7 +287,7 @@ impl Runner for SignerRunner {
}
};

if let Some(signer) = current_signer_with_stake {
if let Some(signer) = current_signer {
debug!(" > got a Signer from pending certificate");

if let Some(protocol_initializer) = self
Expand Down
10 changes: 7 additions & 3 deletions mithril-signer/src/services/aggregator_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use mithril_common::{
},
messages::{
AggregatorFeaturesMessage, CertificatePendingMessage, EpochSettingsMessage,
FromMessageAdapter, TryFromMessageAdapter, TryToMessageAdapter,
TryFromMessageAdapter, TryToMessageAdapter,
},
StdError, MITHRIL_API_VERSION_HEADER, MITHRIL_SIGNER_VERSION_HEADER,
};
Expand Down Expand Up @@ -197,7 +197,11 @@ impl AggregatorClient for AggregatorHTTPClient {
match response {
Ok(response) => match response.status() {
StatusCode::OK => match response.json::<EpochSettingsMessage>().await {
Ok(message) => Ok(Some(FromEpochSettingsAdapter::adapt(message))),
Ok(message) => {
let epoch_settings = FromEpochSettingsAdapter::try_adapt(message)
.map_err(|e| AggregatorClientError::Adapter(anyhow!(e)))?;
Ok(Some(epoch_settings))
}
Err(err) => Err(AggregatorClientError::JsonParseFailed(anyhow!(err))),
},
StatusCode::PRECONDITION_FAILED => Err(self.handle_api_error(&response)),
Expand Down Expand Up @@ -587,7 +591,7 @@ mod tests {
let epoch_settings = certificate_handler.retrieve_epoch_settings().await;
epoch_settings.as_ref().expect("unexpected error");
assert_eq!(
FromEpochSettingsAdapter::adapt(epoch_settings_expected),
FromEpochSettingsAdapter::try_adapt(epoch_settings_expected).unwrap(),
epoch_settings.unwrap().unwrap()
);
}
Expand Down
2 changes: 1 addition & 1 deletion mithril-signer/src/services/epoch_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use mithril_common::StdResult;

use crate::RunnerError;

/// Errors dedicated to the CertifierService.
/// Errors dedicated to the EpochService.
#[derive(Debug, Error)]
pub enum EpochServiceError {
/// Raised when service has not collected data at least once.
Expand Down

0 comments on commit 9339988

Please sign in to comment.