[Merged by Bors] - Indicate that invalid blocks are optimistic #3383
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paulhauner
added
blocked
work-in-progress
paulhauner
force-pushed
the
is-strictly-optimistic
branch
from
ebca20f to
dd2a9b5
Compare
paulhauner
added
ready-for-review
|
I'll block this on #3372 so I can resolve the conflicts here. |
paulhauner
added
blocked
and removed
ready-for-review
Make clippy pass Fix test compile errors
paulhauner
force-pushed
the
is-strictly-optimistic
branch
from
dd2a9b5 to
4508aae
Compare
|
I just fixed a couple of simple conflicts introduced via #3372. bors r+ |
bors bot
pushed a commit
that referenced
this pull request
Jul 30, 2022
## Issue Addressed NA ## Proposed Changes This PR will make Lighthouse return blocks with invalid payloads via the API with `execution_optimistic = true`. This seems a bit awkward, however I think it's better than returning a 404 or some other error. Let's consider the case where the only possible head is invalid (#3370 deals with this). In such a scenario all of the duties endpoints will start failing because the head is invalid. I think it would be better if the duties endpoints continue to work, because it's likely that even though the head is invalid the duties are still based upon valid blocks and we want the VC to have them cached. There's no risk to the VC here because we won't actually produce an attestation pointing to an invalid head. Ultimately, I don't think it's particularly important for us to distinguish between optimistic and invalid blocks on the API. Neither should be trusted and the only *real* reason that we track this is so we can try and fork around the invalid blocks. ## Additional Info - ~~Blocked on #3370~~
bors
bot
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Addressed
NA
Proposed Changes
This PR will make Lighthouse return blocks with invalid payloads via the API with
execution_optimistic = true. This seems a bit awkward, however I think it's better than returning a 404 or some other error.Let's consider the case where the only possible head is invalid (#3370 deals with this). In such a scenario all of the duties endpoints will start failing because the head is invalid. I think it would be better if the duties endpoints continue to work, because it's likely that even though the head is invalid the duties are still based upon valid blocks and we want the VC to have them cached. There's no risk to the VC here because we won't actually produce an attestation pointing to an invalid head.
Ultimately, I don't think it's particularly important for us to distinguish between optimistic and invalid blocks on the API. Neither should be trusted and the only real reason that we track this is so we can try and fork around the invalid blocks.
Additional Info
Blocked on [Merged by Bors] - Don't return errors when fork choice fails #3370