pr review adjustments

eth/ethtest/eth_e2e_test.go

@@ -10,7 +10,7 @@ import (
 	ethcommon "github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/crypto"
 	"github.com/stretchr/testify/require"
-	gomock "go.uber.org/mock/gomock"
+	"go.uber.org/mock/gomock"
 
 	"github.com/ssvlabs/ssv/eth/simulator/simcontract"
 	ssvtypes "github.com/ssvlabs/ssv/protocol/v2/types"
@@ -220,7 +220,8 @@ func TestEthExecLayer(t *testing.T) {
 
 			for _, event := range valRemove.events {
 				valPubKey := event.validator.masterPubKey.Serialize()
-				valShare := nodeStorage.Shares().Get(nil, valPubKey)
+				valShare, exists := nodeStorage.Shares().Get(nil, valPubKey)
+				require.False(t, exists)
 				require.Nil(t, valShare)
 			}
 		}

eth/ethtest/validator_added_test.go

@@ -82,7 +82,8 @@ func (input *testValidatorRegisteredInput) prepare(
 	for i, validatorId := range validatorsIds {
 		// Check there are no shares in the state for the current validator
 		valPubKey := validators[validatorId].masterPubKey.Serialize()
-		share := input.nodeStorage.Shares().Get(nil, valPubKey)
+		share, exists := input.nodeStorage.Shares().Get(nil, valPubKey)
+		require.False(input.t, exists)
 		require.Nil(input.t, share)
 
 		// Create event input
@@ -105,7 +106,8 @@ func (input *testValidatorRegisteredInput) produce() {
 	for _, event := range input.events {
 		val := event.validator
 		valPubKey := val.masterPubKey.Serialize()
-		shares := input.nodeStorage.Shares().Get(nil, valPubKey)
+		shares, exists := input.nodeStorage.Shares().Get(nil, valPubKey)
+		require.False(input.t, exists)
 		require.Nil(input.t, shares)
 
 		// Call the contract method

eth/ethtest/validator_exited_test.go

@@ -83,7 +83,8 @@ func (input *TestValidatorExitedEventsInput) produce() {
 	for _, event := range input.events {
 		valPubKey := event.validator.masterPubKey.Serialize()
 		// Check the validator's shares are present in the state before exiting
-		valShare := input.nodeStorage.Shares().Get(nil, valPubKey)
+		valShare, exists := input.nodeStorage.Shares().Get(nil, valPubKey)
+		require.False(input.t, exists)
 		require.NotNil(input.t, valShare)
 
 		_, err = input.boundContract.SimcontractTransactor.ExitValidator(

eth/ethtest/validator_removed_test.go

@@ -83,7 +83,8 @@ func (input *TestValidatorRemovedEventsInput) produce() {
 	for _, event := range input.events {
 		valPubKey := event.validator.masterPubKey.Serialize()
 		// Check the validator's shares are present in the state before removing
-		valShare := input.nodeStorage.Shares().Get(nil, valPubKey)
+		valShare, exists := input.nodeStorage.Shares().Get(nil, valPubKey)
+		require.False(input.t, exists)
 		require.NotNil(input.t, valShare)
 
 		_, err = input.boundContract.SimcontractTransactor.RemoveValidator(

eth/eventhandler/event_handler_test.go

@@ -688,7 +688,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 		t.Run("ValidatorExited happy flow", func(t *testing.T) {
 			valPubKey := validatorData1.masterPubKey.Serialize()
 			// Check the validator's shares are present in the state before removing
-			valShare := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			valShare, exists := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.False(t, exists)
 			require.NotNil(t, valShare)
 			valShare.BeaconMetadata = &beacon.ValidatorMetadata{
 				Index:           1,
@@ -726,7 +727,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			// Check the validator is in the validator shares storage.
 			shares := eh.nodeStorage.Shares().List(nil)
 			require.Equal(t, 4, len(shares))
-			valShare = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			valShare, exists = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.False(t, exists)
 			require.NotNil(t, valShare)
 		})
 	})
@@ -768,7 +770,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			blockNum++
 
 			// Check the validator's shares are still present in the state after incorrect ValidatorRemoved event
-			valShare := eh.nodeStorage.Shares().Get(nil, validatorData1.masterPubKey.Serialize())
+			valShare, exists := eh.nodeStorage.Shares().Get(nil, validatorData1.masterPubKey.Serialize())
+			require.False(t, exists)
 			require.NotNil(t, valShare)
 		})
 
@@ -805,7 +808,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			blockNum++
 
 			// Check the validator's shares are still present in the state after incorrect ValidatorRemoved event
-			valShare := eh.nodeStorage.Shares().Get(nil, validatorData1.masterPubKey.Serialize())
+			valShare, exists := eh.nodeStorage.Shares().Get(nil, validatorData1.masterPubKey.Serialize())
+			require.False(t, exists)
 			require.NotNil(t, valShare)
 		})
 
@@ -815,7 +819,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 		t.Run("ValidatorRemoved happy flow", func(t *testing.T) {
 			valPubKey := validatorData1.masterPubKey.Serialize()
 			// Check the validator's shares are present in the state before removing
-			valShare := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			valShare, exists := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.False(t, exists)
 			require.NotNil(t, valShare)
 			requireKeyManagerDataToExist(t, eh, 4, validatorData1)
 
@@ -851,7 +856,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			// Check the validator was removed from the validator shares storage.
 			shares := eh.nodeStorage.Shares().List(nil)
 			require.Equal(t, 3, len(shares))
-			valShare = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			valShare, exists = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.False(t, exists)
 			require.Nil(t, valShare)
 			requireKeyManagerDataToNotExist(t, eh, 3, validatorData1)
 		})
@@ -887,7 +893,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 		// Using validator 2 because we've removed validator 1 in ValidatorRemoved tests. This one has to be in the state
 		valPubKey := validatorData2.masterPubKey.Serialize()
 
-		share := eh.nodeStorage.Shares().Get(nil, valPubKey)
+		share, exists := eh.nodeStorage.Shares().Get(nil, valPubKey)
+		require.True(t, exists)
 		require.NotNil(t, share)
 		require.False(t, share.Liquidated)
 
@@ -896,7 +903,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 		require.NoError(t, err)
 		blockNum++
 
-		share = eh.nodeStorage.Shares().Get(nil, valPubKey)
+		share, exists = eh.nodeStorage.Shares().Get(nil, valPubKey)
+		require.True(t, exists)
 		require.NotNil(t, share)
 		require.True(t, share.Liquidated)
 		// check that slashing data was not deleted
@@ -1030,7 +1038,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 		// Using validator 2 because we've removed validator 1 in ValidatorRemoved tests
 		valPubKey := validatorData2.masterPubKey.Serialize()
 
-		share := eh.nodeStorage.Shares().Get(nil, valPubKey)
+		share, exists := eh.nodeStorage.Shares().Get(nil, valPubKey)
+		require.True(t, exists)
 		require.NotNil(t, share)
 		require.True(t, share.Liquidated)
 		currentSlot.SetSlot(100)
@@ -1055,7 +1064,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 
 		blockNum++
 
-		share = eh.nodeStorage.Shares().Get(nil, valPubKey)
+		share, exists = eh.nodeStorage.Shares().Get(nil, valPubKey)
+		require.True(t, exists)
 		require.NotNil(t, share)
 		require.False(t, share.Liquidated)
 	})
@@ -1160,7 +1170,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			require.NoError(t, err)
 
 			valPubKey := validatorData4.masterPubKey.Serialize()
-			valShare := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			valShare, exists := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.False(t, exists)
 			require.Nil(t, valShare)
 
 			// Call the contract method
@@ -1211,7 +1222,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			require.NoError(t, err)
 			blockNum++
 
-			valShare = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			valShare, exists = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.False(t, exists)
 			require.Nil(t, valShare)
 
 			// Check that validator was registered
@@ -1226,8 +1238,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 		t.Run("test ClusterLiquidated + ClusterReactivated events handling", func(t *testing.T) {
 			// Using validator 2 because we've removed validator 1 in ValidatorRemoved tests
 			valPubKey := validatorData2.masterPubKey.Serialize()
-			share := eh.nodeStorage.Shares().Get(nil, valPubKey)
-
+			share, exists := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.True(t, exists)
 			require.NotNil(t, share)
 			require.False(t, share.Liquidated)
 			_, err = boundContract.SimcontractTransactor.Liquidate(
@@ -1274,7 +1286,8 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			require.NoError(t, err)
 			blockNum++
 
-			share = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			share, exists = eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.True(t, exists)
 			require.NotNil(t, share)
 			require.False(t, share.Liquidated)
 		})

eth/eventhandler/handlers.go

@@ -175,9 +175,8 @@ func (eh *EventHandler) handleValidatorAdded(txn basedb.Txn, event *contract.Con
 		return nil, &MalformedEventError{Err: ErrSignatureVerification}
 	}
 
-	validatorShare := eh.nodeStorage.Shares().Get(txn, event.PublicKey)
-
-	if validatorShare == nil {
+	validatorShare, exists := eh.nodeStorage.Shares().Get(txn, event.PublicKey)
+	if exists {
 		shareCreated, err := eh.handleShareCreation(txn, event, sharePublicKeys, encryptedKeys)
 		if err != nil {
 			var malformedEventError *MalformedEventError
@@ -339,8 +338,8 @@ func (eh *EventHandler) handleValidatorRemoved(txn basedb.Txn, event *contract.C
 	logger.Debug("processing event")
 
 	// TODO: handle metrics
-	share := eh.nodeStorage.Shares().Get(txn, event.PublicKey)
-	if share == nil {
+	share, exists := eh.nodeStorage.Shares().Get(txn, event.PublicKey)
+	if !exists {
 		logger.Warn("malformed event: could not find validator share")
 		return emptyPK, &MalformedEventError{Err: ErrValidatorShareNotFound}
 	}
@@ -473,8 +472,8 @@ func (eh *EventHandler) handleValidatorExited(txn basedb.Txn, event *contract.Co
 	logger.Debug("processing event")
 	defer logger.Debug("processed event")
 
-	share := eh.nodeStorage.Shares().Get(txn, event.PublicKey)
-	if share == nil {
+	share, exists := eh.nodeStorage.Shares().Get(txn, event.PublicKey)
+	if !exists {
 		logger.Warn("malformed event: could not find validator share")
 		return nil, &MalformedEventError{Err: ErrValidatorShareNotFound}
 	}

message/validation/genesis/validation.go

@@ -32,7 +32,7 @@ import (
 	"github.com/ssvlabs/ssv/operator/duties/dutystore"
 	"github.com/ssvlabs/ssv/operator/keys"
 	operatorstorage "github.com/ssvlabs/ssv/operator/storage"
-	genesisqueue "github.com/ssvlabs/ssv/protocol/genesis/ssv/genesisqueue"
+	"github.com/ssvlabs/ssv/protocol/genesis/ssv/genesisqueue"
 	ssvmessage "github.com/ssvlabs/ssv/protocol/v2/message"
 	ssvtypes "github.com/ssvlabs/ssv/protocol/v2/types"
 )
@@ -412,10 +412,11 @@ func (mv *messageValidator) validateSSVMessage(msg *genesisqueue.GenesisSSVMessa
 	}
 
 	var share *ssvtypes.SSVShare
+	var exists bool
 	if mv.nodeStorage != nil {
 		shareStorage := mv.nodeStorage.Shares()
-		share = shareStorage.Get(nil, publicKey.Serialize())
-		if share == nil {
+		share, exists = shareStorage.Get(nil, publicKey.Serialize())
+		if !exists {
 			e := ErrUnknownValidator
 			e.got = publicKey.SerializeToHexStr()
 			return nil, descriptor, e

message/validation/validation.go

@@ -213,8 +213,8 @@ func (mv *messageValidator) getCommitteeAndValidatorIndices(msgID spectypes.Mess
 		committeeID := spectypes.CommitteeID(msgID.GetDutyExecutorID()[16:])
 
 		// Rule: Cluster does not exist
-		committee := mv.validatorStore.Committee(committeeID) // TODO: consider passing whole duty executor ID
-		if committee == nil {
+		committee, exists := mv.validatorStore.Committee(committeeID) // TODO: consider passing whole duty executor ID
+		if !exists {
 			e := ErrNonExistentCommitteeID
 			e.got = hex.EncodeToString(committeeID[:])
 			return CommitteeInfo{}, e
@@ -231,8 +231,8 @@ func (mv *messageValidator) getCommitteeAndValidatorIndices(msgID spectypes.Mess
 		}, nil
 	}
 
-	validator := mv.validatorStore.Validator(msgID.GetDutyExecutorID())
-	if validator == nil {
+	validator, exists := mv.validatorStore.Validator(msgID.GetDutyExecutorID())
+	if !exists {
 		e := ErrUnknownValidator
 		e.got = hex.EncodeToString(msgID.GetDutyExecutorID())
 		return CommitteeInfo{}, e

message/validation/validation_test.go

@@ -63,7 +63,7 @@ func Test_ValidateSSVMessage(t *testing.T) {
 
 	committeeID := shares.active.CommitteeID()
 
-	validatorStore.EXPECT().Committee(gomock.Any()).DoAndReturn(func(id spectypes.CommitteeID) *registrystorage.Committee {
+	validatorStore.EXPECT().Committee(gomock.Any()).DoAndReturn(func(id spectypes.CommitteeID) (*registrystorage.Committee, bool) {
 		if id == committeeID {
 			beaconMetadata1 := *shares.active.BeaconMetadata
 			beaconMetadata2 := beaconMetadata1
@@ -79,6 +79,7 @@ func Test_ValidateSSVMessage(t *testing.T) {
 			share3 := cloneSSVShare(share2)
 			share3.ValidatorIndex = share2.ValidatorIndex + 1
 			share3.BeaconMetadata = &beaconMetadata3
+
 			return &registrystorage.Committee{
 				ID:        id,
 				Operators: committee,
@@ -92,13 +93,13 @@ func Test_ValidateSSVMessage(t *testing.T) {
 					share2.ValidatorIndex,
 					share3.ValidatorIndex,
 				},
-			}
+			}, true
 		}
 
-		return nil
+		return nil, false
 	}).AnyTimes()
 
-	validatorStore.EXPECT().Validator(gomock.Any()).DoAndReturn(func(pubKey []byte) *ssvtypes.SSVShare {
+	validatorStore.EXPECT().Validator(gomock.Any()).DoAndReturn(func(pubKey []byte) (*ssvtypes.SSVShare, bool) {
 		for _, share := range []*ssvtypes.SSVShare{
 			shares.active,
 			shares.liquidated,
@@ -108,10 +109,10 @@ func Test_ValidateSSVMessage(t *testing.T) {
 			shares.noMetadata,
 		} {
 			if bytes.Equal(share.ValidatorPubKey[:], pubKey) {
-				return share
+				return share, true
 			}
 		}
-		return nil
+		return nil, false
 	}).AnyTimes()
 
 	signatureVerifier := signatureverifier.NewMockSignatureVerifier(ctrl)

network/p2p/p2p_genesis.go

@@ -40,8 +40,8 @@ func (p *GenesisP2P) Broadcast(message *genesisspectypes.SSVMessage) error {
 	}
 	encodedMsg = commons.EncodeGenesisSignedSSVMessage(encodedMsg, p.Network.operatorDataStore.GetOperatorID(), signature)
 
-	share := p.Network.nodeStorage.ValidatorStore().Validator(message.MsgID.GetPubKey())
-	if share == nil {
+	_, exists := p.Network.nodeStorage.ValidatorStore().Validator(message.MsgID.GetPubKey())
+	if !exists {
 		return fmt.Errorf("could not find share for validator %s", hex.EncodeToString(message.MsgID.GetPubKey()))
 	}
 

network/p2p/p2p_pubsub.go

@@ -56,8 +56,8 @@ func (n *p2pNetwork) Broadcast(msgID spectypes.MessageID, msg *spectypes.SignedS
 	if msg.SSVMessage.MsgID.GetRoleType() == spectypes.RoleCommittee {
 		topics = commons.CommitteeTopicID(spectypes.CommitteeID(msg.SSVMessage.MsgID.GetDutyExecutorID()[16:]))
 	} else {
-		val := n.nodeStorage.ValidatorStore().Validator(msg.SSVMessage.MsgID.GetDutyExecutorID())
-		if val == nil {
+		val, exists := n.nodeStorage.ValidatorStore().Validator(msg.SSVMessage.MsgID.GetDutyExecutorID())
+		if !exists {
 			return fmt.Errorf("could not find share for validator %s", hex.EncodeToString(msg.SSVMessage.MsgID.GetDutyExecutorID()))
 		}
 		topics = commons.CommitteeTopicID(val.CommitteeID())
@@ -123,8 +123,8 @@ func (n *p2pNetwork) Subscribe(pk spectypes.ValidatorPK) error {
 		return p2pprotocol.ErrNetworkIsNotReady
 	}
 
-	share := n.nodeStorage.ValidatorStore().Validator(pk[:])
-	if share == nil {
+	share, exists := n.nodeStorage.ValidatorStore().Validator(pk[:])
+	if !exists {
 		return fmt.Errorf("could not find share for validator %s", hex.EncodeToString(pk[:]))
 	}
 
@@ -199,8 +199,8 @@ func (n *p2pNetwork) Unsubscribe(logger *zap.Logger, pk spectypes.ValidatorPK) e
 		n.activeValidators.Del(pkHex)
 	}
 
-	share := n.nodeStorage.ValidatorStore().Validator(pk[:])
-	if share == nil {
+	share, exists := n.nodeStorage.ValidatorStore().Validator(pk[:])
+	if !exists {
 		return fmt.Errorf("could not find share for validator %s", hex.EncodeToString(pk[:]))
 	}
 

operator/duties/scheduler.go

@@ -83,7 +83,6 @@ type ExecutionClient interface {
 type ValidatorProvider interface {
 	ParticipatingValidators(epoch phase0.Epoch) []*types.SSVShare
 	SelfParticipatingValidators(epoch phase0.Epoch) []*types.SSVShare
-	Validator(pubKey []byte) *types.SSVShare
 }
 
 // ValidatorController represents the component that controls validators via the scheduler