Skip to content

feat: add rate limiter for L1 deprecation#889

Merged
Theodus merged 2 commits intomainfrom
theodus/l1-deprecation
Jul 17, 2024
Merged

feat: add rate limiter for L1 deprecation#889
Theodus merged 2 commits intomainfrom
theodus/l1-deprecation

Conversation

@Theodus
Copy link
Member

@Theodus Theodus commented Jul 16, 2024

This adds a per API key rate limiter for the L1 deprecation strategy. Note that this will only be configured on the L1 gateway which only gets 5-15 queries per second total at the time of writing.

@Theodus Theodus requested review from IainM22 and LNSD July 16, 2024 16:00
@Theodus Theodus force-pushed the theodus/l1-deprecation branch from 8ba42e5 to 6cebf1d Compare July 16, 2024 16:05

if let Some(rate_limiter) = &self.rate_limiter {
if rate_limiter.above_limit(token) {
bail!("rate limit exceeded");
Copy link

Choose a reason for hiding this comment

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

Should we mention why its being rate limited? Maybe something along the lines of Rate limit exceeded. Querying L1 subgraphs is deprecated and will be removed soon.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good suggestion. Done

@LNSD
Copy link
Contributor

LNSD commented Jul 17, 2024

We already have https://github.com/edgeandnode/gateway/blob/main/gateway-framework/src/http/middleware/rate_limiter.rs; why don't we use it instead of modifying and adding another "unrelated" responsibility to the require_auth module?

@Theodus
Copy link
Member Author

Theodus commented Jul 17, 2024

@LNSD I don't see how this is "unrelated". We're imposing a limit on API keys similar to checking authorized subgraphs, payment status, etc. So the behavior is all localized, and will be trivial to remove when we no longer need it.

Copy link
Contributor

@LNSD LNSD left a comment

Choose a reason for hiding this comment

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

Ok. Fain enough 👍✅

@Theodus Theodus merged commit a7864a5 into main Jul 17, 2024
@Theodus Theodus deleted the theodus/l1-deprecation branch July 17, 2024 15:28
@Theodus Theodus mentioned this pull request Jul 22, 2024
Theodus added a commit that referenced this pull request Jul 22, 2024
# Release Notes
- feat: add to indexer & client query kafka messages (#879)
- fix: add optimistic progress to indexing status report (#887)
- feat: add rate limiter for L1 deprecation (#889, #898)
- fix: handle indexer-service error format (#896)
- fix: remove block constraint rewrites (#895)
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 this pull request may close these issues.

3 participants