fix: batch pre-Boole sync contribution proofs

[shane-moore]

Closes #1050.

Problem, Evidence, and Context

Ethereum sync committees are position-based, so the same validator may occupy positions on multiple subnets or appear more than once within one subnet. For example, positions [0, 1, 128] map to subnet 0 twice and subnet 1 once. Lighthouse therefore calls Anchor once per unique subnet, two callbacks in this example, while the deployed SSV shape used by go-ssv and ssv-spec requires one ContributionProofs envelope per validator and slot with one entry per raw position, three entries here. Anchor instead sent one envelope per callback, causing InvalidPartialSignatureTypeCount and wrong-root-count errors.

This is especially visible on concentrated devnets and testnets. Although Boole bypasses this path, Anchor's built-in mainnet, Hoodi, and Holesky schedules currently have no Boole activation configured, so hardening the Alan producer remains useful.

Change Overview

• Preserve raw sync committee position multiplicity while retaining one callback per unique subnet.
• Give every callback a canonical descriptor of all roots and multiplicities.
• Eagerly admit one complete envelope per validator and slot.
• Keep failed synchronous admission retryable through a Pending | Admitted record.
• Enforce the 13-position producer bound and reject malformed assignments.

Runtime behavior changes only for pre-Boole sync committee contribution selection proofs. Regular sync committee messages, other duties, Boole committee batching, public wire types, and inbound validation behavior are unchanged.

Design Choice

We considered an all-callback accumulator, buffering in ValidatorStore or the generic sender, and a broader Lighthouse batch API. Accepting one envelope per subnet was rejected because it preserves a wire shape that diverges from go-ssv and ssv-spec.

The eager full descriptor keeps ownership aligned: ValidatorStore derives Ethereum duty facts and roots, while the signature collector owns SSV envelope construction, admission, retry, and cleanup. Any usable callback can admit the complete envelope, avoiding an all-callback barrier and allowing a sibling to retry synchronous admission failure.

Risks, Trade-offs, and Mitigations

The blast radius is intentionally narrow. The new mutex covers only bounded synchronous signing, SSZ construction, and nonblocking queue admission, with no await or blocking send while held. Descriptor, retry, cleanup, mismatch, multiplicity, and concurrency tests cover the new state.

Validation

Before the standalone rebase:

• git diff --check
• make cargo-fmt-check
• Focused release tests for signature_collector, anchor_validator_store, message_validator, and ssv_types
• make lint
• Full make test

After replaying the #1050 commit onto current unstable:

• git range-diff confirmed the rebased commit preserves the original patch identity
• git diff --check
• make cargo-fmt-check
• cargo test --release -p signature_collector, 20 passed
• cargo test --release -p anchor_validator_store, 40 passed
• cargo test --release -p message_validator, 57 passed

A mixed pre-Boole ssv-mini run used three candidate Anchor operators and one go-ssv operator. Ten validators each held seven raw positions across four unique subnets.

• 3,360 / 3,360 validator-slot pre-consensus quorums succeeded
• Every accepted envelope contained all seven entries
• Every quorum contained exactly four unique roots
• Zero type 3 wrong-root-count rejections
• Zero restarts, OOMs, monitor alerts, or descriptor mismatch logs

Rollback

Revert the #1050 commit. There are no configuration, storage, migration, or wire-format changes.

Dependencies

#1198 has merged. This branch has been replayed onto current unstable and now contains one standalone #1050 commit with only this PR's diff. There are no remaining open PR dependencies.

Additional Info / Next Steps

The ssv-mini run also exposed a separate pre-existing post-consensus multi-root batching issue. It is intentionally out of scope here. The owned descriptor and eager Pending | Admitted admission model can be extended with a phase-aware key, or a separate post-consensus record, to address that path next.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.99165% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 64.82%. Comparing base (1322601) to head (2ef996a).

Files with missing lines	Patch %	Lines
anchor/signature_collector/src/lib.rs	85.98%	22 Missing ⚠️
anchor/validator_store/src/metadata_service.rs	87.75%	6 Missing ⚠️
anchor/validator_store/src/lib.rs	98.01%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #1201      +/-   ##
============================================
+ Coverage     62.98%   64.82%   +1.84%     
============================================
  Files           162      163       +1     
  Lines         27235    27785     +550     
============================================
+ Hits          17154    18013     +859     
+ Misses        10081     9772     -309     

Flag	Coverage Δ
rust	64.82% <94.99%> (+1.84%)	⬆️

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, please take a look

[claude-code-actions-sigp]

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

---

Code Review: PR #1201

• Read the PR diff against origin/unstable
• Review signature_collector changes (core logic)
• Review validator_store changes
• Review message_validator and ssv_types changes
• Review new tests
• Post review feedback

---

Overall this is a well-designed, well-documented, and well-tested fix. The ownership split (ValidatorStore derives Ethereum facts/roots; SignatureCollector owns envelope construction + admission + retry + cleanup) is clean, and the Pending | Admitted model with per-record mutex correctly serializes admission so exactly one envelope is sent per validator/slot. I did not find any correctness defects. Notes below are verification results and optional observations, not blocking issues.

Correctness — verified sound

• Sibling signing uses the same share. In process_single_validator_batch (signature_collector/src/lib.rs:380+), sibling roots are signed with validator_signing_data.share. Since all callbacks for a given validator/slot carry that validator's share regardless of subnet, signing every descriptor root with it is correct.
• Mutex invariant holds. The guard is held across share.sign() (CPU-bound, ≤13 iterations), SSZ construction, and message_sender.sign_and_send(...). I confirmed sign_and_send → Sender::send_blocking → try_send (processor/src/senders.rs:66), which is non-blocking and never awaits — so the documented "no await / no blocking send while held" invariant is satisfied.
• No DashMap ↔ mutex deadlock. The DashMap entry guard is dropped (Arc cloned out of the match) before record.state.lock() is taken, and remove_stale_entries never takes the record mutex. No lock-order inversion.
• Idempotent local injection. Re-injecting the same operator's share into a root collector is a no-op — add_partial_signature dedups by operator_id (lib.rs:1172). So the first-callback "inject all roots" + later-sibling "inject current root" pattern is safe.
• Per-subnet return semantics preserved. Each Lighthouse callback still awaits reconstruction of its own subnet root via result_rx, while the expanded envelope is admitted once. Confirmed the batch path doesn't change what each caller receives.
• Outbound length bound. The 13-position cap is enforced upstream in sync_selection_proof_descriptor (TooManyPositions, validator_store/src/lib.rs:483) before any collector work; inbound validation independently caps Role::SyncCommittee at MAX_SYNC_COMMITTEE_POSITIONS (partial_signature.rs:273).
• Position multiplicity is correct. build_sync_validator_assignments unions raw positions into a HashSet per validator before counting per-subnet, so duplicate JSON duty records can't inflate multiplicity while legitimate distinct positions are preserved (test at metadata_service.rs:1407).
• Deterministic ordering. Descriptor is sort_unstable_by_key(subnet_id), making retries byte-identical.

Optional observations (non-blocking)

1. Function length / repeated guard pattern. process_single_validator_batch is ~130 lines and repeats the error!(reason = "...", ...); return vec![current_message] shape across four mismatch guards (pubkey, descriptor, committee_id, validator_index). Per .claude/rules/rust-style.md (functions >50 lines / DRY), extracting a small suppress(reason, ...) helper or grouping the identity checks would improve readability. Purely a maintainability nit.

2. Trust boundary on the position cap. The collector re-derives expanded_positions but relies on ValidatorStore having enforced the ≤13 cap. That's a fine ownership choice, but a one-line comment at the collector noting "cap already enforced in sync_selection_proof_descriptor" would make the trust boundary explicit for future readers.

3. Coverage of defensive branches. Codecov flags several of the mismatch/error arms in signature_collector/src/lib.rs as uncovered. committee_id_mismatch and validator_index_mismatch are hard to hit given the (slot, pubkey) key, so this is expected — just calling it out; no action needed.

Note on local verification

I reviewed statically. I was unable to run cargo build / the focused test suites in this environment (the commands require approval here), so I'm relying on the compilation and test evidence in the PR description (signature_collector 20 passed, anchor_validator_store 40, message_validator 57). If you'd like, I can attempt the build/tests again with the needed permissions.
· branch fix/1050-eager-sync-contribution-proof-batching

[shane-moore]

@claude Thanks for the review. I checked the optional observations against the current head and am not changing #1201:

• The function-length concern is fair, but keeping process_single_validator_batch linear makes record validation, the mutex boundary, message construction, and admission/retry invariants easy to audit. A generic suppression helper would also obscure the distinct structured diagnostics. Any pure construction refactor can be separate.
• The 13-entry semantic cap intentionally belongs to ValidatorStore, with independent inbound enforcement. Duplicating it in SignatureCollector would split ownership; fix: batch pre-Boole post-consensus sync contributions #1208 generalizes the same boundary to both phases.
• committee_id_mismatch and validator_index_mismatch are already exercised by mismatched_callbacks_suppress_publication_but_inject_current_share. The Codecov zero-hit lines are structured logging fields, not the branch and return. fix: batch pre-Boole post-consensus sync contributions #1208 covers the genuinely missing defensive paths.

No changes are needed for these non-blocking observations.

[claude-code-actions-sigp]

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

---

Re: optional observations

Acknowledged — all three are reasonable to defer, and I agree no changes are needed for #1201.

• Function length / suppression helper. Fair call to keep process_single_validator_batch linear. When the four mismatch guards each emit distinct structured diagnostics, a generic helper would flatten them into a single reason string and cost more in auditability than it saves in line count. Splitting a pure-construction refactor into its own PR is the right call.
• 13-entry cap ownership. Agreed — the semantic cap belongs in ValidatorStore (sync_selection_proof_descriptor) with independent inbound enforcement in partial_signature.rs, and re-deriving it in SignatureCollector would split ownership. Generalizing the boundary across both phases in fix: batch pre-Boole post-consensus sync contributions #1208 is the cleaner path.
• Coverage of mismatch arms. Verified: mismatched_callbacks_suppress_publication_but_inject_current_share (signature_collector/src/tests.rs:820) exercises both the committee_id branch (line 852–862, other_committee with a distinct CommitteeId) and the validator_index branch (line 863–871, ValidatorIndex(43)), each asserting a single injection with no extra publication (sender.attempts() == 1, line 894). The Codecov zero-hit lines are the structured logging fields, not the branch-and-return, as you noted.

Nothing blocking here. Closing these out — the deferrals to #1208 are well-scoped.
· branch fix/1050-eager-sync-contribution-proof-batching

[mergify]

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

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-08-05 19:13 UTC · Rule: default · triggered by rule Add outdated, ready-to-merge PRs to merge queue
• ✅ Checks passed · on draft merge queue: checking unstable (1322601) and #1201 together #1219
• ✅ Merged — 2026-08-05 19:45 UTC · at 2ef996a8dd60edede730c137f89d8e4f36357d7f · squash

This pull request spent 31 minutes 56 seconds in the queue, including 29 minutes 15 seconds running CI.

Required conditions to merge

• check-success=cli-reference-check
• check-success=run-local-testnet
• check-success=test-suite-success
• github-review-approved [🛡 GitHub repository ruleset rule unstable and release-v*]
  • fix: batch pre-Boole sync contribution proofs #1201
• any of [🛡 GitHub repository ruleset rule unstable and release-v*]:
  • check-success = license/cla
  • check-neutral = license/cla
  • check-skipped = license/cla