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

Investigate UnknownTargetRoot slasher errors #5006

Merged
merged 1 commit into from
Jun 4, 2024

Conversation

armaganyildirak
Copy link
Member

@armaganyildirak armaganyildirak commented Dec 13, 2023

Issue Addressed

#4972

Proposed Changes

This PR addresses the issue where attestations failing with UnknownHeadBlock are incorrectly processed through the slasher, leading to subsequent failures in signature verification (SignatureNotChecked) The subsequent failure occurs when attempting to load the state for the target block. This is expected when the target is identical to the head, leading to an unknown state. The proposed solution involves requeuing attestations without involving the slasher when encountering UnknownHeadBlock.

Some UnknownTargetRoot errors could still occur particularly during startup. It happens when the beacon_block_root slot is later than target_root slot. However they are harmless because the requeueing will usually take care of processing them later.

Additional Info

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. v5.2.0 Q2 2024 and removed work-in-progress PR is a work-in-progress labels Jun 4, 2024
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

LGTM!

@michaelsproul
Copy link
Member

@Mergifyio queue

Copy link

mergify bot commented Jun 4, 2024

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

mergify bot added a commit that referenced this pull request Jun 4, 2024
@jimmygchen
Copy link
Member

@mergify requeue

Copy link

mergify bot commented Jun 4, 2024

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

Copy link

mergify bot commented Jun 4, 2024

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

mergify bot added a commit that referenced this pull request Jun 4, 2024
mergify bot added a commit that referenced this pull request Jun 4, 2024
michaelsproul added a commit that referenced this pull request Jun 4, 2024
Squashed commit of the following:

commit 7adfecc
Author: armaganyildirak <armaganyildirak@gmail.com>
Date:   Wed Dec 13 15:22:02 2023 -0500

    Fix the UnknownTargetRoot
@michaelsproul michaelsproul mentioned this pull request Jun 4, 2024
@jimmygchen
Copy link
Member

@mergify requeue

Copy link

mergify bot commented Jun 4, 2024

requeue

✅ This pull request will be re-embarked automatically

The followup queue command will be automatically executed to re-embark the pull request

Copy link

mergify bot commented Jun 4, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 8a247eb

mergify bot added a commit that referenced this pull request Jun 4, 2024
@mergify mergify bot merged commit 8a247eb into sigp:unstable Jun 4, 2024
28 of 29 checks passed
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. slasher UX-and-logs v5.2.0 Q2 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants