Skip to content
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(engine): do not ignore provider errors #8519

Merged
merged 1 commit into from
May 31, 2024

Conversation

rkrasiuk
Copy link
Member

Description

We have a couple of provider lookups which ignore potentially critical errors. Fix that by always propagating provider errors.

@rkrasiuk rkrasiuk added C-debt A clean up/refactor of existing code A-consensus Related to the consensus engine labels May 30, 2024
@rkrasiuk
Copy link
Member Author

pre-emptive hive run in case i missed smth
https://github.com/paradigmxyz/reth/actions/runs/9308670438

) -> ProviderResult<Option<B256>> {
// Check if parent exists in side chain or in canonical chain.
if self.blockchain.find_block_by_hash(parent_hash, BlockSource::Any)?.is_some() {
return Ok(Some(parent_hash))
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

insert_err was present only during single invocation, the check was moved there

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, we effectively return these errors as the response now

@rkrasiuk rkrasiuk added this pull request to the merge queue May 31, 2024
Merged via the queue into main with commit 21613bb May 31, 2024
78 of 92 checks passed
@rkrasiuk rkrasiuk deleted the rkrasiuk/engine-do-not-ignore-errors branch May 31, 2024 07:33
mw2000 pushed a commit to mw2000/reth that referenced this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Related to the consensus engine C-debt A clean up/refactor of existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants