msg-validation: round-timeout validation refactoring

[iurii-ssv]

Builds on top of #2807 to clean up the message-validation code related to validating QBFT Rounds:

• fixes an edge-case identified in message/validation: enable lowest allowed round check #2807
• provides additional comments, renames and restructures things for clarity
• increases tests coverage

Resolves https://github.com/ssvlabs/ssv-node-board/issues/1002

[codecov]

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.1%. Comparing base (83debab) to head (1f6ffdc).

Files with missing lines	Patch %	Lines
message/validation/consensus_validation.go	87.5%	1 Missing and 1 partial ⚠️

☔ 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 refactors the QBFT round-timeout validation logic: it extracts a shared roundTimeoutOffset helper that both RoundTimeout and EstimatedRoundAt derive from, fixes an edge-case where lowestAllowedRound could be computed as specqbft.NoRound when estimatedRound == allowedRoundsInPast, removes the now-unused DurationToUint64 from casts.go, and adds comprehensive boundary/cross-validation tests to enforce alignment between the two previously independent calculation paths.

Confidence Score: 5/5

Safe to merge — all changes are correct, well-tested, and the edge-case fix is validated by targeted boundary tests.

The > vs >= fix in roundBelongsToAllowedSpread is semantically correct (prevents lowestAllowedRound = specqbft.NoRound) and is harmless in practice since prior guards reject round-0 messages. EstimatedRoundAt correctly inverts roundTimeoutOffset across all phases and roles, verified by TestEstimatedRoundAtBoundaries and the new cross-validation test. Removed DurationToUint64 has zero callers. No P0/P1 issues found.

No files require special attention.

Important Files Changed

Filename	Overview
protocol/v2/qbft/roundtimer/timer.go	Adds roundTimeoutOffset, round1HeadStart, and defaultTimeoutOptions; restructures EstimatedRoundAt to invert the shared piecewise formula; logic and boundary semantics are correct.
message/validation/consensus_validation.go	Fixes roundBelongsToAllowedSpread guard from >= to > to prevent lowestAllowedRound being set to specqbft.NoRound when estimatedRound == allowedRoundsInPast; otherwise unchanged.
message/validation/consensus_validation_test.go	New unit tests for estimatedRoundAt and roundBelongsToAllowedSpread covering clamp boundary, subtraction path, and proposer special-case; coverage is thorough.
protocol/v2/qbft/roundtimer/timer_test.go	Adds TestRoundTimeoutOffset, TestEstimatedRoundAtBoundaries, TestEstimatedRoundAtEdgeCases, TestRoundTimeoutMatchesRoundTimeoutOffset, and TestEstimatedRoundAtMatchesRoundTimeout — comprehensive boundary and cross-validation coverage for all roles.
utils/casts/casts.go	Removes unused DurationToUint64, ErrNegativeTime, and ErrMaxDurationOverflow; no callers exist in the codebase, cleanup is safe.
network/topics/msg_validator_test.go	Adds an explanatory comment clarifying why GenesisTime = time.Now() is required for the round-validation window; no functional change.
message/validation/validation_test.go	Minor adjustments to existing consensus round test helpers to align with the refactored estimatedRoundAt API; no new logic concerns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["roundBelongsToAllowedSpread(role, round, receivedAt)"] --> B["estimatedRoundAt(role, timeIntoSlot)\n→ EstimatedRoundAt(role, slotDuration, timeIntoSlot)"]
    B --> C["elapsed = timeIntoSlot - round1HeadStart(role)"]
    C --> D{elapsed < 0?}
    D -- yes --> E["return FirstRound"]
    D -- no --> F{"elapsed < quickEnd?\nquickEnd = T × QuickTimeout"}
    F -- yes --> G["return FirstRound + elapsed/QuickTimeout\n(quick phase: rounds 1..T)"]
    F -- no --> H["slowElapsed = elapsed − quickEnd\nreturn FirstRound + T + slowElapsed/SlowTimeout\n(slow phase: rounds T+1…)"]
    E & G & H --> I["estimatedRound"]
    I --> J{"estimatedRound > allowedRoundsInPast (=2)?"}
    J -- no --> K["lowestAllowed = FirstRound\n✅ edge-case fix: was ≥, now >"]
    J -- yes --> L["lowestAllowed = estimatedRound − allowedRoundsInPast"]
    K & L --> M["highestAllowed = estimatedRound + allowedRoundsInFuture"]
    M --> N{role == Proposer?}
    N -- yes --> O["lowestAllowed = FirstRound\nhighestAllowed = FirstRound + 1"]
    N -- no --> P["keep computed bounds"]
    O & P --> Q{"msg.Round in\n[lowestAllowed, highestAllowed]?"}
    Q -- yes --> R["✅ Accept"]
    Q -- no --> S["❌ ErrEstimatedRoundNotInAllowedSpread"]

Loading

_{Reviews (3): Last reviewed commit: "clean up" | Re-trigger Greptile}

[iurii-ssv]

@greptile pls re-review

[nkryuchkov]

I'm not sure that 'offset' describes the duration very well

[iurii-ssv]

WDYT about renaming roundTimeoutOffset->roundTimeoutSlotOffset / roundTimeoutIntoSlot ? Or do you have some other name in mind

Or maybe we could name it just roundTimeout to keep consistent/similar to RoundTimer.RoundTimeout

[nkryuchkov]

Maybe something like RoundTimeoutAfter?

[iurii-ssv]

After part doesn't quite describe it, how about roundTimeoutForRound ?