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

Remove check for duplicates in pending attestation queue #13814

Merged
merged 2 commits into from
Mar 27, 2024

Conversation

potuz
Copy link
Contributor

@potuz potuz commented Mar 27, 2024

The current queue will only save 1 unaggregated attestation for a pending block because we wrap the object into a SignedAggregatedAttestationAndProof with a zeroed aggregator. Since the unique callers of this queue already make sure that the objects are not duplicated in the gossip pipeline, we can safely remove the check

This is a bug present since genesis.

@potuz potuz added the Bug Something isn't working label Mar 27, 2024
@potuz potuz requested a review from a team as a code owner March 27, 2024 15:01
@potuz potuz requested review from saolyn, terencechain and rkapka March 27, 2024 15:01
The current queue will only save 1 unaggregated attestation for a pending block because we wrap the object into a SignedAggregatedAttestationAndProof with a zeroed aggregator.
@potuz potuz force-pushed the remove_duplicate_check_pending_att branch from 1c19a01 to 662d937 Compare March 27, 2024 15:53
Comment on lines +183 to +184
// Skip if the attestation from the same aggregator already exists in
// the pending queue.
Copy link
Member

Choose a reason for hiding this comment

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

We should fix this comment right?

Copy link
Member

Choose a reason for hiding this comment

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

Actually I think it's fine. The idea didn't change

terencechain
terencechain previously approved these changes Mar 27, 2024
@potuz potuz added this pull request to the merge queue Mar 27, 2024
Merged via the queue into develop with commit 12482ee Mar 27, 2024
16 of 17 checks passed
@potuz potuz deleted the remove_duplicate_check_pending_att branch March 27, 2024 17:01
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