Skip to content

Strict match of errors in backfill sync#6520

Merged
mergify[bot] merged 2 commits intosigp:unstablefrom
dapplion:strict-match-backfill-sync
Nov 5, 2024
Merged

Strict match of errors in backfill sync#6520
mergify[bot] merged 2 commits intosigp:unstablefrom
dapplion:strict-match-backfill-sync

Conversation

@dapplion
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

@mergify queue

@mergify
Copy link
Copy Markdown

mergify Bot commented Nov 5, 2024

queue

✅ The pull request has been merged automatically

Details

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
@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