-
Notifications
You must be signed in to change notification settings - Fork 85
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
Feat: Mark invalid shares #1313
base: stage
Are you sure you want to change the base?
Changes from 7 commits
1889b38
361f64b
e70a11b
bc35ed7
775df26
1400653
94493ac
02f7287
ec7286c
be64685
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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.InvalidSecret { | ||
eh.metrics.ValidatorInactive(event.PublicKey) | ||
ownShare = validatorShare | ||
logger = logger.With(zap.Bool("own_validator", isOperatorShare)) | ||
|
@@ -233,11 +233,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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is the 2nd check needed? as far as i can see that method returns no error when the secret is invalid There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @moshe-blox, No we can't remove the 2nd check because we have to check that there is no error and share is valid. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh sry, i thought that we're not returning error in can we try this? i'm interested to see how it would look, we can just log the reason for invalid secret instead of returning it as error (for example then this check becomes just a simple |
||
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 +254,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 +268,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{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why did we do this change? is returning an empty share anything better or different than returning nil? |
||
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 +306,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 | ||
MatusKysel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
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) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can log that its our validator regardless of invalid secret, and also we should probably log if its an invalid secret (so its hard to miss in Kibana)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. Added this