refactor: cleanup upgrade init app callback args (#4359)

* WIP: adding initial implementation of changes * proto format * commenting out more failing tests from timeouts * fix compiler error * commenting out failing testcases due to timeout logic * fix: reorder proto msg fields correctly * refactor: move increment upgrade sequence to write fn, rename currentChannel -> channel * refactor: rename msg server vars for consistency * update FirstChannelID to FirstConnectionID in msg validate basic tests * rename test var and use mock.UpgradeVersion * comment out failing tests * refactor upgrade init state to open. refactor crossing hellos and try verification logic * updating godoc and error return in chanUpgradeAck2 * refactor: rm unnecessary args in application upgrade init callback
cosmos · Aug 16, 2023 · f648383 · f648383
1 parent 7cd3ddf
commit f648383
Show file tree

Hide file tree

Showing 9 changed files with 12 additions and 23 deletions.
diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -230,7 +230,7 @@ func (im IBCMiddleware) OnTimeoutPacket(
 }
 
 // OnChanUpgradeInit implements the IBCModule interface
-func (IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, sequence uint64, version, previousVersion string) (string, error) {
+func (IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
 	return icatypes.Version, nil
 }
 

diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go
@@ -151,7 +151,7 @@ func (IBCModule) OnTimeoutPacket(
 }
 
 // OnChanUpgradeInit implements the IBCModule interface
-func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, sequence uint64, version, previousVersion string) (string, error) {
+func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
 	return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
 }
 

diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go
@@ -331,24 +331,22 @@ func (im IBCMiddleware) OnChanUpgradeInit(
 	channelID string,
 	order channeltypes.Order,
 	connectionHops []string,
-	upgradeSequence uint64,
 	upgradeVersion string,
-	previousVersion string,
 ) (string, error) {
 	versionMetadata, err := types.MetadataFromVersion(upgradeVersion)
 	if err != nil {
 		// fee version is unable to be parsed from upgrade version, disable fee
 		im.keeper.DeleteFeeEnabled(ctx, portID, channelID)
 		// since it is valid for fee version to not be specified, the upgrade version may be for a middleware
 		// or application further down in the stack. Thus, passthrough to next middleware or application in callstack.
-		return im.app.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, upgradeSequence, upgradeVersion, previousVersion)
+		return im.app.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, upgradeVersion)
 	}
 
 	if versionMetadata.FeeVersion != types.Version {
 		return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
 	}
 
-	appVersion, err := im.app.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, upgradeSequence, versionMetadata.AppVersion, previousVersion)
+	appVersion, err := im.app.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, versionMetadata.AppVersion)
 	if err != nil {
 		return "", err
 	}

diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go
@@ -1053,7 +1053,7 @@ func (suite *FeeTestSuite) TestOnChanUpgradeInit() {
 				path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibctesting.InvalidID
 				path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibctesting.InvalidID
 
-				suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanUpgradeInit = func(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ uint64, _, _ string) (string, error) {
+				suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanUpgradeInit = func(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ string) (string, error) {
 					// intentionally force the error here so we can assert that a passthrough occurs when fees should not be enabled for this channel
 					return "", ibcmock.MockApplicationCallbackError
 				}
@@ -1074,7 +1074,7 @@ func (suite *FeeTestSuite) TestOnChanUpgradeInit() {
 			"underlying app callback returns error",
 			func() {
 				expFeeEnabled = false
-				suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanUpgradeInit = func(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ uint64, _, _ string) (string, error) {
+				suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanUpgradeInit = func(_ sdk.Context, _, _ string, _ channeltypes.Order, _ []string, _ string) (string, error) {
 					return "", ibcmock.MockApplicationCallbackError
 				}
 			},

diff --git a/modules/apps/transfer/ibc_module.go b/modules/apps/transfer/ibc_module.go
@@ -308,7 +308,7 @@ func (im IBCModule) OnTimeoutPacket(
 }
 
 // OnChanUpgradeInit implements the IBCModule interface
-func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, upgradeSequence uint64, upgradeVersion, previousVersion string) (string, error) {
+func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, upgradeVersion string) (string, error) {
 	if err := ValidateTransferChannelParams(ctx, im.keeper, order, portID, channelID); err != nil {
 		return "", err
 	}

diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go
@@ -115,8 +115,7 @@ type UpgradableModule interface {
 		portID, channelID string,
 		order channeltypes.Order,
 		connectionHops []string,
-		sequence uint64,
-		version, previousVersion string,
+		version string,
 	) (string, error)
 
 	// OnChanUpgradeTry verifies the counterparty upgrade and sets the upgrade on TRY chain

diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go
@@ -719,28 +719,21 @@ func (k Keeper) ChannelUpgradeInit(goCtx context.Context, msg *channeltypes.MsgC
 		return nil, errorsmod.Wrap(err, "channel upgrade init failed")
 	}
 
-	channel, found := k.ChannelKeeper.GetChannel(ctx, msg.PortId, msg.ChannelId)
-	if !found {
-		return nil, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", msg.PortId, msg.ChannelId)
-	}
-
 	upgradeVersion, err := cbs.OnChanUpgradeInit(
 		ctx,
 		msg.PortId,
 		msg.ChannelId,
 		upgrade.Fields.Ordering,
 		upgrade.Fields.ConnectionHops,
-		channel.UpgradeSequence,
 		upgrade.Fields.Version,
-		channel.Version,
 	)
 	if err != nil {
 		ctx.Logger().Error("channel upgrade init callback failed", "port-id", msg.PortId, "channel-id", msg.ChannelId, "error", err.Error())
 		return nil, errorsmod.Wrapf(err, "channel upgrade init callback failed for port ID: %s, channel ID: %s", msg.PortId, msg.ChannelId)
 	}
 
 	upgrade.Fields.Version = upgradeVersion
-	k.ChannelKeeper.WriteUpgradeInitChannel(ctx, msg.PortId, msg.ChannelId, upgrade)
+	channel := k.ChannelKeeper.WriteUpgradeInitChannel(ctx, msg.PortId, msg.ChannelId, upgrade)
 
 	ctx.Logger().Info("channel upgrade init succeeded", "channel-id", msg.ChannelId, "version", upgradeVersion)
 

diff --git a/testing/mock/ibc_app.go b/testing/mock/ibc_app.go
@@ -91,8 +91,7 @@ type IBCApp struct {
 		portID, channelID string,
 		order channeltypes.Order,
 		connectionHops []string,
-		sequence uint64,
-		version, previousVersion string,
+		version string,
 	) (string, error)
 
 	OnChanUpgradeTry func(

diff --git a/testing/mock/ibc_module.go b/testing/mock/ibc_module.go
@@ -177,9 +177,9 @@ func (im IBCModule) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet,
 }
 
 // OnChanUpgradeInit implements the IBCModule interface
-func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, sequence uint64, version, previousVersion string) (string, error) {
+func (im IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
 	if im.IBCApp.OnChanUpgradeInit != nil {
-		return im.IBCApp.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, sequence, version, previousVersion)
+		return im.IBCApp.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, version)
 	}
 
 	return version, nil