Skip to content

Commit

Permalink
Consistency is more important than perfection.
Browse files Browse the repository at this point in the history
  • Loading branch information
DimitrisJim committed May 30, 2023
1 parent 5307950 commit 5d91eaa
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 115 deletions.
28 changes: 19 additions & 9 deletions modules/core/02-client/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,10 @@ type Keeper struct {
legacySubspace paramtypes.Subspace
stakingKeeper types.StakingKeeper
upgradeKeeper types.UpgradeKeeper

// the address capable of executing a MsgUpdateClientParams message. Typically, this
// should be the x/gov module account.
authority string
}

// NewKeeper creates a new NewKeeper instance
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace, sk types.StakingKeeper, uk types.UpgradeKeeper, authority string) Keeper {
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace, sk types.StakingKeeper, uk types.UpgradeKeeper) Keeper {
// set KeyTable if it has not already been set
if !legacySubspace.HasKeyTable() {
legacySubspace = legacySubspace.WithKeyTable(types.ParamKeyTable())
Expand All @@ -51,7 +47,6 @@ func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace pa
legacySubspace: legacySubspace,
stakingKeeper: sk,
upgradeKeeper: uk,
authority: authority,
}
}

Expand Down Expand Up @@ -419,7 +414,22 @@ func (k Keeper) GetClientStatus(ctx sdk.Context, clientState exported.ClientStat
return clientState.Status(ctx, k.ClientStore(ctx, clientID), k.cdc)
}

// GetAuthority returns the client module's authority.
func (k Keeper) GetAuthority() string {
return k.authority
// GetParams returns the total set of ibc-client parameters.
func (k Keeper) GetParams(ctx sdk.Context) types.Params {
var p types.Params
store := ctx.KVStore(k.storeKey)
bz := store.Get([]byte(types.ParamsKey))
if bz == nil {
return types.Params{}
}

k.cdc.MustUnmarshal(bz, &p)
return p
}

// SetParams sets the total set of ibc-client parameters.
func (k Keeper) SetParams(ctx sdk.Context, params types.Params) {
store := ctx.KVStore(k.storeKey)
bz := k.cdc.MustMarshal(&params)
store.Set([]byte(types.ParamsKey), bz)
}
54 changes: 54 additions & 0 deletions modules/core/02-client/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,3 +450,57 @@ func (suite KeeperTestSuite) TestIterateClientStates() { //nolint:govet // this
})
}
}

// TestDefaultSetParams tests the default params set are what is expected
func (suite *KeeperTestSuite) TestDefaultSetParams() {
expParams := types.DefaultParams()

clientKeeper := suite.chainA.App.GetIBCKeeper().ClientKeeper
params := clientKeeper.GetParams(suite.chainA.GetContext())

suite.Require().Equal(expParams, params)
suite.Require().Equal(expParams.AllowedClients, clientKeeper.GetParams(suite.chainA.GetContext()).AllowedClients)
}

// TestParams tests that Param setting and retrieval works properly
func (suite *KeeperTestSuite) TestParams() {
testCases := []struct {
name string
input types.Params
expPass bool
}{
{"success: set default params", types.DefaultParams(), true},
{"success: empty allowedClients", types.NewParams(), true},
{"success: subset of allowedClients", types.NewParams(exported.Tendermint, exported.Localhost), true},
{"failure: contains a single empty string value as allowedClient", types.NewParams(exported.Localhost, ""), false},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest() // reset
ctx := suite.chainA.GetContext()
err := tc.input.Validate()
suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.SetParams(ctx, tc.input)
if tc.expPass {
suite.Require().NoError(err)
expected := tc.input
p := suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.GetParams(ctx)
suite.Require().Equal(expected, p)
} else {
suite.Require().Error(err)
}
})
}
}

// TestUnsetParams tests that trying to get params that are not set returns empty params.
func (suite *KeeperTestSuite) TestUnsetParams() {
suite.SetupTest()
ctx := suite.chainA.GetContext()
store := ctx.KVStore(suite.chainA.GetSimApp().GetKey(exported.StoreKey))
store.Delete([]byte(types.ParamsKey))

suite.Require().Equal(suite.chainA.GetSimApp().IBCKeeper.ClientKeeper.GetParams(ctx), types.Params{})
}
33 changes: 0 additions & 33 deletions modules/core/02-client/keeper/params.go

This file was deleted.

60 changes: 0 additions & 60 deletions modules/core/02-client/keeper/params_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion modules/core/02-client/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,7 @@ func (suite *TypesTestSuite) TestMsgSubmitMisbehaviour_ValidateBasic() {

// TestMsgUpdateClientParams_ValidateBasic tests ValidateBasic for MsgUpdateClientParams
func (suite *TypesTestSuite) TestMsgUpdateClientParams_ValidateBasic() {
authority := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAuthority()
authority := suite.chainA.App.GetIBCKeeper().GetAuthority()
testCases := []struct {
name string
msg *types.MsgUpdateClientParams
Expand Down
18 changes: 12 additions & 6 deletions modules/core/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (

"github.com/cosmos/cosmos-sdk/codec"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
capabilitykeeper "github.com/cosmos/ibc-go/modules/capability/keeper"

Expand Down Expand Up @@ -35,13 +33,17 @@ type Keeper struct {
ChannelKeeper channelkeeper.Keeper
PortKeeper portkeeper.Keeper
Router *porttypes.Router

// the address capable of executing a MsgUpdateClientParams message. Typically, this
// should be the x/gov module account.
authority string
}

// NewKeeper creates a new ibc Keeper
func NewKeeper(
cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace,
stakingKeeper clienttypes.StakingKeeper, upgradeKeeper clienttypes.UpgradeKeeper,
scopedKeeper capabilitykeeper.ScopedKeeper,
scopedKeeper capabilitykeeper.ScopedKeeper, authority string,
) *Keeper {
// register paramSpace at top level keeper
// set KeyTable if it has not already been set
Expand All @@ -63,9 +65,7 @@ func NewKeeper(
panic(fmt.Errorf("cannot initialize IBC keeper: empty scoped keeper"))
}

authority := authtypes.NewModuleAddress(govtypes.ModuleName).String()

clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, stakingKeeper, upgradeKeeper, authority)
clientKeeper := clientkeeper.NewKeeper(cdc, key, paramSpace, stakingKeeper, upgradeKeeper)
connectionKeeper := connectionkeeper.NewKeeper(cdc, key, paramSpace, clientKeeper)
portKeeper := portkeeper.NewKeeper(scopedKeeper)
channelKeeper := channelkeeper.NewKeeper(cdc, key, clientKeeper, connectionKeeper, portKeeper, scopedKeeper)
Expand All @@ -76,6 +76,7 @@ func NewKeeper(
ConnectionKeeper: connectionKeeper,
ChannelKeeper: channelKeeper,
PortKeeper: portKeeper,
authority: authority,
}
}

Expand All @@ -96,6 +97,11 @@ func (k *Keeper) SetRouter(rtr *porttypes.Router) {
k.Router.Seal()
}

// GetAuthority returns the client module's authority.
func (k Keeper) GetAuthority() string {
return k.authority
}

// isEmpty checks if the interface is an empty struct or a pointer pointing
// to an empty struct
func isEmpty(keeper interface{}) bool {
Expand Down
1 change: 1 addition & 0 deletions modules/core/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
stakingKeeper,
upgradeKeeper,
scopedKeeper,
suite.chainA.App.GetIBCKeeper().GetAuthority(),
)
}
)
Expand Down
7 changes: 3 additions & 4 deletions modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -697,13 +697,12 @@ func (k Keeper) Acknowledgement(goCtx context.Context, msg *channeltypes.MsgAckn

// UpdateClientParams defines a rpc handler method for MsgUpdateClientParams.
func (k Keeper) UpdateClientParams(goCtx context.Context, msg *clienttypes.MsgUpdateClientParams) (*clienttypes.MsgUpdateClientParamsResponse, error) {
ck := k.ClientKeeper
if ck.GetAuthority() != msg.Authority {
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", ck.GetAuthority(), msg.Authority)
if k.GetAuthority() != msg.Authority {
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Authority)
}

ctx := sdk.UnwrapSDKContext(goCtx)
ck.SetParams(ctx, msg.Params)
k.ClientKeeper.SetParams(ctx, msg.Params)

return &clienttypes.MsgUpdateClientParamsResponse{}, nil
}
2 changes: 1 addition & 1 deletion modules/core/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ func (suite *KeeperTestSuite) TestUpgradeClient() {

// TestUpdateClientParams tests the UpdateClientParams rpc handler
func (suite *KeeperTestSuite) TestUpdateClientParams() {
validAuthority := suite.chainA.App.GetIBCKeeper().ClientKeeper.GetAuthority()
validAuthority := suite.chainA.App.GetIBCKeeper().GetAuthority()
testCases := []struct {
name string
msg *clienttypes.MsgUpdateClientParams
Expand Down
2 changes: 1 addition & 1 deletion testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ func NewSimApp(
// IBC Keepers

app.IBCKeeper = ibckeeper.NewKeeper(
appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper,
appCodec, keys[ibcexported.StoreKey], app.GetSubspace(ibcexported.ModuleName), app.StakingKeeper, app.UpgradeKeeper, scopedIBCKeeper, authtypes.NewModuleAddress(govtypes.ModuleName).String(),
)

// register the proposal types
Expand Down

0 comments on commit 5d91eaa

Please sign in to comment.