-
-
Notifications
You must be signed in to change notification settings - Fork 300
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: fix the publish blinded block api parsing for optional header verison #6966
Conversation
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.
I missed that because didn't run Lighthouse VC with builder enabled, but I am surprised they are using JSON and v1 api, a bit strange.
Need to run the Lodestar interop config I used for testing the ssz branch with builder enabled, last time this didn't work as builders didn't produce blocks so it always fallback to local, but will give it another try.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## unstable #6966 +/- ##
============================================
- Coverage 62.49% 62.49% -0.01%
============================================
Files 576 576
Lines 61171 61189 +18
Branches 2133 2132 -1
============================================
+ Hits 38230 38239 +9
- Misses 22902 22911 +9
Partials 39 39 |
fork = toForkName(versionHeader); | ||
} else { | ||
// Determine fork from slot in JSON payload | ||
fork = config.getForkName((body as SignedBlindedBeaconBlock).message.slot); |
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 is why I mean it's a hack btw, we kinda just yolo it here and try to access body.message.slot
but this will result in "Cannot access property x of undefined" if data is not formatted correctyl, while if you get version from header we can properly apply validation before we try to access any property of body.
Performance Report✔️ no performance regression detected Full benchmark results
|
🎉 This PR is included in v1.20.2 🎉 |
🎉 This PR is included in v1.21.0 🎉 |
lighthouse default still uses produceblinded/publishblinded with json api with option version.
because of this a user failed a block publish.
this pr fixes the scenario and should be released as a patch