eip7549: Ensure non-zero bits for each committee bitfield comprising an aggregate #4002
Conversation
|
Do we need to add another condition to here? consensus-specs/specs/electra/p2p-interface.md Lines 51 to 53 in 1b408e9 |
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. |
Then the bit corresponding to this committee should be |
Note that a network aggregate only allows a single committee, so the following Phase0 check is enough: consensus-specs/specs/phase0/p2p-interface.md Lines 371 to 372 in 49b6840 |
| #### Modified `get_indexed_attestation` | ||
|
|
||
| *Note*: The function is modified to use the new `get_attesting_indices`. | ||
|
|
||
| ```python | ||
| def get_indexed_attestation(state: BeaconState, attestation: Attestation) -> IndexedAttestation: | ||
| """ | ||
| Return the indexed attestation corresponding to ``attestation``. | ||
| """ | ||
| # [Modified in Electra:EIP7549] | ||
| attesting_indices = get_attesting_indices(state, attestation) | ||
|
|
||
| return IndexedAttestation( | ||
| attesting_indices=sorted(attesting_indices), | ||
| data=attestation.data, | ||
| signature=attestation.signature, | ||
| ) | ||
| ``` | ||
|
|
There was a problem hiding this comment.
For pyspec, it is not needed since get_attesting_indices would be swapped automatically.
|
|
||
| # Participation flag indices | ||
| participation_flag_indices = get_attestation_participation_flag_indices(state, data, state.slot - data.slot) | ||
|
|
||
| # Verify signature | ||
| # Verify signature [Modified in Electra:EIP7549] |
| @@ -1286,6 +1314,7 @@ def process_attestation(state: BeaconState, attestation: Attestation) -> None: | |||
| epoch_participation = state.previous_epoch_participation | |||
|
|
|||
| proposer_reward_numerator = 0 | |||
| # [Modified in Electra:EIP7549] | |||
There was a problem hiding this comment.
imo it's not required because it didn't change "literally" in the specs. Devs should address it when they see the get_attesting_indices change.
If we apply this comment, it's weird that there were many other lines in the previous forks that we didn't highlight like this.
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.