From 6504ceceb7a32f9e089c4093ecf93ad45acc8166 Mon Sep 17 00:00:00 2001 From: draoi Date: Sat, 30 Mar 2024 09:06:42 +0100 Subject: [PATCH] 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 | 4 +- src/net/session/outbound_session.rs | 87 ++++++++++------------------- src/net/settings.rs | 22 +++----- src/net/tests.rs | 3 - 4 files changed, 39 insertions(+), 77 deletions(-) diff --git a/src/net/hosts/store.rs b/src/net/hosts/store.rs index 0d67da22e..a80a5b6a0 100644 --- a/src/net/hosts/store.rs +++ b/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 diff --git a/src/net/session/outbound_session.rs b/src/net/session/outbound_session.rs index 46fefdfbb..b75151554 100644 --- a/src/net/session/outbound_session.rs +++ b/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) { 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 { diff --git a/src/net/settings.rs b/src/net/settings.rs index 3360c6510..f2275e450 100644 --- a/src/net/settings.rs +++ b/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, - /// Percent of connections to come from the whitelist + /// Number of whitelist connections #[structopt(skip)] - pub white_connection_percent: Option, + pub white_connect_count: Option, /// Number of anchorlist connections #[structopt(skip)] - pub anchor_connection_count: Option, + pub anchor_connect_count: Option, /// Number of seconds with no connections after which refinery /// process is paused. @@ -251,12 +251,8 @@ impl From 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), diff --git a/src/net/tests.rs b/src/net/tests.rs index 911f79fcc..147c5ec4c 100644 --- a/src/net/tests.rs +++ b/src/net/tests.rs @@ -102,7 +102,6 @@ async fn hostlist_propagation(ex: Arc>) { 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>) { 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() };