Skip to content

Commit

Permalink
Merge pull request #2206 from input-output-hk/jpraynaud/fix-todos
Browse files Browse the repository at this point in the history
Chore: clean TODOs in repository
  • Loading branch information
jpraynaud authored Jan 10, 2025
2 parents 493c182 + 55b2e82 commit 62578b1
Show file tree
Hide file tree
Showing 16 changed files with 64 additions and 104 deletions.
8 changes: 8 additions & 0 deletions internal/mithril-doc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ impl StructDoc {
};
result
}

/// Get a field by its name.
pub fn get_field(&self, name: &str) -> &FieldDoc {
let mut fields = self.data.iter().filter(|f| f.parameter == name);

assert_eq!(1, fields.clone().count());
fields.next().unwrap()
}
}

/// Extractor for struct without Default trait.
Expand Down
18 changes: 5 additions & 13 deletions internal/mithril-doc/src/test_doc_macro.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#[cfg(test)]
mod tests {
use crate::{Documenter, DocumenterDefault, FieldDoc, StructDoc};
use crate::{Documenter, DocumenterDefault, StructDoc};
use config::{Map, Source, Value, ValueKind};

#[allow(dead_code)]
Expand Down Expand Up @@ -41,18 +41,10 @@ mod tests {
}
}

// TODO May be part of StructDoc.
pub fn get_field<'a>(struct_doc: &'a StructDoc, name: &str) -> &'a FieldDoc {
let mut fields = struct_doc.data.iter().filter(|f| f.parameter == name);

assert_eq!(1, fields.clone().count());
fields.next().unwrap()
}

#[test]
fn test_extract_struct_of_default_configuration() {
let doc = MyDefaultConfiguration::extract();
let field = get_field(&doc, "environment");
let field = doc.get_field("environment");

assert_eq!("environment", field.parameter);
assert_eq!("ENVIRONMENT", field.environment_variable.as_ref().unwrap());
Expand All @@ -63,7 +55,7 @@ mod tests {
#[test]
fn test_extract_struct_of_configuration() {
let doc = MyConfiguration::extract();
let field = get_field(&doc, "environment");
let field = doc.get_field("environment");

assert_eq!("environment", field.parameter);
assert_eq!("ENVIRONMENT", field.environment_variable.as_ref().unwrap());
Expand All @@ -75,12 +67,12 @@ mod tests {
fn test_extract_example_of_configuration() {
{
let doc_with_example = MyConfiguration::extract();
let field = get_field(&doc_with_example, "environment");
let field = doc_with_example.get_field("environment");
assert_eq!(Some("dev".to_string()), field.example);
}
{
let doc_without_example = MyDefaultConfiguration::extract();
let field = get_field(&doc_without_example, "environment");
let field = doc_without_example.get_field("environment");
assert_eq!(None, field.example);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use mithril_common::StdResult;
use crate::file_uploaders::{url_sanitizer::sanitize_url_path, FileUploader, FileUri};
use crate::tools;

// TODO: This specific local uploader will be removed.
// It's only used by the legacy snapshot that uploads the entire Cardano database.
/// LocalSnapshotUploader is a file uploader working using local files
pub struct LocalSnapshotUploader {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use crate::http_server::routes::middlewares;
use crate::http_server::routes::router::RouterState;
use crate::http_server::SERVER_BASE_PATH;
use warp::hyper::Uri;
use warp::Filter;

pub fn routes(
Expand All @@ -11,8 +9,6 @@ pub fn routes(
.or(artifact_cardano_full_immutable_snapshot_by_id(router_state))
.or(serve_snapshots_dir(router_state))
.or(snapshot_download(router_state))
.or(artifact_cardano_full_immutable_snapshots_legacy())
.or(artifact_cardano_full_immutable_snapshot_by_id_legacy())
}

/// GET /artifact/snapshots
Expand Down Expand Up @@ -67,34 +63,6 @@ fn serve_snapshots_dir(
.and_then(handlers::ensure_downloaded_file_is_a_snapshot)
}

/// GET /snapshots
// TODO: This legacy route should be removed when this code is released with a new distribution
fn artifact_cardano_full_immutable_snapshots_legacy(
) -> impl Filter<Extract = (impl warp::Reply,), Error = warp::Rejection> + Clone {
warp::path!("snapshots").map(|| {
warp::redirect(
format!("/{SERVER_BASE_PATH}/artifact/snapshots")
.as_str()
.parse::<Uri>()
.unwrap(),
)
})
}

/// GET /snapshot/digest
// TODO: This legacy route should be removed when this code is released with a new distribution
fn artifact_cardano_full_immutable_snapshot_by_id_legacy(
) -> impl Filter<Extract = (impl warp::Reply,), Error = warp::Rejection> + Clone {
warp::path!("snapshot" / String).map(|digest| {
warp::redirect(
format!("/{SERVER_BASE_PATH}/artifact/snapshot/{digest}")
.as_str()
.parse::<Uri>()
.unwrap(),
)
})
}

mod handlers {
use crate::http_server::routes::reply;
use crate::http_server::SERVER_BASE_PATH;
Expand Down
9 changes: 5 additions & 4 deletions mithril-common/src/crypto_helper/cardano/key_certification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ pub type KESPeriod = u32;
#[derive(Error, Debug)]
pub enum ProtocolRegistrationErrorWrapper {
/// Error raised when a party id is needed but not provided
// TODO: Should be removed once the signer certification is fully deployed
///
/// Used only for testing when SPO pool id is not certified
#[error("missing party id")]
PartyIdMissing,

Expand Down Expand Up @@ -246,9 +247,9 @@ impl KeyRegWrapper {
/// Mithril key (with its corresponding Proof of Possession).
pub fn register(
&mut self,
party_id: Option<ProtocolPartyId>, // TODO: Parameter should be removed once the signer certification is fully deployed
opcert: Option<ProtocolOpCert>, // TODO: Option should be removed once the signer certification is fully deployed
kes_sig: Option<ProtocolSignerVerificationKeySignature>, // TODO: Option should be removed once the signer certification is fully deployed
party_id: Option<ProtocolPartyId>, // Used for only for testing when SPO pool id is not certified
opcert: Option<ProtocolOpCert>, // Used for only for testing when SPO pool id is not certified
kes_sig: Option<ProtocolSignerVerificationKeySignature>, // Used for only for testing when SPO pool id is not certified
kes_period: Option<KESPeriod>,
pk: ProtocolSignerVerificationKey,
) -> Result<ProtocolPartyId, ProtocolRegistrationErrorWrapper> {
Expand Down
18 changes: 12 additions & 6 deletions mithril-common/src/entities/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,22 @@ use sha2::{Digest, Sha256};
#[derive(Clone, Eq, Serialize, Deserialize)]
pub struct Signer {
/// The unique identifier of the signer
// TODO: Should be removed once the signer certification is fully deployed
///
/// Used only for testing when SPO pool id is not certified
pub party_id: PartyId,

/// The public key used to authenticate signer signature
pub verification_key: ProtocolSignerVerificationKey,

/// The encoded signer 'Mithril verification key' signature (signed by the Cardano node KES secret key)
// TODO: Option should be removed once the signer certification is fully deployed
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub verification_key_signature: Option<ProtocolSignerVerificationKeySignature>,

/// The encoded operational certificate of stake pool operator attached to the signer node
// TODO: Option should be removed once the signer certification is fully deployed
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub operational_certificate: Option<ProtocolOpCert>,

Expand Down Expand Up @@ -136,19 +139,22 @@ impl From<SignerWithStake> for Signer {
#[derive(Clone, Eq, Serialize, Deserialize)]
pub struct SignerWithStake {
/// The unique identifier of the signer
// TODO: Should be removed once the signer certification is fully deployed
///
/// Used only for testing when SPO pool id is not certified
pub party_id: PartyId,

/// The public key used to authenticate signer signature
pub verification_key: ProtocolSignerVerificationKey,

/// The encoded signer 'Mithril verification key' signature (signed by the Cardano node KES secret key)
// TODO: Option should be removed once the signer certification is fully deployed
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub verification_key_signature: Option<ProtocolSignerVerificationKeySignature>,

/// The encoded operational certificate of stake pool operator attached to the signer node
// TODO: Option should be removed once the signer certification is fully deployed
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub operational_certificate: Option<ProtocolOpCert>,

Expand Down
2 changes: 0 additions & 2 deletions mithril-common/src/entities/single_signatures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,6 @@ impl SingleSignatures {
cfg_test_tools! {
impl SingleSignatures {
/// Create a fake [SingleSignatures] with valid cryptographic data for testing purposes.
// TODO: this method is slow due to the fixture creation, we should either make
// the fixture faster or find a faster alternative.
pub fn fake<TPartyId: Into<String>, TMessage: Into<String>>(party_id: TPartyId, message: TMessage) -> Self {
use crate::entities::{ProtocolParameters};
use crate::test_utils::{MithrilFixtureBuilder, StakeDistributionGenerationMethod};
Expand Down
22 changes: 12 additions & 10 deletions mithril-common/src/messages/message_parts/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,24 @@ use std::fmt::{Debug, Formatter};
#[derive(Clone, PartialEq, Eq, Default, Serialize, Deserialize)]
pub struct SignerWithStakeMessagePart {
/// The unique identifier of the signer
// TODO: Should be removed once the signer certification is fully deployed
///
/// Used only for testing when SPO pool id is not certified
pub party_id: PartyId,

/// The public key used to authenticate signer signature
pub verification_key: HexEncodedVerificationKey,

/// The encoded signer 'Mithril verification key' signature (signed by the
/// Cardano node KES secret key).
// TODO: Option should be removed once the signer certification is fully
// deployed.
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub verification_key_signature: Option<HexEncodedVerificationKeySignature>,

/// The encoded operational certificate of stake pool operator attached to
/// the signer node.
// TODO: Option should be removed once the signer certification is fully
// deployed.
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub operational_certificate: Option<HexEncodedOpCert>,

Expand Down Expand Up @@ -161,23 +162,24 @@ impl Debug for SignerMessagePart {
#[derive(Clone, PartialEq, Eq, Default, Serialize, Deserialize)]
pub struct SignerMessagePart {
/// The unique identifier of the signer
// TODO: Should be removed once the signer certification is fully deployed
///
/// Used only for testing when SPO pool id is not certified
pub party_id: PartyId,

/// The public key used to authenticate signer signature
pub verification_key: HexEncodedVerificationKey,

/// The encoded signer 'Mithril verification key' signature (signed by the
/// Cardano node KES secret key).
// TODO: Option should be removed once the signer certification is fully
// deployed.
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub verification_key_signature: Option<HexEncodedVerificationKeySignature>,

/// The encoded operational certificate of stake pool operator attached to
/// the signer node.
// TODO: Option should be removed once the signer certification is fully
// deployed.
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub operational_certificate: Option<HexEncodedOpCert>,

Expand Down
11 changes: 6 additions & 5 deletions mithril-common/src/messages/register_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,24 @@ pub struct RegisterSignerMessage {
pub epoch: Epoch,

/// The unique identifier of the signer
// TODO: Should be removed once the signer certification is fully deployed
///
/// Used only for testing when SPO pool id is not certified
pub party_id: PartyId,

/// The public key used to authenticate signer signature
pub verification_key: HexEncodedVerificationKey,

/// The encoded signer 'Mithril verification key' signature (signed by the
/// Cardano node KES secret key).
// TODO: Option should be removed once the signer certification is fully
// deployed.
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub verification_key_signature: Option<HexEncodedVerificationKeySignature>,

/// The encoded operational certificate of stake pool operator attached to
/// the signer node.
// TODO: Option should be removed once the signer certification is fully
// deployed.
///
/// None is used only for testing when SPO pool id is not certified
#[serde(skip_serializing_if = "Option::is_none")]
pub operational_certificate: Option<HexEncodedOpCert>,

Expand Down
13 changes: 4 additions & 9 deletions mithril-relay/src/relay/passive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ use slog::{debug, info, Logger};
/// A passive relay
pub struct PassiveRelay {
/// Relay peer
// TODO: should be private
pub peer: Peer,
peer: Peer,
logger: Logger,
}

Expand All @@ -24,13 +23,9 @@ impl PassiveRelay {
})
}

/// Convert event to broadcast message
/// TODO: should be removed
pub fn convert_peer_event_to_message(
&mut self,
event: PeerEvent,
) -> StdResult<Option<BroadcastMessage>> {
self.peer.convert_peer_event_to_message(event)
/// Get the peer of the passive relay
pub fn peer_mut(&mut self) -> &mut Peer {
&mut self.peer
}

/// Tick the passive relay
Expand Down
12 changes: 6 additions & 6 deletions mithril-relay/tests/register_signer_signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,15 @@ async fn should_receive_registrations_from_signers_when_subscribed_to_pubsub() {
.await
.expect("P2P client start failed");
p2p_client1
.peer
.peer_mut()
.dial(relay_peer_address.clone())
.expect("P2P client dial to the relay should not fail");

let mut p2p_client2 = PassiveRelay::start(&addr, &logger)
.await
.expect("P2P client start failed");
p2p_client2
.peer
.peer_mut()
.dial(relay_peer_address.clone())
.expect("P2P client dial to the relay should not fail");

Expand Down Expand Up @@ -138,15 +138,15 @@ async fn should_receive_registrations_from_signers_when_subscribed_to_pubsub() {
loop {
tokio::select! {
event = p2p_client1.tick_peer() => {
if let Ok(Some(BroadcastMessage::RegisterSigner(signer_message_received))) = p2p_client1.convert_peer_event_to_message(event.unwrap().unwrap())
if let Ok(Some(BroadcastMessage::RegisterSigner(signer_message_received))) = p2p_client1.peer_mut().convert_peer_event_to_message(event.unwrap().unwrap())
{
info!("Test: client1 consumed signer registration"; "message" => #?signer_message_received);
assert_eq!(signer_message_sent, signer_message_received);
total_peers_has_received_message += 1
}
}
event = p2p_client2.tick_peer() => {
if let Ok(Some(BroadcastMessage::RegisterSigner(signer_message_received))) = p2p_client2.convert_peer_event_to_message(event.unwrap().unwrap())
if let Ok(Some(BroadcastMessage::RegisterSigner(signer_message_received))) = p2p_client2.peer_mut().convert_peer_event_to_message(event.unwrap().unwrap())
{
info!("Test: client2 consumed signer registration"; "message" => #?signer_message_received);
assert_eq!(signer_message_sent, signer_message_received);
Expand Down Expand Up @@ -183,15 +183,15 @@ async fn should_receive_registrations_from_signers_when_subscribed_to_pubsub() {
loop {
tokio::select! {
event = p2p_client1.tick_peer() => {
if let Ok(Some(BroadcastMessage::RegisterSignature(signature_message_received))) = p2p_client1.convert_peer_event_to_message(event.unwrap().unwrap())
if let Ok(Some(BroadcastMessage::RegisterSignature(signature_message_received))) = p2p_client1.peer_mut().convert_peer_event_to_message(event.unwrap().unwrap())
{
info!("Test: client1 consumed signature"; "message" => #?signature_message_received);
assert_eq!(signature_message_sent, signature_message_received);
total_peers_has_received_message += 1
}
}
event = p2p_client2.tick_peer() => {
if let Ok(Some(BroadcastMessage::RegisterSignature(signature_message_received))) = p2p_client2.convert_peer_event_to_message(event.unwrap().unwrap())
if let Ok(Some(BroadcastMessage::RegisterSignature(signature_message_received))) = p2p_client2.peer_mut().convert_peer_event_to_message(event.unwrap().unwrap())
{
info!("Test: client2 consumed signature"; "message" => #?signature_message_received);
assert_eq!(signature_message_sent, signature_message_received);
Expand Down
3 changes: 2 additions & 1 deletion mithril-signer/src/configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ pub struct Configuration {
pub relay_endpoint: Option<String>,

/// Party Id
// TODO: Field should be removed once the signer certification is fully deployed
///
/// Used only for testing when SPO pool id is not certified
#[example = "`pool1pxaqe80sqpde7902er5kf6v0c7y0sv6d5g676766v2h829fvs3x`"]
pub party_id: Option<PartyId>,

Expand Down
Loading

0 comments on commit 62578b1

Please sign in to comment.