From 39070be7b3ce38b12e4323892ac3259ebde83934 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Thu, 22 Jun 2023 05:09:50 +0300 Subject: [PATCH 1/8] Amend Ack packet to keep acknowledging if we're in the process of flushing pre-upgrade packets. --- modules/core/04-channel/keeper/packet.go | 7 +++- modules/core/04-channel/keeper/packet_test.go | 32 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go index 0c212d55c3e..cdf0f2ae2a5 100644 --- a/modules/core/04-channel/keeper/packet.go +++ b/modules/core/04-channel/keeper/packet.go @@ -374,7 +374,7 @@ func (k Keeper) AcknowledgePacket( ) } - if channel.State != types.OPEN { + if channel.State != types.OPEN && channel.FlushStatus != types.FLUSHING { return errorsmod.Wrapf( types.ErrInvalidChannelState, "channel state is not OPEN (got %s)", channel.State.String(), @@ -471,6 +471,11 @@ func (k Keeper) AcknowledgePacket( // Delete packet commitment, since the packet has been acknowledged, the commitement is no longer necessary k.deletePacketCommitment(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) + if channel.FlushStatus == types.FLUSHING && !k.hasInflightPackets(ctx, packet.GetSourcePort(), packet.GetSourceChannel()) { + channel.FlushStatus = types.FLUSHCOMPLETE + k.SetChannel(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), channel) + } + // log that a packet has been acknowledged k.Logger(ctx).Info( "packet acknowledged", diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 6aeb51cf4ca..a1463de7e4d 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -692,6 +692,26 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, true}, + {"channel in ackupgrade flush status in flushing", func() { + // setup uses an UNORDERED channel + suite.coordinator.Setup(path) + + // create packet commitment + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().NoError(err) + + channel := path.EndpointA.GetChannel() + channel.State = types.ACKUPGRADE + channel.FlushStatus = types.FLUSHING + path.EndpointA.SetChannel(channel) + + // create packet receipt and acknowledgement + 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) + + channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + }, true}, {"packet already acknowledged ordered channel (no-op)", func() { expError = types.ErrNoOpMsg @@ -749,6 +769,18 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.Require().NoError(err) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, false}, + {"channel in ackupgrade flush status not in flush", func() { + expError = types.ErrInvalidChannelState + + suite.coordinator.Setup(path) + packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, defaultTimeoutHeight, disabledTimeoutTimestamp) + + channel := path.EndpointA.GetChannel() + channel.State = types.ACKUPGRADE + channel.FlushStatus = types.NOTINFLUSH + path.EndpointA.SetChannel(channel) + channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + }, false}, {"capability authentication failed ORDERED", func() { expError = types.ErrInvalidChannelCapability From ffa311ea254bc5514cff29a8686b167955fba5d3 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Fri, 23 Jun 2023 15:37:14 +0300 Subject: [PATCH 2/8] Use handshake to reach correct state before mutating any fields. --- modules/core/04-channel/keeper/packet.go | 2 +- modules/core/04-channel/keeper/packet_test.go | 38 ++++++++++++++----- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/modules/core/04-channel/keeper/packet.go b/modules/core/04-channel/keeper/packet.go index cdf0f2ae2a5..7dd4adc0157 100644 --- a/modules/core/04-channel/keeper/packet.go +++ b/modules/core/04-channel/keeper/packet.go @@ -377,7 +377,7 @@ func (k Keeper) AcknowledgePacket( if channel.State != types.OPEN && channel.FlushStatus != types.FLUSHING { return errorsmod.Wrapf( types.ErrInvalidChannelState, - "channel state is not OPEN (got %s)", channel.State.String(), + "packets cannot be acknowledged on channel with state (%s) and flush status (%s)", channel.State, channel.FlushStatus, ) } diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index a1463de7e4d..fe5376ae64c 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -692,7 +692,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, true}, - {"channel in ackupgrade flush status in flushing", func() { + {"success on channel in tryupgrade flush status in flushing", func() { // setup uses an UNORDERED channel suite.coordinator.Setup(path) @@ -700,17 +700,22 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) - channel := path.EndpointA.GetChannel() - channel.State = types.ACKUPGRADE - channel.FlushStatus = types.FLUSHING - path.EndpointA.SetChannel(channel) - // create packet receipt and acknowledgement 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) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + + // Move channel to correct state. + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion + + err = path.EndpointB.ChanUpgradeInit() + suite.Require().NoError(err) + + err = path.EndpointA.ChanUpgradeTry() + suite.Require().NoError(err) }, true}, {"packet already acknowledged ordered channel (no-op)", func() { expError = types.ErrNoOpMsg @@ -769,17 +774,30 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.Require().NoError(err) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, false}, - {"channel in ackupgrade flush status not in flush", func() { + {"channel in tryupgrade flush status not in flush", func() { expError = types.ErrInvalidChannelState suite.coordinator.Setup(path) 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) + // 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() + suite.Require().NoError(err) + + err = path.EndpointB.ChanUpgradeTry() + suite.Require().NoError(err) + + err = path.EndpointA.ChanUpgradeAck() + suite.Require().NoError(err) + + // TODO: Manually set until #3928 is implemented. channel := path.EndpointA.GetChannel() - channel.State = types.ACKUPGRADE - channel.FlushStatus = types.NOTINFLUSH + channel.FlushStatus = types.FLUSHCOMPLETE path.EndpointA.SetChannel(channel) - channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, false}, {"capability authentication failed ORDERED", func() { expError = types.ErrInvalidChannelCapability From 864011a453f59b2dbafde8f1f91be63739f7180f Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Wed, 28 Jun 2023 16:00:06 +0300 Subject: [PATCH 3/8] Add test to verify post-ack channel state after last in-flight packet. --- modules/core/04-channel/keeper/packet_test.go | 48 +++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index fe5376ae64c..8b1e661e5ce 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -708,9 +708,6 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) // Move channel to correct state. - path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion - path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion - err = path.EndpointB.ChanUpgradeInit() suite.Require().NoError(err) @@ -990,3 +987,48 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { }) } } + +// TestAcknowledgeFlush tests that Acknowledging the last in-flight packet moves the channel +// state to FLUSHINGCOMPLETE. +func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { + var channelCap *capabilitytypes.Capability + + suite.SetupTest() // reset + path := ibctesting.NewPath(suite.chainA, suite.chainB) + + // setup uses an UNORDERED channel + suite.coordinator.Setup(path) + + // create packet commitment + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().NoError(err) + + // create packet receipt and acknowledgement + 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) + + // Move channel to UPGRADE_ACK, flush status set to flushing due to previous SendPacket + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion + + err = path.EndpointA.ChanUpgradeInit() + suite.Require().NoError(err) + + err = path.EndpointB.ChanUpgradeTry() + suite.Require().NoError(err) + + err = path.EndpointA.ChanUpgradeAck() + suite.Require().NoError(err) + + packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + proof, proofHeight := path.EndpointB.QueryProof(packetKey) + + channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + err = suite.chainA.App.GetIBCKeeper().ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight) + suite.Require().NoError(err) + + // Check that we've moved to FLUSHCOMPLETE + channelA := path.EndpointA.GetChannel() + suite.Require().Equal(types.FLUSHCOMPLETE, channelA.FlushStatus) +} From aef4e8f2fb56c35c818e64d3239af11c360c5b03 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Wed, 28 Jun 2023 16:26:38 +0300 Subject: [PATCH 4/8] Remove unecessary modifications of version for non initializing channel end. --- modules/core/04-channel/keeper/packet_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 8b1e661e5ce..0586289880c 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -708,6 +708,8 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) // Move channel to correct state. + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion + err = path.EndpointB.ChanUpgradeInit() suite.Require().NoError(err) @@ -780,7 +782,6 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { // 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() suite.Require().NoError(err) @@ -1010,7 +1011,6 @@ func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { // Move channel to UPGRADE_ACK, flush status set to flushing due to previous SendPacket path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion - path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion err = path.EndpointA.ChanUpgradeInit() suite.Require().NoError(err) From 27bbd5e123ae1cbcd050c1d89a5fbb153bedf2eb Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Wed, 28 Jun 2023 16:47:53 +0300 Subject: [PATCH 5/8] Test both cases: final in-flight packet and non-final one. --- modules/core/04-channel/keeper/packet_test.go | 85 ++++++++++++------- 1 file changed, 56 insertions(+), 29 deletions(-) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 0586289880c..5b5011d8edb 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -792,7 +792,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { err = path.EndpointA.ChanUpgradeAck() suite.Require().NoError(err) - // TODO: Manually set until #3928 is implemented. + // TODO: Manually set until #3929 is implemented. channel := path.EndpointA.GetChannel() channel.FlushStatus = types.FLUSHCOMPLETE path.EndpointA.SetChannel(channel) @@ -992,43 +992,70 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { // TestAcknowledgeFlush tests that Acknowledging the last in-flight packet moves the channel // state to FLUSHINGCOMPLETE. func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { - var channelCap *capabilitytypes.Capability + var path *ibctesting.Path - suite.SetupTest() // reset - path := ibctesting.NewPath(suite.chainA, suite.chainB) + testCases := []struct { + msg string + malleate func() + expectedFlushStatus types.FlushStatus + }{ + { + "success: final packet commitment cleared, flush status set to FLUSHCOMPLETE", + func() {}, + types.FLUSHCOMPLETE, + }, + { + "success: has in-flight packets, flush status remains FLUSHING", + func() { + // Send an additional packet + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().Equal(uint64(1), sequence) + suite.Require().NoError(err) + }, + types.FLUSHING, + }, + } - // setup uses an UNORDERED channel - suite.coordinator.Setup(path) + for i, tc := range testCases { + tc := tc + suite.Run(fmt.Sprintf("Case %s, %d/%d tests", tc.msg, i, len(testCases)), func() { + suite.SetupTest() // reset + path = ibctesting.NewPath(suite.chainA, suite.chainB) + suite.coordinator.Setup(path) - // create packet commitment - sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) - suite.Require().NoError(err) + tc.malleate() - // create packet receipt and acknowledgement - 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) + // create packet commitment + sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) + suite.Require().NoError(err) + + // create packet receipt and acknowledgement + 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) - // Move channel to UPGRADE_ACK, flush status set to flushing due to previous SendPacket - path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion + // Move channel to UPGRADE_ACK, flush status set to flushing due to previous SendPacket + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion - err = path.EndpointA.ChanUpgradeInit() - suite.Require().NoError(err) + err = path.EndpointA.ChanUpgradeInit() + suite.Require().NoError(err) - err = path.EndpointB.ChanUpgradeTry() - suite.Require().NoError(err) + err = path.EndpointB.ChanUpgradeTry() + suite.Require().NoError(err) - err = path.EndpointA.ChanUpgradeAck() - suite.Require().NoError(err) + err = path.EndpointA.ChanUpgradeAck() + suite.Require().NoError(err) - packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) - proof, proofHeight := path.EndpointB.QueryProof(packetKey) + packetKey := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) + proof, proofHeight := path.EndpointB.QueryProof(packetKey) - channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) - err = suite.chainA.App.GetIBCKeeper().ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight) - suite.Require().NoError(err) + channelCap := suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + err = suite.chainA.App.GetIBCKeeper().ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight) + suite.Require().NoError(err) - // Check that we've moved to FLUSHCOMPLETE - channelA := path.EndpointA.GetChannel() - suite.Require().Equal(types.FLUSHCOMPLETE, channelA.FlushStatus) + // Check that we've moved to FLUSHCOMPLETE + channelA := path.EndpointA.GetChannel() + suite.Require().Equal(tc.expectedFlushStatus, channelA.FlushStatus) + }) + } } From f76b257a49fd34775866321fb9ad262eff6f5723 Mon Sep 17 00:00:00 2001 From: Jim Fasarakis-Hilliard Date: Fri, 30 Jun 2023 10:48:12 +0300 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Carlos Rodriguez --- modules/core/04-channel/keeper/packet_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 5b5011d8edb..64f4499d347 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -692,7 +692,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, true}, - {"success on channel in tryupgrade flush status in flushing", func() { + {"success on channel in tryupgrade and flush status in flushing", func() { // setup uses an UNORDERED channel suite.coordinator.Setup(path) @@ -773,7 +773,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.Require().NoError(err) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, false}, - {"channel in tryupgrade flush status not in flush", func() { + {"channel in tryupgrade and flush status not in flush", func() { expError = types.ErrInvalidChannelState suite.coordinator.Setup(path) @@ -989,8 +989,8 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { } } -// TestAcknowledgeFlush tests that Acknowledging the last in-flight packet moves the channel -// state to FLUSHINGCOMPLETE. +// TestAcknowledgeFlushStatus tests that Acknowledging the last in-flight packet moves the channel +// flush status to FLUSHCOMPLETE. func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { var path *ibctesting.Path @@ -1009,7 +1009,7 @@ func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { func() { // Send an additional packet sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) - suite.Require().Equal(uint64(1), sequence) + suite.Require().Equal(uint64(2), sequence) suite.Require().NoError(err) }, types.FLUSHING, @@ -1023,8 +1023,6 @@ func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { path = ibctesting.NewPath(suite.chainA, suite.chainB) suite.coordinator.Setup(path) - tc.malleate() - // create packet commitment sequence, err := path.EndpointA.SendPacket(defaultTimeoutHeight, disabledTimeoutTimestamp, ibctesting.MockPacketData) suite.Require().NoError(err) @@ -1034,6 +1032,8 @@ func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { err = path.EndpointB.RecvPacket(packet) suite.Require().NoError(err) + tc.malleate() + // Move channel to UPGRADE_ACK, flush status set to flushing due to previous SendPacket path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion @@ -1053,7 +1053,7 @@ func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { err = suite.chainA.App.GetIBCKeeper().ChannelKeeper.AcknowledgePacket(suite.chainA.GetContext(), channelCap, packet, ibcmock.MockAcknowledgement.Acknowledgement(), proof, proofHeight) suite.Require().NoError(err) - // Check that we've moved to FLUSHCOMPLETE + // Check that we've moved to expected flush status channelA := path.EndpointA.GetChannel() suite.Require().Equal(tc.expectedFlushStatus, channelA.FlushStatus) }) From 3ed00e9f8317aef304051e08737dad2094bee522 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Mon, 3 Jul 2023 11:45:48 +0300 Subject: [PATCH 7/8] Remove manual setting of flush status. --- modules/core/04-channel/keeper/packet_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index 64f4499d347..df8d39748f0 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -791,11 +791,6 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { err = path.EndpointA.ChanUpgradeAck() suite.Require().NoError(err) - - // TODO: Manually set until #3929 is implemented. - channel := path.EndpointA.GetChannel() - channel.FlushStatus = types.FLUSHCOMPLETE - path.EndpointA.SetChannel(channel) }, false}, {"capability authentication failed ORDERED", func() { expError = types.ErrInvalidChannelCapability From d24777707cbfe08400dee9cc86344efc97e97d37 Mon Sep 17 00:00:00 2001 From: DimitrisJim Date: Wed, 5 Jul 2023 21:28:33 +0300 Subject: [PATCH 8/8] Update test name, pass mock version to both channels. --- modules/core/04-channel/keeper/packet_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/core/04-channel/keeper/packet_test.go b/modules/core/04-channel/keeper/packet_test.go index bbecebeac17..fc2089a2a89 100644 --- a/modules/core/04-channel/keeper/packet_test.go +++ b/modules/core/04-channel/keeper/packet_test.go @@ -789,6 +789,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) // Move channel to correct state. + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion err = path.EndpointB.ChanUpgradeInit() @@ -854,7 +855,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { suite.Require().NoError(err) channelCap = suite.chainA.GetChannelCapability(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, false}, - {"channel in tryupgrade and flush status not in flush", func() { + {"channel in ackupgrade and flush status flush complete", func() { expError = types.ErrInvalidChannelState suite.coordinator.Setup(path) @@ -863,6 +864,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() { // 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() suite.Require().NoError(err) @@ -1112,6 +1114,7 @@ func (suite *KeeperTestSuite) TestAcknowledgeFlushStatus() { // Move channel to UPGRADE_ACK, flush status set to flushing due to previous SendPacket path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = ibcmock.UpgradeVersion err = path.EndpointA.ChanUpgradeInit() suite.Require().NoError(err)