Remove setting counterparty upgrade info in upgrade try (#5238)

* chore: updated tests to reflect the timeout and ack changes

* chore: adding status check on channel when channel is flushing

* review comment

---------

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

modules/core/04-channel/keeper/packet.go

@@ -475,14 +475,10 @@ func (k Keeper) AcknowledgePacket(
 
 	// if an upgrade is in progress, handling packet flushing and update channel state appropriately
 	if channel.State == types.FLUSHING {
+		// counterparty upgrade is written in the OnChanUpgradeAck step.
 		counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel())
-		if !found {
-			return errorsmod.Wrapf(types.ErrUpgradeNotFound, "counterparty upgrade not found for channel: %s", packet.GetSourceChannel())
-		}
-
-		timeout := counterpartyUpgrade.Timeout
-		// if the timeout is valid then use it, otherwise it has not been set in the upgrade handshake yet.
-		if timeout.IsValid() {
+		if found {
+			timeout := counterpartyUpgrade.Timeout
 			if hasPassed, err := timeout.HasPassed(ctx); hasPassed {
 				// packet flushing timeout has expired, abort the upgrade and return nil,
 				// committing an error receipt to state, restoring the channel and successfully acknowledging the packet.

modules/core/04-channel/keeper/packet_test.go

@@ -822,14 +822,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 
 				channel := path.EndpointA.GetChannel()
 				channel.State = types.FLUSHING
-
 				path.EndpointA.SetChannel(channel)
-
-				counterpartyUpgrade := types.Upgrade{
-					Timeout: types.NewTimeout(clienttypes.ZeroHeight(), 0),
-				}
-
-				path.EndpointA.SetChannelCounterpartyUpgrade(counterpartyUpgrade)
 			},
 			expResult: func(commitment []byte, err error) {
 				suite.Require().NoError(err)

modules/core/04-channel/keeper/timeout.go

@@ -153,13 +153,11 @@ func (k Keeper) TimeoutExecuted(
 	// if an upgrade is in progress, handling packet flushing and update channel state appropriately
 	if channel.State == types.FLUSHING && channel.Ordering == types.UNORDERED {
 		counterpartyUpgrade, found := k.GetCounterpartyUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel())
-		if !found {
-			return errorsmod.Wrapf(types.ErrUpgradeNotFound, "counterparty upgrade not found for channel: %s", packet.GetSourceChannel())
-		}
-
-		timeout := counterpartyUpgrade.Timeout
-		// if the timeout is valid then use it, otherwise it has not been set in the upgrade handshake yet.
-		if timeout.IsValid() {
+		// once we have received the counterparty timeout in the channel UpgradeAck or UpgradeConfirm handshake steps
+		// then we can move to flushing complete if the timeout has not passed and there are no in-flight packets
+		if found {
+			timeout := counterpartyUpgrade.Timeout
+			// if the timeout is valid then use it, otherwise it has not been set in the upgrade handshake yet.
 			if hasPassed, err := timeout.HasPassed(ctx); hasPassed {
 				// packet flushing timeout has expired, abort the upgrade and return nil,
 				// committing an error receipt to state, restoring the channel and successfully timing out the packet.
@@ -170,8 +168,6 @@ func (k Keeper) TimeoutExecuted(
 				k.SetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), channel)
 				emitChannelFlushCompleteEvent(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), channel)
 			}
-			// upgrade fields have been set but the timeout has not. This can happen when the counterparty
-			// upgrade is partially written in WriteUpgradeTryChannel.
 		}
 	}
 

modules/core/04-channel/keeper/timeout_test.go

@@ -365,7 +365,6 @@ func (suite *KeeperTestSuite) TestTimeoutExecuted() {
 				channel := path.EndpointA.GetChannel()
 				channel.State = types.FLUSHING
 				path.EndpointA.SetChannel(channel)
-				path.EndpointA.SetChannelCounterpartyUpgrade(types.Upgrade{})
 			},
 			func(packetCommitment []byte, err error) {
 				suite.Require().NoError(err)

modules/core/04-channel/keeper/upgrade.go

@@ -188,7 +188,7 @@ func (k Keeper) ChanUpgradeTry(
 
 // WriteUpgradeTryChannel writes the channel end and upgrade to state after successfully passing the UpgradeTry handshake step.
 // An event is emitted for the handshake step.
-func (k Keeper) WriteUpgradeTryChannel(ctx sdk.Context, portID, channelID string, upgrade types.Upgrade, upgradeVersion string, counterpartyUpgradeFields types.UpgradeFields) (types.Channel, types.Upgrade) {
+func (k Keeper) WriteUpgradeTryChannel(ctx sdk.Context, portID, channelID string, upgrade types.Upgrade, upgradeVersion string) (types.Channel, types.Upgrade) {
 	defer telemetry.IncrCounter(1, "ibc", "channel", "upgrade-try")
 
 	channel, found := k.GetChannel(ctx, portID, channelID)
@@ -199,9 +199,6 @@ func (k Keeper) WriteUpgradeTryChannel(ctx sdk.Context, portID, channelID string
 	upgrade.Fields.Version = upgradeVersion
 	k.SetUpgrade(ctx, portID, channelID, upgrade)
 
-	counterpartyUpgrade := types.Upgrade{Fields: counterpartyUpgradeFields}
-	k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade)
-
 	k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", types.OPEN, "new-state", channel.State)
 	emitChannelUpgradeTryEvent(ctx, portID, channelID, channel, upgrade)
 
@@ -328,6 +325,8 @@ func (k Keeper) WriteUpgradeAckChannel(ctx sdk.Context, portID, channelID string
 		channel.State = types.FLUSHCOMPLETE
 		k.SetChannel(ctx, portID, channelID, channel)
 	} else {
+		// the counterparty upgrade is only required if the channel is still in the FLUSHING state.
+		// this gets read when timing out and acknowledging packets.
 		k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade)
 	}
 
@@ -437,6 +436,8 @@ func (k Keeper) WriteUpgradeConfirmChannel(ctx sdk.Context, portID, channelID st
 
 		k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", channel.State)
 	} else {
+		// the counterparty upgrade is only required if the channel is still in the FLUSHING state.
+		// this gets read when timing out and acknowledging packets.
 		k.SetCounterpartyUpgrade(ctx, portID, channelID, counterpartyUpgrade)
 	}
 

modules/core/04-channel/keeper/upgrade_test.go

@@ -355,7 +355,6 @@ func (suite *KeeperTestSuite) TestWriteUpgradeTry() {
 				path.EndpointB.ChannelID,
 				proposedUpgrade,
 				proposedUpgrade.Fields.Version,
-				path.EndpointA.GetProposedUpgrade().Fields,
 			)
 
 			channel := path.EndpointB.GetChannel()
@@ -934,12 +933,12 @@ func (suite *KeeperTestSuite) TestWriteUpgradeConfirm() {
 			if !tc.hasPacketCommitments {
 				suite.Require().Equal(types.FLUSHCOMPLETE, channel.State)
 				// Counterparty was set in UPGRADETRY but without timeout, latest sequence send set.
-				counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
-				suite.Require().True(ok)
-				suite.Require().NotEqual(proposedUpgrade, counterpartyUpgrade)
+				_, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
+				suite.Require().False(ok, "counterparty upgrade should not be present when there are no in flight packets")
 			} else {
+				suite.Require().Equal(types.FLUSHING, channel.State)
 				counterpartyUpgrade, ok := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
-				suite.Require().True(ok)
+				suite.Require().True(ok, "counterparty upgrade should be present when there are in flight packets")
 				suite.Require().Equal(proposedUpgrade, counterpartyUpgrade)
 			}
 		})

modules/core/keeper/msg_server.go

@@ -821,7 +821,7 @@ func (k Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgCh
 		return nil, err
 	}
 
-	channel, upgrade := k.ChannelKeeper.WriteUpgradeTryChannel(ctx, msg.PortId, msg.ChannelId, upgrade, upgradeVersion, msg.CounterpartyUpgradeFields)
+	channel, upgrade := k.ChannelKeeper.WriteUpgradeTryChannel(ctx, msg.PortId, msg.ChannelId, upgrade, upgradeVersion)
 
 	ctx.Logger().Info("channel upgrade try succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId)