From 247068bb02ceeb3e2117bebe7262e0d44a98de83 Mon Sep 17 00:00:00 2001 From: Dzung Do | Decentrio Date: Fri, 19 Apr 2024 16:45:36 +0700 Subject: [PATCH] test: add panic cases in tests for `UpgradeOpen` (#5550) ## Description - Add panic test cases for `OnChanUpgradeOpen` and `OnChanUpgradeRestore` - Reuse mock middleware as the app which doesn't implement the `UpgradeableModule` interface from [#5406](https://github.com/cosmos/ibc-go/pull/5406) closes: #5471 ### Commit Message / Changelog Entry ```text test: add panic cases in tests when channel upgrade open or restore ``` see the [guidelines](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) for commit messages. (view raw markdown for examples) --- Before we can merge this PR, please make sure that all the following items have been checked off. If any of the checklist items are not applicable, please leave them but write a little note why. - [x] Targeted PR against the correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#pull-request-targeting)). - [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work. - [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/build/building-modules/11-structure.md) and [Go style guide](../docs/dev/go-style-guide.md). - [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/main/testing/README.md#ibc-testing-package). - [ ] Updated relevant documentation (`docs/`) or specification (`x//spec/`). - [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code). - [x] Provide a [commit message](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) to be used for the changelog entry in the PR description for review. - [ ] Re-reviewed `Files changed` in the Github PR explorer. - [ ] Review `Codecov Report` in the comment section below once CI passes. ## Summary by CodeRabbit - **Tests** - Enhanced testing for channel upgrades in interchain accounts, including new scenarios for error handling. - Added import for `cosmossdk.io/errors`. - Added `expPanic` field to test cases in `TestOnChanUpgradeOpen` function. - Updated the logic in `TestOnChanUpgradeOpen` to handle expected errors and panics based on test cases. --- .../controller/ibc_middleware_test.go | 56 ++++++++++++++----- 1 file changed, 41 insertions(+), 15 deletions(-) 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 e0e6ef288f0..95a25ac65a9 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -6,6 +6,8 @@ import ( testifysuite "github.com/stretchr/testify/suite" + errorsmod "cosmossdk.io/errors" + sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" @@ -1005,20 +1007,21 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { testCases := []struct { name string malleate func() + expPanic error }{ { - "success w/ ORDERED channel", func() {}, + "success w/ ORDERED channel", func() {}, nil, }, { "success w/ UNORDERED channel", func() { channelOrder = channeltypes.UNORDERED }, + nil, }, { - "success: nil app", - func() { + "success: nil app", func() { isNilApp = true - }, + }, nil, }, { "middleware disabled", func() { @@ -1027,6 +1030,19 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { return ibcmock.MockApplicationCallbackError } }, + nil, + }, + { + "failure: upgrade route not found", + func() {}, + errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack"), + }, + { + "failure: connection not found", + func() { + path.EndpointA.ChannelID = "invalid-channel" + }, + errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", TestPortID, "invalid-channel"), }, } @@ -1055,20 +1071,30 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { cbs, ok := app.(porttypes.UpgradableModule) suite.Require().True(ok) - if isNilApp { - cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper) + upgradeOpenCb := func(cbs porttypes.UpgradableModule) { + cbs.OnChanUpgradeOpen( + suite.chainA.GetContext(), + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + channelOrder, + []string{path.EndpointA.ConnectionID}, + counterpartyVersion, + ) } - channelOrder = channeltypes.ORDERED + 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.OnChanUpgradeOpen( - suite.chainA.GetContext(), - path.EndpointA.ChannelConfig.PortID, - path.EndpointA.ChannelID, - channelOrder, - []string{path.EndpointA.ConnectionID}, - counterpartyVersion, - ) + suite.Require().PanicsWithError(tc.expPanic.Error(), func() { upgradeOpenCb(cbs) }) + } else { + if isNilApp { + cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper) + } + + upgradeOpenCb(cbs) + } }) } }