Feature/minor healthchecker improvements (#165)

* Integrated new tcp client into healthchecker

* Printing detailed healtcheck score if in debug

* Decreased logging level for when establishing new connection

* Slightly better formatting for printing healthcheck scores

Cargo.lock

@@ -937,7 +937,7 @@ dependencies = [
  "futures 0.3.4",
  "itertools",
  "log 0.4.8",
- "mix-client",
+ "multi-tcp-client",
  "pretty_env_logger",
  "provider-client",
  "rand 0.7.3",

common/clients/multi-tcp-client/src/lib.rs

@@ -66,7 +66,7 @@ impl Client {
         wait_for_response: bool,
     ) -> io::Result<()> {
         if !self.connections_managers.contains_key(&address) {
-            info!(
+            debug!(
                 "There is no existing connection to {:?} - it will be established now",
                 address
             );

common/healthcheck/Cargo.toml

@@ -21,7 +21,7 @@ tokio = { version = "0.2", features = ["full"] }
 addressing = {path = "../addressing" }
 crypto = { path = "../crypto" }
 directory-client = { path = "../clients/directory-client" }
-mix-client = { path = "../clients/mix-client" }
+multi-tcp-client = { path = "../clients/multi-tcp-client" }
 provider-client = { path = "../clients/provider-client" }
 sfw-provider-requests = { path = "../../sfw-provider/sfw-provider-requests" }
 topology = {path = "../topology" }

common/healthcheck/src/path_check.rs

@@ -1,11 +1,11 @@
 use crypto::identity::MixIdentityKeyPair;
 use itertools::Itertools;
 use log::{debug, error, info, trace, warn};
-use mix_client::MixClient;
 use provider_client::{ProviderClient, ProviderClientError};
 use sphinx::header::delays::Delay;
 use sphinx::route::{Destination, Node as SphinxNode};
 use std::collections::HashMap;
+use std::time::Duration;
 use topology::provider;
 
 #[derive(Debug, PartialEq, Clone)]
@@ -17,10 +17,7 @@ pub enum PathStatus {
 
 pub(crate) struct PathChecker {
     provider_clients: HashMap<[u8; 32], Option<ProviderClient>>,
-    // currently this is an overkill as MixClient is extremely cheap to create,
-    // however, once we introduce persistent connection between client and layer one mixes,
-    // this will be extremely helpful to have
-    layer_one_clients: HashMap<[u8; 32], Option<MixClient>>,
+    mixnet_client: multi_tcp_client::Client,
     paths_status: HashMap<Vec<u8>, PathStatus>,
     our_destination: Destination,
     check_id: [u8; 16],
@@ -41,12 +38,12 @@ impl PathChecker {
                 ProviderClient::new(provider.client_listener, address.clone(), None);
             let insertion_result = match provider_client.register().await {
                 Ok(token) => {
-                    debug!("registered at provider {}", provider.pub_key);
+                    debug!("[Healthcheck] registered at provider {}", provider.pub_key);
                     provider_client.update_token(token);
                     provider_clients.insert(provider.get_pub_key_bytes(), Some(provider_client))
                 }
                 Err(ProviderClientError::ClientAlreadyRegisteredError) => {
-                    info!("We were already registered");
+                    info!("[Healthcheck] We were already registered");
                     provider_clients.insert(provider.get_pub_key_bytes(), Some(provider_client))
                 }
                 Err(err) => {
@@ -63,9 +60,15 @@ impl PathChecker {
             }
         }
 
+        // there's no reconnection allowed - if it fails, then it fails.
+        let mixnet_client_config = multi_tcp_client::Config::new(
+            Duration::from_secs(u64::max_value()),
+            Duration::from_secs(u64::max_value()),
+        );
+
         PathChecker {
             provider_clients,
-            layer_one_clients: HashMap::new(),
+            mixnet_client: multi_tcp_client::Client::new(mixnet_client_config),
             our_destination: Destination::new(address, Default::default()),
             paths_status: HashMap::new(),
             check_id,
@@ -212,33 +215,12 @@ impl PathChecker {
         let layer_one_mix = path
             .first()
             .expect("We checked the path to contain at least one entry");
-        let first_node_key = layer_one_mix.pub_key.to_bytes();
 
         // we generated the bytes data so unwrap is fine
         let first_node_address =
             addressing::socket_address_from_encoded_bytes(layer_one_mix.address.to_bytes())
                 .unwrap();
 
-        let first_node_client = self
-            .layer_one_clients
-            .entry(first_node_key)
-            .or_insert_with(|| Some(mix_client::MixClient::new()));
-
-        if first_node_client.is_none() {
-            debug!("we can ignore this path as layer one mix is inaccessible");
-            if self
-                .paths_status
-                .insert(path_identifier, PathStatus::Unhealthy)
-                .is_some()
-            {
-                panic!("Overwriting path checks!")
-            }
-            return;
-        }
-
-        // we already checked for 'None' case
-        let first_node_client = first_node_client.as_ref().unwrap();
-
         let delays: Vec<_> = path.iter().map(|_| Delay::new_from_nanos(0)).collect();
 
         // all of the data used to create the packet was created by us
@@ -251,7 +233,12 @@ impl PathChecker {
         .unwrap();
 
         debug!("sending test packet to {}", first_node_address);
-        match first_node_client.send(packet, first_node_address).await {
+
+        match self
+            .mixnet_client
+            .send(first_node_address, packet.to_bytes(), true)
+            .await
+        {
             Err(err) => {
                 debug!("failed to send packet to {} - {}", first_node_address, err);
                 if self

common/healthcheck/src/score.rs

@@ -153,7 +153,7 @@ impl std::fmt::Display for NodeScore {
         let stringified_key = self.pub_key.to_base58_string();
         write!(
             f,
-            "({})\t{}/{}\t({}%)\t|| {}\tv{} <{}> - {}",
+            "({})\t\t{}/{}\t({:.2}%) \t|| {}\tv{} <{}> - {}",
             self.typ,
             self.packets_received,
             self.packets_sent,

nym-client/src/client/topology_control.rs

@@ -183,6 +183,8 @@ impl<T: 'static + NymTopology> TopologyRefresher<T> {
             Ok(scores) => scores,
         };
 
+        debug!("{}", healthcheck_scores);
+
         let healthy_topology = healthcheck_scores
             .filter_topology_by_score(&version_filtered_topology, self.node_score_threshold);