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

feature(provider): Rate limiting #15583

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open

feature(provider): Rate limiting #15583

wants to merge 22 commits into from

Conversation

0xTxbi
Copy link
Member

@0xTxbi 0xTxbi commented Feb 28, 2025

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

  • IP-based limits (10 req/sec, 1000 req/hour)
  • Exemptions for Locksmith IPs and Unlock contracts
  • Configurable via environment variables

Lock Address Caching

  • In-memory cache (unlimited) for fast access
  • Edge-distributed cache (24h TTL) for frequent lookups
  • Persistent KV storage (1-year TTL) for long-term consistency
  • Auto-initialization and non-blocking operations for efficiency

These changes improve response times, reduce RPC overhead, and ensure fair resource usage.

Issues

Fixes #15559
Refs #15559

Checklist:

  • 1 PR, 1 purpose: my Pull Request applies to a single purpose
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the docs to reflect my changes if applicable
  • I have added tests (and stories for frontend components) that prove my fix is effective or that my feature works
  • I have performed a self-review of my own code
  • If my code involves visual changes, I am adding applicable screenshots to this thread

@0xTxbi 0xTxbi requested a review from julien51 February 28, 2025 18:03
@cla-bot cla-bot bot added the cla-signed label Feb 28, 2025
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
Copy link
Member

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!

Copy link
Member Author

Choose a reason for hiding this comment

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

got it!

Comment on lines 231 to 250
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
},
}
)
}
}
Copy link
Member

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

sounds like a plan!

Copy link

vercel bot commented Feb 28, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unlock-airdrops ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 28, 2025 8:43pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
unlock-storybook ⬜️ Ignored (Inspect) Feb 28, 2025 8:43pm

Comment on lines 231 to 233
console.log(
`RATE_LIMIT_WOULD_BLOCK: IP=${getClientIP(request)}, Method=${body.method}, Contract=${contractAddress || 'none'}, ID=${body.id || 'none'}`
)
Copy link
Member

Choose a reason for hiding this comment

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

💯

Copy link
Member

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.

Copy link
Member

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!)

Copy link
Member

@julien51 julien51 left a 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.

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

Successfully merging this pull request may close these issues.

Add rate limit to rpc
2 participants