peer_store: Increase peer ban time until escapes banned threshold #4031
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
lexnv
added
A1-insubstantial
dmitry-markin
approved these changes
Apr 8, 2024
| @@ -39,15 +39,22 @@ use crate::protocol_controller::ProtocolHandle; | |||
| 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); | |||
There was a problem hiding this comment.
Worth double-checking we don't have this constant hardcoded anywhere in reputation costs.
lexnv
commented
Apr 8, 2024
| /// - `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; |
There was a problem hiding this comment.
We could also include a more complex mechanism for this (where the numbers are also configurable constants):
- if a peer is reported as banned at least 3 times within 5 minutes
- then we wait 10 minutes until the reputation is decayed (basically banning the peer for around 10 mins + 69 seconds)
It may be overkill for now, but something to think about if we still have misbehaving peers (like shown in kusama). Would love to get your thoughts on this @dmitry-markin 🙏
There was a problem hiding this comment.
The goal is to conserve resources not handling misbehaving peers' messages/requests. It seems already fine with ~1 min banning, but we have yet to observe the real DoS attack to make informed decisions regarding the banning mechanism in general.
|
The CI pipeline was cancelled due to failure one of the required jobs. |
Signed-off-by: Alexandru Vasile <[email protected]>
bkchr
approved these changes
Apr 8, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 28, 2024
Counterpart of: #4031 cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Sebastian Kunert <[email protected]>
TomaszWaszczyk
pushed a commit
to TomaszWaszczyk/polkadot-sdk
that referenced
this pull request
Jul 7, 2024
…ech#4906) Counterpart of: paritytech#4031 cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Sebastian Kunert <[email protected]>
TarekkMA
pushed a commit
to moonbeam-foundation/polkadot-sdk
that referenced
this pull request
Aug 2, 2024
…ech#4906) Counterpart of: paritytech#4031 cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Sebastian Kunert <[email protected]>
sfffaaa
pushed a commit
to peaqnetwork/polkadot-sdk
that referenced
this pull request
Dec 27, 2024
…ech#4906) Counterpart of: paritytech#4031 cc @paritytech/networking --------- Signed-off-by: Alexandru Vasile <[email protected]> Co-authored-by: Sebastian Kunert <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
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:
This is a followup of:
Testing Done
cc @paritytech/networking