Skip to content

Commit

Permalink
peer_store: Increase peer ban time until escapes banned threshold (pa…
Browse files Browse the repository at this point in the history
…ritytech#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:
- paritytech#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 <[email protected]>
  • Loading branch information
lexnv authored Apr 9, 2024
1 parent 4e73c0f commit b1c9209
Showing 1 changed file with 15 additions and 8 deletions.
23 changes: 15 additions & 8 deletions substrate/client/network/src/peer_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit b1c9209

Please sign in to comment.