From 1baa59c44da8abdcbe4ffd7804111c7b21effeba Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Tue, 15 Dec 2020 19:11:51 +0000 Subject: [PATCH] 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 | 7 ------- servers/src/grin/seed.rs | 14 +++++--------- servers/src/grin/sync/body_sync.rs | 27 +++++++++++++++++++-------- servers/src/grin/sync/header_sync.rs | 27 ++++++++++++++++++--------- servers/src/grin/sync/state_sync.rs | 27 ++++++++++++++++++--------- servers/src/grin/sync/syncer.rs | 11 +++++++++-- 6 files changed, 69 insertions(+), 44 deletions(-) diff --git a/p2p/src/peers.rs b/p2p/src/peers.rs index 520a7c356..c1bbccfaa 100644 --- a/p2p/src/peers.rs +++ b/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; diff --git a/servers/src/grin/seed.rs b/servers/src/grin/seed.rs index 401696990..fc310bcbe 100644 --- a/servers/src/grin/seed.rs +++ b/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). \ diff --git a/servers/src/grin/sync/body_sync.rs b/servers/src/grin/sync/body_sync.rs index 65f7c417f..6a048c5a0 100644 --- a/servers/src/grin/sync/body_sync.rs +++ b/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 - .iter() - .outbound() - .with_difficulty(|x| x > head.total_difficulty) - .connected() - .into_iter() - .collect(); + let peers_iter = || { + self.peers + .iter() + .with_difficulty(|x| x > head.total_difficulty) + .connected() + }; + + // 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 diff --git a/servers/src/grin/sync/header_sync.rs b/servers/src/grin/sync/header_sync.rs index a55c425ec..abe91444e 100644 --- a/servers/src/grin/sync/header_sync.rs +++ b/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> { if let Ok(header_head) = self.chain.header_head() { - let max_diff = self.peers.max_peer_difficulty(); - let peer = self - .peers - .iter() - .outbound() - .with_capabilities(Capabilities::HEADER_HIST) - .with_difficulty(|x| x >= max_diff) - .connected() - .choose_random(); + let peers_iter = || { + self.peers + .iter() + .with_capabilities(Capabilities::HEADER_HIST) + .connected() + }; + + // 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); diff --git a/servers/src/grin/sync/state_sync.rs b/servers/src/grin/sync/state_sync.rs index 86bceca6d..e1af88170 100644 --- a/servers/src/grin/sync/state_sync.rs +++ b/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 - .iter() - .outbound() - .with_capabilities(Capabilities::TXHASHSET_HIST) - .with_difficulty(|x| x >= max_diff) - .connected() - .choose_random(); + let peers_iter = || { + self.peers + .iter() + .with_capabilities(Capabilities::TXHASHSET_HIST) + .connected() + }; + + // 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 diff --git a/servers/src/grin/sync/syncer.rs b/servers/src/grin/sync/syncer.rs index 4f71b48fd..0b8cea3cb 100644 --- a/servers/src/grin/sync/syncer.rs +++ b/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();