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

Don't create payload with same timestamp as terminal block #11129

Merged
merged 3 commits into from
Jul 31, 2022

Conversation

terencechain
Copy link
Member

Fixes #11069

This PR fixes an issue where prysm will create a transition block where block.execution_payload().timestamp == terminal_block.timestamp and such block would have failed forkchoiceUpdate call

@terencechain terencechain requested a review from a team as a code owner July 28, 2022 18:05
@terencechain terencechain self-assigned this Jul 28, 2022
Comment on lines +263 to +273

// If terminal block has time same timestamp or greater than transition time,
// then the node violates the invariant that a block's timestamp must be
// greater than its parent's timestamp. Execution layer will reject
// a fcu call with such payload attributes. It's best that we return `None` in this a case.
parentReachedTTD := parentTotalDifficulty.Cmp(terminalTotalDifficulty) >= 0
if !parentReachedTTD {
if blk.Time >= transitionTime {
return nil, false, nil
}

Copy link
Member Author

Choose a reason for hiding this comment

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

review

Copy link
Contributor

Choose a reason for hiding this comment

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

I am worried about this solution, the transition time being passed to this function is purely on the CL side (you're passing a slot time) while the condition we want to prevent is that the block timestamp is not that of the TTD. Why not simply comparing the timestamp of these two blocks here instead of comparing to transitionTime?

@terencechain terencechain force-pushed the invalid-terminal-timestamp branch from fed1058 to 80db733 Compare July 28, 2022 18:34
@prylabs-bulldozer prylabs-bulldozer bot merged commit 758d286 into develop Jul 31, 2022
@delete-merged-branch delete-merged-branch bot deleted the invalid-terminal-timestamp branch July 31, 2022 17:17
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.

Transition payload production might fail if the TTD block timestamp coincides with the slot time
2 participants