[MegaRedHand]

Motivation

On the heze-devnet-3 kurtosis devnet, ethlambda reads no inclusion lists at the first slot of every epoch:

• As proposer, its bid carries inclusion_list_bits = 0x0000 and the EL is asked to include no IL transactions (4 of 4 epoch-first blocks it proposed; lodestar, prysm and teku carry 0xffff at the same slots).
• As verifier, every epoch-first envelope is judged against an empty list set (inclusion_list_transactions=0 inclusion_list_satisfied=true), 25 of 27 boundaries. Every other slot sees a non-empty set (832 of 832).

So FOCIL enforcement is silently off on ethlambda for one slot in every epoch. Liveness and finality are unaffected.

Cause

1. Lists are stored under (slot, dependent_root), where dependent_root is the block at compute_shuffling_dependent_slot(epoch(slot)) = start(epoch - 1) - 1.
2. For the lists of an epoch's last slot, that block sits one slot below the finalized checkpoint the node adopts at the next epoch-boundary tick, and that tick prunes LiveChain to the finalized slot.
3. inclusion_list_dependent_root walked only LiveChain (get_shuffling_dependent_root), whose walk falls back to the lowest indexed block, i.e. the finalized block, so the key never matched.

Devnet log, slot 192 (lists of slot 191 depend on block 127):

14:46:39.002186Z Pruned finalized beacon live chain finalized_slot=128 block_slot=128 pruned_chain=32
14:46:39.230187Z Building on the previous slot's inclusion lists slot=192 inclusion_lists=0 inclusion_list_transactions=0

The same ordering (prune, then ~230 ms later the proposal) holds at every boundary ethlambda proposed. The two boundaries that did read lists (slots 64 and 96) precede the first prune.

Fix

inclusion_list_dependent_root keeps walking through the block table, which keeps every block, where the LiveChain walk stops above the dependent slot. The common case still answers from the index alone; a boundary costs one block read. get_shuffling_dependent_root itself is unchanged, since its proposer-boost caller only compares two of its answers.

Both callers are fixed by this: block production (heze_inclusion_lists) and envelope satisfaction (payload_inclusion_list_transactions).

Tests

New unit tests in inclusion_list::tests, on a store whose blocks go through insert_signed_block (so both BlockHeaders and LiveChain hold them):

Test	Chain	Expected
the_dependent_root_below_a_pruned_live_chain_is_the_real_ancestor	0, 100, 127, 128, 150, 191; LiveChain pruned to 128	block 127 (was block 128)
a_skipped_dependent_slot_resolves_to_the_block_before_it_past_the_prune	0, 100, 120, 128, 150, 191; pruned to 128	block 120 (was block 128)
an_unpruned_live_chain_answers_as_the_index_walk_does	same, unpruned	block 127, equal to get_shuffling_dependent_root

The first two fail without the fix. fork_choice::tests exposes its empty_store and block helpers as pub(crate) for reuse.

Ran locally: cargo test -p ethlambda-state-transition --profile release-fast --lib (627 passed, 15 ignored), cargo clippy -p ethlambda-state-transition --all-targets -- -D warnings, cargo check -p ethlambda-rpc.

Follow-up

The bug dates from the original EIP-7805 commit, so after this lands it gets merged forward into heze-devnet-2 and heze-devnet-3.

[github-actions]

🤖 Kimi Code Review

I'll review this PR focusing on the inclusion_list_dependent_root function changes and the test visibility modifications.

Summary

The PR fixes a bug where inclusion_list_dependent_root returns incorrect results when the live chain has been pruned past the shuffling-dependent slot. The fix walks past the live-chain index into stored blocks. However, I found several issues ranging from correctness bugs to code quality concerns.

---

Critical Issues

1. Logic Bug: block_entry returns (Root, Slot, Root), not (Slot, Root)

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Lines: 5650-5651

let entry = index
    .get(&root)
    .copied()
    .or_else(|| store.block_entry(&root));
match entry {
    Some((block_slot, parent_root)) if block_slot > dependent_slot => root = parent_root,

The destructuring Some((block_slot, parent_root)) assumes a 2-tuple, but store.block_entry() likely returns a 3-tuple (root, slot, parent_root) or (block_root, slot, parent_root). Check the actual return type of Store::block_entry.

If it's a 3-tuple, this pattern matches the first two elements, meaning block_slot would actually be the block root and parent_root would be the slot. This would cause:

• The comparison block_slot > dependent_slot to compare a hash with a slot (nonsense)
• root = parent_root to assign a slot number as a root

Action: Verify Store::block_entry signature and fix the destructuring accordingly.

---

2. Potential Infinite Loop on Malformed/Circular Data

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Lines: 5644-5656

The loop has no iteration limit. If stored block data is corrupted or contains a cycle (e.g., a block pointing to itself as parent), this loops forever.

loop {
    let entry = index.get(&root).copied().or_else(|| store.block_entry(&root));
    match entry {
        Some((block_slot, parent_root)) if block_slot > dependent_slot => root = parent_root,
        _ => return root,
    }
}

Action: Add a maximum iteration bound (e.g., SLOTS_PER_EPOCH * 2 or a reasonable chain depth limit) and return an error or panic with context if exceeded.

---

Security Vulnerabilities

3. Missing Validation: block_slot could be from wrong fork or invalid chain

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Lines: 5644-5656

The function walks through stored blocks without verifying:

• The blocks form a valid chain (no signature/transition validation)
• The head_root is actually on the canonical chain
• The retrieved blocks haven't been reorged out

For consensus-critical shuffling computation, this could allow an attacker to influence committee selection by feeding non-canonical blocks if they can manipulate stored block access.

Action: Document the trust assumptions. If this reads from untrusted storage, add chain validity checks or document why it's safe.

---

Performance Concerns

4. Repeated store.block_entry Calls Without Caching

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Lines: 5649-5651

let entry = index
    .get(&root)
    .copied()
    .or_else(|| store.block_entry(&root));

Each iteration may hit disk/database for store.block_entry. For long chains (e.g., 32 slots back), this could be 32 database reads per call.

Action: Consider if the common case can be optimized, or document the expected access pattern. The index.get(&root).copied() fast path helps, but clarify expected performance.

---

Code Quality & Maintainability

5. Comment-Code Mismatch in Loop Logic

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Lines: 5643-5648

The comment says "The index starts at the finalized block" but the code first calls get_shuffling_dependent_root which operates on the live chain index, then falls back to stored blocks. The comment structure is confusing—clarify that it's a two-phase walk: index first, then storage.

---

6. Test Helper Visibility Change: pub(crate) in #[cfg(test)] module

File: crates/blockchain/state_transition/src/beacon/fork_choice.rs
Lines: 5642, 5656, 5691

Changing mod tests and its items to pub(crate) exposes test helpers to production code. While useful for cross-module testing, this:

• Allows production code to accidentally depend on test utilities
• Makes it unclear what's a real API vs. test helper

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Line: 5320

use crate::beacon::fork_choice::tests::{block, empty_store};

This imports from a tests module in production test code, which is the motivation for the visibility change. Consider instead:

Action: Extract shared test helpers to a testing module (not #[cfg(test)]) or a test_utils crate, rather than exposing tests internals.

---

7. Test Root Generation: Root::repeat_byte(position as u8 + 1) is Collidable

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Lines: 5336-5337

let root = Root::repeat_byte(position as u8 + 1);

This generates only 256 possible distinct roots. With 6 slots in tests it's fine, but if tests expand or this pattern copies, collisions become likely. More importantly, it's not cryptographically realistic.

Action: Use a deterministic hash or sequential counter with proper byte width. At minimum, document this is test-only and collision-acceptable.

---

8. Magic Numbers in Tests

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Lines: 5326-5327

const FINALIZED: Slot = 128;
const LAST_SLOT: Slot = 191;

These appear to assume SLOTS_PER_EPOCH = 32 (epochs 4 and 5, with 128 = 432, 191 = 532 + 31 - 1? Actually 191 = 5*32 + 31 = 191, so epoch 5 last slot). But this isn't explicit.

Action: Compute from constants: const FINALIZED: Slot = 4 * SLOTS_PER_EPOCH; etc., or add comments explaining the epoch math.

---

9. Missing Test: Dependent Root at Exact Prune Boundary

The tests cover:

• Exact dependent slot exists in stored blocks (Item 2's test)
• Dependent slot is skipped (Item 3's test)
• Unpruned chain (Item 4's test)

Missing:

• What if head_root itself is pruned?
• What if the walk reaches Root::ZERO (genesis)?
• Error/edge case: empty store, no blocks at all

Action: Add tests for boundary conditions, especially genesis and fully-pruned scenarios.

---

Minor Issues

10. copied() on Option<&(Slot, Root)> — check if type implements Copy

File: crates/blockchain/state_transition/src/beacon/inclusion_list.rs
Line: 5649

.get(&root)
.copied()

If the index stores (Slot, Root) and both are simple integers, this works. If it stores a struct or complex type, this won't compile. Verify the index value type.

---

Positive Acknowledgments

• The bug being fixed (pruned live chain causing incorrect dependent roots) is real and the fix direction is correct
• Tests are well-named and cover the core scenarios
• The detailed comment explaining the pruning interaction is valuable for future maintainers

---

Recommended Changes Summary

Priority	Item	File	Action
Critical	1	inclusion_list.rs:5650	Verify and fix block_entry return type destructuring
Critical	2	inclusion_list.rs:5644	Add iteration limit to prevent infinite loops
High	6	fork_choice.rs, inclusion_list.rs	Extract shared test helpers to test_utils module
Medium	3	inclusion_list.rs	Document trust assumptions for stored block reads
Medium	8	inclusion_list.rs:5326	Replace magic numbers with computed constants
Low	7	inclusion_list.rs:5336	Improve root generation in tests

---

Automated review by Kimi (Moonshot AI) · kimi-k2.6 · custom prompt

[github-actions]

🤖 Claude Code Review

Review of PR 692: read inclusion lists under the real dependent root

I found no blocking issues. I read the diff and checked get_shuffling_dependent_root and Store::block_entry. I did not build the code or run the tests.

Correctness

• The diagnosis in the PR description is plausible. get_ancestor_or_lowest_indexed falls back to the lowest indexed block, which is the finalized block after LiveChain is pruned. The loop in inclusion_list_dependent_root (inclusion_list.rs:~130-147) starts from that fallback and keeps walking parents until block_slot <= dependent_slot, which gives the real ancestor.
• Termination is safe. Every step moves to a strictly lower slot, and a root missing from the store ends the walk.
• Skipped slots resolve to the latest block at or below the dependent slot, which matches get_ancestor semantics. The second test covers this case.
• A checkpoint-anchor parent that the store doesn't hold returns that root. The doc comment says so, and it is the correct ancestor when the dependent slot is just below the anchor.
• The common path is cheap. When the index walk already lands at or below the dependent slot, the loop does one index lookup and returns without touching storage.
• Both callers are covered (heze_inclusion_lists and payload_inclusion_list_transactions). Leaving get_shuffling_dependent_root unchanged is right, because the proposer-boost caller only compares two of its answers.

Minor points (non-blocking)

1. store.block_entry decodes the whole beacon block, which the code comment acknowledges. This happens only at epoch-boundary slots, and only for blocks no longer in the index. Even so, this function is called per slot from production and from envelope satisfaction. If it ever shows up in profiles, a header-only read could replace it.
2. The loop does not cap how many steps it takes. After a long period without finality, the walk could cross many stored blocks, each a DB read. That is bounded by chain length and rare, so I would accept it.
3. In the tests, chain_store builds roots with Root::repeat_byte(position as u8 + 1). That is fine for six blocks, but it would wrap if someone passed more than 255 slots.
4. The pub(crate) visibility changes on fork_choice::tests helpers are fine, since they are #[cfg(test)].

The fix is targeted, the tests exercise the pruned and skipped-slot cases (the PR description says the first two fail without the fix), and the unpruned test confirms the answer is unchanged when no prune has happened. Looks good to merge.

---

Automated review by Claude (Anthropic) · sonnet · custom prompt

[github-actions]

🤖 Codex Code Review

Looks good overall — the fix in crates/blockchain/state_transition/src/beacon/inclusion_list.rs:133 appears directionally correct and addresses a real fork-choice/pruning edge case.

• crates/blockchain/state_transition/src/beacon/inclusion_list.rs:137
  Correctness: this now repairs get_shuffling_dependent_root when LiveChain has been pruned below finalized, by continuing through persisted block headers. That matches the intended “latest ancestor at or below dependent slot” behavior and avoids returning the finalized block spuriously for epoch-boundary cases.

• crates/blockchain/state_transition/src/beacon/inclusion_list.rs:140
  Performance: the fallback does an extra block_entry read per hop after the walk falls off the in-memory index. That’s probably fine here because inclusion-list dependent-root lookup is not in a hot per-validator loop, and the code wisely keeps the index lookup first.

• crates/blockchain/state_transition/src/beacon/inclusion_list.rs:145
  Semantics look right for skipped slots too: walking while block_slot > dependent_slot returns the latest block at or below the target slot, which is consistent with ancestor semantics used elsewhere in fork choice.

• crates/blockchain/state_transition/src/beacon/inclusion_list.rs:350
  Test coverage is good: you covered the pruned-boundary case, skipped-slot case, and preserved behavior on an unpruned chain. Those are the right consensus-sensitive scenarios for this change.

One small maintainability concern:

• crates/blockchain/state_transition/src/beacon/fork_choice.rs:5642
  Making the whole test module pub(crate) broadens visibility more than necessary. If the goal is only to reuse empty_store and block, consider moving those helpers into a dedicated test-support module instead, or exposing only a narrowly scoped helper module. This is low risk, but it avoids test internals becoming semi-public API inside the crate.

No obvious security or memory-safety issues in this diff, and I don’t see a consensus bug introduced by the new walk.

---

Automated review by OpenAI Codex · gpt-5.4 · custom prompt