Merge branch 'main' into sean/issue#802-update-register-ica

docs/app-modules/interchain-accounts/integration.md

@@ -94,7 +94,7 @@ app.ICAAuthKeeper = icaauthkeeper.NewKeeper(appCodec, keys[icaauthtypes.StoreKey
 icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper)
 
 // ICA auth IBC Module
-ICAAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper)
+icaAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper)
 
 // Create host and controller IBC Modules as desired
 icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthIBCModule)
@@ -122,4 +122,44 @@ app.mm.SetOrderInitGenesis(
     icatypes.ModuleName,
     ...
 )
-```
+```
+
+### Using submodules exclusively
+
+As described above, the Interchain Accounts application module is structured to support the ability of exclusively enabling controller or host functionality.
+This can be achieved by simply omitting either controller or host `Keeper` from the Interchain Accounts `NewAppModule` constructor function, and mounting only the desired submodule via the `IBCRouter`.
+Alternatively, submodules can be enabled and disabled dynamically using [on-chain parameters](./parameters.md).
+
+The following snippets show basic examples of statically disabling submodules using `app.go`.
+
+#### Disabling controller chain functionality
+
+```go
+// Create Interchain Accounts AppModule omitting the controller keeper
+icaModule := ica.NewAppModule(nil, &app.ICAHostKeeper)
+
+// Create host IBC Module
+icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper)
+
+// Register host route
+ibcRouter.AddRoute(icahosttypes.SubModuleName, icaHostIBCModule)
+```
+
+#### Disabling host chain functionality
+
+```go
+// Create Interchain Accounts AppModule omitting the host keeper
+icaModule := ica.NewAppModule(&app.ICAControllerKeeper, nil)
+
+// Create your Interchain Accounts authentication module, setting up the Keeper, AppModule and IBCModule appropriately
+app.ICAAuthKeeper = icaauthkeeper.NewKeeper(appCodec, keys[icaauthtypes.StoreKey], app.ICAControllerKeeper, scopedICAAuthKeeper)
+icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper)
+icaAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper)
+
+// Create controller IBC Module
+icaControllerIBCModule := icacontroller.NewIBCModule(app.ICAControllerKeeper, icaAuthIBCModule)
+
+// Register controller and authentication routes
+ibcRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerIBCModule)
+ibcRouter.AddRoute(icaauthtypes.ModuleName, icaControllerIBCModule) // Note, the authentication module is routed to the top level of the middleware stack
+```

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

@@ -9,7 +9,6 @@ import (
 
 	icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types"
 	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
-	porttypes "github.com/cosmos/ibc-go/v3/modules/core/05-port/types"
 )
 
 // OnChanOpenInit performs basic validation of channel initialization.
@@ -52,7 +51,7 @@ func (k Keeper) OnChanOpenInit(
 
 	activeChannelID, found := k.GetOpenActiveChannel(ctx, connectionHops[0], portID)
 	if found {
-		return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "existing active channel %s for portID %s", activeChannelID, portID)
+		return sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s", activeChannelID, portID)
 	}
 
 	return nil
@@ -79,6 +78,10 @@ func (k Keeper) OnChanOpenAck(
 		return sdkerrors.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata")
 	}
 
+	if activeChannelID, found := k.GetOpenActiveChannel(ctx, metadata.ControllerConnectionId, portID); found {
+		return sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s", activeChannelID, portID)
+	}
+
 	channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
 	if !found {
 		return sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)

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

@@ -247,6 +247,17 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() {
 			},
 			false,
 		},
+		{
+			"active channel already set",
+			func() {
+				// create a new channel and set it in state
+				ch := channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), []string{path.EndpointB.ConnectionID}, ibctesting.DefaultChannelVersion)
+				suite.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper.SetChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, ch)
+
+				// set the active channelID in state
+				suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+			}, false,
+		},
 	}
 
 	for _, tc := range testCases {

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

@@ -55,7 +55,7 @@ func (suite *KeeperTestSuite) TestExportGenesis() {
 	genesisState := keeper.ExportGenesis(suite.chainB.GetContext(), suite.chainB.GetSimApp().ICAHostKeeper)
 
 	suite.Require().Equal(path.EndpointB.ChannelID, genesisState.ActiveChannels[0].ChannelId)
-	suite.Require().Equal(path.EndpointB.ChannelConfig.PortID, genesisState.ActiveChannels[0].PortId)
+	suite.Require().Equal(path.EndpointA.ChannelConfig.PortID, genesisState.ActiveChannels[0].PortId)
 
 	suite.Require().Equal(TestAccAddress.String(), genesisState.InterchainAccounts[0].AccountAddress)
 	suite.Require().Equal(path.EndpointA.ChannelConfig.PortID, genesisState.InterchainAccounts[0].PortId)

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

@@ -47,6 +47,10 @@ func (k Keeper) OnChanOpenTry(
 		return "", err
 	}
 
+	if activeChannelID, found := k.GetOpenActiveChannel(ctx, connectionHops[0], counterparty.PortId); found {
+		return "", sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s", activeChannelID, portID)
+	}
+
 	// 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 {
@@ -78,7 +82,11 @@ func (k Keeper) OnChanOpenConfirm(
 		return sdkerrors.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
 	}
 
-	k.SetActiveChannelID(ctx, channel.ConnectionHops[0], portID, channelID)
+	// It is assumed the controller chain will not allow multiple active channels to be created for the same connectionID/portID
+	// If the controller chain does allow multiple active channels to be created for the same connectionID/portID, 
+	// disallowing overwriting the current active channel guarantees the channel can no longer be used as the controller
+	// and host will disagree on what the currently active channel is 
+	k.SetActiveChannelID(ctx, channel.ConnectionHops[0], channel.Counterparty.PortId, channelID)
 
 	return nil
 }

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

@@ -111,6 +111,17 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
 			},
 			false,
 		},
+		{
+			"active channel already set",
+			func() {
+				// create a new channel and set it in state
+				ch := channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), []string{path.EndpointA.ConnectionID}, ibctesting.DefaultChannelVersion)
+				suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.SetChannel(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointB.ChannelID, ch)
+
+				// set the active channelID in state
+				suite.chainB.GetSimApp().ICAHostKeeper.SetActiveChannelID(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointB.ChannelID)
+			}, false,
+		},
 	}
 
 	for _, tc := range testCases {

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

@@ -14,6 +14,7 @@ import (
 
 	"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/host/types"
 	icatypes "github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts/types"
+	channeltypes "github.com/cosmos/ibc-go/v3/modules/core/04-channel/types"
 	host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
 )
 
@@ -102,6 +103,22 @@ func (k Keeper) GetActiveChannelID(ctx sdk.Context, connectionID, portID string)
 	return string(store.Get(key)), true
 }
 
+// GetOpenActiveChannel retrieves the active channelID from the store, keyed by the provided connectionID and portID & checks if the channel in question is in state OPEN
+func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, connectionID, portID string) (string, bool) {
+	channelID, found := k.GetActiveChannelID(ctx, connectionID, portID)
+	if !found {
+		return "", false
+	}
+
+	channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID)
+
+	if found && channel.State == channeltypes.OPEN {
+		return channelID, true
+	}
+
+	return "", false
+}
+
 // GetAllActiveChannels returns a list of all active interchain accounts host channels and their associated connection and port identifiers
 func (k Keeper) GetAllActiveChannels(ctx sdk.Context) []icatypes.ActiveChannel {
 	store := ctx.KVStore(k.storeKey)

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

@@ -165,7 +165,7 @@ func (suite *KeeperTestSuite) TestGetAllActiveChannels() {
 	expectedChannels := []icatypes.ActiveChannel{
 		{
 			ConnectionId: ibctesting.FirstConnectionID,
-			PortId:       path.EndpointB.ChannelConfig.PortID,
+			PortId:       path.EndpointA.ChannelConfig.PortID,
 			ChannelId:    path.EndpointB.ChannelID,
 		},
 		{
@@ -223,7 +223,7 @@ func (suite *KeeperTestSuite) TestIsActiveChannel() {
 	err := SetupICAPath(path, TestOwnerAddress)
 	suite.Require().NoError(err)
 
-	isActive := suite.chainB.GetSimApp().ICAHostKeeper.IsActiveChannel(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointB.ChannelConfig.PortID)
+	isActive := suite.chainB.GetSimApp().ICAHostKeeper.IsActiveChannel(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID)
 	suite.Require().True(isActive)
 }