ICA OnChanOpenTry update + tests (#299)

* update OnChanOpenTry to match ICS specs

* write OnChanOpenTry and OnChanOpenAck tests

* update comment

modules/apps/27-interchain-accounts/keeper/account.go

@@ -41,12 +41,13 @@ func (k Keeper) InitInterchainAccount(ctx sdk.Context, connectionId, owner strin
 }
 
 // Register interchain account if it has not already been created
-func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, portId string) (types.InterchainAccountI, error) {
+func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, portId string) {
 	address := k.GenerateAddress(portId)
 	account := k.accountKeeper.GetAccount(ctx, address)
 
 	if account != nil {
-		return nil, sdkerrors.Wrap(types.ErrAccountAlreadyExist, account.String())
+		// account already created, return no-op
+		return
 	}
 
 	interchainAccount := types.NewInterchainAccount(
@@ -57,7 +58,7 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, portId string) (types
 	k.accountKeeper.SetAccount(ctx, interchainAccount)
 	_ = k.SetInterchainAccountAddress(ctx, portId, interchainAccount.Address)
 
-	return interchainAccount, nil
+	return
 }
 
 func (k Keeper) SetInterchainAccountAddress(ctx sdk.Context, portId string, address string) string {

modules/apps/27-interchain-accounts/keeper/handshake.go

@@ -18,6 +18,8 @@ import (
 // there must not be an active channel for the specfied port identifier,
 // and the interchain accounts module must be able to claim the channel
 // capability.
+//
+// Controller Chain
 func (k Keeper) OnChanOpenInit(
 	ctx sdk.Context,
 	order channeltypes.Order,
@@ -50,9 +52,10 @@ func (k Keeper) OnChanOpenInit(
 	return nil
 }
 
-// register account (if it doesn't exist)
-// check if counterpary version is the same
-// TODO: remove ics27-1 hardcoded
+// OnChanOpenTry performs basic validation of the ICA channel
+// and registers a new interchain account (if it doesn't exist).
+//
+// Host Chain
 func (k Keeper) OnChanOpenTry(
 	ctx sdk.Context,
 	order channeltypes.Order,
@@ -67,19 +70,21 @@ func (k Keeper) OnChanOpenTry(
 	if order != channeltypes.ORDERED {
 		return sdkerrors.Wrapf(channeltypes.ErrInvalidChannelOrdering, "invalid channel ordering: %s, expected %s", order.String(), channeltypes.ORDERED.String())
 	}
+	if version != types.Version {
+		return sdkerrors.Wrapf(types.ErrInvalidVersion, "got: %s, expected %s", version, types.Version)
+	}
+	if counterpartyVersion != types.Version {
+		return sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: %s, expected %s", counterpartyVersion, types.Version)
+	}
 
-	// TODO: Check counterparty version
-	// if counterpartyVersion != types.Version {
-	// 	return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "invalid counterparty version: %s, expected %s", counterpartyVersion, "ics20-1")
-	// }
-
-	// Claim channel capability passed back by IBC module
+	// 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 {
-		return sdkerrors.Wrap(channeltypes.ErrChannelCapabilityNotFound, err.Error())
+		return err
 	}
 
 	// Register interchain account if it does not already exist
-	_, _ = k.RegisterInterchainAccount(ctx, counterparty.PortId)
+	k.RegisterInterchainAccount(ctx, counterparty.PortId)
 	return nil
 }
 

modules/apps/27-interchain-accounts/keeper/handshake_test.go

@@ -96,3 +96,137 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() {
 		})
 	}
 }
+
+// ChainA is controller, ChainB is host chain
+func (suite *KeeperTestSuite) TestOnChanOpenTry() {
+	var (
+		channel             *channeltypes.Channel
+		path                *ibctesting.Path
+		chanCap             *capabilitytypes.Capability
+		counterpartyVersion string
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+
+		{
+			"success", func() {}, true,
+		},
+		{
+			"invalid order - UNORDERED", func() {
+				channel.Ordering = channeltypes.UNORDERED
+			}, false,
+		},
+		{
+			"invalid version", func() {
+				channel.Version = "version"
+			}, false,
+		},
+		{
+			"invalid counterparty version", func() {
+				counterpartyVersion = "version"
+			}, false,
+		},
+		{
+			"capability already claimed", func() {
+				err := suite.chainB.GetSimApp().ScopedICAKeeper.ClaimCapability(suite.chainB.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID))
+				suite.Require().NoError(err)
+			}, false,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		suite.Run(tc.name, func() {
+			suite.SetupTest() // reset
+			path = NewICAPath(suite.chainA, suite.chainB)
+			owner := "owner"
+			counterpartyVersion = types.Version
+			suite.coordinator.SetupConnections(path)
+
+			err := InitInterchainAccount(path.EndpointA, owner)
+			suite.Require().NoError(err)
+
+			// default values
+			counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+			channel = &channeltypes.Channel{
+				State:          channeltypes.TRYOPEN,
+				Ordering:       channeltypes.ORDERED,
+				Counterparty:   counterparty,
+				ConnectionHops: []string{path.EndpointB.ConnectionID},
+				Version:        types.Version,
+			}
+
+			chanCap, err = suite.chainB.App.GetScopedIBCKeeper().NewCapability(suite.chainB.GetContext(), host.ChannelCapabilityPath(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID))
+			suite.Require().NoError(err)
+
+			tc.malleate() // explicitly change fields in channel and testChannel
+
+			err = suite.chainB.GetSimApp().ICAKeeper.OnChanOpenTry(suite.chainB.GetContext(), channel.Ordering, channel.GetConnectionHops(),
+				path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, chanCap, channel.Counterparty, channel.GetVersion(),
+				counterpartyVersion,
+			)
+
+			if tc.expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.Require().Error(err)
+			}
+
+		})
+	}
+}
+
+// ChainA is controller, ChainB is host chain
+func (suite *KeeperTestSuite) TestOnChanOpenAck() {
+	var (
+		path                *ibctesting.Path
+		counterpartyVersion string
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+
+		{
+			"success", func() {}, true,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		suite.Run(tc.name, func() {
+			suite.SetupTest() // reset
+			path = NewICAPath(suite.chainA, suite.chainB)
+			owner := "owner"
+			counterpartyVersion = types.Version
+			suite.coordinator.SetupConnections(path)
+
+			err := InitInterchainAccount(path.EndpointA, owner)
+			suite.Require().NoError(err)
+
+			err = path.EndpointB.ChanOpenTry()
+			suite.Require().NoError(err)
+
+			tc.malleate() // explicitly change fields in channel and testChannel
+
+			err = suite.chainA.GetSimApp().ICAKeeper.OnChanOpenAck(suite.chainA.GetContext(),
+				path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, counterpartyVersion,
+			)
+
+			if tc.expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.Require().Error(err)
+			}
+
+		})
+	}
+}

modules/apps/27-interchain-accounts/keeper/keeper.go

@@ -161,3 +161,8 @@ func (k Keeper) IsActiveChannel(ctx sdk.Context, portId string) bool {
 	_, found := k.GetActiveChannel(ctx, portId)
 	return found
 }
+
+// AuthenticateCapability wraps the scopedKeeper's AuthenticateCapability function
+func (k Keeper) AuthenticateCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) bool {
+	return k.scopedKeeper.AuthenticateCapability(ctx, cap, name)
+}

modules/apps/27-interchain-accounts/keeper/keeper_test.go

@@ -6,6 +6,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/cosmos/ibc-go/modules/apps/27-interchain-accounts/types"
+	channeltypes "github.com/cosmos/ibc-go/modules/core/04-channel/types"
 	ibctesting "github.com/cosmos/ibc-go/testing"
 )
 
@@ -31,10 +32,33 @@ func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path {
 	path := ibctesting.NewPath(chainA, chainB)
 	path.EndpointA.ChannelConfig.PortID = types.PortID
 	path.EndpointB.ChannelConfig.PortID = types.PortID
+	path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED
+	path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED
+	path.EndpointA.ChannelConfig.Version = types.Version
+	path.EndpointB.ChannelConfig.Version = types.Version
 
 	return path
 }
 
+// InitInterchainAccount is a helper function for starting the channel handshake
+func InitInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error {
+	portID := endpoint.Chain.GetSimApp().ICAKeeper.GeneratePortId(owner, endpoint.ConnectionID)
+	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
+
+	if err := endpoint.Chain.GetSimApp().ICAKeeper.InitInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner); err != nil {
+		return err
+	}
+
+	// commit state changes for proof verification
+	endpoint.Chain.App.Commit()
+	endpoint.Chain.NextBlock()
+
+	// update port/channel ids
+	endpoint.ChannelID = channeltypes.FormatChannelIdentifier(channelSequence)
+	endpoint.ChannelConfig.PortID = portID
+	return nil
+}
+
 func TestKeeperTestSuite(t *testing.T) {
 	suite.Run(t, new(KeeperTestSuite))
 }

modules/apps/27-interchain-accounts/types/errors.go

@@ -5,13 +5,14 @@ import (
 )
 
 var (
-	ErrUnknownPacketData           = sdkerrors.Register(ModuleName, 1, "Unknown packet data")
-	ErrAccountAlreadyExist         = sdkerrors.Register(ModuleName, 2, "Account already exist")
-	ErrPortAlreadyBound            = sdkerrors.Register(ModuleName, 3, "Port is already bound for address")
-	ErrUnsupportedChain            = sdkerrors.Register(ModuleName, 4, "Unsupported chain")
-	ErrInvalidOutgoingData         = sdkerrors.Register(ModuleName, 5, "Invalid outgoing data")
-	ErrInvalidRoute                = sdkerrors.Register(ModuleName, 6, "Invalid route")
-	ErrInterchainAccountNotFound   = sdkerrors.Register(ModuleName, 7, "Interchain account not found")
-	ErrInterchainAccountAlreadySet = sdkerrors.Register(ModuleName, 8, "Interchain Account is already set")
-	ErrActiveChannelNotFound       = sdkerrors.Register(ModuleName, 9, "No active channel for this owner")
+	ErrUnknownPacketData           = sdkerrors.Register(ModuleName, 2, "unknown packet data")
+	ErrAccountAlreadyExist         = sdkerrors.Register(ModuleName, 3, "account already exist")
+	ErrPortAlreadyBound            = sdkerrors.Register(ModuleName, 4, "port is already bound for address")
+	ErrUnsupportedChain            = sdkerrors.Register(ModuleName, 5, "unsupported chain")
+	ErrInvalidOutgoingData         = sdkerrors.Register(ModuleName, 6, "invalid outgoing data")
+	ErrInvalidRoute                = sdkerrors.Register(ModuleName, 7, "invalid route")
+	ErrInterchainAccountNotFound   = sdkerrors.Register(ModuleName, 8, "Interchain Account not found")
+	ErrInterchainAccountAlreadySet = sdkerrors.Register(ModuleName, 9, "Interchain Account is already set")
+	ErrActiveChannelNotFound       = sdkerrors.Register(ModuleName, 10, "no active channel for this owner")
+	ErrInvalidVersion              = sdkerrors.Register(ModuleName, 11, "invalid interchain accounts version")
 )