clippy and review comments

This commit is contained in:
Jędrzej Stuczyński
2026-03-04 09:08:56 +00:00
parent 0d9d97e31e
commit 4464d12103
8 changed files with 60 additions and 73 deletions
+3 -10
View File
@@ -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))
+16 -16
View File
@@ -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.");
}
}
+32 -36
View File
@@ -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<LpReceiverIndex, LpTransportSession>,
sessions: HashMap<LpReceiverIndex, LpTransportSession>,
}
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<LpAction, LpError> {
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<LpReceiverIndex, LpError> {
self.with_state_machine(lp_id, |sm| sm.receiver_index())
fn get_session_id(&self, lp_id: LpReceiverIndex) -> Result<LpReceiverIndex, LpError> {
self.with_session(lp_id, |sm| sm.receiver_index())
}
pub fn current_packet_cnt(&self, lp_id: LpReceiverIndex) -> Result<PacketCount, LpError> {
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<F, R>(&self, lp_id: LpReceiverIndex, f: F) -> Result<R, LpError>
pub fn with_session<F, R>(&self, lp_id: LpReceiverIndex, f: F) -> Result<R, LpError>
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<F, R>(
&mut self,
lp_id: LpReceiverIndex,
f: F,
) -> Result<R, LpError>
pub fn with_session_mut<F, R>(&mut self, lp_id: LpReceiverIndex, f: F) -> Result<R, LpError>
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<LpReceiverIndex, LpError> {
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);
}
+6 -8
View File
@@ -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")
};
@@ -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};
+1 -1
View File
@@ -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;
@@ -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};