Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: be less strict when slashing trusted peers #7052

Merged
merged 2 commits into from
Mar 8, 2024

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Mar 8, 2024

ref #7037

This adds a check before applying a reputation change and prevents slashing of trusted peers for certain errors and is less strict when applying the reputation change

this could perhaps need a bit more tuning or even prevent banning of trusted peers entirely (will open an issue).

@mattsse mattsse added the A-networking Related to networking in general label Mar 8, 2024
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Comment on lines +40 to +45
/// 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this rather a minimum since REPUTATION_UNIT is negative?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's negative, but it should act as the maximum reputation change we want to apply:

max(4 * Unit, MAX_TRUSTED_PEER_REPUTATION_CHANGE) == MAX_TRUSTED_PEER_REPUTATION_CHANGE

@mattsse mattsse enabled auto-merge March 8, 2024 19:50
@mattsse mattsse added this pull request to the merge queue Mar 8, 2024
Merged via the queue into main with commit bf948d1 Mar 8, 2024
27 checks passed
@mattsse mattsse deleted the matt/less-struct-trusted-peers branch March 8, 2024 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-networking Related to networking in general
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants