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

Make IBCtesting GetStakingKeeper be based on an interface #1971

Closed
3 tasks
ValarDragon opened this issue Aug 10, 2022 · 1 comment · Fixed by #2028
Closed
3 tasks

Make IBCtesting GetStakingKeeper be based on an interface #1971

ValarDragon opened this issue Aug 10, 2022 · 1 comment · Fixed by #2028
Assignees
Labels
testing Testing package and unit/integration tests
Milestone

Comments

@ValarDragon
Copy link
Contributor

Summary

The IBCtesting GetStakingKeeper required interface method (https://github.com/cosmos/ibc-go/blob/main/testing/app.go#L39) should not depend on the literal staking keeper in the hub.

This should instead have an interface dependency, to account for apps doing weirder things with their staking modules. E.g. Osmosis superfluid staking, we really need to look at what IBC depends on, to determine what we should be returning.

Also would recommend renaming to GetIBCTestingStakingKeeper as part of this.

Problem Definition

The current design inhibits modularity of ibctesting, for a small reason.

Proposal

Define an interface for what keeper methods IBC depends on from the staking keeper, and then define an interface accordingly. Use that interface in the required ibctesting app interface.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@crodriguezvega crodriguezvega added the testing Testing package and unit/integration tests label Aug 15, 2022
@charleenfei charleenfei self-assigned this Aug 17, 2022
@crodriguezvega crodriguezvega moved this to In review in ibc-go Aug 19, 2022
@crodriguezvega crodriguezvega added this to the v5.0.0 milestone Aug 22, 2022
@colin-axner
Copy link
Contributor

colin-axner commented Aug 22, 2022

@ValarDragon if the API is changed to:

GetStakingKeeper() ibctestingtypes.StakingKeeper

do you still want this change:

Also would recommend renaming to GetIBCTestingStakingKeeper as part of this.

still the return value implies it is a test staking keeper

Repository owner moved this from In review to Done in ibc-go Aug 23, 2022
@crodriguezvega crodriguezvega moved this from Todo to Done in ibc-go Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Testing package and unit/integration tests
Projects
Status: Done 🥳
Development

Successfully merging a pull request may close this issue.

4 participants