diff --git a/eth/eventhandler/event_handler_test.go b/eth/eventhandler/event_handler_test.go index fa55d0dab6..d43d116770 100644 --- a/eth/eventhandler/event_handler_test.go +++ b/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) diff --git a/eth/eventhandler/handlers.go b/eth/eventhandler/handlers.go index d4632ddf6f..89ed63b4b2 100644 --- a/eth/eventhandler/handlers.go +++ b/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 { diff --git a/operator/validator/controller.go b/operator/validator/controller.go index 3048a654d6..aaefbf4a76 100644 --- a/operator/validator/controller.go +++ b/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) diff --git a/protocol/v2/types/ssvshare.go b/protocol/v2/types/ssvshare.go index 6dad2af123..dfbe665803 100644 --- a/protocol/v2/types/ssvshare.go +++ b/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 } diff --git a/registry/storage/shares_test.go b/registry/storage/shares_test.go index 58fb6af59b..028bd9f506 100644 --- a/registry/storage/shares_test.go +++ b/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) {