Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chore: clean TODOs in repository #2206

Merged
merged 8 commits into from
Jan 10, 2025
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