Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Decrease the peer reputation on invalid block requests #8260

Merged
merged 5 commits into from
Mar 9, 2021

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Mar 3, 2021

This pr changes the block request handler to decrease the reputation of
peers when they send the same request multiple times or they send us an
invalid block request.

Fix: #8257

This pr changes the block request handler to decrease the reputation of
peers when they send the same request multiple times or they send us an
invalid block request.
@bkchr bkchr added A0-please_review Pull request needs code review. B5-clientnoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Mar 3, 2021
@bkchr bkchr requested review from tomaka and arkpar March 3, 2021 21:55
client/network/src/block_request_handler.rs Outdated Show resolved Hide resolved
client/network/src/block_request_handler.rs Outdated Show resolved Hide resolved
Comment on lines +51 to +52
/// Reputation change when a peer sent us the same request multiple times.
pub const SAME_REQUEST: Rep = Rep::new(i32::min_value(), "Same block request multiple times");
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it so punitive to ask the same thing twice?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm still waiting for some answer from the author of the linked issue to see if that fixes the problem. As reported, they assume that someone is spamming them with the same request over and over again.

It is also fine to send the same request twice (or even more times when you wait long enough between the requests), however the same request should not be send multiple times in a short time period. This probably just reveals some bug in the sync code, but I still don't see any problem why we should no restrict this.

@crystalin
Copy link
Contributor

crystalin commented Mar 4, 2021

moved the comment directly to the issue: #8257

@bkchr bkchr requested a review from tomaka March 5, 2021 08:57
bkchr added a commit to paritytech/polkadot that referenced this pull request Mar 5, 2021
@bkchr bkchr merged commit 7e5c307 into master Mar 9, 2021
@bkchr bkchr deleted the bkchr-block-request-reputation branch March 9, 2021 13:43
eskimor added a commit that referenced this pull request Mar 10, 2021
eskimor added a commit that referenced this pull request Mar 10, 2021
bkchr pushed a commit that referenced this pull request Mar 10, 2021
KalitaAlexey pushed a commit to KalitaAlexey/substrate that referenced this pull request Jul 9, 2021
* Decrease the peer reputation on invalid block requests

This pr changes the block request handler to decrease the reputation of
peers when they send the same request multiple times or they send us an
invalid block request.

* Review feedback

* Change log target

* Remove unused code
jordy25519 pushed a commit to cennznet/substrate that referenced this pull request Sep 17, 2021
* Decrease the peer reputation on invalid block requests

This pr changes the block request handler to decrease the reputation of
peers when they send the same request multiple times or they send us an
invalid block request.

* Review feedback

* Change log target

* Remove unused code
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Peer flooding block requests should get their reputation impacted
4 participants