prefer outbound peers when syncing (consistently) (#3521)

but use inbound peer for header and body sync if necessary
sync state from inbound peer if no outbound peers to sync from

p2p/src/peers.rs

@@ -128,13 +128,6 @@ impl Peers {
 		self.iter().connected().by_addr(addr)
 	}
 
-	pub fn max_peer_difficulty(&self) -> Difficulty {
-		self.iter()
-			.connected()
-			.max_difficulty()
-			.unwrap_or(Difficulty::zero())
-	}
-
 	pub fn is_banned(&self, peer_addr: PeerAddr) -> bool {
 		if let Ok(peer) = self.store.get_peer(peer_addr) {
 			return peer.flags == State::Banned;

servers/src/grin/seed.rs

@@ -26,6 +26,7 @@ use std::sync::{mpsc, Arc};
 use std::{cmp, str, thread, time};
 
 use crate::core::global;
+use crate::core::pow::Difficulty;
 use crate::p2p;
 use crate::p2p::types::PeerAddr;
 use crate::p2p::ChainAdapter;
@@ -172,15 +173,10 @@ fn monitor_peers(
 		total_count += 1;
 	}
 
-	let peers_count = peers.iter().connected().count();
-
-	let max_diff = peers.max_peer_difficulty();
-	let most_work_count = peers
-		.iter()
-		.outbound()
-		.with_difficulty(|x| x >= max_diff)
-		.connected()
-		.count();
+	let peers_iter = || peers.iter().connected();
+	let peers_count = peers_iter().count();
+	let max_diff = peers_iter().max_difficulty().unwrap_or(Difficulty::zero());
+	let most_work_count = peers_iter().with_difficulty(|x| x >= max_diff).count();
 
 	debug!(
 		"monitor_peers: on {}:{}, {} connected ({} most_work). \

servers/src/grin/sync/body_sync.rs

@@ -93,14 +93,25 @@ impl BodySync {
 		let head = self.chain.head()?;
 
 		// Find connected peers with strictly greater difficulty than us.
-		let peers: Vec<_> = self
-			.peers
+		let peers_iter = || {
+			self.peers
 				.iter()
-			.outbound()
 				.with_difficulty(|x| x > head.total_difficulty)
 				.connected()
-			.into_iter()
-			.collect();
+		};
+
+		// We prefer outbound peers with greater difficulty.
+		let mut peers: Vec<_> = peers_iter().outbound().into_iter().collect();
+		if peers.is_empty() {
+			debug!("no outbound peers with more work, considering inbound");
+			peers = peers_iter().inbound().into_iter().collect();
+		}
+
+		// If we have no peers (outbound or inbound) then we are done for now.
+		if peers.is_empty() {
+			debug!("no peers (inbound or outbound) with more work");
+			return Ok(false);
+		}
 
 		// if we have 5 peers to sync from then ask for 50 blocks total (peer_count *
 		// 10) max will be 80 if all 8 peers are advertising more work

servers/src/grin/sync/header_sync.rs

@@ -19,6 +19,7 @@ use std::sync::Arc;
 use crate::chain::{self, SyncState, SyncStatus};
 use crate::common::types::Error;
 use crate::core::core::hash::{Hash, Hashed};
+use crate::core::pow::Difficulty;
 use crate::p2p::{self, types::ReasonForBan, Capabilities, Peer};
 
 pub struct HeaderSync {
@@ -170,15 +171,23 @@ impl HeaderSync {
 
 	fn header_sync(&mut self) -> Option<Arc<Peer>> {
 		if let Ok(header_head) = self.chain.header_head() {
-			let max_diff = self.peers.max_peer_difficulty();
-			let peer = self
-				.peers
+			let peers_iter = || {
+				self.peers
 					.iter()
-				.outbound()
 					.with_capabilities(Capabilities::HEADER_HIST)
-				.with_difficulty(|x| x >= max_diff)
 					.connected()
-				.choose_random();
+			};
+
+			// Filter peers further based on max difficulty.
+			let max_diff = peers_iter().max_difficulty().unwrap_or(Difficulty::zero());
+			let peers_iter = || peers_iter().with_difficulty(|x| x >= max_diff);
+
+			// Choose a random "most work" peer, preferring outbound if at all possible.
+			let peer = peers_iter().outbound().choose_random().or_else(|| {
+				warn!("no suitable outbound peer for header sync, considering inbound");
+				peers_iter().inbound().choose_random()
+			});
+
 			if let Some(peer) = peer {
 				if peer.info.total_difficulty() > header_head.total_difficulty {
 					return self.request_headers(peer);

servers/src/grin/sync/state_sync.rs

@@ -19,6 +19,7 @@ use std::sync::Arc;
 use crate::chain::{self, SyncState, SyncStatus};
 use crate::core::core::hash::Hashed;
 use crate::core::global;
+use crate::core::pow::Difficulty;
 use crate::p2p::{self, Capabilities, Peer};
 
 /// Fast sync has 3 "states":
@@ -158,15 +159,23 @@ impl StateSync {
 		let mut txhashset_height = header_head.height.saturating_sub(threshold);
 		txhashset_height = txhashset_height.saturating_sub(txhashset_height % archive_interval);
 
-		let max_diff = self.peers.max_peer_difficulty();
-		let peer = self
-			.peers
+		let peers_iter = || {
+			self.peers
 				.iter()
-			.outbound()
 				.with_capabilities(Capabilities::TXHASHSET_HIST)
-			.with_difficulty(|x| x >= max_diff)
 				.connected()
-			.choose_random();
+		};
+
+		// Filter peers further based on max difficulty.
+		let max_diff = peers_iter().max_difficulty().unwrap_or(Difficulty::zero());
+		let peers_iter = || peers_iter().with_difficulty(|x| x >= max_diff);
+
+		// Choose a random "most work" peer, preferring outbound if at all possible.
+		let peer = peers_iter().outbound().choose_random().or_else(|| {
+			warn!("no suitable outbound peer for state sync, considering inbound");
+			peers_iter().inbound().choose_random()
+		});
+
 		if let Some(peer) = peer {
 			// ask for txhashset at state_sync_threshold
 			let mut txhashset_head = self

servers/src/grin/sync/syncer.rs

@@ -231,11 +231,18 @@ impl SyncRunner {
 		let mut is_syncing = self.sync_state.is_syncing();
 
 		// Find a peer with greatest known difficulty.
-		let max_diff = self.peers.max_peer_difficulty();
+		// Consider all peers, both inbound and outbound.
+		// We will prioritize syncing against outbound peers exclusively when possible.
+		// But we do support syncing against an inbound peer if greater work than any outbound peers.
+		let max_diff = self
+			.peers
+			.iter()
+			.connected()
+			.max_difficulty()
+			.unwrap_or(Difficulty::zero());
 		let peer = self
 			.peers
 			.iter()
-			.outbound()
 			.with_difficulty(|x| x >= max_diff)
 			.connected()
 			.choose_random();