Skip to content

[Merged by Bors] - PeerDB Status unknown bug fix #2907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - PeerDB Status unknown bug fix #2907

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5fa84cf
Fix Status of unknown peer error
AgeManning Jan 13, 2022
a5eb038
Cleanup dialing connection update code
AgeManning Jan 13, 2022
d1dc998
Merge unstable
AgeManning Jan 13, 2022
b279ce4
Merge unstable
AgeManning Jan 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 71 additions & 29 deletions beacon_node/lighthouse_network/src/peer_manager/peerdb.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,28 +609,8 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {

/// A peer is being dialed.
// VISIBILITY: Only the peer manager can adjust the connection state
// TODO: Remove the internal logic in favour of using the update_connection_state() function.
// This will be compatible once the ENR parameter is removed in the imminent behaviour tests PR.
pub(super) fn dialing_peer(&mut self, peer_id: &PeerId, enr: Option<Enr>) {
let info = self.peers.entry(*peer_id).or_default();
if let Some(enr) = enr {
info.set_enr(enr);
}

if let Err(e) = info.set_dialing_peer() {
error!(self.log, "{}", e; "peer_id" => %peer_id);
}

// If the peer was banned, remove the banned peer and addresses.
if info.is_banned() {
self.banned_peers_count
.remove_banned_peer(info.seen_ip_addresses());
}

// If the peer was disconnected, reduce the disconnected peer count.
if info.is_disconnected() {
self.disconnected_peers = self.disconnected_peers().count().saturating_sub(1);
}
self.update_connection_state(peer_id, NewConnectionState::Dialing { enr });
}

/// Sets a peer as connected with an ingoing connection.
Expand Down Expand Up @@ -686,7 +666,9 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
// connection state for an unknown peer.
if !matches!(
new_state,
NewConnectionState::Connected { .. } | NewConnectionState::Disconnecting { .. }
NewConnectionState::Connected { .. }
| NewConnectionState::Disconnecting { .. }
| NewConnectionState::Dialing { .. }
) {
warn!(log_ref, "Updating state of unknown peer";
"peer_id" => %peer_id, "new_state" => ?new_state);
Expand All @@ -708,7 +690,11 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {

// Handle all the possible state changes
match (info.connection_status().clone(), new_state) {
/* Handle the transition to a connected state */
/* CONNECTED
*
*
* Handles the transition to a connected state
*/
(
current_state,
NewConnectionState::Connected {
Expand Down Expand Up @@ -765,7 +751,47 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
}
}

/* Handle the transition to the disconnected state */
/* DIALING
*
*
* Handles the transition to a dialing state
*/
(old_state, NewConnectionState::Dialing { enr }) => {
match old_state {
PeerConnectionStatus::Banned { .. } => {
warn!(self.log, "Dialing a banned peer"; "peer_id" => %peer_id);
self.banned_peers_count
.remove_banned_peer(info.seen_ip_addresses());
}
PeerConnectionStatus::Disconnected { .. } => {
self.disconnected_peers = self.disconnected_peers.saturating_sub(1);
}
PeerConnectionStatus::Connected { .. } => {
warn!(self.log, "Dialing an already connected peer"; "peer_id" => %peer_id)
}
PeerConnectionStatus::Dialing { .. } => {
warn!(self.log, "Dialing an already dialing peer"; "peer_id" => %peer_id)
}
PeerConnectionStatus::Disconnecting { .. } => {
warn!(self.log, "Dialing a disconnecting peer"; "peer_id" => %peer_id)
}
PeerConnectionStatus::Unknown => {} // default behaviour
}
// Update the ENR if one is known.
if let Some(enr) = enr {
info.set_enr(enr);
}

if let Err(e) = info.set_dialing_peer() {
error!(self.log, "{}", e; "peer_id" => %peer_id);
}
}

/* DISCONNECTED
*
*
* Handle the transition to the disconnected state
*/
(old_state, NewConnectionState::Disconnected) => {
// Remove all subnets for disconnected peers.
info.clear_subnets();
Expand Down Expand Up @@ -799,7 +825,11 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
}
}

/* Handle the transition to the disconnecting state */
/* DISCONNECTING
*
*
* Handles the transition to a disconnecting state
*/
(PeerConnectionStatus::Banned { .. }, NewConnectionState::Disconnecting { to_ban }) => {
error!(self.log, "Disconnecting from a banned peer"; "peer_id" => %peer_id);
info.set_connection_status(PeerConnectionStatus::Disconnecting { to_ban });
Expand All @@ -821,7 +851,11 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
info.set_connection_status(PeerConnectionStatus::Disconnecting { to_ban });
}

/* Handle transitioning to the banned state */
/* BANNED
*
*
* Handles the transition to a banned state
*/
(PeerConnectionStatus::Disconnected { .. }, NewConnectionState::Banned) => {
// It is possible to ban a peer that is currently disconnected. This can occur when
// there are many events that score it poorly and are processed after it has disconnected.
Expand Down Expand Up @@ -879,7 +913,11 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
return Some(BanOperation::ReadyToBan(banned_ips));
}

/* Handle the connection state of unbanning a peer */
/* UNBANNED
*
*
* Handles the transition to an unbanned state
*/
(old_state, NewConnectionState::Unbanned) => {
if matches!(info.score_state(), ScoreState::Banned) {
error!(self.log, "Unbanning a banned peer"; "peer_id" => %peer_id);
Expand All @@ -899,8 +937,7 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
// Increment the disconnected count and reduce the banned count
self.banned_peers_count
.remove_banned_peer(info.seen_ip_addresses());
self.disconnected_peers =
self.disconnected_peers().count().saturating_add(1);
self.disconnected_peers = self.disconnected_peers.saturating_add(1);
}
}
}
Expand Down Expand Up @@ -1059,6 +1096,11 @@ enum NewConnectionState {
/// Whether the peer should be banned after the disconnect occurs.
to_ban: bool,
},
/// We are dialing this peer.
Dialing {
/// An optional known ENR for the peer we are dialing.
enr: Option<Enr>,
},
/// The peer has been disconnected from our local node.
Disconnected,
/// The peer has been banned and actions to shift the peer to the banned state should be
Expand Down