Skip to content

Next sync committee branch bug #7443

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 13, 2025
Merged

Next sync committee branch bug #7443

merged 2 commits into from
May 13, 2025

Conversation

eserilev
Copy link
Member

@eserilev eserilev commented May 12, 2025 •
edited
Loading

Issue Addressed

#7441

Proposed Changes

Make sure we're correctly caching light client data

Additional Info

Previously we were setting current_sync_committee_branch to next_sync_committee_branch (and vice versa). I've removed the let (_,_,_) to make things clearer

@eserilev eserilev requested a review from jxs as a code owner May 12, 2025 16:32
@eserilev eserilev added light-client bug Something isn't working ready-for-review The code is ready for review labels May 12, 2025
@eserilev eserilev changed the base branch from stable to unstable May 12, 2025 16:32
@michaelsproul
Copy link
Member

How come we didn't trigger this while verifying light client updates from the network?

michaelsproul
michaelsproul approved these changes May 12, 2025
View reviewed changes
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.

(Fix looks good btw)

@eserilev
Copy link
Member Author

eserilev commented May 12, 2025 •
edited
Loading

How come we didn't trigger this while verifying light client updates from the network?

Only the LightClientFinalityUpdate and LightClientOptimisticUpdate objects get verified over the network

The current_ sync_committee_branch and next_sync_committee_branch are only included in LightClientBootstrap and LightClientUpdate objects

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels May 12, 2025
@michaelsproul
Copy link
Member

Oh right, makes sense. Thanks!

@mergify mergify bot merged commit 807848b into sigp:unstable May 13, 2025
31 checks passed
@eserilev eserilev mentioned this pull request May 13, 2025
Closed
@ParthDesai ParthDesai mentioned this pull request Jun 3, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelsproul michaelsproul michaelsproul approved these changes

@jxs jxs Awaiting requested review from jxs

Assignees
No one assigned
Labels
bug Something isn't working light-client ready-for-merge This PR is ready to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eserilev @michaelsproul