diff --git a/modules/core/04-channel/keeper/timeout.go b/modules/core/04-channel/keeper/timeout.go index 863af2499db..b02820e2074 100644 --- a/modules/core/04-channel/keeper/timeout.go +++ b/modules/core/04-channel/keeper/timeout.go @@ -2,6 +2,7 @@ package keeper import ( "bytes" + "fmt" "strconv" errorsmod "cosmossdk.io/errors" @@ -150,15 +151,8 @@ func (k Keeper) TimeoutExecuted( k.deletePacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) - if channel.Ordering == types.ORDERED { - channel.State = types.CLOSED - k.SetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), channel) - emitChannelClosedEvent(ctx, packet, channel) - } - - // TODO: handle situation outlined in https://github.com/cosmos/ibc-go/issues/4454 // if an upgrade is in progress, handling packet flushing and update channel state appropriately - if channel.State == types.FLUSHING { + 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()) @@ -171,17 +165,31 @@ func (k Keeper) TimeoutExecuted( // 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. k.MustAbortUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), err) - return nil - } - - // set the channel state to flush complete if all packets have been flushed. - if !k.HasInflightPackets(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) { + } else if !k.HasInflightPackets(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) { + // set the channel state to flush complete if all packets have been flushed. channel.State = types.FLUSHCOMPLETE k.SetChannel(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. + } else if counterpartyUpgrade.Fields.Version != "" { + k.MustAbortUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), fmt.Errorf("uh oh")) } } + if channel.Ordering == types.ORDERED { + // NOTE: if the channel is ORDERED and a packet is timed out in FLUSHING state then + // the upgrade is aborted and the channel is set to CLOSED. + if channel.State == types.FLUSHING { + // an error receipt is written to state and the channel is restored to OPEN + k.MustAbortUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), types.ErrPacketTimeout) + } + + channel.State = types.CLOSED + k.SetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), channel) + emitChannelClosedEvent(ctx, packet, channel) + } + k.Logger(ctx).Info( "packet timed-out", "sequence", strconv.FormatUint(packet.GetSequence(), 10), diff --git a/modules/core/04-channel/keeper/timeout_test.go b/modules/core/04-channel/keeper/timeout_test.go index 996bf4ec38f..1565bc5adf3 100644 --- a/modules/core/04-channel/keeper/timeout_test.go +++ b/modules/core/04-channel/keeper/timeout_test.go @@ -444,6 +444,51 @@ func (suite *KeeperTestSuite) TestTimeoutExecuted() { suite.Require().False(found, "error receipt should not be written") }, }, + { + "ordered channel is closed and upgrade is aborted when timeout is executed", + func() { + path.SetChannelOrdered() + suite.coordinator.Setup(path) + + timeoutHeight := clienttypes.GetSelfHeight(suite.chainB.GetContext()) + timeoutTimestamp := uint64(suite.chainB.GetContext().BlockTime().UnixNano()) + + sequence, err := path.EndpointA.SendPacket(timeoutHeight, timeoutTimestamp, ibctesting.MockPacketData) + suite.Require().NoError(err) + + packet = types.NewPacket(ibctesting.MockPacketData, sequence, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, timeoutTimestamp) + chanCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + + channel := path.EndpointA.GetChannel() + channel.State = types.FLUSHING + path.EndpointA.SetChannel(channel) + path.EndpointA.SetChannelUpgrade(types.Upgrade{ + Fields: path.EndpointA.GetProposedUpgrade().Fields, + Timeout: types.NewTimeout(clienttypes.ZeroHeight(), 1), + }) + path.EndpointA.SetChannelCounterpartyUpgrade(types.Upgrade{ + Timeout: types.NewTimeout(clienttypes.ZeroHeight(), 1), + }) + }, + func(packetCommitment []byte, err error) { + suite.Require().NoError(err) + + channel := path.EndpointA.GetChannel() + suite.Require().Equal(types.CLOSED, channel.State, "channel state should be CLOSED") + + upgrade, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().False(found, "upgrade should not be present") + suite.Require().Equal(types.Upgrade{}, upgrade, "upgrade should be zero value") + + upgrade, found = suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().False(found, "counterparty upgrade should not be present") + suite.Require().Equal(types.Upgrade{}, upgrade, "counterparty upgrade should be zero value") + + errorReceipt, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.Require().True(found, "error receipt should be present") + suite.Require().Equal(channel.UpgradeSequence, errorReceipt.Sequence, "error receipt sequence should be equal to channel upgrade sequence") + }, + }, } for i, tc := range testCases {