fix(p2p): process one request per reqresp stream to enforce rate limit#24552
Open
spalladino wants to merge 1 commit into
Open
Conversation
8f434d3 to
55deb1c
Compare
This file contains hidden or 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
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.
Context
The reqresp rate limiter is consulted once per inbound stream (
streamHandler), but the sub-protocol handler was invoked once per chunk read from that stream (processStream). Req/resp is one-request-one-response and an honest sender writes a single payload before half-closing, but a malicious peer can open one stream (costing a single rate-limit token) and then push many request frames on it — each frame arrives as its own chunk and drives a full handler invocation (mempool / block / tree lookups). Per-peer and global rate limits are bypassed by the fan-out factor.Approach
Make
processStreamhandle exactly one request per stream: after emitting the response for the first chunk, the pipeline generator returns instead of looping over the rest of the source. Extra frames a peer queued on the same stream are discarded when the stream closes, so work is bounded to one request per token. This does not regress legitimate traffic — no code path pipelines multiple requests on a single stream, and the one-chunk-per-request framing is already in force.The alternative (per-invocation rate checks inside the loop) was rejected: nothing pipelines, and mid-stream status signalling is broken on the requester side, which parses the first chunk as status and concatenates the rest as data.
Adds a unit test that drives
processStreamwith three frames on one stream and asserts the handler runs once and the sink receives a single SUCCESS + response pair.Fixes A-1324