imp: modify ica controller NewIBCMiddleware to default to nil auth …

…module (#6749) * changed NewIBCMiddleware signature * update docs * Update docs/docs/05-migrations/13-v8-to-v9.md * Update modules/apps/27-interchain-accounts/controller/ibc_middleware.go Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com> * Update docs/docs/05-migrations/13-v8-to-v9.md * add changelog --------- Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
cosmos · Jul 3, 2024 · 356e433 · 356e433
1 parent 9a0493a
commit 356e433
Show file tree

Hide file tree

Showing 10 changed files with 41 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -61,6 +61,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (apps/27-interchain-accounts) [\#6598](https://github.com/cosmos/ibc-go/pull/6598) Mark the `requests` repeated field of `MsgModuleQuerySafe` non-nullable.
 * (23-commmitment) [\#6644](https://github.com/cosmos/ibc-go/pull/6644) Introduce commitment/v2 `MerklePath` to include `repeated bytes` in favour of `repeated string`. This supports using merkle path keys which include non UTF-8 encoded runes.
 * (23-commmitment) [\#6633](https://github.com/cosmos/ibc-go/pull/6633) MerklePath has been changed to use `repeated bytes` in favour of `repeated strings`.
+* (apps/27-interchain-accounts) [\#6749](https://github.com/cosmos/ibc-go/pull/6749) The ICA controller `NewIBCMiddleware` constructor function sets by default the auth module to nil.
 
 ### State Machine Breaking
 

diff --git a/docs/docs/02-apps/02-interchain-accounts/04-integration.md b/docs/docs/02-apps/02-interchain-accounts/04-integration.md
@@ -104,7 +104,7 @@ app.ICAAuthKeeper = icaauthkeeper.NewKeeper(appCodec, keys[icaauthtypes.StoreKey
 icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper)
 
 // Create controller IBC application stack and host IBC module as desired
-icaControllerStack := icacontroller.NewIBCMiddleware(nil, app.ICAControllerKeeper)
+icaControllerStack := icacontroller.NewIBCMiddleware(app.ICAControllerKeeper)
 icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper)
 
 // Register host and authentication routes
@@ -194,7 +194,7 @@ icaModule := ica.NewAppModule(&app.ICAControllerKeeper, nil)
 ...
 
 // Create controller IBC application stack
-icaControllerStack := icacontroller.NewIBCMiddleware(nil, app.ICAControllerKeeper)
+icaControllerStack := icacontroller.NewIBCMiddleware(app.ICAControllerKeeper)
 
 // Register controller route
 ibcRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)

diff --git a/docs/docs/02-apps/02-interchain-accounts/10-legacy/02-integration.md b/docs/docs/02-apps/02-interchain-accounts/10-legacy/02-integration.md
@@ -110,7 +110,7 @@ icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper)
 icaAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper)
 
 // Create controller IBC application stack and host IBC module as desired
-icaControllerStack := icacontroller.NewIBCMiddleware(icaAuthIBCModule, app.ICAControllerKeeper)
+icaControllerStack := icacontroller.NewIBCMiddlewareWithAuth(icaAuthIBCModule, app.ICAControllerKeeper)
 icaHostIBCModule := icahost.NewIBCModule(app.ICAHostKeeper)
 
 // Register host and authentication routes
@@ -192,7 +192,7 @@ icaAuthModule := icaauth.NewAppModule(appCodec, app.ICAAuthKeeper)
 icaAuthIBCModule := icaauth.NewIBCModule(app.ICAAuthKeeper)
 
 // Create controller IBC application stack
-icaControllerStack := icacontroller.NewIBCMiddleware(icaAuthIBCModule, app.ICAControllerKeeper)
+icaControllerStack := icacontroller.NewIBCMiddlewareWithAuth(icaAuthIBCModule, app.ICAControllerKeeper)
 
 // Register controller and authentication routes
 ibcRouter.

diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md
@@ -88,6 +88,17 @@ func NewMsgModuleQuerySafe(
 ) *MsgModuleQuerySafe {
 ```
 
+The signature of the [`NewIBCMiddleware` constructor function](https://github.com/cosmos/ibc-go/blob/v8.0.0/modules/apps/27-interchain-accounts/controller/ibc_middleware.go#L35) in the controller submodule now only takes the controller keeper as an argument:
+
+```diff
+func NewIBCMiddleware(
+- app porttypes.IBCModule, 
+  k keeper.Keeper,
+) IBCMiddleware {
+```
+
+The base application is then set by default to nil and thus authentication is assumed to be done by a Cosmos SDK module, such as the `x/gov`, `x/group` or `x/auth`, that sends messages to the controller submodule's message server. An authentication module can be set using the newly added [`NewIBCMiddlewareWithAuth` constructor function](https://github.com/cosmos/ibc-go/blob/82b5fb668b6f1c918023fb7be72a8606d2329d81/modules/apps/27-interchain-accounts/controller/ibc_middleware.go#L46).
+
 ### IBC core
 
 ### API removals

diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -32,8 +32,18 @@ type IBCMiddleware struct {
 	keeper keeper.Keeper
 }
 
-// NewIBCMiddleware creates a new IBCMiddleware given the associated keeper and underlying application
-func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware {
+// NewIBCMiddleware creates a new IBCMiddleware given the associated keeper.
+// The underlying application is set to nil and authentication is assumed to
+// be performed by a Cosmos SDK module that sends messages to controller message server.
+func NewIBCMiddleware(k keeper.Keeper) IBCMiddleware {
+	return IBCMiddleware{
+		app:    nil,
+		keeper: k,
+	}
+}
+
+// NewIBCMiddlewareWithAuth creates a new IBCMiddleware given the associated keeper and underlying application
+func NewIBCMiddlewareWithAuth(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware {
 	return IBCMiddleware{
 		app:    app,
 		keeper: k,

diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go
@@ -234,7 +234,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
 				suite.Require().True(ok)
 
 				if isNilApp {
-					cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 				}
 
 				version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops,
@@ -384,7 +384,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenAck() {
 				err = cbs.OnChanOpenAck(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelID, path.EndpointB.ChannelConfig.Version)
 
 				if isNilApp {
-					cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 				}
 
 				if tc.expPass {
@@ -517,7 +517,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanCloseConfirm() {
 				suite.Require().True(ok)
 
 				if isNilApp {
-					cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 				}
 
 				err = cbs.OnChanCloseConfirm(
@@ -679,7 +679,7 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
 				suite.Require().True(ok)
 
 				if isNilApp {
-					cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 				}
 
 				err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, []byte("ack"), nil)
@@ -776,7 +776,7 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
 				suite.Require().True(ok)
 
 				if isNilApp {
-					cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 				}
 
 				err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil)
@@ -867,7 +867,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 				suite.Require().True(ok)
 
 				if isNilApp {
-					cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 				}
 
 				version, err = cbs.OnChanUpgradeInit(
@@ -995,7 +995,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 				suite.Require().True(ok)
 
 				if isNilApp {
-					cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 				}
 
 				err = cbs.OnChanUpgradeAck(
@@ -1098,12 +1098,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() {
 				if tc.expPanic != nil {
 					mockModule := ibcmock.NewAppModule(suite.chainA.App.GetIBCKeeper().PortKeeper)
 					mockApp := ibcmock.NewIBCApp(path.EndpointA.ChannelConfig.PortID, suite.chainA.App.GetScopedIBCKeeper())
-					cbs = controller.NewIBCMiddleware(ibcmock.NewBlockUpgradeMiddleware(&mockModule, mockApp), suite.chainA.GetSimApp().ICAControllerKeeper)
+					cbs = controller.NewIBCMiddlewareWithAuth(ibcmock.NewBlockUpgradeMiddleware(&mockModule, mockApp), suite.chainA.GetSimApp().ICAControllerKeeper)
 
 					suite.Require().PanicsWithError(tc.expPanic.Error(), func() { upgradeOpenCb(cbs) })
 				} else {
 					if isNilApp {
-						cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
+						cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
 					}
 
 					upgradeOpenCb(cbs)

diff --git a/modules/apps/29-fee/fee_test.go b/modules/apps/29-fee/fee_test.go
@@ -88,7 +88,7 @@ func RemoveFeeMiddleware(chain *ibctesting.TestChain) {
 
 	// Remove Fee middleware from icacontroller submodule
 	chain.GetSimApp().ICAControllerKeeper.WithICS4Wrapper(channelKeeper)
-	icaControllerStack := icacontroller.NewIBCMiddleware(nil, chain.GetSimApp().ICAControllerKeeper)
+	icaControllerStack := icacontroller.NewIBCMiddleware(chain.GetSimApp().ICAControllerKeeper)
 	newRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
 
 	// Override and seal the router

diff --git a/modules/apps/callbacks/testing/simapp/app.go b/modules/apps/callbacks/testing/simapp/app.go
@@ -535,7 +535,7 @@ func NewSimApp(
 	if !ok {
 		panic(fmt.Errorf("cannot convert %T to %T", icaControllerStack, app.ICAAuthModule))
 	}
-	icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
+	icaControllerStack = icacontroller.NewIBCMiddlewareWithAuth(icaControllerStack, app.ICAControllerKeeper)
 	icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
 	var icaICS4Wrapper porttypes.ICS4Wrapper
 	icaICS4Wrapper, ok = icaControllerStack.(porttypes.ICS4Wrapper)

diff --git a/modules/light-clients/08-wasm/testing/simapp/app.go b/modules/light-clients/08-wasm/testing/simapp/app.go
@@ -571,7 +571,7 @@ func NewSimApp(
 	if !ok {
 		panic(fmt.Errorf("cannot convert %T into %T", icaControllerStack, app.ICAAuthModule))
 	}
-	icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
+	icaControllerStack = icacontroller.NewIBCMiddlewareWithAuth(icaControllerStack, app.ICAControllerKeeper)
 	icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
 
 	// RecvPacket, message that originates from core IBC and goes down to app, the flow is:

diff --git a/testing/simapp/app.go b/testing/simapp/app.go
@@ -528,7 +528,7 @@ func NewSimApp(
 	if !ok {
 		panic(fmt.Errorf("cannot convert %T into %T", icaControllerStack, app.ICAAuthModule))
 	}
-	icaControllerStack = icacontroller.NewIBCMiddleware(icaControllerStack, app.ICAControllerKeeper)
+	icaControllerStack = icacontroller.NewIBCMiddlewareWithAuth(icaControllerStack, app.ICAControllerKeeper)
 	icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
 
 	// RecvPacket, message that originates from core IBC and goes down to app, the flow is: