Skip to content

Commit

Permalink
Add nil checks on the controller module for underlying app (backport #…
Browse files Browse the repository at this point in the history
…2102) (#2131)

* Add nil checks on the controller module for underlying app (#2102)

(cherry picked from commit dcd616c)

# Conflicts:
#	CHANGELOG.md

* fix conflicts

Co-authored-by: Cian Hatton <[email protected]>
Co-authored-by: Colin Axnér <[email protected]>
  • Loading branch information
3 people authored Aug 31, 2022
1 parent bf7468e commit cc16ae4
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (apps/27-interchain-accounts) [\#2102](https://github.com/cosmos/ibc-go/pull/2102) ICS27 controller middleware now supports a nil underlying application. This allows chains to make use of interchain accounts with existing auth mechanisms such as x/group and x/gov.
* (testing) [\#1942](https://github.com/cosmos/ibc-go/pull/1942) Add a balance for the mock module account upon testing package initialization.
* (linting) [\#1418](https://github.com/cosmos/ibc-go/pull/1418) Fix linting errors, resulting compatiblity with go1.18 linting style, golangci-lint 1.46.2 and the revivie linter. This caused breaking changes in core/04-channel, core/ante, and the testing library.

Expand Down
24 changes: 19 additions & 5 deletions modules/apps/27-interchain-accounts/controller/ibc_middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,10 @@ func (im IBCMiddleware) OnChanOpenInit(
// call underlying app's OnChanOpenInit callback with the passed in version
// the version returned is discarded as the ica-auth module does not have permission to edit the version string.
// ics27 will always return the version string containing the Metadata struct which is created during the `RegisterInterchainAccount` call.
if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil {
return "", err
if im.app != nil {
if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version); err != nil {
return "", err
}
}

return version, nil
Expand Down Expand Up @@ -101,7 +103,11 @@ func (im IBCMiddleware) OnChanOpenAck(
}

// call underlying app's OnChanOpenAck callback with the counterparty app version.
return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion)
if im.app != nil {
return im.app.OnChanOpenAck(ctx, portID, channelID, counterpartyChannelID, counterpartyVersion)
}

return nil
}

// OnChanOpenAck implements the IBCMiddleware interface
Expand Down Expand Up @@ -156,7 +162,11 @@ func (im IBCMiddleware) OnAcknowledgementPacket(
}

// call underlying app's OnAcknowledgementPacket callback.
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
if im.app != nil {
return im.app.OnAcknowledgementPacket(ctx, packet, acknowledgement, relayer)
}

return nil
}

// OnTimeoutPacket implements the IBCMiddleware interface
Expand All @@ -173,7 +183,11 @@ func (im IBCMiddleware) OnTimeoutPacket(
return err
}

return im.app.OnTimeoutPacket(ctx, packet, relayer)
if im.app != nil {
return im.app.OnTimeoutPacket(ctx, packet, relayer)
}

return nil
}

// SendPacket implements the ICS4 Wrapper interface
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/suite"
"github.com/tendermint/tendermint/crypto"

"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller"
"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
fee "github.com/cosmos/ibc-go/v5/modules/apps/29-fee"
Expand Down Expand Up @@ -120,7 +121,10 @@ func SetupICAPath(path *ibctesting.Path, owner string) error {
}

func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
var channel *channeltypes.Channel
var (
channel *channeltypes.Channel
isNilApp bool
)

testCases := []struct {
name string
Expand Down Expand Up @@ -162,13 +166,19 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
}
}, false,
},
{
"nil underlying app", func() {
isNilApp = true
}, true,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest() // reset
isNilApp = false

path := NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)
Expand Down Expand Up @@ -207,6 +217,10 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

if isNilApp {
cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
}

version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.GetConnectionHops(),
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.GetVersion(),
)
Expand Down Expand Up @@ -271,7 +285,10 @@ func (suite *InterchainAccountsTestSuite) TestChanOpenTry() {
}

func (suite *InterchainAccountsTestSuite) TestOnChanOpenAck() {
var path *ibctesting.Path
var (
path *ibctesting.Path
isNilApp bool
)

testCases := []struct {
name string
Expand Down Expand Up @@ -300,13 +317,19 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenAck() {
}
}, false,
},
{
"nil underlying app", func() {
isNilApp = true
}, true,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest() // reset
isNilApp = false

path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)
Expand All @@ -327,6 +350,10 @@ 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)
}

if tc.expPass {
suite.Require().NoError(err)
} else {
Expand Down Expand Up @@ -497,7 +524,10 @@ func (suite *InterchainAccountsTestSuite) TestOnRecvPacket() {
}

func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
var path *ibctesting.Path
var (
path *ibctesting.Path
isNilApp bool
)

testCases := []struct {
msg string
Expand All @@ -523,11 +553,17 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
}
}, false,
},
{
"nil underlying app", func() {
isNilApp = true
}, true,
},
}

for _, tc := range testCases {
suite.Run(tc.msg, func() {
suite.SetupTest() // reset
isNilApp = false

path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)
Expand All @@ -554,6 +590,10 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

if isNilApp {
cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
}

err = cbs.OnAcknowledgementPacket(suite.chainA.GetContext(), packet, []byte("ack"), nil)

if tc.expPass {
Expand All @@ -566,7 +606,10 @@ func (suite *InterchainAccountsTestSuite) TestOnAcknowledgementPacket() {
}

func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
var path *ibctesting.Path
var (
path *ibctesting.Path
isNilApp bool
)

testCases := []struct {
msg string
Expand All @@ -592,11 +635,17 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
}
}, false,
},
{
"nil underlying app", func() {
isNilApp = true
}, true,
},
}

for _, tc := range testCases {
suite.Run(tc.msg, func() {
suite.SetupTest() // reset
isNilApp = false

path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)
Expand All @@ -623,6 +672,10 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

if isNilApp {
cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
}

err = cbs.OnTimeoutPacket(suite.chainA.GetContext(), packet, nil)

if tc.expPass {
Expand Down

0 comments on commit cc16ae4

Please sign in to comment.