QBFT Round Validation and CreateMsgSpecTest Fix

[MatheusFranco99]

Overview

This PR adds a round validation to QBFT messages and fixes issues with CreateMsgSpecTest.

Changes

• Round Validation: Adds a validation check to ensure qbft.Message.Round is valid (!= NoRound, i.e. 0) to the validation function.
• Fixes to CreateMsgSpecTest:
  • Ensures Round set by test cases is used in commit message tests.
  • Sets State.Round in tests where it was previously missing and required for correct message creation.
  • Adds a message validation step to the test processing function to catch invalid messages during test execution.

Related Issues

Closes:

• Spec tests json files will define specific inputs, but these wont actually be set in the tests #534
• some tests define round = 0 but there is validation to ensure that round != 0 #535

[Copilot]

Pull Request Overview

This PR fixes the CreateMsgSpecTest so that the Round configured in test cases is now properly used during message creation by adding it to the qbft.State, while also updating type references from the deprecated spec alias to phase0.

• Propagates the Round field into qbft state instance creation in multiple message creation functions
• Updates type references (e.g. BLSPubKey, Domain, Slot) from spec to phase0 across testing utilities, key manager, and runner files

Reviewed Changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
types/testingutils/validator_pubkeys.go	Migrates BLSPubKey type usage from spec to phase0
types/testingutils/ssv_msgs_committee_duty.go	Replaces if/else with a clear switch statement for validator duty types
types/testingutils/keymanager.go	Updates mapping and function signatures to use phase0 types
types/testingutils/beacon_node_committee_duty.go	Applies similar phase0 type conversions and switch case for duty types
ssv/spectest/tests/valcheck/*	Updates test structs to use phase0 types for checkpoints and roots
ssv/runner.go	Converts map keys and fields to phase0.ValidatorIndex and phase0.Slot
qbft/spectest/tests/messages/create_commit.go	Updates ExpectedRoot to the new value reflecting Round usage
qbft/spectest/tests/create_message_spectest.go	Adds setting of the Round field in qbft.State during message creation

Files not reviewed (2)

• qbft/spectest/generate/state_comparison/tests_CreateMsgSpecTest/qbft create message create commit.json: Language not supported
• qbft/spectest/generate/tests/tests.CreateMsgSpecTest_qbft_create_message_create_commit.json: Language not supported

Comments suppressed due to low confidence (1)

qbft/spectest/tests/create_message_spectest.go:83

• Ensure that test cases validate the correct handling of the Round field in downstream message processing to prevent potential mismatches in behavior.

Round:           test.Round,

[GalRogozinski]

Good job!

[alan-ssvlabs]

LGTM!

[y0sher]

lgtm

[Copilot]

Pull Request Overview

This PR enhances QBFT message validation by enforcing non-zero rounds and corrects the CreateMsgSpecTest suite to use and verify the Round field in commit, prepare, proposal, and round-change tests. It also refactors imports and type qualifiers in testing utilities and updates expected SSZ roots in generated JSON.

• Added a round-zero check to qbft.Message.Validate and incorporated runtime validation in CreateMsgSpecTest.
• Updated all message creation test cases to set Round and adjusted expected message roots accordingly.
• Cleaned up import aliases and unified spec → phase0 qualifiers; refactored an if/else chain to a switch with explicit panic.

Reviewed Changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
types/testingutils/beacon_node_committee_duty.go	Replaced if/else with switch and added default panic
ssv/spectest/tests/valcheck/valcheckattestations/*.go	Removed unused alias import and qualified types with phase0
ssv/runner.go	Swapped spec types to phase0 and updated signatures
qbft/spectest/tests/messages/create_*.go	Set Round in all CreateMsgSpecTest instances and updated SSZ roots
qbft/spectest/tests/create_message_spectest.go	Added message.Validate/Decode/Validate steps and passed test.Round
qbft/messages.go	Added msg.Round != NoRound validation

Comments suppressed due to low confidence (4)

qbft/messages.go:105

• [nitpick] The validation error for zero round is generic. Consider including the invalid round value, e.g., fmt.Errorf("invalid message round: %d", msg.Round), to aid debugging.

if msg.Round == NoRound {

qbft/spectest/tests/create_message_spectest.go:75

• Add a negative test case (e.g., Round = NoRound) to verify that msg.Validate() fails as expected, covering the new round validation branch.

// Validate message

types/testingutils/beacon_node_committee_duty.go:237

• [nitpick] The panic message could be more descriptive by listing the valid BNRole values or including context about the failing function.

panic(fmt.Sprintf("type %v not expected", validatorDuty.Type))

ssv/runner.go:74

• [nitpick] Fix the comment grammar to SetHighestDecidedSlot sets the highest decided slot for the base runner for clarity.

// SetHighestDecidedSlot set highestDecidedSlot for base runner