net: call refresh_greylist() inside outbound_session::run()

also cleanup the refresh_greylist() method

src/net/hosts.rs

@@ -33,7 +33,7 @@ use super::{
 };
 use crate::{
     system::{Subscriber, SubscriberPtr, Subscription},
-    Error, Result,
+    Result,
 };
 
 /// Atomic pointer to hosts object
@@ -287,7 +287,7 @@ impl Hosts {
     // Probe random peers on the greylist. If a peer is responsive, update the last_seen field and
     // add it to the whitelist. If a node does not respond, remove it from the greylist.
     // Called periodically.
-    async fn refresh_greylist(&self, p2p: P2pPtr, ex: Arc<Executor<'_>>) -> Result<()> {
+    pub async fn refresh_greylist(&self, p2p: P2pPtr, ex: Arc<Executor<'_>>) {
         let mut greylist = self.greylist.write().await;
         let mut whitelist = self.whitelist.write().await;
 
@@ -297,49 +297,38 @@ impl Hosts {
         let url = &entry.0;
 
         // Probe node to see if it's active.
-        let result: Result<()> = self.probe_node(url, p2p.clone(), ex.clone()).await;
+        let online: bool = self.probe_node(url, p2p.clone(), ex.clone()).await;
 
-        match result {
+        if online {
             // Peer is responsive. Update last_seen and add it to the whitelist.
-            Ok(()) => {
-                let last_seen =
-                    SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs();
-
-                // Remove oldest element if the whitelist reaches max size.
-                if whitelist.len() == 1000 {
-                    // Last element in vector should have the oldest timestamp.
-                    // TODO: Test this
-                    let removed_entry = whitelist.pop();
-                    match removed_entry {
-                        Some(e) => {
-                            debug!(target: "net::hosts::refresh_greylist()", "Whitelist reached max size. Removed host {}", e.0);
-                        }
-                        // TODO: whitelist is empty.
-                        None => {}
-                    }
-                }
-                // Append it to the whitelist.
-                debug!(target: "net::hosts::refresh_greylist()", "Adding peer {} to whitelist", url);
-                whitelist.push((url.clone(), last_seen));
-
-                // Sort whitelist by last_seen.
-                whitelist.sort_unstable_by_key(|entry| entry.1);
+            let last_seen =
+                SystemTime::now().duration_since(SystemTime::UNIX_EPOCH).unwrap().as_secs();
 
-                // Remove whitelisted peer from the greylist.
-                debug!(target: "net::hosts::refresh_greylist()", "Removing whitelisted peer {} to greylist", url);
-                greylist.remove(position);
+            // Remove oldest element if the whitelist reaches max size.
+            if whitelist.len() == 1000 {
+                // Last element in vector should have the oldest timestamp.
+                // This should never crash as only returns None when whitelist len() == 0.
+                let entry = whitelist.pop().unwrap();
+                debug!(target: "net::hosts::refresh_greylist()", "Whitelist reached max size. Removed host {}", entry.0);
             }
+            // Append to the whitelist.
+            debug!(target: "net::hosts::refresh_greylist()", "Adding peer {} to whitelist", url);
+            whitelist.push((url.clone(), last_seen));
+
+            // Sort whitelist by last_seen.
+            whitelist.sort_unstable_by_key(|entry| entry.1);
+
+            // Remove whitelisted peer from the greylist.
+            debug!(target: "net::hosts::refresh_greylist()", "Removing whitelisted peer {} from greylist", url);
+            greylist.remove(position);
+        } else {
             // Peer is not responsive. Remove it from the greylist.
-            Err(e) => {
-                debug!(target: "net::hosts::refresh_greylist()", "Peer {} is not response. Removing from greylist {}", url, e);
-                greylist.remove(position);
-            }
+            debug!(target: "net::hosts::refresh_greylist()", "Peer {} is not response. Removing from greylist", url);
+            greylist.remove(position);
         }
-
-        Ok(())
     }
 
-    async fn probe_node(&self, host: &Url, p2p: P2pPtr, ex: Arc<Executor<'_>>) -> Result<()> {
+    async fn probe_node(&self, host: &Url, p2p: P2pPtr, ex: Arc<Executor<'_>>) -> bool {
         let p2p_ = p2p.clone();
         let ex_ = ex.clone();
         let session_out = p2p_.session_outbound();
@@ -369,18 +358,18 @@ impl Hosts {
                     Ok(()) => {
                         debug!(target: "net::hosts::probe_node()", "Handshake success! Stopping channel.");
                         channel.stop().await;
-                        Ok(())
+                        return true
                     }
                     Err(e) => {
                         debug!(target: "net::hosts::probe_node()", "Handshake failure! {}", e);
-                        Err(Error::ConnectFailed)
+                        return false
                     }
                 }
             }
 
             Err(e) => {
                 debug!(target: "net::hosts::probe_node()", "Failed to connect to {}, ({})", host, e);
-                Err(Error::ConnectFailed)
+                return false
             }
         }
     }

src/net/session/outbound_session.rs

@@ -307,7 +307,8 @@ impl Slot {
             let transports = &self.p2p().settings().allowed_transports;
 
             // Find a whitelisted address to connect to. We also do peer discovery here if needed.
-            let addr = if let Some(addr) = self.whitelist_fetch_address_with_lock(transports).await {
+            let addr = if let Some(addr) = self.whitelist_fetch_address_with_lock(transports).await
+            {
                 addr
             } else {
                 dnetev!(self, OutboundSlotSleeping, {
@@ -393,6 +394,14 @@ impl Slot {
 
             self.channel_id.store(channel.info.id, Ordering::Relaxed);
 
+            // Randomly select a peer on the greylist and probe it.
+            // TODO: put this somewhere better.
+            // TODO: This frequency of this call can be set in net::Settings.
+            // Right now we are just doing at the same frequency of outbound_connect_timeout.
+            let p2p = self.p2p();
+            let ex = self.p2p().executor();
+            hosts.refresh_greylist(p2p, ex).await;
+
             // Wait for channel to close
             stop_sub.receive().await;
             self.channel_id.store(0, Ordering::Relaxed);