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

implement getStateValidators in lodestar-validator #2234

Merged
merged 18 commits into from
Apr 6, 2021

Conversation

3xtr4t3rr3str14l
Copy link
Contributor

@3xtr4t3rr3str14l 3xtr4t3rr3str14l commented Mar 23, 2021

resolve #2232, #2235, and #2293 by implementing getStateValidators on the lodestar-validator client API side and then use that inside updateValidators()

@codeclimate
Copy link

codeclimate bot commented Mar 23, 2021

Code Climate has analyzed commit 1c52b59 and detected 4 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 4

View more on Code Climate.

@wemeetagain
Copy link
Member

wemeetagain commented Mar 23, 2021

We want to update getStateValidators to accept an array of validator indices/pubkeys (

)
(using https://ethereum.github.io/eth2.0-APIs/#/Beacon/getStateValidators), and then only call that once in updateValidators

@3xtr4t3rr3str14l
Copy link
Contributor Author

3xtr4t3rr3str14l commented Mar 23, 2021

We want to update getStateValidators to accept an array of validator indices/pubkeys (


)
(using https://ethereum.github.io/eth2.0-APIs/#/Beacon/getStateValidators), and then only call that once in updateValidators

for the request you linked (from the eth2 api docs), that would return all the validators for the whole state. the other similar call is this one, which just gets one validator per id:
https://ethereum.github.io/eth2.0-APIs/#/Beacon/getStateValidator

so in order to stay within the spec, wouldn't we just call getStateValidator in batch like i'm doing here? (since i'm assuming we don't want to fetch the whole state validators every time we run updateValidators). Or do you think it's better to deviate from the spec in this case?

@wemeetagain
Copy link
Member

wemeetagain commented Mar 23, 2021

The getStateValidators request has an id param that takes an array of validator indices/pubkeys, used to filter the response to only the validators provided.

@3xtr4t3rr3str14l
Copy link
Contributor Author

The getStateValidators request has an id param that takes an array of validator indices/pubkeys, used to filter the response to only the validators provided.

ah gotcha, i didn't see that. i'll restructure this PR accordingly

@3xtr4t3rr3str14l 3xtr4t3rr3str14l changed the title change updateValidators to use Promise.all WIP: change updateValidators to use Promise.all Mar 23, 2021
@3xtr4t3rr3str14l 3xtr4t3rr3str14l changed the title WIP: change updateValidators to use Promise.all WIP: implement getStateValidators in lodestar-validator Mar 23, 2021
@github-actions github-actions bot added the Api label Mar 25, 2021
@3xtr4t3rr3str14l 3xtr4t3rr3str14l marked this pull request as ready for review March 26, 2021 17:22
@3xtr4t3rr3str14l 3xtr4t3rr3str14l changed the title WIP: implement getStateValidators in lodestar-validator implement getStateValidators in lodestar-validator Mar 26, 2021
@wemeetagain wemeetagain merged commit e01f1b3 into master Apr 6, 2021
@wemeetagain wemeetagain deleted the P0/batch-update-promise-all branch April 6, 2021 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants