check ordered channel ordering in RecvPacket (cosmos#6574)

* move nextrecvseq check to recvpacket * add test for next recv seq not found * fix test bug, increase cov * remove dup check and test * Update x/ibc/04-channel/keeper/packet_test.go Co-authored-by: Aditya <adityasripal@gmail.com> Co-authored-by: Aditya <adityasripal@gmail.com> Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
mdaleem · Jul 2, 2020 · 5a22342 · 5a22342
1 parent a3d9cc5
commit 5a22342
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 22 deletions.
diff --git a/x/ibc/04-channel/keeper/packet.go b/x/ibc/04-channel/keeper/packet.go
@@ -212,6 +212,24 @@ func (k Keeper) RecvPacket(
 		}
 	}
 
+	// check if the packet is being received in order
+	if channel.Ordering == types.ORDERED {
+		nextSequenceRecv, found := k.GetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel())
+		if !found {
+			return sdkerrors.Wrapf(
+				types.ErrSequenceReceiveNotFound,
+				"destination port: %s, destination channel: %s", packet.GetDestPort(), packet.GetDestChannel(),
+			)
+		}
+
+		if packet.GetSequence() != nextSequenceRecv {
+			return sdkerrors.Wrapf(
+				types.ErrInvalidPacket,
+				"packet sequence ≠ next receive sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceRecv,
+			)
+		}
+	}
+
 	if err := k.connectionKeeper.VerifyPacketCommitment(
 		ctx, connectionEnd, proofHeight, proof,
 		packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence(),
@@ -274,13 +292,6 @@ func (k Keeper) PacketExecuted(
 			)
 		}
 
-		if packet.GetSequence() != nextSequenceRecv {
-			return sdkerrors.Wrapf(
-				types.ErrInvalidPacket,
-				"packet sequence ≠ next receive sequence (%d ≠ %d)", packet.GetSequence(), nextSequenceRecv,
-			)
-		}
-
 		nextSequenceRecv++
 
 		k.SetNextSequenceRecv(ctx, packet.GetDestPort(), packet.GetDestChannel(), nextSequenceRecv)

diff --git a/x/ibc/04-channel/keeper/packet_test.go b/x/ibc/04-channel/keeper/packet_test.go
@@ -232,8 +232,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			suite.Require().NoError(err)
 			// attempts to receive packet 2 without receiving packet 1
 		}, true},
-		// TODO: uncomment with implementation of issue: https://github.com/cosmos/cosmos-sdk/issues/6519
-		/*	{"out of order packet failure with ORDERED channel", func() {
+		{"out of order packet failure with ORDERED channel", func() {
 			_, clientB, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint)
 			channelA, channelB := suite.coordinator.CreateChannel(suite.chainA, suite.chainB, connA, connB, types.ORDERED)
 			packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp)
@@ -246,7 +245,7 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			err = suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB)
 			suite.Require().NoError(err)
 			// attempts to receive packet 2 without receiving packet 1
-		}, false}, */
+		}, false},
 		{"channel not found", func() {
 			// use wrong channel naming
 			_, _, _, _, channelA, _ := suite.coordinator.Setup(suite.chainA, suite.chainB)
@@ -313,6 +312,23 @@ func (suite *KeeperTestSuite) TestRecvPacket() {
 			// write packet acknowledgement
 			suite.coordinator.PacketExecuted(suite.chainB, suite.chainA, packet, clientA)
 		}, false},
+		{"next receive sequence is not found", func() {
+			_, _, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint)
+			channelA := connA.NextTestChannel()
+			channelB := connB.NextTestChannel()
+
+			// manually creating channel prevents next recv sequence from being set
+			suite.chainB.App.IBCKeeper.ChannelKeeper.SetChannel(
+				suite.chainB.GetContext(),
+				channelB.PortID, channelB.ID,
+				types.NewChannel(types.OPEN, types.ORDERED, types.NewCounterparty(channelA.PortID, channelA.ID), []string{connB.ID}, ibctesting.ChannelVersion),
+			)
+
+			packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp)
+
+			// manually set packet commitment
+			suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.TestHash)
+		}, false},
 		{"validation failed", func() {
 			// packet commitment not set resulting in invalid proof
 			_, _, _, _, channelA, channelB := suite.coordinator.Setup(suite.chainA, suite.chainB)
@@ -392,17 +408,6 @@ func (suite *KeeperTestSuite) TestPacketExecuted() {
 			suite.chainB.CreateChannelCapability(channelB.PortID, channelB.ID)
 			channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID)
 		}, false},
-		{"packet sequence ≠ next sequence receive", func() {
-			clientA, clientB, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint)
-			channelA, channelB := suite.coordinator.CreateChannel(suite.chainA, suite.chainB, connA, connB, types.ORDERED)
-			packet = types.NewPacket(validPacketData, 1, channelA.PortID, channelA.ID, channelB.PortID, channelB.ID, timeoutHeight, disabledTimeoutTimestamp)
-			suite.coordinator.SendPacket(suite.chainA, suite.chainB, packet, clientB)
-			channelCap = suite.chainB.GetChannelCapability(channelB.PortID, channelB.ID)
-
-			// increments sequence receive
-			err := suite.coordinator.PacketExecuted(suite.chainB, suite.chainA, packet, clientA)
-			suite.Require().NoError(err)
-		}, false},
 		{"capability not found", func() {
 			_, clientB, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint)
 			channelA, channelB := suite.coordinator.CreateChannel(suite.chainA, suite.chainB, connA, connB, types.ORDERED)
@@ -553,7 +558,7 @@ func (suite *KeeperTestSuite) TestAcknowledgePacket() {
 			suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketCommitment(suite.chainA.GetContext(), channelA.PortID, channelA.ID, packet.GetSequence(), ibctesting.TestHash)
 
 			// manually set packet acknowledgement
-			suite.chainA.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(suite.chainA.GetContext(), channelB.PortID, channelB.ID, packet.GetSequence(), ibctesting.TestHash)
+			suite.chainB.App.IBCKeeper.ChannelKeeper.SetPacketAcknowledgement(suite.chainB.GetContext(), channelB.PortID, channelB.ID, packet.GetSequence(), ibctesting.TestHash)
 		}, false},
 		{"next ack sequence mismatch", func() {
 			clientA, clientB, connA, connB := suite.coordinator.SetupClientConnections(suite.chainA, suite.chainB, clientexported.Tendermint)