-
Notifications
You must be signed in to change notification settings - Fork 566
feat: Archiver does not sync blocks with invalid attestations #15896
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
Conversation
940acfd to
13b1036
Compare
2b10c89 to
c35f762
Compare
941beed to
4119d4c
Compare
97e4074 to
dd1e4ee
Compare
| } | ||
|
|
||
| const committeeSet = new Set(committee.map(member => member.toString())); | ||
| const requiredAttestationCount = Math.floor((committee.length * 2) / 3) + 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this 2 thirds configured on the contract somewhere? Should we read it rather than hard-code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hardcoded in the contract as well
| } | ||
|
|
||
| rollupStatus.lastBlockIsInvalid = false; | ||
| validBlocks.push(block); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was going to ask why we sync blocks after an invalid one. But then I saw the comment that we will fix that in a follow up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reason for syncing valid blocks after an invalid one is because we expect the valid block to be built following the last valid one. What remains to be fixed in a future PR is syncing an invalid block if it fits the criteria for it (a descendant block must be attested and proven).
Archiver now checks committee attestations and refuses to sync a block if it does not pass validation.
Note that this addresses scenarios where the proposer is malicious, but does not handle cases where the entire committee is and produces signatures for a block with an unattested parent. That'll be left for a future PR.
Builds on #15813