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

Disable light client when fork epoch is misaligned #7022

Open
wants to merge 8 commits into
base: release-v7.0.0
Choose a base branch
from

Conversation

eserilev
Copy link
Collaborator

@eserilev eserilev commented Feb 21, 2025

Closes #7002

Temporarily disables light client functionality when fork epoch is misaligned

Add metrics for Light client gossip verification failures. I've added two categories of errors Failures and Ignores so we can differentiate between "real" failures and janky data. I've also added a new error type TooLate so that we dont include stale data as part of our "Failures" metrics. Ideally it'd be nice to have this included in the 7.0 release

@eserilev eserilev added ready-for-review The code is ready for review light-client v7.0.0 New release c. Q1 2025 labels Feb 21, 2025
@eserilev eserilev changed the title Light client gossip failure metrics Disable Light client when fork epoch is misaligned Feb 21, 2025
@eserilev eserilev changed the title Disable Light client when fork epoch is misaligned Disable light client when fork epoch is misaligned Feb 21, 2025
@eserilev eserilev added work-in-progress PR is a work-in-progress and removed ready-for-review The code is ready for review labels Feb 22, 2025
@eserilev eserilev added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels Feb 22, 2025
@eserilev eserilev requested a review from jxs as a code owner February 24, 2025 04:48
@michaelsproul
Copy link
Member

Oops, looks like you merged unstable into this branch. You should rebase on release-v7.0.0

@michaelsproul
Copy link
Member

A git rebase -i origin/release-v7.0.0 should fix it, justing deleting all the commits that aren't yours

@eserilev eserilev force-pushed the light-client-gossip-failure-metrics branch from 4fa7acc to cedc381 Compare February 24, 2025 05:10
@eserilev
Copy link
Collaborator Author

lol thanks, apparently my subconscious needs to merge unstable

@michaelsproul
Copy link
Member

I got scared by the size of the diff 😱 . Now it's fixed I can review 😌

LightClientFinalityUpdateError::Disabled
| LightClientFinalityUpdateError::TooLate => debug!(
self.log,
":ight client finality update ignored";
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
":ight client finality update ignored";
"Light client finality update ignored";

&self,
current_slot: Slot,
) -> bool {
let is_fork_boundary_inside_sync_committee_period = |fork_epoch: Epoch| {
Copy link
Member

@jimmygchen jimmygchen Feb 24, 2025

Choose a reason for hiding this comment

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

There's an existing check below, but it only log a warning if it's misaligned. Perhaps we can remove that one given that it would be a bit more obvious now if the fork schedule is misaligned? (light client disabled)

fn validator_fork_epochs(spec: &ChainSpec) -> Result<(), Vec<(ForkName, Epoch)>> {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
light-client ready-for-review The code is ready for review v7.0.0 New release c. Q1 2025
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not attempt light client computations when fork epoch is misaligned
3 participants