From 4464d12103abd675a521611112e4baa603889ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Stuczy=C5=84ski?= Date: Wed, 4 Mar 2026 09:08:56 +0000 Subject: [PATCH] clippy and review comments --- common/nym-lp/src/session.rs | 13 +--- common/nym-lp/src/session_integration/mod.rs | 32 ++++----- common/nym-lp/src/session_manager.rs | 68 +++++++++---------- nym-node/src/node/lp/control/handler.rs | 14 ++-- .../src/lp_client/client.rs | 2 +- nym-registration-client/src/lp_client/mod.rs | 2 +- .../src/lp_client/nested_session/mod.rs | 2 +- ..._machine_helpers.rs => session_helpers.rs} | 0 8 files changed, 60 insertions(+), 73 deletions(-) rename nym-registration-client/src/lp_client/{state_machine_helpers.rs => session_helpers.rs} (100%) diff --git a/common/nym-lp/src/session.rs b/common/nym-lp/src/session.rs index 02d5effa23..d7b8d29999 100644 --- a/common/nym-lp/src/session.rs +++ b/common/nym-lp/src/session.rs @@ -311,20 +311,13 @@ impl LpTransportSession { let ctr = packet.outer_header().counter; // 1. Check replay protection - if let Err(e) = self.receiving_counter_quick_check(ctr) { - return Err(e); - } + self.receiving_counter_quick_check(ctr)?; // 2. decrypt the packet and attempt to deliver data - let packet = match self.decrypt_packet(packet) { - Ok(packet) => packet, - Err(e) => return Err(e), - }; + let packet = self.decrypt_packet(packet)?; // 3. Mark counter as received - if let Err(e) = self.receiving_counter_mark(ctr) { - return Err(e); - } + self.receiving_counter_mark(ctr)?; // 4. deliver the message Ok(LpAction::DeliverData(packet.message)) diff --git a/common/nym-lp/src/session_integration/mod.rs b/common/nym-lp/src/session_integration/mod.rs index b628c4324b..f60f8e5d13 100644 --- a/common/nym-lp/src/session_integration/mod.rs +++ b/common/nym-lp/src/session_integration/mod.rs @@ -41,10 +41,10 @@ mod tests { // 2. Create sessions using the pre-built Noise states let peer_a_sm = session_manager_1 - .create_session_state_machine(sessions.initiator) + .insert_session(sessions.initiator) .unwrap(); let peer_b_sm = session_manager_2 - .create_session_state_machine(sessions.responder) + .insert_session(sessions.responder) .unwrap(); // 3. Send multiple encrypted messages both ways @@ -129,24 +129,24 @@ mod tests { let session2 = sessions.responder; // 2. Create a session (using real noise state) - let _session = session_manager.create_session_state_machine(session1); + let _session = session_manager.insert_session(session1); // 3. Try to get a non-existent session - let result = session_manager.state_machine_exists(non_existent); + let result = session_manager.session_exists(non_existent); assert!(!result, "Non-existent session should return None"); // 4. Try to remove a non-existent session - let result = session_manager.remove_state_machine(non_existent); + let result = session_manager.remove_session(non_existent); assert!( !result, "Remove session should not remove a non-existent session" ); // 5. Create and immediately remove a session - let _temp_session = session_manager.create_session_state_machine(session2); + let _temp_session = session_manager.insert_session(session2); assert!( - session_manager.remove_state_machine(session_id), + session_manager.remove_session(session_id), "Should remove the session" ); } @@ -170,16 +170,16 @@ mod tests { // 2. Create sessions state machines session_manager_1 - .create_session_state_machine(sessions.initiator) + .insert_session(sessions.initiator) .unwrap(); session_manager_2 - .create_session_state_machine(sessions.responder) + .insert_session(sessions.responder) .unwrap(); assert_eq!(session_manager_1.session_count(), 1); assert_eq!(session_manager_2.session_count(), 1); - assert!(session_manager_1.state_machine_exists(session_id)); - assert!(session_manager_2.state_machine_exists(session_id)); + assert!(session_manager_1.session_exists(session_id)); + assert!(session_manager_2.session_exists(session_id)); // --- 3. Simulate Data Transfer via process_input --- println!("Starting data transfer simulation via process_input..."); @@ -310,15 +310,15 @@ mod tests { println!("Out-of-order test passed."); // --- 6. Session Removal --- - assert!(session_manager_1.remove_state_machine(session_id)); + assert!(session_manager_1.remove_session(session_id)); assert_eq!(session_manager_1.session_count(), 0); - assert!(!session_manager_1.state_machine_exists(session_id)); + assert!(!session_manager_1.session_exists(session_id)); // B's session manager still has it until removed - assert!(session_manager_2.state_machine_exists(session_id)); - assert!(session_manager_2.remove_state_machine(session_id)); + assert!(session_manager_2.session_exists(session_id)); + assert!(session_manager_2.remove_session(session_id)); assert_eq!(session_manager_2.session_count(), 0); - assert!(!session_manager_2.state_machine_exists(session_id)); + assert!(!session_manager_2.session_exists(session_id)); println!("Session removal test passed."); } } diff --git a/common/nym-lp/src/session_manager.rs b/common/nym-lp/src/session_manager.rs index d944b478ca..a005b30cdd 100644 --- a/common/nym-lp/src/session_manager.rs +++ b/common/nym-lp/src/session_manager.rs @@ -19,7 +19,7 @@ use crate::session::{LpAction, LpInput}; /// The SessionManager is responsible for creating, storing, and retrieving sessions pub struct SessionManager { /// Manages state machines directly, keyed by lp_id - state_machines: HashMap, + sessions: HashMap, } impl Default for SessionManager { @@ -32,7 +32,7 @@ impl SessionManager { /// Creates a new session manager with empty session storage. pub fn new() -> Self { Self { - state_machines: HashMap::new(), + sessions: HashMap::new(), } } @@ -41,7 +41,7 @@ impl SessionManager { lp_id: LpReceiverIndex, input: LpInput, ) -> Result { - self.with_state_machine_mut(lp_id, |sm| sm.process_input(input))? + self.with_session_mut(lp_id, |sm| sm.process_input(input))? } pub fn send_data( @@ -61,27 +61,27 @@ impl SessionManager { } #[cfg(test)] - fn get_state_machine_id(&self, lp_id: LpReceiverIndex) -> Result { - self.with_state_machine(lp_id, |sm| sm.receiver_index()) + fn get_session_id(&self, lp_id: LpReceiverIndex) -> Result { + self.with_session(lp_id, |sm| sm.receiver_index()) } pub fn current_packet_cnt(&self, lp_id: LpReceiverIndex) -> Result { - self.with_state_machine(lp_id, |sm| Ok(sm.current_packet_cnt()))? + self.with_session(lp_id, |sm| Ok(sm.current_packet_cnt()))? } pub fn session_count(&self) -> usize { - self.state_machines.len() + self.sessions.len() } - pub fn state_machine_exists(&self, lp_id: LpReceiverIndex) -> bool { - self.state_machines.contains_key(&lp_id) + pub fn session_exists(&self, lp_id: LpReceiverIndex) -> bool { + self.sessions.contains_key(&lp_id) } - pub fn with_state_machine(&self, lp_id: LpReceiverIndex, f: F) -> Result + pub fn with_session(&self, lp_id: LpReceiverIndex, f: F) -> Result where F: FnOnce(&LpTransportSession) -> R, { - if let Some(sm) = self.state_machines.get(&lp_id) { + if let Some(sm) = self.sessions.get(&lp_id) { Ok(f(sm)) } else { Err(LpError::StateMachineNotFound(lp_id)) @@ -89,38 +89,34 @@ impl SessionManager { } // For mutable access (like running process_input) - pub fn with_state_machine_mut( - &mut self, - lp_id: LpReceiverIndex, - f: F, - ) -> Result + pub fn with_session_mut(&mut self, lp_id: LpReceiverIndex, f: F) -> Result where F: FnOnce(&mut LpTransportSession) -> R, // Closure takes mutable ref { - if let Some(sm) = self.state_machines.get_mut(&lp_id) { + if let Some(sm) = self.sessions.get_mut(&lp_id) { Ok(f(sm)) } else { Err(LpError::StateMachineNotFound(lp_id)) } } - pub fn create_session_state_machine( + pub fn insert_session( &mut self, lp_session: LpTransportSession, ) -> Result { let session_id = lp_session.receiver_index(); - if self.state_machines.contains_key(&session_id) { + if self.sessions.contains_key(&session_id) { return Err(LpError::DuplicateSessionId(session_id)); } - self.state_machines.insert(session_id, lp_session); + self.sessions.insert(session_id, lp_session); Ok(session_id) } /// Method to remove a state machine - pub fn remove_state_machine(&mut self, lp_id: LpReceiverIndex) -> bool { - let removed = self.state_machines.remove(&lp_id); + pub fn remove_session(&mut self, lp_id: LpReceiverIndex) -> bool { + let removed = self.sessions.remove(&lp_id); removed.is_some() } @@ -138,13 +134,13 @@ mod tests { let local_session = mock_session_for_test(); let id = local_session.receiver_index(); - let sm_1_id = manager.create_session_state_machine(local_session).unwrap(); + let sm_1_id = manager.insert_session(local_session).unwrap(); assert_eq!(sm_1_id, id); - let retrieved = manager.state_machine_exists(id); + let retrieved = manager.session_exists(id); assert!(retrieved); - let not_found = manager.state_machine_exists(123); + let not_found = manager.session_exists(123); assert!(!not_found); } @@ -152,13 +148,13 @@ mod tests { fn test_session_manager_remove() { let mut manager = SessionManager::new(); let local_session = mock_session_for_test(); - let sm_1_id = manager.create_session_state_machine(local_session).unwrap(); + let sm_1_id = manager.insert_session(local_session).unwrap(); - let removed = manager.remove_state_machine(sm_1_id); + let removed = manager.remove_session(sm_1_id); assert!(removed); assert_eq!(manager.session_count(), 0); - let removed_again = manager.remove_state_machine(sm_1_id); + let removed_again = manager.remove_session(sm_1_id); assert!(!removed_again); } @@ -170,15 +166,15 @@ mod tests { let session2 = SessionsMock::mock_seeded_post_handshake(124, kem).initiator; let session3 = SessionsMock::mock_seeded_post_handshake(125, kem).initiator; - let sm_1 = manager.create_session_state_machine(session1).unwrap(); - let sm_2 = manager.create_session_state_machine(session2).unwrap(); - let sm_3 = manager.create_session_state_machine(session3).unwrap(); + let sm_1 = manager.insert_session(session1).unwrap(); + let sm_2 = manager.insert_session(session2).unwrap(); + let sm_3 = manager.insert_session(session3).unwrap(); assert_eq!(manager.session_count(), 3); - let retrieved1 = manager.get_state_machine_id(sm_1).unwrap(); - let retrieved2 = manager.get_state_machine_id(sm_2).unwrap(); - let retrieved3 = manager.get_state_machine_id(sm_3).unwrap(); + let retrieved1 = manager.get_session_id(sm_1).unwrap(); + let retrieved2 = manager.get_session_id(sm_2).unwrap(); + let retrieved3 = manager.get_session_id(sm_3).unwrap(); assert_eq!(retrieved1, sm_1); assert_eq!(retrieved2, sm_2); @@ -192,10 +188,10 @@ mod tests { let sesion = mock_session_for_test(); - let sm = manager.create_session_state_machine(sesion).unwrap(); + let sm = manager.insert_session(sesion).unwrap(); assert_eq!(manager.session_count(), 1); - let retrieved = manager.get_state_machine_id(sm); + let retrieved = manager.get_session_id(sm); assert!(retrieved.is_ok()); assert_eq!(retrieved.unwrap(), sm); } diff --git a/nym-node/src/node/lp/control/handler.rs b/nym-node/src/node/lp/control/handler.rs index b9bcd1d6cb..e84d300399 100644 --- a/nym-node/src/node/lp/control/handler.rs +++ b/nym-node/src/node/lp/control/handler.rs @@ -715,8 +715,8 @@ mod tests { let (init, resp) = sessions_for_tests(); let mut init_sm = SessionManager::new(); let mut resp_sm = SessionManager::new(); - resp_sm.create_session_state_machine(resp).unwrap(); - let id = init_sm.create_session_state_machine(init).unwrap(); + resp_sm.insert_session(resp).unwrap(); + let id = init_sm.insert_session(init).unwrap(); // Bind to localhost let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); @@ -731,8 +731,7 @@ mod tests { let packet = handler.receive_raw_packet().await?; let header = packet.outer_header(); assert_eq!(packet.outer_header().receiver_idx, id); - let Some(LpAction::DeliverData(data)) = resp_sm.receive_packet(id, packet).unwrap() - else { + let LpAction::DeliverData(data) = resp_sm.receive_packet(id, packet)? else { panic!("illegal state") }; Ok::<_, LpHandlerError>((header, data)) @@ -772,8 +771,8 @@ mod tests { let (init, resp) = sessions_for_tests(); let mut init_sm = SessionManager::new(); let mut resp_sm = SessionManager::new(); - resp_sm.create_session_state_machine(resp).unwrap(); - let id = init_sm.create_session_state_machine(init).unwrap(); + resp_sm.insert_session(resp).unwrap(); + let id = init_sm.insert_session(init).unwrap(); let server_task = tokio::spawn(async move { let (mut stream, _) = listener.accept().await.unwrap(); @@ -802,8 +801,7 @@ mod tests { .await .unwrap(); let header = received.outer_header(); - let Some(LpAction::DeliverData(data)) = init_sm.receive_packet(id, received).unwrap() - else { + let LpAction::DeliverData(data) = init_sm.receive_packet(id, received).unwrap() else { panic!("illegal state") }; diff --git a/nym-registration-client/src/lp_client/client.rs b/nym-registration-client/src/lp_client/client.rs index da6dc3bd57..2daa774f07 100644 --- a/nym-registration-client/src/lp_client/client.rs +++ b/nym-registration-client/src/lp_client/client.rs @@ -7,7 +7,7 @@ use super::config::LpRegistrationConfig; use super::error::{LpClientError, Result}; use crate::lp_client::helpers::{LpDataDeliverExt, LpDataSendExt}; use crate::lp_client::nested_session::connection::NestedConnection; -use crate::lp_client::state_machine_helpers::{extract_forwarded_response, prepare_send_packet}; +use crate::lp_client::session_helpers::{extract_forwarded_response, prepare_send_packet}; use nym_bandwidth_controller::{BandwidthTicketProvider, DEFAULT_TICKETS_TO_SPEND}; use nym_credentials_interface::TicketType; use nym_crypto::asymmetric::{ed25519, x25519}; diff --git a/nym-registration-client/src/lp_client/mod.rs b/nym-registration-client/src/lp_client/mod.rs index 492b01ed2e..3e7221c3b0 100644 --- a/nym-registration-client/src/lp_client/mod.rs +++ b/nym-registration-client/src/lp_client/mod.rs @@ -36,7 +36,7 @@ mod config; pub(crate) mod error; pub(crate) mod helpers; mod nested_session; -mod state_machine_helpers; +mod session_helpers; pub use client::LpRegistrationClient; pub use config::LpRegistrationConfig; diff --git a/nym-registration-client/src/lp_client/nested_session/mod.rs b/nym-registration-client/src/lp_client/nested_session/mod.rs index c86b0fdf62..a1e3002976 100644 --- a/nym-registration-client/src/lp_client/nested_session/mod.rs +++ b/nym-registration-client/src/lp_client/nested_session/mod.rs @@ -21,7 +21,7 @@ use super::client::LpRegistrationClient; use super::error::{LpClientError, Result}; use crate::lp_client::helpers::{LpDataDeliverExt, LpDataSendExt}; -use crate::lp_client::state_machine_helpers::{extract_forwarded_response, prepare_send_packet}; +use crate::lp_client::session_helpers::{extract_forwarded_response, prepare_send_packet}; use nym_bandwidth_controller::{BandwidthTicketProvider, DEFAULT_TICKETS_TO_SPEND}; use nym_credentials_interface::TicketType; use nym_crypto::asymmetric::{ed25519, x25519}; diff --git a/nym-registration-client/src/lp_client/state_machine_helpers.rs b/nym-registration-client/src/lp_client/session_helpers.rs similarity index 100% rename from nym-registration-client/src/lp_client/state_machine_helpers.rs rename to nym-registration-client/src/lp_client/session_helpers.rs