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

Re-evaluate rate-limiting for BlobsByRange requests #6773

Closed
michaelsproul opened this issue Jan 9, 2025 · 2 comments
Closed

Re-evaluate rate-limiting for BlobsByRange requests #6773

michaelsproul opened this issue Jan 9, 2025 · 2 comments
Labels
electra Required for the Electra/Prague fork Networking

Comments

@michaelsproul
Copy link
Member

Description

In the max_blobs_per_block PR we changed the heuristic for estimating the maximum number of blobs exchanged in a BlobsByRange request:

We are using a hardcoded constant (16) which is a bit higher than what we're likely to see in practice (6 before Electra, 9 after).

In a future PR we could try to plumb the ChainSpec into this function so we can get a more accurate estimate, or we could stick with the current approach if no issues arise because of it.

@michaelsproul
Copy link
Member Author

Plumbing in the ChainSpec is a bit of a PITA. The RateLimiter really seems like it doesn't want to have a ChainSpec near it. The RPCHandler is fine because it has a ForkContext.

Maybe we could re-do @jxs's changes from this PR, but with ssz(skip_deserializing, skip_serializing) on the max_blobs_per_block field?

@michaelsproul
Copy link
Member Author

This ended up getting fixed in the version of the max_blobs_per_block PR that we merged to unstable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electra Required for the Electra/Prague fork Networking
Projects
None yet
Development

No branches or pull requests

1 participant