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

imp (claims): validate authorized channels in message to update claims params #1378

Merged
merged 16 commits into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

- (claims) [#1378](https://github.com/evmos/evmos/pull/1378) Validate authorized channels when updating claims params
- (test) [#1348](https://github.com/evmos/evmos/pull/1348) Add query executions to e2e upgrade test suite
- (deps) [#1370](https://github.com/evmos/evmos/pull/1370) Bump Cosmos SDK version to [`v0.46.9-ledger`](https://github.com/evmos/cosmos-sdk/releases/tag/v0.46.9-ledger)
- (deps) [#1370](https://github.com/evmos/evmos/pull/1370) Bump Tendermint version to [`v0.34.26`](https://github.com/informalsystems/tendermint/releases/tag/v0.34.26)
Expand Down
2 changes: 1 addition & 1 deletion app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ func NewEvmos(

app.ClaimsKeeper = claimskeeper.NewKeeper(
appCodec, keys[claimstypes.StoreKey], authtypes.NewModuleAddress(govtypes.ModuleName),
app.AccountKeeper, app.BankKeeper, &stakingKeeper, app.DistrKeeper,
app.AccountKeeper, app.BankKeeper, &stakingKeeper, app.DistrKeeper, app.IBCKeeper.ChannelKeeper,
)

// register the staking hooks
Expand Down
3 changes: 3 additions & 0 deletions x/claims/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type Keeper struct {
bankKeeper types.BankKeeper
stakingKeeper types.StakingKeeper
distrKeeper types.DistrKeeper
channelKeeper types.ChannelKeeper
ics4Wrapper porttypes.ICS4Wrapper
}

Expand All @@ -52,6 +53,7 @@ func NewKeeper(
bk types.BankKeeper,
sk types.StakingKeeper,
dk types.DistrKeeper,
ck types.ChannelKeeper,
) *Keeper {
// ensure gov module account is set and is not nil
if err := sdk.VerifyAddressFormat(authority); err != nil {
Expand All @@ -66,6 +68,7 @@ func NewKeeper(
bankKeeper: bk,
stakingKeeper: sk,
distrKeeper: dk,
channelKeeper: ck,
}
}

Expand Down
45 changes: 45 additions & 0 deletions x/claims/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,15 @@ package keeper

import (
"context"
"fmt"

errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"

"github.com/evmos/evmos/v11/x/claims/types"
)

Expand All @@ -35,9 +40,49 @@ func (k *Keeper) UpdateParams(goCtx context.Context, req *types.MsgUpdateParams)
}

ctx := sdk.UnwrapSDKContext(goCtx)

// Validate the requested authorized channels
for _, channelID := range req.Params.AuthorizedChannels {
if err := checkIfChannelOpen(ctx, k.channelKeeper, channelID); err != nil {
return nil, errorsmod.Wrapf(err, "invalid authorized channel")
}
}

// Validate the requested EVM channels
for _, channelID := range req.Params.EVMChannels {
if err := checkIfChannelOpen(ctx, k.channelKeeper, channelID); err != nil {
return nil, errorsmod.Wrapf(err, "invalid evm channel")
}
MalteHerrmann marked this conversation as resolved.
Show resolved Hide resolved
}

if err := k.SetParams(ctx, req.Params); err != nil {
return nil, err
}

return &types.MsgUpdateParamsResponse{}, nil
}

// checkIfChannelOpen checks if an IBC channel with the given channel id is registered
// in the channel keeper and is in the OPEN state. It also requires the channel id to
// be in a valid format.
//
// NOTE: this function is looking for a channel with the default transfer port id and will fail
// if no channel with the given channel id has an open connection to this port.
func checkIfChannelOpen(ctx sdk.Context, ck types.ChannelKeeper, channelID string) error {
channel, found := ck.GetChannel(ctx, transfertypes.PortID, channelID)
if !found {
return fmt.Errorf(
"trying to add a channel to the claims module's available channels parameters, when it is not found in the app's IBCKeeper.ChannelKeeper: %s",
channelID,
)
}

if channel.State != channeltypes.OPEN {
return fmt.Errorf(
"trying to add a channel to the claims module's available channels parameters, when it is not in the OPEN state: %s",
channelID,
)
}

return nil
}
92 changes: 82 additions & 10 deletions x/claims/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
@@ -1,38 +1,110 @@
package keeper_test

import (
"fmt"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"

"github.com/evmos/evmos/v11/x/claims/types"
)

func (suite *KeeperTestSuite) TestUpdateParams() {
// Add open channels to the channel keeper (0 & 3 are the default channels, 2 is the default evm channel)
channel := channeltypes.Channel{State: channeltypes.OPEN}
suite.app.IBCKeeper.ChannelKeeper.SetChannel(suite.ctx, transfertypes.PortID, channeltypes.ChannelPrefix+"0", channel)
suite.app.IBCKeeper.ChannelKeeper.SetChannel(suite.ctx, transfertypes.PortID, channeltypes.ChannelPrefix+"3", channel)
suite.app.IBCKeeper.ChannelKeeper.SetChannel(suite.ctx, transfertypes.PortID, channeltypes.ChannelPrefix+"2", channel)
// Add closed channel to the channel keeper
closedChannel := channeltypes.Channel{State: channeltypes.CLOSED}
suite.app.IBCKeeper.ChannelKeeper.SetChannel(suite.ctx, transfertypes.PortID, channeltypes.ChannelPrefix+"4", closedChannel)

testCases := []struct {
name string
request *types.MsgUpdateParams
expectErr bool
name string
request *types.MsgUpdateParams
expectErr bool
errContains string
}{
{
name: "fail - invalid authority",
request: &types.MsgUpdateParams{Authority: "foobar"},
expectErr: true,
name: "fail - invalid authority",
request: &types.MsgUpdateParams{Authority: "foobar"},
expectErr: true,
errContains: "invalid authority",
},
{
name: "pass - valid Update msg",
request: &types.MsgUpdateParams{
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Params: types.DefaultParams(),
},
expectErr: false,
expectErr: false,
errContains: "",
},
{
name: "fail - valid Update msg with unknown channel",
request: &types.MsgUpdateParams{
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Params: types.Params{
AuthorizedChannels: []string{"channel-0", "channel-1"},
},
},
expectErr: true,
errContains: "it is not found in the app's IBCKeeper.ChannelKeeper: channel-1",
},
{
name: "fail - valid Update msg with closed channel",
request: &types.MsgUpdateParams{
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Params: types.Params{
AuthorizedChannels: []string{"channel-0", "channel-4"},
},
},
expectErr: true,
errContains: "it is not in the OPEN state: channel-4",
},
{
name: "pass - valid Update msg with valid EVM channels",
request: &types.MsgUpdateParams{
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Params: types.Params{
EVMChannels: []string{"channel-0", "channel-2"},
},
},
expectErr: false,
errContains: "",
},
{
name: "fail - valid Update msg with unknown EVM channel",
request: &types.MsgUpdateParams{
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Params: types.Params{
EVMChannels: []string{"channel-6"},
},
},
expectErr: true,
errContains: "it is not found in the app's IBCKeeper.ChannelKeeper: channel-6",
},
{
name: "fail - valid Update msg with closed EVM channel",
request: &types.MsgUpdateParams{
Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
Params: types.Params{
EVMChannels: []string{"channel-4"},
},
},
expectErr: true,
errContains: "it is not in the OPEN state: channel-4",
},
}

for _, tc := range testCases {
tc := tc
suite.Run("MsgUpdateParams", func() {
suite.Run(fmt.Sprintf("MsgUpdateParams - %s", tc.name), func() {
_, err := suite.app.ClaimsKeeper.UpdateParams(suite.ctx, tc.request)
if tc.expectErr {
suite.Require().Error(err)
suite.Require().ErrorContains(err, tc.errContains)
} else {
suite.Require().NoError(err)
}
Expand Down
6 changes: 6 additions & 0 deletions x/claims/types/interfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
)

// BankKeeper defines the banking contract that must be fulfilled when
Expand All @@ -43,6 +44,11 @@ type AccountKeeper interface {
RemoveAccount(ctx sdk.Context, account authtypes.AccountI)
}

// ChannelKeeper is the keeper that controls access to IBC channels
type ChannelKeeper interface {
GetChannel(ctx sdk.Context, portID, channelID string) (channel channeltypes.Channel, found bool)
}

// DistrKeeper is the keeper of the distribution store
type DistrKeeper interface {
FundCommunityPool(ctx sdk.Context, amount sdk.Coins, sender sdk.AccAddress) error
Expand Down