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

Opti sync: how to apply latestValidHash when payload is INVALID #2954

Merged
merged 4 commits into from
Aug 3, 2022

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Jul 27, 2022

Clarifies on how to handle latestValidHash on CL side. This PR doesn't introduce any breaking changes but instead makes current CL behaviour into the spec and leaves recommendation on what to do when latestValidHash can't be found in a block tree

UPD
It's written in a way that uses latestValidHash to only invalidate blocks. No valid semantics of latestValidHash is leveraged by this PR

cc @potuz @ajsutton @paulhauner

@mkalinin mkalinin requested review from djrtwo and hwwhww July 27, 2022 10:14
sync/optimistic.md Outdated Show resolved Hide resolved
@hwwhww hwwhww mentioned this pull request Jul 27, 2022
@ajsutton
Copy link
Contributor

LGTM. This matches how Teku is interpreting things now.

@hwwhww
Copy link
Contributor

hwwhww commented Jul 29, 2022

It would be great to get approvals/confirmations from Lighthouse (@paulhauner @michaelsproul), Lodestar (@dapplion @tuyennhv), and Nimbus (@etan-status @arnetheduck @tersec) too. 🙏

Copy link
Contributor

@djrtwo djrtwo left a comment

Choose a reason for hiding this comment

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

minor copy nits

sync/optimistic.md Outdated Show resolved Hide resolved
sync/optimistic.md Outdated Show resolved Hide resolved
sync/optimistic.md Outdated Show resolved Hide resolved
sync/optimistic.md Outdated Show resolved Hide resolved
sync/optimistic.md Outdated Show resolved Hide resolved
@mkalinin
Copy link
Collaborator Author

mkalinin commented Aug 2, 2022

Last call for approval from @paulhauner and @tersec, merging in 24 hours

@mkalinin mkalinin merged commit 3985276 into dev Aug 3, 2022
@hwwhww hwwhww deleted the mkalinin-patch-2 branch February 21, 2023 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants