Skip to content

Strict match of errors in backfill sync #6520

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

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

dapplion
Copy link
Collaborator

@dapplion dapplion commented Oct 21, 2024

Issue Addressed

Same spirit as

Handling errors in sync with a fallback match is not great. In the case of backfill sync it's very easy to fix! =D

(tangent) Noting a chat with @pawanjay176 in #6321 BeaconChainError actually includes errors that are not internal. In this case it was some of the variants of HistoricalBlockError.. I'll review BeaconChainError to double check there's no other hidden scorable variants.

Proposed Changes

Turns out that its processing returns almost only variants of HistoricalBlockError, so I changed import_historical_block_batch to return HistoricalBlockError instead of BeaconChainError.

All logic is the same, just made the match on network beacon processor cleaner a bit cleaner.

@dapplion dapplion added ready-for-review The code is ready for review syncing labels Oct 21, 2024
Copy link
Member

@pawanjay176 pawanjay176 left a comment

Choose a reason for hiding this comment

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

Neat

@pawanjay176 pawanjay176 added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Oct 31, 2024
@jimmygchen
Copy link
Member

@mergify queue

Copy link

mergify bot commented Nov 5, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 3838897

mergify bot added a commit that referenced this pull request Nov 5, 2024
@mergify mergify bot merged commit 3838897 into sigp:unstable Nov 5, 2024
29 checks passed
@dapplion dapplion deleted the strict-match-backfill-sync branch November 8, 2024 03:53
chong-he pushed a commit to chong-he/lighthouse that referenced this pull request Nov 26, 2024
* Strict match of errors in backfill sync

* Fix tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. syncing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants