Skip to content

Make channel-upgrade execute command fully idempotent #171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Make channel-upgrade execute command fully idempotent #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7a9496
Make channel-upgrade execute command fully idempotent
abicky Aug 1, 2025
f9b6353
Replace tab with spaces in shell script
abicky Aug 1, 2025
5dd0d0d
Handle case where current state can never reach target state
abicky Aug 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 45 additions & 5 deletions core/channel-upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -381,10 +386,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
}
Expand All @@ -394,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 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
Expand Down Expand Up @@ -790,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
}
}
46 changes: 34 additions & 12 deletions tests/cases/tm2tm/scripts/test-channel-upgrade
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -140,3 +140,25 @@ $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
# 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 finished with exit code 0, but non-zero code was expected' >&2
exit 1
fi
checkResult alt
Loading