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

Move state endpoint to beacon namespace #353

Merged
merged 3 commits into from
Sep 18, 2023
Merged

Conversation

rkapka
Copy link
Collaborator

@rkapka rkapka commented Sep 15, 2023

The /eth/v2/debug/beacon/states/{state_id} endpoint is widely used by regular users and as such should probably not be present in the debug namespace. I think the original reason for putting it there was the large size of the response, but currently requesting all validators from /eth/v1/beacon/states/head/validators results in an even larger response.

This PR:

  • creates a new /eth/v1/beacon/states/{state_id} endpoint (the debug version is v2, but since we are moving it to a new namespace, in my opinion versioning should start anew)
  • deprecates the debug endpoint

Copy link
Collaborator

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

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

LGTM, add this to the change list so we don't forget :)

Copy link
Collaborator

@rolfyone rolfyone left a comment

Choose a reason for hiding this comment

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

LGTM

@rolfyone rolfyone merged commit 4cc8484 into master Sep 18, 2023
3 checks passed
@rolfyone rolfyone deleted the move-state-to-beacon branch September 18, 2023 23:46
@arnetheduck
Copy link
Contributor

Uh, this was merged a bit quickly without time for feedback, but the condition we've discussed in the past for having the state endpoint in the official part of the api was to limit it to SSZ only - I think that change should be made, ie it's not feasible to create an expectation that client open up providing JSON for the state, which soon will reach GB sizes.

@arnetheduck
Copy link
Contributor

There was also discussion of fixing the flags encoding for json which is irregular with respect to its underlying intent.

rolfyone added a commit to rolfyone/beacon-APIs that referenced this pull request Sep 19, 2023
rolfyone added a commit that referenced this pull request Sep 19, 2023
@rolfyone rolfyone restored the move-state-to-beacon branch September 19, 2023 20:52
@rolfyone
Copy link
Collaborator

sorry @rkapka can you re-open a pull request for this to allow discussion to happen? I've reverted it from master after out of band discussions.

@rolfyone rolfyone mentioned this pull request Sep 19, 2023
@Falehfale Falehfale mentioned this pull request Sep 28, 2023
Closed
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.

4 participants