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

Add back BLS changes from orphaned blocks #11822

Merged
merged 8 commits into from
Jan 1, 2023

Conversation

potuz
Copy link
Contributor

@potuz potuz commented Dec 23, 2022

In case of a reorg, add back the BLS changes in the orphaned blocks.

@potuz potuz requested a review from a team as a code owner December 23, 2022 13:09
beacon-chain/blockchain/head.go Outdated Show resolved Hide resolved
// It also filters out the attestations that is one epoch older as a defense so invalid attestations don't flow into the attestation pool.
func (s *Service) saveOrphanedAtts(ctx context.Context, orphanedRoot [32]byte, newHeadRoot [32]byte) error {
func (s *Service) saveOrphanedOperations(ctx context.Context, orphanedRoot [32]byte, newHeadRoot [32]byte) error {
Copy link
Member

Choose a reason for hiding this comment

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

Tbf this doesn't save all the operations, we should move slashing and exit to here in subsequent PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, was working on it when vacations started :)

@prylabs-bulldozer prylabs-bulldozer bot merged commit ff82d9e into develop Jan 1, 2023
@delete-merged-branch delete-merged-branch bot deleted the orphaned_bls_changes branch January 1, 2023 18:22
return errors.Wrap(err, "could not get BLSToExecutionChanges")
}
for _, c := range changes {
s.cfg.BLSToExecPool.InsertBLSToExecChange(c)
Copy link
Member

Choose a reason for hiding this comment

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

What happens if the current branch already has these bls changes included ? Do we rely on our block packing to filter them out ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Insertion is a noop in this case

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.

3 participants