Use invalid committee duty errors for malformed duties - F-ssv-spec-053

[julienharbulot]

Ticket

F-ssv-spec-053

The ticket reported that Committee.StartDuty returned UnknownDutyRoleDataErrorCode for a nil *CommitteeDuty, while nil aggregator committee duties used InvalidAggregatorCommitteeDutyErrorCode. This conflated malformed committee duties with unknown consensus duty roles.

Changes

• Added InvalidCommitteeDutyErrorCode without shifting existing error code values.
• Used the new code for malformed committee duty validation paths.
• Added focused tests covering the committee duty error code.

Validation

• go test ./types
• go test ./ssv
• make fmt
• make lint

Fixes - Fixes ssvlabs/ssv-node-board#1031

[greptile-apps]

Greptile Summary

This PR introduces InvalidCommitteeDutyErrorCode to distinguish malformed committee duty errors from unrelated UnknownDutyRoleDataErrorCode usages, and updates the three validation paths in CommitteeDuty.Validate() and Committee.StartDuty accordingly. The new error code is appended at the end of the iota block, so no existing numeric values shift. Tests covering all three validation paths are added.

Confidence Score: 5/5

Safe to merge — focused error-code rename with no logic changes and good test coverage.

The change is narrowly scoped: it adds a single error code constant at the tail of the iota (no value shifts), replaces three error-code references in validation, and adds matching tests. No logic is altered and existing numeric codes remain stable.

No files require special attention.

Important Files Changed

Filename	Overview
types/error.go	Adds InvalidCommitteeDutyErrorCode at end of iota block — no existing codes are shifted.
types/beacon_types.go	Replaces three UnknownDutyRoleDataErrorCode returns in CommitteeDuty.Validate() with the new InvalidCommitteeDutyErrorCode.
ssv/committee.go	Nil *CommitteeDuty check in StartDuty now returns InvalidCommitteeDutyErrorCode instead of UnknownDutyRoleDataErrorCode.
ssv/committee_test.go	Extends existing nil-duty test to assert the error type and new error code.
types/validation_test.go	Adds three new subtests covering nil duty, nil validator duty, and mismatched slot in CommitteeDuty.Validate().

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Committee.StartDuty] --> B{duty type?}
    B -- "*CommitteeDuty" --> C{d == nil?}
    C -- yes --> D[InvalidCommitteeDutyErrorCode\n'nil committee duty']
    C -- no --> E[CommitteeDuty.Validate - called separately]
    B -- "*AggregatorCommitteeDuty" --> F{d == nil?}
    F -- yes --> G[InvalidAggregatorCommitteeDutyErrorCode]
    F -- no --> H[continue StartDuty]

    E --> I{cd == nil?}
    I -- yes --> D2[InvalidCommitteeDutyErrorCode]
    I -- no --> J{ValidatorDuties empty?}
    J -- yes --> K[NoBeaconDutiesErrorCode]
    J -- no --> L{nil vd?}
    L -- yes --> D3[InvalidCommitteeDutyErrorCode]
    L -- no --> M{slot mismatch?}
    M -- yes --> D4[InvalidCommitteeDutyErrorCode]
    M -- no --> N{invalid role?}
    N -- yes --> O[WrongBeaconRoleTypeErrorCode]
    N -- no --> P[ValidatorDuty.Validate]

Loading

_{Reviews (1): Last reviewed commit: "Use invalid committee duty errors for ma..." | Re-trigger Greptile}

[momosh-ssv]

Nice symmetry with the aggregator path. One coverage gap and one migration nit below — neither blocking.

[momosh-ssv]

Worth adding a subcase for ValidatorDuties: [] so the NoBeaconDutiesErrorCode path is covered too. This new test is now the canonical suite for CommitteeDuty.Validate(), so leaving the empty-slice branch untested is an obvious gap.

[momosh-ssv]

Might be worth calling out in the PR body that consumers matching the raw code UnknownDutyRoleDataErrorCode for nil/malformed CommitteeDuty inputs need to migrate to InvalidCommitteeDutyErrorCode. This is the spec repo, so a migration note reduces friction for ssv-node maintainers picking up the change.

[iurii-ssv]

LGTM, check out Momo's suggestions

[github-actions]

This pull request has been marked as stale due to 60 days of inactivity.