-
-
Notifications
You must be signed in to change notification settings - Fork 262
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
feature(provider): Rate limiting #15583
base: master
Are you sure you want to change the base?
Conversation
try { | ||
// Create a key that combines IP with contract address or method to provide granular rate limiting | ||
// This is a more stable identifier than just IP alone, as recommended by Cloudflare | ||
const rateKey = contractAddress |
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.
In our case I think it will be almost exclusively POST
though,,, but not a a big deal!
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.
got it!
provider/src/handler.ts
Outdated
if (!isRateLimitAllowed) { | ||
return Response.json( | ||
{ | ||
id: body.id || 42, | ||
jsonrpc: '2.0', | ||
error: { | ||
code: -32005, | ||
message: 'Rate limit exceeded', | ||
}, | ||
}, | ||
{ | ||
status: 429, | ||
headers: { | ||
...headers, | ||
'Retry-After': '60', // Suggest retry after 60 seconds | ||
}, | ||
} | ||
) | ||
} | ||
} |
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.
ok for now, in order to not break anything, let;'s not actaully block things but add a log message so we cna see how that would perform. Once we have it running for 10 days or more we can go check at what would have actually been blocked and check that it works as expected. wdyt?
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.
sounds like a plan!
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
provider/src/handler.ts
Outdated
console.log( | ||
`RATE_LIMIT_WOULD_BLOCK: IP=${getClientIP(request)}, Method=${body.method}, Contract=${contractAddress || 'none'}, ID=${body.id || 'none'}` | ||
) |
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.
Actually, can we log the full body as well? This could be useful to debug 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.
(insead of just the ID which is not very useful!)
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.
I am feeling a bit anxious about merging this in one go because it's pretty big and we don't have tests.
Description
This PR introduces rate limiting, and a multi-tier lock address caching system for our
provider
to improve performance, reduce blockchain calls, and prevent abuse.Rate Limiting
Lock Address Caching
These changes improve response times, reduce RPC overhead, and ensure fair resource usage.
Issues
Fixes #15559
Refs #15559
Checklist: