feat(validator_store): implement sign_payload_attestation

[shane-moore]

Problem, Evidence, and Context

Under Gloas/ePBS, PTC validators must sign payload attestations (SSV-side section 3 of SIP-94). Lighthouse's PayloadAttestationService calls sign_payload_attestation(pubkey, data) per validator, but Anchor's implementation is an Unsupported stub, so the duty cannot run. Both dependencies are merged to epbs: #1076 (PTC value object removal) and #1080 (Role::PTCAttester + PartialSignatureKind::PTCAttester).

Addresses #1077.

Change Overview

The method runs a SingleValidator partial-signature collection over the PayloadAttestationData LH passes in, signed under Domain::PTCAttester, mirroring the voluntary-exit flow (the one merged SingleValidator single-signature precedent). LH fetches the data once at the 75% slot cutoff and abstains on no-block, so the method only signs what it is handed.

Collection failures are classified for telemetry before the error is surfaced, because LH crit-logs whatever we return and the trait has no abstain path: a no-threshold miss (surfacing as QueueClosedError via collector eviction) warns and increments anchor_ptc_reconstruction_failures_total{reason="no_signature"}; local faults count as reason="infra". Errors from the method's own validator/cluster/index resolution propagate untouched; non-collection errors raised inside collect_signature (share lookup, key decryption, threshold arithmetic) are error-logged for context but emit no metric.

Suggested reading order:

• lib.rs: sign_payload_attestation (the duty) and report_ptc_collection_failure (the effects)
• instrumentation.rs: pure failure classifier, next to the existing failure_reason mapper
• metrics.rs: the new counter
• testing/: harness extensions (failure injection, slashing flag, richer call capture) and the 5 new tests

Intentionally unchanged:

• No PayloadAttestationService spawning (feat(client): spawn LH PayloadAttestationService with AnchorValidatorStore backend #1078)
• No signature_collector changes; at current collector granularity a no-threshold miss and a genuine channel close are both QueueClosedError, so reason="no_signature" is an upper bound on true observation divergence. Clean measurement needs a collector change (separate follow-up issue).
• No is_synced gating: LH only schedules duties when synced. Flagging for reviewer confirmation.

Risks, Trade-offs, and Mitigations

• Blast radius is small: nothing calls this method until feat(client): spawn LH PayloadAttestationService with AnchorValidatorStore backend #1078 spawns the service.
• On a minority split, the call blocks until the collector cleaner evicts (about 2 slots) and LH awaits validators sequentially. This is a pre-existing property of collect_signature shared by all duties, not introduced here.
• Recoverable no-threshold misses still surface through LH at crit!; unavoidable Anchor-side given the trait shape. The metric, not the log, is the measurement channel.

Validation

cargo test -p anchor_validator_store: 44 passed (5 new). The new tests lock:

• message construction, and the signing root recomputed independently in the test (locks the Domain::PTCAttester choice, epoch derivation, and signed object), plus kind/role/slot wiring into the collector
• MissingIndex fails before any collection attempt
• QueueClosedError returns the original error and increments {reason="no_signature"}
• EmptySignature increments {reason="infra"} and leaves {reason="no_signature"} at zero delta, pinning that infra faults cannot leak into the divergence bucket
• slashing-DB tripwire: protection enabled against an empty slashing DB, so any future slashing check added to this path fails the test (payload attestations are not slashable)

make cargo-fmt and make lint clean.

Rollback

Revert the commit; the method returns to the Unsupported stub. No config, data, or operational impact (no caller yet; the metric simply disappears).

Blockers / Dependencies

None for merge. Follow-ups: #1078 (spawn the LH service) and a research issue for clean divergence measurement in signature_collector.

[shane-moore]

@claude, please take a look

[claude-code-actions-sigp]

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

---

Review of PR #1082 — feat(validator_store): implement sign_payload_attestation

• Read PR diff against origin/epbs
• Read changed files for context (lib.rs, instrumentation.rs, metrics.rs, testing/*)
• Cross-check against existing voluntary-exit precedent and Domain/Role wiring
• Verify slashing-DB tripwire and failure classifier
• Post review feedback

Summary

Implementation looks solid. The signing path mirrors collect_voluntary_exit_partial_signatures (lib.rs:821) one-for-one, the PartialSignatureKind::PTCAttester / Role::PTCAttester / Domain::PTCAttester triple is consistently wired, and the disable_slashing_protection: false tripwire test is a nice way to lock in "no slashing-DB access on this path" as a behavioral invariant rather than a comment. Test coverage on the happy path and MissingIndex/QueueClosedError failure paths is good.

A few observations below, none blocking.

Observations

1. Infra classification arm has no test coverage — instrumentation.rs:53-56

classify_ptc_collection_failure returns PtcFailureClass::Infra for QueueFullError, OwnOperatorIdUnknown, EmptySignature, and RecoverError(_). Only NoSignature is exercised in sign_payload_attestation_collection_failure_increments_no_signature_metric. A future refactor that swaps which variants land in which bucket would not be caught. Worth a parallel test that injects e.g. CollectionError::EmptySignature and asserts the infra metric increments.

2. validator_index is resolved twice — lib.rs:3315 and lib.rs:512

sign_payload_attestation resolves validator.index.ok_or(SpecificError::MissingIndex)? up front, and collect_signature resolves it again internally before building ValidatorSigningData. The PR comment justifies the early resolution ("must surface as a pre-collection error rather than routing through the collection-failure reporter"), which is reasonable — but worth noting that both call sites must stay in sync if the error variant ever changes.

3. Error-then-return pattern could use inspect_err — lib.rs:3325-3340

let signature = match self.collect_signature(...).await {
    Ok(signature) => signature,
    Err(err) => {
        self.report_ptc_collection_failure(&err, &validator_pubkey, data.slot);
        return Err(err);
    }
};

is equivalent to:

let signature = self
    .collect_signature(...)
    .await
    .inspect_err(|err| self.report_ptc_collection_failure(err, &validator_pubkey, data.slot))?;

Subjective; the explicit form is also fine.

4. Metric delta is racy across parallel tests — testing/payload_attestation.rs:158-167

The comment in the test already calls this out. cargo test runs binary-internal tests in parallel by default; the assertion no_signature_counter.get() - count_before == 1 is only safe today because this is the sole test touching the no_signature label. If a second test ever increments the same label, this becomes flaky. A .fetch_add-style pattern or per-test labels would be more robust if the PTC test surface grows.

5. report_ptc_collection_failure runs for all collect_signature errors, not just collection failures

The PR description says "pre-collection errors propagate untouched", but errors like Error::UnknownPubkey or KeyShareDecryptionFailed raised inside collect_signature (before sign_and_collect is called) do flow through the reporter as PtcFailureClass::NonCollection, which error!-logs them with the generic "Failed to sign payload attestation" message. No metric is emitted, so the operator-visible effect is limited to the log line. Worth aligning the description with the actual behavior, or adjusting so genuinely pre-collection errors short-circuit the reporter.

Verification

• I did not run make cargo-fmt, make lint, or make test in this review session — relying on the PR description's claim that cargo test -p anchor_validator_store passed (43 tests, 4 new) and make cargo-fmt / make lint are clean. Per verification.md these should be confirmed by CI before merge.
  · branch: ptc-sign-payload-attestation

[codecov-commenter]

Codecov Report

❌ Patch coverage is 97.60000% with 6 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (epbs@5eab764). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
...validator_store/src/testing/payload_attestation.rs	97.53%	4 Missing ⚠️
anchor/validator_store/src/instrumentation.rs	87.50%	1 Missing ⚠️
anchor/validator_store/src/lib.rs	97.67%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             epbs    #1082   +/-   ##
=======================================
  Coverage        ?   62.89%           
=======================================
  Files           ?      157           
  Lines           ?    26787           
  Branches        ?        0           
=======================================
  Hits            ?    16848           
  Misses          ?     9939           
  Partials        ?        0           

Flag	Coverage Δ
rust	62.89% <97.60%> (?)

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]

@claude, went through each review item:

1 (Infra arm coverage): Added sign_payload_attestation_infra_failure_increments_infra_metric in 2cad7fb: injects EmptySignature, asserts {reason="infra"} delta == 1 and {reason="no_signature"} delta == 0. The zero-delta half is the valuable direction: an infra variant drifting into the no_signature bucket would silently inflate the SIP-94 divergence upper bound.

2 (double index resolution): Leaving as is. Resolving validator.index at the duty site is the crate-wide pattern (six other call sites do the same outside collect_signature), and the early resolution is load-bearing: MissingIndex must surface before any collection attempt and bypass the failure reporter, which sign_payload_attestation_missing_index_errors pins.

3 (inspect_err): Keeping the explicit match. inspect_err has zero uses in the workspace, so the combinator would be a novel idiom for an equivalent form.

4 (metric delta race): Resolved alongside item 1. The new zero-delta read crosses a label the existing test writes, so the sole-writer argument no longer held; both metric tests now serialize via a shared lock, which also closes the symmetric race on the existing delta assertion.

5 (description vs behavior): Description updated. Behavior is per #1077's spec (non-collection errors from collect_signature are error-logged without the metric); the overview now distinguishes errors that bypass the reporter from those logged without a metric.

[claude-code-actions-sigp]

Claude finished @shane-moore's task in 1m 18s —— View job

---

Follow-up review

Thanks — verified the responses against the diff and updated description.

• Item 1 (infra arm coverage): sign_payload_attestation_infra_failure_increments_infra_metric at testing/payload_attestation.rs:206 injects CollectionError::EmptySignature, asserts {reason="infra"} Δ==1 and {reason="no_signature"} Δ==0. The zero-delta direction is the load-bearing one — agreed it's the cleanest way to pin the classification boundary against future drift.
• Item 2 (double index resolution): Justification accepted — early MissingIndex resolution is what sign_payload_attestation_missing_index_errors actively pins (no report_ptc_collection_failure invocation, no metric increment). Worth keeping the two sites visually aligned in any future refactor.
• Item 3 (inspect_err): Fair — workspace-wide novelty isn't worth it for a stylistic equivalence. Withdrawn.
• Item 4 (metric race): METRIC_TEST_LOCK: LazyLock<tokio::sync::Mutex<()>> at testing/payload_attestation.rs:23 is acquired by both metric tests before any counter read. tokio::sync::Mutex is correct here since the guard crosses awaits on the multi-thread runtime. Comments on both tests now point future failure tests at the lock or distinct labels, which closes the loop.
• Item 5 (description vs behavior): Updated text now distinguishes two paths cleanly — pre-collection errors from validator/cluster/index resolution propagate untouched (no reporter), vs non-collection errors raised inside collect_signature (share lookup, key decryption, threshold arithmetic) which are error-logged via the reporter but emit no metric. Matches the implementation in report_ptc_collection_failure.

No further requests. LGTM modulo the usual CI confirmation of make cargo-fmt-check / make lint / make test.
· branch: ptc-sign-payload-attestation

[jnhsigmap]

The observation divergence property looks like it's going to occur more frequently than what could be considered 'exceptional' as it looks more like 1. an artefact of Lighthouse's architecture and 2. an artefact of how the network behaves. The scope of this PR is just to ensure metrics attempt to capture? is this something we can try to resolve in Lighthouse?

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-06-15 15:57 UTC · Rule: default
• ✅ Checks passed · on draft merge queue: queuing epbs (a7eff24) and #1082 together #1089
• ✅ Merged — 2026-06-15 16:18 UTC · at 2cad7fb175395aa0b9ecb52828824140d9f397a7 · squash

This pull request spent 20 minutes 45 seconds in the queue, including 18 minutes 18 seconds running CI.

Required conditions to merge

• check-success=test-suite-success

[shane-moore]

agreed it won't be exceptional, but that's the expected design, not a defect:
the sip deliberately runs PTC as a pre-consensus partial-sig round with no QBFT, so each operator signs what its own BN observed and near-boundary divergence is inherent. There's no convergence phase to add because the spec chose not to have one. This PR just measures it: {reason="no_signature"} is an upper bound on the rate, no behavior change.

On "resolve in LH": no. It isn't an LH bug (any client observes independently), and the only LH-touchable part is the crit! on a recoverable miss (no Ok(None) abstain path), which is pure ergonomics. We push to LH pretty much only for ssv-spec compliance, not ergonomics, and even those aren't guaranteed: they're gated on how much risk the change adds to LH.

The one real follow-up is Anchor-side and measurement-only, tracked in #1079 (research): make signature_collector resolve a no-threshold eviction to a distinguishable error, so the metric can split true observation_divergence from a
genuine channel-close instead of today's coarse {reason="no_signature"} upper bound