test: SszSpecTest

[shane-moore]

Problem, Evidence, and Context (Required)

Anchor is missing the SSZSpecTest type from ssv-spec, leaving SSZ-level parity with the Go reference unverified for proposer consensus data. Regressions in withdrawals merkleization or ProposerConsensusData decoding are currently uncovered by the spec-test suite.

Part of the spec-test expansion in #954 (split from #840); sub-issue #955.

Change Overview (Required)

Adds an SSZSpecTest runner that mirrors Go's SSZSpecTest.Run(): decode ProposerConsensusData from SSZ, extract the beacon block (blinded first, then full), and assert the execution-payload withdrawals root matches the fixture's expected root. Wired into the type dispatcher under "ssz.SSZSpecTest" with one fixture.

Intentional non-changes:

• Test-harness only; no production paths touched.
• ExpectedErrorCode is not asserted, matching the upstream Go runner. The generator emits only success fixtures today; Anchor-only enforcement would diverge from the reference contract.

Risks, Trade-offs, and Mitigations (Required)

The Capella fixture uses placeholder BLS values that Lighthouse rejects during SSZ decode (BLST_BAD_ENCODING) but Go's fastssz accepts. So the default cargo test -p spec_tests path fails decode and never reaches the withdrawals-root assertion; it is kept only for parity with the current ssv-spec runner. The meaningful coverage path is --features fake_crypto, which skips BLS validation and verifies the withdrawals root end-to-end. #938 tracks making the fake_crypto invocation explicit in the Makefile / CI. Upstream fixture fix is ssvlabs/ssv-spec#622; once bumped, the fallback becomes dead code (cleanup #953).

Validation (Required)

• cargo test -p spec_tests: 55 passed (parity path).
• cargo test -p spec_tests --features fake_crypto: 55 passed (full withdrawals-root verification).
• cargo clippy, cargo +nightly fmt --check: clean.

Rollback

N/A, test-only.

Additional Info

Remaining spec test types from #954: #956, #957, #958.

[shane-moore]

@claude please review this pr

[claude-code-actions-sigp]

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

---

Review: PR #959 — test: SszSpecTest

• Read changed files and gather context
• Review code for correctness, safety, and design issues
• Post review feedback

---

Verdict: This PR looks good. The implementation is clean, follows the established patterns in the spec test crate, and the BLS fallback logic is well-justified given the upstream fixture limitations.

I reviewed the three changed files and cross-referenced with the existing test patterns (ConsensusDataProposerTest, can_decode_block, is_bls_validation_error). No correctness, security, or design issues found.

Minor observations (non-blocking)

1. ssz_spec_test.rs:38,45 — execution_payload().and_then(|p| p.withdrawals_root()): This chain will return an error for pre-Capella forks (since there's no execution payload). This is fine for the current fixture (Capella data), but worth noting that if future fixtures cover pre-merge forks, this test would fail with an opaque "Failed to get withdrawals root" message rather than a clear "fork doesn't have execution payload" message. Not a concern given the current fixture contract, just something to be aware of.

2. No ExpectedErrorCode handling: The PR description explains this well — it matches upstream Go behavior. Agree this is the right call for now.

3. BLS fallback returning Ok(()): The silent success on BLS validation failure (ssz_spec_test.rs:59) means the test provides no meaningful coverage without fake_crypto. This is explicitly acknowledged in the PR description and tracked by Add a Makefile target for spec tests with fake_crypto coverage #938. The approach mirrors how ConsensusDataProposerTest handles the same situation.

Everything else — the dispatcher wiring, module exports, serde annotations, and error formatting — follows the established conventions in the crate.

---

[shane-moore]

@claude On (1): I agree in the abstract, but I think it is outside the current fixture contract for this test type. SSZSpecTest here is specifically a withdrawals test, and there are no withdrawals pre-Capella, so pre-Capella fixtures would not be valid inputs as currently defined. The upstream ssv-spec generator also only emits a Capella fixture today, so I do not think it is worth complicating the error handling in this PR.

I also agree with your other two observations: the lack of ExpectedErrorCode handling is intentionally preserving upstream parity, and the degraded non-fake_crypto path is already called out in the PR description, with #938 tracking the intended fake_crypto Makefile / CI path.

[claude-code-actions-sigp]

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

---

• Read and understand the response to review observations

---

Agreed on all points. The withdrawals-specific scope of SSZSpecTest means pre-Capella data is not a valid input by definition, so adding defensive error handling for that case would be speculative. No changes needed here.

---