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

POST /eth/v1/beacon/blocks for altair #142

Merged
merged 3 commits into from
May 17, 2021

Conversation

rolfyone
Copy link
Collaborator

Given the discussion is leaning towards a POST that takes no indicators for which fork the block is, it would be simple to extend the existing POST to also accept altair blocks.

This PR demonstrates the concept for discussion, as an alternative to #141

Given the discussion is leaning towards a POST that takes no indicators for which fork the block is, it would be simple to extend the existing POST to also accept altair blocks.

This PR demonstrates the concept for discussion, as an alternative to ethereum#141
@dapplion
Copy link
Collaborator

dapplion commented May 13, 2021

Using this approach we could remove all other v2 methods right?

  • /eth/v2/debug/beacon/states/{state_id}: fork can be obtained from the JSON payload at state.slot. Also the binary version of this route does not include the fork version anyway. Even getting an SSZ serialized payload you can still read the slot only then deserialize.
  • /eth/v2/validator/blocks/:slot: same as this PR

@rolfyone
Copy link
Collaborator Author

Using this approach we could remove all other v2 methods right?

  • /eth/v2/debug/beacon/states/{state_id}: fork can be obtained from the JSON payload at state.slot. Also the binary version of this route does not include the fork version anyway. Even getting an SSZ serialized payload you can still read the slot only then deserialize.
  • /eth/v2/validator/blocks/:slot: same as this PR

I was implementing a remote validator create unsigned block yesterday, and it actually was easier in that instance to know it was an altair block, because otherwise the deserializer needed to be aware of the spec, so in that instance the v2 was actually helpful.

I guess the thing with the POST here, is that the old request payload is 100% valid, we're just allowing a new payload to be sent as an alternative. In a GET, we'd be returning a different structure to the client, and that's a bigger problem in a way - which is why REST apis generally don't make incompatible changes on a http response.

@mpetrunic mpetrunic merged commit d236812 into ethereum:master May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants