ICA OnChanOpenTry update + tests

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

@@ -41,12 +41,12 @@ 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.IBCAccountI, error) {
+func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, portId string) error {
 	address := k.GenerateAddress(portId)
 	account := k.accountKeeper.GetAccount(ctx, address)
 
 	if account != nil {
-		return nil, sdkerrors.Wrap(types.ErrAccountAlreadyExist, account.String())
+		return sdkerrors.Wrap(types.ErrAccountAlreadyExist, account.String())
 	}
 
 	interchainAccount := types.NewIBCAccount(
@@ -57,7 +57,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 nil
 }
 
 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,28 @@ 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
-	if err := k.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
-		return sdkerrors.Wrap(channeltypes.ErrChannelCapabilityNotFound, err.Error())
+	// Module may have already claimed capability in OnChanOpenInit in the case of crossing hellos
+	// (ie chainA and chainB both call ChanOpenInit before one of them calls ChanOpenTry)
+	// If module can already authenticate the capability then module already owns it so we don't need to claim
+	// Otherwise, module does not have channel capability and we must claim it from IBC
+	if !k.AuthenticateCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)) {
+		// Only claim channel capability passed back by IBC module if we do not already own it
+		if err := k.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
+			return err
+		}
 	}
 
 	// Register interchain account if it does not already exist
-	_, _ = k.RegisterInterchainAccount(ctx, counterparty.PortId)
+	if err := k.RegisterInterchainAccount(ctx, counterparty.PortId); err != nil {
+		return err
+	}
 	return nil
 }
 

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

@@ -96,3 +96,92 @@ 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
+		err     error
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expPass  bool
+	}{
+
+		{
+			"success", func() {}, true,
+		},
+		{
+			"invalid order - UNORDERED", func() {
+				channel.Ordering = channeltypes.UNORDERED
+			}, false,
+		},
+		{
+			"invalid counterparty port ID", func() {
+				channel.Counterparty.PortId = ibctesting.MockPort
+			}, false,
+		},
+		{
+			"invalid version", func() {
+				channel.Version = "version"
+			}, false,
+		},
+		{
+			"channel is already active", func() {
+				suite.chainA.GetSimApp().ICAKeeper.SetActiveChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+			}, false,
+		},
+		{
+			"capability already claimed", func() {
+				err := suite.chainA.GetSimApp().ScopedICAKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(path.EndpointA.ChannelConfig.PortID, path.EndpointA.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.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
+			channel = &channeltypes.Channel{
+				State:          channeltypes.TRYOPEN,
+				Ordering:       channeltypes.ORDERED,
+				Counterparty:   counterparty,
+				ConnectionHops: []string{path.EndpointA.ConnectionID},
+				Version:        types.Version,
+			}
+
+			chanCap, err = suite.chainA.App.GetScopedIBCKeeper().NewCapability(suite.chainA.GetContext(), host.ChannelCapabilityPath(portID, path.EndpointA.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)
+			}
+
+		})
+	}
+}

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"
 )
 
@@ -35,6 +36,21 @@ func NewICAPath(chainA, chainB *ibctesting.TestChain) *ibctesting.Path {
 	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
+	}
+
+	// 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")
-	ErrIBCAccountNotFound    = sdkerrors.Register(ModuleName, 7, "Ibc account not found")
-	ErrIBCAccountAlreadySet  = 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")
+	ErrIBCAccountNotFound    = sdkerrors.Register(ModuleName, 8, "ibc account not found")
+	ErrIBCAccountAlreadySet  = 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")
 )