Attestation validation differences between fork choice and process_attestation

[ericsson49]

An attestation contains three votes: LMD GHOST vote, FFG vote and Crosslink vote.
The beacon chain specs specify how to validate an attestation and its votes.
However, the validation is split in several parts: some checks defined in fork_choice/on_attestation and others - in process_attestation. They both invoke is_valid_indexed_attestation.

The on_attestation deals mostly with LMD GHOST vote and part of FFG vote (target field). So, if there arrives an attestation with an invalid crosslink vote or an invalid source fields, then it is still accepted by the fork choice spec and affects the LMD GHOST fork choice.
However, the attestation should not be included in newly proposed blocks, since it won't pass process_attestation checks (see validator spec.

While there may be reasons to split validation in two parts, this can lead to inconsistencies in fork choice. Consider a case, when there are two head candidates, both having the same amount of votes and tie break chooses the first one. If a partially correct attestation added, which votes for the second block, then the second block becomes the head, based on off-chain plus on-chain attestation. But since this last vote cannot be incorporated into the chain, from on-chain attestations perspective, the first candidate is the head. I do not fully understand consequences of the discrepancy, but it seems awkward at least.

Another example. If such a partially valid attestations is propagated only to part of the network nodes, it cannot reach other (honest) validators via proposed blocks, because of process_attestation assertion failures. It can result in different views on the state of the blockchain, which can be only corrected by propagating such invalid attestations to other nodes.
So this looks like a potential unnecessary source of forks, which can be exploited by an attacker.

[protolambda]

There are multiple things to consider here:

There is a related thing with "should we verify twice"; I'm heavily opposed to doing a single-pass verification of the attestation for several reasons:

• relying on your pool of operations is bad. Bitcoin Core had a bug where consensus was partially reliant on the mempool being validated (iirc).
• the fork-choice is different from the state transition, since voting is not the only thing happening with attestation, more on this below. Fork-choice can just verify the format, and the weight, and it is valid enough.

Fork-choice accepting partially invalid attestations is not that bad. If an attestation has weight and is signed, it still helps determine the head. We shouldn't rely too much on full state details being available to process it in a large graph of forks.
And if the attestation is really bad, it can be slashed. It having an invalid crosslink may hurt it for the attesters economical purposes as it won't get included on-chain, but it won't hurt the chain itself.

[mkalinin]

Fork-choice accepting partially invalid attestations is not that bad.

We might want to have same inputs for the fork choice disregard the source of attestations. I.e. it should have no matter whether attestation was gossiped or came within one of the blocks. If fork choice doesn't run those checks then we may get different heads depending on attestation source.

Also, if you've checked signature, don't you already have a state to check crosslink and checkpoits?

[ericsson49]

There are two cases with partially valid attestations:

• block-wise invalid but fork-choice-wise valid attestation, i.e. which affects fork choice, but cannot be included in a block
• fork-choice-wise invalid but block-wise valid attestation, i.e. which can be included in a block, but shouldn't affect fork choice (of honest validators)

The second case doesn't look like a problem - the block is used as a transport for attestations, and invalid ones will be filtered out in on_attestation method. They could have come from wire as plain messages as well.

The first cases looks like a problem, since attestations affecting fork choice cannot be persisted, i.e. included in the block chain. And will be forgotten eventually. So, if an impact of such partially valid attestations is significant (i.e. leading to a situation where an alternate block becomes the head), then it looks like somewhat severe problem. Since the on-chain information cannot explain the choice made, in some cases.

[paulhauner]

It's worth nothing that another difference between fork choice vs. block inclusion is this line in validate_on_attestation:

 # Attestations must not be for blocks in the future. If not, the attestation should not be considered
assert store.blocks[attestation.data.beacon_block_root].slot <= attestation.data.slot

This condition is not checked for block inclusion or propagation on the gossip network.

[adiasg]

More differences:

• In the fork choice, but not in block processing:

assert target.epoch == compute_epoch_at_slot(attestation.data.slot)

• In block processing, but not the fork choice:

assert data.index < get_committee_count_at_slot(state, data.slot)