-
Notifications
You must be signed in to change notification settings - Fork 809
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
Fix builder API headers #7009
base: release-v7.0.0
Are you sure you want to change the base?
Fix builder API headers #7009
Conversation
let's target this at the release-v7.0.0-beta.0 branch, which we'll use for future releases too (I should rename the branch most likely) |
1e2593b
to
d131e1d
Compare
Tested this with lighthouse and other CL clients with the mock builder and ssz support works as expected. |
dc3f870
to
f931c16
Compare
Nice, ty. Rebased on a new branch called |
/// Indicates that the `get_header`` response had content-type ssz | ||
/// so we can set content-type header to ssz to make the `submit_blinded_blocks` | ||
/// request. | ||
ssz_used: Arc<AtomicBool>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might be a better name?
ssz_used: Arc<AtomicBool>, | |
ssz_available: Arc<AtomicBool>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, substance looks good, just a few formatting nits
Co-authored-by: Michael Sproul <[email protected]>
Issue Addressed
Resolves #7000
Proposed Changes
Set the accept header on builder to the correct value when requesting ssz.
This PR also adds a flag to disable ssz over the builder api altogether. In the case that builders/relays have an ssz bug, we can react quickly by asking clients to restart their nodes with the
--disable-ssz-builder
flag to force json. I'm not fully convinced if this is useful so open to removing it or opening another PR for it.Testing this currently.