feat(rpc): serve GET /eth/v2/validator/duties/proposer/{epoch}

[MegaRedHand]

Stacked on #642.

Motivation

beacon-APIs #563 added GET /eth/v2/validator/duties/proposer/{epoch} and deprecated v1. The two return the same duties. They differ only in dependent_root:

	dependent slot for epoch E
v1	compute_start_slot_at_epoch(E) - 1
v2	compute_start_slot_at_epoch(E - 1) - 1, genesis on underflow

v2's slot is the one fulu's proposer_lookahead depends on. process_proposer_lookahead writes epoch E's proposers during the transition into E-1. v1's later root also changes on reorgs inside E-1, which leave the duties as they were, so a validator client watching it refetches for nothing.

Changes

• New route /eth/v2/validator/duties/proposer/{epoch}. v1 and v2 share one proposer_duties body, which takes the dependent slot as a function argument.
• last_slot_before / last_slot_before_previous helpers. Attester duties already used v2's formula and now call the shared helper.
• docs/rpc.md: a table row for v2, v1 marked as deprecated by the spec, and the dependent_root note rewritten for each endpoint.

Not in this PR

• ethlambda validator still fetches v1.
• The v2 spec tells clients to compare against head_v2 events' current_epoch_dependent_root / next_epoch_dependent_root. The events endpoint (feat(beacon): serve the Beacon API eventstream, GET /eth/v1/events #626) doesn't emit those yet.

Testing

• v2_dependent_root_is_the_block_before_the_previous_epoch: for both lookahead epochs, v2's data equals v1's, and its dependent_root matches the spec slot and differs from v1's. The head's epoch is 1, so this test also covers the underflow-to-genesis case.
• an_epoch_outside_the_lookahead_is_a_400 now covers both versions.
• cargo test -p ethlambda-rpc --lib -- beacon::validator: 25 passed. cargo clippy -p ethlambda-rpc --all-targets -D warnings is clean.

[github-actions]

🤖 Kimi Code Review

I'll review this PR which adds v2 proposer duties endpoint support to ethlambda's Beacon API.

Overall Assessment

This is a well-structured, focused PR that correctly implements the Beacon API v2 proposer duties specification. The refactoring to share logic between v1 and v2 is clean. I found one minor correctness issue in test code and several code quality suggestions.

---

Issues Found

1. Test Bug: Potential Underflow in v2_dependent_root_is_the_block_before_the_previous_epoch (crates/net/rpc/src/beacon/validator.rs:681-682)

let dependent = compute_start_slot_at_epoch(epoch - 1).saturating_sub(1);

Problem: epoch - 1 will panic on underflow when epoch is 0. While the test loop starts at state_epoch (which is 1 in fulu_state), this is fragile. If fulu_state() ever changes to return epoch 0, this test will panic.

Fix:

let dependent = compute_start_slot_at_epoch(epoch.saturating_sub(1)).saturating_sub(1);

This matches the production last_slot_before_previous implementation and is defensive.

---

2. Inconsistent Documentation Comment (crates/net/rpc/src/beacon/validator.rs:338)

/// `GET /eth/v2/validator/duties/proposer/{epoch}`: v1's duties, with
/// `dependent_root` at `compute_start_slot_at_epoch(epoch - 1) - 1` (the
/// genesis block's on underflow).

Problem: The comment says "genesis block's on underflow" but should say "genesis block on underflow" or "the genesis block's root on underflow". Minor grammar issue.

Also, the formula compute_start_slot_at_epoch(epoch - 1) - 1 doesn't show the saturating_sub behavior explicitly, unlike the v1 comment which shows saturating_sub(1). This could mislead readers into thinking it panics.

Fix:

/// `GET /eth/v2/validator/duties/proposer/{epoch}`: v1's duties, with
/// `dependent_root` at `compute_start_slot_at_epoch(epoch.saturating_sub(1)).saturating_sub(1)`
/// (the genesis block's root on underflow).

---

3. Function Pointer vs. Trait Object Trade-off (crates/net/rpc/src/beacon/validator.rs:340)

fn proposer_duties(
    store: &Store,
    epoch: &str,
    dependent_slot: fn(Epoch) -> Slot,
) -> Result<serde_json::Value, ApiError> {

Observation: Using fn pointer is correct here since both last_slot_before and last_slot_before_previous are bare functions. This is zero-cost and clearer than a closure or trait object. No change needed, but good to confirm this was intentional (it appears so).

---

4. Missing Test Coverage for v2 Epoch 0 Edge Case (crates/net/rpc/src/beacon/validator.rs)

The v2 test only covers state_epoch and state_epoch + 1 where state_epoch = 1. The v1 test at line 650-663 tests epoch 0 explicitly:

#[tokio::test]
async fn dependent_root_is_the_block_at_the_last_slot_of_the_previous_epoch() {
    // ...
    let epoch = compute_epoch_at_slot(state.slot()); // epoch 1
    // ...
    assert_eq!(json["dependent_root"], format!("{head_root}"));
}

Suggestion: Add a test for v2 with epoch 0, or extend the existing v2 test to include epoch 0, to verify the genesis underflow behavior. The v2 dependent_root for epoch 0 should equal the genesis block root (same as v1 for epoch 0, and same as v2 for epoch 1 in the current test since both underflow).

Actually, re-reading: for epoch 0, v2's last_slot_before_previous(0) = last_slot_before(0.saturating_sub(1)) = last_slot_before(0) = compute_start_slot_at_epoch(0).saturating_sub(1) = 0.saturating_sub(1) = 0. So both v1 and v2 for epoch 0 depend on slot 0 (genesis). This is worth asserting.

---

5. Route Ordering (Minor) (crates/net/rpc/src/beacon/validator.rs:49-52)

.route(
    "/eth/v2/validator/duties/proposer/{epoch}",
    get(get_proposer_duties_v2),
)

Observation: The v2 route is added after v1. With axum's route matching, order shouldn't matter here since the paths are distinct. However, if there were ever a wildcard or fallback, v1 before v2 is the conventional ordering. This is fine as-is.

---

Security Considerations

1. No new DoS vectors: The v2 endpoint reuses the same proposer_duties logic with identical epoch validation and lookahead bounds checking. No concern.

2. No state mutation: Both endpoints are read-only from the store, consistent with existing design.

3. SSZ/serialization: No changes to encoding; JSON responses use existing json_response helper.

---

Consensus Correctness

The v2 dependent_root definition matches the Beacon API specification:

The dependent root is get_block_root_at_slot(state, compute_start_slot_at_epoch(epoch - 1) - 1)

The implementation correctly uses saturating_sub to handle the genesis case, matching the spec's underflow behavior.

The comment at line 334-336 accurately explains why v2's definition is superior: it references the block that the proposer lookahead actually depends on, making reorg detection more precise for validators.

---

Positive Acknowledgments

1. Clean refactoring: Extracting proposer_duties with a dependent_slot function parameter is elegant and DRY.
2. Helper functions last_slot_before and last_slot_before_previous are well-named and reusable — good that attester_duties was updated to use last_slot_before_previous too.
3. Test updates are thorough: Both v1 and v2 are tested for the out-of-lookahead 400 case.
4. Documentation (rpc.md) is updated with accurate descriptions of the new endpoint and the dependent_root semantics.

---

Summary

Priority	Item	Location
Fix	Test underflow: epoch - 1 → epoch.saturating_sub(1)	validator.rs:681
Fix	Grammar in doc comment	validator.rs:338
Suggest	Add v2 epoch 0 test case	validator.rs tests
—	All other code is correct and well-structured	—

---

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

[github-actions]

🤖 Claude Code Review

Review: GET /eth/v2/validator/duties/proposer/{epoch}

The change looks correct and I found no blocking issues.

• Dependent slot: v2 uses last_slot_before_previous(epoch), which is compute_start_slot_at_epoch(epoch-1) - 1, saturating to genesis. That matches beacon-APIs refactor(storage): cache ChainConfig in Store #563 and the attester-duties formula. The attester handler now calls the same helper, so the two cannot drift apart.
• Underflow: epoch.saturating_sub(1) and compute_start_slot_at_epoch(..).saturating_sub(1) mean epochs 0 and 1 both resolve to the genesis block. No panic is possible.
• Handler structure: proposer_duties takes dependent_slot: fn(Epoch) -> Slot, so v1 and v2 share one body and differ only in that argument. The fn pointer needs no boxing and is easy to read.
• Lookahead window: the checked_sub and MIN_SEED_LOOKAHEAD filter bound the slice indexing, so window_start..[..SLOTS_PER_EPOCH] cannot go out of range. This logic was already there and the diff does not change it.
• block_root_at_or_before: a slot at or past the head's slot returns head_root. An out-of-window slot returns a 500 rather than a guess. A v2 dependent slot is never further back than v1's, so the window risk is no worse than before.
• Tests: v2_dependent_root_is_the_block_before_the_previous_epoch checks both lookahead epochs. It asserts that the duties data equals v1's and that the roots differ. The 400 path for out-of-range epochs is also run against v2 (line 969).

Minor nits:

1. In v2_dependent_root_is_the_block_before_the_previous_epoch, assert_ne! on the two roots depends on the fixture's distinct per-slot roots (repeat_byte(slot + 1)). That holds today. A comment saying so would stop it breaking if someone changes the fixture.
2. get_proposer_duties and get_proposer_duties_v2 are identical except for the function pointer. A small helper would remove the repetition, though two three-line handlers are acceptable.

I did not run the tests.

---

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

[github-actions]

🤖 Codex Code Review

Looks good overall. The change is narrowly scoped, aligns with the Beacon API v2 proposer-duties semantics, and reuses the existing duty computation cleanly.

• crates/net/rpc/src/beacon/validator.rs:321 The new v2 handler appears correct: only dependent_root changes, while duties still come from proposer_lookahead. That matches the stated intent and avoids touching fork-choice, attestation validation, justification/finalization, or state-transition logic.
• crates/net/rpc/src/beacon/validator.rs:304 Using saturating_sub for the previous-epoch dependent slot is a good choice for epochs 0 and 1; it avoids underflow and preserves the documented “genesis block on underflow” behavior.
• crates/net/rpc/src/beacon/validator.rs:342 Refactoring proposer_duties to take a dependent_slot function keeps the v1/v2 split explicit without duplicating proposer selection logic. No obvious performance or safety concern here.
• crates/net/rpc/src/beacon/validator.rs:483 Reusing last_slot_before_previous for attester duties improves consistency with the already-documented semantics and reduces the chance of drift.

One small test nit:

• crates/net/rpc/src/beacon/validator.rs:683 The test uses compute_start_slot_at_epoch(epoch - 1).saturating_sub(1). Since epoch - 1 happens before the saturating call, this can underflow in debug builds if the fixture ever changes to state_epoch == 0. The production code avoids this with last_slot_before_previous(epoch). I’d suggest mirroring that in the test, e.g. let dependent = last_slot_before_previous(epoch);.

No blockchain-security or memory-safety concerns jump out in this diff.

---

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

[MegaRedHand]

Already included in #653