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

[Proposal] The bft module's gateway should check block request and block response from peers to avoid spam #3324

Closed
elderhammer opened this issue Jun 23, 2024 · 3 comments · Fixed by #3369
Assignees

Comments

@elderhammer
Copy link
Contributor

💥 Proposal

Issue #3315 points out that malicious validator can DDoS other validator by sending block response spam.
To solve this problem, in addition to putting the deserialization logic into a separate rayon thread #3316, we should also:

  1. Check the request frequency of block request
  2. Check whether the block response has a corresponding block request

In fact, node/router has already implemented the above two checks:

// Insert the block request for the peer, and fetch the recent frequency.
let frequency = self.router().cache.insert_inbound_block_request(peer_ip);
// Check if the number of block requests is within the limit.
if frequency > Self::MAXIMUM_BLOCK_REQUESTS_PER_INTERVAL {
bail!("Peer '{peer_ip}' is not following the protocol (excessive block requests)")
}

// Remove the block request, checking if this node previously sent a block request to this peer.
if !self.router().cache.remove_outbound_block_request(peer_ip, &request) {
bail!("Peer '{peer_ip}' is not following the protocol (unexpected block response)")
}

@raychu86 raychu86 assigned raychu86 and vicsn and unassigned raychu86 Jul 17, 2024
@raychu86
Copy link
Contributor

Thanks for bringing this up @elderhammer, I believe that this is something we should address, and should in theory be fairly straightforward to do so. The changes should resolve #3315.

@vicsn It may be easier for you to tackle the rayon thread for deserialization in the block deserialization since you already did it for the Router - #3304.

I can tackle the cache checks here, which hopefully should also be pretty simple.

@elderhammer
Copy link
Contributor Author

elderhammer commented Jul 18, 2024

@vicsn It may be easier for you to tackle the rayon thread for deserialization in the block deserialization since you already did it for the Router - #3304.

There is already a pull request to tackle this part of the issue: #3316

@vicsn vicsn removed their assignment Jul 18, 2024
@raychu86
Copy link
Contributor

@ljedrz @niklaslong May be better if you guys tackle the cache updates. We may also want to consider if we can unify the caches down the line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants