mark invalid encrypted shares as 'invalid'

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,88 @@ 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.Invalid)
 			// 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),
-				})
-			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)
+		t.Run("test invalid encrypted shares", func(t *testing.T) {
+			validatorData, err := createNewValidator(ops)
 			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, pub: pb, priv: sk})
+
+				wrongOps = append(wrongOps, ops[1:]...)
+
+				require.Equal(t, len(wrongOps), len(ops))
+				require.NotEqual(t, wrongOps[0].pub, ops[0].pub)
+
+				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.Invalid)
+				// 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 +581,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 +631,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)
@@ -736,7 +760,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)
@@ -1464,7 +1488,6 @@ func generateSharesData(validatorData *testValidatorData, operators []*testOpera
 
 		pubkeys = append(pubkeys, validatorData.operatorsShares[i].pub.Serialize()...)
 		encryptedShares = append(encryptedShares, ciphertext...)
-
 	}
 
 	tosign := fmt.Sprintf("%s:%d", owner.String(), nonce)

eth/eventhandler/handlers.go

@@ -179,12 +179,12 @@ func (eh *EventHandler) handleValidatorAdded(txn basedb.Txn, event *contract.Con
 	}
 
 	validatorShare := eh.nodeStorage.Shares().Get(txn, event.PublicKey)
+	var malformedEventError *MalformedEventError
 
 	if validatorShare == nil {
 		createdShare, err := eh.handleShareCreation(txn, event, sharePublicKeys, encryptedKeys)
 		if err != nil {
-			var malformedEventError *MalformedEventError
-			if errors.As(err, &malformedEventError) {
+			if errors.As(err, &malformedEventError) && malformedEventError != nil && !malformedEventError.IsInvalidEncryptedShare {
 				logger.Warn("malformed event", zap.Error(err))
 
 				return nil, err
@@ -209,7 +209,7 @@ func (eh *EventHandler) handleValidatorAdded(txn basedb.Txn, event *contract.Con
 	}
 
 	isOperatorShare := validatorShare.BelongsToOperator(eh.operatorData.GetOperatorData().ID)
-	if isOperatorShare {
+	if isOperatorShare && !validatorShare.Invalid {
 		eh.metrics.ValidatorInactive(event.PublicKey)
 		ownShare = validatorShare
 		logger = logger.With(zap.Bool("own_validator", isOperatorShare))
@@ -233,11 +233,21 @@ func (eh *EventHandler) handleShareCreation(
 		sharePublicKeys,
 		encryptedKeys,
 	)
+
+	var malformedEventError *MalformedEventError = nil
+
 	if err != nil {
-		return nil, fmt.Errorf("could not extract validator share from event: %w", err)
+		if errors.As(err, &malformedEventError) &&
+			malformedEventError.IsInvalidEncryptedShare &&
+			share.BelongsToOperator(eh.operatorData.GetOperatorData().ID) {
+
+			share.Metadata.Invalid = true
+		} else {
+			return nil, fmt.Errorf("could not extract validator share from event: %w", err)
+		}
 	}
 
-	if share.BelongsToOperator(eh.operatorData.GetOperatorData().ID) {
+	if malformedEventError == nil && share.BelongsToOperator(eh.operatorData.GetOperatorData().ID) {
 		if shareSecret == nil {
 			return nil, errors.New("could not decode shareSecret")
 		}
@@ -253,7 +263,11 @@ func (eh *EventHandler) handleShareCreation(
 		return nil, fmt.Errorf("could not save validator share: %w", err)
 	}
 
-	return share, nil
+	if malformedEventError == nil {
+		return share, nil
+	}
+
+	return share, malformedEventError
 }
 
 func validatorAddedEventToShare(
@@ -301,18 +315,21 @@ func validatorAddedEventToShare(
 		shareSecret = &bls.SecretKey{}
 		decryptedSharePrivateKey, err := rsaencryption.DecodeKey(operatorPrivateKey, encryptedKeys[i])
 		if err != nil {
-			return nil, nil, &MalformedEventError{
-				Err: fmt.Errorf("could not decrypt share private key: %w", err),
+			return &validatorShare, nil, &MalformedEventError{
+				Err:                     fmt.Errorf("could not decrypt share private key: %w", err),
+				IsInvalidEncryptedShare: true,
 			}
 		}
 		if err = shareSecret.SetHexString(string(decryptedSharePrivateKey)); err != nil {
-			return nil, nil, &MalformedEventError{
-				Err: fmt.Errorf("could not set decrypted share private key: %w", err),
+			return &validatorShare, nil, &MalformedEventError{
+				Err:                     fmt.Errorf("could not set decrypted share private key: %w", err),
+				IsInvalidEncryptedShare: true,
 			}
 		}
 		if !bytes.Equal(shareSecret.GetPublicKey().Serialize(), validatorShare.SharePubKey) {
-			return nil, nil, &MalformedEventError{
-				Err: errors.New("share private key does not match public key"),
+			return &validatorShare, nil, &MalformedEventError{
+				Err:                     fmt.Errorf("share private key does not match public key"),
+				IsInvalidEncryptedShare: true,
 			}
 		}
 	}
@@ -321,6 +338,7 @@ func validatorAddedEventToShare(
 	validatorShare.DomainType = ssvtypes.GetDefaultDomain()
 	validatorShare.Committee = committee
 	validatorShare.Graffiti = []byte("ssv.network")
+	validatorShare.Metadata.Invalid = false
 
 	return &validatorShare, shareSecret, nil
 }
@@ -520,7 +538,8 @@ func (eh *EventHandler) processClusterEvent(
 
 // MalformedEventError is returned when event is malformed
 type MalformedEventError struct {
-	Err error
+	Err                     error
+	IsInvalidEncryptedShare bool
 }
 
 func (e *MalformedEventError) Error() string {

operator/validator/controller.go

@@ -686,6 +686,10 @@ func (c *controller) onShareStart(share *ssvtypes.SSVShare) (bool, error) {
 		c.logger.Warn("skipping validator until it becomes active", fields.PubKey(share.ValidatorPubKey))
 		return false, nil
 	}
+	if share.Invalid {
+		c.logger.Warn("skipping validator with invalid share", fields.PubKey(share.ValidatorPubKey))
+		return false, nil
+	}
 
 	if err := c.setShareFeeRecipient(share, c.recipientsStorage.GetRecipientData); err != nil {
 		return false, fmt.Errorf("could not set share fee recipient: %w", err)

protocol/v2/types/ssvshare.go

@@ -16,8 +16,17 @@ import (
 )
 
 const (
-	MaxPossibleShareSize = 1245
-	MaxAllowedShareSize  = MaxPossibleShareSize * 8 // Leaving some room for protocol updates and calculation mistakes.
+	// InitMaxPossibleShareSize - initial protocol max share size
+	InitMaxPossibleShareSize = 1245
+
+	// CurrentMaxPossibleShareSize - Current protocol version max share size;
+	// can be generated by generateMaxPossibleShare
+	// Used only for testing purposes
+	CurrentMaxPossibleShareSize = 1257
+
+	// MaxAllowedShareSize - constant protocol value. Should not be changed
+	// Leaving some room for protocol updates and calculation mistakes.
+	MaxAllowedShareSize = InitMaxPossibleShareSize * 8
 )
 
 // SSVShare is a combination of spectypes.Share and its Metadata.
@@ -103,4 +112,5 @@ type Metadata struct {
 	BeaconMetadata *beaconprotocol.ValidatorMetadata
 	OwnerAddress   common.Address
 	Liquidated     bool
+	Invalid        bool
 }

registry/storage/shares_test.go

@@ -64,7 +64,7 @@ func TestMaxPossibleShareSize(t *testing.T) {
 	b, err := s.Encode()
 	require.NoError(t, err)
 
-	require.Equal(t, ssvtypes.MaxPossibleShareSize, len(b))
+	require.Equal(t, ssvtypes.CurrentMaxPossibleShareSize, len(b))
 }
 
 func TestSaveAndGetValidatorStorage(t *testing.T) {