Skip to content
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

ETH2 APIs: Implement Validator Status support #8940

Merged
merged 7 commits into from
May 26, 2021
Merged

Conversation

0xKiwi
Copy link
Contributor

@0xKiwi 0xKiwi commented May 25, 2021

What type of PR is this?
Feature

What does this PR do? Why is it needed?
This PR fixes #8901 and is part of #7510. This PR adds validator status enum support to your API, allowing the api to return validators status, and allowing filtering the validator set based on their status.

@0xKiwi 0xKiwi requested a review from a team as a code owner May 25, 2021 21:40
@codecov
Copy link

codecov bot commented May 25, 2021

Codecov Report

Merging #8940 (79d86bf) into develop (2e322bd) will increase coverage by 0.04%.
The diff coverage is 92.45%.

@@             Coverage Diff             @@
##           develop    #8940      +/-   ##
===========================================
+ Coverage    60.88%   60.92%   +0.04%     
===========================================
  Files          529      529              
  Lines        37368    37418      +50     
===========================================
+ Hits         22750    22798      +48     
+ Misses       11358    11353       -5     
- Partials      3260     3267       +7     

@0xKiwi 0xKiwi merged commit d5ad15e into develop May 26, 2021
@delete-merged-branch delete-merged-branch bot deleted the validator-statuss branch May 26, 2021 04:24
}
}

return 0
Copy link
Contributor

@rkapka rkapka May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 is ValidatorStatus_PENDING_INITIALIZED, which is an incorrect value to return. We should return an error instead.

Status: []ethpb.ValidatorStatus{ethpb.ValidatorStatus_PENDING_INITIALIZED, ethpb.ValidatorStatus_EXITED_UNSLASHED},
})
require.NoError(t, err)
assert.Equal(t, 4 /* 4 exited */, len(resp.Data))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the comment is incorrect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its correct, at the epoch there are 4 validators that fir the exit status criteria.

validator *ethpb.Validator
epoch types.Epoch
}
tests := []struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot about pending_queued

validator *ethpb.Validator
epoch types.Epoch
}
tests := []struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You forgot about pending_queued

case ethpb.ValidatorStatus_WITHDRAWAL_POSSIBLE, ethpb.ValidatorStatus_WITHDRAWAL_DONE:
return ethpb.ValidatorStatus_WITHDRAWAL
}
return 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ETH2.0 API: Implement accurate validator status selection
3 participants