Feat: Mark invalid shares

eth/eventhandler/event_handler_test.go

@@ -417,6 +417,7 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			// Corrupt the encrypted last share key of the 4th operator
 			malformedSharesData[len(malformedSharesData)-1] ^= 1
 
+			valPubKey := validatorData2.masterPubKey.Serialize()
 			// Call the contract method
 			_, err = boundContract.SimcontractTransactor.RegisterValidator(
 				auth,
@@ -454,65 +455,87 @@ func TestHandleBlockEventsStream(t *testing.T) {
 			// Check that validator was registered for op1,
 			shares = eh.nodeStorage.Shares().List(nil)
 			require.Equal(t, 2, len(shares))
+			valShare := eh.nodeStorage.Shares().Get(nil, valPubKey)
+			require.NotNil(t, valShare)
+			require.False(t, valShare.InvalidSecret)
 			// and nonce was bumped
 			nonce, err = eh.nodeStorage.GetNextNonce(nil, testAddr)
 			require.NoError(t, err)
 			require.Equal(t, registrystorage.Nonce(3), nonce)
 		})
 
-		// Share for 1st operator is malformed; check nonce is bumped correctly; validator wasn't added
-		// slashing protection data is not added
-		t.Run("test malformed ValidatorAdded and nonce is bumped", func(t *testing.T) {
-			malformedSharesData := sharesData3[:]
-
-			operatorCount := len(ops)
-			signatureOffset := phase0.SignatureLength
-			pubKeysOffset := phase0.PublicKeyLength*operatorCount + signatureOffset
-
-			// Corrupt the encrypted share key of the operator 1
-			malformedSharesData[pubKeysOffset+encryptedKeyLength-1] ^= 1
-
-			// Call the contract method
-			_, err = boundContract.SimcontractTransactor.RegisterValidator(
-				auth,
-				validatorData3.masterPubKey.Serialize(),
-				[]uint64{1, 2, 3, 4},
-				malformedSharesData,
-				big.NewInt(100_000_000),
-				simcontract.CallableCluster{
-					ValidatorCount:  1,
-					NetworkFeeIndex: 1,
-					Index:           2,
-					Active:          true,
-					Balance:         big.NewInt(100_000_000),
-				})
+		t.Run("test invalid encrypted shares", func(t *testing.T) {
+			validatorData, err := createNewValidator(ops)
 			require.NoError(t, err)
-			sim.Commit()
-
-			block = <-logs
-			require.NotEmpty(t, block.Logs)
-			require.Equal(t, ethcommon.HexToHash("0x48a3ea0796746043948f6341d17ff8200937b99262a0b48c2663b951ed7114e5"), block.Logs[0].Topics[0])
-
-			eventsCh = make(chan executionclient.BlockLogs)
-			go func() {
-				defer close(eventsCh)
-				eventsCh <- block
-			}()
-
-			lastProcessedBlock, err = eh.HandleBlockEventsStream(eventsCh, false)
-			require.NoError(t, err)
-			require.Equal(t, blockNum+1, lastProcessedBlock)
-			blockNum++
-
-			requireKeyManagerDataToNotExist(t, eh, 2, validatorData3)
-
-			// Check that validator was not registered
-			shares = eh.nodeStorage.Shares().List(nil)
-			require.Equal(t, 2, len(shares))
-			// and nonce was bumped
-			nonce, err = eh.nodeStorage.GetNextNonce(nil, testAddr)
-			require.NoError(t, err)
-			require.Equal(t, registrystorage.Nonce(4), nonce)
+			t.Run("valid signature + wrong op1 key, nonce is bumped, share is saved, marked as invalid", func(t *testing.T) {
+				sharesData, err := generateSharesData(validatorData, ops, testAddr, 3)
+				require.NoError(t, err)
+
+				// Generating a new list of operators, but with changed op1 rsa keys
+				pb, sk, _ := rsaencryption.GenerateKeys()
+				wrongOps := make([]*testOperator, 0)
+				wrongOps = append(wrongOps, &testOperator{id: 1, rsaPub: pb, rsaPriv: sk})
+
+				wrongOps = append(wrongOps, ops[1:]...)
+
+				require.Equal(t, len(wrongOps), len(ops))
+				require.NotEqual(t, wrongOps[0].rsaPub, ops[0].rsaPub)
+
+				incorrectSharesData, err := generateSharesData(validatorData, wrongOps, testAddr, 4)
+				require.NoError(t, err)
+
+				signatureOffset := phase0.SignatureLength
+				// Replace incorrect signature with a correct one
+				for i := 0; i < signatureOffset; i++ {
+					incorrectSharesData[i] = sharesData[i]
+				}
+
+				valPubKey := validatorData.masterPubKey.Serialize()
+				// Call the contract method
+				_, err = boundContract.SimcontractTransactor.RegisterValidator(
+					auth,
+					valPubKey,
+					[]uint64{1, 2, 3, 4},
+					incorrectSharesData,
+					big.NewInt(100_000_000),
+					simcontract.CallableCluster{
+						ValidatorCount:  1,
+						NetworkFeeIndex: 1,
+						Index:           2,
+						Active:          true,
+						Balance:         big.NewInt(100_000_000),
+					})
+				require.NoError(t, err)
+				sim.Commit()
+
+				block = <-logs
+				require.NotEmpty(t, block.Logs)
+				require.Equal(t, ethcommon.HexToHash("0x48a3ea0796746043948f6341d17ff8200937b99262a0b48c2663b951ed7114e5"), block.Logs[0].Topics[0])
+
+				eventsCh = make(chan executionclient.BlockLogs)
+				go func() {
+					defer close(eventsCh)
+					eventsCh <- block
+				}()
+
+				lastProcessedBlock, err = eh.HandleBlockEventsStream(eventsCh, false)
+				require.NoError(t, err)
+				require.Equal(t, blockNum+1, lastProcessedBlock)
+				blockNum++
+
+				requireKeyManagerDataToNotExist(t, eh, 2, validatorData)
+
+				// Check that validator was not registered
+				shares = eh.nodeStorage.Shares().List(nil)
+				require.Equal(t, 3, len(shares))
+				valShare := eh.nodeStorage.Shares().Get(nil, valPubKey)
+				require.NotNil(t, valShare)
+				require.True(t, valShare.InvalidSecret)
+				// and nonce was bumped
+				nonce, err = eh.nodeStorage.GetNextNonce(nil, testAddr)
+				require.NoError(t, err)
+				require.Equal(t, registrystorage.Nonce(4), nonce)
+			})
 		})
 
 		// Correct event; check nonce is bumped correctly; validator is added
@@ -557,7 +580,7 @@ func TestHandleBlockEventsStream(t *testing.T) {
 
 			// Check that validator was registered
 			shares = eh.nodeStorage.Shares().List(nil)
-			require.Equal(t, 3, len(shares))
+			require.Equal(t, 4, len(shares))
 			// and nonce was bumped
 			nonce, err = eh.nodeStorage.GetNextNonce(nil, testAddr)
 			require.NoError(t, err)
@@ -607,7 +630,7 @@ func TestHandleBlockEventsStream(t *testing.T) {
 
 			// Check that validator was registered
 			shares = eh.nodeStorage.Shares().List(nil)
-			require.Equal(t, 4, len(shares))
+			require.Equal(t, 5, len(shares))
 			// and nonce was bumped
 			nonce, err = eh.nodeStorage.GetNextNonce(nil, testAddr2)
 			require.NoError(t, err)
@@ -709,7 +732,7 @@ 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))
+			require.Equal(t, 5, len(shares))
 			valShare = eh.nodeStorage.Shares().Get(nil, valPubKey)
 			require.NotNil(t, valShare)
 		})
@@ -834,7 +857,7 @@ 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))
+			require.Equal(t, 4, len(shares))
 			valShare = eh.nodeStorage.Shares().Get(nil, valPubKey)
 			require.Nil(t, valShare)
 			requireKeyManagerDataToNotExist(t, eh, 3, validatorData1)

eth/eventhandler/handlers.go

@@ -211,8 +211,13 @@ func (eh *EventHandler) handleValidatorAdded(txn basedb.Txn, event *contract.Con
 	isOperatorShare := validatorShare.BelongsToOperator(eh.operatorData.GetOperatorData().ID)
 	if isOperatorShare {
 		eh.metrics.ValidatorInactive(event.PublicKey)
-		ownShare = validatorShare
 		logger = logger.With(zap.Bool("own_validator", isOperatorShare))
+		if validatorShare.InvalidSecret {
+			logger = logger.With(zap.Bool("invalid_secret", true))
+			logger.Debug("registered validator has an invalid share secret, it will not be started")
+		} else {
+			ownShare = validatorShare
+		}
 	}
 
 	logger.Debug("processed event")
@@ -233,11 +238,12 @@ func (eh *EventHandler) handleShareCreation(
 		sharePublicKeys,
 		encryptedKeys,
 	)
-	if err != nil {
-		return nil, fmt.Errorf("could not extract validator share from event: %w", err)
+
+	if err != nil && !share.Metadata.InvalidSecret {
+		return share, err
 	}
 
-	if share.BelongsToOperator(eh.operatorData.GetOperatorData().ID) {
+	if !share.Metadata.InvalidSecret && share.BelongsToOperator(eh.operatorData.GetOperatorData().ID) {
 		if shareSecret == nil {
 			return nil, errors.New("could not decode shareSecret")
 		}
@@ -253,7 +259,11 @@ func (eh *EventHandler) handleShareCreation(
 		return nil, fmt.Errorf("could not save validator share: %w", err)
 	}
 
-	return share, nil
+	if !share.Metadata.InvalidSecret {
+		return share, nil
+	}
+
+	return share, err
 }
 
 func (eh *EventHandler) validatorAddedEventToShare(
@@ -263,16 +273,17 @@ func (eh *EventHandler) validatorAddedEventToShare(
 	sharePublicKeys [][]byte,
 	encryptedKeys [][]byte,
 ) (*ssvtypes.SSVShare, *bls.SecretKey, error) {
-	validatorShare := ssvtypes.SSVShare{}
+	validatorShare := &ssvtypes.SSVShare{}
 
 	publicKey, err := ssvtypes.DeserializeBLSPublicKey(event.PublicKey)
 	if err != nil {
-		return nil, nil, &MalformedEventError{
+		return validatorShare, nil, &MalformedEventError{
 			Err: fmt.Errorf("failed to deserialize validator public key: %w", err),
 		}
 	}
 	validatorShare.ValidatorPubKey = publicKey.Serialize()
 	validatorShare.OwnerAddress = event.Owner
+
 	var shareSecret *bls.SecretKey
 
 	committee := make([]*spectypes.Operator, 0)
@@ -300,29 +311,33 @@ func (eh *EventHandler) validatorAddedEventToShare(
 
 		shareSecret = &bls.SecretKey{}
 		decryptedSharePrivateKey, err := rsaencryption.DecodeKey(operatorPrivateKey, encryptedKeys[i])
+
+		validatorShare.Metadata.InvalidSecret = true
+
 		if err != nil {
-			return nil, nil, &MalformedEventError{
+			return validatorShare, nil, &MalformedEventError{
 				Err: fmt.Errorf("could not decrypt share private key: %w", err),
 			}
 		}
 		if err = shareSecret.SetHexString(string(decryptedSharePrivateKey)); err != nil {
-			return nil, nil, &MalformedEventError{
+			return validatorShare, nil, &MalformedEventError{
 				Err: fmt.Errorf("could not set decrypted share private key: %w", err),
 			}
 		}
 		if !bytes.Equal(shareSecret.GetPublicKey().Serialize(), validatorShare.SharePubKey) {
-			return nil, nil, &MalformedEventError{
+			return validatorShare, nil, &MalformedEventError{
 				Err: errors.New("share private key does not match public key"),
 			}
 		}
+		validatorShare.Metadata.InvalidSecret = false
 	}
 
 	validatorShare.Quorum, validatorShare.PartialQuorum = ssvtypes.ComputeQuorumAndPartialQuorum(len(committee))
 	validatorShare.DomainType = eh.networkConfig.Domain
 	validatorShare.Committee = committee
 	validatorShare.Graffiti = []byte("ssv.network")
 
-	return &validatorShare, shareSecret, nil
+	return validatorShare, shareSecret, nil
 }
 
 func (eh *EventHandler) handleValidatorRemoved(txn basedb.Txn, event *contract.ContractValidatorRemoved) (spectypes.ValidatorPK, error) {

operator/validator/controller.go

@@ -419,7 +419,10 @@ func (c *controller) StartValidators() {
 		return
 	}
 
-	shares := c.sharesStorage.List(nil, registrystorage.ByNotLiquidated())
+	shares := c.sharesStorage.List(nil,
+		registrystorage.ByNotLiquidated(),
+		registrystorage.ByValidSecret(),
+	)
 	if len(shares) == 0 {
 		c.logger.Info("could not find validators")
 		return
@@ -520,7 +523,10 @@ func (c *controller) startValidators(validators []*validator.Validator) int {
 // to start consensus flow which would save the highest decided instance
 // and sync any gaps (in protocol/v2/qbft/controller/decided.go).
 func (c *controller) setupNonCommitteeValidators() {
-	nonCommitteeShares := c.sharesStorage.List(nil, registrystorage.ByNotLiquidated())
+	nonCommitteeShares := c.sharesStorage.List(nil,
+		registrystorage.ByNotLiquidated(),
+		registrystorage.ByValidSecret(),
+	)
 	if len(nonCommitteeShares) == 0 {
 		c.logger.Info("could not find non-committee validators")
 		return
@@ -720,15 +726,21 @@ func (c *controller) onShareStop(pubKey spectypes.ValidatorPK) {
 }
 
 func (c *controller) onShareInit(share *ssvtypes.SSVShare) (*validator.Validator, error) {
+	validatorPkHexString := hex.EncodeToString(share.ValidatorPubKey)
+
 	if !share.HasBeaconMetadata() { // fetching index and status in case not exist
 		c.logger.Warn("skipping validator until it becomes active", fields.PubKey(share.ValidatorPubKey))
 		return nil, nil
 	}
+
+	if share.InvalidSecret {
+		return nil, fmt.Errorf("skipping validator with invalid share. validatorPubKey: %s", validatorPkHexString)
+	}
 	if err := c.setShareFeeRecipient(share, c.recipientsStorage.GetRecipientData); err != nil {
 		return nil, fmt.Errorf("could not set share fee recipient: %w", err)
 	}
 	// Start a committee validator.
-	v, found := c.validatorsMap.GetValidator(hex.EncodeToString(share.ValidatorPubKey))
+	v, found := c.validatorsMap.GetValidator(validatorPkHexString)
 	if !found {
 		if !share.HasBeaconMetadata() {
 			return nil, fmt.Errorf("beacon metadata is missing")
@@ -741,7 +753,7 @@ func (c *controller) onShareInit(share *ssvtypes.SSVShare) (*validator.Validator
 		opts.SSVShare = share
 		opts.DutyRunners = SetupRunners(ctx, c.logger, opts)
 		v = validator.NewValidator(ctx, cancel, opts)
-		c.validatorsMap.CreateValidator(hex.EncodeToString(share.ValidatorPubKey), v)
+		c.validatorsMap.CreateValidator(validatorPkHexString, v)
 
 		c.printShare(share, "setup validator done")
 
@@ -754,6 +766,7 @@ func (c *controller) onShareInit(share *ssvtypes.SSVShare) (*validator.Validator
 
 func (c *controller) onShareStart(share *ssvtypes.SSVShare) (bool, error) {
 	v, err := c.onShareInit(share)
+
 	if err != nil || v == nil {
 		return false, err
 	}
@@ -827,7 +840,10 @@ func (c *controller) UpdateValidatorMetaDataLoop() {
 	filters := []registrystorage.SharesFilter{}
 
 	// Filter for validators who are not liquidated.
-	filters = append(filters, registrystorage.ByNotLiquidated())
+	filters = append(filters,
+		registrystorage.ByNotLiquidated(),
+		registrystorage.ByValidSecret(),
+	)
 
 	// Filter for validators which haven't been updated recently.
 	filters = append(filters, func(s *ssvtypes.SSVShare) bool {

protocol/v2/types/ssvshare.go

@@ -16,8 +16,8 @@ import (
 )
 
 const (
-	MaxPossibleShareSize = 1245
-	MaxAllowedShareSize  = MaxPossibleShareSize * 8 // Leaving some room for protocol updates and calculation mistakes.
+	MaxPossibleShareSize = 1265
+	MaxAllowedShareSize  = 1245 * 8 // Leaving some room for protocol updates and calculation mistakes. 1245 - initial share max size
 )
 
 // SSVShare is a combination of spectypes.Share and its Metadata.
@@ -100,4 +100,5 @@ type Metadata struct {
 	BeaconMetadata *beaconprotocol.ValidatorMetadata
 	OwnerAddress   common.Address
 	Liquidated     bool
+	InvalidSecret  bool
 }

registry/storage/shares.go

@@ -198,6 +198,13 @@ func ByNotLiquidated() SharesFilter {
 	}
 }
 
+// ByValidSecret filters for not invalid secret.
+func ByValidSecret() SharesFilter {
+	return func(share *types.SSVShare) bool {
+		return !share.InvalidSecret
+	}
+}
+
 // ByActiveValidator filters for active validators.
 func ByActiveValidator() SharesFilter {
 	return func(share *types.SSVShare) bool {