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

CommitteeDutyGuard: share between committees #1788

Open
wants to merge 4 commits into
base: stage
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
16 changes: 13 additions & 3 deletions operator/validator/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import (
genesisspectypes "github.com/ssvlabs/ssv-spec-pre-cc/types"
specqbft "github.com/ssvlabs/ssv-spec/qbft"
spectypes "github.com/ssvlabs/ssv-spec/types"
"go.uber.org/zap"

"github.com/ssvlabs/ssv/exporter/convert"
"github.com/ssvlabs/ssv/ibft/genesisstorage"
"github.com/ssvlabs/ssv/ibft/storage"
Expand Down Expand Up @@ -59,6 +57,7 @@ import (
ssvtypes "github.com/ssvlabs/ssv/protocol/v2/types"
registrystorage "github.com/ssvlabs/ssv/registry/storage"
"github.com/ssvlabs/ssv/storage/basedb"
"go.uber.org/zap"
)

//go:generate mockgen -package=mocks -destination=./mocks/controller.go -source=./controller.go
Expand Down Expand Up @@ -199,6 +198,7 @@ type controller struct {
validatorsMap *validators.ValidatorsMap
validatorStartFunc func(validator *validators.ValidatorContainer) (bool, error)
committeeValidatorSetup chan struct{}
dutyGuard *validator.CommitteeDutyGuard

metadataUpdateInterval time.Duration

Expand Down Expand Up @@ -323,6 +323,7 @@ func NewController(logger *zap.Logger, options ControllerOptions) Controller {
indicesChange: make(chan struct{}),
validatorExitCh: make(chan duties.ExitDescriptor),
committeeValidatorSetup: make(chan struct{}, 1),
dutyGuard: validator.NewCommitteeDutyGuard(),

messageValidator: options.MessageValidator,
}
Expand Down Expand Up @@ -964,7 +965,16 @@ func (c *controller) onShareInit(share *ssvtypes.SSVShare) (*validators.Validato

committeeRunnerFunc := SetupCommitteeRunners(ctx, opts)

vc = validator.NewCommittee(ctx, cancel, logger, c.beacon.GetBeaconNetwork(), operator, committeeRunnerFunc, nil)
vc = validator.NewCommittee(
ctx,
cancel,
logger,
c.beacon.GetBeaconNetwork(),
operator,
committeeRunnerFunc,
nil,
c.dutyGuard,
)
vc.AddShare(&share.Share)
c.validatorsMap.PutCommittee(operator.CommitteeID, vc)

Expand Down
8 changes: 4 additions & 4 deletions protocol/v2/ssv/spectest/msg_processing_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@ import (
spectypes "github.com/ssvlabs/ssv-spec/types"
spectestingutils "github.com/ssvlabs/ssv-spec/types/testingutils"
typescomparable "github.com/ssvlabs/ssv-spec/types/testingutils/comparable"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/ssvlabs/ssv/integration/qbft/tests"
"github.com/ssvlabs/ssv/logging"
"github.com/ssvlabs/ssv/networkconfig"
Expand All @@ -27,6 +24,8 @@ import (
ssvprotocoltesting "github.com/ssvlabs/ssv/protocol/v2/ssv/testing"
"github.com/ssvlabs/ssv/protocol/v2/ssv/validator"
protocoltesting "github.com/ssvlabs/ssv/protocol/v2/testing"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
)

type MsgProcessingSpecTest struct {
Expand Down Expand Up @@ -248,7 +247,7 @@ var baseCommitteeWithRunnerSample = func(
logger *zap.Logger,
keySetMap map[phase0.ValidatorIndex]*spectestingutils.TestKeySet,
runnerSample *runner.CommitteeRunner,
committeeDutyGuard runner.CommitteeDutyGuard,
committeeDutyGuard *validator.CommitteeDutyGuard,
) *validator.Committee {

var keySetSample *spectestingutils.TestKeySet
Expand Down Expand Up @@ -292,6 +291,7 @@ var baseCommitteeWithRunnerSample = func(
spectestingutils.TestingCommitteeMember(keySetSample),
createRunnerF,
shareMap,
committeeDutyGuard,
)

return c
Expand Down
6 changes: 3 additions & 3 deletions protocol/v2/ssv/spectest/ssv_mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ import (
spectypes "github.com/ssvlabs/ssv-spec/types"
"github.com/ssvlabs/ssv-spec/types/testingutils"
spectestingutils "github.com/ssvlabs/ssv-spec/types/testingutils"
"github.com/stretchr/testify/require"
"go.uber.org/zap"

"github.com/ssvlabs/ssv/exporter/convert"
tests2 "github.com/ssvlabs/ssv/integration/qbft/tests"
"github.com/ssvlabs/ssv/logging"
Expand All @@ -35,6 +32,8 @@ import (
ssvtesting "github.com/ssvlabs/ssv/protocol/v2/ssv/testing"
"github.com/ssvlabs/ssv/protocol/v2/ssv/validator"
protocoltesting "github.com/ssvlabs/ssv/protocol/v2/testing"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
)

func TestSSVMapping(t *testing.T) {
Expand Down Expand Up @@ -564,6 +563,7 @@ func fixCommitteeForRun(t *testing.T, ctx context.Context, logger *zap.Logger, c
return r.(*runner.CommitteeRunner), nil
},
specCommittee.Share,
validator.NewCommitteeDutyGuard(),
)
tmpSsvCommittee := &validator.Committee{}
require.NoError(t, json.Unmarshal(byts, tmpSsvCommittee))
Expand Down
18 changes: 9 additions & 9 deletions protocol/v2/ssv/testing/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@ import (
specqbft "github.com/ssvlabs/ssv-spec/qbft"
spectypes "github.com/ssvlabs/ssv-spec/types"
spectestingutils "github.com/ssvlabs/ssv-spec/types/testingutils"
"go.uber.org/zap"

"github.com/ssvlabs/ssv/protocol/v2/qbft/controller"
"github.com/ssvlabs/ssv/protocol/v2/ssv"

"github.com/ssvlabs/ssv/exporter/convert"
"github.com/ssvlabs/ssv/integration/qbft/tests"
"github.com/ssvlabs/ssv/networkconfig"
"github.com/ssvlabs/ssv/protocol/v2/qbft/controller"
"github.com/ssvlabs/ssv/protocol/v2/qbft/testing"
"github.com/ssvlabs/ssv/protocol/v2/ssv"
"github.com/ssvlabs/ssv/protocol/v2/ssv/runner"
"github.com/ssvlabs/ssv/protocol/v2/ssv/validator"
"go.uber.org/zap"
)

var TestingHighestDecidedSlot = phase0.Slot(0)
Expand Down Expand Up @@ -117,6 +115,7 @@ var ConstructBaseRunner = func(

shareMap := make(map[phase0.ValidatorIndex]*spectypes.Share)
shareMap[share.ValidatorIndex] = share
dutyGuard := validator.NewCommitteeDutyGuard()

var r runner.Runner
var err error
Expand All @@ -132,7 +131,7 @@ var ConstructBaseRunner = func(
km,
opSigner,
valCheck,
validator.NewCommitteeDutyGuard(),
dutyGuard,
)
case spectypes.RoleAggregator:
r, err = runner.NewAggregatorRunner(
Expand Down Expand Up @@ -204,7 +203,7 @@ var ConstructBaseRunner = func(
km,
opSigner,
valCheck,
validator.NewCommitteeDutyGuard(),
dutyGuard,
)
r.(*runner.CommitteeRunner).BaseRunner.RunnerRoleType = spectestingutils.UnknownDutyType
default:
Expand Down Expand Up @@ -300,6 +299,7 @@ var ConstructBaseRunnerWithShareMap = func(
var contr *controller.Controller

km := spectestingutils.NewTestingKeyManager()
dutyGuard := validator.NewCommitteeDutyGuard()

if len(shareMap) > 0 {
var keySetInstance *spectestingutils.TestKeySet
Expand Down Expand Up @@ -382,7 +382,7 @@ var ConstructBaseRunnerWithShareMap = func(
km,
opSigner,
valCheck,
validator.NewCommitteeDutyGuard(),
dutyGuard,
)
case spectypes.RoleAggregator:
r, err = runner.NewAggregatorRunner(
Expand Down Expand Up @@ -454,7 +454,7 @@ var ConstructBaseRunnerWithShareMap = func(
km,
opSigner,
valCheck,
validator.NewCommitteeDutyGuard(),
dutyGuard,
)
if r != nil {
r.(*runner.CommitteeRunner).BaseRunner.RunnerRoleType = spectestingutils.UnknownDutyType
Expand Down
6 changes: 3 additions & 3 deletions protocol/v2/ssv/validator/committee.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,13 @@ import (
"github.com/pkg/errors"
"github.com/ssvlabs/ssv-spec/qbft"
spectypes "github.com/ssvlabs/ssv-spec/types"
"go.uber.org/zap"

"github.com/ssvlabs/ssv/ibft/storage"
"github.com/ssvlabs/ssv/logging/fields"
"github.com/ssvlabs/ssv/protocol/v2/message"
"github.com/ssvlabs/ssv/protocol/v2/ssv/queue"
"github.com/ssvlabs/ssv/protocol/v2/ssv/runner"
"github.com/ssvlabs/ssv/protocol/v2/types"
"go.uber.org/zap"
)

var (
Expand Down Expand Up @@ -57,6 +56,7 @@ func NewCommittee(
committeeMember *spectypes.CommitteeMember,
createRunnerFn CommitteeRunnerFunc,
shares map[phase0.ValidatorIndex]*spectypes.Share,
dutyGuard *CommitteeDutyGuard,
) *Committee {
if shares == nil {
shares = make(map[phase0.ValidatorIndex]*spectypes.Share)
Expand All @@ -71,7 +71,7 @@ func NewCommittee(
Shares: shares,
CommitteeMember: committeeMember,
CreateRunnerFn: createRunnerFn,
dutyGuard: NewCommitteeDutyGuard(),
dutyGuard: dutyGuard,
}
}

Expand Down
5 changes: 5 additions & 0 deletions protocol/v2/ssv/validator/committee_guard.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ func (a *CommitteeDutyGuard) StartDuty(role spectypes.BeaconRole, validator spec
if !ok {
return fmt.Errorf("unsupported role %d", role)
}
// If an older committee duty is still running for this validator we won't be interested in it
// anymore now that we have a fresher duty started. The older duty might or might not finish
// successfully, either outcome is fine but since it's always better to execute the freshest
// committee duty CommitteeDutyGuard will invalidate the older duty (potentially preventing it
// from execution so that we don't waste resources on it).
runningSlot, exists := duties[validator]
if exists && runningSlot >= slot {
return fmt.Errorf("duty already running at slot %d", runningSlot)
Expand Down