Skip to content

Commit

Permalink
chore(network): add workspace lints to network
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Gilad Chase committed Nov 5, 2024
1 parent ba1ac5c commit 92e7432
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
3 changes: 3 additions & 0 deletions crates/papyrus_network/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
13 changes: 8 additions & 5 deletions crates/papyrus_network/src/discovery/discovery_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions crates/papyrus_network/src/network_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ impl<SwarmT: SwarmTrait> GenericNetworkManager<SwarmT> {
}

fn handle_swarm_event(&mut self, event: SwarmEvent<mixed_behaviour::Event>) {
#[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:?}");
Expand Down Expand Up @@ -375,6 +376,7 @@ impl<SwarmT: SwarmTrait> GenericNetworkManager<SwarmT> {
}
}

#[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,
Expand Down Expand Up @@ -537,6 +539,7 @@ impl<SwarmT: SwarmTrait> GenericNetworkManager<SwarmT> {
) {
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. \
Expand Down Expand Up @@ -565,6 +568,7 @@ impl<SwarmT: SwarmTrait> GenericNetworkManager<SwarmT> {
}

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;
Expand Down

0 comments on commit 92e7432

Please sign in to comment.