gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-03-10 12:11:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert peer DB changes from #2724 (#2828)

Browse Source 
## Proposed Changes

This reverts commit 53562010ec from PR #2724

Hopefully this will restore the reliability of the sync simulator.
This commit is contained in:
Michael Sproul
2021-11-25 03:45:52 +00:00
parent 3fb8162dcc
commit 2c07a72980
16 changed files with 154 additions and 139 deletions
Download Patch File Download Diff File Expand all files Collapse all files

82
beacon_node/lighthouse_network/src/peer_manager/mod.rs
View File

@@ -143,7 +143,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
/// This will send a goodbye and disconnect the peer if it is connected or dialing.
pub fn goodbye_peer(&mut self, peer_id: &PeerId, reason: GoodbyeReason, source: ReportSource) {
// Update the sync status if required
if let Some(info) = self.network_globals.peers_mut().peer_info_mut(peer_id) {
if let Some(info) = self.network_globals.peers.write().peer_info_mut(peer_id) {
debug!(self.log, "Sending goodbye to peer"; "peer_id" => %peer_id, "reason" => %reason, "score" => %info.score());
if matches!(reason, GoodbyeReason::IrrelevantNetwork) {
info.update_sync_status(SyncStatus::IrrelevantPeer);
@@ -165,7 +165,8 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
) {
let action = self
.network_globals
.peers_mut()
.peers
.write()
.report_peer(peer_id, action, source);
self.handle_score_action(peer_id, action, reason);
}
@@ -263,13 +264,14 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
if (min_ttl.is_some()
&& connected_or_dialing + to_dial_peers.len() < self.max_priority_peers()
|| connected_or_dialing + to_dial_peers.len() < self.max_peers())
&& self.network_globals.peers().should_dial(&peer_id)
&& self.network_globals.peers.read().should_dial(&peer_id)
{
// This should be updated with the peer dialing. In fact created once the peer is
// dialed
if let Some(min_ttl) = min_ttl {
self.network_globals
.peers_mut()
.peers
.write()
.update_min_ttl(&peer_id, min_ttl);
}
to_dial_peers.push(peer_id);
@@ -339,11 +341,11 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
///
/// This is used to determine if we should accept incoming connections.
pub fn ban_status(&self, peer_id: &PeerId) -> BanResult {
self.network_globals.peers().ban_status(peer_id)
self.network_globals.peers.read().ban_status(peer_id)
}
pub fn is_connected(&self, peer_id: &PeerId) -> bool {
self.network_globals.peers().is_connected(peer_id)
self.network_globals.peers.read().is_connected(peer_id)
}
/// Reports whether the peer limit is reached in which case we stop allowing new incoming
@@ -354,7 +356,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
/// Updates `PeerInfo` with `identify` information.
pub fn identify(&mut self, peer_id: &PeerId, info: &IdentifyInfo) {
if let Some(peer_info) = self.network_globals.peers_mut().peer_info_mut(peer_id) {
if let Some(peer_info) = self.network_globals.peers.write().peer_info_mut(peer_id) {
let previous_kind = peer_info.client().kind.clone();
let previous_listening_addresses =
peer_info.set_listening_addresses(info.listen_addrs.clone());
@@ -401,7 +403,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
direction: ConnectionDirection,
) {
let client = self.network_globals.client(peer_id);
let score = self.network_globals.peers().score(peer_id);
let score = self.network_globals.peers.read().score(peer_id);
debug!(self.log, "RPC Error"; "protocol" => %protocol, "err" => %err, "client" => %client,
"peer_id" => %peer_id, "score" => %score, "direction" => ?direction);
metrics::inc_counter_vec(
@@ -503,7 +505,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
/// A ping request has been received.
// NOTE: The behaviour responds with a PONG automatically
pub fn ping_request(&mut self, peer_id: &PeerId, seq: u64) {
if let Some(peer_info) = self.network_globals.peers().peer_info(peer_id) {
if let Some(peer_info) = self.network_globals.peers.read().peer_info(peer_id) {
// received a ping
// reset the to-ping timer for this peer
debug!(self.log, "Received a ping request"; "peer_id" => %peer_id, "seq_no" => seq);
@@ -540,7 +542,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
/// A PONG has been returned from a peer.
pub fn pong_response(&mut self, peer_id: &PeerId, seq: u64) {
if let Some(peer_info) = self.network_globals.peers().peer_info(peer_id) {
if let Some(peer_info) = self.network_globals.peers.read().peer_info(peer_id) {
// received a pong
// if the sequence number is unknown send update the meta data of the peer.
@@ -563,7 +565,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
/// Received a metadata response from a peer.
pub fn meta_data_response(&mut self, peer_id: &PeerId, meta_data: MetaData<TSpec>) {
if let Some(peer_info) = self.network_globals.peers_mut().peer_info_mut(peer_id) {
if let Some(peer_info) = self.network_globals.peers.write().peer_info_mut(peer_id) {
if let Some(known_meta_data) = &peer_info.meta_data() {
if *known_meta_data.seq_number() < *meta_data.seq_number() {
debug!(self.log, "Updating peer's metadata";
@@ -590,7 +592,8 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
pub(crate) fn update_gossipsub_scores(&mut self, gossipsub: &Gossipsub) {
let actions = self
.network_globals
.peers_mut()
.peers
.write()
.update_gossipsub_scores(self.target_peers, gossipsub);
for (peer_id, score_action) in actions {
@@ -630,7 +633,11 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
///
/// This is also called when dialing a peer fails.
fn inject_disconnect(&mut self, peer_id: &PeerId) {
let ban_operation = self.network_globals.peers_mut().inject_disconnect(peer_id);
let ban_operation = self
.network_globals
.peers
.write()
.inject_disconnect(peer_id);
if let Some(ban_operation) = ban_operation {
// The peer was awaiting a ban, continue to ban the peer.
@@ -656,7 +663,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
enr: Option<Enr>,
) -> bool {
{
let mut peerdb = self.network_globals.peers_mut();
let mut peerdb = self.network_globals.peers.write();
if !matches!(peerdb.ban_status(peer_id), BanResult::NotBanned) {
// don't connect if the peer is banned
error!(self.log, "Connection has been allowed to a banned peer"; "peer_id" => %peer_id);
@@ -693,7 +700,8 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
// Increment the PEERS_PER_CLIENT metric
if let Some(kind) = self
.network_globals
.peers()
.peers
.read()
.peer_info(peer_id)
.map(|peer_info| peer_info.client().kind.clone())
{
@@ -712,7 +720,8 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
self.events
.push(PeerManagerEvent::DisconnectPeer(peer_id, reason));
self.network_globals
.peers_mut()
.peers
.write()
.notify_disconnecting(&peer_id, false);
}
@@ -728,7 +737,8 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
.filter_map(|(k, v)| {
if self
.network_globals
.peers()
.peers
.read()
.good_peers_on_subnet(Subnet::SyncCommittee(*k))
.count()
< TARGET_SUBNET_PEERS
@@ -777,7 +787,7 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
}
// Updates peer's scores and unban any peers if required.
let actions = self.network_globals.peers_mut().update_scores();
let actions = self.network_globals.peers.write().update_scores();
for (peer_id, action) in actions {
self.handle_score_action(&peer_id, action, None);
}
@@ -796,7 +806,8 @@ impl<TSpec: EthSpec> PeerManager<TSpec> {
let mut n_outbound_removed = 0;
for (peer_id, info) in self
.network_globals
.peers()
.peers
.read()
.worst_connected_peers()
.iter()
.filter(|(_, info)| !info.has_future_duty())
@@ -915,14 +926,16 @@ mod tests {
// Set the outbound-only peers to have the lowest score.
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&outbound_only_peer1)
.unwrap()
.add_to_score(-1.0);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&outbound_only_peer2)
.unwrap()
.add_to_score(-2.0);
@@ -938,11 +951,13 @@ mod tests {
assert_eq!(peer_manager.network_globals.connected_or_dialing_peers(), 3);
assert!(peer_manager
.network_globals
.peers()
.peers
.read()
.is_connected(&outbound_only_peer1));
assert!(!peer_manager
.network_globals
.peers()
.peers
.read()
.is_connected(&outbound_only_peer2));
peer_manager.heartbeat();
@@ -971,7 +986,8 @@ mod tests {
);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&(outbound_only_peer))
.unwrap()
.add_to_score(-1.0);
@@ -1011,25 +1027,29 @@ mod tests {
);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&(inbound_only_peer1))
.unwrap()
.add_to_score(-19.8);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&(outbound_only_peer1))
.unwrap()
.add_to_score(-19.8);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&(inbound_only_peer1))
.unwrap()
.set_gossipsub_score(-85.0);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&(outbound_only_peer1))
.unwrap()
.set_gossipsub_score(-85.0);
@@ -1067,13 +1087,15 @@ mod tests {
);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&(inbound_only_peer1))
.unwrap()
.add_to_score(-19.9);
peer_manager
.network_globals
.peers_mut()
.peers
.write()
.peer_info_mut(&(inbound_only_peer1))
.unwrap()
.set_gossipsub_score(-85.0);

11
beacon_node/lighthouse_network/src/peer_manager/network_behaviour.rs
View File

@@ -146,7 +146,8 @@ impl<TSpec: EthSpec> NetworkBehaviour for PeerManager<TSpec> {
if self.peer_limit_reached()
&& self
.network_globals
.peers()
.peers
.read()
.peer_info(peer_id)
.map_or(true, |peer| !peer.has_future_duty())
{
@@ -184,7 +185,8 @@ impl<TSpec: EthSpec> NetworkBehaviour for PeerManager<TSpec> {
// There are no more connections
if self
.network_globals
.peers()
.peers
.read()
.is_connected_or_disconnecting(peer_id)
{
// We are disconnecting the peer or the peer has already been connected.
@@ -198,7 +200,8 @@ impl<TSpec: EthSpec> NetworkBehaviour for PeerManager<TSpec> {
// Decrement the PEERS_PER_CLIENT metric
if let Some(kind) = self
.network_globals
.peers()
.peers
.read()
.peer_info(peer_id)
.map(|info| info.client().kind.clone())
{
@@ -259,7 +262,7 @@ impl<TSpec: EthSpec> NetworkBehaviour for PeerManager<TSpec> {
_error: &DialError,
) {
if let Some(peer_id) = peer_id {
if !self.network_globals.peers().is_connected(&peer_id) {
if !self.network_globals.peers.read().is_connected(&peer_id) {
self.inject_disconnect(&peer_id);
}
}

2
beacon_node/lighthouse_network/src/peer_manager/peerdb.rs
View File

@@ -314,7 +314,7 @@ impl<TSpec: EthSpec> PeerDB<TSpec> {
.map(|(id, _)| id)
}
/// Returns the peer's connection status. Returns None if the peer is not in the DB.
/// Returns the peer's connection status. Returns unknown if the peer is not in the DB.
pub fn connection_status(&self, peer_id: &PeerId) -> Option<PeerConnectionStatus> {
self.peer_info(peer_id)
.map(|info| info.connection_status().clone())

1
beacon_node/lighthouse_network/src/peer_manager/peerdb/peer_info.rs
View File

@@ -236,6 +236,7 @@ impl<T: EthSpec> PeerInfo<T> {
/* Mutable Functions */
/// Updates the sync status. Returns true if the status was changed.
// VISIBILITY: Both the peer manager the network sync is able to update the sync state of a peer
pub fn update_sync_status(&mut self, sync_status: SyncStatus) -> bool {
self.sync_status.update(sync_status)
}

26
beacon_node/lighthouse_network/src/peer_manager/peerdb/sync_status.rs
View File

@@ -27,6 +27,19 @@ pub struct SyncInfo {
pub finalized_root: Hash256,
}
impl std::cmp::PartialEq for SyncStatus {
fn eq(&self, other: &Self) -> bool {
matches!(
(self, other),
(SyncStatus::Synced { .. }, SyncStatus::Synced { .. })
| (SyncStatus::Advanced { .. }, SyncStatus::Advanced { .. })
| (SyncStatus::Behind { .. }, SyncStatus::Behind { .. })
| (SyncStatus::IrrelevantPeer, SyncStatus::IrrelevantPeer)
| (SyncStatus::Unknown, SyncStatus::Unknown)
)
}
}
impl SyncStatus {
/// Returns true if the peer has advanced knowledge of the chain.
pub fn is_advanced(&self) -> bool {
@@ -48,7 +61,7 @@ impl SyncStatus {
/// E.g. returns `true` if the state changed from `Synced` to `Advanced`, but not if
/// the status remained `Synced` with different `SyncInfo` within.
pub fn update(&mut self, new_state: SyncStatus) -> bool {
let changed_status = !(self.is_same_kind(&new_state));
let changed_status = *self != new_state;
*self = new_state;
changed_status
}
@@ -62,17 +75,6 @@ impl SyncStatus {
SyncStatus::IrrelevantPeer => "Irrelevant",
}
}
pub fn is_same_kind(&self, other: &Self) -> bool {
matches!(
(self, other),
(SyncStatus::Synced { .. }, SyncStatus::Synced { .. })
| (SyncStatus::Advanced { .. }, SyncStatus::Advanced { .. })
| (SyncStatus::Behind { .. }, SyncStatus::Behind { .. })
| (SyncStatus::IrrelevantPeer, SyncStatus::IrrelevantPeer)
| (SyncStatus::Unknown, SyncStatus::Unknown)
)
}
}
impl std::fmt::Display for SyncStatus {