diff --git a/crates/net/network/src/peers/manager.rs b/crates/net/network/src/peers/manager.rs index fba99c2aad..5dcb54f4eb 100644 --- a/crates/net/network/src/peers/manager.rs +++ b/crates/net/network/src/peers/manager.rs @@ -1,7 +1,9 @@ use crate::{ error::{BackoffKind, SessionError}, peers::{ - reputation::{is_banned_reputation, DEFAULT_REPUTATION}, + reputation::{ + is_banned_reputation, DEFAULT_REPUTATION, MAX_TRUSTED_PEER_REPUTATION_CHANGE, + }, ReputationChangeWeights, DEFAULT_MAX_COUNT_CONCURRENT_DIALS, DEFAULT_MAX_COUNT_PEERS_INBOUND, DEFAULT_MAX_COUNT_PEERS_OUTBOUND, }, @@ -349,15 +351,37 @@ impl PeersManager { self.peers.get(peer_id).map(|peer| peer.reputation) } - /// Apply the corresponding reputation change to the given peer + /// Apply the corresponding reputation change to the given peer. + /// + /// If the peer is a trusted peer, it will be exempt from reputation slashing for certain + /// reputation changes that can be attributed to network conditions. If the peer is a + /// trusted peer, it will also be less strict with the reputation slashing. pub(crate) fn apply_reputation_change(&mut self, peer_id: &PeerId, rep: ReputationChangeKind) { let outcome = if let Some(peer) = self.peers.get_mut(peer_id) { // First check if we should reset the reputation if rep.is_reset() { peer.reset_reputation() } else { - let reputation_change = self.reputation_weights.change(rep); - peer.apply_reputation(reputation_change.as_i32()) + let mut reputation_change = self.reputation_weights.change(rep).as_i32(); + if peer.is_trusted() { + // exempt trusted peers from reputation slashing for + if matches!( + rep, + ReputationChangeKind::Dropped | + ReputationChangeKind::BadAnnouncement | + ReputationChangeKind::Timeout | + ReputationChangeKind::AlreadySeenTransaction + ) { + return + } + + // also be less strict with the reputation slashing for trusted peers + if reputation_change < MAX_TRUSTED_PEER_REPUTATION_CHANGE { + // this caps the reputation change to the maximum allowed for trusted peers + reputation_change = MAX_TRUSTED_PEER_REPUTATION_CHANGE; + } + } + peer.apply_reputation(reputation_change) } } else { return @@ -1962,6 +1986,58 @@ mod tests { } } + #[tokio::test] + async fn test_reputation_change_trusted_peer() { + let peer = PeerId::random(); + let socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 1, 2)), 8008); + let mut peers = PeersManager::default(); + peers.add_trusted_peer(peer, socket_addr); + + match event!(peers) { + PeerAction::PeerAdded(peer_id) => { + assert_eq!(peer_id, peer); + } + _ => unreachable!(), + } + match event!(peers) { + PeerAction::Connect { peer_id, remote_addr } => { + assert_eq!(peer_id, peer); + assert_eq!(remote_addr, socket_addr); + } + _ => unreachable!(), + } + + assert_eq!(peers.peers.get_mut(&peer).unwrap().state, PeerConnectionState::PendingOut); + peers.on_active_outgoing_established(peer); + assert_eq!(peers.peers.get_mut(&peer).unwrap().state, PeerConnectionState::Out); + + peers.apply_reputation_change(&peer, ReputationChangeKind::BadMessage); + + { + let p = peers.peers.get(&peer).unwrap(); + assert_eq!(p.state, PeerConnectionState::Out); + // not banned yet + assert!(!p.is_banned()); + } + + // ensure peer is banned eventually + loop { + peers.apply_reputation_change(&peer, ReputationChangeKind::BadMessage); + + let p = peers.peers.get(&peer).unwrap(); + if p.is_banned() { + break; + } + } + + match event!(peers) { + PeerAction::Disconnect { peer_id, .. } => { + assert_eq!(peer_id, peer); + } + _ => unreachable!(), + } + } + #[tokio::test] async fn test_reputation_management() { let peer = PeerId::random(); diff --git a/crates/net/network/src/peers/reputation.rs b/crates/net/network/src/peers/reputation.rs index aa431d71d7..de5ec27484 100644 --- a/crates/net/network/src/peers/reputation.rs +++ b/crates/net/network/src/peers/reputation.rs @@ -37,6 +37,13 @@ const BAD_PROTOCOL_REPUTATION_CHANGE: i32 = i32::MIN; // todo: current value is a hint, needs to be set properly const BAD_ANNOUNCEMENT_REPUTATION_CHANGE: i32 = REPUTATION_UNIT; +/// The maximum reputation change that can be applied to a trusted peer. +/// This is used to prevent a single bad message from a trusted peer to cause a significant change. +/// This gives a trusted peer more leeway when interacting with the node, which is useful for in +/// custom setups. By not setting this to `0` we still allow trusted peer penalization but less than +/// untrusted peers. +pub(crate) const MAX_TRUSTED_PEER_REPUTATION_CHANGE: Reputation = 2 * REPUTATION_UNIT; + /// Returns `true` if the given reputation is below the [`BANNED_REPUTATION`] threshold #[inline] pub(crate) fn is_banned_reputation(reputation: i32) -> bool {