message/validation: enable lowest allowed round check

[nkryuchkov]

Summary

Enable the lower bound of the estimated round window in message validation.

This restores rejecting non-decided messages that are too far behind the locally estimated round, while keeping the lower bound clamped to specqbft.FirstRound to avoid unsigned underflow.

The round spread check now runs after slot-time and slot-advance validation so existing late slot and slot already advanced errors keep the same precedence.

Tests

• go test ./message/validation

QA

This PR requires thorough QA, because we had some message validation failures when this check was enabled. I suspect it was because of the underflow bug, though.

[codecov]

Codecov Report

❌ Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.6%. Comparing base (d18ff74) to head (16cb630).
⚠️ Report is 2 commits behind head on stage.

Files with missing lines	Patch %	Lines
message/validation/consensus_validation.go	78.9%	2 Missing and 2 partials ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[greptile-apps]

Greptile Summary

This PR re-enables the lower-bound enforcement of the estimated-round window in consensus message validation (roundBelongsToAllowedSpread). The previous implementation had an integer underflow risk — subtracting allowedRoundsInPast from estimatedRound without guarding against values smaller than allowedRoundsInPast — which is now fixed with a > allowedRoundsInPast guard that clamps lowestAllowedRound to specqbft.FirstRound. The round-spread check is also repositioned to run after validateSlotTime so existing late-slot/slot-advance errors keep their original precedence. Alongside this, the PR completes a per-duty RoundTimer refactoring (carrying slot and role) and adds a TimeoutData.Slot field so timeout events are fully scoped to a single duty.

Confidence Score: 5/5

Safe to merge; the underflow fix is correct, test coverage is solid, and the check-ordering change is a net improvement.

All findings are P2 or informational. The core logic (underflow guard, clamp to FirstRound, proposer bypass, post-slot-time positioning) is correct and well-tested. The per-duty RoundTimer refactoring is clean with no cross-duty state leakage. No blocking issues found.

No files require special attention; message/validation/consensus_validation.go is the most sensitive and the change there is minimal and correct.

Important Files Changed

Filename	Overview
message/validation/consensus_validation.go	Core fix: adds the lower-bound clamp in roundBelongsToAllowedSpread and repositions the round-spread check after validateSlotTime; logic is correct and safe from underflow.
message/validation/consensus_validation_test.go	New unit tests cover the three key clamp boundaries (estimatedRound=2, 3, 4) and the proposer special case; comprehensive and accurate.
message/validation/const.go	Introduces allowedRoundsInPast=2 and allowedRoundsInFuture=1 constants; values are consistent with the validation logic.
protocol/v2/qbft/roundtimer/timer.go	Per-duty RoundTimer now carries slot and role, enabling slot-synchronized timeouts for all non-proposer roles; EstimatedRoundAt logic is clean and well-commented.
protocol/v2/types/messages.go	TimeoutData extended with Slot field to fully scope timer events to a single duty; no issues.
protocol/v2/ssv/runner/timer.go	createTimer wires per-duty slot and role into the RoundTimer constructor; cancel of previous timer preserved.
protocol/v2/ssv/validator/timer.go	onTimeout handlers updated to include slot in TimeoutData; no logic changes, only struct population updated.
utils/casts/casts.go	DurationFromUint64 helper with MaxInt64 guard; straightforward and correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[validateQBFTMessageByDutyLogic] --> B[validateSlotTime]
    B -->|fail: late slot / slot too early| E[return ErrLateSlot / ErrEarlySlot]
    B -->|pass| C{len OperatorIDs == 1?}
    C -->|No - decided msg| D[skip round spread check]
    C -->|Yes - non-decided| F[roundBelongsToAllowedSpread]
    F --> G[Compute estimatedRoundAt]
    G --> H{estimatedRound > allowedRoundsInPast=2?}
    H -->|No - clamp| I[lowestAllowedRound = FirstRound=1]
    H -->|Yes| J[lowestAllowedRound = estimatedRound - 2]
    I --> K[highestAllowedRound = estimatedRound + 1]
    J --> K
    K --> L{role == Proposer?}
    L -->|Yes| M[lowestAllowed=1, highestAllowed=2]
    L -->|No| N{msg.Round in lowestAllowed..highestAllowed?}
    M --> N
    N -->|No| O[return ErrEstimatedRoundNotInAllowedSpread]
    N -->|Yes| P[pass - continue to duty count check]

Loading

_{Reviews (4): Last reviewed commit: "msg-validation: round-timeout validation..." | Re-trigger Greptile}

[iurii-ssv]

I think there is a discrepancy in the code-area around these changes we need to look into

[nkryuchkov]

@greptileai review it again

[nkryuchkov]

@greptileai review it again

[iurii-ssv]

Should work better now, but the code is still a bit overly-complex - I would suggest a bunch of changes before merging #2811 (addresses all my discussions from below, hence they are resolved).

[iurii-ssv]

@greptileai review this again