From ef9a479781fe05a9a98ebbb2c425cca94ea3a2fe Mon Sep 17 00:00:00 2001
From: DimitrisJim <d.f.hilliard@gmail.com>
Date: Wed, 20 Dec 2023 09:48:21 +0200
Subject: [PATCH] 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.
---
 .../controller/ibc_middleware.go              | 20 ++++++++--------
 .../controller/ibc_middleware_test.go         | 24 +++++++++++--------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
index a5f8dcaada8..6e7b9b071f4 100644
--- 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
index 1c1cf517801..24212a88f23 100644
--- 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)