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

[Clarification] Skipped slots in /eth/v1/beacon/blocks/{block_id} endpoint #126

Open
realbigsean opened this issue Feb 18, 2021 · 6 comments

Comments

@realbigsean
Copy link
Contributor

Should skipped slots return a 404 in the /eth/v1/beacon/blocks/{block_id} endpoint? In Lighthouse we return information about the most recent non-skipped slot presently. I don't see it explicitly in the spec.

Relevant lighthouse issue: sigp/lighthouse#2186

@ajsutton
Copy link
Contributor

For the record Teku will return a 404 in this case.
I dislike it being a 404 though as that suggests the user has made a mistake and it becomes impossible to tell if the block didn't exist or if you just got a part of the URL wrong (most often for me /block/ instead of /blocks/).

Might be too invasive to change now but I'd have gone with something like No Content which confirms we understood the request, found the slot you were asking about but didn't have any block there.

@paulhauner
Copy link
Contributor

I dislike it being a 404 though as that suggests the user has made a mistake and it becomes impossible to tell if the block didn't exist or if you just got a part of the URL wrong (most often for me /block/ instead of /blocks/).

Agreed. Something else that's worth distinguishing between is:

  • There is no block at this slot
  • The node did a weak-subjectivity start and has not yet synced back this far.

Might be too invasive to change now but I'd have gone with something like No Content which confirms we understood the request, found the slot you were asking about but didn't have any block there.

Agree for both points.

@mcdee
Copy link
Contributor

mcdee commented Feb 19, 2021

A more general comment, but does it make sense to start a v2 version of the API that contains enhancements such as those suggested above, as well as the alterations required for HF1? The structural items such as this can be handled now, and those that are dependent on the structures and processes in HF1 follow on afterwards.

Main reason is that I'd hate for solutions like the above to be unimplemented, but it appears to be generally felt that v1 is now final so cannot accommodate such enhancements.

@mpetrunic
Copy link
Collaborator

Adding proper response message and error code to 404 response to indicate whether it's a skip slot or simply node doesn't have it would be more appropriate?

@benjaminion
Copy link

benjaminion commented Feb 19, 2021

Adding proper response message and error code to 404 response to indicate whether it's a skip slot or simply node doesn't have it would be more appropriate?

As per @ajsutton , I think 204 No Content would be ideal. It could be accompanied with a message to distinguish between @paulhauner's scenarios*.

4xx codes indicate a client error, but it's not an error to request info about a skip slot (imo).

(*) Edit: 204 is "cacheable by default", so may not suit the temporarily-empty scenario. But, then again, so is 404

@rolfyone
Copy link
Collaborator

This is older, so in current context it sounds like what we want is:

  • add 204 to /eth/v2/beacon/blocks/{block_id}
  • add 204 to /eth/v1/beacon/blinded_blocks/{block_id}

and

  • fetch block_id and not found == 204 response, this might be a slot number or a root realistically
  • invalid block_id like foo would remain a 400...

QUESTIONS:

  • also add 204 to: /eth/v1/beacon/blocks/{block_id}/root?
  • also add 204 to: /eth/v1/beacon/blocks/{block_id}/attestations ?
  • does this extend to /eth/v1/beacon/headers/{block_id}?

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

No branches or pull requests

7 participants