Improve error types for envelopes#9414
Open
pawanjay176 wants to merge 5 commits into
Open
Conversation
pawanjay176
commented
Jun 4, 2026
| }, | ||
| /// Some Beacon Chain Error | ||
| BeaconChainError(Arc<BeaconChainError>), | ||
| BeaconChainError(Box<BeaconChainError>), |
Member
Author
There was a problem hiding this comment.
Did this Arc->Box just to stay consistent with BlockError::BeaconChainError.
pawanjay176
commented
Jun 4, 2026
| /// It's unclear if this block is valid, but it cannot be processed without already knowing | ||
| /// its parent. | ||
| ParentUnknown { parent_root: Hash256 }, | ||
| ParentUnknown { |
Member
Author
There was a problem hiding this comment.
Not sure why formatting for these changed, but the code is the same
pawanjay176
commented
Jun 4, 2026
| /// In the future, we could make the import error types more generic and then | ||
| /// this function could return an `EnvelopeError` as well. | ||
| #[instrument(skip_all, fields(block_root = ?block_root, envelope_source = %envelope_source))] | ||
| pub async fn process_execution_payload_envelope( |
Member
Author
There was a problem hiding this comment.
This is the top level function where we need to return a BlockError to stay consistent across the other block + data import function signatures.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
N/A
Proposed Changes
Currently, we have
EnvelopeErrorhaving aImportErrorwrapping aBlockError. I feel this is extremely unintuitive because most of the envelope processing functions can simply return anEnvelopeErrorthat makes sense in the function's context. It revealed further ugliness when implementing range sync in #9362This PR does 2 main things:
ImportError(BlockError)variantEnvelopeError(EnvelopeError)variant to aBlockError.I feel this is more natural as there can be envelope errors when we try importing a Block but envelope errors can be contained to just envelope related errors.
The main blocker to doing this was
PayloadVerificationHandlereturning aBlockError. It uses a very small subset ofBlockErrorwhich I extracted to its own error type which can be converted into both a BlockError and EnvelopeError.This allows us to keep most of the pure envelope processing functions to just return EnvelopeErrors while we convert it to a
BlockErroronly in import paths where we need to return a consolidatedBlockError.