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

Do not fill in blocks that are not in the finalized branch #10776

Merged
merged 4 commits into from
May 30, 2022

Conversation

potuz
Copy link
Contributor

@potuz potuz commented May 28, 2022

The function fillInForkChoiceMissingBlocks(ctx, blk, finalizedChkpt, justifiedChkpt) obtains from DB all blocks starting from the parent of blk and goes up the parent chain. It inserts all blocks with slots higher than the last finalized checkpoint plus the parent of the last such slot.

This function does not check that this last parent is the finalized block, if it isn't then the function inserts a contending chain to the finalized block and it should be declared invalid.

In addition, this function has an off-by-one in which it passes also the finalized block to forkchoice, all tests for this functions were wrongfully counting nodes adding the extra finalized checkpoint that should not have been saved.

@potuz potuz requested a review from a team as a code owner May 28, 2022 18:32
@potuz potuz requested review from rkapka, nisdas and symbolpunk May 28, 2022 18:32
@potuz potuz added Bug Something isn't working Ready For Review Blocked Blocked by research or external factors labels May 28, 2022
@potuz potuz removed the Blocked Blocked by research or external factors label May 29, 2022
@prylabs-bulldozer prylabs-bulldozer bot merged commit 3ff285d into develop May 30, 2022
@delete-merged-branch delete-merged-branch bot deleted the dont_fill_contending_branches branch May 30, 2022 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants