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

Allow querying of /staking/validators #4099

Closed
4 tasks
alexanderbez opened this issue Apr 11, 2019 · 11 comments · Fixed by #4177
Closed
4 tasks

Allow querying of /staking/validators #4099

alexanderbez opened this issue Apr 11, 2019 · 11 comments · Fixed by #4177
Assignees
Milestone

Comments

@alexanderbez
Copy link
Contributor

alexanderbez commented Apr 11, 2019

Summary

Currently /staking/validators doesn't have a preference on bond status OR power, it simply returns the top MaxValidators from the store. So this could mean unbonded or even jailed validators could be included and bonded validators excluded (ref: #4096).

Proposal

Allow this query endpoint to take query params -- at the very least status. Also, support direct pagination instead of forcing to MaxValidators.

Acceptance Criteria

  • Implement pagination for /staking/validators
  • Implement status query param for /staking/validators

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@faboweb
Copy link
Contributor

faboweb commented Apr 18, 2019

Can we hotfix this by just returning all validators for now? Probably less then 200 am I right?
This problem results in a validator not showing up in Lunie.

@alexanderbez
Copy link
Contributor Author

I'm not sure @faboweb. We already have this issue in the upcoming release and we're not making a patch release of 0.34.0, so what would be the point?

@faboweb
Copy link
Contributor

faboweb commented Apr 18, 2019

When is 0.35 coming?

@alexanderbez
Copy link
Contributor Author

It's currently being worked on -- the active sprint/release. I'm not sure we have a set-in-stone date on it yet. Do we @alessio?

In any case, check the board: https://app.zenhub.com/workspaces/sdk-sprint-board-5acbc6a3ebb7e24fed9a8499/board?milestones=v0.35.0%232019-04-26&filterLogic=any&repos=51193526

@alessio
Copy link
Contributor

alessio commented Apr 18, 2019

@faboweb The team hasn't yet come to an agreement on what release model we should adopt. It's nobody's fault, we just are not in the position to address such issue until we manage to split gaia from sdk [1].
Once we've got there, we'll make a decision about how to go about managing releases for each of the two products and will announce our strategy timely.

In other words: no dates quite yet.

[1] #4071 (ZenHub is required to be able to see issue's dependencies)

@faboweb
Copy link
Contributor

faboweb commented Apr 19, 2019

Ok. To me it sounds more like a bug then a feature request as it breaks expected behaviour. Therefor I would have fixed it asap and put it into a 0.34.1 which could be rolled out as a non breaking update. But I am probably missing sth.. Looking forward to the fix. Thanks guys.

@clawmvp
Copy link

clawmvp commented Apr 23, 2019

hey guys, any update on this? i m still out of the lists on every explorer that uses this.

@alexanderbez
Copy link
Contributor Author

alexanderbez commented Apr 23, 2019

@clawmvp yes, I just started work on this right now. Since it's critical for many clients, I suggest that once I complete this, we merge this into develop and into a patch release, v0.34.2 (cc @jackzampolin).

If all goes well, expect something this week.

@clawmvp
Copy link

clawmvp commented Apr 23, 2019

Great, thank you!

@faboweb
Copy link
Contributor

faboweb commented Apr 24, 2019

Great thanks @alexanderbez

@1xCL
Copy link

1xCL commented Apr 26, 2019

Thanks @alexanderbez !
and you got a typo :)

 - in: query
          name: page
          description: The gage number.

The gage number

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

Successfully merging a pull request may close this issue.

6 participants