-
-
Notifications
You must be signed in to change notification settings - Fork 291
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: set consensus version header in state and block response #6152
Conversation
const response = await api.getBlockV2(...reqSerializers.getBlockV2.parseReq(req)); | ||
if (response instanceof Uint8Array) { | ||
const slot = extractSlotFromBlockBytes(response); | ||
const version = config.getForkName(slot); | ||
void res.header("Eth-Consensus-Version", version); |
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.
The void
is required here because eslint warns about floating promises due to header
return value being thenable but it does not return a promise. Using await here will keep the process stuck when calling the API. This is a known issue, see fastify/fastify#4246 and fastify/help#775.
Performance Report✔️ no performance regression detected Full benchmark results
|
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.
lgtm
🎉 This PR is included in v1.13.0 🎉 |
Motivation
Beacon-API spec requires to set
Eth-Consensus-Version
in getStateV2 and getBlockV2 response.Description
Set consensus version header in state and block response in both json and ssz response as per spec both require the header.
Confirmed results manually as current API client design does not allow to access header values in tests.
Note: This is not a long-term solution, all the code here will be removed in API refactoring done in #6080
Closes #6123
Closes #6124