-
-
Notifications
You must be signed in to change notification settings - Fork 290
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
feat: reject builder blocks if engine indicates censorship #6258
Conversation
c5b747b
to
761f37f
Compare
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.
Looks like type checks are failing because produceBlockV3 tests are not yet updated
lodestar/packages/beacon-node/test/unit/api/impl/validator/produceBlockV3.test.ts
Line 88 in 0c13db6
chainStub.produceBlock.mockResolvedValue({ |
86c60b8
to
a289561
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## unstable #6258 +/- ##
=========================================
Coverage 80.38% 80.38%
=========================================
Files 202 202
Lines 19622 19622
Branches 1176 1176
=========================================
Hits 15773 15773
Misses 3821 3821
Partials 28 28 |
packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts
Outdated
Show resolved
Hide resolved
|
||
[routes.validator.BuilderSelection.BuilderOnly, 0, 2, 0, false, "builder"], | ||
[routes.validator.BuilderSelection.ExecutionOnly, 2, 0, 1, false, "engine"], | ||
[routes.validator.BuilderSelection.BuilderOnly, 1, 1, 0, true, "builder"], |
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.
In this case we produce a builder block even though the execution client asked us to override builder, are we fine with this behavior, maybe we should at least log a warning that builders are suspected to censor transactions?
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 will be changed in followup PR where builderselection will be removed on the beacon side (didn't want to mix it in this PR), so just ignore for now, followup on builderselection coming shortly
packages/beacon-node/src/chain/produceBlock/produceBlockBody.ts
Outdated
Show resolved
Hide resolved
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.14.0 🎉 |
…#6258) * feat: reject builder blocks if engine indicates censorship * apply feeback * fix tests * small refac and test fixes and selection test extenstion for override combinations * make typing tighter for blindedblock * apply feedbac * Fix typo --------- Co-authored-by: Nico Flaig <nflaig@protonmail.com>
a mechanism was added where local engine could indicate censorship happening by builders via
shouldOverrideBuilder
which should make beacon produce engine blocks only irrespective of the builder boost (or to be deprecated builder selection) indicated by the validator.
This PR handles and implements the same.