Move cast to Upgradable module after checking app is set. (#5465)

* Move cast to Upgradable module _after_ checking app is set. * Add test to check nil apps are allowed.
cosmos · Dec 20, 2023 · ef9a479 · ef9a479
1 parent 5bad8f2
commit ef9a479
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 20 deletions.
diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -234,11 +234,6 @@ func (im IBCMiddleware) OnTimeoutPacket(
 
 // OnChanUpgradeInit implements the IBCModule interface
 func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
-	cbs, ok := im.app.(porttypes.UpgradableModule)
-	if !ok {
-		return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
-	}
-
 	if !im.keeper.GetParams(ctx).ControllerEnabled {
 		return "", types.ErrControllerSubModuleDisabled
 	}
@@ -254,6 +249,11 @@ func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID str
 	}
 
 	if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionID) {
+		// Only cast to UpgradableModule if the application is set.
+		cbs, ok := im.app.(porttypes.UpgradableModule)
+		if !ok {
+			return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
+		}
 		return cbs.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, version)
 	}
 
@@ -267,11 +267,6 @@ func (IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string,
 
 // OnChanUpgradeAck implements the IBCModule interface
 func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
-	cbs, ok := im.app.(porttypes.UpgradableModule)
-	if !ok {
-		return errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
-	}
-
 	if !im.keeper.GetParams(ctx).ControllerEnabled {
 		return types.ErrControllerSubModuleDisabled
 	}
@@ -286,6 +281,11 @@ func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, cou
 	}
 
 	if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionID) {
+		// Only cast to UpgradableModule if the application is set.
+		cbs, ok := im.app.(porttypes.UpgradableModule)
+		if !ok {
+			return errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
+		}
 		return cbs.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
 	}
 

diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go
@@ -780,6 +780,13 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 		{
 			"success", func() {}, nil,
 		},
+		{
+			"success: nil underlying app",
+			func() {
+				isNilApp = true
+			},
+			nil,
+		},
 		{
 			"controller submodule disabled", func() {
 				suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false))
@@ -797,11 +804,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 				}
 			}, ibcmock.MockApplicationCallbackError,
 		},
-		{
-			"nil underlying app", func() {
-				isNilApp = true
-			}, porttypes.ErrInvalidRoute,
-		},
 		{
 			"middleware disabled", func() {
 				suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)
@@ -876,6 +878,13 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 		{
 			"success", func() {}, nil,
 		},
+		{
+			"success: nil underlying app",
+			func() {
+				isNilApp = true
+			},
+			nil,
+		},
 		{
 			"controller submodule disabled", func() {
 				suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false))
@@ -893,11 +902,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 				}
 			}, ibcmock.MockApplicationCallbackError,
 		},
-		{
-			"nil underlying app", func() {
-				isNilApp = true
-			}, porttypes.ErrInvalidRoute,
-		},
 		{
 			"middleware disabled", func() {
 				suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)