feat(rpc): serve validator/liveness

[pablodeymo]

🗒️ Description / Motivation

POST /eth/v1/validator/liveness/{epoch} is what a validator client's doppelganger protection calls before it signs anything: "did anyone see these validators act in epoch E?" If the node answers 404:

• Lighthouse's validator client with doppelganger protection on keeps its validators muted forever.
• Prysm's fails at startup.

This is the last of the four missing endpoints; the other three are in #642, which this PR is stacked on.

The Beacon API leaves the answer to the node's own view ("network, chain or API"), and clients differ:

	Source of is_live	Current epoch
Prysm	State participation flags only	Late by however long inclusion takes
Lighthouse	Its gossip, block and aggregator caches	Complete, but relies on caches that on this node only cover the subnets it joined
This PR	Participation flags OR what this node observed	Covered as soon as the node sees the validator act

What Changed

• crates/storage/src/liveness.rs (new): ObservedLiveness, one bitset per epoch for the newest three.
  • An epoch older than the window is ignored.
  • Indices are capped at 1 << 24, so one epoch is at most 2 MiB. Every writer passes a validated index anyway; mainnet is ~2.4M validators, about 300 KB.
• crates/storage/src/store.rs: an observed_liveness field and accessor, shared by every Store clone like committee_cache, because the set has three writers and a reader that all already hold a Store.
• Writers:
  • crates/net/p2p/src/beacon/verdict.rs: on Accept, an aggregate marks its aggregator and every attester its signature verified; a subnet attestation marks its attester.
  • crates/blockchain/src/lib.rs: a successfully imported beacon block marks its proposer. This is done at import, not at gossip acceptance, so range-synced blocks and blocks published through this node's API count too.
  • crates/net/rpc/src/beacon/pool.rs: submissions through pool/attestations and aggregate_and_proofs, since gossip never delivers a node its own messages. The aggregate path now keeps the attesting indices that stateful_checks already returned.
• crates/net/rpc/src/beacon/validator.rs: post_liveness / liveness.
• docs/rpc.md: the route and the liveness semantics.

Correctness / Behavior Guarantees

• Nothing unverified counts. Only Accepted gossip and validated API submissions are recorded; an Ignore, Reject or Overloaded object marks nobody.
• Participation is read from the head state for its own epoch (current_epoch_participation) and the one before (previous_epoch_participation). Before altair there are no flags, so only observations count.
• Epoch window: the store clock's previous, current and next epoch, not the head state's. The next epoch is answered but always false, because a doppelganger check at an epoch boundary can land on it; Lighthouse accepts it too.
• Errors:
  • An epoch outside the window is a 400.
  • An unknown index is a 400, as in Prysm (Lighthouse answers false). An index outside the registry is a client bug, better reported than hidden.
  • 503 while syncing, as the spec lists; neither client returns it.
• Lock cost. Writers take a Mutex once per accepted gossip object, to set bits. That includes every accepted attestation on the backbone subnets, which don't touch the attestation pool: roughly a couple of thousand short locks a slot on mainnet, against a reader that only runs when a validator client asks.

Tests Added / Run

• ObservedLiveness:
  • a_recorded_validator_is_live_in_that_epoch_only
  • record_all_sets_every_index
  • epochs_older_than_the_window_are_pruned
  • an_epoch_below_the_window_is_not_recorded
  • an_index_past_the_guard_is_ignored
  • observed_liveness_is_shared_across_store_clones
• Endpoint (state at epoch 5 with its store clock set to match):
  • a_participation_flag_makes_a_validator_live (current and previous epoch flags)
  • an_observed_validator_is_live_without_a_flag
  • the_next_epoch_is_answered_and_nobody_is_live_in_it
  • epochs_outside_the_window_are_a_400
  • an_unknown_validator_is_a_400
  • a_syncing_node_answers_503
• P2P writers:
  • an_accepted_aggregate_marks_its_aggregator_and_attesters_live
  • an_accepted_subnet_attestation_marks_its_attester_live
  • an_object_that_was_not_accepted_marks_nobody_live
• RPC writers:
  • a_published_attestation_marks_its_attester_live
  • a_refused_attestation_marks_nobody_live
  • a_published_aggregate_marks_its_aggregator_and_attesters_live: the aggregate is built on one node and submitted to a fresh one, so only the aggregate itself can have marked its attesters.
• Not covered by a unit test: the proposer write on block import. The chain actor's tests have no successful beacon import to extend, since that needs a block passing the full state transition. It's one line, exercised on a devnet.
• CI's minimal-preset commands pass locally.
• Not yet run: kurtosis with Lighthouse's validator client and --enable-doppelganger-protection. Its validators should come online after the doppelganger wait instead of staying muted.

Related Issues / PRs

• Stacked on feat(rpc): serve states/{id}/fork, config/deposit_contract and validator/duties/sync #642 (fork, deposit_contract, duties/sync); retarget to beacon-chain-integration once feat(rpc): serve states/{id}/fork, config/deposit_contract and validator/duties/sync #642 merges.

✅ Verification Checklist

• Ran make fmt — clean
• Ran make lint (clippy with -D warnings) — clean
• Ran make test (test-consensus plus test-node, at release-fast) — all passing (1966 passed, 0 failed, 26 ignored)

[github-actions]

🤖 Codex Code Review

Looks good overall. The change is narrowly scoped, thread-safe, and doesn’t touch fork choice, state transition, justification/finalization, XMSS, or SSZ logic.

• crates/net/p2p/src/beacon/verdict.rs:155 only records liveness on Outcome::Accept, which is the right trust boundary for gossip-derived evidence.
• crates/net/p2p/src/beacon/verdict.rs:227 uses the aggregate’s target epoch plus validated attesting_indices, so it does not over-credit unverified participants.
• crates/blockchain/src/lib.rs:2779 records proposer liveness only after successful import, which correctly avoids counting invalid blocks and also covers range sync/API-imported blocks.
• crates/storage/src/liveness.rs:54 is simple and safe: bounded retention, bounded index space, and a compact bitset representation with no obvious memory-safety concerns.
• crates/net/rpc/src/beacon/pool.rs:119 and crates/net/rpc/src/beacon/pool.rs:319 mirror the P2P behavior for locally submitted attestations/aggregates, which is important because gossip won’t echo self-published messages.

Minor note:

• crates/storage/src/liveness.rs:81 does not guard validator >= MAX_TRACKED_INDEX on reads. It still returns false safely because .get() fails, so this is not a bug; adding the same guard there could make the intent slightly clearer and avoid a large index-to-usize cast in theory, but it’s not necessary in practice given the RPC already rejects unknown validator indices.

I don’t see correctness, security, or performance regressions in this PR.

---

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

[github-actions]

🤖 Claude Code Review

Review of PR 643: Beacon API POST /eth/v1/validator/liveness/{epoch}

Overall this is a clean, well-scoped change. I read the diff and crates/storage/src/liveness.rs, but I did not build the code or run the tests.

What I checked and found fine

• Storage: ObservedLiveness is a bitset per epoch behind a Mutex. It keeps 3 epochs, caps indices at 1 << 24 (2 MiB per epoch) and prunes under the same lock. That is bounded and cheap.
• Stale epochs: An epoch below the retained window is ignored. A range-sync import therefore can't create and then prune an entry.
• Recording only on acceptance: P2P records on Outcome::Accept only. The RPC records only after validation passes. The chain actor records after a successful import. Unverified objects never mark anyone live. The negative tests (an_object_that_was_not_accepted_marks_nobody_live, a_refused_attestation_marks_nobody_live) cover this.
• Aggregate validation path: stateful_checks now returns the attesting indices instead of discarding them. The aggregator plus every verified attester are recorded.

Points to consider

1. Overflow on a user-supplied epoch (validator.rs, in liveness()):

   • epoch + 1 < current, epoch > current + 1 and epoch + 1 == state_epoch all add to the path-supplied u64.
   • /liveness/18446744073709551615 would panic in a debug build, and wrap to 0 in a release build.
   • The wrapped value then falls through to the 400 branch only by accident.
   • Use saturating_add, or compare as epoch < current.saturating_sub(1).
   • This holds only if parse_epoch doesn't already bound the value; I didn't check that.

2. Self-submitted messages can trigger a doppelganger false positive (pool.rs, post_pool_attestations and post_aggregate_and_proofs):

   • The PR records a validator as live when its own validator client submits through this node's API.
   • If that client restarts mid-epoch and runs its doppelganger check against the same node, the node reports it live because of its own earlier messages.
   • The client then sees a clone that doesn't exist and may shut itself down.
   • This may be the intended trade-off, since the doc comment says gossip never delivers a node its own messages. But it should be called out in docs/rpc.md, or made distinguishable, for example by tracking API-sourced observations separately.

3. Minor:

   • In record_all, entry(epoch).or_default() happens before the empty-iterator case. A call with no validators still allocates an empty entry and can advance the pruning floor.
   • This is harmless, because every caller passes at least one index, but you could early-return.
   • A far-future epoch from a buggy caller would advance newest and evict the real epochs. The writers are all validated, so I don't see this as exploitable now.

4. Block proposer recording (lib.rs): Recording happens after the "Block imported successfully" log and is gated on Chain::Beacon, which is correct. It would help to confirm that slot here is the block's own slot and not the clock slot.

I didn't find any consensus-affecting bugs. Item 1 is the one I'd fix before merge, and Item 2 is a behavior decision worth documenting.

---

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

[github-actions]

🤖 Kimi Code Review

I'll review this PR which implements the POST /eth/v1/validator/liveness/{epoch} endpoint for doppelganger protection. Let me analyze each file carefully.

crates/storage/src/liveness.rs (new file)

Line 28-29: MAX_TRACKED_INDEX guard

const MAX_TRACKED_INDEX: ValidatorIndex = 1 << 24;

The comment says "Mainnet has about 2.4 million validators, well under it" — 2.4M < 16M (2^24), correct. However, this silently drops validators above this index. Consider whether this should be logged or if this bound is appropriate for all networks (testnets with large validator sets might approach this).

Line 41-42: Mutex over BTreeMap

pub struct ObservedLiveness(Mutex<BTreeMap<Epoch, Vec<u64>>>);

This is fine for the expected contention pattern (writes from P2P/RPC/chain, reads from RPC). The Mutex is held briefly for bit operations.

Line 55-62: Pruning logic

let newest = epochs
    .keys()
    .next_back()
    .copied()
    .unwrap_or(epoch)
    .max(epoch);
let floor = newest.saturating_sub(RETAINED_EPOCHS - 1);

This computes floor based on the newest recorded epoch, not necessarily epoch. This means if you record epoch 100, then record epoch 50, the floor is still 98 (100-2), so epoch 50 is silently ignored. This is correct per the spec comment ("An epoch older than the retained window is ignored"), but the interaction between newest and epoch could be clearer.

Line 64-69: Bitset growth

let word = (validator / 64) as usize;
if bits.len() <= word {
    bits.resize(word + 1, 0);
}
bits[word] |= 1 << (validator % 64);

Correct. Note that validator % 64 is u64, and 1 is u64, so 1 << (validator % 64) is well-defined for values 0-63. No overflow issue.

Line 78-82: is_live

pub fn is_live(&self, epoch: Epoch, validator: ValidatorIndex) -> bool {
    let epochs = self.0.lock().expect("liveness lock poisoned");
    epochs
        .get(&epoch)
        .and_then(|bits| bits.get((validator / 64) as usize))
        .is_some_and(|word| word & (1 << (validator % 64)) != 0)
}

Correct. Returns false for out-of-range validators (not in bitset), which is the safe default.

crates/storage/src/store.rs

Line 1522: observed_liveness field added

observed_liveness: Arc::new(ObservedLiveness::default()),

Good — Arc ensures sharing across Store clones, confirmed by test at line 5500-5506.

Line 2669-2671: Accessor returns reference

pub fn observed_liveness(&self) -> &ObservedLiveness {
    &self.observed_liveness
}

Returns &ObservedLiveness (not Arc<ObservedLiveness>), which is fine since ObservedLiveness itself uses internal Mutex synchronization. This is the right pattern.

crates/blockchain/src/lib.rs

Line 2775-2780: Block proposer liveness recording

if self.store.chain() == Chain::Beacon {
    let epoch = ethlambda_types::beacon::signing::compute_epoch_at_slot(slot);
    self.store.observed_liveness().record(epoch, proposer);
}

Correctly records proposer liveness at block import time, covering range sync and API submissions. The Chain::Beacon guard prevents recording on lean chain.

Potential issue: This is recorded after import_block succeeds but the comment says "arrived by range sync or through this node's own API counts too." What if import_block fails after this point? Looking at the context, this appears to be after successful import (the log says "Block imported successfully"), so this is fine.

Actually, re-reading: the log is at line 2768-2771, and this code follows it. The block is already imported. Good.

crates/net/p2p/src/beacon/verdict.rs

Line 150-157: forward method

fn forward(self, server: &P2PServer, received_at: Instant, outcome: Outcome) {
    if outcome == Outcome::Accept {
        record_liveness(server, &self);
    }
    if let Self::Aggregate { aggregate, .. } = &self
        && outcome == Outcome::Accept
    {

The liveness recording is now done for all Accept outcomes, not just aggregates. This is correct per the new behavior.

Line 208-231: record_liveness function

fn record_liveness(server: &P2PServer, object: &Validated) {
    let observed = server.store.observed_liveness();
    match object {
        Validated::Aggregate {
            aggregate,
            attesting_indices,
        } => {
            let (epoch, _root) = aggregate.target();
            let aggregator = std::iter::once(aggregate.aggregator_index());
            observed.record_all(epoch, aggregator.chain(attesting_indices.iter().copied()));
        }
        Validated::Attestation { attestation, .. } => {
            observed.record(attestation.data.target.epoch, attestation.attester_index);
        }
        Validated::Block { .. } | Validated::Column(_) => {}
    }
}

Issue at line 221: For Validated::Attestation, uses attestation.attester_index. Need to verify this field exists and is correct for the attestation type. Assuming electra_attestation returns a SingleAttestation or similar with this field, this seems correct.

Issue at line 226: Validated::Block and Validated::Column are no-ops. The comment in liveness.rs says "imported blocks' proposers" are recorded by the chain actor, which is correct. Columns (DAS) don't have proposers to record.

Line 771-824: Tests
Tests verify aggregator + attesters are marked live for aggregates, single attester for subnet attestations, and nothing for non-accept outcomes. Good coverage.

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

Line 117-120: Attestation liveness in post_pool_attestations

store
    .observed_liveness()
    .record(attestation.data.target.epoch, validator);

Recorded before pool insertion, which is correct. The comment explains this covers the node's own validator clients.

Line 315-319: Aggregate liveness in post_aggregate_and_proofs

let (epoch, _root) = aggregate.target();
store
    .observed_liveness()
    .record_all(epoch, std::iter::once(aggregator).chain(attesting_indices));

This now uses attesting_indices returned from aggregate::stateful_checks. Let me verify this return type change...

Line 308-309:

let checked = aggregate::cheap_checks(&seen, &store, &aggregate, now_ms)
    .and_then(|()| aggregate::stateful_checks(&store, &aggregate));

Previously: .map(|_| ()) — discarded the return value. Now stateful_checks returns attesting_indices which is used. Need to verify stateful_checks actually returns Vec<ValidatorIndex> or similar iterable.

Assuming this is correct based on the .chain(attesting_indices) usage.

Line 739-799: Tests
Good coverage for attestation and aggregate liveness, including refusal cases.

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

Line 164-253: liveness function

Line 177-180: Epoch validation

let current = get_current_store_epoch(store, &store.config());
if epoch + 1 < current || epoch > current + 1 {
    return Err(ApiError::BadRequest(
        "epoch is not the previous, current or next epoch",
    ));
}

This allows epoch in [current-1, current+1]. The comment says "previous, current and next epoch". Let's verify:

• epoch + 1 < current → epoch < current - 1 → rejects epochs before previous
• epoch > current + 1 → rejects epochs after next

So allowed: current-1, current, current+1. Correct.

Line 182: Head state retrieval

let (_head_root, state) = head(store)?;

Uses head helper which returns the head block root and state. The state is used for participation flags and validator lookup.

Line 184-196: Participation flags

let participation = state
    .altair_validator_lists()
    .ok()
    .and_then(|(previous, current, _)| {
        if epoch == state_epoch {
            Some(current)
        } else if epoch + 1 == state_epoch {
            Some(previous)
        } else {
            None
        }
    });

This gets participation flags for the state's own epoch and previous epoch. For epoch == state_epoch, uses current; for epoch + 1 == state_epoch (i.e., epoch == state_epoch - 1), uses previous.

What about epoch == state_epoch + 1 (next epoch)? Returns None, so no participation flags. Correct — the next epoch hasn't happened yet, so no flags exist.

Line 199-208: Validator check and response building

for index in indices {
    state
        .validator(index)
        .map_err(|_| ApiError::BadRequest("unknown validator index"))?;
    let credited = participation
        .and_then(|flags| flags.get(index as usize))
        .is_some_and(|flags| *flags != 0);
    data.push(Liveness {
        index,
        is_live: credited || observed.is_live(epoch, index),
    });
}

Checks validator exists in state, then ORs participation flags with observed liveness. Correct.

Line 1141-1273: Tests
Comprehensive tests for participation flags, observed liveness, epoch boundaries, error cases.

Line 1155: credited_state test helper

fulu.current_epoch_participation[2] = 0b001;
fulu.previous_epoch_participation[3] = 0b111;

Sets specific participation flags for testing.

Line 1179-1186: post_liveness test helper

async fn post_liveness(
    store: Store,
    epoch: u64,
    indices: &[&str],
    sync_status: SyncStatusController,
) -> (StatusCode, serde_json::Value) {

Note: indices is &[&str] which gets serialized to JSON array of strings. The endpoint parses these as Vec<String>. Good.

Security and Consensus Considerations

1. Doppelganger protection correctness: The endpoint correctly ORs on-chain participation with off-chain observation. This is the node's own view per Beacon API spec, not a consensus value.

2. No amplification risk: Recording liveness doesn't affect consensus, only this local endpoint's response.

3. Memory bound: MAX_TRACKED_INDEX and RETAINED_EPOCHS limit memory to ~3 * 2MB = 6MB worst case.

4. Sync status check: Returns 503 when syncing, preventing false negatives that could trigger doppelganger false positives.

Issues Found

Issue 1: record_all epoch floor calculation can be surprising

File: crates/storage/src/liveness.rs, lines 55-62

let newest = epochs
    .keys()
    .next_back()
    .copied()
    .unwrap_or(epoch)
    .max(epoch);

If the BTreeMap is empty, unwrap_or(epoch) is used, then .max(epoch) is redundant. If not empty, newest is the max of the existing newest and epoch. This is correct but the logic could be clearer with:

let newest = epochs.keys().next_back().copied().unwrap_or(epoch).max(epoch);

Actually this is fine as-is, just slightly verbose. Not a bug.

Issue 2: is_live doesn't check MAX_TRACKED_INDEX

File: crates/storage/src/liveness.rs, line 78-82

pub fn is_live(&self, epoch: Epoch, validator: ValidatorIndex) -> bool {

If validator >= MAX_TRACKED_INDEX, this returns false due to bits.get((validator / 64) as usize) returning None. This is safe (conservatively returns not live), but inconsistent with record which silently ignores. Consider documenting this or adding an explicit check for clarity.

Issue 3: Potential integer truncation in validator / 64 as usize

File: crates/storage/src/liveness.rs, lines 66, 81

ValidatorIndex is u64. (validator / 64) as usize could truncate on 32-bit platforms. However, given MAX_TRACKED_INDEX = 1 << 24, this is bounded and safe in practice. The project likely targets 64-bit.

Issue 4: liveness function uses epoch + 1 < current check

File: crates/net/rpc/src/beacon/validator.rs, line 178

if epoch + 1 < current || epoch > current + 1 {

If current is u64::MAX, current + 1 wraps. However, get_current_store_epoch returns a reasonable epoch number. Not a practical concern.

Issue 5: Test uses Default::default() for SyncStatusController

File: crates/net/rpc/src/beacon/validator.rs, lines 1199, 1208, etc.

post_liveness(store.clone(), EPOCH, &["2", "3"], Default::default()).await;

Verify SyncStatusController::default() returns SyncStatus::Synced. If it returns Syncing, tests would fail. This appears to work based on the test expectations.

Positive Observations

1. Good separation of concerns: ObservedLiveness is a clean, testable unit with clear invariants.

2. Proper Arc sharing: The Store correctly shares ObservedLiveness across clones.

3. Comprehensive tests: Each component has unit tests, including edge cases (refused attestations, out-of-window epochs, unknown validators).

4. Clear documentation: Comments explain why liveness is recorded where it is (gossip doesn't deliver own messages).

5. Conservative defaults: is_live returns false for unknown/out-of-range validators, which is safe for doppelganger protection.

Summary

Severity	Count	Description
Critical	0	None found
High	0	None found
Medium	0	None found
Low/Info	2	Minor clarity improvements possible

The PR is well-implemented and correct. The liveness tracking properly covers all paths (gossip aggregates, gossip attestations, block proposers, API submissions) with appropriate epoch windowing and memory bounds. The endpoint correctly combines on-chain participation with off-chain observation for doppelganger protection.

Optional suggestions:

1. File: crates/storage/src/liveness.rs, line 78: Add explicit validator < MAX_TRACKED_INDEX check in is_live for symmetry with record_all, or document the implicit false return.

2. File: crates/net/rpc/src/beacon/validator.rs, line 178: Consider epoch.saturating_add(1) < current for absolute safety, though not practically needed.

---

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