Skip to content

QBFT Round Validation and CreateMsgSpecTest Fix #537

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
GalRogozinski merged 4 commits into from
Jun 11, 2025
Merged

QBFT Round Validation and CreateMsgSpecTest Fix #537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81d83c0
set the test defined round in qbft state
MatheusFranco99 Apr 24, 2025
59a0f8d
fix lint issues
MatheusFranco99 Apr 24, 2025
68ce198
add invalid round validation check
MatheusFranco99 Apr 25, 2025
2f919f6
fix tests with round as 0
MatheusFranco99 Apr 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions qbft/messages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ func (msg *Message) Validate() error {
if msg.MsgType > RoundChangeMsgType {
return errors.New("message type is invalid")
}
if msg.Round == NoRound {
return errors.New("message round is invalid")
}
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion .../generate/state_comparison/tests_CreateMsgSpecTest/qbft create message create commit.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@
"RoundChangeJustifications": null,
"PrepareJustifications": null,
"CreateType": "CreateCommit",
"ExpectedRoot": "22763e542ac1faba4c01c9aefee5b96dc23889c1fb079e46e00a13c9fcc367c7",
"ExpectedRoot": "9ca79f5875b289eb3cbb318db864d9afb5c7cb2d5e48722137e7df6d3df1b0d5",
"ExpectedError": ""
}
4 changes: 2 additions & 2 deletions .../tests_CreateMsgSpecTest/qbft create message create proposal not previously prepared.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
0
],
"StateValue": null,
"Round": 0,
"Round": 10,
"RoundChangeJustifications": [
{
"Signatures": [
Expand Down Expand Up @@ -253,6 +253,6 @@
],
"PrepareJustifications": null,
"CreateType": "createProposal",
"ExpectedRoot": "39e9ac7d177bf75fffa99354d498ffea607dc473a68b3d12b6425b15efda9c9e",
"ExpectedRoot": "373d38164afdb7d5b4f995bdb49c92c9b4b76621f078f5f4ffaf113a2c1a9aa7",
"ExpectedError": ""
}
4 changes: 2 additions & 2 deletions ...ison/tests_CreateMsgSpecTest/qbft create message create proposal previously prepared.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
0
],
"StateValue": null,
"Round": 0,
"Round": 10,
"RoundChangeJustifications": [
{
"Signatures": [
Expand Down Expand Up @@ -467,6 +467,6 @@
}
],
"CreateType": "createProposal",
"ExpectedRoot": "5ef5acc64100f6bef7bcd243ddbc71dde12bca8732941dadac6d21673952e4a0",
"ExpectedRoot": "db5c994563408850f8493320b9f5ec369ec49ca5552b25f4d80a24e9c4896e96",
"ExpectedError": ""
}
4 changes: 2 additions & 2 deletions ...enerate/state_comparison/tests_CreateMsgSpecTest/qbft create message create proposal.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
0
],
"StateValue": null,
"Round": 0,
"Round": 10,
"RoundChangeJustifications": null,
"PrepareJustifications": null,
"CreateType": "createProposal",
"ExpectedRoot": "82802b9530012981e50ab2ea72ea2e914e584106a8c5795a2dc0a2ce494cafa3",
"ExpectedRoot": "bef9eabbeaee0d8067760f17aabe154af09a6678fbe65c370a68421e3a0ecfd2",
"ExpectedError": ""
}
2 changes: 1 addition & 1 deletion qbft/spectest/generate/tests/tests.CreateMsgSpecTest_qbft_create_message_create_commit.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@
"RoundChangeJustifications": null,
"PrepareJustifications": null,
"CreateType": "CreateCommit",
"ExpectedRoot": "22763e542ac1faba4c01c9aefee5b96dc23889c1fb079e46e00a13c9fcc367c7",
"ExpectedRoot": "9ca79f5875b289eb3cbb318db864d9afb5c7cb2d5e48722137e7df6d3df1b0d5",
"ExpectedError": ""
}
4 changes: 2 additions & 2 deletions .../spectest/generate/tests/tests.CreateMsgSpecTest_qbft_create_message_create_proposal.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@
0
],
"StateValue": null,
"Round": 0,
"Round": 10,
"RoundChangeJustifications": null,
"PrepareJustifications": null,
"CreateType": "createProposal",
"ExpectedRoot": "82802b9530012981e50ab2ea72ea2e914e584106a8c5795a2dc0a2ce494cafa3",
"ExpectedRoot": "bef9eabbeaee0d8067760f17aabe154af09a6678fbe65c370a68421e3a0ecfd2",
"ExpectedError": ""
}
4 changes: 2 additions & 2 deletions .../tests.CreateMsgSpecTest_qbft_create_message_create_proposal_not_previously_prepared.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
0
],
"StateValue": null,
"Round": 0,
"Round": 10,
"RoundChangeJustifications": [
{
"Signatures": [
Expand Down Expand Up @@ -253,6 +253,6 @@
],
"PrepareJustifications": null,
"CreateType": "createProposal",
"ExpectedRoot": "39e9ac7d177bf75fffa99354d498ffea607dc473a68b3d12b6425b15efda9c9e",
"ExpectedRoot": "373d38164afdb7d5b4f995bdb49c92c9b4b76621f078f5f4ffaf113a2c1a9aa7",
"ExpectedError": ""
}
4 changes: 2 additions & 2 deletions ...ests/tests.CreateMsgSpecTest_qbft_create_message_create_proposal_previously_prepared.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
0
],
"StateValue": null,
"Round": 0,
"Round": 10,
"RoundChangeJustifications": [
{
"Signatures": [
Expand Down Expand Up @@ -467,6 +467,6 @@
}
],
"CreateType": "createProposal",
"ExpectedRoot": "5ef5acc64100f6bef7bcd243ddbc71dde12bca8732941dadac6d21673952e4a0",
"ExpectedRoot": "db5c994563408850f8493320b9f5ec369ec49ca5552b25f4d80a24e9c4896e96",
"ExpectedError": ""
}
15 changes: 15 additions & 0 deletions qbft/spectest/tests/create_message_spectest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,17 @@ func (test *CreateMsgSpecTest) Run(t *testing.T) {
}
require.EqualValues(t, test.ExpectedRoot, hex.EncodeToString(r[:]))

// Validate message
err = msg.Validate()
require.NoError(t, err)

qbftMsg := &qbft.Message{}
err = qbftMsg.Decode(msg.SSVMessage.Data)
require.NoError(t, err)

err = qbftMsg.Validate()
require.NoError(t, err)

typescomparable.CompareWithJson(t, test, test.TestName(), reflect.TypeOf(test).String())
}

Expand All @@ -80,6 +91,7 @@ func (test *CreateMsgSpecTest) createCommit() (*types.SignedSSVMessage, error) {
state := &qbft.State{
CommitteeMember: testingutils.TestingCommitteeMember(ks),
ID: []byte{1, 2, 3, 4},
Round: test.Round,
}
signer := testingutils.TestingOperatorSigner(ks)

Expand All @@ -91,6 +103,7 @@ func (test *CreateMsgSpecTest) createPrepare() (*types.SignedSSVMessage, error)
state := &qbft.State{
CommitteeMember: testingutils.TestingCommitteeMember(ks),
ID: []byte{1, 2, 3, 4},
Round: test.Round,
}
signer := testingutils.TestingOperatorSigner(ks)

Expand All @@ -102,6 +115,7 @@ func (test *CreateMsgSpecTest) createProposal() (*types.SignedSSVMessage, error)
state := &qbft.State{
CommitteeMember: testingutils.TestingCommitteeMember(ks),
ID: []byte{1, 2, 3, 4},
Round: test.Round,
}
signer := testingutils.TestingOperatorSigner(ks)

Expand All @@ -115,6 +129,7 @@ func (test *CreateMsgSpecTest) createRoundChange() (*types.SignedSSVMessage, err
CommitteeMember: testingutils.TestingCommitteeMember(ks),
ID: []byte{1, 2, 3, 4},
PrepareContainer: qbft.NewMsgContainer(),
Round: test.Round,
}
signer := testingutils.TestingOperatorSigner(ks)

Expand Down
2 changes: 1 addition & 1 deletion qbft/spectest/tests/messages/create_commit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,6 @@ func CreateCommit() tests.SpecTest {
Name: "create commit",
Value: [32]byte{1, 2, 3, 4},
Round: 10,
ExpectedRoot: "22763e542ac1faba4c01c9aefee5b96dc23889c1fb079e46e00a13c9fcc367c7",
ExpectedRoot: "9ca79f5875b289eb3cbb318db864d9afb5c7cb2d5e48722137e7df6d3df1b0d5",
}
}
3 changes: 2 additions & 1 deletion qbft/spectest/tests/messages/create_proposal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ func CreateProposal() tests.SpecTest {
CreateType: tests.CreateProposal,
Name: "create proposal",
Value: [32]byte{1, 2, 3, 4},
ExpectedRoot: "82802b9530012981e50ab2ea72ea2e914e584106a8c5795a2dc0a2ce494cafa3",
Round: 10,
ExpectedRoot: "bef9eabbeaee0d8067760f17aabe154af09a6678fbe65c370a68421e3a0ecfd2",
}
}
3 changes: 2 additions & 1 deletion qbft/spectest/tests/messages/create_proposal_not_prev_prepared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,12 @@ func CreateProposalNotPreviouslyPrepared() tests.SpecTest {
CreateType: tests.CreateProposal,
Name: "create proposal not previously prepared",
Value: [32]byte{1, 2, 3, 4},
Round: 10,
RoundChangeJustifications: []*types.SignedSSVMessage{
testingutils.TestingProposalMessageWithRound(ks.OperatorKeys[1], types.OperatorID(1), 2),
testingutils.TestingProposalMessageWithRound(ks.OperatorKeys[2], types.OperatorID(2), 2),
testingutils.TestingProposalMessageWithRound(ks.OperatorKeys[3], types.OperatorID(3), 2),
},
ExpectedRoot: "39e9ac7d177bf75fffa99354d498ffea607dc473a68b3d12b6425b15efda9c9e",
ExpectedRoot: "373d38164afdb7d5b4f995bdb49c92c9b4b76621f078f5f4ffaf113a2c1a9aa7",
}
}
3 changes: 2 additions & 1 deletion qbft/spectest/tests/messages/create_proposal_prev_prepared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ func CreateProposalPreviouslyPrepared() tests.SpecTest {
CreateType: tests.CreateProposal,
Name: "create proposal previously prepared",
Value: [32]byte{1, 2, 3, 4},
Round: 10,
RoundChangeJustifications: []*types.SignedSSVMessage{
testingutils.TestingRoundChangeMessageWithRound(ks.OperatorKeys[1], types.OperatorID(1), 2),
testingutils.TestingRoundChangeMessageWithRound(ks.OperatorKeys[2], types.OperatorID(2), 2),
Expand All @@ -23,6 +24,6 @@ func CreateProposalPreviouslyPrepared() tests.SpecTest {
testingutils.TestingPrepareMessage(ks.OperatorKeys[2], types.OperatorID(2)),
testingutils.TestingPrepareMessage(ks.OperatorKeys[3], types.OperatorID(3)),
},
ExpectedRoot: "5ef5acc64100f6bef7bcd243ddbc71dde12bca8732941dadac6d21673952e4a0",
ExpectedRoot: "db5c994563408850f8493320b9f5ec369ec49ca5552b25f4d80a24e9c4896e96",
}
}
15 changes: 7 additions & 8 deletions ssv/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package ssv

import (
"github.com/attestantio/go-eth2-client/spec/phase0"
spec "github.com/attestantio/go-eth2-client/spec/phase0"
ssz "github.com/ferranbt/fastssz"
"github.com/pkg/errors"
"github.com/ssvlabs/ssv-spec/qbft"
Expand Down Expand Up @@ -35,32 +34,32 @@ type Runner interface {
ProcessPostConsensus(signedMsg *types.PartialSignatureMessages) error

// expectedPreConsensusRootsAndDomain an INTERNAL function, returns the expected pre-consensus roots to sign
expectedPreConsensusRootsAndDomain() ([]ssz.HashRoot, spec.DomainType, error)
expectedPreConsensusRootsAndDomain() ([]ssz.HashRoot, phase0.DomainType, error)
// expectedPostConsensusRootsAndDomain an INTERNAL function, returns the expected post-consensus roots to sign
expectedPostConsensusRootsAndDomain() ([]ssz.HashRoot, spec.DomainType, error)
expectedPostConsensusRootsAndDomain() ([]ssz.HashRoot, phase0.DomainType, error)
// executeDuty an INTERNAL function, executes a duty.
executeDuty(duty types.Duty) error
}

type BaseRunner struct {
State *State
Share map[spec.ValidatorIndex]*types.Share
Share map[phase0.ValidatorIndex]*types.Share
QBFTController *qbft.Controller
BeaconNetwork types.BeaconNetwork
RunnerRoleType types.RunnerRole
*types.OperatorSigner

// highestDecidedSlot holds the highest decided duty slot and gets updated after each decided is reached
highestDecidedSlot spec.Slot
highestDecidedSlot phase0.Slot
}

func NewBaseRunner(
state *State,
share map[spec.ValidatorIndex]*types.Share,
share map[phase0.ValidatorIndex]*types.Share,
controller *qbft.Controller,
beaconNetwork types.BeaconNetwork,
runnerRoleType types.RunnerRole,
highestDecidedSlot spec.Slot,
highestDecidedSlot phase0.Slot,
) *BaseRunner {
return &BaseRunner{
State: state,
Expand All @@ -73,7 +72,7 @@ func NewBaseRunner(
}

// SetHighestDecidedSlot set highestDecidedSlot for base runner
func (b *BaseRunner) SetHighestDecidedSlot(slot spec.Slot) {
func (b *BaseRunner) SetHighestDecidedSlot(slot phase0.Slot) {
b.highestDecidedSlot = slot
}

Expand Down
5 changes: 2 additions & 3 deletions ssv/spectest/tests/valcheck/valcheckattestations/majority_slashable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/hex"

"github.com/attestantio/go-eth2-client/spec/phase0"
spec "github.com/attestantio/go-eth2-client/spec/phase0"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/valcheck"
"github.com/ssvlabs/ssv-spec/types"
Expand All @@ -15,11 +14,11 @@ import (
func MajoritySlashable() tests.SpecTest {
data := &types.BeaconVote{
BlockRoot: testingutils.TestingBlockRoot,
Source: &spec.Checkpoint{
Source: &phase0.Checkpoint{
Epoch: 0,
Root: testingutils.TestingBlockRoot,
},
Target: &spec.Checkpoint{
Target: &phase0.Checkpoint{
Epoch: 1,
Root: testingutils.TestingBlockRoot,
},
Expand Down
5 changes: 2 additions & 3 deletions ssv/spectest/tests/valcheck/valcheckattestations/minority_slashable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/hex"

"github.com/attestantio/go-eth2-client/spec/phase0"
spec "github.com/attestantio/go-eth2-client/spec/phase0"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/valcheck"
"github.com/ssvlabs/ssv-spec/types"
Expand All @@ -15,11 +14,11 @@ import (
func MinoritySlashable() tests.SpecTest {
data := &types.BeaconVote{
BlockRoot: testingutils.TestingBlockRoot,
Source: &spec.Checkpoint{
Source: &phase0.Checkpoint{
Epoch: 0,
Root: testingutils.TestingBlockRoot,
},
Target: &spec.Checkpoint{
Target: &phase0.Checkpoint{
Epoch: 1,
Root: testingutils.TestingBlockRoot,
},
Expand Down
11 changes: 5 additions & 6 deletions ssv/spectest/tests/valcheck/valcheckattestations/slashable.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/hex"

"github.com/attestantio/go-eth2-client/spec/phase0"
spec "github.com/attestantio/go-eth2-client/spec/phase0"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/valcheck"
"github.com/ssvlabs/ssv-spec/types"
Expand All @@ -14,14 +13,14 @@ import (
// Slashable tests a slashable AttestationData
func Slashable() tests.SpecTest {
data := &types.BeaconVote{
BlockRoot: spec.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Source: &spec.Checkpoint{
BlockRoot: phase0.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Source: &phase0.Checkpoint{
Epoch: 0,
Root: spec.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Root: phase0.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
},
Target: &spec.Checkpoint{
Target: &phase0.Checkpoint{
Epoch: 1,
Root: spec.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Root: phase0.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
},
}

Expand Down
11 changes: 5 additions & 6 deletions ssv/spectest/tests/valcheck/valcheckattestations/valid_non_slashable_slot.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"encoding/hex"

"github.com/attestantio/go-eth2-client/spec/phase0"
spec "github.com/attestantio/go-eth2-client/spec/phase0"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/valcheck"
"github.com/ssvlabs/ssv-spec/types"
Expand All @@ -14,14 +13,14 @@ import (
// ValidNonSlashableSlot tests a valid AttestationData with a slot that is not slashable
func ValidNonSlashableSlot() tests.SpecTest {
data := &types.BeaconVote{
BlockRoot: spec.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Source: &spec.Checkpoint{
BlockRoot: phase0.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Source: &phase0.Checkpoint{
Epoch: 0,
Root: spec.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Root: phase0.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
},
Target: &spec.Checkpoint{
Target: &phase0.Checkpoint{
Epoch: 1,
Root: spec.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
Root: phase0.Root{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 1, 2},
},
}

Expand Down
Loading