diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index fc2089a2a89..0183adc6416 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -862,11 +862,16 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + // Send a packet on B to disallow channel automatically moving to OPEN on UpgradeAck + sequence, err := path.EndpointB.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().Equal(uint64(1), sequence) + suite.Require().NoError(err) + // Move channel to correct state. path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion - err := path.EndpointA.ChanUpgradeInit() + err = path.EndpointA.ChanUpgradeInit() suite.Require().NoError(err) err = path.EndpointB.ChanUpgradeTry() diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index 4e82d8e06f3..2fdfc4b2476 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -775,11 +775,6 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpenCounterpartyStates() { err = path.EndpointB.ChanUpgradeAck() suite.Require().NoError(err) - // TODO: Remove when #4030 is closed. Channel will automatically - // move to OPEN in that case. - err = path.EndpointB.ChanUpgradeOpen() - suite.Require().NoError(err) - suite.coordinator.CommitBlock(suite.chainA, suite.chainB) suite.Require().NoError(path.EndpointA.UpdateClient()) }, @@ -788,7 +783,14 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpenCounterpartyStates() { { "success, counterparty in TRYUPGRADE", func() { - err := path.EndpointA.ChanUpgradeInit() + // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, 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, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointB.RecvPacket(packet) + suite.Require().NoError(err) + + err = path.EndpointA.ChanUpgradeInit() suite.Require().NoError(err) err = path.EndpointB.ChanUpgradeTry() @@ -796,6 +798,10 @@ func (suite *KeeperTestSuite) TestChanUpgradeOpenCounterpartyStates() { err = path.EndpointA.ChanUpgradeAck() suite.Require().NoError(err) + + // Ack packet to delete packet commitment before calling ChanUpgradeOpen + err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) + suite.Require().NoError(err) }, nil, }, @@ -846,6 +852,13 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { path := ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.Setup(path) + // Need to create a packet commitment on A so as to keep it from going to OPEN if no inflight packets exist. + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, 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, defaultTimeoutHeight, disabledTimeoutTimestamp) + err = path.EndpointB.RecvPacket(packet) + suite.Require().NoError(err) + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Ordering = types.ORDERED @@ -855,6 +868,10 @@ func (suite *KeeperTestSuite) TestWriteUpgradeOpenChannel() { suite.Require().NoError(path.EndpointB.ChanUpgradeTry()) suite.Require().NoError(path.EndpointA.ChanUpgradeAck()) + // Ack packet to delete packet commitment before calling WriteUpgradeOpenChannel + err = path.EndpointA.AcknowledgePacket(packet, ibctesting.MockAcknowledgement) + suite.Require().NoError(err) + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeOpenChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) channel := path.EndpointA.GetChannel() diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index c65ceabb4b7..ef1d6b3b43c 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -852,6 +852,22 @@ func (k Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgCh ctx.Logger().Info("channel upgrade ack succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId) + // Move channel to OPEN state if both chains have finished flushing any in-flight packets. Counterparty flush status + // has been verified in ChanUpgradeAck. + if msg.CounterpartyFlushStatus == channeltypes.FLUSHCOMPLETE { + channel, found := k.ChannelKeeper.GetChannel(ctx, msg.PortId, msg.ChannelId) + if !found { + return nil, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", msg.PortId, msg.ChannelId) + } + if channel.FlushStatus == channeltypes.FLUSHCOMPLETE { + cbs.OnChanUpgradeOpen(ctx, msg.PortId, msg.ChannelId) + + k.ChannelKeeper.WriteUpgradeOpenChannel(ctx, msg.PortId, msg.ChannelId) + + ctx.Logger().Info("channel upgrade open succeeded", "port-id", msg.PortId, "channel-id", msg.ChannelId) + } + } + return &channeltypes.MsgChannelUpgradeAckResponse{Result: channeltypes.SUCCESS}, nil } diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index ba62d025cfe..6a34e09a6be 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/modules/core/keeper/msg_server_test.go @@ -937,17 +937,36 @@ func (suite *KeeperTestSuite) TestChannelUpgradeAck() { expResult func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) }{ { - "success", + "success, no pending in-flight packets", func() {}, func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().Equal(channeltypes.SUCCESS, res.Result) + channel := path.EndpointA.GetChannel() + suite.Require().Equal(channeltypes.OPEN, channel.State) + suite.Require().Equal(uint64(1), channel.UpgradeSequence) + suite.Require().Equal(channeltypes.NOTINFLUSH, channel.FlushStatus) + }, + }, + { + "success, pending in-flight packets", + func() { + portID := path.EndpointA.ChannelConfig.PortID + channelID := path.EndpointA.ChannelID + // Set a dummy packet commitment to simulate in-flight packets + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), portID, channelID, 1, []byte("hash")) + }, + func(res *channeltypes.MsgChannelUpgradeAckResponse, err error) { + suite.Require().NoError(err) + suite.Require().NotNil(res) + suite.Require().Equal(channeltypes.SUCCESS, res.Result) + channel := path.EndpointA.GetChannel() suite.Require().Equal(channeltypes.ACKUPGRADE, channel.State) suite.Require().Equal(uint64(1), channel.UpgradeSequence) - suite.Require().Equal(channeltypes.FLUSHCOMPLETE, channel.FlushStatus) + suite.Require().Equal(channeltypes.FLUSHING, channel.FlushStatus) }, }, {