feat: add attestation aggregate coverage metrics

[pablodeymo]

Description / Motivation

Ports two upstream changes that together add attestation aggregate coverage observability:

• leanSpec #735: metric registration (mirrors zeam #898).
• zeam #876: per-slot coverage computation and emission.

After this PR, all 14 coverage series receive real per-slot updates from chain activity.

What Changed

Registration (crates/blockchain/src/metrics.rs)

• Two pub const &[&str] label-set constants — single source of truth for sections and directions.
• Three IntGaugeVec statics:
  • lean_attestation_aggregate_coverage_validators{section, subnet} — subnet="combined" is the section total; subnet="subnet_N" is per-subnet coverage (lazy).
  • lean_attestation_aggregate_coverage_subnets{section} — covered-subnet count per section.
  • lean_attestation_aggregate_coverage_diff_validators{direction} — symmetric difference between block-included and locally-aggregated pre-merge aggregates.
• init() forces the statics and seeds 14 default zero-valued series.

Sections: timely, late, block, combined, agg_start_new, proposal_combined.
Directions: block_only, timely_only.

Emission (4 sites)

Every per-slot section is keyed by the attestation data.slot (the voting round being reported), so timely/late/block/combined describe the same cohort (matching zeam #876).

• accept_new_attestations (blockchain/store.rs) — before each promote, snapshots new_payloads participant bits, tagged per entry with their data.slot, stashed on the Store. Read at the next slot boundary to populate the timely section ("prev_new" in zeam).
• on_tick interval 1 (blockchain/lib.rs) — emits the post-block report for slot - 1. Fired at interval 1, not 0, so the block carrying that round's votes (proposed at interval 0 of this slot) has typically been received and processed, letting the block section observe the same round. Computes timely (pre-merge snapshot filtered to the round), late (current new_payloads for the round), block (attestations in the canonical head block filtered to the round — canonical by construction, so a fork block can't poison it), and combined (their union); then emits diff_validators as the symmetric difference between block and timely. Emits nothing when no section has coverage for the round, so a missed slot doesn't surface as a misleading block_only=0, timely_only=N.
• start_aggregation_session (blockchain/lib.rs) — emits agg_start_new from current new_payloads right before fork-choice aggregation at interval 2.
• propose_block (blockchain/lib.rs) — emits proposal_combined (the full set of validators included across the block's aggregated attestations) after the block is built. (A payload-vs-gossip split was considered but dropped: ethlambda builds blocks exclusively from known_aggregated_payloads, so every included validator is payload-sourced by construction — proposal_gossip would always be 0 and proposal_payloads would always equal proposal_combined.)

Supporting changes

• The three emitters live as private free fns at the bottom of blockchain/src/lib.rs. They use Vec<bool> locals (seen for validators, has_subnet for subnets) plus three small helpers (cov_add, cov_record, or_into). Subnet derivation is vid % committee_count, matching the gossip subnet assignment in crates/net/p2p/src/lib.rs.
• Store gets a single CoverageSnapshot field (pre_merge_new_coverage) holding (data.slot, AggregationBits) entries captured before each promote — only participant bits, no proof duplication. The block section needs no stored state: it is read from the canonical head block at report time.
• BlockChain::spawn now takes attestation_committee_count (resolved in main.rs from CLI > validator-config.yaml > 1; previously only known to P2P for subnet subscriptions).

Operator interpretation of diff_validators

The aggregation pipeline produces two pools for the same slot: timely (locally aggregated pre-merge) and block (what the proposer included). The diff counts validators in the symmetric difference:

• block_only persistently high → this node was slow to receive/aggregate via gossip; proposer had a better view.
• timely_only persistently high → proposer omitted attestations the network had time to gossip.
• Both near zero → local aggregation tracks proposers; the network is converging.

Correctness / Behavior Guarantees

• No fork-choice or state-transition change. Coverage snapshots are pure observability and do not feed back into block processing.
• The diff_validators help text intentionally diverges from upstream's terse phrasing to spell out the symmetric-difference semantics. Metric name, labels, and values are unchanged; dashboards built against any client's schema work as-is.
• Cardinality: 14 series at registration; per-subnet series appear lazily when written. Even with full per-subnet instrumentation at 64 subnets, the validators gauge tops out at 6 × 65 = 390 series.

Tests Added / Run

No new unit tests. The earlier draft of this PR included unit tests for an internal Coverage bitset wrapper; once the wrapper was inlined as Vec<bool> locals, those tests were exercising trivial iterator ops rather than the emission contract, so they were dropped.

Commands run:

• cargo fmt --all -- --check — clean
• make lint (clippy -D warnings) — clean
• cargo test -p ethlambda-blockchain --release --lib --bins — 21 passed
• cargo test -p ethlambda-storage --release — 38 passed
• cargo test -p ethlambda-blockchain --release --test forkchoice_spectests — 62 passed, 8 failed (all AttestationTooFarInFuture, pre-existing fixture flakes on main, unrelated to this PR).

Related Issues / PRs

• Ports: metrics: add attestation aggregate coverage gauges with subnet labels leanEthereum/leanSpec#735
• Mirrors: Label aggregate coverage metrics by slot and subnet blockblaz/zeam#898 + log attestation subnet coverage in chain status blockblaz/zeam#876

✅ Verification Checklist

• Ran make fmt — clean
• Ran make lint (clippy with -D warnings) — clean
• Ran cargo test --workspace --release — passing modulo 8 pre-existing forkchoice fixture flakes documented above (unrelated to this PR)

[github-actions]

🤖 Kimi Code Review

This PR adds attestation aggregate coverage metrics following the existing patterns in the codebase. The implementation is correct and idiomatic.

Assessment:

1. Code correctness: The use of std::sync::LazyLock for static initialization and the force() pattern in init() is consistent with the existing metrics in the file. Label value construction (&[section, "combined"]) is correct.

2. Cardinality management: Item 1 (lines 469-488) correctly seeds only the combined subnet series at startup, deferring per-subnet series to lazy creation as noted in the comment. This prevents unbounded cardinality explosions in Prometheus.

3. Error handling: The .unwrap() calls on register_int_gauge_vec! (lines 131, 141, 152) match the existing codebase's fail-fast approach for metrics registration failures. This is acceptable for initialization-time panics.

4. Documentation: Metric help strings (lines 129-130, 143-144, 165-168) clearly describe the label semantics and update frequency, including the important distinction between subnet=combined and subnet=subnet_N.

Minor observation:

• The ATTESTATION_AGGREGATE_COVERAGE_SECTIONS slice (lines 10-20) contains 8 sections. Ensure the instrumentation code that actually writes these metrics uses the same string literals to avoid label mismatch errors at runtime.

No security vulnerabilities or consensus risks identified—this is pure instrumentation with no state transition logic.

Verdict: LGTM. Clean, well-documented addition that follows established patterns.

---

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

[github-actions]

🤖 Codex Code Review

No findings.

This PR is limited to new Prometheus label constants and gauge registration in metrics.rs and startup seeding in metrics.rs. I don’t see any impact on fork choice, attestation validation, justification/finalization, state transition, XMSS handling, or SSZ behavior. The added code is straightforward, uses the existing LazyLock/Prometheus patterns consistently, and doesn’t introduce new memory-safety or error-handling concerns beyond the existing .unwrap() registration style already used in this module.

One residual note: this patch only registers/seeds the new metrics; there are no writer APIs or call sites in this PR, so the metrics will stay at their seeded values until follow-up instrumentation lands. I also couldn’t complete cargo check in this environment because Cargo/Rustup cannot write to /home/runner/.cargo and /home/runner/.rustup, so the review is based on static inspection rather than a successful local build.

---

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

[github-actions]

🤖 Claude Code Review

Here is the full code review:

---

PR #386 — feat: add attestation aggregate coverage metrics

Overview

Ports leanSpec PR #735: registers three IntGaugeVec Prometheus metrics for attestation aggregate coverage, pre-seeding 18 zero-valued series in init() so dashboards render from node startup. Registration-only — no instrumentation/setter functions yet (coming in a follow-up). Single file changed: crates/blockchain/src/metrics.rs.

---

Code Quality

Correct and idiomatic. The three statics follow the established LazyLock + register_*_vec! + .unwrap() pattern exactly. IntGaugeVec is the right choice — integer counts, consistent with all other labeled gauges (LEAN_NODE_INFO, LEAN_TABLE_BYTES, LEAN_NODE_SYNC_STATUS). Labels are compact and match the upstream schema, so cross-client Grafana dashboards remain compatible.

---

Specific Observations

1. Redundant force() calls before seeding loops (lines 472–474)

std::sync::LazyLock::force(&LEAN_ATTESTATION_AGGREGATE_COVERAGE_VALIDATORS);
std::sync::LazyLock::force(&LEAN_ATTESTATION_AGGREGATE_COVERAGE_SUBNETS);
std::sync::LazyLock::force(&LEAN_ATTESTATION_AGGREGATE_COVERAGE_DIFF_VALIDATORS);
for &section in ATTESTATION_AGGREGATE_COVERAGE_SECTIONS {
    LEAN_ATTESTATION_AGGREGATE_COVERAGE_VALIDATORS
        .with_label_values(&[section, "combined"])
        .set(0);
    ...
}

Calling .with_label_values(...).set(0) forces the LazyLock internally, so the preceding explicit force() calls are redundant. They're harmless and consistent with the existing style (every static gets an explicit force() in init()), so this is fine to keep — just worth knowing it doesn't add behavior.

2. pub const label sets vs. SyncStatus::ALL inconsistency

SyncStatus::ALL is a private const on the enum and is only used internally in set_node_sync_status(). The new ATTESTATION_AGGREGATE_COVERAGE_SECTIONS and ATTESTATION_AGGREGATE_COVERAGE_DIFF_DIRECTIONS are pub const at module scope. The pub is justified because setter functions in the follow-up PR may need these from call sites outside metrics.rs (e.g., store.rs) — so this is forward-looking but reasonable. Consider keeping them pub(crate) if they won't be exported from the crate's public API, to limit exposure.

3. LEAN_NODE_SYNC_STATUS is not seeded in init() — pre-existing inconsistency

The existing LEAN_NODE_SYNC_STATUS is force()d in init() but its label values are not pre-seeded with zero; they appear only after the first set_node_sync_status() call. The new coverage metrics are seeded up front, which is the correct behavior per the PR's stated goal ("appear at /metrics from node startup"). No action needed in this PR, but the existing LEAN_NODE_SYNC_STATUS inconsistency could be addressed separately.

4. The three force() calls for all three statics happen before either seeding loop

LEAN_ATTESTATION_AGGREGATE_COVERAGE_DIFF_VALIDATORS is forced on line 474, but its seeding loop doesn't start until line 483. No bug, but co-locating each force() with its respective seeding block would make the grouping clearer. Again, a trivial style point given the existing init() structure.

---

Documentation

Excellent. The doc comments are precise and operationally useful:

• The distinction between subnet=combined (section total) and subnet=subnet_N (per-subnet) is clearly stated.
• The diff_validators help text spelling out symmetric-difference semantics is better than the upstream terse version and doesn't break dashboard compatibility.
• The init() comment correctly explains the lazy-seeding strategy.

---

Correctness / Safety

• No behavior change: confirmed. State transition, fork choice, attestation processing — all untouched.
• Cardinality: 18 seeded series now; 520 max at 64 subnets — well within Prometheus limits.
• No panics introduced: the register_*!(...).unwrap() calls are consistent with all existing statics and fail only at program startup (acceptable for misconfigured registries).
• No security concerns.

---

Verdict

Approved. The implementation is correct, follows project conventions faithfully, and is appropriately scoped. The two actionable points worth considering before merge:

• Consider pub(crate) instead of pub for the label-set constants if they won't be part of the crate's public API.
• Track the LEAN_NODE_SYNC_STATUS zero-seeding gap as a follow-up (not a blocker for this PR).

---

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

[greptile-apps]

Greptile Summary

Registers three new IntGaugeVec Prometheus metrics for attestation aggregate coverage, porting leanSpec#735. The change is registration-only — no code path yet produces or consumes the new gauges.

• Three new statics follow the exact same LazyLock<IntGaugeVec> pattern used by every other labeled gauge in the file.
• init() forces all three and seeds 18 zero-valued series, keeping per-subnet series lazy for the follow-up instrumentation PR.
• Two pub const &[&str] label-set constants are exported so the follow-up setter functions can reference them without hard-coding strings.

Confidence Score: 5/5

This PR is safe to merge — it is purely additive metric registration with no changes to any logic, state, or existing code paths.

The change adds three new IntGaugeVec statics and seeds 18 default zero series in init(). No existing code is modified, no consumers of the new metrics exist yet, and the implementation closely mirrors the patterns already established in the file.

No files require special attention. The single changed file is straightforward metric registration.

Important Files Changed

Filename	Overview
crates/blockchain/src/metrics.rs	Adds three IntGaugeVec statics and two pub label-set constants; seeds 18 default zero series in init(). Follows all existing patterns; no logic, no consumers yet.

_{Reviews (1): Last reviewed commit: "feat: add attestation aggregate coverage..." | Re-trigger Greptile}

[MegaRedHand]

We still need to emit the metrics

[pablodeymo]

Instrumentation added in 855f56d. Five emission sites now write to the 18 series registered in e9a04f7:

• timely / late / block / combined / diff_validators — emitted at the slot boundary (on_tick interval 0) for slot - 1, sourcing from a pre-merge snapshot of new_payloads, current late arrivals, and the last-imported block's aggregation bits.
• agg_start_new — emitted right before fork-choice aggregation runs (interval 2).
• proposal_payloads / proposal_gossip / proposal_combined — emitted from propose_block after produce_block_with_signatures, classifying validators in the proposed block as known-payload-covered vs. gossip-only.

Coverage lives in a new crates/blockchain/src/coverage.rs (~280 lines, 6 unit tests covering add_bits, merge_from, diff_counts, plus empty/zero/out-of-range edge cases). Subnet derivation uses vid % committee_count to match the gossip subnet assignment in crates/net/p2p/src/lib.rs:241.

Store gets a small CoverageSnapshot type and two Arc<Mutex<Option<…>>> fields — no proofs duplicated, only AggregationBits. The pre-merge capture happens inside accept_new_attestations just before promote_new_aggregated_payloads, so consumer-side timing concerns stay in the existing tick path.

BlockChain::spawn gains an attestation_committee_count: u64 parameter — only bin/ethlambda/src/main.rs calls it, and the value was already resolved there (CLI > validator-config.yaml > 1).

Ready for another look.

[pablodeymo]

@MegaRedHand please re-review the PR

[MegaRedHand]

I think we should move these out of the storage crate