Remove invalid blocks from fork choice

[michaelsproul]

Description

To simplify handling of invalid blocks, we could consider removing them entirely from fork choice.

This interacts with optimistic sync, FCR, and normal fork choice operation.

Needs further investigation.

[michaelsproul]

This would solve a current issue in our FCR implementation whereby invalid blocks can be considered during FCR, and are not always ruled out (I'm not even sure of the extent of this issue, but Fable spotted it while reviewing FCR).

[NikhilSharmaWe]

@michaelsproul I had a look into this.

When a payload is invalid we keep the block in fork choice and mark it Invalid - zero weight, no children, excluded in filter_block_tree. That covers the optimistic sync requirements in practice, but it isn't real removal. filter_block_tree already notes the spec drops these from store.blocks and we only emulate that (same protoarray tradeoff Prysm had).

For FCR: is_one_confirmed rejects optimistic/invalid, but ancestor walks, attestation scores, and FFG helpers can still see those nodes. So incomplete filtering looks real, though I haven't reproduced a bad confirmation yet. Low urgency for default nodes since FCR is opt-in. Any actual removal also needs to be Gloas-aware - V29 nodes have no execution_status.

Please let me know what you think. Happy to dig further either on a reproducing FCR test / filtering, or a removal spike.

[eserilev]

is this necessary post-gloas ?

[NikhilSharmaWe]

I looked into this a bit.

Post-Gloas, it seems like this mostly goes away in the form the issue describes:

• V29 nodes don’t have execution_status
• The invalidate path only updates V17 nodes; on V29 it stops
• An invalid envelope is rejected before on_valid_payload_envelope_received, so the beacon block stays in fork choice but never becomes FULL
• So there isn’t really an Invalid node left sitting in FC to remove

The FCR concern looks pre-Gloas-shaped too, since is_optimistic_or_invalid depends on execution_status.

It still seems relevant while we’re pre-Gloas (and maybe during the transition), but I’m not sure a large “remove invalid blocks from protoarray” change is needed for post-Gloas.