Ignore RPC messages of disconnected peers and remove old peers based on disconnection time (#1854)

## Issue Addressed

NA

## Proposed Changes

Lets the networking behavior ignore messages of peers that are not connected. Furthermore, old peers are not removed from the peerdb based on score anymore but based on the disconnection time.
This commit is contained in:
blacktemplar 2020-11-03 23:43:10 +00:00
parent 0a0f4daf9d
commit 7e7fad5734
3 changed files with 26 additions and 13 deletions

View File

@ -589,6 +589,17 @@ impl<TSpec: EthSpec> Behaviour<TSpec> {
fn on_rpc_event(&mut self, message: RPCMessage<TSpec>) { fn on_rpc_event(&mut self, message: RPCMessage<TSpec>) {
let peer_id = message.peer_id; let peer_id = message.peer_id;
if !self.peer_manager.is_connected(&peer_id) {
//ignore this event
debug!(
self.log,
"Ignoring rpc message of disconnected peer";
"peer" => peer_id.to_string()
);
return;
}
let handler_id = message.conn_id; let handler_id = message.conn_id;
// The METADATA and PING RPC responses are handled within the behaviour and not propagated // The METADATA and PING RPC responses are handled within the behaviour and not propagated
match message.event { match message.event {

View File

@ -316,6 +316,10 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
self.network_globals.peers.read().is_banned(peer_id) self.network_globals.peers.read().is_banned(peer_id)
} }
pub fn is_connected(&self, peer_id: &PeerId) -> bool {
self.network_globals.peers.read().is_connected(peer_id)
}
/// Reports whether the peer limit is reached in which case we stop allowing new incoming /// Reports whether the peer limit is reached in which case we stop allowing new incoming
/// connections. /// connections.
pub fn peer_limit_reached(&self) -> bool { pub fn peer_limit_reached(&self) -> bool {

View File

@ -537,16 +537,15 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
pub fn shrink_to_fit(&mut self) { pub fn shrink_to_fit(&mut self) {
// Remove excess banned peers // Remove excess banned peers
while self.banned_peers_count.banned_peers() > MAX_BANNED_PEERS { while self.banned_peers_count.banned_peers() > MAX_BANNED_PEERS {
if let Some(to_drop) = if let Some((id, info)) = self if let Some(to_drop) = if let Some((id, info, _)) = self
.peers .peers
.iter() .iter()
.filter(|(_, info)| info.is_banned()) .filter_map(|(id, info)| match info.connection_status() {
.min_by(|(_, info_a), (_, info_b)| { PeerConnectionStatus::Banned { since } => Some((id, info, since)),
info_a _ => None,
.score() })
.partial_cmp(&info_b.score()) .min_by_key(|(_, _, since)| *since)
.unwrap_or(std::cmp::Ordering::Equal) {
}) {
self.banned_peers_count self.banned_peers_count
.remove_banned_peer(info.seen_addresses()); .remove_banned_peer(info.seen_addresses());
Some(id.clone()) Some(id.clone())
@ -571,12 +570,11 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
.peers .peers
.iter() .iter()
.filter(|(_, info)| info.is_disconnected()) .filter(|(_, info)| info.is_disconnected())
.min_by(|(_, info_a), (_, info_b)| { .filter_map(|(id, info)| match info.connection_status() {
info_a PeerConnectionStatus::Disconnected { since } => Some((id, since)),
.score() _ => None,
.partial_cmp(&info_b.score())
.unwrap_or(std::cmp::Ordering::Equal)
}) })
.min_by_key(|(_, since)| *since)
.map(|(id, _)| id.clone()) .map(|(id, _)| id.clone())
{ {
debug!(self.log, "Removing old disconnected peer"; "peer_id" => to_drop.to_string()); debug!(self.log, "Removing old disconnected peer"; "peer_id" => to_drop.to_string());