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

Dong/pagination for all intermediary accounts #4752

Conversation

DongLieu
Copy link
Contributor

Closes: #4168

What is the purpose of the change

Add pagination support for AllIntermediaryAccounts query in Superfluid Module

Testing and Verifying

golangci-lint run --fix

Add testcase for AllIntermediaryAccounts

@mattverse
Copy link
Member

Closing then re-opening this PR to properly trigger CI

@mattverse mattverse closed this Mar 27, 2023
@mattverse mattverse reopened this Mar 27, 2023
Copy link
Member

@pysel pysel left a comment

Choose a reason for hiding this comment

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

hey @DongLieu, I think the branch you are working from might be out of date, because go.work.sum has been recently added to .gitignore file

@DongLieu
Copy link
Contributor Author

Ok. Thank you

@DongLieu DongLieu requested a review from pysel March 28, 2023 06:29
@github-actions
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

@github-actions github-actions bot added the Stale label Apr 12, 2023
Copy link
Member

@mattverse mattverse left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! LGTM once test case improvised

Comment on lines +36 to +39
res, err := suite.querier.AllIntermediaryAccounts(sdk.WrapSDKContext(suite.Ctx), &types.AllIntermediaryAccountsRequest{})
suite.Require().NoError(err)
suite.Require().Equal(3, len(res.Accounts))
suite.Require().Equal(uint64(3), res.Pagination.Total)
Copy link
Member

Choose a reason for hiding this comment

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

can we also check the returned values as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have tested the returned values including ValAddr it is the same address
image

@github-actions github-actions bot removed the Stale label Apr 18, 2023
@mattverse mattverse self-assigned this Apr 19, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you!

@github-actions github-actions bot added the Stale label May 4, 2023
@czarcas7ic
Copy link
Member

@DongLieu has this change been tested locally? If so and it works, ping me and I will merge.

@czarcas7ic czarcas7ic added V:state/compatible/no_backport State machine compatible PR, depends on prior breaks no-changelog labels May 7, 2023
@DongLieu
Copy link
Contributor Author

DongLieu commented May 7, 2023

I tested it locally with a test TestAllIntermediaryAccounts(), everything is great

@czarcas7ic
Copy link
Member

@DongLieu oh no I meant, have you compiled this binary locally and queried mainnet or testnet with this and works as intended

@github-actions github-actions bot removed the Stale label May 8, 2023
@DongLieu
Copy link
Contributor Author

DongLieu commented May 8, 2023

i tried it successfully on testnet

@faddat
Copy link
Member

faddat commented May 9, 2023

Nice :D

@czarcas7ic czarcas7ic added V:state/breaking State machine breaking PR and removed V:state/compatible/no_backport State machine compatible PR, depends on prior breaks labels May 9, 2023
@czarcas7ic czarcas7ic merged commit a186836 into osmosis-labs:main May 9, 2023
pysel pushed a commit that referenced this pull request Jun 6, 2023
* Add pagination support for AllIntermediaryAccounts query in Superfluid Module

* lint

* merge main
@github-actions github-actions bot mentioned this pull request Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:x/superfluid V:state/breaking State machine breaking PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add pagination support for AllIntermediaryAccounts query in Superfluid Module
5 participants