-
Notifications
You must be signed in to change notification settings - Fork 324
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
[api] Add ratelimit for websocket API #4031
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4031 +/- ##
==========================================
- Coverage 76.51% 76.19% -0.32%
==========================================
Files 340 332 -8
Lines 29273 28330 -943
==========================================
- Hits 22397 21587 -810
+ Misses 5761 5643 -118
+ Partials 1115 1100 -15 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can rate limit be implemented in gateway?
No, because the websocket is a long connection, the gateway cannot ratelimit requests inside the connection. |
api/serverV2.go
Outdated
@@ -65,7 +66,8 @@ func NewServerV2( | |||
|
|||
wrappedWeb3Handler := otelhttp.NewHandler(newHTTPHandler(web3Handler), "web3.jsonrpc") | |||
|
|||
wrappedWebsocketHandler := otelhttp.NewHandler(NewWebsocketHandler(web3Handler), "web3.websocket") | |||
limiter := rate.NewLimiter(rate.Limit(cfg.WebsocketMaxRateMessages), 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it means allowing 5 req/s and only 1 req at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it means allowing 5 req/s and only 1 req at the same time?
once a client is connected, the WebSocket server will limit the request rate to 5 req/s for this client. There is no limit to the number of client sides connected to the server( can limit in gateway)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, what does the second param 1
means?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, what does the second param
1
means?
The second parameter determines the maximum burst events that the limiter can tolerate. For example, if the value of b
is 10, then even if 10 events suddenly occur in a short time. set to 1
means that the rate limit is based on the setting of the first parameter, and too many bursts are not allowed.
"net/http" | ||
"sync" | ||
"time" | ||
|
||
"github.com/gorilla/websocket" | ||
"go.uber.org/zap" | ||
"golang.org/x/time/rate" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
golang.org/x/time/rate
already meets our needs, i think no need to import a third-party library
Quality Gate failedFailed conditions 6.6% Duplication on New Code (required ≤ 3%) |
Quality Gate passedIssues Measures |
Description
This PR introduces a rate limit for the websocket API to control the maximum number of messages a client can send per second. This is to ensure the stability and performance of the API by preventing any potential abuse or overload.
Fixes #(issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Checklist: