Skip to content
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.

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

Share length validation #478

Merged
merged 9 commits into from
Aug 20, 2024
9 changes: 7 additions & 2 deletions ssv/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ func NewAggregatorRunner(
operatorSigner *types.OperatorSigner,
valCheck qbft.ProposedValueCheckF,
highestDecidedSlot phase0.Slot,
) Runner {
) (Runner, error) {

if len(share) != 1 {
return nil, errors.New("must have one share")
}

return &AggregatorRunner{
BaseRunner: &BaseRunner{
RunnerRoleType: types.RoleAggregator,
Expand All @@ -44,7 +49,7 @@ func NewAggregatorRunner(
signer: signer,
operatorSigner: operatorSigner,
valCheck: valCheck,
}
}, nil
}

func (r *AggregatorRunner) StartNewDuty(duty types.Duty, quorum uint64) error {
Expand Down
9 changes: 6 additions & 3 deletions ssv/committee_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ func NewCommitteeRunner(beaconNetwork types.BeaconNetwork,
signer types.BeaconSigner,
operatorSigner *types.OperatorSigner,
valCheck qbft.ProposedValueCheckF,
) Runner {
) (Runner, error) {
if len(share) == 0 {
return nil, errors.New("no shares")
}
return &CommitteeRunner{
BaseRunner: &BaseRunner{
RunnerRoleType: types.RoleCommittee,
Expand All @@ -42,7 +45,7 @@ func NewCommitteeRunner(beaconNetwork types.BeaconNetwork,
operatorSigner: operatorSigner,
valCheck: valCheck,
submittedDuties: make(map[types.BeaconRole]map[phase0.ValidatorIndex]struct{}),
}
}, nil
}

func (cr CommitteeRunner) StartNewDuty(duty types.Duty, quorum uint64) error {
Expand Down Expand Up @@ -261,7 +264,7 @@ func (cr CommitteeRunner) ProcessPostConsensus(signedMsg *types.PartialSignature
for _, att := range attestationsToSubmit {
attestations = append(attestations, att)
}

if len(attestations) > 0 {
if err := cr.beacon.SubmitAttestations(attestations); err != nil {
return errors.Wrap(err, "could not submit to Beacon chain reconstructed attestation")
Expand Down
9 changes: 7 additions & 2 deletions ssv/proposer.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ func NewProposerRunner(
operatorSigner *types.OperatorSigner,
valCheck qbft.ProposedValueCheckF,
highestDecidedSlot phase0.Slot,
) Runner {
) (Runner, error) {

if len(share) != 1 {
return nil, errors.New("must have one share")
}

return &ProposerRunner{
BaseRunner: &BaseRunner{
RunnerRoleType: types.RoleProposer,
Expand All @@ -44,7 +49,7 @@ func NewProposerRunner(
signer: signer,
operatorSigner: operatorSigner,
valCheck: valCheck,
}
}, nil
}

func (r *ProposerRunner) StartNewDuty(duty types.Duty, quorum uint64) error {
Expand Down
7 changes: 6 additions & 1 deletion ssv/spectest/all_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/partialsigcontainer"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/consensus"
runnerconstruction "github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/construction"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/duties/newduty"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/duties/proposer"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/duties/synccommitteeaggregator"
Expand Down Expand Up @@ -183,5 +184,9 @@ var AllTests = []tests.TestF{
partialsigcontainer.Quorum,
partialsigcontainer.Duplicate,
partialsigcontainer.DuplicateQuorum,
// partialsigcontainer.Invalid,
partialsigcontainer.Invalid,

runnerconstruction.OneShare,
runnerconstruction.NoShares,
runnerconstruction.ManyShares,
}
Binary file modified ssv/spectest/generate/tests.json.gz
Binary file not shown.
8 changes: 8 additions & 0 deletions ssv/spectest/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
tests2 "github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/committee"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/partialsigcontainer"
runnerconstruction "github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/construction"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/duties/newduty"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/runner/duties/synccommitteeaggregator"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests/valcheck"
Expand Down Expand Up @@ -172,6 +173,13 @@ func parseAndTest(t *testing.T, name string, test interface{}) {
Tests: typedTests,
}

typedTest.Run(t)
case reflect.TypeOf(&runnerconstruction.RunnerConstructionSpecTest{}).String():
byts, err := json.Marshal(test)
require.NoError(t, err)
typedTest := &runnerconstruction.RunnerConstructionSpecTest{}
require.NoError(t, json.Unmarshal(byts, &typedTest))

typedTest.Run(t)
default:
panic("unsupported test type " + testType)
Expand Down
28 changes: 28 additions & 0 deletions ssv/spectest/tests/runner/construction/many_shares.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package runnerconstruction

import (
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/types"
"github.com/ssvlabs/ssv-spec/types/testingutils"
)

func ManyShares() tests.SpecTest {

ks := testingutils.KeySetMapForValidators(10)
shares := testingutils.ShareMapFromKeySetMap(ks)

expectedErrors := map[types.RunnerRole]string{
types.RoleCommittee: "", // No errors since it can handle multiple shares
types.RoleProposer: "must have one share",
types.RoleAggregator: "must have one share",
types.RoleSyncCommitteeContribution: "must have one share",
types.RoleValidatorRegistration: "must have one share",
types.RoleVoluntaryExit: "must have one share",
}

return &RunnerConstructionSpecTest{
Name: "many shares",
Shares: shares,
RoleError: expectedErrors,
}
}
27 changes: 27 additions & 0 deletions ssv/spectest/tests/runner/construction/no_shares.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package runnerconstruction

import (
"github.com/attestantio/go-eth2-client/spec/phase0"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/types"
)

func NoShares() tests.SpecTest {

shares := map[phase0.ValidatorIndex]*types.Share{}

expectedErrors := map[types.RunnerRole]string{
types.RoleCommittee: "no shares",
types.RoleProposer: "must have one share",
types.RoleAggregator: "must have one share",
types.RoleSyncCommitteeContribution: "must have one share",
types.RoleValidatorRegistration: "must have one share",
types.RoleVoluntaryExit: "must have one share",
}

return &RunnerConstructionSpecTest{
Name: "no shares",
Shares: shares,
RoleError: expectedErrors,
}
}
31 changes: 31 additions & 0 deletions ssv/spectest/tests/runner/construction/one_share.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package runnerconstruction

import (
"github.com/attestantio/go-eth2-client/spec/phase0"
"github.com/ssvlabs/ssv-spec/ssv/spectest/tests"
"github.com/ssvlabs/ssv-spec/types"
"github.com/ssvlabs/ssv-spec/types/testingutils"
)

func OneShare() tests.SpecTest {
ks := testingutils.Testing4SharesSet()
shares := testingutils.ShareMapFromKeySetMap(map[phase0.ValidatorIndex]*testingutils.TestKeySet{
testingutils.TestingValidatorIndex: ks,
})

// No errors since one share must be valid for all runners
expectedErrors := map[types.RunnerRole]string{
types.RoleCommittee: "",
types.RoleProposer: "",
types.RoleAggregator: "",
types.RoleSyncCommitteeContribution: "",
types.RoleValidatorRegistration: "",
types.RoleVoluntaryExit: "",
}

return &RunnerConstructionSpecTest{
Name: "one share",
Shares: shares,
RoleError: expectedErrors,
}
}
44 changes: 44 additions & 0 deletions ssv/spectest/tests/runner/construction/test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package runnerconstruction

import (
"testing"

"github.com/attestantio/go-eth2-client/spec/phase0"
"github.com/ssvlabs/ssv-spec/types"
"github.com/ssvlabs/ssv-spec/types/testingutils"
"github.com/stretchr/testify/require"
)

type RunnerConstructionSpecTest struct {
Name string
Shares map[phase0.ValidatorIndex]*types.Share
RoleError map[types.RunnerRole]string
}

func (test *RunnerConstructionSpecTest) TestName() string {
return "RunnerConstruction " + test.Name
}

func (test *RunnerConstructionSpecTest) Run(t *testing.T) {

if len(test.RoleError) == 0 {
panic("no roles")
}

for role, expectedError := range test.RoleError {
// Construct runner and get construction error
_, err := testingutils.ConstructBaseRunnerWithShareMap(role, test.Shares)

// Check error
if len(expectedError) > 0 {
require.Error(t, err)
require.Contains(t, err.Error(), expectedError)
} else {
require.NoError(t, err)
}
}
}

func (test *RunnerConstructionSpecTest) GetPostState() (interface{}, error) {
return nil, nil
}
9 changes: 7 additions & 2 deletions ssv/sync_committee_aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ func NewSyncCommitteeAggregatorRunner(
operatorSigner *types.OperatorSigner,
valCheck qbft.ProposedValueCheckF,
highestDecidedSlot phase0.Slot,
) Runner {
) (Runner, error) {

if len(share) != 1 {
return nil, errors.New("must have one share")
}

return &SyncCommitteeAggregatorRunner{
BaseRunner: &BaseRunner{
RunnerRoleType: types.RoleSyncCommitteeContribution,
Expand All @@ -47,7 +52,7 @@ func NewSyncCommitteeAggregatorRunner(
signer: signer,
operatorSigner: operatorSigner,
valCheck: valCheck,
}
}, nil
}

func (r *SyncCommitteeAggregatorRunner) StartNewDuty(duty types.Duty, quorum uint64) error {
Expand Down
9 changes: 7 additions & 2 deletions ssv/validator_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ func NewValidatorRegistrationRunner(
network Network,
signer types.BeaconSigner,
operatorSigner *types.OperatorSigner,
) Runner {
) (Runner, error) {

if len(share) != 1 {
return nil, errors.New("must have one share")
}

return &ValidatorRegistrationRunner{
BaseRunner: &BaseRunner{
RunnerRoleType: types.RoleValidatorRegistration,
Expand All @@ -39,7 +44,7 @@ func NewValidatorRegistrationRunner(
network: network,
signer: signer,
operatorSigner: operatorSigner,
}
}, nil
}

func (r *ValidatorRegistrationRunner) StartNewDuty(duty types.Duty, quorum uint64) error {
Expand Down
9 changes: 7 additions & 2 deletions ssv/voluntary_exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ func NewVoluntaryExitRunner(
network Network,
signer types.BeaconSigner,
operatorSigner *types.OperatorSigner,
) Runner {
) (Runner, error) {

if len(share) != 1 {
return nil, errors.New("must have one share")
}

return &VoluntaryExitRunner{
BaseRunner: &BaseRunner{
RunnerRoleType: types.RoleVoluntaryExit,
Expand All @@ -41,7 +46,7 @@ func NewVoluntaryExitRunner(
network: network,
signer: signer,
operatorSigner: operatorSigner,
}
}, nil
}

func (r *VoluntaryExitRunner) StartNewDuty(duty types.Duty, quorum uint64) error {
Expand Down
8 changes: 6 additions & 2 deletions types/testingutils/committee.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ var BaseCommitteeWithCreatorFieldsFromRunner = func(keySetMap map[phase0.Validat
}

createRunnerF := func(shareMap map[phase0.ValidatorIndex]*types.Share) *ssv.CommitteeRunner {
return ssv.NewCommitteeRunner(
runner, err := ssv.NewCommitteeRunner(
runnerSample.BaseRunner.BeaconNetwork,
shareMap,
qbft.NewController(
Expand All @@ -82,7 +82,11 @@ var BaseCommitteeWithCreatorFieldsFromRunner = func(keySetMap map[phase0.Validat
runnerSample.GetSigner(),
runnerSample.GetOperatorSigner(),
runnerSample.GetValCheckF(),
).(*ssv.CommitteeRunner)
)
if err != nil {
panic(err)
}
return runner.(*ssv.CommitteeRunner)
}

return ssv.NewCommittee(
Expand Down
Loading