From b1c9209a6ce23c3eb29f770fd4f9065f9bb65e7c Mon Sep 17 00:00:00 2001 From: Alexandru Vasile <60601340+lexnv@users.noreply.github.com> Date: Tue, 9 Apr 2024 14:24:49 +0300 Subject: [PATCH] peer_store: Increase peer ban time until escapes banned threshold (#4031) This is a tiny PR to increase the time a peer remains banned. A peer is banned when the reputation drops below a threshold. With every second, the peer reputation is exponentially decayed towards zero. For the previous setup: - decaying to zero from (i32::MAX or i32::MIN) would take 948 seconds (15mins 48seconds) - from i32::MIN to escaping the banned threshold would take 10 seconds This means we are decaying reputation a bit too aggressive and misbehaving peers can misbehave again in 10 seconds. Another side effect of this is that we have encountered multiple warnings caused by a few misbehaving peers. In the new setup: - decaying to zero from (i32::MAX or i32::MIN) would take 3544 seconds (59 minutes) - from i32::MIN to escaping the banned threshold would take ~69 seconds This is a followup of: - https://github.com/paritytech/polkadot-sdk/pull/4000. ### Testing Done - Created a misbehaving client with [subp2p-explorer](https://github.com/lexnv/subp2p-explorer), the client is banned for approx 69seconds until it is allowed to connect again. cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile --- substrate/client/network/src/peer_store.rs | 23 ++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/substrate/client/network/src/peer_store.rs b/substrate/client/network/src/peer_store.rs index 15d5b746c2..a4c739f144 100644 --- a/substrate/client/network/src/peer_store.rs +++ b/substrate/client/network/src/peer_store.rs @@ -38,15 +38,22 @@ use wasm_timer::Delay; pub const LOG_TARGET: &str = "peerset"; /// We don't accept nodes whose reputation is under this value. -pub const BANNED_THRESHOLD: i32 = 82 * (i32::MIN / 100); +pub const BANNED_THRESHOLD: i32 = 71 * (i32::MIN / 100); /// Reputation change for a node when we get disconnected from it. const DISCONNECT_REPUTATION_CHANGE: i32 = -256; /// Relative decrement of a reputation value that is applied every second. I.e., for inverse -/// decrement of 50 we decrease absolute value of the reputation by 1/50. This corresponds to a -/// factor of `k = 0.98`. It takes ~ `ln(0.5) / ln(k)` seconds to reduce the reputation by half, -/// or 34.3 seconds for the values above. In this setup the maximum allowed absolute value of -/// `i32::MAX` becomes 0 in ~1100 seconds (actually less due to integer arithmetic). -const INVERSE_DECREMENT: i32 = 50; +/// decrement of 200 we decrease absolute value of the reputation by 1/200. +/// +/// This corresponds to a factor of `k = 0.955`, where k = 1 - 1 / INVERSE_DECREMENT. +/// +/// It takes ~ `ln(0.5) / ln(k)` seconds to reduce the reputation by half, or 138.63 seconds for the +/// values above. +/// +/// In this setup: +/// - `i32::MAX` becomes 0 in exactly 3544 seconds, or approximately 59 minutes +/// - `i32::MIN` becomes 0 in exactly 3544 seconds, or approximately 59 minutes +/// - `i32::MIN` escapes the banned threshold in 69 seconds +const INVERSE_DECREMENT: i32 = 200; /// Amount of time between the moment we last updated the [`PeerStore`] entry and the moment we /// remove it, once the reputation value reaches 0. const FORGET_AFTER: Duration = Duration::from_secs(3600); @@ -455,7 +462,7 @@ mod tests { #[test] fn decaying_max_reputation_finally_yields_zero() { const INITIAL_REPUTATION: i32 = i32::MAX; - const SECONDS: u64 = 1000; + const SECONDS: u64 = 3544; let mut peer_info = PeerInfo::default(); peer_info.reputation = INITIAL_REPUTATION; @@ -470,7 +477,7 @@ mod tests { #[test] fn decaying_min_reputation_finally_yields_zero() { const INITIAL_REPUTATION: i32 = i32::MIN; - const SECONDS: u64 = 1000; + const SECONDS: u64 = 3544; let mut peer_info = PeerInfo::default(); peer_info.reputation = INITIAL_REPUTATION;