ethereum / builder-specs

Specification for the external block builders.
https://ethereum.github.io/builder-specs/
Creative Commons Zero v1.0 Universal
179 stars 61 forks source link

Add SSZ support to builder api #104

Open nflaig opened 4 months ago

nflaig commented 4 months ago

This is a proposal to add SSZ support to builder API in a backward compatible way without requiring to bump the apis to v2.

It outlines what is expected from the client and how a well-behaved server should handle different content types, headers, and use appropriate error codes to allow the client to handle the error.

Notable changes are

Since we can't know if a builder implemented the changes as per spec, the client can determine if SSZ is supported by requesting an SSZ encoded response in the getHeader request. Preferably this should be done setting a Accept header with multiple entries and quality values like Accept: application/octet-stream;q=1.0,application/json;q=0.9 to give the builder the option to send a JSON response.

Related to https://github.com/ethereum/builder-specs/issues/53 and https://github.com/ethereum/builder-specs/pull/54

metachris commented 3 months ago

In support of this change. It's nicely backwards-compatible, doesn't add significant risk, decreases latency and could simplify CL flows.

ltitanb commented 3 months ago

Full support, it's a great improvement over the existing API and should be particularly useful for larger blocks. Maybe worth adding an example payload for SSZ as well?

nflaig commented 3 months ago

Maybe worth adding an example payload for SSZ as well?

I gave this a try but it has it's limitations since you can't display binary data on the explorer. The solution seems to be to use external examples so we could add ssz to existing examples and reference the files in the ssz examples.

However, there seems to be an issue https://github.com/swagger-api/swagger-ui/issues/5433, the examples won't be displayed on the explorer, it would be great if it would show an option to download the example file but it only shows no example available.

image

There might be a fix for this at some point like https://github.com/swagger-api/swagger-js/pull/2013. For now could do something like here https://github.com/nflaig/builder-specs/compare/ssz-support...nflaig:builder-specs:ssz-examples with the caveat that it won't be available on the explorer.

barnabasbusa commented 1 month ago

Would it make sense to mark the json api deprecated with this same PR? That way we can push to get json removed in the next fork.

nflaig commented 1 month ago

That way we can push to get json removed in the next fork.

In my opinion, having JSON as an option is a good thing, it can be quite useful for debugging.

If this reduces complexity significantly for some implementation it could be considered, however in Lodestar for example it does not make a difference at all as we have to keep JSON support for beacon-api anyways.

eserilev commented 1 month ago

Im in support of this. In Lighthouse its not a problem for us to support both JSON and SSZ

nflaig commented 1 week ago

Lodestar supports this in the latest release (v1.23.0) in case anyone needs a client to test with.