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

Support SSZ for v2 blocks API endpoint #6564

Closed
mcdee opened this issue Mar 19, 2024 · 5 comments · Fixed by #6749
Closed

Support SSZ for v2 blocks API endpoint #6564

mcdee opened this issue Mar 19, 2024 · 5 comments · Fixed by #6749
Labels
meta-feature-request Issues to track feature requests. scope-interop Issues that fix interop issues between Lodestar and CL, EL or tooling. scope-ux Issues for CLI UX or general consumer UX.

Comments

@mcdee
Copy link

mcdee commented Mar 19, 2024

Problem description

The API endpoint /eth/v2/beacon/blocks for POSTing blocks does not accept SSZ-encoded blocks. Attempting to send one returns the following response:

{
  "statusCode": 415,
  "code": "FST_ERR_CTP_INVALID_MEDIA_TYPE",
  "error": "Unsupported Media Type",
  "message": "Unsupported Media Type: application/octet-stream"
}

These are accepted by other consensus clients, meaning that lodestar currently needs special handling. It would be great if lodestar supported SSZ on this endpoint.

Solution description

Accept the content type encoding of "application/octet-stream" and decode the body as SSZ.

Additional context

No response

@mcdee mcdee added the meta-feature-request Issues to track feature requests. label Mar 19, 2024
@philknows philknows added scope-interop Issues that fix interop issues between Lodestar and CL, EL or tooling. scope-ux Issues for CLI UX or general consumer UX. labels Mar 20, 2024
@philknows philknows added this to the Short-Term Features/Issues milestone Mar 20, 2024
@nflaig
Copy link
Member

nflaig commented Mar 21, 2024

We can't properly support this right now, but it will be addressed by API overhaul which adds first class ssz support of request and responses to (almost) all APIs.

@nflaig
Copy link
Member

nflaig commented Mar 21, 2024

@mcdee Was just digging a bit more into the code, we could probably add a quickfix for this but as the api refactoring is currently wip I would rather not spent time on that.

What about adding more general handling of 415 errors if request body is not supported, e.g. Teku currently will send the body as ssz and if it receive a 415 will resend the same request as json and store that ssz is not supported for this route (see #5693 (comment)).

In Lodestar, we plan to implement this as well as we want to send ssz request bodies for (almost) all APIs. In ethereum/beacon-APIs#250 you outlined pretty well how responses should be handled but I think we should also define how a well-behaved client has to handle requests. First class ssz support should ideally cover both request and response.

@mcdee
Copy link
Author

mcdee commented Mar 22, 2024

Although it would be nice, the problem with sending SSZ first always, and then falling back to JSON, is the cost of doing so in terms of time taken. This particularly matters for block proposals. As for internally flagging if the endpoint takes SSZ, it is of course possible but still at the cost of the initial hit. And it requires other beacon nodes to follow a similar strategy of responses for unhandled content types.

At current I'm hard-coding SSZ support for each endpoint by different beacon nodes on a per-endpoint basis as I'm adding SSZ support to these endpoints. I'll see if some sort of fallback option would be workable for the other beacon nodes and non-critical endpoints.

@mcdee
Copy link
Author

mcdee commented Mar 22, 2024

After taking a look at the other beacon nodes none of them return 415 if they don't support the supplied content type for an arbitrary endpoint, so it would be a big push to get them all to that level of support. Looks like hard-coding is the only sane way to go for now.

@nflaig
Copy link
Member

nflaig commented Mar 22, 2024

other beacon nodes none of them return 415 if they don't support the supplied content type

That's definitely something that needs to be pushed for, I would have expected that at least Teku BN supports this if their VC relies on it. The approach I wanna use in Lodestar still kinda works because we will default to ssz if the spec has it defined for the route, otherwise we probably have to default to json for now but configurable via flag to always use ssz first to get full benefit if Lodestar BN <> VC is used. I guess this would mean just bad luck if the implementation does not follow the spec in regard to supported request body and does not return 415.

hard-coding is the only sane way to go for now

This is probably not something we will go for in Lodestar but it great that you can do it in Vouch for now, and workaround us not following the spec currently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta-feature-request Issues to track feature requests. scope-interop Issues that fix interop issues between Lodestar and CL, EL or tooling. scope-ux Issues for CLI UX or general consumer UX.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants