fix(rpc): version the attester slashings pool by the wall clock

[MegaRedHand]

Motivation

GET /eth/v2/beacon/pool/attester_slashings took its version and Eth-Consensus-Version from the head slot's fork. Right after a fork boundary whose first block has not arrived, the head is still on the old fork while beacon-APIs means the current (wall-clock) epoch's fork. Found while auditing for the head-versus-wall-clock bug class (#663, #670).

Change

• version and Eth-Consensus-Version come from fork_at_epoch of the wall clock's epoch.
• No container conversion is needed: the pool holds electra-shaped slashings and their JSON is the same in every fork from electra on (only the SSZ bound on attesting indices differs; gloas adds no container), noted in the doc comment.
• The other operation-pool GETs (proposer_slashings, voluntary_exits, bls_to_execution_changes) are not versioned by the head, so nothing else changes.
• Test: head before a fork boundary, clock after it: the version is the clock's fork, not the head's.

cargo test -p ethlambda-rpc --lib --profile release-fast, clippy and fmt clean.

[github-actions]

🤖 Kimi Code Review

I'll review this PR focusing on the consensus-critical change from head-based to wall-clock-based fork versioning for attester slashings.

Overall Assessment

This is a correct and necessary fix for a consensus edge case at fork boundaries. The change aligns with the Beacon API specification's intent that the "active consensus version" should follow the wall clock, not the head state.

---

Detailed Review

crates/net/rpc/src/beacon/operations.rs

Lines 76-86: Comment and logic change

The new comment is excellent — it clearly explains:

• Why the wall clock matters (head can lag at fork boundaries)
• Why no conversion is needed (JSON representation is identical across forks for this container)

The logic change is correct. Using wall_slot(&store) instead of head_slot ensures validators receive the proper fork version for serialization when operating near fork boundaries.

Line 82: Potential panic/unwrap concern

.fork_at_epoch(compute_epoch_at_slot(crate::beacon::node::wall_slot(
    &store,
)));

wall_slot internally may panic or return an error if the store's time source is unavailable. However, this appears consistent with existing patterns in the codebase, and the function is used elsewhere. No change needed if wall_slot's error handling is already established.

Lines 545-566: Test quality

The test is well-constructed:

• Explicitly asserts the precondition that head and wall forks differ
• Verifies both the HTTP header and JSON response body
• Uses ForkName::Fulu which suggests this is forward-looking to future forks

One minor suggestion on line 555:

fixture.store = beacon_store_at(state).0;

The .0 tuple access is slightly opaque. Consider destructuring for clarity:

let (store, _) = beacon_store_at(state);
fixture.store = store;

This improves readability and makes it explicit that a second value is being ignored intentionally.

---

docs/rpc.md

Line 245: Documentation update

The parenthetical addition is accurate and helpful. Matches the implementation correctly.

---

Consensus Correctness Analysis

Aspect	Assessment
Fork choice safety	No impact — this is an API presentation layer change
Validator client compatibility	Improved — prevents mis-decoding at fork boundaries
SSZ correctness	Unaffected — JSON serialization is fork-agnostic for this container
Spec compliance	Aligned with Beacon API "active consensus version" concept

The critical insight in the comment (line 80-81) is correct: AttesterSlashing containers differ only in SSZ list bounds for attesting indices, which don't affect JSON representation. This means the pool can store electra-shaped slashings indefinitely without conversion overhead.

---

Minor Suggestions

1. Line 555: Destructure tuple instead of .0 access for clarity
2. Line 550-551: Consider adding a brief comment explaining why beacon_store_at(state) creates the wall-clock/head divergence, since the test setup's timing behavior may not be obvious to future readers

---

Verdict

Approve with minor nits. The fix is correct, well-tested, and properly documented. The edge case at fork boundaries is real and this prevents validator clients from misinterpreting fork versions when blocks are delayed or missing at transition epochs.

---

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

[github-actions]

🤖 Claude Code Review

Review of PR 672: version the attester slashings pool by the wall clock

The change is small and looks correct. I only read the diff and the code around it. I didn't build the crate or run the tests.

• Fix: get_attester_slashings now takes its fork from fork_at_epoch(compute_epoch_at_slot(wall_slot(&store))) (operations.rs:88). Before, it used the head slot. This matches the Beacon API's "active consensus version", so a validator client no longer decodes the list as the wrong fork's container when the head lags the clock at a fork boundary.
• wall_slot helper: It is millisecond-based and uses saturating_sub and .max(1), so there is no underflow or divide-by-zero. It is the same helper get_syncing already uses, so the two endpoints agree. Reusing it is the right call.
• Dropped error path: The old no head block 500 is gone, and the handler no longer needs a head at all. That is an improvement, because the response doesn't depend on the head.
• Test: The new test sets up a head before the fork boundary and a clock after it. It asserts the two forks differ, then checks both the Eth-Consensus-Version header and the JSON version. The assert_ne! precondition makes it fail loudly if the fork schedule changes. It also doesn't touch the clock, because it reads wall_slot directly.
• Docs: The docs/rpc.md row is updated to match.

Two minor points, neither blocking:

1. The doc comment says the pool's JSON is identical across forks and only the SSZ list bound differs. That holds for JSON output. A POST of an attester slashing is validated against a fork too, so it's worth checking that the POST path's fork choice agrees (head vs. clock), or the two halves of the endpoint could disagree.
2. Other pool endpoints (for example the proposer slashings and exits) don't carry a version, so there is nothing to align there. GET for attester slashings is the only one that does.

I'd approve this.

---

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

[github-actions]

🤖 Codex Code Review

Looks good overall. The change aligns the Eth-Consensus-Version/version with the Beacon API’s active consensus version semantics and avoids a bad edge case at fork boundaries.

• No correctness bug stands out in crates/net/rpc/src/beacon/operations.rs:85. Using wall_slot() instead of the head slot fixes the late/missing-boundary-block case without touching fork choice, attestation validation, STF, or SSZ paths.
• The old no head block error path is intentionally removed, and that seems correct here: pool reads should not depend on having a head if the version is derived from wall time. That is a small robustness improvement in crates/net/rpc/src/beacon/operations.rs:85.
• The security/perf impact is negligible and favorable: one wall-clock computation replaces a head lookup, with no new deserialization, signature, or pool mutation surface in crates/net/rpc/src/beacon/operations.rs:86.
• The key assumption is documented well: JSON shape is stable across forks for these electra-shaped slashings, despite SSZ container-bound differences. Given this endpoint returns JSON, that rationale in crates/net/rpc/src/beacon/operations.rs:76 looks sound.
• The added regression test is well targeted and covers the consensus-boundary behavior that matters most for interoperability in crates/net/rpc/src/beacon/operations.rs:546.

No requested changes from me.

---

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