# Teku Altair Invalid Blocks

In the first epoch after the Altair fork on Prater, Teku produced a number of invalid blocks.

The blocks were rejected because the batch signature verification failed - this means that one or more signatures in the block, or the block signature itself was invalid.

Analysis showed that the block included a number of attestations from nodes which had not upgraded to Altair. All of these attestations had a `beacon_block_root` pointing to the last phase0 block and a low number of aggregate bits set. The attestation signatures however were rejected with both Altair-enabled and Altair-disabled code paths when using the pre-state of the invalid block.

## Block Production

When Teku creates a block, it pulls attestations from a pool of pre-validated attestations and only performs the checks required to ensure that the attestations can still be included in the block being created (appropriate slot, not from a significantly diverged fork etc). It does not revalidate the signatures because they are verified prior to adding the attestation to the pool.

## How are invalid signatures in the pool?
It was confirmed that it is not possible for an attestation to be added to the pool without the signature being verified. Additionally the fork used is not hard coded based on the gossip topic the attestation was received on.

The real issue is that Teku reuses the same code to validate attestation signatures for processing gossip and for blocks - our version of [`is_valid_indexed_attetation`](https://github.com/ethereum/consensus-specs/blob/dev/specs/phase0/beacon-chain.md#is_valid_indexed_attestation). The issue arises because that method calculates the domain based on the fork in the state:

domain = get_domain(state, DOMAIN_BEACON_ATTESTER, indexed_attestation.data.target.epoch)

While it specifies the epoch required by the attestation, the fork information in `BeaconState` can only provide information about *previous* forks, not future forks. When verifying the block, this works as the block is from after the Altair fork, the state has information about the Altair fork.

However, when validating gossip, Teku skips processing empty slots if the start referenced by the `beacon_block_root` if it's is already within the committee look-ahead period. For these attestations the state is in the phase0 fork but the attestation slot is in the altair fork. Since the state has no information about the future fork, when validating the attestations as they arrive via gossip the phase0 fork is used, but when validating as part of a block the altair fork is used. That results in a different domain, thus different signing root and invalid signature.

## Other Areas of Concern

The same issue may apply to any operation that can be included in blocks:

* Attestations
* Broken, see above
* Proposer Slashings
* Uses latest chain head for gossip
* May have problems if there are empty slots immediately after the fork activates
* Attester Slashings
* Uses `is_valid_indexed_attestation`
* Uses latest chain head for gossip
* May have problems if there are empty slots immediately after the fork activates
* Voluntary exists
* Safe - fork chosen based on target exit epoch which cannot be in the future
* Sync Aggregate
* Only gossipped during the current slot, validated against head state
* Safe for Altair as head state has to be rolled forward to Altair to validate, but possibly not safe in future forks if there are empty slots

Eth1Data and Deposits do not use forks in signing so are safe.

## Solutions?

I’m still not quite sure how best to completely eliminate this class of problem. I’m tempted to get rid of the `get_domain(state, …)` type calls that get the fork from the state and pass the `Fork` to use through - block processing would get it from the state but gossip would get it from the fork schedule so it includes future forks even if we haven't wound forward the state.

## Example Bad Block

https://gist.github.com/ajsutton/3bade8af0c1929759dc4136c8030a6fb

## Potential Reference Tests?

Currently we have reference tests for importing blocks, but not for validating gossip. Using a format similar to the fork choice tests it should be possible to test gossip validation logic and pick up these kinds of issues.

- Build a chain by importing specific blocks
- Advance time as required with tick events
- Provide serialized gossip messages to simulate receiving them at specific points in the chain with specific topics and assert on the result (ACCEPT/REJECT/IGNORE/SAVE_FOR_LATER)