-
Notifications
You must be signed in to change notification settings - Fork 808
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
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 |
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 <michael@sigmaprime.io>
I keep being notified for PR's like #7009 where it doesn't touch the specified directories in the `CODEOWNERS` file. After reading the [docs](https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners) not having a forward slash in beginning of the path means: > In this example, @octocat owns any file in an apps directory > anywhere in your repository. whereas with the slashes: > In this example, @doctocat owns any file in the `/docs` > directory in the root of your repository and any of its > subdirectories. this update makes it more rigid for the files in the jxs ownership
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.