Skip to content

chore(deps): bump Lighthouse to unstable HEAD e58ec88fe and migrate sign_attestations to SingleAttestation - #1215

Merged
mergify[bot] merged 4 commits into
sigp:epbsfrom
shane-moore:chore/lh-pin-e58ec88fe-single-attestation
Aug 7, 2026
Merged

chore(deps): bump Lighthouse to unstable HEAD e58ec88fe and migrate sign_attestations to SingleAttestation#1215
mergify[bot] merged 4 commits into
sigp:epbsfrom
shane-moore:chore/lh-pin-e58ec88fe-single-attestation

Conversation

@shane-moore

@shane-moore shane-moore commented Aug 4, 2026

Copy link
Copy Markdown
Member

Problem, Evidence, and Context (Required)

Change Overview (Required)

  • Bump the 16 Lighthouse crate revs to e58ec88fe and mirror Lighthouse's six EIP-7688 [patch.crates-io] git pins into our root workspace (Cargo only applies patches from the root workspace).
  • Migrate validator_store to the new signing interface: AttestationToSign is no longer generic and carries only AttestationData; sign_attestations now returns SingleAttestation batches. Duty identity fields (attester_index, committee_index) are compared against our own metadata as a diagnostic only (warn/info log plus a reason-labelled counter); publication proceeds regardless, since the fields are not part of the signing root and the beacon node validates them authoritatively. A duty cannot be dropped before signature collection because the committee's partial-signature batch size is exact and dropping would stall the whole committee.
  • Add two fork-aware decode helpers on DataVersion in ssv_types (one for Attestation, one for AggregateAndProof). Gloas decodes the EIP-7688 progressive shapes; Heze fails closed until a wire shape is pinned upstream. All four fork-conditional decode sites now go through them.
  • Bind the leader-supplied version to our own candidate in both aggregator validators, matching the existing proposer check.
  • Merkleize BlindedExecutionPayloadEnvelope as a progressive container so blinded/full root parity holds at the new pin (SIP-94 section 6).
  • Smaller adaptations: Heze DataVersion arms, EnrExt dialable_multiaddrs_* rename.
  • Reading order: Cargo.toml, then ssv_types/src/consensus.rs (helpers, version binding, envelope), then validator_store/src/lib.rs (migration), then tests.
  • Not changed: SSV wire encodings, QBFT value hashes, and partial-signature messages are byte-identical (EIP-7688 progressive types serialize the same as their positional counterparts; only merkleization differs). Pre-Gloas networks behave exactly as before.

Risks, Trade-offs, and Mitigations (Required)

  • On Gloas networks, operators computing progressive signing roots and operators computing positional ones would split threshold-signature reconstruction on identical decided bytes. Rollout must be lockstep on Gloas devnets. No cross-client risk: go-ssv cannot represent versions past Fulu.
  • The patched SSZ crates apply workspace-wide, so they could in principle shift our own wire encodings. The ssv-spec fixture tests act as the tripwire and pass unchanged.
  • The patch mirror must be re-diffed against Lighthouse's on every future pin bump (marked with a FIXME).

Validation (Required)

  • Full workspace test suite green, including ssv-spec fixtures.
  • New tests: identity mismatch reaches the slashing DB, is published with the duty's identity fields echoed verbatim, and does not stall the committee; per-fork decode shape selection and Heze fail-closed; identical bytes decoded as Electra and Gloas shapes produce different roots (EIP-7495 serialization compatibility); version-binding rejections in both validators; max-size Gloas aggregate fits the 131,308-byte bound; blinded/full envelope root parity against the pinned Lighthouse type.
  • cargo fmt, clippy, and cargo check --workspace --all-targets clean.

Rollback (Required for behavior or runtime changes; optional otherwise)

  • Revert the single commit. No config, data, or DB migrations. Only relevant before any Gloas devnet is running.

Blockers / Dependencies (Optional)

  • N/A

…ign_attestations to SingleAttestation

Bump the Lighthouse pin from 81d576943 to e58ec88fe (EIP-7688 merge) and
adapt to the interface changes in the window:

- Mirror Lighthouse's six EIP-7688 [patch.crates-io] git pins into the
  root workspace so the patched SSZ stack applies.
- Migrate the validator_store attestation path to the de-generified
  AttestationToSign / SingleAttestation interface (LH #9578), with
  duty-identity checks that withhold mismatched duties from publication
  without stalling the committee's exact-count signature collection.
- Add fork-aware decode helpers on DataVersion (EIP-7688 progressive
  shapes at Gloas, Heze fails closed) and bind leader-supplied versions
  to our own candidate in both aggregator validators (SIP-94).
- Merkleize BlindedExecutionPayloadEnvelope as a progressive container
  to preserve blinded/full root parity at the new pin (SIP-94).
- Fill Heze DataVersion arms and adopt the EnrExt dialable_multiaddrs
  rename.

Closes sigp#1214
@codecov-commenter

codecov-commenter commented Aug 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.59016% with 33 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.11%. Comparing base (01f19c4) to head (147af27).

Files with missing lines Patch % Lines
anchor/common/ssv_types/src/consensus.rs 95.55% 14 Missing ⚠️
anchor/validator_store/src/lib.rs 77.19% 13 Missing ⚠️
...tore/src/testing/committee_attestation_slashing.rs 97.85% 3 Missing ⚠️
anchor/network/src/peer_manager/discovery.rs 0.00% 2 Missing ⚠️
anchor/spec_tests/src/utils/error_codes.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             epbs    #1215      +/-   ##
==========================================
+ Coverage   69.67%   70.11%   +0.44%     
==========================================
  Files         165      166       +1     
  Lines       32475    32959     +484     
==========================================
+ Hits        22626    23109     +483     
- Misses       9849     9850       +1     
Flag Coverage Δ
rust 70.11% <94.59%> (+0.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shane-moore
shane-moore marked this pull request as ready for review August 5, 2026 03:11
@shane-moore

Copy link
Copy Markdown
Member Author

@claude, please take a look

@claude-code-actions-sigp

claude-code-actions-sigp Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @shane-moore's task in 5m 42s —— View job


Review: PR #1215 — Lighthouse pin bump + SingleAttestation migration

  • Gather context (diff vs origin/epbs)
  • Review Cargo.toml / patch mirror
  • Review ssv_types/src/consensus.rs (decode helpers, version binding, envelope merkleization)
  • Review validator_store/src/lib.rs (SingleAttestation migration, identity-mismatch handling)
  • Review tests
  • Post review feedback

I read the full diff against origin/epbs. This is a carefully-scoped, well-documented change and I did not find any correctness, safety, or security issues. No inline comments to raise. Details below.

What I verified

Fork-aware decode helpers (consensus.rs)wire_shape() is the single source of truth for the fork→shape mapping, and both decode_attestation / decode_aggregate_and_proof route through it. The match is exhaustive and Heze fails closed (UnsupportedFork) rather than silently mis-decoding. for_attestation_shape covers all three Attestation<E> variants, replacing the previous non-exhaustive Base/Electra inline match. The EIP-7495 serialization-compatibility hazard (identical Electra bytes decode under both shapes with distinct roots) is exactly the risk this addresses, and it's pinned by a dedicated test.

Version binding — Aggregator branches in both ProposerConsensusDataValidator::do_validation and AggregatorCommitteeDataValidator::validate now bind the leader-supplied version to our own candidate before decoding, matching the existing proposer check. The asymmetry (committee check in validate, not do_validation) is deliberate and documented: do_validation is the ssv-spec parity surface. error_codes.rs maps the new VersionMismatch to UNMAPPED_ERROR_CODE (no go-ssv equivalent), keeping the match exhaustive.

SingleAttestation migration (validator_store/src/lib.rs) — The identity-mismatch design is sound: attester_index/committee_index aren't part of the signing root, so a mismatched duty still signs (must, since the committee's partial-signature batch size is exact) and still lands in the slashing DB, but is withheld from publication via the publishable flag on AttestationCandidate. The check fails closed when validator.index is None. AggregateSignature::from(&signature) correctly wraps the reconstructed threshold signature into the single-entry aggregate SingleAttestation expects.

Progressive envelopeBlindedExecutionPayloadEnvelope gains struct_behaviour = "progressive_container" with active_fields(1,1,1,1,1), matching its 5 fields, preserving blinded/full root parity at the new pin (SIP-94 §6). Root parity is asserted in tests.

Cargo patch mirror — The six EIP-7688 [patch.crates-io] git pins mirror Lighthouse's own set (needed because Cargo only applies patches from the root workspace). The FIXME(eip-7688) correctly flags that this must be re-diffed on every future pin bump.

Tests — Comprehensive and falsifiable: per-fork shape selection, Heze fail-closed (both containers), identical-bytes-distinct-roots for both Attestation and AggregateAndProof (incl. signing root), version-binding accept/reject in both validators, max-size Gloas aggregate fits the 131,308-byte bound, and the new slashing-protection suite (first-time / same-data / conflicting / identity-mismatch-recorded-but-withheld, with committee-stall guard). Prod paths use no new unwrap/expect.

Minor, optional (non-blocking)

  • In slashing_protection_attestations, the withheld-but-slash-safe branch increments SIGNED_ATTESTATIONS_TOTAL with the OTHER_ERROR label (lib.rs:1987). The sign itself succeeded — it's only withheld — so this may slightly inflate error-rate dashboards. It's a deliberate, commented choice; flagging only in case a dedicated withheld/identity-mismatch label would read better in metrics.

CI

Prior commit (01f19c4) is green across test-suite, coverage, local testnet, and CLI check. The current run is still in progress — worth confirming it lands green before merge.

@shane-moore

Copy link
Copy Markdown
Member Author

@claude Applied the metrics nit in fa585db: the identity-mismatch withheld branch now increments SIGNED_ATTESTATIONS_TOTAL with a dedicated withheld label instead of other_error.

@claude-code-actions-sigp

claude-code-actions-sigp Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @shane-moore's task in 33s —— View job


Follow-up: withheld metrics label (fa585db)

Confirmed the fix addresses the metrics nit from my earlier review.

  • Inspect fa585db6 diff
  • Verify label applied to correct branch
  • Confirm no stale other_error references remain

What changed

  • metrics.rs:13 — new pub const WITHHELD: &str = "withheld", with a doc comment explaining it means signed + recorded in the slashing DB but withheld from publication (e.g. attestation duty identity mismatch).
  • lib.rs:1987 — the withheld-but-slash-safe branch now increments SIGNED_ATTESTATIONS_TOTAL with metrics::WITHHELD instead of metrics::OTHER_ERROR.

Verification

  • The relabeled counter sits in the Ok(()) (non-publishable) arm at lib.rs:1982-1989, which is exactly the identity-mismatch path — the sign succeeded and was recorded, only publication is withheld. Correct branch.
  • The Ok(()) if candidate.publishable arm still records SUCCESS, and genuine error arms (SameData, Slashable, etc.) are untouched, so error-rate dashboards no longer absorb withheld attestations.
  • No stale references: other_error is no longer used for this path, and WITHHELD is referenced only where intended.

Looks good — this resolves the nit cleanly. No further concerns on this change.

…of withholding

The duty's attester_index/committee_index are not part of the signing
root and the beacon node validates them authoritatively on publish, so
a true mismatch cannot land on chain, while withholding could suppress
a valid attestation whenever the slot-start snapshot went stale
mid-slot (dependent-root change).

The comparison stays as a diagnostic: warn for attester_index
mismatches (indices are permanent, so always a real fault), info for
committee drift, plus a reason-labelled
anchor_attestation_duty_identity_mismatches_total counter. Removes
AttestationCandidate, the publishable flag, and the withheld metrics
label; every slash-safe attestation is returned for publication and
slashing-DB recording is unchanged.
@shane-moore

Copy link
Copy Markdown
Member Author

@claude, please review the new commit 147af27: the identity-mismatch action flips from withhold-from-publication to publish-with-diagnostic. The identity fields are not part of the signing root and the beacon node validates them authoritatively, so withholding could only suppress valid attestations when the slot-start snapshot went stale mid-slot. The withheld label from the earlier metrics nit is gone; mismatches now count in anchor_attestation_duty_identity_mismatches_total{reason} (warn for attester_index, info for committee drift). PR body updated to match.

@claude-code-actions-sigp

claude-code-actions-sigp Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude finished @shane-moore's task in 2m 25s —— View job


Review: commit 147af27 — identity mismatch flips to publish-with-diagnostic

  • Inspect the commit diff
  • Review validator_store/src/lib.rs mismatch handling
  • Review metrics.rs (new counter, withheld removal)
  • Review test changes
  • Post review feedback

I reviewed 147af278 against origin/epbs. The change is clean, well-reasoned, and I found no correctness, safety, or security issues. No inline comments to raise.

What I verified

Design rationale is sound. attester_index/committee_index are echoed verbatim into SingleAttestation but are not part of the signing root (att.data.signing_root(domain_hash) at lib.rs:1728 covers only data), and the beacon node validates them authoritatively at publication. So publishing a mismatched duty cannot produce an on-chain fault, while the previous withhold path could suppress a valid attestation when the slot-start snapshot went stale mid-slot (dependent-root change). Flipping to publish-with-diagnostic is the safer default.

Reason classification is correct and exhaustive (lib.rs:1665-1697). The if/else if chain assigns exactly one reason per duty (attester → missing-from-snapshot → committee → continue), and continue is the clean no-divergence path. Severity split matches the stated intent: attester_index warns (indices are permanent, always a real fault), everything else is info!. Note that a None validator.index falls into the attester branch (None != Some(..) is always true) → warn + publish, which is the right call since a missing local index is a real identity fault.

Simplification is a net win. AttestationCandidate and the publishable flag are gone; sign_committee_attestations now returns Vec<(SingleAttestation, PublicKeyBytes)> and slashing_protection_attestations collapses the two Ok(()) arms into one. Slashing-DB recording is unchanged — every slash-safe attestation is still recorded and now also returned. The diagnostic pass borrows &attestations before the later .into_iter() consume, so no ownership issue.

Metrics wiring is complete (metrics.rs:41-54). New anchor_attestation_duty_identity_mismatches_total{reason} counter with three reason constants; WITHHELD and its use site are fully removed. Confirmed no stale references anywhere in anchor/: AttestationCandidate, publishable, and WITHHELD all return zero hits. SIGNED_ATTESTATIONS_TOTAL now correctly records SUCCESS on the slash-safe arm — withheld attestations no longer skew that counter.

Test updated coherently (committee_attestation_slashing.rs). The renamed identity_mismatch_published_and_recorded_in_slashing_db now asserts both duties reach the publication batch (signed.len() == COMMITTEE_VALIDATOR_COUNT), the bad attester_index is echoed verbatim, the collector still sent the full committee batch (no stall), and the mismatched data still lands in the slashing DB (re-probe yields SameData). Assertions match the new behavior exactly.

Minor, optional (non-blocking)

  • missing_from_snapshot (pubkey entirely absent from the slot-start attesting snapshot) is logged at info! alongside benign committee drift. A pubkey missing from the snapshot altogether is arguably more suspicious than a committee-index shift — you may eventually want it at warn!. But since it can arise from the same stale-snapshot scenario the commit describes, grouping it as informational is defensible. Purely a log-level judgment call; the {reason} label already lets operators alert on it independently.

Verification

  • Confirmed imports (info, warn from tracing at lib.rs:64; metrics::inc_counter_vec re-exported via pub use metrics::*), symbol wiring, and zero stale references by grep. I did not run cargo check/tests in this session (compilation requires approval here) — CI on the PR is the authority; worth confirming the run lands green before merge.

Looks good — the flip is well-justified and the surrounding cleanup is a genuine simplification.

@mergify

mergify Bot commented Aug 7, 2026

Copy link
Copy Markdown

Queued — the merge queue status continues in this comment ↓.

@mergify

mergify Bot commented Aug 7, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 29 minutes 13 seconds in the queue, including 27 minutes 39 seconds running CI.

Required conditions to merge
  • check-success=cli-reference-check
  • check-success=run-local-testnet
  • check-success=test-suite-success

@mergify mergify Bot added the queued label Aug 7, 2026
@mergify
mergify Bot merged commit 36950b6 into sigp:epbs Aug 7, 2026
25 checks passed
@mergify mergify Bot removed the queued label Aug 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants