Handle case where current state can never reach target state

According to the IBC spec
https://github.com/cosmos/ibc/blob/main/spec/core/ics-004-channel-and-packet-semantics/UPGRADES.md#upgrade-handshake,
some states may be skipped during the channel upgrade handshake.
For example, the stat pair (INIT, FLUSHING) may transition
directly to (FLUSHCOMPLETE, FLUSHING) without passing through
FLUSHING from INIT.

Signed-off-by: Takeshi Arabiki <takeshi.arabiki@datachain.jp>

Author: abicky

core/channel-upgrade.go

@@ -144,6 +144,11 @@ func ExecuteChannelUpgrade(ctx context.Context, pathName string, src, dst *Prova
 	logger := GetChannelPairLogger(src, dst)
 	defer logger.TimeTrackContext(ctx, time.Now(), "ExecuteChannelUpgrade")
 
+	if (targetSrcState == UPGRADE_STATE_UNINIT && targetDstState == UPGRADE_STATE_INIT) ||
+		(targetSrcState == UPGRADE_STATE_INIT && targetDstState == UPGRADE_STATE_UNINIT) {
+		return fmt.Errorf("unreachable target state pair: (%s, %s)", targetSrcState, targetDstState)
+	}
+
 	failures := 0
 	firstCall := true
 	err := runUntilComplete(ctx, interval, func() (bool, error) {
@@ -381,12 +386,15 @@ func upgradeChannelStep(ctx context.Context, src, dst *ProvableChain, targetSrcS
 		),
 	)}
 
-	// check if both chains have reached the target states or UNINIT states
-	if srcState == targetSrcState && dstState == targetDstState {
-		if firstCall && srcState == UPGRADE_STATE_UNINIT && dstState == UPGRADE_STATE_UNINIT {
-			logger.InfoContext(ctx, "both chains have already reached the target states, or the channel upgrade has not been initialized")
+	if hasReachedOrPassedTargetState(srcState, targetSrcState, dstState) && hasReachedOrPassedTargetState(dstState, targetDstState, srcState) {
+		if firstCall {
+			if srcState == UPGRADE_STATE_UNINIT && targetSrcState == UPGRADE_STATE_UNINIT {
+				logger.InfoContext(ctx, "both chains have already reached or passed the target states, or the channel upgrade has not been initialized")
+			} else {
+				logger.InfoContext(ctx, "both chains have already reached or passed the target states")
+			}
 		} else {
-			logger.InfoContext(ctx, "both chains have reached the target states")
+			logger.InfoContext(ctx, "both chains have reached or passed the target states")
 		}
 		out.Last = true
 		return out, nil
@@ -397,7 +405,8 @@ func upgradeChannelStep(ctx context.Context, src, dst *ProvableChain, targetSrcS
 	dstAction := UPGRADE_ACTION_NONE
 	switch {
 	case srcState == UPGRADE_STATE_UNINIT && dstState == UPGRADE_STATE_UNINIT:
-		return nil, errors.New("channel upgrade has not been initialized; it will never reach the target states")
+		// This line should never be reached because the channel upgrade is considered completed
+		return nil, errors.New("unexpected transition")
 	case srcState == UPGRADE_STATE_INIT && dstState == UPGRADE_STATE_UNINIT:
 		if dstChan.Channel.UpgradeSequence >= srcChan.Channel.UpgradeSequence {
 			srcAction = UPGRADE_ACTION_CANCEL
@@ -793,3 +802,31 @@ func buildActionMsg(
 		panic(fmt.Errorf("unexpected action: %s", action))
 	}
 }
+
+// hasReachedOrPassedTargetState checks if the current state has reached or passed the target state,
+// including cases where the target state is skipped.
+func hasReachedOrPassedTargetState(currentState, targetState, counterpartyCurrentState UpgradeState) bool {
+	return currentState == targetState || hasPassedTargetState(currentState, targetState, counterpartyCurrentState)
+}
+
+// hasPassedTargetState checks if the current state has passed the target state,
+// including cases where the target state is skipped.
+func hasPassedTargetState(currentState, targetState, counterpartyCurrentState UpgradeState) bool {
+	// Check if the current state has passed the target state.
+	// For simplicity, we consider that any state that can be reached after the target state has passed the target state.
+	// NOTE: Each chain can cancel the upgrade and initialize another upgrade at any time. This means that the state
+	// can transition to UNINIT from any state except the case where the counterparty is in INIT state.
+
+	isUninitReachable := counterpartyCurrentState != UPGRADE_STATE_INIT
+	switch targetState {
+	case UPGRADE_STATE_INIT:
+		return currentState == UPGRADE_STATE_FLUSHING || currentState == UPGRADE_STATE_FLUSHCOMPLETE ||
+			(isUninitReachable && currentState == UPGRADE_STATE_UNINIT)
+	case UPGRADE_STATE_FLUSHING:
+		return currentState == UPGRADE_STATE_FLUSHCOMPLETE || (isUninitReachable && currentState == UPGRADE_STATE_UNINIT)
+	case UPGRADE_STATE_FLUSHCOMPLETE:
+		return isUninitReachable && currentState == UPGRADE_STATE_UNINIT
+	default:
+		return false
+	}
+}

tests/cases/tm2tm/scripts/test-channel-upgrade

@@ -147,10 +147,18 @@ $RLY tx channel-upgrade execute ibc01
 checkResult orig
 
 echo '##### case 11 #####'
-# The target states will never be reached
-if $RLY tx channel-upgrade execute ibc01 --target-src-state INIT --target-dst-state FLUSHING
+$RLY tx channel-upgrade init ibc01 ibc0 $altSrcOpts
+$RLY tx channel-upgrade execute ibc01 --target-src-state FLUSHING --target-dst-state FLUSHING
+# Both chains have already passed the target states
+$RLY tx channel-upgrade execute ibc01 --target-src-state INIT --target-dst-state INIT
+$RLY tx channel-upgrade execute ibc01
+checkResult alt
+
+echo '##### case 12 #####'
+# Unreachable target pair (INIT, UNINIT)
+if $RLY tx channel-upgrade execute ibc01 --target-src-state INIT
 then
-    echo 'channel-upgrade completed with exit code 0, but non-zero code was expected' >&2
+    echo 'channel-upgrade finished with exit code 0, but non-zero code was expected' >&2
     exit 1
 fi
-checkResult orig
+checkResult alt