Add state transition vectors for aggregation bits bounds handling#899
Merged
tcoratger merged 2 commits intoJun 10, 2026
Merged
Conversation
Collaborator
|
Thanks a lot, can you just rebase after #895 and follow the new doc writer conventions for the documentation of the test vectors just for uniformity? :) Also I haven't checked in details but would be nice to ensure that this test doesn't already exist with the new coverage added in the PR. I imagine this test coverage was not present before since it helped you guys to spot the bug. |
…itly A set aggregation bit at an index past the validator registry previously crashed the vote tally write with a bare IndexError. That exception is not an AssertionError, so rejection handlers cannot catch it, and it carries no language-neutral reason for cross-client fixtures to assert against. Raise SpecRejectionError(VALIDATOR_INDEX_OUT_OF_RANGE) instead, matching the equivalent check in signature verification. The check sits after the vote filters, so filtered attestations are still skipped without touching their bits.
…unds Cover the malformed-bits block rejection paths and the benign padding case (prompted by an audit finding in ethlambda, where a skipped attestation left a divergent post-state): - set bit past the validator registry -> VALIDATOR_INDEX_OUT_OF_RANGE - all-false bits and zero-length bits -> EMPTY_AGGREGATION_BITS - trailing unset padding past the registry -> processed normally; the pinned post-state root fails any client that skips the attestation Two framework changes make these expressible: - AggregatedAttestationSpec gains a raw aggregation bits override for the unsigned forced-attestation path, since deriving bits from validator indices can only produce tight bitfields. - The placeholder zero state root of rejection tests now applies only to the final block, so earlier setup blocks process normally instead of failing with STATE_ROOT_MISMATCH. All existing rejection tests are single-block, where this changes nothing.
88d9731 to
3cd715e
Compare
tcoratger
approved these changes
Jun 10, 2026
tcoratger
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for flagging this!
tcoratger
added a commit
that referenced
this pull request
Jun 10, 2026
…ions (#919) * docs(testing): align aggregation-bits vectors with doc-writer conventions Follow-up to #899. Brings the aggregation-bits bounds vectors in line with the consensus test-vector documentation standard: - Drop all inline body comments from the test vectors; the Given/When/Then docstring is the single source of truth. - Strip the "a client that..." narration from every Then section so it states only the asserted outcome (rejection reason or post-state). - Move the bitfield layout and SSZ-encoding rationale into Given as atomic facts, and note the bitfield-limit vs registry distinction. - Tighten the aggregation_bits field and resolver docstrings to terse one-sentence-per-line phrasing. - Note in process_attestations that signature verification normally rejects an out-of-range bit first, so this guards the unsigned path. - Make is_final_block keyword-only on the block builder. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(testing): use bullets for the aggregation_bits field docstring Per the doc-writer rules, render the field's three behaviors as bullet points instead of prose, and fix the summary line so it describes the default (derived) case rather than only the verbatim override. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
An audit finding in ethlambda (fixed in lambdaclass/ethlambda#425) showed that clients can diverge on blocks carrying malformed
aggregation_bits: ethlambda skipped an attestation whose bitfield was longer than the validator registry, while zeam and lantern reject such blocks. A skipped attestation also left a spurious all-false entry in the post-state — a cross-client state-divergence vector. The spec had no vectors pinning this behavior.Spec change
process_attestationspreviously crashed with a bareIndexErrorwhen a set bit pointed past the validator registry. That exception is not anAssertionError, so rejection handlers cannot catch it, and it carries no language-neutral reason for fixtures. It now raisesSpecRejectionError(VALIDATOR_INDEX_OUT_OF_RANGE), matching the equivalent check inverify_signatures. The check sits after the vote filters, so filtered attestations are still skipped without their bits being touched. The pre-existingEMPTY_AGGREGATION_BITSrejection is unchanged, now resolved once before the tally insert.New vectors (
state_transitionformat)VALIDATOR_INDEX_OUT_OF_RANGEEMPTY_AGGREGATION_BITSEMPTY_AGGREGATION_BITSThe last vector is the sharp one: validated against ethlambda, it passes on the fixed branch and fails on pre-fix code with a state root mismatch. The rejection vectors discriminate via the emitted
rejectionReason, so client runners should assert it rather than only checking that the block was rejected.Framework changes
AggregatedAttestationSpecgains a rawaggregation_bitsoverride, honored only by the unsigned forced-attestation path of the state transition format. Deriving bits from validator indices can only produce tight bitfields, so empty or padded bitfields were previously inexpressible.STATE_ROOT_MISMATCH. All existing rejection tests are single-block, where this changes nothing (verified: full fill passes unchanged).Testing
uv run pytest: 2872 passeduv run fill --fork=lstar -n auto: 530 fillers pass, determinism check passesjust checkclean