Skip to content

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

Open
julienh-ssv wants to merge 1 commit into
mainfrom
F-ssv-spec-053
Open

Use invalid committee duty errors for malformed duties - F-ssv-spec-053#623
julienh-ssv wants to merge 1 commit into
mainfrom
F-ssv-spec-053

Conversation

@julienh-ssv
Copy link
Copy Markdown
Contributor

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
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 28, 2026

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

@julienh-ssv julienh-ssv requested a review from nkryuchkov April 28, 2026 06:57
@julienh-ssv julienh-ssv enabled auto-merge (squash) April 28, 2026 07:02
Copy link
Copy Markdown

@momosh-ssv momosh-ssv left a comment

Choose a reason for hiding this comment

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

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

Comment thread types/validation_test.go
require.Error(t, err)
require.ErrorIs(t, err, &Error{})
require.Equal(t, InvalidCommitteeDutyErrorCode, err.(*Error).Code)
})
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Comment thread types/error.go
InvalidCommitteeMemberErrorCode
InvalidValidatorDutyErrorCode
BeaconVoteNilCheckpointErrorCode
InvalidCommitteeDutyErrorCode
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants