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.

Sign up for GitHub

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

chore: set up IBCTestStakingKeeper interface #2028

Merged
merged 13 commits into from
Aug 23, 2022
Merged

chore: set up IBCTestStakingKeeper interface #2028

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
c278d65
initial interface definition
charleenfei Aug 17, 2022
4e374f5
wip
charleenfei Aug 18, 2022
62916e1
update simapp
charleenfei Aug 18, 2022
5c5aa52
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 18, 2022
3673054
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 19, 2022
46f0480
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 22, 2022
bd27422
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 22, 2022
d1d88da
updates re: comments
charleenfei Aug 22, 2022
92f44ea
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 22, 2022
b27f8e5
update godocs
charleenfei Aug 22, 2022
ae50357
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 22, 2022
eea436e
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 22, 2022
782e84d
Merge branch 'main' into charly/update_testing_stakingkeeper_interface
charleenfei Aug 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking

* (testing)[\#2028](https://github.com/cosmos/ibc-go/pull/2028) New interface `ibctestingtypes.StakingKeeper` added and set for the testing app `StakingKeeper` setup.
* (core/04-channel) [\#1418](https://github.com/cosmos/ibc-go/pull/1418) `NewPacketId` has been renamed to `NewPacketID` to comply with go linting rules.
* (core/ante) [\#1418](https://github.com/cosmos/ibc-go/pull/1418) `AnteDecorator` has been renamed to `RedundancyDecorator` to comply with go linting rules and to give more clarity to the purpose of the Decorator.
* (core/ante) [\#1820](https://github.com/cosmos/ibc-go/pull/1418) `RedundancyDecorator` has been renamed to `RedundantRelayDecorator` to make the name for explicit.
Expand Down
2 changes: 1 addition & 1 deletion testing/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type TestingApp interface {

// ibc-go additions
GetBaseApp() *baseapp.BaseApp
GetStakingKeeper() stakingkeeper.Keeper
GetStakingKeeper() ibctestingtypes.StakingKeeper
GetIBCKeeper() *keeper.Keeper
GetScopedIBCKeeper() capabilitykeeper.ScopedKeeper
GetTxConfig() client.TxConfig
Expand Down
4 changes: 2 additions & 2 deletions testing/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
stakingkeeper "github.com/cosmos/cosmos-sdk/x/staking/keeper"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
Expand All @@ -27,6 +26,7 @@ import (

"github.com/cosmos/ibc-go/v5/modules/core/keeper"
"github.com/cosmos/ibc-go/v5/testing/simapp"
ibctestingtypes "github.com/cosmos/ibc-go/v5/testing/types"
)

var DefaultTestingAppInit = SetupTestingApp
Expand All @@ -36,7 +36,7 @@ type TestingApp interface {

// ibc-go additions
GetBaseApp() *baseapp.BaseApp
GetStakingKeeper() stakingkeeper.Keeper
GetStakingKeeper() ibctestingtypes.StakingKeeper
GetIBCKeeper() *keeper.Keeper
GetScopedIBCKeeper() capabilitykeeper.ScopedKeeper
GetTxConfig() client.TxConfig
Expand Down
6 changes: 3 additions & 3 deletions testing/chain_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ func TestChangeValSet(t *testing.T) {
amount2, ok := sdk.NewIntFromString("30000000000000000000")
require.True(t, ok)

val := chainA.App.GetStakingKeeper().GetValidators(chainA.GetContext(), 4)
val := chainA.GetSimApp().StakingKeeper.GetValidators(chainA.GetContext(), 4)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it not possible to keep using App.GetIBCTestStakingKeeper() and add GetValidators and Delegate to the StakingKeeper interface?

Copy link
Contributor Author

@charleenfei charleenfei Aug 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

i think the idea is that in our simApp, we would continue using the staking module's staking keeper so we don't actually need to add these two methods as they are already there.

it's just that in the testing chain and testing app setup, we would like to enable osmosis to swap out their customised interfluid staking keeper so they can use our testing setup. this is why we only need to add that one method which is used in the testing/chain.go file to the new interface.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually requested those functions to not be added. I think we should keep our interfaces as small as possible (ideally we wouldn't even need the staking keeper interface)


chainA.App.GetStakingKeeper().Delegate(chainA.GetContext(), chainA.SenderAccounts[1].SenderAccount.GetAddress(),
chainA.GetSimApp().StakingKeeper.Delegate(chainA.GetContext(), chainA.SenderAccounts[1].SenderAccount.GetAddress(),
amount, types.Unbonded, val[1], true)
chainA.App.GetStakingKeeper().Delegate(chainA.GetContext(), chainA.SenderAccounts[3].SenderAccount.GetAddress(),
chainA.GetSimApp().StakingKeeper.Delegate(chainA.GetContext(), chainA.SenderAccounts[3].SenderAccount.GetAddress(),
amount2, types.Unbonded, val[3], true)

coord.CommitBlock(chainA)
Expand Down
3 changes: 2 additions & 1 deletion testing/simapp/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ import (
ibckeeper "github.com/cosmos/ibc-go/v5/modules/core/keeper"
ibcmock "github.com/cosmos/ibc-go/v5/testing/mock"
simappparams "github.com/cosmos/ibc-go/v5/testing/simapp/params"
ibctestingtypes "github.com/cosmos/ibc-go/v5/testing/types"
)

const appName = "SimApp"
Expand Down Expand Up @@ -748,7 +749,7 @@ func (app *SimApp) GetBaseApp() *baseapp.BaseApp {
}

// GetStakingKeeper implements the TestingApp interface.
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
func (app *SimApp) GetStakingKeeper() stakingkeeper.Keeper {
func (app *SimApp) GetStakingKeeper() ibctestingtypes.StakingKeeper {
return app.StakingKeeper
}

Expand Down
12 changes: 12 additions & 0 deletions testing/types/expected_keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package types
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

import (
sdk "github.com/cosmos/cosmos-sdk/types"
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
)

// IBCTestingStakingKeeper defines the expected staking keeper interface used in the
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
// IBC testing package
type StakingKeeper interface {
GetHistoricalInfo(ctx sdk.Context, height int64) (stakingtypes.HistoricalInfo, bool)
}