Skip to content

Add gossip validation spec tests for proposer/attester slashings#9323

Merged
mergify[bot] merged 7 commits into
sigp:unstablefrom
jimmygchen:ef-tests-gossip
May 28, 2026
Merged

Add gossip validation spec tests for proposer/attester slashings#9323
mergify[bot] merged 7 commits into
sigp:unstablefrom
jimmygchen:ef-tests-gossip

Conversation

@jimmygchen

@jimmygchen jimmygchen commented May 20, 2026

Copy link
Copy Markdown
Member

Issue Addressed

Addresses #9232 partially. This PR covers two topics only.

Wires up networking test vectors for gossip_proposer_slashing and gossip_attester_slashing topics.

The tests also revealed minor spec non-compliance where invalid slashings were ignored rather than rejected.

Proposed Changes

  • Refactor process_gossip_proposer_slashing and process_gossip_attester_slashing to return MessageAcceptance, so it can be verified in the tests
  • Add GossipValidation test case, handler, and test entries
  • Spec compliance fix: distinguish between internal errors and validation error - return Reject when the slashing is invalid and only penalise on invalid messages

AI Assistance Disclosure

Tools used (required — write none if no AI was used): claude for the test boilerplate. Rewrote most of the production changes manually.

Attestation (required):

  • I have read every line of this diff, understand what it does, and can explain it in review.

…tor `process_gossip_*_slashing` methods to return `MessageAcceptance` for testability.
@jimmygchen jimmygchen added test improvement Improve tests work-in-progress PR is a work-in-progress labels May 20, 2026
Validation errors were returning `Ignore` instead of `Reject`, which is incorrect per the spec. Internal errors still return `Ignore`.
@jimmygchen jimmygchen marked this pull request as ready for review May 20, 2026 06:19
@jimmygchen jimmygchen requested a review from jxs as a code owner May 20, 2026 06:19
@jimmygchen jimmygchen added ready-for-review The code is ready for review and removed work-in-progress PR is a work-in-progress labels May 20, 2026
Comment thread beacon_node/network/src/network_beacon_processor/gossip_methods.rs
Comment thread testing/ef_tests/src/cases/gossip_validation.rs
Comment thread beacon_node/network/src/network_beacon_processor/gossip_methods.rs
@jimmygchen jimmygchen requested a review from hopinheimer May 20, 2026 23:17

@michaelsproul michaelsproul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall approach looks really good.

@mergify

mergify Bot commented May 25, 2026

Copy link
Copy Markdown

This pull request has merge conflicts. Could you please resolve them @jimmygchen? 🙏

@mergify mergify Bot added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review labels May 25, 2026
# Conflicts:
#	Cargo.lock
#	beacon_node/network/src/network_beacon_processor/mod.rs
#	testing/ef_tests/Cargo.toml
@jimmygchen jimmygchen added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels May 27, 2026

@dknopik dknopik left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@jimmygchen jimmygchen added the ready-for-merge This PR is ready to merge. label May 27, 2026
@mergify mergify Bot added the queued label May 27, 2026
@mergify

mergify Bot commented May 27, 2026

Copy link
Copy Markdown

Merge Queue Status

This pull request spent 30 minutes 15 seconds in the queue, including 28 minutes 14 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request May 27, 2026
@mergify mergify Bot merged commit 5636030 into sigp:unstable May 28, 2026
38 checks passed
@mergify mergify Bot removed the queued label May 28, 2026
jimmygchen added a commit to jimmygchen/lighthouse that referenced this pull request May 29, 2026
…p#9323)

Addresses sigp#9232 partially. This PR covers two topics only.
* sigp#9232

Wires up networking test vectors for `gossip_proposer_slashing` and `gossip_attester_slashing` topics.

The tests also revealed minor spec non-compliance where invalid slashings were ignored rather than rejected.

  - Refactor `process_gossip_proposer_slashing` and `process_gossip_attester_slashing` to return `MessageAcceptance`, so it can be verified in the tests
- Add `GossipValidation` test case, handler, and test entries
- Spec compliance fix: distinguish between internal errors and validation error - return `Reject` when the slashing is invalid and only penalise on invalid messages

Co-Authored-By: Jimmy Chen <[email protected]>
jimmygchen added a commit to jimmygchen/lighthouse that referenced this pull request May 29, 2026
…p#9323)

Addresses sigp#9232 partially. This PR covers two topics only.
* sigp#9232

Wires up networking test vectors for `gossip_proposer_slashing` and `gossip_attester_slashing` topics.

The tests also revealed minor spec non-compliance where invalid slashings were ignored rather than rejected.

  - Refactor `process_gossip_proposer_slashing` and `process_gossip_attester_slashing` to return `MessageAcceptance`, so it can be verified in the tests
- Add `GossipValidation` test case, handler, and test entries
- Spec compliance fix: distinguish between internal errors and validation error - return `Reject` when the slashing is invalid and only penalise on invalid messages

Co-Authored-By: Jimmy Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge This PR is ready to merge. ready-for-review The code is ready for review test improvement Improve tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants