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

eip7549: Ensure non-zero bits for each committee bitfield comprising an aggregate #4002

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

mkalinin
Copy link
Collaborator

@mkalinin mkalinin commented Nov 4, 2024

Currently, the spec allows an on chain aggregate to have only a single non-zero bit regardless of the number of committees. This allows to create an on chain aggregate with 63 committees worth of zeroes which successfully passes all validations (~4kb of uncompressed and wasteful data per aggregate).

This PR adds a validation that requires at least a single non-zero bit for each attesting committee bitfield comprising the aggregate.

Copy link
Member

@jtraglia jtraglia left a comment

Choose a reason for hiding this comment

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

Good idea, I agree we should check for this. LGTM, with one little nit.

specs/electra/beacon-chain.md Outdated Show resolved Hide resolved
@jtraglia
Copy link
Member

jtraglia commented Nov 4, 2024

Do we need to add another condition to here?

The following validations are added:
* [REJECT] `len(committee_indices) == 1`, where `committee_indices = get_committee_indices(aggregate)`.
* [REJECT] `aggregate.data.index == 0`

@arnetheduck
Copy link
Contributor

This allows to create an on chain aggregate with 63 committees worth of zeroes which successfully passes all validations (~4kb of uncompressed and wasteful data per aggregate).

what if a whole committee is not voting? ie there are no attestations from that commiteee?

@ensi321
Copy link
Contributor

ensi321 commented Nov 5, 2024

This allows to create an on chain aggregate with 63 committees worth of zeroes which successfully passes all validations (~4kb of uncompressed and wasteful data per aggregate).

what if a whole committee is not voting? ie there are no attestations from that commiteee?

The validator who does aggregation should include its own vote at the very least. So there should be at least one bit in aggregation bits.

edit: I notice https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/validator.md#construct-aggregate says aggregator should collect all the votes (including its own) that come solely from gossip. But I believe aggregator should include its own vote even if it doesn't receive its own vote through gossip.

@mkalinin
Copy link
Collaborator Author

mkalinin commented Nov 5, 2024

This allows to create an on chain aggregate with 63 committees worth of zeroes which successfully passes all validations (~4kb of uncompressed and wasteful data per aggregate).

what if a whole committee is not voting? ie there are no attestations from that commiteee?

Then the bit corresponding to this committee should be 0 in the committee_bits. The case this PR prevents is when a committee bit is 1 while aggregation_bits corresponding to this committee are all zeroes.

@mkalinin
Copy link
Collaborator Author

mkalinin commented Nov 5, 2024

Do we need to add another condition to here?

The following validations are added:
* [REJECT] `len(committee_indices) == 1`, where `committee_indices = get_committee_indices(aggregate)`.
* [REJECT] `aggregate.data.index == 0`

Note that a network aggregate only allows a single committee, so the following Phase0 check is enough:

- _[REJECT]_ The aggregate attestation has participants --
that is, `len(get_attesting_indices(state, aggregate)) >= 1`.

@jtraglia jtraglia mentioned this pull request Nov 5, 2024
17 tasks
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.

4 participants