# [Fork choice] Attestation time consideration [TOC] ## The Problem The fork choice currently does not process attestations from more than 1 epoch ago. This presents a problem when processing old blocks in the unfinalized portion of the chain that may update justification/finalization, because the `store` will not be updated (even though `state_transition` executes just fine). See this (currently failing) test case for an example situation: https://github.com/ethereum/consensus-specs/compare/dev...fc-process-block-old-attns ## Recap: Fork choice rule helpers ### `on_attestation` > Run ``on_attestation`` upon receiving a new ``attestation`` from either **(i) a block** or **(ii) directly on the wire**. ```python def on_attestation(store: Store, attestation: Attestation) -> None: """ Run ``on_attestation`` upon receiving a new ``attestation`` from either within a block or directly on the wire. An ``attestation`` that is asserted as invalid may be valid at a later time, consider scheduling it for later processing in such case. """ validate_on_attestation(store, attestation) store_target_checkpoint_state(store, attestation.data.target) # Get state at the `target` to fully validate attestation target_state = store.checkpoint_states[attestation.data.target] indexed_attestation = get_indexed_attestation(target_state, attestation) assert is_valid_indexed_attestation(target_state, indexed_attestation) # Update latest messages for attesting indices update_latest_messages(store, indexed_attestation.attesting_indices, attestation) ``` It expects that the client would process `[on_attestation(store, attestation) for attestation in block.body.attestations]` right after processing `on_block(store, block)`. ### `validate_on_attestation` In [validate_on_attestation](https://github.com/ethereum/eth2.0-specs/blob/v1.1.5/specs/phase0/fork-choice.md#validate_on_attestation) helper: ```python def validate_on_attestation(store: Store, attestation: Attestation) -> None: target = attestation.data.target # Attestations must be from the current or previous epoch current_epoch = compute_epoch_at_slot(get_current_slot(store)) # Use GENESIS_EPOCH for previous when genesis to avoid underflow previous_epoch = current_epoch - 1 if current_epoch > GENESIS_EPOCH else GENESIS_EPOCH # If attestation target is from a future epoch, delay consideration until the epoch arrives assert target.epoch in [current_epoch, previous_epoch] ... ``` *"Attestations must be from the current or previous epoch"* assertion was from [PR#1466 the decoy flip flop resistance](https://github.com/ethereum/eth2.0-specs/pull/1466). > Decoy flip flop issue: https://ethresear.ch/t/decoy-flip-flop-attack-on-lmd-ghost/6001 It checks if `target.epoch` is within `[current_epoch, previous_epoch]`. :::warning Note that `current_epoch` and `previous_epoch` are **computed from `store.time`**. (system clock) ::: --- ## Optimization notes For optimization, clients may skip some fork-choice attestation validations when attestation is from block. We may have to evaluate how critical decoy-flip-flop is; if it's really bad, ask client team to add these validation. --- ## Recent changes From [PR#2727](https://github.com/ethereum/consensus-specs/pull/2727), to make it pass our old test cases, we only validate `assert target.epoch in [current_epoch, previous_epoch]` when the attestation is from wire (not from a block). You can read this commit diff: https://github.com/ethereum/consensus-specs/pull/2727/commits/f643554aa519ce6efe015ddc97d564e90c0fe248 --- ## Discussions ### Option 1: Should we remove `assert target.epoch in [current_epoch, previous_epoch]` check when the given attestation is from a block? - Current `dev` branch. - Danny: Essentially, attestations from blocks should be allowed regardless of timestamps. If you learn of a new chain at time T and it has blocks and attestatiosn from an arbitrary depth (+2 epochs), you should be able to incorporate it. ### Option 2: Or, should we totally remove `assert target.epoch in [current_epoch, previous_epoch]`? - Implementation: https://github.com/ethereum/consensus-specs/compare/dev...fc-dev-allow-all-old-attns-patch - Aditya: If old attestation is allowed when it's from a block [option 1], then we *already* lose some decoy-flip-flop resistance anyway. ### Option 3: Or, should we revert the recent attestation validation change? i.e., validate attestation time for both cases. ### Option 4: Ignore old attestations in LMD (to prevent head flip-flop from saved votes), but process them for FFG and update store if better justified/finalized checkpoints are found