-
Notifications
You must be signed in to change notification settings - Fork 743
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
web3Signer: set header "Accept: application/json" as we expect json in the response #5692
Conversation
…n the response The web3signer handler in lighthouse requires a json response. Setting the header "Accept: application/json" indicates to the web3signer that json is an acceptable response. Signed-off-by: Lukas Rusak <lorusak@gmail.com>
Nice find, thanks! If you don't mind filling in the contributor agreement, we can get this merged soon! |
Signed 😊 |
… json in the response
I pushed a fixup to make the ci happy |
thanks, will merge once CI passes |
It looks like there are a couple unrelated failures. Should I just squash the fixup? |
Don't worry about it, I'll merge your PR in a batch with this one which should fix the Clippy failure: |
Some of the other failures (the Windows one) are just flakiness. I'll kick the CI server to get it to try harder 🤣 |
@Mergifyio queue |
🛑 Command
|
…n the response (#5692) Squashed commit of the following: commit 92f5ad0 Author: Lukas Rusak <lorusak@gmail.com> Date: Thu May 2 15:53:58 2024 -0700 fixup! web3Signer: set header "Accept: application/json" as we expect json in the response commit 8965009 Author: Lukas Rusak <lorusak@gmail.com> Date: Wed May 1 21:16:27 2024 -0700 web3Signer: set header "Accept: application/json" as we expect json in the response The web3signer handler in lighthouse requires a json response. Setting the header "Accept: application/json" indicates to the web3signer that json is an acceptable response. Signed-off-by: Lukas Rusak <lorusak@gmail.com>
@Mergifyio requeue |
❌ This pull request head commit has not been previously disembarked from queue. |
@Mergifyio requeue |
❌ This pull request head commit has not been previously disembarked from queue. |
@mergify queue |
🛑 The pull request has been removed from the queue
|
@mergify unqueue |
✅ The pull request has been removed from the queue
|
@mergify queue |
🛑 The pull request has been removed from the queue
|
Anything else I can help out with here? |
@mergify queue |
🛑 The pull request has been removed from the queue
|
@Mergifyio requeue |
✅ This pull request will be re-embarked automaticallyThe followup |
🛑 The pull request has been removed from the queue
|
Sorry, our CI seems to be particularly flaky. |
@Mergifyio requeue |
✅ This pull request will be re-embarked automaticallyThe followup |
🛑 The pull request has been removed from the queue
|
✅ This pull request will be re-embarked automaticallyThe followup |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at da9d386 |
Can this be included in #5664? |
@lrusak it already is |
Hmm ok, I thought I saw it in there before but now I don't see the commit there and I don't see the change in the change list. Maybe it got dropped accidentally? |
@lrusak Your PR got merged to unstable. The v5.2.0 PR just shows new commits on top of unstable. If you do a git log on that branch you'll see it |
ah ok, sorry for the noise! |
Issue Addressed
#5691
Proposed Changes
This PR sets the request header for the web3signer to
Accept: application/json
. This is required to indicate to a remote signer that json is the acceptable return type (instead ofplain/text
)Additional Info
the web3signer api indicates that both
application/json
andtext/plain
are acceptable response types for signing requests.The web3signer handler in lighthouse requires a json response. Setting the header "Accept: application/json" indicates to the web3signer that json is an acceptable response.