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

Redundant pagination parameters in API #304

Closed
gusin13 opened this issue Feb 6, 2023 · 4 comments
Closed

Redundant pagination parameters in API #304

gusin13 opened this issue Feb 6, 2023 · 4 comments

Comments

@gusin13
Copy link
Contributor

gusin13 commented Feb 6, 2023

Summary of Bug

On a given epoch range pagination params in endpoint /babylon/epoching/v1/epochs?start_epoch=0&end_epoch=5 are redundant

Steps to Reproduce

curl --location --request GET 'http://rpc.testnet.babylonchain.io:1317/babylon/epoching/v1/epochs?start_epoch=0&end_epoch=5&pagination.key=AAAAAAAAAAY='

pagination.key in response should be nil

@SebastianElvis
Copy link
Member

Initially we introduced start_epoch and end_epoch to ease the query via a URL, given the difficulty of encoding/decoding pagination.key in URLs. If we query via a Cosmos SDK-based client then this will not be a problem and we can query a range of epochs via the pagination parameters only. So I would suggest the following:

  • Before we replace the URL-based queries in the API backend with Cosmos SDK clients, we keep both approaches for querying epoch ranges, and removes pagination response if using start_epoch/end_epoch.
  • Once we used Cosmos SDK clients everywhere, we remove the start_epoch and end_epoch parameters in this API.

Wdyt?

@vitsalis
Copy link
Member

vitsalis commented Feb 7, 2023

In the current implementation the start_epoch and end_epoch parameters do not work with the pagination arguments and lead to non-intuitive results (since if start_epoch or end_epoch are provided the pagination params are overriden). If we do not plan to make pagination work with start_epoch and end_epoch we should remove them. In a Babylon API PR I did a refactoring in which we use the Babylon rpc-client. Eitherway, the Babylon API currently supports version v0.5.0, so future changes won't affect much.

@SebastianElvis
Copy link
Member

Agree, let's remove these two params after https://github.com/babylonchain/babylon-api/pull/60

@SebastianElvis
Copy link
Member

Closed by #305

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

No branches or pull requests

3 participants