remaining review comments from #1613 (backport #5539) (#5617)

* remaining review comments from #1613 (#5539)

* e2e test: send incentivised packet after upgrade, add extra tests for cbs

* update hermes docker image

* add prune acknowledgements to successful upgrade test

* use correct tx response

* getting further with the e2e test, addressing a couple of other review items

* refactor test to use sync incentivization instead of async

* update hermes image tag

* revert change that was breaking a test

* Apply suggestions from code review

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

* rename variables for consistency

* rename variables for clarification

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
(cherry picked from commit 5d77221)

# Conflicts:
#	e2e/README.md
#	e2e/tests/core/04-channel/upgrades_test.go
#	e2e/tests/transfer/incentivized_test.go
#	e2e/testsuite/grpc_query.go
#	e2e/testsuite/tx.go

* remove e2e folder

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -863,6 +863,33 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 	}
 }
 
+// OnChanUpgradeTry callback returns error on controller chains
+func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {
+	suite.SetupTest() // reset
+	path := NewICAPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupConnections(path)
+
+	err := SetupICAPath(path, TestOwnerAddress)
+	suite.Require().NoError(err)
+
+	// call application callback directly
+	module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
+	suite.Require().NoError(err)
+
+	app, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
+	suite.Require().True(ok)
+	cbs, ok := app.(porttypes.UpgradableModule)
+	suite.Require().True(ok)
+
+	version, err := cbs.OnChanUpgradeTry(
+		suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID,
+		path.EndpointA.ChannelConfig.Order, []string{path.EndpointA.ConnectionID}, path.EndpointB.ChannelConfig.Version,
+	)
+	suite.Require().Error(err)
+	suite.Require().ErrorIs(err, icatypes.ErrInvalidChannelFlow)
+	suite.Require().Equal("", version)
+}
+
 func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 	var (
 		path                *ibctesting.Path

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -604,6 +604,33 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
 	}
 }
 
+// OnChanUpgradeInit callback returns error on host chains
+func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
+	path := NewICAPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupConnections(path)
+
+	err := SetupICAPath(path, TestOwnerAddress)
+	suite.Require().NoError(err)
+
+	// call application callback directly
+	module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
+	suite.Require().NoError(err)
+
+	app, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module)
+	suite.Require().True(ok)
+	cbs, ok := app.(porttypes.UpgradableModule)
+	suite.Require().True(ok)
+
+	version, err := cbs.OnChanUpgradeInit(
+		suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID,
+		path.EndpointB.ChannelConfig.Order, []string{path.EndpointB.ConnectionID}, path.EndpointB.ChannelConfig.Version,
+	)
+
+	suite.Require().Error(err)
+	suite.Require().ErrorIs(err, icatypes.ErrInvalidChannelFlow)
+	suite.Require().Equal("", version)
+}
+
 func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {
 	testCases := []struct {
 		name     string
@@ -672,6 +699,31 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {
 	}
 }
 
+// OnChanUpgradeAck callback returns error on host chains
+func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
+	path := NewICAPath(suite.chainA, suite.chainB)
+	suite.coordinator.SetupConnections(path)
+
+	err := SetupICAPath(path, TestOwnerAddress)
+	suite.Require().NoError(err)
+
+	// call application callback directly
+	module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
+	suite.Require().NoError(err)
+
+	app, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module)
+	suite.Require().True(ok)
+	cbs, ok := app.(porttypes.UpgradableModule)
+	suite.Require().True(ok)
+
+	err = cbs.OnChanUpgradeAck(
+		suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.Version,
+	)
+
+	suite.Require().Error(err)
+	suite.Require().ErrorIs(err, icatypes.ErrInvalidChannelFlow)
+}
+
 func (suite *InterchainAccountsTestSuite) fundICAWallet(ctx sdk.Context, portID string, amount sdk.Coins) {
 	interchainAccountAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(ctx, ibctesting.FirstConnectionID, portID)
 	suite.Require().True(found)

modules/core/04-channel/types/msgs.go

@@ -832,7 +832,9 @@ func NewMsgChannelUpgradeCancel(
 	}
 }
 
-// ValidateBasic implements sdk.Msg
+// ValidateBasic implements sdk.Msg. No checks are done for ErrorReceipt and ProofErrorReceipt
+// since they are not required if the current channel state is not in FLUSHCOMPLETE and the signer
+// is the designated authority (e.g. the governance module).
 func (msg MsgChannelUpgradeCancel) ValidateBasic() error {
 	if err := host.PortIdentifierValidator(msg.PortId); err != nil {
 		return errorsmod.Wrap(err, "invalid port ID")