Add support to builder validator registration using ssz - #9065
Conversation
| + " is not a supported milestone for the builder rest api. Milestones >= Bellatrix are supported.")); | ||
| + " is not a supported milestone for the builder rest api. Milestones >= Bellatrix are " | ||
| + "supported.")); |
There was a problem hiding this comment.
could be unchanged really...
| assertThat(mockWebServer.getRequestCount()).isEqualTo(3); | ||
| verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); | ||
| verifyPostRequestJson("/eth/v1/builder/validators", signedValidatorRegistrationsRequest); | ||
| verifyPostRequestSsz("/eth/v1/builder/validators", signedValidatorRegistrations); |
There was a problem hiding this comment.
nit: we should probably make a tiny change here to show that the first set are verified and the second set is just the ssz call... we basically show that with the asserts above but it'd be clearer if this last check is just seeing 1 request, and just ssz.
There was a problem hiding this comment.
I guess I can move the early assertions closer to where we exercise the request to make them a bit more "sequential".
| } else { | ||
| return SafeFuture.completedFuture(response); | ||
| } |
There was a problem hiding this comment.
you could avoid else here ;-)
tbenr
left a comment
tbenr
left a comment
There was a problem hiding this comment.
I think what is missing is a test checking that the fallback doesn't happen on non-http errors, like timeout or connection errors.
tbenr
left a comment
tbenr
left a comment
There was a problem hiding this comment.
LGTM. Seems like adding a missing final makes it good to go
3 participants
PR Description
Implement support for SSZ requests when registering validators via the Builder endpoint. Logic based on this spec ethereum/builder-specs#110.
Since we don't have a way to know if the relay supports SSZ, our best bet is to try SSZ and if it fails fallback into JSON. However, we do not want to keep trying SSZ for every request after a failure so we are adding a backoff logic (24h).
To support testing, I created a
RestBuilderClientOptionsrecord. The reasoning was to bypass the way that we were relying directly on static configs, which made testing harder.Fixed Issue(s)
fixes #9003
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog