ensure epochOf(attestation.data.slot) == attestation.target.epoch

[ericsson49]

Recently an explicit slot field was added to AttestationData.
According to validator spec attestation.data.target.epoch should be the same as epoch of attestation.data.slot (because they are calculated based on the slot the validator assigned on with Attester responsibility).

However, neither on_attestation nor process_attestation ensures that.
Which makes possible for a validator to create an attestation with data.target.epoch different from the epoch of data.slot. E.g. one can use previous or next epoch as a target (relative to data.slot).

We propose to eliminate the problem by enforcing the constraint by:

• either adding assert compute_epoch_at_slot(attestation.data.slot) == attestation.data.target to both on_attestation and process_attestation methods
• or removing AttestationData.target.epoch field (i.e. replace AttestationData.target with AttestationData.target_root). The target Checkpoint can be reconstructed based on the target_root and compute_epoch_at_slot(attestation.data.slot). AttestationData will occupy a bit less space in this case.

It's not clear if there is any benefit of supplying attestations with inconsistent slot and target.epoch fields.

[djrtwo]

Nice catch. I'm investigating this and will propose a fix for this coming release

[djrtwo]

This could lead to a validator, or (some subset of) committee of validators, casting a vote for a previous (or next) epoch. Although they still couldn't vote twice in an epoch without committing a slashable offense, this could lead to strange issues in accounting at the epoch boundary. It is even more worrisome in phase 2 where this issue could lead to potential manipulation of crosslinks.

We want attestations to have explicitly the Checkpoint they are voting on, so I will add a conditional statement to the two functions to ensure that the justified checkpoint and epoch of the assigned slot match

[djrtwo]

closed via #1509