Merge pull request #104 from nymtech/feature/remove-topology-equality-check

Feature/remove topology equality check

common/clients/directory-client/src/presence/coconodes.rs

@@ -1,6 +1,4 @@
-use super::PresenceEq;
 use serde::{Deserialize, Serialize};
-use std::collections::HashSet;
 use topology::coco;
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
@@ -33,24 +31,3 @@ impl From<topology::coco::Node> for CocoPresence {
         }
     }
 }
-
-impl PresenceEq for Vec<CocoPresence> {
-    fn presence_eq(&self, other: &Self) -> bool {
-        if self.len() != other.len() {
-            return false;
-        }
-
-        // we can't take the whole thing into set as it does not implement 'Eq' and we can't
-        // derive it as we don't want to take 'last_seen' into consideration
-        let self_set: HashSet<_> = self
-            .iter()
-            .map(|c| (&c.host, &c.pub_key, &c.version))
-            .collect();
-        let other_set: HashSet<_> = other
-            .iter()
-            .map(|c| (&c.host, &c.pub_key, &c.version))
-            .collect();
-
-        self_set == other_set
-    }
-}

common/clients/directory-client/src/presence/mixnodes.rs

@@ -1,6 +1,4 @@
-use super::PresenceEq;
 use serde::{Deserialize, Serialize};
-use std::collections::HashSet;
 use std::convert::TryInto;
 use std::io;
 use std::net::ToSocketAddrs;
@@ -49,24 +47,3 @@ impl From<topology::mix::Node> for MixNodePresence {
         }
     }
 }
-
-impl PresenceEq for Vec<MixNodePresence> {
-    fn presence_eq(&self, other: &Self) -> bool {
-        if self.len() != other.len() {
-            return false;
-        }
-
-        // we can't take the whole thing into set as it does not implement 'Eq' and we can't
-        // derive it as we don't want to take 'last_seen' into consideration
-        let self_set: HashSet<_> = self
-            .iter()
-            .map(|m| (&m.host, &m.pub_key, &m.version, &m.layer))
-            .collect();
-        let other_set: HashSet<_> = other
-            .iter()
-            .map(|m| (&m.host, &m.pub_key, &m.version, &m.layer))
-            .collect();
-
-        self_set == other_set
-    }
-}

common/clients/directory-client/src/presence/mod.rs

@@ -12,12 +12,6 @@ pub mod coconodes;
 pub mod mixnodes;
 pub mod providers;
 
-// special version of 'PartialEq' that does not care about last_seen field
-//(where applicable) or order of elements in vectors
-trait PresenceEq {
-    fn presence_eq(&self, other: &Self) -> bool;
-}
-
 // Topology shows us the current state of the overall Nym network
 #[derive(Clone, Debug, Deserialize, Serialize)]
 #[serde(rename_all = "camelCase")]
@@ -27,23 +21,6 @@ pub struct Topology {
     pub mix_provider_nodes: Vec<providers::MixProviderPresence>,
 }
 
-impl PartialEq for Topology {
-    // we need a custom implementation as the order of nodes does not matter
-    // also we do not care about 'last_seen' when comparing topologies
-    fn eq(&self, other: &Self) -> bool {
-        if !self.coco_nodes.presence_eq(&other.coco_nodes)
-            || !self.mix_nodes.presence_eq(&other.mix_nodes)
-            || !self
-                .mix_provider_nodes
-                .presence_eq(&other.mix_provider_nodes)
-        {
-            return false;
-        }
-
-        true
-    }
-}
-
 impl NymTopology for Topology {
     fn new(directory_server: String) -> Self {
         debug!("Using directory server: {:?}", directory_server);

common/clients/directory-client/src/presence/providers.rs

@@ -1,6 +1,4 @@
-use super::PresenceEq;
 use serde::{Deserialize, Serialize};
-use std::collections::HashSet;
 use topology::provider;
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
@@ -14,32 +12,6 @@ pub struct MixProviderPresence {
     pub version: String,
 }
 
-impl PresenceEq for MixProviderPresence {
-    fn presence_eq(&self, other: &Self) -> bool {
-        if self.registered_clients.len() != other.registered_clients.len() {
-            return false;
-        }
-
-        if self.client_listener != other.client_listener
-            || self.mixnet_listener != other.mixnet_listener
-            || self.pub_key != other.pub_key
-            || self.version != other.version
-        {
-            return false;
-        }
-
-        let clients_self_set: HashSet<_> =
-            self.registered_clients.iter().map(|c| &c.pub_key).collect();
-        let clients_other_set: HashSet<_> = other
-            .registered_clients
-            .iter()
-            .map(|c| &c.pub_key)
-            .collect();
-
-        clients_self_set == clients_other_set
-    }
-}
-
 impl Into<topology::provider::Node> for MixProviderPresence {
     fn into(self) -> topology::provider::Node {
         topology::provider::Node {
@@ -95,55 +67,3 @@ impl From<topology::provider::Client> for MixProviderClient {
         }
     }
 }
-
-impl PresenceEq for Vec<MixProviderPresence> {
-    fn presence_eq(&self, other: &Self) -> bool {
-        if self.len() != other.len() {
-            return false;
-        }
-
-        // we can't take the whole thing into set as it does not implement 'Eq' and we can't
-        // derive it as we don't want to take 'last_seen' into consideration.
-        // We also don't care about order of registered_clients
-
-        // since we're going to be getting rid of this very soon anyway, just clone registered
-        // clients vector and sort it
-
-        let self_set: HashSet<_> = self
-            .iter()
-            .map(|p| {
-                (
-                    &p.client_listener,
-                    &p.mixnet_listener,
-                    &p.pub_key,
-                    &p.version,
-                    p.registered_clients
-                        .iter()
-                        .cloned()
-                        .map(|c| c.pub_key)
-                        .collect::<Vec<_>>()
-                        .sort(),
-                )
-            })
-            .collect();
-        let other_set: HashSet<_> = other
-            .iter()
-            .map(|p| {
-                (
-                    &p.client_listener,
-                    &p.mixnet_listener,
-                    &p.pub_key,
-                    &p.version,
-                    p.registered_clients
-                        .iter()
-                        .cloned()
-                        .map(|c| c.pub_key)
-                        .collect::<Vec<_>>()
-                        .sort(),
-                )
-            })
-            .collect();
-
-        self_set == other_set
-    }
-}

common/topology/src/lib.rs

@@ -10,7 +10,7 @@ mod filter;
 pub mod mix;
 pub mod provider;
 
-pub trait NymTopology: Sized + PartialEq + std::fmt::Debug + Send + Sync {
+pub trait NymTopology: Sized + std::fmt::Debug + Send + Sync {
     fn new(directory_server: String) -> Self;
     fn new_from_nodes(
         mix_nodes: Vec<mix::Node>,

nym-client/src/client/topology_control.rs

@@ -58,7 +58,7 @@ impl<T: NymTopology> TopologyControl<T> {
 
         let healthcheck_scores = match healthcheck_result {
             Err(err) => {
-                error!("Error while performing the healtcheck: {:?}", err);
+                error!("Error while performing the healthcheck: {:?}", err);
                 return Err(TopologyError::HealthCheckError);
             }
             Ok(scores) => scores,
@@ -91,18 +91,6 @@ impl<T: NymTopology> TopologyControl<T> {
         write_lock.topology = new_topology;
     }
 
-    async fn should_update_topology(&mut self, new_topology: &Option<T>) -> bool {
-        let read_lock = self.inner.read().await;
-        match new_topology {
-            // if new topology is invalid, we MUST update to it as it is impossible to send packets through
-            None => true,
-            Some(new_topology) => match &read_lock.topology {
-                None => true,
-                Some(old_topology) => new_topology != old_topology,
-            },
-        }
-    }
-
     pub(crate) async fn run_refresher(mut self) {
         info!("Starting topology refresher");
         let delay_duration = Duration::from_secs_f64(self.refresh_rate);
@@ -119,12 +107,7 @@ impl<T: NymTopology> TopologyControl<T> {
                 }
             };
 
-            if self.should_update_topology(&new_topology).await {
-                info!("Detected changes in topology - updating global view!");
-
-                self.update_global_topology(new_topology).await;
-            }
-
+            self.update_global_topology(new_topology).await;
             tokio::time::delay_for(delay_duration).await;
         }
     }