Problems with attestations from incompatible forks

[ericsson49]

Attester and proposer roles assignment are determined by a shuffling of a set of active validators, which depends on a seed. The seed and the set of active validators, in their turn, are detemined by a chain of blocks (randao, slashings, deposits, exits, etc). An attestation data doesn't contain indices of a block proposer and attesters in an explicit form, they are (re)calculated based on a state and aggregation_bits.

That means it's important that a state that is used to restore attester and proposer indices contain the right seed and the right set of active validators. If it doesn't hold, then it can lead to problems (described later).

When an attestation is created or consumed/processed, the reference states (used to calculate proposer/attesters for the attestation's slot) will be different. However, that doesn't necessarily mean the seed and the set of active validators will differ too. As proposer and attester assignment for an epoch are determined at the beginning of the previous epoch, some beacon states can lead to the same assignments. Let's call such states compatible.

For example, when an attester creates an attestation, the reference state is the head state (with slot transitions applied up to the current slot). When an attestation is used in a fork choice, the reference state is the state at the beginning of the target epoch. When an attestation is to be included in a block or is processed as a part of a block, the reference state is the state of the block's parent with slot transitions applied. So, in general, the states can be incompatible between each other, i.e. having different seeds and/or sets of active validators. And thus leading to incorrect reconstruction of attester and proposer indices.

Such incompatibility can arise when there has been a fork and an attestation referencing one branch of the fork is inculded in a block from another branch of the fork. If the fork happend in an epoch before the previous epoch, then it's enough time that differences in block chains start to affect shufflings.

Potential problems:

• during process_attestation(state, attestation), if the state is incompatible with the attestation's target state, the aggregation_bits will translate to a wrong set of validator indices. It will most likely mean the attestation signature check will fail, since another set of public keys will be used to construct an aggregate key. Such attestation cannot be included in a block (or a block with such an attestation will be rejected by correct nodes). It's not a problem. However, a node can decide that the attestation signature is wrong (and, for example, punish the attestation's sender in some way). But the real problem is that the attestation validity is verified against the wrong state.
• another possibility is that the signature validation can succeed. It could be a quite rare situation, but two different shufflings can still have some intersecting subsets. E.g. a validator with index 1 can happen to be at the same position at both shufflings (while other positions being different). So, if aggregation_bits contain only this position, then the public keys will be the same with both states. That is a real problem, since the state incompatibility can sneak undetected.
• an additional problem, when the incompatibility is not prevented by the signature check, is that the beacon proposer index will be wrong most likely, since it's calculated based on the incompatible state (i.e. different shuffling).

So, in order to prevent such problems we suggest that only attestations from compatible states can be included in blocks. And the appropriate check should be implemented as a part of process_attestation method (and/or get_indexed_attestation method). Most likely such attestations cannot be included anyway, but when they can it leads to a covert problem (wrong proposer index in a pending attestation).

[protolambda]

if the state is incompatible with the attestation's target state, the aggregation_bits will translate to a wrong set of validator indices

A fork should not change the validity of attestations before the fork start, unless that's the intention of the fork. I don't think the states will be incompatible; ensuring the active validators are the same across a fork version is part of forking, similar to the seed look-ahead, it's part of knowing what to validate before it's too late to do anything. If a fork would change the active validators or the shuffling etc., it should do so only after a look-ahead period has passed since the new rules are applied.

another possibility is that the signature validation can succeed

Are we talking about a fork induced difference here? If so, the sig validation cannot succeed, since you sign with the fork version in the signature domain (== domain type concatenated with fork version)

[ericsson49]

The issue is not about manual forks, but about the forks in the sense of the fork-choice spec, i.e. situations where a block has two or more immediate descendants.

[protolambda]

Ah I see. For slashings attestation checking is not as strictly coupled; validator indices are embedded in the slashing itself, so there is no dependency on the exact state of the chain. For normal attestations, they would only be included if their scope is part of the chain (and at that point you have consistency and are just dealing with fork versions). So I don't see what edge case you need inclusion of attestations from different forks for.

[ericsson49]

One problem is that in some cases such an attestation can still pass signature check and be included in a block.
So, the suggestion is to explicitly forbid inclusion of such attestations in a block and to add an explicit check preventing that.

The second thing is to make implementers aware of such cases so that they be more careful when validating attestations.
For example, is_valid_indexed_attestation is called during fork choice's on_attestation procedure. Since BLS verification is an expensive, it's likely that the result will be cached. E.g. in some implementations, the signature check can be skipped before including the verification in a block. Or signature validation of attestations arrived as a part of a block may be skipped, if an attestation has been already received via wire and it's signature has been verified.
As I wrote before, in on_attestation and process_attestation methods such signature checks can be actually different, e.g. based on incompatible states (on_attestation verifies against target.root while process_attestation - against the head).
Even if the signature check is not cached, an implementation can decided that the signature is wrong, and blame the attestation sender (e.g. put it in a black list).

[JustinDrake]

@protolambda @ericsson49 What is the outcome of this discussion? Do we want to make a change in phase 0?

[protolambda]

As I wrote before, in on_attestation and process_attestation methods such signature checks can be actually different, e.g. based on incompatible states (on_attestation verifies against target.root while process_attestation - against the head).

I think this is the most confusing part. I don't see others having problems with the way attestations are currently processed, and was assuming this issue was already solved offline. Anyway, ping @ericsson49 if there is a concrete implementation problem, otherwise I think we can close this issue.

[ericsson49]

The idea has been to forbid inclusion of attestations from such incompatible forks on the spec level, so that implementations handle it uniformly.
It can be handled on an implementation level, though implementers can do that in different ways.