From 5eba9078fcc41806748fee9ed39b2240e8c1d807 Mon Sep 17 00:00:00 2001 From: Vurich Date: Fri, 30 Jun 2017 10:58:48 +0200 Subject: [PATCH 1/2] Report whether a peer was kept from `Handler::on_connect` --- ethcore/light/src/net/error.rs | 4 ++ ethcore/light/src/net/mod.rs | 67 ++++++++++++++++++++++++++++-- ethcore/light/src/on_demand/mod.rs | 25 +++++++++-- sync/src/light_sync/mod.rs | 47 ++++++++++++--------- 4 files changed, 117 insertions(+), 26 deletions(-) diff --git a/ethcore/light/src/net/error.rs b/ethcore/light/src/net/error.rs index 1c0374c7e..578978348 100644 --- a/ethcore/light/src/net/error.rs +++ b/ethcore/light/src/net/error.rs @@ -66,6 +66,8 @@ pub enum Error { BadProtocolVersion, /// Peer is overburdened. Overburdened, + /// No handler kept the peer. + RejectedByHandlers, } impl Error { @@ -85,6 +87,7 @@ impl Error { Error::UnsupportedProtocolVersion(_) => Punishment::Disable, Error::BadProtocolVersion => Punishment::Disable, Error::Overburdened => Punishment::None, + Error::RejectedByHandlers => Punishment::Disconnect, } } } @@ -117,6 +120,7 @@ impl fmt::Display for Error { Error::UnsupportedProtocolVersion(pv) => write!(f, "Unsupported protocol version: {}", pv), Error::BadProtocolVersion => write!(f, "Bad protocol version in handshake"), Error::Overburdened => write!(f, "Peer overburdened"), + Error::RejectedByHandlers => write!(f, "No handler kept this peer"), } } } diff --git a/ethcore/light/src/net/mod.rs b/ethcore/light/src/net/mod.rs index 6ab5903df..24f50e5d5 100644 --- a/ethcore/light/src/net/mod.rs +++ b/ethcore/light/src/net/mod.rs @@ -31,6 +31,7 @@ use std::collections::{HashMap, HashSet}; use std::fmt; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; +use std::ops::{BitOr, BitAnd, Not}; use provider::Provider; use request::{Request, NetworkRequests as Requests, Response}; @@ -157,6 +158,54 @@ pub struct Peer { awaiting_acknowledge: Option<(SteadyTime, Arc)>, } +/// Whether or not a peer was kept by a handler +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PeerStatus { + /// The peer was kept + Kept, + /// The peer was not kept + Unkept, +} + +impl Not for PeerStatus { + type Output = Self; + + fn not(self) -> Self { + use self::PeerStatus::*; + + match self { + Kept => Unkept, + Unkept => Kept, + } + } +} + +impl BitAnd for PeerStatus { + type Output = Self; + + fn bitand(self, other: Self) -> Self { + use self::PeerStatus::*; + + match (self, other) { + (Kept, Kept) => Kept, + _ => Unkept, + } + } +} + +impl BitOr for PeerStatus { + type Output = Self; + + fn bitor(self, other: Self) -> Self { + use self::PeerStatus::*; + + match (self, other) { + (_, Kept) | (Kept, _) => Kept, + _ => Unkept, + } + } +} + /// A light protocol event handler. /// /// Each handler function takes a context which describes the relevant peer @@ -168,7 +217,12 @@ pub struct Peer { /// that relevant data will be stored by interested handlers. pub trait Handler: Send + Sync { /// Called when a peer connects. - fn on_connect(&self, _ctx: &EventContext, _status: &Status, _capabilities: &Capabilities) { } + fn on_connect( + &self, + _ctx: &EventContext, + _status: &Status, + _capabilities: &Capabilities + ) -> PeerStatus { PeerStatus::Kept } /// Called when a peer disconnects, with a list of unfulfilled request IDs as /// of yet. fn on_disconnect(&self, _ctx: &EventContext, _unfulfilled: &[ReqId]) { } @@ -766,15 +820,20 @@ impl LightProtocol { awaiting_acknowledge: None, })); - for handler in &self.handlers { + let any_kept = self.handlers.iter().map( + |handler| handler.on_connect(&Ctx { peer: *peer, io: io, proto: self, }, &status, &capabilities) - } + ).fold(PeerStatus::Kept, PeerStatus::bitor); - Ok(()) + if any_kept == PeerStatus::Unkept { + Err(Error::RejectedByHandlers) + } else { + Ok(()) + } } // Handle an announcement. diff --git a/ethcore/light/src/on_demand/mod.rs b/ethcore/light/src/on_demand/mod.rs index 435c72cf7..8306f3a1a 100644 --- a/ethcore/light/src/on_demand/mod.rs +++ b/ethcore/light/src/on_demand/mod.rs @@ -29,7 +29,17 @@ use futures::sync::oneshot::{self, Sender, Receiver, Canceled}; use network::PeerId; use util::{RwLock, Mutex}; -use net::{self, Handler, Status, Capabilities, Announcement, EventContext, BasicContext, ReqId}; +use net::{ + self, + Handler, + PeerStatus, + Status, + Capabilities, + Announcement, + EventContext, + BasicContext, + ReqId, +}; use cache::Cache; use request::{self as basic_request, Request as NetworkRequest}; use self::request::CheckedRequest; @@ -402,9 +412,18 @@ impl OnDemand { } impl Handler for OnDemand { - fn on_connect(&self, ctx: &EventContext, status: &Status, capabilities: &Capabilities) { - self.peers.write().insert(ctx.peer(), Peer { status: status.clone(), capabilities: capabilities.clone() }); + fn on_connect( + &self, + ctx: &EventContext, + status: &Status, + capabilities: &Capabilities + ) -> PeerStatus { + self.peers.write().insert( + ctx.peer(), + Peer { status: status.clone(), capabilities: capabilities.clone() } + ); self.attempt_dispatch(ctx.as_basic()); + PeerStatus::Kept } fn on_disconnect(&self, ctx: &EventContext, unfulfilled: &[ReqId]) { diff --git a/sync/src/light_sync/mod.rs b/sync/src/light_sync/mod.rs index 2cfbafa17..558e28ecd 100644 --- a/sync/src/light_sync/mod.rs +++ b/sync/src/light_sync/mod.rs @@ -39,8 +39,9 @@ use std::sync::Arc; use ethcore::encoded; use light::client::{AsLightClient, LightChainClient}; use light::net::{ - Announcement, Handler, BasicContext, EventContext, - Capabilities, ReqId, Status, Error as NetError, + PeerStatus, Announcement, Handler, BasicContext, + EventContext, Capabilities, ReqId, Status, + Error as NetError, }; use light::request::{self, CompleteHeadersRequest as HeadersRequest}; use network::PeerId; @@ -229,26 +230,34 @@ pub struct LightSync { } impl Handler for LightSync { - fn on_connect(&self, ctx: &EventContext, status: &Status, capabilities: &Capabilities) { - if !capabilities.serve_headers { + fn on_connect( + &self, + ctx: &EventContext, + status: &Status, + capabilities: &Capabilities + ) -> PeerStatus { + if capabilities.serve_headers { + let chain_info = ChainInfo { + head_td: status.head_td, + head_hash: status.head_hash, + head_num: status.head_num, + }; + + { + let mut best = self.best_seen.lock(); + *best = ::std::cmp::max(best.clone(), Some(chain_info.clone())); + } + + self.peers.write().insert(ctx.peer(), Mutex::new(Peer::new(chain_info))); + self.maintain_sync(ctx.as_basic()); + + PeerStatus::Kept + } else { trace!(target: "sync", "Disconnecting irrelevant peer: {}", ctx.peer()); ctx.disconnect_peer(ctx.peer()); - return; + + PeerStatus::Unkept } - - let chain_info = ChainInfo { - head_td: status.head_td, - head_hash: status.head_hash, - head_num: status.head_num, - }; - - { - let mut best = self.best_seen.lock(); - *best = ::std::cmp::max(best.clone(), Some(chain_info.clone())); - } - - self.peers.write().insert(ctx.peer(), Mutex::new(Peer::new(chain_info))); - self.maintain_sync(ctx.as_basic()); } fn on_disconnect(&self, ctx: &EventContext, unfulfilled: &[ReqId]) { From 17de29e69a92d14641f635f9e4d9b09ce404f57a Mon Sep 17 00:00:00 2001 From: Vurich Date: Fri, 30 Jun 2017 12:10:12 +0200 Subject: [PATCH 2/2] Prevent disconnect from within handler (and style cleanup) --- ethcore/light/src/net/mod.rs | 15 +++++++++------ ethcore/light/src/on_demand/mod.rs | 11 ++--------- sync/src/light_sync/mod.rs | 7 +++---- 3 files changed, 14 insertions(+), 19 deletions(-) diff --git a/ethcore/light/src/net/mod.rs b/ethcore/light/src/net/mod.rs index 24f50e5d5..5af921d80 100644 --- a/ethcore/light/src/net/mod.rs +++ b/ethcore/light/src/net/mod.rs @@ -821,12 +821,15 @@ impl LightProtocol { })); let any_kept = self.handlers.iter().map( - |handler| - handler.on_connect(&Ctx { - peer: *peer, - io: io, - proto: self, - }, &status, &capabilities) + |handler| handler.on_connect( + &Ctx { + peer: *peer, + io: io, + proto: self, + }, + &status, + &capabilities + ) ).fold(PeerStatus::Kept, PeerStatus::bitor); if any_kept == PeerStatus::Unkept { diff --git a/ethcore/light/src/on_demand/mod.rs b/ethcore/light/src/on_demand/mod.rs index 8306f3a1a..c1919ebd1 100644 --- a/ethcore/light/src/on_demand/mod.rs +++ b/ethcore/light/src/on_demand/mod.rs @@ -30,15 +30,8 @@ use network::PeerId; use util::{RwLock, Mutex}; use net::{ - self, - Handler, - PeerStatus, - Status, - Capabilities, - Announcement, - EventContext, - BasicContext, - ReqId, + self, Handler, PeerStatus, Status, Capabilities, + Announcement, EventContext, BasicContext, ReqId, }; use cache::Cache; use request::{self as basic_request, Request as NetworkRequest}; diff --git a/sync/src/light_sync/mod.rs b/sync/src/light_sync/mod.rs index 558e28ecd..512ba7943 100644 --- a/sync/src/light_sync/mod.rs +++ b/sync/src/light_sync/mod.rs @@ -236,6 +236,8 @@ impl Handler for LightSync { status: &Status, capabilities: &Capabilities ) -> PeerStatus { + use std::cmp; + if capabilities.serve_headers { let chain_info = ChainInfo { head_td: status.head_td, @@ -245,7 +247,7 @@ impl Handler for LightSync { { let mut best = self.best_seen.lock(); - *best = ::std::cmp::max(best.clone(), Some(chain_info.clone())); + *best = cmp::max(best.clone(), Some(chain_info.clone())); } self.peers.write().insert(ctx.peer(), Mutex::new(Peer::new(chain_info))); @@ -253,9 +255,6 @@ impl Handler for LightSync { PeerStatus::Kept } else { - trace!(target: "sync", "Disconnecting irrelevant peer: {}", ctx.peer()); - ctx.disconnect_peer(ctx.peer()); - PeerStatus::Unkept } }