search

status-im / nimbus-eth2

Nim implementation of the Ethereum Beacon Chain
https://nimbus.guide
Other
543 stars 233 forks source link

stop tracking phase0 spec artifact of current/prev target epoch for attestation block packing #6508

Closed tersec closed 2 months ago

tersec commented 2 months ago

This was added to begin with in a phase 0 context because https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.5/specs/phase0/beacon-chain.md#beaconstate includes

class BeaconState(Container):
  ...
    # Attestations
    previous_epoch_attestations: List[PendingAttestation, MAX_ATTESTATIONS * SLOTS_PER_EPOCH]
    current_epoch_attestations: List[PendingAttestation, MAX_ATTESTATIONS * SLOTS_PER_EPOCH]

and therefore it was otherwise possible to overflow previous_epoch_attestations if one was not careful, to create blocks which would be invalid to apply.

However, Altair fixes this; https://github.com/ethereum/consensus-specs/blob/v1.5.0-alpha.5/specs/altair/beacon-chain.md#beaconstate instead contains:

    # Participation
    previous_epoch_participation: List[ParticipationFlags, VALIDATOR_REGISTRY_LIMIT]  # [Modified in Altair]
    current_epoch_participation: List[ParticipationFlags, VALIDATOR_REGISTRY_LIMIT]  # [Modified in Altair]

So there's no need to special-case previous-epoch attestations.

The tradeoff is that it can't 100% correctly pack phase 0 blocks anymore, but that's fine, it will never have to.

github-actions[bot] commented 2 months ago

Unit Test Results

         9 files  ±0    1 337 suites  ±0   31m 14s :stopwatch: +52s   5 064 tests ±0    4 716 :heavy_check_mark: ±0  348 :zzz: ±0  0 :x: ±0  20 991 runs  ±0  20 587 :heavy_check_mark: ±0  404 :zzz: ±0  0 :x: ±0 

Results for commit 8cba5f72. ± Comparison against base commit 44cc72c1.