Skip to content

Commit

Permalink
feat(connection): migrate connection params
Browse files Browse the repository at this point in the history
  • Loading branch information
DimitrisJim committed May 30, 2023
1 parent d61886d commit 812e67f
Show file tree
Hide file tree
Showing 21 changed files with 793 additions and 140 deletions.
3 changes: 1 addition & 2 deletions docs/migrations/v7-to-v8.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,12 @@ TODO: https://github.com/cosmos/ibc-go/pull/3505 (extra parameter added to trans
)
```

- You should pass the `authority` to the IBC keeper. ([#3640](https://github.com/cosmos/ibc-go/pull/3640)) See [diff](https://github.com/cosmos/ibc-go/pull/3640/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).
- You should pass the `authority` to the IBC keeper. ([#3640](https://github.com/cosmos/ibc-go/pull/3640) and [#3650](https://github.com/cosmos/ibc-go/pull/3650)) See [diff](https://github.com/cosmos/ibc-go/pull/3640/files#diff-d18972debee5e64f16e40807b2ae112ddbe609504a93ea5e1c80a5d489c3a08a).

```diff
// app.go

// 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(),
Expand Down
5 changes: 5 additions & 0 deletions modules/core/03-connection/genesis.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package connection

import (
"fmt"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v7/modules/core/03-connection/keeper"
Expand All @@ -18,6 +20,9 @@ func InitGenesis(ctx sdk.Context, k keeper.Keeper, gs types.GenesisState) {
k.SetClientConnectionPaths(ctx, connPaths.ClientId, connPaths.Paths)
}
k.SetNextConnectionSequence(ctx, gs.NextConnectionSequence)
if err := gs.Params.Validate(); err != nil {
panic(fmt.Sprintf("invalid ibc connection genesis state parameters: %v", err))
}
k.SetParams(ctx, gs.Params)

k.CreateSentinelLocalhostConnection(ctx)
Expand Down
42 changes: 31 additions & 11 deletions modules/core/03-connection/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,24 +20,24 @@ type Keeper struct {
// implements gRPC QueryServer interface
types.QueryServer

storeKey storetypes.StoreKey
paramSpace paramtypes.Subspace
cdc codec.BinaryCodec
clientKeeper types.ClientKeeper
storeKey storetypes.StoreKey
legacySubspace paramtypes.Subspace
cdc codec.BinaryCodec
clientKeeper types.ClientKeeper
}

// NewKeeper creates a new IBC connection Keeper instance
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace, ck types.ClientKeeper) Keeper {
func NewKeeper(cdc codec.BinaryCodec, key storetypes.StoreKey, legacySubspace paramtypes.Subspace, ck types.ClientKeeper) Keeper {
// set KeyTable if it has not already been set
if !paramSpace.HasKeyTable() {
paramSpace = paramSpace.WithKeyTable(types.ParamKeyTable())
if !legacySubspace.HasKeyTable() {
legacySubspace = legacySubspace.WithKeyTable(types.ParamKeyTable())
}

return Keeper{
storeKey: key,
cdc: cdc,
paramSpace: paramSpace,
clientKeeper: ck,
storeKey: key,
cdc: cdc,
legacySubspace: legacySubspace,
clientKeeper: ck,
}
}

Expand Down Expand Up @@ -221,3 +221,23 @@ func (k Keeper) addConnectionToClient(ctx sdk.Context, clientID, connectionID st
k.SetClientConnectionPaths(ctx, clientID, conns)
return nil
}

// GetParams returns the total set of ibc-connection parameters.
func (k Keeper) GetParams(ctx sdk.Context) types.Params {
store := ctx.KVStore(k.storeKey)
bz := store.Get([]byte(types.ParamsKey))
if len(bz) == 0 {
return types.Params{}
}

var params types.Params
k.cdc.MustUnmarshal(bz, &params)
return params
}

// SetParams sets the total set of ibc-connection 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)
}
50 changes: 50 additions & 0 deletions modules/core/03-connection/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,53 @@ func (suite *KeeperTestSuite) TestLocalhostConnectionEndCreation() {
expectedCounterParty := types.NewCounterparty(exported.LocalhostClientID, exported.LocalhostConnectionID, commitmenttypes.NewMerklePrefix(connectionKeeper.GetCommitmentPrefix().Bytes()))
suite.Require().Equal(expectedCounterParty, connectionEnd.Counterparty)
}

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

params := suite.chainA.App.GetIBCKeeper().ConnectionKeeper.GetParams(suite.chainA.GetContext())
suite.Require().Equal(expParams, params)
}

// 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: valid value for MaxExpectedTimePerBlock", types.NewParams(10), true},
{"failure: invalid value for MaxExpectedTimePerBlock", types.NewParams(0), 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.ConnectionKeeper.SetParams(ctx, tc.input)
if tc.expPass {
suite.Require().NoError(err)
expected := tc.input
p := suite.chainA.GetSimApp().IBCKeeper.ConnectionKeeper.GetParams(ctx)
suite.Require().Equal(expected, p)
} else {
suite.Require().Error(err)
}
})
}
}

// TestUnsetParams tests that trying to get params that are not set panics.
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.ConnectionKeeper.GetParams(ctx), types.Params{})
}
15 changes: 15 additions & 0 deletions modules/core/03-connection/keeper/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"

connectionv7 "github.com/cosmos/ibc-go/v7/modules/core/03-connection/migrations/v7"
"github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
)

// Migrator is a struct for handling in-place store migrations.
Expand All @@ -22,3 +23,17 @@ func (m Migrator) Migrate3to4(ctx sdk.Context) error {
connectionv7.MigrateLocalhostConnection(ctx, m.keeper)
return nil
}

// MigrateParams migrates from consensus version 4 to 5.
// This migration takes the parameters that are currently stored and managed by x/params
// and stores them directly in the ibc module's state.
func (m Migrator) MigrateParams(ctx sdk.Context) error {
var params types.Params
m.keeper.legacySubspace.GetParamSet(ctx, &params)

if err := params.Validate(); err != nil {
return err
}
m.keeper.SetParams(ctx, params)
return nil
}
43 changes: 43 additions & 0 deletions modules/core/03-connection/keeper/migrations_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package keeper_test

import (
"github.com/cosmos/ibc-go/v7/modules/core/03-connection/keeper"
"github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
ibcexported "github.com/cosmos/ibc-go/v7/modules/core/exported"
)

// TestMigrateParams tests that the params for the connection are properly migrated
func (suite *KeeperTestSuite) TestMigrateParams() {
testCases := []struct {
name string
malleate func()
expectedParams types.Params
}{
{
"success: default params",
func() {
params := types.DefaultParams()
subspace := suite.chainA.GetSimApp().GetSubspace(ibcexported.ModuleName)
subspace.SetParamSet(suite.chainA.GetContext(), &params) // set params
},
types.DefaultParams(),
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest() // reset

tc.malleate()

ctx := suite.chainA.GetContext()
migrator := keeper.NewMigrator(suite.chainA.GetSimApp().IBCKeeper.ConnectionKeeper)
err := migrator.MigrateParams(ctx)
suite.Require().NoError(err)

params := suite.chainA.GetSimApp().IBCKeeper.ConnectionKeeper.GetParams(ctx)
suite.Require().Equal(tc.expectedParams, params)
})
}
}
24 changes: 0 additions & 24 deletions modules/core/03-connection/keeper/params.go

This file was deleted.

17 changes: 0 additions & 17 deletions modules/core/03-connection/keeper/params_test.go

This file was deleted.

2 changes: 1 addition & 1 deletion modules/core/03-connection/keeper/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ func (k Keeper) VerifyNextSequenceRecv(
func (k Keeper) getBlockDelay(ctx sdk.Context, connection exported.ConnectionI) uint64 {
// expectedTimePerBlock should never be zero, however if it is then return a 0 blcok delay for safety
// as the expectedTimePerBlock parameter was not set.
expectedTimePerBlock := k.GetMaxExpectedTimePerBlock(ctx)
expectedTimePerBlock := k.GetParams(ctx).MaxExpectedTimePerBlock
if expectedTimePerBlock == 0 {
return 0
}
Expand Down
1 change: 1 addition & 0 deletions modules/core/03-connection/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func RegisterInterfaces(registry codectypes.InterfaceRegistry) {
&MsgConnectionOpenTry{},
&MsgConnectionOpenAck{},
&MsgConnectionOpenConfirm{},
&MsgUpdateConnectionParams{},
)

msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
Expand Down
3 changes: 3 additions & 0 deletions modules/core/03-connection/types/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const (

// ConnectionPrefix is the prefix used when creating a connection identifier
ConnectionPrefix = "connection-"

// ParamsKey is the store key for the IBC connection parameters
ParamsKey = "connectionParams"
)

// FormatConnectionIdentifier returns the connection identifier with the sequence appended.
Expand Down
26 changes: 26 additions & 0 deletions modules/core/03-connection/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ var (
_ sdk.Msg = (*MsgConnectionOpenConfirm)(nil)
_ sdk.Msg = (*MsgConnectionOpenAck)(nil)
_ sdk.Msg = (*MsgConnectionOpenTry)(nil)
_ sdk.Msg = (*MsgUpdateConnectionParams)(nil)

_ codectypes.UnpackInterfacesMessage = (*MsgConnectionOpenTry)(nil)
_ codectypes.UnpackInterfacesMessage = (*MsgConnectionOpenAck)(nil)
Expand Down Expand Up @@ -289,3 +290,28 @@ func (msg MsgConnectionOpenConfirm) GetSigners() []sdk.AccAddress {
}
return []sdk.AccAddress{accAddr}
}

// NewMsgUpdateConnectionParams creates a new MsgUpdateConnectionParams instance
func NewMsgUpdateConnectionParams(authority string, params Params) *MsgUpdateConnectionParams {
return &MsgUpdateConnectionParams{
Authority: authority,
Params: params,
}
}

// GetSigners returns the expected signers for a MsgUpdateConnectionParams message.
func (msg *MsgUpdateConnectionParams) GetSigners() []sdk.AccAddress {
accAddr, err := sdk.AccAddressFromBech32(msg.Authority)
if err != nil {
panic(err)
}
return []sdk.AccAddress{accAddr}
}

// ValidateBasic performs basic checks on a MsgUpdateConnectionParams.
func (msg *MsgUpdateConnectionParams) ValidateBasic() error {
if _, err := sdk.AccAddressFromBech32(msg.Authority); err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
}
return msg.Params.Validate()
}
35 changes: 35 additions & 0 deletions modules/core/03-connection/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,3 +231,38 @@ func (suite *MsgTestSuite) TestNewMsgConnectionOpenConfirm() {
}
}
}

// TestMsgUpdateConnectionParams_ValidateBasic tests ValidateBasic for MsgUpdateConnectionParams
func (suite *MsgTestSuite) TestMsgUpdateConnectionParams_ValidateBasic() {
authority := suite.chainA.App.GetIBCKeeper().GetAuthority()
testCases := []struct {
name string
msg *types.MsgUpdateConnectionParams
expPass bool
}{
{
"success: valid authority and params",
types.NewMsgUpdateConnectionParams(authority, types.DefaultParams()),
true,
},
{
"failure: invalid authority address",
types.NewMsgUpdateConnectionParams("invalid", types.DefaultParams()),
false,
},
{
"failure: invalid time per block",
types.NewMsgUpdateConnectionParams(authority, types.NewParams(0)),
false,
},
}

for _, tc := range testCases {
err := tc.msg.ValidateBasic()
if tc.expPass {
suite.Require().NoError(err, "valid case %s failed", tc.name)
} else {
suite.Require().Error(err, "invalid case %s passed", tc.name)
}
}
}
Loading

0 comments on commit 812e67f

Please sign in to comment.