From f505c2992689d32b6084bfdfb3f7b053fed76dce Mon Sep 17 00:00:00 2001 From: Simon Wicky Date: Wed, 20 May 2026 11:33:44 +0200 Subject: [PATCH] some PR review --- common/nym-lp-data/README.md | 2 +- common/nym-lp-data/src/common/traits.rs | 2 +- common/nym-lp-data/src/lib.rs | 37 ++----------------- .../src/{mixnodes => nymnodes}/mod.rs | 0 .../src/{mixnodes => nymnodes}/traits.rs | 0 .../tests/integration/common/mod.rs | 22 +++++------ common/nym-lp-data/tests/integration/main.rs | 11 +++--- nym-mix-sim/src/bin/mix_client.rs | 2 +- nym-mix-sim/src/node/simple.rs | 2 +- nym-mix-sim/src/node/sphinx.rs | 2 +- nym-mix-sim/src/packet/simple.rs | 7 ++-- 11 files changed, 29 insertions(+), 58 deletions(-) rename common/nym-lp-data/src/{mixnodes => nymnodes}/mod.rs (100%) rename common/nym-lp-data/src/{mixnodes => nymnodes}/traits.rs (100%) diff --git a/common/nym-lp-data/README.md b/common/nym-lp-data/README.md index 49212888d6..32a5df9569 100644 --- a/common/nym-lp-data/README.md +++ b/common/nym-lp-data/README.md @@ -86,7 +86,7 @@ Implementors fill in `mix()`; everything else is provided by the [`MixnodeProces - **Client-stage no-op marker traits** ([`NoOpReliability`], [`NoOpRoutingSecurity`], [`NoOpObfuscation`] in [`clients/helpers.rs`](src/clients/helpers.rs)) — implement these to opt out of a pipeline stage with zero overhead. Useful for stub or testing pipelines. - **Wire-layer no-op marker traits** ([`NoOpWireWrapper`], [`NoOpWireUnwrapper`] in [`common/helpers.rs`](src/common/helpers.rs)) — collapse the entire wire layer (framing + transport, or their inverses) to a pass-through. Use these when your packet type is already self-contained on the wire (e.g. a Sphinx packet) and needs no extra framing or transport header. `NoOpWireWrapper` requires `Pkt: From>`; `NoOpWireUnwrapper` requires `Pkt: Into>` and `Mk: Default`. -- **`Pipeline` composition structs** (in [`clients/types.rs`](src/clients/types.rs) and [`mixnodes/types.rs`](src/mixnodes/types.rs)) — generic structs that aggregate one component per pipeline stage and provide blanket impls of the relevant supertraits, so you can build a working pipeline by plugging in any combination of stage implementations. +- **`Pipeline` composition structs** (in [`clients/types.rs`](src/clients/types.rs)) — generic structs that aggregate one component per pipeline stage and provide blanket impls of the relevant supertraits, so you can build a working pipeline by plugging in any combination of stage implementations. - **[`ClientWrappingPipelineDriver`](src/clients/driver.rs)** — wraps a dyn-compatible client pipeline behind a tick-driven `tick(timestamp) -> Vec<(Pkt, NdId)>` interface, with an internal mpsc channel for application-supplied input payloads. Reads new input only when the internal buffer is empty so buffered packets do not stack additional latency on top. [`NoOpReliability`]: src/clients/helpers.rs diff --git a/common/nym-lp-data/src/common/traits.rs b/common/nym-lp-data/src/common/traits.rs index 66b9102d3a..edfcf5896e 100644 --- a/common/nym-lp-data/src/common/traits.rs +++ b/common/nym-lp-data/src/common/traits.rs @@ -122,7 +122,7 @@ where NdId: Clone, { // IMPORTANT NOTE : This fn can be not constant to allow e.g. flexible MTU - // However, every possible value must be able to accomodate the different overhead. + // However, every possible value must be able to accommodate the different overhead. // If it doesn't, the pipeline becomes unusable fn packet_size(&self) -> usize; diff --git a/common/nym-lp-data/src/lib.rs b/common/nym-lp-data/src/lib.rs index 3ea6a1918a..ac3c8de1ed 100644 --- a/common/nym-lp-data/src/lib.rs +++ b/common/nym-lp-data/src/lib.rs @@ -22,7 +22,7 @@ use std::fmt::Debug; pub mod clients; pub mod common; -pub mod mixnodes; +pub mod nymnodes; pub mod packet; /// Convenience alias for [`TimedData`] when the payload is a raw byte buffer. @@ -38,29 +38,12 @@ pub type PipelinePayload = PipelineData, Opts, NdId> /// pipeline. It is produced by [`clients::traits::Chunking`] and propagated /// unchanged (or with the timestamp transformed) through every subsequent /// pipeline stage until the packet is sent on the wire. -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct TimedData { pub timestamp: Ts, pub data: D, } -impl Debug for TimedData -where - D: Debug, - Ts: Debug, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "TimedData {{")?; - writeln!(f, " data:")?; - let data_debug = format!("{:#?}", &self.data); - for line in data_debug.lines() { - writeln!(f, " {}", line)?; - } - writeln!(f, " timestamp: {:#?},", &self.timestamp)?; - write!(f, "}}") - } -} - impl TimedData { pub fn new(timestamp: Ts, data: D) -> Self { TimedData { timestamp, data } @@ -109,7 +92,7 @@ impl TimedData { /// [`Framing`]: crate::common::traits::Framing /// [`Transport`]: crate::common::traits::Transport /// [`InputOptions`]: crate::clients::InputOptions -#[derive(Clone)] +#[derive(Clone, Debug)] pub struct PipelineData { pub data: TimedData, pub options: Opts, @@ -163,20 +146,6 @@ impl PipelineData { } } -impl Debug for PipelineData -where - D: Debug, - Ts: Debug, - NdId: Debug, -{ - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "PipelineData {{")?; - writeln!(f, " dst: {:#?}", &self.dst)?; - writeln!(f, " data: {:#?}", &self.data)?; - write!(f, "}}") - } -} - /// Convenience alias for [`PipelineData`] when no per-message pipeline options /// are needed. Avoids duplicating the pipeline data structure. pub type AddressedTimedData = PipelineData; diff --git a/common/nym-lp-data/src/mixnodes/mod.rs b/common/nym-lp-data/src/nymnodes/mod.rs similarity index 100% rename from common/nym-lp-data/src/mixnodes/mod.rs rename to common/nym-lp-data/src/nymnodes/mod.rs diff --git a/common/nym-lp-data/src/mixnodes/traits.rs b/common/nym-lp-data/src/nymnodes/traits.rs similarity index 100% rename from common/nym-lp-data/src/mixnodes/traits.rs rename to common/nym-lp-data/src/nymnodes/traits.rs diff --git a/common/nym-lp-data/tests/integration/common/mod.rs b/common/nym-lp-data/tests/integration/common/mod.rs index 15360cde73..d6bcb52de4 100644 --- a/common/nym-lp-data/tests/integration/common/mod.rs +++ b/common/nym-lp-data/tests/integration/common/mod.rs @@ -69,13 +69,13 @@ where } } -pub struct KcpReliability; +pub struct MockReliability; -impl KcpReliability { +impl MockReliability { const HEADER: &[u8; 5] = b"0KCP0"; } -impl Reliability for KcpReliability { +impl Reliability for MockReliability { const OVERHEAD_SIZE: usize = Self::HEADER.len(); fn reliable_encode( &mut self, @@ -94,15 +94,15 @@ impl Reliability for KcpReliability { } } -pub struct SphinxSecurity { +pub struct MockSphinxSecurity { pub nb_frames: usize, } -impl SphinxSecurity { +impl MockSphinxSecurity { const HEADER: &[u8; 8] = b"0SPHINX0"; } -impl RoutingSecurity for SphinxSecurity { +impl RoutingSecurity for MockSphinxSecurity { const OVERHEAD_SIZE: usize = Self::HEADER.len(); fn nb_frames(&self) -> usize { @@ -167,13 +167,13 @@ impl Obfuscation for ReallyOddObfuscation { } } -pub struct LpFraming; +pub struct MockLpFraming; -impl LpFraming { +impl MockLpFraming { const FRAME_ATTRIBUTES: &[u8; 14] = b"0LpFrameAttrs0"; } -impl Framing for LpFraming +impl Framing for MockLpFraming where Ts: Clone, { @@ -204,9 +204,9 @@ where } } -pub struct LpTransport; +pub struct MockLpTransport; -impl Transport for LpTransport { +impl Transport for MockLpTransport { type Frame = LpFrame; const OVERHEAD_SIZE: usize = LpHeader::SIZE; fn to_transport_packet( diff --git a/common/nym-lp-data/tests/integration/main.rs b/common/nym-lp-data/tests/integration/main.rs index 2a2e8ba82f..97c4f8d0fd 100644 --- a/common/nym-lp-data/tests/integration/main.rs +++ b/common/nym-lp-data/tests/integration/main.rs @@ -4,7 +4,8 @@ use nym_lp_data::clients::{traits::ClientWrappingPipeline, types::Pipeline}; use crate::common::{ - KcpReliability, KekwObfuscation, LpFraming, LpTransport, MockChunking, SphinxSecurity, + KekwObfuscation, MockChunking, MockLpFraming, MockLpTransport, MockReliability, + MockSphinxSecurity, }; mod common; @@ -16,13 +17,13 @@ fn empty_input_yields_empty_output() { let mut mock_pipeline = Pipeline { chunking: MockChunking, - reliability: KcpReliability, - security: SphinxSecurity { + reliability: MockReliability, + security: MockSphinxSecurity { nb_frames: security_layer_nb_frames, }, obfuscation: KekwObfuscation, - framing: LpFraming, - transport: LpTransport, + framing: MockLpFraming, + transport: MockLpTransport, packet_size, }; diff --git a/nym-mix-sim/src/bin/mix_client.rs b/nym-mix-sim/src/bin/mix_client.rs index 43dad03915..b731a92be9 100644 --- a/nym-mix-sim/src/bin/mix_client.rs +++ b/nym-mix-sim/src/bin/mix_client.rs @@ -81,7 +81,7 @@ fn main() -> anyhow::Result<()> { break; // EOF } - let text = line.trim_end_matches('\n').trim_end_matches('\r'); + let text = line.trim(); let bytes = text.as_bytes(); // Prepend the destination node ID. diff --git a/nym-mix-sim/src/node/simple.rs b/nym-mix-sim/src/node/simple.rs index 3b666674dc..3b3758723a 100644 --- a/nym-mix-sim/src/node/simple.rs +++ b/nym-mix-sim/src/node/simple.rs @@ -11,7 +11,7 @@ use nym_lp_data::{ Framing, FramingUnwrap, Transport, TransportUnwrap, WireUnwrappingPipeline, WireWrappingPipeline, }, - mixnodes::traits::MixnodeProcessingPipeline, + nymnodes::traits::MixnodeProcessingPipeline, }; use crate::{ diff --git a/nym-mix-sim/src/node/sphinx.rs b/nym-mix-sim/src/node/sphinx.rs index 336bf271be..d8663cef7b 100644 --- a/nym-mix-sim/src/node/sphinx.rs +++ b/nym-mix-sim/src/node/sphinx.rs @@ -9,7 +9,7 @@ use nym_crypto::asymmetric::x25519; use nym_lp_data::{ AddressedTimedData, AddressedTimedPayload, TimedPayload, common::helpers::{NoOpWireUnwrapper, NoOpWireWrapper}, - mixnodes::traits::MixnodeProcessingPipeline, + nymnodes::traits::MixnodeProcessingPipeline, }; use nym_sphinx::SphinxPacket; diff --git a/nym-mix-sim/src/packet/simple.rs b/nym-mix-sim/src/packet/simple.rs index 7a58ae857b..c70aa8ef62 100644 --- a/nym-mix-sim/src/packet/simple.rs +++ b/nym-mix-sim/src/packet/simple.rs @@ -62,13 +62,14 @@ impl SimplePacket { /// /// Layout: 16 bytes UUID (LE) + 48 bytes payload = 64 bytes total. const SIZE: usize = 64; + const UUID_SIZE: usize = 16; /// Create a new [`SimplePacket`] with a freshly generated UUID v4 and the /// provided 48-byte payload. /// /// The payload array is exactly `SIZE - 16 = 48` bytes so that the packet /// serialises to exactly [`SimplePacket::SIZE`] bytes. - pub fn new(data: [u8; Self::SIZE - 16]) -> Self { + pub fn new(data: [u8; Self::SIZE - Self::UUID_SIZE]) -> Self { Self { id: Uuid::new_v4(), data: data.to_vec(), @@ -114,8 +115,8 @@ impl SimplePacket { )); } #[allow(clippy::unwrap_used)] - let uuid = Uuid::from_bytes_le(bytes[0..16].try_into().unwrap()); - let data = bytes[16..Self::SIZE].to_vec(); + let uuid = Uuid::from_bytes_le(bytes[0..Self::UUID_SIZE].try_into().unwrap()); + let data = bytes[Self::UUID_SIZE..Self::SIZE].to_vec(); Ok(SimplePacket { id: uuid, data }) } }