From 92e7432fe9c1c4c7c116706ce40bab824c71398d Mon Sep 17 00:00:00 2001 From: Gilad Chase Date: Tue, 5 Nov 2024 11:07:47 +0200 Subject: [PATCH] chore(network): add workspace lints to network Lior banned `as` repo-wide, unless absolutely necessary. Nearly all as uses can be replaced with [Try]From which doesn't have implicit coercions like as (we encountered several bugs due to these coercions). Motivation: we are standardizing lints across the repo and CI, instead of each crate having separate sets of lints. Note: i left the `as f64` since `gauge!` expects f64, even though the metrics themselves are integers. This is probably fixable by using `counter!` instead of `gauge!`, but i haven't found anyone who knows how to test these metrics so i'm leaving it for now. I left them at the statement level intentionaly, instead of the module level, so that it won't be forgotten. --- crates/papyrus_network/Cargo.toml | 3 +++ .../papyrus_network/src/discovery/discovery_test.rs | 13 ++++++++----- crates/papyrus_network/src/network_manager/mod.rs | 4 ++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/crates/papyrus_network/Cargo.toml b/crates/papyrus_network/Cargo.toml index 3b791b29b1..04a22212a7 100644 --- a/crates/papyrus_network/Cargo.toml +++ b/crates/papyrus_network/Cargo.toml @@ -53,3 +53,6 @@ pretty_assertions.workspace = true tokio = { workspace = true, features = ["full", "sync", "test-util"] } tokio-stream.workspace = true void.workspace = true + +[lints] +workspace = true diff --git a/crates/papyrus_network/src/discovery/discovery_test.rs b/crates/papyrus_network/src/discovery/discovery_test.rs index 97de37edea..a0516a611a 100644 --- a/crates/papyrus_network/src/discovery/discovery_test.rs +++ b/crates/papyrus_network/src/discovery/discovery_test.rs @@ -31,12 +31,12 @@ use crate::mixed_behaviour::BridgedBehaviour; use crate::test_utils::next_on_mutex_stream; const TIMEOUT: Duration = Duration::from_secs(1); -const BOOTSTRAP_DIAL_SLEEP: Duration = Duration::from_secs(1); +const BOOTSTRAP_DIAL_SLEEP_MILLIS: u64 = 1000; // 1 second const CONFIG: DiscoveryConfig = DiscoveryConfig { bootstrap_dial_retry_config: RetryConfig { - base_delay_millis: BOOTSTRAP_DIAL_SLEEP.as_millis() as u64, - max_delay_seconds: BOOTSTRAP_DIAL_SLEEP, + base_delay_millis: BOOTSTRAP_DIAL_SLEEP_MILLIS, + max_delay_seconds: Duration::from_millis(BOOTSTRAP_DIAL_SLEEP_MILLIS), factor: 1, }, heartbeat_interval: Duration::ZERO, @@ -116,8 +116,11 @@ async fn discovery_redials_on_dial_failure() { connection_id: ConnectionId::new_unchecked(0), })); - let event = - check_event_happens_after_given_duration(&mut behaviour, BOOTSTRAP_DIAL_SLEEP).await; + let event = check_event_happens_after_given_duration( + &mut behaviour, + Duration::from_millis(BOOTSTRAP_DIAL_SLEEP_MILLIS), + ) + .await; assert_matches!( event, ToSwarm::Dial{opts} if opts.get_peer_id() == Some(bootstrap_peer_id) diff --git a/crates/papyrus_network/src/network_manager/mod.rs b/crates/papyrus_network/src/network_manager/mod.rs index b41e692af0..ceb7fc4879 100644 --- a/crates/papyrus_network/src/network_manager/mod.rs +++ b/crates/papyrus_network/src/network_manager/mod.rs @@ -253,6 +253,7 @@ impl GenericNetworkManager { } fn handle_swarm_event(&mut self, event: SwarmEvent) { + #[allow(clippy::as_conversions)] // FIXME: use int metrics so `as f64` may be removed. match event { SwarmEvent::ConnectionEstablished { peer_id, .. } => { debug!("Connected to peer id: {peer_id:?}"); @@ -375,6 +376,7 @@ impl GenericNetworkManager { } } + #[allow(clippy::as_conversions)] // FIXME: use int metrics so `as f64` may be removed. fn handle_sqmr_event_new_inbound_session( &mut self, peer_id: PeerId, @@ -537,6 +539,7 @@ impl GenericNetworkManager { ) { let SqmrClientPayload { query, report_receiver, responses_sender } = client_payload; match self.swarm.send_query(query, PeerId::random(), protocol.clone()) { + #[allow(clippy::as_conversions)] // FIXME: use int metrics so `as f64` may be removed. Ok(outbound_session_id) => { debug!( "Network received new query. waiting for peer assignment. \ @@ -565,6 +568,7 @@ impl GenericNetworkManager { } fn report_session_removed_to_metrics(&mut self, session_id: SessionId) { + #[allow(clippy::as_conversions)] // FIXME: use int metrics so `as f64` may be removed. match session_id { SessionId::InboundSessionId(_) => { self.num_active_inbound_sessions -= 1;