-
Notifications
You must be signed in to change notification settings - Fork 303
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
Track produced block when builder doesn't reveal the payload #8855
Conversation
if (ex != null) { | ||
// in case of relay API timeouts when unblinding, we still want to assume the block | ||
// as produced, since the relay could have published it | ||
if (ExceptionUtil.hasCause(ex, TimeoutException.class)) { | ||
performanceTracker.saveProducedBlock(blockContainer.getSignedBlock()); | ||
} | ||
} else { | ||
performanceTracker.saveProducedBlock(block); |
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.
do we really need to discriminate the exception here? wandering if we can save the block as produced
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.
It doesn't hurt since we filter in the tracker compared to the chain we have anyways and exceptions shouldn't really happen
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.
but you could have a bunch of errors on API side, not just a timeout, and you never know if on the other side it got the info to publish.
Moreover, I think that we know we attempted to publish a produced block, so it make sense to me to blindly track it at the very beginning of the flow...
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.
Committed a change to always save the block as produced
2ea8fe9
to
7c892af
Compare
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.
Here we change the definition of "produced" from "produced, signed and about to publish" to "block produced but not yet signed nor published".
To me makes sense.
PR Description
Move the tracking of the produced block to the ValidatorApiHandler where we produce the unsigned block. I find this approach easier and more consistent than passing the
PerformanceTracker
all the way down toExecutionBuilderModule
Fixed Issue(s)
fixes #8841
Documentation
doc-change-required
label to this PR if updates are required.Changelog