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

Paginate supply and balances methods #8761

Closed
1 of 6 tasks
sahith-narahari opened this issue Mar 3, 2021 · 7 comments · Fixed by #8798
Closed
1 of 6 tasks

Paginate supply and balances methods #8761

sahith-narahari opened this issue Mar 3, 2021 · 7 comments · Fixed by #8798
Assignees
Labels
Milestone

Comments

@sahith-narahari
Copy link
Contributor

sahith-narahari commented Mar 3, 2021

Summary

Ref: #8517
To have a better support for multiple tokens, we need to paginate GetTotalSupply and GetAllBalances which is being used across multiple modules

For Admin Use

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

There's an item "Paginate GetAllBalances", but afaict it's already implemented. Can you confirm @sahith-narahari ?

@mergify mergify bot closed this as completed in #8798 Apr 6, 2021
@aaronc aaronc removed the in-review label Apr 6, 2021
@sahith-narahari
Copy link
Contributor Author

There's an item "Paginate GetAllBalances", but afaict it's already implemented. Can you confirm @sahith-narahari ?

The question is do we want to paginate GetAllBalances similar to how we did supply?

It's used across various modules and maybe we should imo.

func (k BaseViewKeeper) GetAllBalances(ctx sdk.Context, addr sdk.AccAddress) sdk.Coins {

@amaury1093
Copy link
Contributor

The question is do we want to paginate GetAllBalances similar to how we did supply?

I'm not sure I understand. Both RPC methods take a PaginationRequest param.

@sahith-narahari
Copy link
Contributor Author

I'm referring to the keeper methods, ref: #8517 (comment)

@sahith-narahari
Copy link
Contributor Author

I don't see much difference in using it without pagination though

@amaury1093
Copy link
Contributor

amaury1093 commented Apr 6, 2021

OK, the gRPC handler correctly handles pagination. Only the keeper method doesn't. I'm fine with it as-is, you're probably right @sahith-narahari the "huge denom list" problem should be smaller on individual balances.

Unless if you want/started work on this, then feel free to continue!

@sahith-narahari
Copy link
Contributor Author

I didn't start yet and agree with you. I propose to leave it as-is and get back to it if we see any problem in the future

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

Successfully merging a pull request may close this issue.

5 participants