ethereum / ethereum/consensus-specs
childless payload-status variants are never FFG-tested, so `get_head` can return a non-viable node
- Dominant language
- Python
- Stars
- 4k
- Forks
- 1.3k
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 58
Description
### Description
In pre-Gloas, [`filter_block_tree`](https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.12/specs/phase0/fork-choice.md#filter_block_tree) keeps a block in the filtered block tree if at least one of its children is kept. A childless block is kept only if it passes the FFG test itself ie.
https://github.com/ethereum/consensus-specs/blob/0ddf13b3dcdae1cc855c9aa8aa28af5e8c6f3c82/specs/phase0/fork-choice.md?plain=1#L437
The recursion therefore FFG-tests the leaf of every branch and keeps the ancestors of the ones that pass and discarding the rest.
So every node that `get_head` returns must have passed the FFG test.
Post-Gloas, [`get_node_children`](https://github.com/ethereum/consensus-specs/blob/v1.7.0-alpha.12/specs/gloas/fork-choice.md#modified-get_node_children) expands a `PENDING` node into its `EMPTY`/`FULL` variants without consulting the filtered block tree or any FFG test. So a childless variant of a block not only can stay in the tree but also become a leaf or head while never having been tested.
### Example
Pre-Gloas: B fails the FFG test, K passes, K' fails:
```
┌── K (passes)
J ── B ─┤
└── K' (fails)
```
K' is pruned. K is kept because it passes. B is kept because its child K is. B itself is never tested, but the walk can only pass *through* B, never stop at it.
Post-Gloas: same B, with K built on B's *empty* variant:
```
┌── EMPTY(B) ── K (passes)
J ── PENDING(B) ─┤
└── FULL(B) childless, never tested
```
`FULL(B)` is not pruned because it is never tested. `filter_block_tree` operates on blocks, not on variants. But now FULL(B) is a leaf, and the walk can stop here, and thus `get_head` can return it albeit it fails the FFG test.
I am not sure whether this is intended. Lodestar applies the FFG test per variant, so it prunes `FULL(B)` and can never return it as head, whereas the spec as written keeps it eligible. Several comments in the spec contradict the current behavior:
https://github.com/ethereum/consensus-specs/blob/0ddf13b3dcdae1cc855c9aa8aa28af5e8c6f3c82/specs/phase0/fork-choice.md?plain=1#L448-L452
https://github.com/ethereum/consensus-specs/blob/0ddf13b3dcdae1cc855c9aa8aa28af5e8c6f3c82/specs/gloas/fork-choice.md?plain=1#L572-L573
Contributor guide
No contributing guide indexed for this repository
Research direction
Read filter_block_tree in specs/phase0/fork-choice.md alongside get_node_children and get_head in specs/gloas/fork-choice.md. Trace the EMPTY/FULL childless-variant example and compare it with the cited FFG-test comments. Done means resolving whether variants require per-variant FFG filtering and updating the specification consistently.
Written by the indexing model from the issue text.
Assessment
- Domain
- distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100