-
Notifications
You must be signed in to change notification settings - Fork 2.3k
feat(anvil): extend Content-Type support on Beacon API #12611
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
feat(anvil): extend Content-Type support on Beacon API #12611
Conversation
- Support "application/octet-stream" and "application/json" content types - Implemented `must_be_ssz` helper to determine if the Accept header prefers SSZ encoding. - Updated `handle_get_blobs` to return SSZ-encoded blobs when requested. - Enhanced tests to verify both JSON and SSZ responses for blob sidecars. - Added accept header assertions in tests to ensure correct content type selection.
mattsse
left a comment
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.
lgtm
grandizzy
left a comment
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.
thank you, lgtm, left couple of comments
|
|
||
| /// Helper function to determine if the Accept header indicates a preference for SSZ (octet-stream) | ||
| /// over JSON. | ||
| pub fn must_be_ssz(headers: &HeaderMap) -> bool { |
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.
nit: shouldn't be public
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.
This was merged too fast 😅
I may fix it as a follow-up. Btw Anvil's server module can be split/reorganized for clarity:
- crates/anvil/src/server/mod.rs
- crates/anvil/src/server/beacon/
- crates/anvil/src/server/rpc/
I can wrap it all in one PR.
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.
thank you! yeah not a prio, can include in future PR
| let mut json_q = 0.0; | ||
|
|
||
| // Parse each media type in the Accept header | ||
| for media_type in accept_str.split(',') { |
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.
should we worry if same media type present multiple times? (I think that's technically allowed) - probably not an issue as we'll consider the last one
Motivation
Close #12610
Solution
must_be_sszhelper to determine if the Accept header prefers SSZ encoding.handle_get_blobsto return SSZ-encoded blobs when requested.PR Checklist