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

Clean up blockv3 metadata and client #5015

Merged
merged 9 commits into from
Dec 22, 2023

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Dec 18, 2023

Proposed Changes

  • Add the metadata fields like execution_payload_value to the JSON response for blocks/v3. Previously we were not following the spec.
  • Overhaul (simplify?) ForkVersionedResponse so that it can carry arbitrary metadata. This is used to smuggle the metadata without adding a new variant of ForkVersionedResponse. This required some refactoring elsewhere, but on the whole I think it's an improvement.
  • Parse and use the HTTP headers in the client code for the produce_block v3 API. This ensures the headers are checked in the tests, and enables blockdreamer to make use of the header metadata (see: Enable block v3 blockprint-collective/blockdreamer#5).

Additional Info

This PR conflicts slightly with #4813. I was thinking we could merge this PR first.

This PR does not include the builder_comparison_factor change (ethereum/beacon-APIs#386). That needs to happen in another PR.

@michaelsproul
Copy link
Member Author

I ended up wanting to test v3 JSON in blockdreamer, so I overhauled the JSON API and made it return the same type as the SSZ one. This PR will now clash with #4813, however unifying the types has simplified things a bit, and could allow us to do nice things in the VC, like select between SSZ (default!) and JSON requests based on a CLI flag.

@michaelsproul michaelsproul added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Dec 19, 2023
@michaelsproul michaelsproul changed the title Improve block production v3 client Update block v3 endpoint to conform to spec Dec 20, 2023
@michaelsproul michaelsproul changed the title Update block v3 endpoint to conform to spec Clean up blockv3 metadata and client Dec 20, 2023
@realbigsean
Copy link
Member

Nice!

Overhaul (simplify?) ForkVersionedResponse so that it can carry arbitrary metadata. This is used to smuggle the metadata without adding a new variant of ForkVersionedResponse. This required some refactoring elsewhere, but on the whole I think it's an improvement.

This was really smart! would be getting pretty hairy otherwise

Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

Love this, so much cleaner. Just a small question

@realbigsean realbigsean merged commit af11e78 into sigp:unstable Dec 22, 2023
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request HTTP-API ready-for-review The code is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants