net: fix bug in outbound session that was restricting slot connections

Previously we were using the wrong variable in
fetch_addrs_with_preference, such that Greylist entries were always
returned when in fact we want to select from the Gold and White list
first.

We also clean up the code and simplify the associated net settings.

src/net/hosts/store.rs

@@ -378,7 +378,7 @@ impl HostContainer {
     /// Fetch addresses that match the provided transports or acceptable
     /// mixed transports.  Will return an empty Vector if no such addresses
     /// were found.
-    pub async fn fetch_addrs(
+    pub async fn fetch(
         &self,
         color: HostColor,
         transports: &[String],
@@ -853,7 +853,7 @@ impl Hosts {
     // Loop through hosts selected by Outbound Session and see if any of them are
     // free to connect to.
     pub async fn check_addrs(&self, hosts: Vec<(Url, u64)>) -> Option<(Url, u64)> {
-        debug!(target: "net::hosts::check_addrs()", "Starting checks");
+        trace!(target: "net::hosts::check_addrs()", "[START]");
         for (host, last_seen) in hosts {
             // Print a warning if we are trying to connect to a seed node in
             // Outbound session. This shouldn't happen as we reject configured

src/net/session/outbound_session.rs

@@ -196,44 +196,47 @@ impl Slot {
     unreliable connections. A network that purely favors uptime over
     unreliable connections may be vulnerable to sybil by attackers with
     good uptime.*/
-    async fn fetch_addrs_with_preference(
-        &self,
-        preference: usize,
-        slot_count: usize,
-        transports: &[String],
-        transport_mixing: bool,
-        white_count: usize,
-        anchor_count: usize,
-    ) -> Vec<(Url, u64)> {
+    async fn fetch_addrs_with_preference(&self, preference: usize) -> Vec<(Url, u64)> {
+        let slot = self.slot;
+        let settings = self.p2p().settings();
         let hosts = &self.p2p().hosts().container;
 
+        let white_count = settings.white_connect_count;
+        let anchor_count = settings.anchor_connect_count;
+
+        let transports = &settings.allowed_transports;
+        let transport_mixing = settings.transport_mixing;
+
+        debug!(target: "net::outbound_session::fetch_addrs_with_preference()",
+        "slot={}, preference={}", slot, preference);
+
         match preference {
             // Highest preference that corresponds to the anchor and white count preference set in
             // Settings.
             0 => {
-                if slot_count < anchor_count {
-                    hosts.fetch_addrs(HostColor::Gold, transports, transport_mixing).await
-                } else if slot_count < white_count {
-                    hosts.fetch_addrs(HostColor::White, transports, transport_mixing).await
+                if slot < anchor_count {
+                    hosts.fetch(HostColor::Gold, transports, transport_mixing).await
+                } else if slot < white_count {
+                    hosts.fetch(HostColor::White, transports, transport_mixing).await
                 } else {
-                    hosts.fetch_addrs(HostColor::Grey, transports, transport_mixing).await
+                    hosts.fetch(HostColor::Grey, transports, transport_mixing).await
                 }
             }
             // Reduced preference in case we don't have sufficient hosts to satisfy our highest
             // preference.
             1 => {
-                if slot_count < anchor_count {
-                    hosts.fetch_addrs(HostColor::White, transports, transport_mixing).await
-                } else if slot_count < white_count {
-                    hosts.fetch_addrs(HostColor::Grey, transports, transport_mixing).await
+                if slot < anchor_count {
+                    hosts.fetch(HostColor::White, transports, transport_mixing).await
+                } else if slot < white_count {
+                    hosts.fetch(HostColor::Grey, transports, transport_mixing).await
                 } else {
                     vec![]
                 }
             }
             // Lowest preference if we still haven't been able to find a host.
             2 => {
-                if slot_count < anchor_count {
-                    hosts.fetch_addrs(HostColor::Grey, transports, transport_mixing).await
+                if slot < anchor_count {
+                    hosts.fetch(HostColor::Grey, transports, transport_mixing).await
                 } else {
                     vec![]
                 }
@@ -246,25 +249,13 @@ impl Slot {
 
     // Fetch an address we can connect to acccording to the white and anchor connection counts
     // configured in Settings.
-    async fn fetch_addrs(&self, slot_count: usize, transports: &[String]) -> Option<(Url, u64)> {
+    async fn fetch_addrs(&self) -> Option<(Url, u64)> {
         let hosts = self.p2p().hosts();
-        let transport_mixing = self.p2p().settings().transport_mixing;
-        let anchor_count = self.p2p().settings().anchor_connection_count;
-        let white_count = slot_count * self.p2p().settings().white_connection_percent / 100;
 
         // First select an addresses that match our white and anchor requirements configured in
         // Settings.
         let preference = 0;
-        let addrs = self
-            .fetch_addrs_with_preference(
-                preference,
-                slot_count,
-                transports,
-                transport_mixing,
-                white_count,
-                anchor_count,
-            )
-            .await;
+        let addrs = self.fetch_addrs_with_preference(preference).await;
 
         if !addrs.is_empty() {
             return hosts.check_addrs(addrs).await;
@@ -272,16 +263,7 @@ impl Slot {
 
         // If no addresses were returned, go for the second best thing (white and grey).
         let preference = 1;
-        let addrs = self
-            .fetch_addrs_with_preference(
-                preference,
-                slot_count,
-                transports,
-                transport_mixing,
-                white_count,
-                anchor_count,
-            )
-            .await;
+        let addrs = self.fetch_addrs_with_preference(preference).await;
 
         if !addrs.is_empty() {
             return hosts.check_addrs(addrs).await;
@@ -289,16 +271,7 @@ impl Slot {
 
         // If we still have no addresses, go for the least favored option.
         let preference = 2;
-        let addrs = self
-            .fetch_addrs_with_preference(
-                preference,
-                slot_count,
-                transports,
-                transport_mixing,
-                white_count,
-                anchor_count,
-            )
-            .await;
+        let addrs = self.fetch_addrs_with_preference(preference).await;
 
         if !addrs.is_empty() {
             return hosts.check_addrs(addrs).await;
@@ -313,7 +286,6 @@ impl Slot {
     // connections we make from the greylist.
     async fn run(self: Arc<Self>) {
         let hosts = self.p2p().hosts();
-        let slot_count = self.p2p().settings().outbound_connections;
 
         loop {
             // Activate the slot
@@ -323,9 +295,6 @@ impl Slot {
                 self.slot,
             );
 
-            // Retrieve outbound transports
-            let transports = &self.p2p().settings().allowed_transports;
-
             // Do peer discovery if we don't have a hostlist (first time connecting
             // to the network).
             if hosts.container.is_empty(HostColor::Grey).await {
@@ -342,7 +311,7 @@ impl Slot {
                 continue
             }
 
-            let addr = if let Some(addr) = self.fetch_addrs(slot_count, transports).await {
+            let addr = if let Some(addr) = self.fetch_addrs().await {
                 debug!(target: "net::outbound_session::run()", "Fetched address: {:?}", addr);
                 addr
             } else {

src/net/settings.rs

@@ -71,9 +71,9 @@ pub struct Settings {
     /// Pause interval within greylist refinery process
     pub greylist_refinery_interval: u64,
     /// Percent of connections to come from the whitelist
-    pub white_connection_percent: usize,
+    pub white_connect_count: u32,
     /// Number of anchorlist connections
-    pub anchor_connection_count: usize,
+    pub anchor_connect_count: u32,
     /// Number of seconds with no connections after which refinery
     /// process is paused.
     pub time_with_no_connections: u64,
@@ -104,8 +104,8 @@ impl Default for Settings {
             outbound_peer_discovery_attempt_time: 5,
             hostlist: "/dev/null".to_string(),
             greylist_refinery_interval: 15,
-            white_connection_percent: 90,
-            anchor_connection_count: 2,
+            white_connect_count: 90,
+            anchor_connect_count: 2,
             time_with_no_connections: 30,
         }
     }
@@ -201,13 +201,13 @@ pub struct SettingsOpt {
     #[structopt(skip)]
     pub greylist_refinery_interval: Option<u64>,
 
-    /// Percent of connections to come from the whitelist
+    /// Number of whitelist connections
     #[structopt(skip)]
-    pub white_connection_percent: Option<usize>,
+    pub white_connect_count: Option<u32>,
 
     /// Number of anchorlist connections
     #[structopt(skip)]
-    pub anchor_connection_count: Option<usize>,
+    pub anchor_connect_count: Option<u32>,
 
     /// Number of seconds with no connections after which refinery
     /// process is paused.
@@ -251,12 +251,8 @@ impl From<SettingsOpt> for Settings {
             greylist_refinery_interval: opt
                 .greylist_refinery_interval
                 .unwrap_or(def.greylist_refinery_interval),
-            white_connection_percent: opt
-                .white_connection_percent
-                .unwrap_or(def.white_connection_percent),
-            anchor_connection_count: opt
-                .anchor_connection_count
-                .unwrap_or(def.anchor_connection_count),
+            white_connect_count: opt.white_connect_count.unwrap_or(def.white_connect_count),
+            anchor_connect_count: opt.anchor_connect_count.unwrap_or(def.anchor_connect_count),
             time_with_no_connections: opt
                 .time_with_no_connections
                 .unwrap_or(def.time_with_no_connections),

src/net/tests.rs

@@ -102,7 +102,6 @@ async fn hostlist_propagation(ex: Arc<Executor<'static>>) {
         inbound_addrs: vec![seed_addr.clone()],
         external_addrs: vec![seed_addr.clone()],
         outbound_connections: 0,
-        //outbound_connect_timeout: 10,
         inbound_connections: usize::MAX,
         seeds: vec![],
         peers: vec![],
@@ -130,13 +129,11 @@ async fn hostlist_propagation(ex: Arc<Executor<'static>>) {
             inbound_addrs: vec![Url::parse(&format!("tcp://127.0.0.1:{}", 53200 + i)).unwrap()],
             external_addrs: vec![Url::parse(&format!("tcp://127.0.0.1:{}", 53200 + i)).unwrap()],
             outbound_connections: 8,
-            //outbound_connect_timeout: 10,
             inbound_connections: usize::MAX,
             seeds: vec![seed_addr.clone()],
             peers,
             allowed_transports: vec!["tcp".to_string()],
             node_id: i.to_string(),
-            anchor_connection_count: 2,
             ..Default::default()
         };