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: ctrl port connection id validation #454

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
755ce69
adding pipe char | to identifier regex
damiannolan Sep 29, 2021
a8128ce
updating GeneratePortID to use pipe delimiter in favour of dash, Pars…
damiannolan Sep 29, 2021
bce3407
adding CounterpartyHops method to expected channel keeper interface
damiannolan Sep 29, 2021
cd8ac79
updating tests to satisy delimiter updates
damiannolan Sep 29, 2021
45d8f8d
adding connection seq validation of ctrl port id and updating tests
damiannolan Sep 29, 2021
1ec19f1
cleanup
damiannolan Sep 29, 2021
5d01500
adding defensive check for ParseAddressFromVersion
damiannolan Sep 30, 2021
0c56d06
adding conn sequence parsing funcs to pkg types
damiannolan Oct 1, 2021
d3039bb
moving conn sequence validation to reusable func
damiannolan Oct 1, 2021
6761d2d
updating error msgs, adding tests for conn seq parsers
damiannolan Oct 1, 2021
e22f3ea
adding expected sequence to error msgs
damiannolan Oct 1, 2021
09f741a
updating ParseCtrlConnSequence to ParseControllerConnSequence
damiannolan Oct 1, 2021
6982141
fixing counterparty port error
damiannolan Oct 1, 2021
88bc3c8
Update modules/apps/27-interchain-accounts/keeper/handshake.go
damiannolan Oct 1, 2021
31536af
Update modules/apps/27-interchain-accounts/keeper/handshake.go
damiannolan Oct 1, 2021
d09703e
Update modules/apps/27-interchain-accounts/types/account.go
damiannolan Oct 1, 2021
ac87f9d
removing pipe from valid identifier regex
damiannolan Oct 1, 2021
b6925ab
adding error returns to parsing funcs, updating tests, error messages
damiannolan Oct 1, 2021
c21c153
separting imports in keys.go
damiannolan Oct 1, 2021
a8bab3a
Merge branch 'interchain-accounts' into damian/444-ctrl-port-connecti…
damiannolan Oct 1, 2021
6cedcf1
updating handshake tests
damiannolan Oct 1, 2021
59340ac
Update modules/apps/27-interchain-accounts/types/keys.go
damiannolan Oct 4, 2021
c84996f
renaming validation func, removing parenthesis in error msgs
damiannolan Oct 5, 2021
495c864
renaming func validateControllerPort -> validateControllerPortParams
damiannolan Oct 6, 2021
f68d7ec
Merge branch 'interchain-accounts' into damian/444-ctrl-port-connecti…
damiannolan Oct 6, 2021
b4d1191
Merge branch 'interchain-accounts' into damian/444-ctrl-port-connecti…
damiannolan Oct 7, 2021
9d4dfd7
Merge branch 'interchain-accounts' into damian/444-ctrl-port-connecti…
damiannolan Oct 7, 2021
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
81 changes: 78 additions & 3 deletions modules/apps/27-interchain-accounts/keeper/handshake.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
package keeper

import (
"strconv"
"strings"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/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"
porttypes "github.com/cosmos/ibc-go/v2/modules/core/05-port/types"
host "github.com/cosmos/ibc-go/v2/modules/core/24-host"
Expand All @@ -31,16 +35,50 @@ func (k Keeper) OnChanOpenInit(
version string,
) error {
if order != channeltypes.ORDERED {
return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "invalid channel ordering: %s, expected %s", order.String(), channeltypes.ORDERED.String())
return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
}

if counterparty.PortId != types.PortID {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "counterparty port-id must be '%s', (%s != %s)", types.PortID, counterparty.PortId, types.PortID)
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", types.PortID, portID)
}

if err := types.ValidateVersion(version); err != nil {
return sdkerrors.Wrap(err, "version validation failed")
}

s := strings.Split(portID, types.Delimiter)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
if len(s) != 4 {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "unexpected format in port ID (%s)", portID)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
}

channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
if !found {
return sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
}

counterpartyHops, found := k.channelKeeper.CounterpartyHops(ctx, channel)
if !found {
return sdkerrors.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0])
}

connSeq, err := connectiontypes.ParseConnectionSequence(connectionHops[0])
if err != nil {
return sdkerrors.Wrap(connectiontypes.ErrInvalidConnection, "failed to parse connection sequence")
}

counterpartyConnSeq, err := connectiontypes.ParseConnectionSequence(counterpartyHops[0])
if err != nil {
return sdkerrors.Wrap(connectiontypes.ErrInvalidConnection, "failed to parse counterparty connection sequence")
}

if strconv.FormatUint(connSeq, 10) != s[1] {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid connection sequence in port (%s)", s[1])
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
}

if strconv.FormatUint(counterpartyConnSeq, 10) != s[2] {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid connection sequence in port (%s)", s[2])
}

existingChannelID, found := k.GetActiveChannel(ctx, portID)
if found {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "existing active channel (%s) for portID (%s)", existingChannelID, portID)
Expand Down Expand Up @@ -70,7 +108,7 @@ func (k Keeper) OnChanOpenTry(
counterpartyVersion string,
) error {
if order != channeltypes.ORDERED {
return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "invalid channel ordering: %s, expected %s", order.String(), channeltypes.ORDERED.String())
return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order)
}

if err := types.ValidateVersion(version); err != nil {
Expand All @@ -81,6 +119,43 @@ func (k Keeper) OnChanOpenTry(
return sdkerrors.Wrap(err, "counterparty version validation failed")
}

if portID != types.PortID {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", types.PortID, portID)
}

s := strings.Split(counterparty.PortId, types.Delimiter)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
if len(s) != 4 {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "unexpected format in port ID (%s)", counterparty.PortId)
}

channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
if !found {
return sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
}

counterpartyHops, found := k.channelKeeper.CounterpartyHops(ctx, channel)
if !found {
return sdkerrors.Wrap(connectiontypes.ErrConnectionNotFound, channel.ConnectionHops[0])
}

connSeq, err := connectiontypes.ParseConnectionSequence(connectionHops[0])
if err != nil {
return sdkerrors.Wrap(connectiontypes.ErrInvalidConnection, "failed to parse connection sequence")
}

counterpartyConnSeq, err := connectiontypes.ParseConnectionSequence(counterpartyHops[0])
if err != nil {
return sdkerrors.Wrap(connectiontypes.ErrInvalidConnection, "failed to parse counterparty connection sequence")
}

if strconv.FormatUint(connSeq, 10) != s[2] {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid connection sequence in port (%s)", s[2])
}

if strconv.FormatUint(counterpartyConnSeq, 10) != s[1] {
return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid connection sequence in port (%s)", s[1])
}

// On the host chain the capability may only be claimed during the OnChanOpenTry
// The capability being claimed in OpenInit is for a controller chain (the port is different)
if err := k.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
Expand Down
182 changes: 157 additions & 25 deletions modules/apps/27-interchain-accounts/keeper/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,33 +23,92 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
}{

{
"success", func() {}, true,
"success",
func() {
path.EndpointA.SetChannel(*channel)
},
true,
},
{
"invalid order - UNORDERED", func() {
"invalid order - UNORDERED",
func() {
channel.Ordering = channeltypes.UNORDERED
}, false,
},
false,
},
{
"invalid counterparty port ID", func() {
channel.Counterparty.PortId = ibctesting.MockPort
}, false,
"invalid port ID",
func() {
path.EndpointA.ChannelConfig.PortID = "invalid-port-id"
},
false,
},
{
"invalid version", func() {
"invalid counterparty port ID",
func() {
channel.Counterparty.PortId = "invalid-port-id"
},
false,
},
{
"invalid version",
func() {
channel.Version = "version"
}, false,
},
false,
},
{
"channel is already active", func() {
"channel not found",
func() {
path.EndpointA.ChannelID = "invalid-channel-id"
},
false,
},
{
"connection not found",
func() {
channel.ConnectionHops = []string{"invalid-connnection-id"}
path.EndpointA.SetChannel(*channel)
},
false,
},
{
"invalid connection sequence",
func() {
portID, err := types.GeneratePortID(TestOwnerAddress, "connection-1", "connection-0")
suite.Require().NoError(err)

path.EndpointA.ChannelConfig.PortID = portID
path.EndpointA.SetChannel(*channel)
},
false,
},
{
"invalid counterparty connection sequence",
func() {
portID, err := types.GeneratePortID(TestOwnerAddress, "connection-0", "connection-1")
suite.Require().NoError(err)

path.EndpointA.ChannelConfig.PortID = portID
path.EndpointA.SetChannel(*channel)
},
false,
},
{
"channel is already active",
func() {
suite.chainA.GetSimApp().ICAKeeper.SetActiveChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
}, false,
},
false,
},
{
"capability already claimed", func() {
"capability already claimed",
func() {
path.EndpointA.SetChannel(*channel)
err := suite.chainA.GetSimApp().ScopedICAKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID))
suite.Require().NoError(err)
}, false,
},
false,
},
}

Expand Down Expand Up @@ -97,7 +156,6 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
}
}

// ChainA is controller, ChainB is host chain
func (suite *KeeperTestSuite) TestOnChanOpenTry() {
var (
channel *channeltypes.Channel
Expand All @@ -113,33 +171,103 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
}{

{
"success", func() {}, true,
"success",
func() {
path.EndpointB.SetChannel(*channel)
},
true,
},
{
"invalid order - UNORDERED", func() {
"invalid order - UNORDERED",
func() {
channel.Ordering = channeltypes.UNORDERED
}, false,
},
false,
},
{
"invalid port",
func() {
path.EndpointB.ChannelConfig.PortID = "invalid-port-id"
},
false,
},
{
"invalid counterparty port",
func() {
channel.Counterparty.PortId = "invalid-port-id"
},
false,
},
{
"invalid version", func() {
"channel not found",
func() {
path.EndpointB.ChannelID = "invalid-channel-id"
},
false,
},
{
"connection not found",
func() {
channel.ConnectionHops = []string{"invalid-connnection-id"}
path.EndpointB.SetChannel(*channel)
},
false,
},
{
"invalid connection sequence",
func() {
portID, err := types.GeneratePortID(TestOwnerAddress, "connection-0", "connection-1")
suite.Require().NoError(err)

channel.Counterparty.PortId = portID
path.EndpointB.SetChannel(*channel)
},
false,
},
{
"invalid counterparty connection sequence",
func() {
portID, err := types.GeneratePortID(TestOwnerAddress, "connection-1", "connection-0")
suite.Require().NoError(err)

channel.Counterparty.PortId = portID
path.EndpointB.SetChannel(*channel)
},
false,
},
{
"invalid version",
func() {
channel.Version = "version"
}, false,
},
false,
},
{
"invalid counterparty version", func() {
"invalid counterparty version",
func() {
counterpartyVersion = "version"
}, false,
},
false,
},
{
"capability already claimed", func() {
"capability already claimed",
func() {
path.EndpointB.SetChannel(*channel)
err := suite.chainB.GetSimApp().ScopedICAKeeper.ClaimCapability(suite.chainB.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID))
suite.Require().NoError(err)
}, false,
},
false,
},
{
"invalid account address", func() {
channel.Counterparty.PortId = "invalid-port-id"
}, false,
"invalid account address",
func() {
portID, err := types.GeneratePortID("invalid-owner-addr", "connection-0", "connection-0")
suite.Require().NoError(err)

channel.Counterparty.PortId = portID
path.EndpointB.SetChannel(*channel)
},
false,
},
}

Expand All @@ -155,6 +283,10 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
err := InitInterchainAccount(path.EndpointA, TestOwnerAddress)
suite.Require().NoError(err)

// set the channel id on host
channelSequence := path.EndpointB.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(path.EndpointB.Chain.GetContext())
path.EndpointB.ChannelID = channeltypes.FormatChannelIdentifier(channelSequence)

// default values
counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
channel = &channeltypes.Channel{
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/27-interchain-accounts/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ var (
// TestOwnerAddress defines a reusable bech32 address for testing purposes
TestOwnerAddress = "cosmos17dtl0mjt3t77kpuhg2edqzjpszulwhgzuj9ljs"
// TestPortID defines a resuable port identifier for testing purposes
TestPortID = fmt.Sprintf("%s-0-0-%s", types.VersionPrefix, TestOwnerAddress)
TestPortID = fmt.Sprintf("%s|0|0|%s", types.VersionPrefix, TestOwnerAddress)
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
// TestVersion defines a resuable interchainaccounts version string for testing purposes
TestVersion = types.NewAppVersion(types.VersionPrefix, TestAccAddress.String())
)
Expand Down
Loading