attestantio / go-eth2-client

Apache License 2.0
102 stars 59 forks source link

Latest lodestar dev build is incompatible due to unexpected SSZ responses #144

Closed pk910 closed 4 weeks ago

pk910 commented 4 weeks ago

Lodestar recently introduced SSZ serialization for endpoints that do not require SSZ serialization according to spec: PR-6749

This unfortunately breaks compatibility with this library because go-eth2-client uses a generic Accept header that prefers SSZ responses for all endpoints. In particular it breaks s.Genesis() as the response is always decoded as JSON data, which breaks with SSZ responses. I'm pretty sure there are more endpoints with the same problem.

I think this should be fixed on go-eth2-client side, so the lib only asks for SSZ encoded data if the underlying implementation is capable of parsing the SSZ response.

nflaig commented 4 weeks ago

Nice catch @pk910, I have not tested the branch against Vouch since I was just using Kurtosis for client interop testing. Definitely agree here that the client should only sent media types in Accept header which it can actually handle.

mcdee commented 4 weeks ago

145 should address this; please could you take a look to see if you're happy with this and I'll merge it?

Note that I want to add support for SSZ to all of the endpoints, so ultimately should be able to remove the flag but for a quick fix this is the easiest way to proceed.

nflaig commented 4 weeks ago

https://github.com/attestantio/go-eth2-client/pull/145 should address this; please could you take a look to see if you're happy with this and I'll merge it?

LGTM

pk910 commented 4 weeks ago

Looks good 👍

mcdee commented 4 weeks ago

v0.21.6 is now available with #145 .