Skip to content
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

json decoding: allow unknown fields (in preparation for Capella and EIP-4844) #393

Closed
wants to merge 1 commit into from

Conversation

metachris
Copy link
Collaborator

@metachris metachris commented Nov 4, 2022

📝 Summary

This PR allows unknown fields in the JSON request payloads.

  • New payload fields will be part of the upcoming Capella and EIP-4844 updates (see also Changes for Capella and EIP-4844 #392)
  • These new payload fields would make mev-boost break on the getPayload call unless a new API version is introduced
  • Since there's multiple upcoming changes, it seems to be overkill to add a new API version for each update
  • The simple solution is to allow unknown fields in the json payload, and have all users upgrade before any payload changes are deployed. This way the can be backwards compatible.
  • This limitation was applied only to the registerValidator and getPayload request payload from the beacon-node, where there is not real risk of resource exhaustion (and in case of getPayload the payload was fully read beforehand anyway)

I think this should go into the v1.4.0 release (see #377) in order to strengthen backwards compatibility with future payload additions.


✅ I have run these commands

  • make lint
  • make test-race
  • go mod tidy

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2022

Codecov Report

Merging #393 (e9775da) into main (f146349) will increase coverage by 0.14%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##             main     #393      +/-   ##
==========================================
+ Coverage   66.07%   66.21%   +0.14%     
==========================================
  Files           7        7              
  Lines         902      894       -8     
==========================================
- Hits          596      592       -4     
+ Misses        271      267       -4     
  Partials       35       35              
Flag Coverage Δ
unittests 66.21% <33.33%> (+0.14%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/service.go 78.57% <0.00%> (+0.92%) ⬆️
server/utils.go 72.72% <ø> (-2.95%) ⬇️
server/mock_relay.go 86.92% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@metachris metachris changed the title json decoding: allow unknown fields json decoding: allow unknown fields (in preparation for Capella and EIP-4844) Nov 4, 2022
@metachris metachris force-pushed the remove-strict-json-encoding branch from 222f9e3 to e9775da Compare November 4, 2022 10:10
@metachris metachris closed this Feb 21, 2023
@metachris metachris deleted the remove-strict-json-encoding branch March 16, 2023 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants