From 82b5fb668b6f1c918023fb7be72a8606d2329d81 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 2 Jul 2024 23:26:38 +0200 Subject: [PATCH 1/6] changed NewIBCMiddleware signature --- docs/docs/05-migrations/13-v8-to-v9.md | 11 +++++++++++ .../controller/ibc_middleware.go | 12 +++++++++++- .../controller/ibc_middleware_test.go | 18 +++++++++--------- modules/apps/29-fee/fee_test.go | 2 +- modules/apps/callbacks/testing/simapp/app.go | 2 +- .../08-wasm/testing/simapp/app.go | 2 +- testing/simapp/app.go | 2 +- 7 files changed, 35 insertions(+), 14 deletions(-) diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index 02c205f74e8..6e903852bda 100644 --- 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` contrusctor 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 only takes now as argument the controller keeper: + +```diff +func NewIBCMiddleware( +- app porttypes.IBCModule, + k keeper.Keeper, +) IBCMiddleware { +``` + +The underlying app is then set by default to nil and thus authentication fallbacks to a Cosmos SDK module using the message server. An underlying application can be set using the newly added `NewIBCMiddlewareWithAuth` constructor function. + ### 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 index dc27eeda2d8..117ed723902 100644 --- 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. +// The underlying application is set to nil and then authentication fallbacks +// to a Cosmos SDK module using the message server. +func NewIBCMiddleware(k keeper.Keeper) IBCMiddleware { + return IBCMiddleware{ + app: nil, + keeper: k, + } +} + // NewIBCMiddleware creates a new IBCMiddleware given the associated keeper and underlying application -func NewIBCMiddleware(app porttypes.IBCModule, k keeper.Keeper) IBCMiddleware { +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 index 634b66fed31..f5ba5299ef0 100644 --- 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 index fa36aee8093..a5431424a14 100644 --- 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 index 2cad843b1a8..a01a8e01657 100644 --- 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 index c663f035104..1d41543aeb0 100644 --- 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 index 8f6f4def7c6..5fdd714d43a 100644 --- 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: From 2a60860e927d76e182c4059eeda68d3ee84df028 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Wed, 3 Jul 2024 12:52:42 +0200 Subject: [PATCH 2/6] update docs --- docs/docs/02-apps/02-interchain-accounts/04-integration.md | 4 ++-- .../02-interchain-accounts/10-legacy/02-integration.md | 4 ++-- docs/docs/05-migrations/13-v8-to-v9.md | 4 ++-- .../apps/27-interchain-accounts/controller/ibc_middleware.go | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/docs/02-apps/02-interchain-accounts/04-integration.md b/docs/docs/02-apps/02-interchain-accounts/04-integration.md index 8547c2198ef..3c5cb2e62aa 100644 --- 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 index 90a645aaabd..1999223c02f 100644 --- 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 index 6e903852bda..8a41d97773b 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -88,7 +88,7 @@ func NewMsgModuleQuerySafe( ) *MsgModuleQuerySafe { ``` -The signature of the [`NewIBCMiddleware` contrusctor 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 only takes now as argument the controller keeper: +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 only takes now as argument the controller keeper: ```diff func NewIBCMiddleware( @@ -97,7 +97,7 @@ func NewIBCMiddleware( ) IBCMiddleware { ``` -The underlying app is then set by default to nil and thus authentication fallbacks to a Cosmos SDK module using the message server. An underlying application can be set using the newly added `NewIBCMiddlewareWithAuth` constructor function. +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 diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 117ed723902..7b1fc75fb9c 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -33,8 +33,8 @@ type IBCMiddleware struct { } // NewIBCMiddleware creates a new IBCMiddleware given the associated keeper. -// The underlying application is set to nil and then authentication fallbacks -// to a Cosmos SDK module using the message server. +// 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, From 76c8d99945e5fc989fc9e2a699cce1029c5de52f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 3 Jul 2024 16:47:53 +0200 Subject: [PATCH 3/6] Update docs/docs/05-migrations/13-v8-to-v9.md --- docs/docs/05-migrations/13-v8-to-v9.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index 8a41d97773b..c1e9f4edc42 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -88,7 +88,7 @@ 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 only takes now as argument the controller keeper: +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( From 236edbec7948a4434e31aaa1a346fe0f93bd7295 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 3 Jul 2024 16:48:03 +0200 Subject: [PATCH 4/6] Update modules/apps/27-interchain-accounts/controller/ibc_middleware.go Co-authored-by: DimitrisJim --- .../apps/27-interchain-accounts/controller/ibc_middleware.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 7b1fc75fb9c..3ab3144e26d 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -42,7 +42,7 @@ func NewIBCMiddleware(k keeper.Keeper) IBCMiddleware { } } -// NewIBCMiddleware creates a new IBCMiddleware given the associated keeper and underlying application +// 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, From 80acb1989d49adb02350e2f8a3b25201b2b22df0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 3 Jul 2024 16:49:29 +0200 Subject: [PATCH 5/6] Update docs/docs/05-migrations/13-v8-to-v9.md --- docs/docs/05-migrations/13-v8-to-v9.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index c1e9f4edc42..f34ae44017c 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -97,7 +97,7 @@ func NewIBCMiddleware( ) 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). +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 From 436947683b7dbb9e592a26548d293faaaaead961 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Wed, 3 Jul 2024 21:41:26 +0200 Subject: [PATCH 6/6] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92661f15033..f142469a87a 100644 --- 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