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

EIP-7549: Attestation packing #14238

Merged
merged 7 commits into from
Jul 19, 2024
Merged

EIP-7549: Attestation packing #14238

merged 7 commits into from
Jul 19, 2024

Conversation

rkapka
Copy link
Contributor

@rkapka rkapka commented Jul 18, 2024

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Implementation of compute_on_chain_aggregate from https://github.com/ethereum/consensus-specs/blob/dev/specs/electra/validator.md#attestations. This is a naive version that takes the top aggregate for each attestation_data+committee combination.

@rkapka rkapka requested a review from a team as a code owner July 18, 2024 15:46
@rkapka rkapka requested review from nalepae, potuz and james-prysm July 18, 2024 15:46
@rkapka rkapka added Electra electra hardfork Ready For Review A pull request ready for code review labels Jul 18, 2024
}

var attsForInclusion proposerAtts
if postElectra {
Copy link
Contributor

Choose a reason for hiding this comment

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

if we have the latest state why not do the post electra check from the version there instead of the epoch?

Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed postElectra := slots.ToEpoch(blkSlot) >= params.BeaconConfig().ElectraForkEpoch

james-prysm
james-prysm previously approved these changes Jul 18, 2024
Copy link
Contributor

@james-prysm james-prysm left a comment

Choose a reason for hiding this comment

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

please add a test for edge cases for post electra checks in a subsequent pr

@rkapka rkapka added this pull request to the merge queue Jul 19, 2024
@rkapka rkapka removed this pull request from the merge queue due to a manual request Jul 19, 2024
@rkapka rkapka enabled auto-merge July 19, 2024 12:42
Copy link
Contributor

@james-prysm james-prysm left a comment

Choose a reason for hiding this comment

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

nice thanks for the comment

@rkapka rkapka added this pull request to the merge queue Jul 19, 2024
Merged via the queue into develop with commit 49055ac Jul 19, 2024
16 of 17 checks passed
@rkapka rkapka deleted the eip-7549-packing branch July 19, 2024 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Electra electra hardfork Ready For Review A pull request ready for code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants