Skip to content

Commit

Permalink
Validator REST api: adding in check for empty keys changed (#14637)
Browse files Browse the repository at this point in the history
* adding in check for empty keys changed

* changelog

* kasey feedback

* fixing unit tests

* Update CHANGELOG.md

Co-authored-by: Sammy Rosso <[email protected]>

---------

Co-authored-by: Sammy Rosso <[email protected]>
  • Loading branch information
james-prysm and saolyn authored Nov 13, 2024
1 parent 1857496 commit be60504
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 14 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ The format is based on Keep a Changelog, and this project adheres to Semantic Ve
- Small log imporvement, removing some redundant or duplicate logs
- EIP7521 - Fixes withdrawal bug by accounting for pending partial withdrawals and deducting already withdrawn amounts from the sweep balance. [PR](https://github.com/prysmaticlabs/prysm/pull/14578)
- unskip electra merkle spec test

- Fix panic in validator REST mode when checking status after removing all keys

### Security

Expand Down
3 changes: 3 additions & 0 deletions beacon-chain/rpc/prysm/v1alpha1/validator/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,9 @@ func (vs *Server) validatorStatus(
Status: ethpb.ValidatorStatus_UNKNOWN_STATUS,
ActivationEpoch: params.BeaconConfig().FarFutureEpoch,
}
if len(pubKey) == 0 {
return resp, nonExistentIndex
}
vStatus, idx, err := statusForPubKey(headState, pubKey)
if err != nil && !errors.Is(err, errPubkeyDoesNotExist) {
tracing.AnnotateError(span, err)
Expand Down
5 changes: 5 additions & 0 deletions validator/client/beacon-api/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ func (c *beaconApiValidatorClient) validatorsStatusResponse(ctx context.Context,
[]*ethpb.ValidatorStatusResponse,
error,
) {
// if no parameters are provided we should just return an empty response
if len(inPubKeys) == 0 && len(inIndexes) == 0 {
return [][]byte{}, []primitives.ValidatorIndex{}, []*ethpb.ValidatorStatusResponse{}, nil
}
// Represents the target set of keys
stringTargetPubKeysToPubKeys := make(map[string][]byte, len(inPubKeys))
stringTargetPubKeys := make([]string, len(inPubKeys))
Expand Down Expand Up @@ -78,6 +82,7 @@ func (c *beaconApiValidatorClient) validatorsStatusResponse(ctx context.Context,
return nil, nil, nil, errors.Wrap(err, "failed to get state validators")
}

// TODO: we should remove this API call
validatorsCountResponse, err := c.prysmChainClient.ValidatorCount(ctx, "head", nil)
if err != nil && !errors.Is(err, iface.ErrNotSupported) {
return nil, nil, nil, errors.Wrap(err, "failed to get total validator count")
Expand Down
17 changes: 4 additions & 13 deletions validator/client/beacon-api/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,32 +215,23 @@ func TestMultipleValidatorStatus_Nominal(t *testing.T) {
assert.DeepEqual(t, &expectedValidatorStatusResponse, actualValidatorStatusResponse)
}

func TestMultipleValidatorStatus_Error(t *testing.T) {
func TestMultipleValidatorStatus_No_Keys(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

ctx := context.Background()
stateValidatorsProvider := mock.NewMockStateValidatorsProvider(ctrl)

stateValidatorsProvider.EXPECT().StateValidators(
gomock.Any(),
gomock.Any(),
[]primitives.ValidatorIndex{},
nil,
).Return(
&structs.GetValidatorsResponse{},
errors.New("a specific error"),
).Times(1)

validatorClient := beaconApiValidatorClient{stateValidatorsProvider: stateValidatorsProvider}

_, err := validatorClient.MultipleValidatorStatus(
resp, err := validatorClient.MultipleValidatorStatus(
ctx,
&ethpb.MultipleValidatorStatusRequest{
PublicKeys: [][]byte{},
},
)
require.ErrorContains(t, "failed to get validators status response", err)
require.NoError(t, err)
require.DeepEqual(t, &ethpb.MultipleValidatorStatusResponse{}, resp)
}

func TestGetValidatorsStatusResponse_Nominal_SomeActiveValidators(t *testing.T) {
Expand Down
4 changes: 4 additions & 0 deletions validator/client/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,10 @@ func (v *validator) filterAndCacheActiveKeys(ctx context.Context, pubkeys [][fie

// updateValidatorStatusCache updates the validator statuses cache, a map of keys currently used by the validator client
func (v *validator) updateValidatorStatusCache(ctx context.Context, pubkeys [][fieldparams.BLSPubkeyLength]byte) error {
if len(pubkeys) == 0 {
v.pubkeyToStatus = make(map[[fieldparams.BLSPubkeyLength]byte]*validatorStatus, 0)
return nil
}
statusRequestKeys := make([][]byte, 0)
for _, k := range pubkeys {
statusRequestKeys = append(statusRequestKeys, k[:])
Expand Down
5 changes: 5 additions & 0 deletions validator/client/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2899,4 +2899,9 @@ func TestUpdateValidatorStatusCache(t *testing.T) {
require.Equal(t, mockResponse.Statuses[i], status.status)
require.Equal(t, mockResponse.Indices[i], status.index)
}

err = v.updateValidatorStatusCache(ctx, nil)
assert.NoError(t, err)
// make sure the value is 0
assert.Equal(t, 0, len(v.pubkeyToStatus))
}

0 comments on commit be60504

Please sign in to comment.