diff --git a/core/channel-upgrade.go b/core/channel-upgrade.go index 18127c79..d46d3be5 100644 --- a/core/channel-upgrade.go +++ b/core/channel-upgrade.go @@ -144,6 +144,17 @@ func ExecuteChannelUpgrade(ctx context.Context, pathName string, src, dst *Prova logger := GetChannelPairLogger(src, dst) defer logger.TimeTrackContext(ctx, time.Now(), "ExecuteChannelUpgrade") + // Assume the current state pair is (INIT, UNINIT). + // The target state pair (INIT, UNINIT) and (UNINIT, INIT) are invalid: + // - (INIT, UNINIT) is meaningless as a target since this function won't cause any state change + // - (UNINIT, INIT) is unreachable + // Therefore, it is unlikely that a user would intentionally set either pair after + // initializing an upgrade on only one side, so return an error. + 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,10 +392,16 @@ func upgradeChannelStep(ctx context.Context, src, dst *ProvableChain, targetSrcS ), )} - // check if both chains have reached the target states or UNINIT states - if !firstCall && srcState == UPGRADE_STATE_UNINIT && dstState == UPGRADE_STATE_UNINIT || - srcState != UPGRADE_STATE_UNINIT && dstState != UPGRADE_STATE_UNINIT && srcState == targetSrcState && dstState == targetDstState { - logger.InfoContext(ctx, "both chains have reached the target states") + 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 or passed the target states") + } out.Last = true return out, nil } @@ -394,7 +411,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 is not initialized") + // 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 @@ -790,3 +808,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 { + // 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 + + // 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. + 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 + } +} diff --git a/tests/cases/tm2tm/scripts/test-channel-upgrade b/tests/cases/tm2tm/scripts/test-channel-upgrade index a1e0cd1a..b7cb56cf 100755 --- a/tests/cases/tm2tm/scripts/test-channel-upgrade +++ b/tests/cases/tm2tm/scripts/test-channel-upgrade @@ -53,21 +53,21 @@ checkResult() { if [ "$expectedSide" = orig ] then - if [ "$srcConnectionId" != "$srcOrigConnectionId" -o "$dstConnectionId" != "$dstOrigConnectionId" -o "$srcVersion" != "$srcOrigVersion" -o "$dstVersion" != "$dstOrigVersion" -o "$srcOrder" != "$srcOrigOrder" -o "$dstOrder" != "$dstOrigOrder" ] - then - echo "path config is not equal to the original one: $srcConnectionId, $dstConnectionId, $srcVersion, $dstVersion, $srcOrder, $dstOrder" - exit 1 - fi + if [ "$srcConnectionId" != "$srcOrigConnectionId" -o "$dstConnectionId" != "$dstOrigConnectionId" -o "$srcVersion" != "$srcOrigVersion" -o "$dstVersion" != "$dstOrigVersion" -o "$srcOrder" != "$srcOrigOrder" -o "$dstOrder" != "$dstOrigOrder" ] + then + echo "path config is not equal to the original one: $srcConnectionId, $dstConnectionId, $srcVersion, $dstVersion, $srcOrder, $dstOrder" + exit 1 + fi elif [ "$expectedSide" = alt ] then - if [ "$srcConnectionId" != "$srcAltConnectionId" -o "$dstConnectionId" != "$dstAltConnectionId" -o "$srcVersion" != "$srcAltVersion" -o "$dstVersion" != "$dstAltVersion" -o "$srcOrder" != "$srcAltOrder" -o "$dstOrder" != "$dstAltOrder" ] - then - echo "path config is not equal to the alternative one: $srcConnectionId, $dstConnectionId, $srcVersion, $dstVersion, $srcOrder, $dstOrder" - exit 1 - fi + if [ "$srcConnectionId" != "$srcAltConnectionId" -o "$dstConnectionId" != "$dstAltConnectionId" -o "$srcVersion" != "$srcAltVersion" -o "$dstVersion" != "$dstAltVersion" -o "$srcOrder" != "$srcAltOrder" -o "$dstOrder" != "$dstAltOrder" ] + then + echo "path config is not equal to the alternative one: $srcConnectionId, $dstConnectionId, $srcVersion, $dstVersion, $srcOrder, $dstOrder" + exit 1 + fi else - echo "expectedSide is invalid value: $expectedSide" - exit 1 + echo "expectedSide is invalid value: $expectedSide" + exit 1 fi } @@ -140,3 +140,29 @@ $RLY tx channel-upgrade execute ibc01 --target-src-state FLUSHING --target-dst-s sleep 20 # Both chains exceed upgrade.timeout.timestamp $RLY tx channel-upgrade execute ibc01 # ibc0,ibc1 <= chanUpgradeTimeout checkResult orig + +echo '##### case 10 #####' +# Both chains have already reached the target states, or the channel upgrade has not been initialized +$RLY tx channel-upgrade execute ibc01 +checkResult orig + +echo '##### case 11 #####' +$RLY tx channel-upgrade init ibc01 ibc0 $altSrcOpts +$RLY tx channel-upgrade execute ibc01 --target-src-state FLUSHING --target-dst-state FLUSHING +# No action is taken will happen because both chains have already passed the target states (INIT) +$RLY tx channel-upgrade execute ibc01 --target-src-state INIT --target-dst-state INIT +checkResult orig +$RLY tx channel-upgrade execute ibc01 +checkResult alt + +echo '##### case 12 #####' +$RLY tx channel-upgrade init ibc01 ibc0 $origSrcOpts # (UNINIT, INIT) +# Unreachable target pair (INIT, UNINIT) +if $RLY tx channel-upgrade execute ibc01 --target-src-state INIT +then + echo 'channel-upgrade finished with exit code 0, but non-zero code was expected' >&2 + exit 1 +fi +checkResult alt +$RLY tx channel-upgrade execute ibc01 +checkResult orig