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

chore: ica audit nitpicks #483

Merged
merged 6 commits into from
Oct 13, 2021
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
10 changes: 5 additions & 5 deletions modules/apps/27-interchain-accounts/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ func (k Keeper) OnChanOpenInit(
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", types.PortID, counterparty.PortId)
}

if err := types.ValidateVersion(version); err != nil {
return sdkerrors.Wrap(err, "version validation failed")
if version != types.VersionPrefix {
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.VersionPrefix, version)
}

existingChannelID, found := k.GetActiveChannel(ctx, portID)
Expand Down Expand Up @@ -111,8 +111,8 @@ func (k Keeper) OnChanOpenTry(
return sdkerrors.Wrap(err, "version validation failed")
}

if err := types.ValidateVersion(counterpartyVersion); err != nil {
return sdkerrors.Wrap(err, "counterparty version validation failed")
if counterpartyVersion != types.VersionPrefix {
return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.VersionPrefix, version)
}

// On the host chain the capability may only be claimed during the OnChanOpenTry
Expand All @@ -129,7 +129,7 @@ func (k Keeper) OnChanOpenTry(
}

if parsedAddr != accAddr.String() {
return sdkerrors.Wrapf(types.ErrInvalidAccountAddress, "version contains invalid account address: expected %s, got %s", parsedAddr, accAddr)
return sdkerrors.Wrapf(types.ErrInvalidVersion, "version contains invalid account address: expected %s, got %s", parsedAddr, accAddr)
}

// Register interchain account if it does not already exist
Expand Down
21 changes: 1 addition & 20 deletions modules/apps/27-interchain-accounts/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,13 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

connectiontypes "github.com/cosmos/ibc-go/v2/modules/core/03-connection/types"
channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types"
ibcexported "github.com/cosmos/ibc-go/v2/modules/core/exported"
)

type Router interface {
Route(ctx sdk.Context, path string) sdk.Handler
}

// AccountKeeper defines the contract required for account APIs.
// AccountKeeper defines the expected account keeper
type AccountKeeper interface {
NewAccount(ctx sdk.Context, acc authtypes.AccountI) authtypes.AccountI
NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI
GetAccount(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI
SetAccount(ctx sdk.Context, acc authtypes.AccountI)
GetModuleAccount(ctx sdk.Context, name string) authtypes.ModuleAccountI
Expand All @@ -29,24 +23,11 @@ type ChannelKeeper interface {
GetChannel(ctx sdk.Context, srcPort, srcChan string) (channel channeltypes.Channel, found bool)
GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool)
SendPacket(ctx sdk.Context, channelCap *capabilitytypes.Capability, packet ibcexported.PacketI) error
ChanCloseInit(ctx sdk.Context, portID, channelID string, chanCap *capabilitytypes.Capability) error
ChanOpenInit(ctx sdk.Context, order channeltypes.Order, connectionHops []string, portID string, portCap *capabilitytypes.Capability, counterparty channeltypes.Counterparty, version string) (string, *capabilitytypes.Capability, error)
CounterpartyHops(ctx sdk.Context, channel channeltypes.Channel) ([]string, bool)
}

// ClientKeeper defines the expected IBC client keeper
type ClientKeeper interface {
GetClientState(ctx sdk.Context, clientID string) (ibcexported.ClientState, bool)
}

// ConnectionKeeper defines the expected IBC connection keeper
type ConnectionKeeper interface {
GetConnection(ctx sdk.Context, connectionID string) (connection connectiontypes.ConnectionEnd, found bool)
}

// PortKeeper defines the expected IBC port keeper
type PortKeeper interface {
BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability
IsBound(ctx sdk.Context, portID string) bool
LookupModuleByPort(ctx sdk.Context, portID string) (string, *capabilitytypes.Capability, error)
}
25 changes: 11 additions & 14 deletions modules/apps/27-interchain-accounts/types/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,27 +17,24 @@ var IsValidAddr = regexp.MustCompile("^[a-zA-Z0-9]*$").MatchString
// ValidateVersion performs basic validation of the provided ics27 version string.
// An ics27 version string may include an optional account address as per [TODO: Add spec when available]
// ValidateVersion first attempts to split the version string using the standard delimiter, then asserts a supported
// version prefix is included, followed by additional checks which enforce constraints on the optional account address.
// When no delimiter is present, only the version prefix is validated
// version prefix is included, followed by additional checks which enforce constraints on the account address.
func ValidateVersion(version string) error {
s := strings.Split(version, Delimiter)

if len(s) != 2 {
return sdkerrors.Wrapf(ErrInvalidVersion, "expected format <app-version%saccount-address>, got %s", Delimiter, version)
}

if s[0] != VersionPrefix {
return sdkerrors.Wrapf(ErrInvalidVersion, "expected %s, got %s", VersionPrefix, s[0])
}

if len(s) > 1 {
if len(s) != 2 {
return sdkerrors.Wrap(ErrInvalidAccountAddress, "unexpected address format")
}

if !IsValidAddr(s[1]) || len(s[1]) == 0 || len(s[1]) > DefaultMaxAddrLength {
return sdkerrors.Wrapf(
ErrInvalidAccountAddress,
"address must contain strictly alphanumeric characters, not exceeding %d characters in length",
DefaultMaxAddrLength,
)
}
if !IsValidAddr(s[1]) || len(s[1]) == 0 || len(s[1]) > DefaultMaxAddrLength {
return sdkerrors.Wrapf(
ErrInvalidAccountAddress,
"address must contain strictly alphanumeric characters, not exceeding %d characters in length",
DefaultMaxAddrLength,
)
}

return nil
Expand Down
16 changes: 8 additions & 8 deletions modules/apps/27-interchain-accounts/types/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@ func (suite *TypesTestSuite) TestValidateVersion() {
true,
},
{
"success - version prefix only",
types.VersionPrefix,
true,
"unexpected version string format",
"invalid-version-string-format",
false,
},
{
"unexpected version string format, additional delimiter",
types.NewAppVersion(types.VersionPrefix, "cosmos17dtl0mjt3t77kpu.hg2edqzjpszulwhgzuj9ljs"),
false,
},
{
"invalid version",
Expand All @@ -40,11 +45,6 @@ func (suite *TypesTestSuite) TestValidateVersion() {
types.NewAppVersion(types.VersionPrefix, "-_-"),
false,
},
{
"invalid account address - address contains additional delimiter",
types.NewAppVersion(types.VersionPrefix, "cosmos17dtl0mjt3t77kpu|hg2edqzjpszulwhgzuj9ljs"),
false,
},
}

for _, tc := range testCases {
Expand Down