Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

imp: modify ica controller NewIBCMiddleware to default to nil auth module #6749

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions docs/docs/02-apps/02-interchain-accounts/04-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down
11 changes: 11 additions & 0 deletions docs/docs/05-migrations/13-v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/29-fee/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion modules/apps/callbacks/testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/08-wasm/testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading