tm: cleanup clippy warnings

freenet · Nov 6, 2023 · fe8b0ca · fe8b0ca
1 parent 97d2649
commit fe8b0ca
Show file tree

Hide file tree

Showing 7 changed files with 208 additions and 95 deletions.
diff --git a/crates/core/src/topology/connection_evaluator/mod.rs b/crates/core/src/topology/connection_evaluator/mod.rs
@@ -7,10 +7,10 @@ use std::time::{Duration, Instant};
 /// any other scores within a predefined time window. A score is considered better if it's higher
 /// than all other scores in the time window, or if no scores were recorded within the window's
 /// duration.
-/// 
+///
 /// In the Freenet context, this will be used to titrate the rate of new connection requests accepted
 /// by a node. The node will only accept a new connection if the score of the connection is better
-/// than all other scores within the time window. 
+/// than all other scores within the time window.
 pub(crate) struct ConnectionEvaluator {
     scores: VecDeque<(Instant, f64)>,
     window_duration: Duration,

diff --git a/crates/core/src/topology/connection_evaluator/tests.rs b/crates/core/src/topology/connection_evaluator/tests.rs
@@ -4,12 +4,11 @@ use super::*;
 fn test_record_first_score() {
     let mut evaluator = ConnectionEvaluator::new(Duration::from_secs(10));
     let current_time = Instant::now();
-    assert_eq!(evaluator.record_and_eval_with_current_time(5.0, current_time), true);
+    assert!(evaluator.record_and_eval_with_current_time(5.0, current_time));
     // Assert evaluator.scores contains the new score
     assert_eq!(evaluator.scores.len(), 1);
     assert_eq!(evaluator.scores[0].1, 5.0);
     assert_eq!(evaluator.scores[0].0, current_time);
-
 }
 
 #[test]
@@ -18,7 +17,7 @@ fn test_not_best_in_time_window() {
 
     let start_time = Instant::now();
     evaluator.record_and_eval_with_current_time(5.0, start_time);
-    assert_eq!(evaluator.record_and_eval_with_current_time(4.0, start_time + Duration::from_secs(5)), false);
+    assert!(!evaluator.record_and_eval_with_current_time(4.0, start_time + Duration::from_secs(5)),);
 }
 
 #[test]
@@ -27,7 +26,7 @@ fn test_best_in_time_window() {
 
     let start_time = Instant::now();
     evaluator.record_and_eval_with_current_time(5.0, start_time);
-    assert_eq!(evaluator.record_and_eval_with_current_time(4.0, start_time + Duration::from_secs(11)), true);
+    assert!(evaluator.record_and_eval_with_current_time(4.0, start_time + Duration::from_secs(11)),);
 }
 
 #[test]
@@ -45,8 +44,8 @@ fn test_remove_outdated_scores() {
 fn test_empty_window_duration() {
     let mut evaluator = ConnectionEvaluator::new(Duration::from_secs(0));
     let current_time = Instant::now();
-    assert_eq!(evaluator.record_and_eval_with_current_time(5.0, current_time), true);
-    assert_eq!(evaluator.record_and_eval_with_current_time(4.0, current_time), false);
+    assert!(evaluator.record_and_eval_with_current_time(5.0, current_time));
+    assert!(!evaluator.record_and_eval_with_current_time(4.0, current_time));
 }
 
 #[test]
@@ -56,16 +55,20 @@ fn test_multiple_scores_same_timestamp() {
     evaluator.record_only_with_current_time(5.0, current_time);
     evaluator.record_only_with_current_time(6.0, current_time);
     assert_eq!(evaluator.scores.len(), 2);
-    assert_eq!(evaluator.record_and_eval_with_current_time(4.0, current_time + Duration::from_secs(5)), false);
+    assert!(
+        !evaluator.record_and_eval_with_current_time(4.0, current_time + Duration::from_secs(5)),
+    );
 }
 
 #[test]
 fn test_negative_scores() {
     let mut evaluator = ConnectionEvaluator::new(Duration::from_secs(10));
     let start_time = Instant::now();
-    assert_eq!(evaluator.record_and_eval_with_current_time(-5.0, start_time), true);
-    assert_eq!(evaluator.record_and_eval_with_current_time(-4.0, start_time + Duration::from_secs(5)), true);
-    assert_eq!(evaluator.record_and_eval_with_current_time(-6.0, start_time + Duration::from_secs(5)), false);
+    assert!(evaluator.record_and_eval_with_current_time(-5.0, start_time),);
+    assert!(evaluator.record_and_eval_with_current_time(-4.0, start_time + Duration::from_secs(5)),);
+    assert!(
+        !evaluator.record_and_eval_with_current_time(-6.0, start_time + Duration::from_secs(5)),
+    );
 }
 
 #[test]
@@ -75,5 +78,7 @@ fn test_large_number_of_scores() {
     for i in 0..1000 {
         evaluator.record_only_with_current_time(i as f64, start_time + Duration::from_secs(i));
     }
-    assert_eq!(evaluator.record_and_eval_with_current_time(1000.0, start_time + Duration::from_secs(1001)), true);
+    assert!(
+        evaluator.record_and_eval_with_current_time(1000.0, start_time + Duration::from_secs(1001)),
+    );
 }
diff --git a/crates/core/src/topology/mod.rs b/crates/core/src/topology/mod.rs
@@ -12,7 +12,7 @@ mod small_world_rand;
 mod connection_evaluator;
 
 const SLOW_CONNECTION_EVALUATOR_WINDOW_DURATION: Duration = Duration::from_secs(5 * 60);
-const FAST_CONNECTION_EVALUATOR_WINDOW_DURATION: Duration = Duration::from_secs(1 * 60);
+const FAST_CONNECTION_EVALUATOR_WINDOW_DURATION: Duration = Duration::from_secs(60);
 const REQUEST_DENSITY_TRACKER_WINDOW_SIZE: usize = 10_000;
 const REGENERATE_DENSITY_MAP_INTERVAL: Duration = Duration::from_secs(60);
 const RANDOM_CLOSEST_DISTANCE: f64 = 1.0 / 1000.0;

diff --git a/crates/core/src/topology/request_density_tracker/cached_density_map.rs b/crates/core/src/topology/request_density_tracker/cached_density_map.rs
@@ -1,6 +1,10 @@
-use std::{time::{Duration, Instant}, collections::BTreeMap, rc::Rc};
 use crate::ring::Location;
 use crate::topology::request_density_tracker::{self, DensityMapError};
+use std::{
+    collections::BTreeMap,
+    rc::Rc,
+    time::{Duration, Instant},
+};
 
 /// Struct to handle caching of DensityMap
 pub(in crate::topology) struct CachedDensityMap {
@@ -16,7 +20,11 @@ impl CachedDensityMap {
         }
     }
 
-    pub(in crate::topology) fn get_or_create(&mut self, tracker: &request_density_tracker::RequestDensityTracker, current_neighbors: &BTreeMap<Location, usize>) -> Result<Rc<request_density_tracker::DensityMap>, DensityMapError> {
+    pub(in crate::topology) fn get_or_create(
+        &mut self,
+        tracker: &request_density_tracker::RequestDensityTracker,
+        current_neighbors: &BTreeMap<Location, usize>,
+    ) -> Result<Rc<request_density_tracker::DensityMap>, DensityMapError> {
         let now = Instant::now();
         if let Some((density_map, last_update)) = &self.density_map {
             if now.duration_since(*last_update) < self.regenerate_interval {

diff --git a/crates/core/src/topology/request_density_tracker/mod.rs b/crates/core/src/topology/request_density_tracker/mod.rs
@@ -3,9 +3,9 @@ pub mod cached_density_map;
 #[cfg(test)]
 mod tests;
 
+use crate::ring::Location;
 use std::collections::{BTreeMap, LinkedList};
 use thiserror::Error;
-use crate::ring::Location;
 
 /// Tracks requests sent by a node to its neighbors and creates a density map, which
 /// is useful for determining which new neighbors to connect to based on their
@@ -45,35 +45,52 @@ impl RequestDensityTracker {
         }
     }
 
-    pub(crate) fn create_density_map(&self, neighbors: &BTreeMap<Location, usize>) -> Result<DensityMap, DensityMapError> {
+    pub(crate) fn create_density_map(
+        &self,
+        neighbors: &BTreeMap<Location, usize>,
+    ) -> Result<DensityMap, DensityMapError> {
         if neighbors.is_empty() {
             return Err(DensityMapError::EmptyNeighbors);
         }
-    
+
         let smoothing_radius = 2;
         let mut density_map = DensityMap {
             neighbor_request_counts: BTreeMap::new(),
         };
-    
+
         for (sample_location, sample_count) in self.ordered_map.iter() {
-            let previous_neighbor = neighbors.range(..*sample_location).rev().next()
-                .or_else(|| neighbors.iter().rev().next());
-            let next_neighbor = neighbors.range(*sample_location..).next()
+            let previous_neighbor = neighbors
+                .range(..*sample_location)
+                .next_back()
+                .or_else(|| neighbors.iter().next_back());
+            let next_neighbor = neighbors
+                .range(*sample_location..)
+                .next()
                 .or_else(|| neighbors.iter().next());
-
-                match (previous_neighbor, next_neighbor) {
-                    (Some((previous_neighbor_location, _)), Some((next_neighbor_location, _))) => {
-                        if sample_location.distance(*previous_neighbor_location) < sample_location.distance(*next_neighbor_location) {
-                            *density_map.neighbor_request_counts.entry(*previous_neighbor_location).or_insert(0) += sample_count;
-                        } else {
-                            *density_map.neighbor_request_counts.entry(*next_neighbor_location).or_insert(0) += sample_count;
-                        }
-                    },
-                    // The None cases have been removed as they should not occur given the new logic
-                    _ => unreachable!("This shouldn't be possible given that we verify neighbors is not empty"),
+
+            match (previous_neighbor, next_neighbor) {
+                (Some((previous_neighbor_location, _)), Some((next_neighbor_location, _))) => {
+                    if sample_location.distance(*previous_neighbor_location)
+                        < sample_location.distance(*next_neighbor_location)
+                    {
+                        *density_map
+                            .neighbor_request_counts
+                            .entry(*previous_neighbor_location)
+                            .or_insert(0) += sample_count;
+                    } else {
+                        *density_map
+                            .neighbor_request_counts
+                            .entry(*next_neighbor_location)
+                            .or_insert(0) += sample_count;
+                    }
                 }
+                // The None cases have been removed as they should not occur given the new logic
+                _ => unreachable!(
+                    "This shouldn't be possible given that we verify neighbors is not empty"
+                ),
+            }
         }
-    
+
         Ok(density_map)
     }
 }
@@ -89,24 +106,37 @@ impl DensityMap {
         }
 
         // Determine the locations below and above the given location
-        let previous_neighbor = self.neighbor_request_counts.range(..location).rev().next()
-            .or_else(|| self.neighbor_request_counts.iter().rev().next());
-
-        let next_neighbor = self.neighbor_request_counts.range(location..).next()
+        let previous_neighbor = self
+            .neighbor_request_counts
+            .range(..location)
+            .next_back()
+            .or_else(|| self.neighbor_request_counts.iter().next_back());
+
+        let next_neighbor = self
+            .neighbor_request_counts
+            .range(location..)
+            .next()
             .or_else(|| self.neighbor_request_counts.iter().next());
 
         // Determine the value proportionate to the distance to the previous and next neighbor
         let count_estimate = match (previous_neighbor, next_neighbor) {
-            (Some((previous_neighbor_location, previous_neighbor_count)), Some((next_neighbor_location, next_neighbor_count))) => {
-                let previous_neighbor_dist = location.distance(*previous_neighbor_location).as_f64();
+            (
+                Some((previous_neighbor_location, previous_neighbor_count)),
+                Some((next_neighbor_location, next_neighbor_count)),
+            ) => {
+                let previous_neighbor_dist =
+                    location.distance(*previous_neighbor_location).as_f64();
                 let next_neighbor_dist = location.distance(*next_neighbor_location).as_f64();
                 let total_dist = previous_neighbor_dist + next_neighbor_dist;
                 let previous_neighbor_prop = previous_neighbor_dist / total_dist;
                 let next_neighbor_prop = next_neighbor_dist / total_dist;
-                next_neighbor_prop * *previous_neighbor_count as f64 + previous_neighbor_prop * *next_neighbor_count as f64
-            },
+                next_neighbor_prop * *previous_neighbor_count as f64
+                    + previous_neighbor_prop * *next_neighbor_count as f64
+            }
             // The None cases have been removed as they should not occur given the new logic
-            _ => unreachable!("This shouldn't be possible given that we verify neighbors is not empty"),
+            _ => unreachable!(
+                "This shouldn't be possible given that we verify neighbors is not empty"
+            ),
         };
 
         Ok(count_estimate)
@@ -123,25 +153,35 @@ impl DensityMap {
         let mut max_density = 0;
 
         for (
-            (previous_neighbor_location, previous_neighbor_count), (next_neighbor_location, next_neighbor_count)) 
-            in 
-            self.neighbor_request_counts.iter().zip(self.neighbor_request_counts.iter().skip(1)) {
+            (previous_neighbor_location, previous_neighbor_count),
+            (next_neighbor_location, next_neighbor_count),
+        ) in self
+            .neighbor_request_counts
+            .iter()
+            .zip(self.neighbor_request_counts.iter().skip(1))
+        {
             let combined_count = previous_neighbor_count + next_neighbor_count;
             if combined_count > max_density {
                 max_density = combined_count;
-                max_density_location = Location::new((previous_neighbor_location.as_f64() + next_neighbor_location.as_f64()) / 2.0);
+                max_density_location = Location::new(
+                    (previous_neighbor_location.as_f64() + next_neighbor_location.as_f64()) / 2.0,
+                );
             }
         }
 
         // We need to also check the first and last neighbors as locations are circular
         let first_neighbor = self.neighbor_request_counts.iter().next();
-        let last_neighbor = self.neighbor_request_counts.iter().rev().next();
-        if let (Some((first_neighbor_location, first_neighbor_count)), Some((last_neighbor_location, last_neighbor_count))) = (first_neighbor, last_neighbor) {
+        let last_neighbor = self.neighbor_request_counts.iter().next_back();
+        if let (
+            Some((first_neighbor_location, first_neighbor_count)),
+            Some((last_neighbor_location, last_neighbor_count)),
+        ) = (first_neighbor, last_neighbor)
+        {
             let combined_count = first_neighbor_count + last_neighbor_count;
             if combined_count > max_density {
                 // max_density = combined_count; Not needed as this is the last check
                 let distance = first_neighbor_location.distance(*last_neighbor_location);
-                let mut mp = first_neighbor_location.as_f64() - (distance.as_f64()/2.0);
+                let mut mp = first_neighbor_location.as_f64() - (distance.as_f64() / 2.0);
                 if mp < 0.0 {
                     mp += 1.0;
                 }
@@ -160,16 +200,11 @@ pub enum DensityError {
     CantFindBounds,
 
     #[error("Window radius too big. Window radius should be <= 50% of the number of samples ({samples}) and window size ({window_size}).")]
-    WindowTooBig {
-        samples: usize,
-        window_size: usize,
-    },
+    WindowTooBig { samples: usize, window_size: usize },
 }
 
 #[derive(Error, Debug)]
 pub enum DensityMapError {
     #[error("The neighbors BTreeMap is empty.")]
     EmptyNeighbors,
 }
-
-