Standardize sentinel pattern (F-ssv-spec-027)

[julienharbulot]

Fix for:

Inconsistent Enum Sentinel Values
ssv-spec · Inconsistency · Assigned: dev4 · Effort: —
BNRoleUnknown = math.MaxUint64 (BeaconRole) vs RoleUnknown = -1 (RunnerRole as int32). The RunnerRole sentinel -1 can't be properly represented as uint32 (line 50-52 in messages.go converts to uint32), potentially matching corrupted data.

Impact: Type confusion between sentinel values. Corrupted message data could match RoleUnknown unintentionally.

Recommendation: Standardize sentinel pattern across all enums.

[greptile-apps]

Greptile Summary

This PR standardizes the sentinel pattern for RunnerRole wire encoding by introducing WireUint32() / RunnerRoleFromWireUint32() helpers that map RoleUnknown to math.MaxUint32 — consistent with how BeaconRole uses math.MaxUint64. The change is backward-compatible: uint32(int32(-1)) already produced 0xFFFFFFFF before, and the new helpers produce the same wire bytes.

• types/runner_role.go: New WireUint32() method and RunnerRoleFromWireUint32() function centralise sentinel handling; any negative role value maps to the sentinel on the wire.
• types/messages.go: GetRoleType() and NewMsgID() updated to use the new helpers.
• types/runner_role_test.go: New tests validate the sentinel and a round-trip for RoleProposer, but leave the remaining four valid roles untested.

Confidence Score: 4/5

Safe to merge — backward-compatible encoding, no protocol changes

The wire encoding is unchanged for all previously valid values; the only behavioural change is that RoleUnknown (previously an implicit cast of -1 to uint32) is now explicit and correct. Minor style concerns (redundant guard, incomplete test coverage) do not affect correctness.

types/runner_role.go — verify the redundant sentinel check is intentional

Important Files Changed

Filename	Overview
types/runner_role.go	Adds WireUint32() and RunnerRoleFromWireUint32() to properly handle RoleUnknown sentinel (MaxUint32) during wire encoding/decoding; minor redundant condition in RunnerRoleFromWireUint32
types/messages.go	Updated GetRoleType() and NewMsgID() to use new wire encoding helpers; backward-compatible with prior encoding
types/runner_role_test.go	New test file covering RoleUnknown sentinel and round-trip for RoleProposer; missing coverage for other valid roles

Sequence Diagram

sequenceDiagram
    participant C as Caller
    participant M as messages.go
    participant R as runner_role.go

    Note over C,R: Encoding (NewMsgID)
    C->>M: NewMsgID(domain, dutyExecutorID, role)
    M->>R: role.WireUint32()
    R-->>M: uint32 (MaxUint32 if role<0, else uint32(role))
    M-->>C: MessageID bytes

    Note over C,R: Decoding (GetRoleType)
    C->>M: msgID.GetRoleType()
    M->>M: binary.LittleEndian.Uint32(bytes)
    M->>R: RunnerRoleFromWireUint32(v)
    R-->>M: RunnerRole (RoleUnknown if v>MaxInt32)
    M-->>C: RunnerRole

Loading

_{Reviews (1): Last reviewed commit: "Standardize sentinel pattern" | Re-trigger Greptile}

[MatheusFranco99]

Good change! Looks good to me! :)
However, I didn't get why the .json file tests got modified (since the edge case was already the same 0xff... , and some tests don't seem related to the issue)
Can you give me a help on that?

[julienharbulot]

@MatheusFranco99 I had regenerated the fixtures after merge but you are right it wasn't necessary. I have cleaned up the git history and removed the unnecessary changes to allow for a clean merge on top of main

[julienharbulot]

Fixes https://github.com/ssvlabs/ssv-node-board/issues/866
@MatheusFranco99 ready for your review

[github-actions]

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