Handle ordered packets in UnreceivedPackets query. (backport #3346) (#…

…3614) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]> Co-authored-by: colin axner <[email protected]>
cosmos · May 18, 2023 · bd6c832 · bd6c832
1 parent 3a593b2
commit bd6c832
Show file tree

Hide file tree

Showing 4 changed files with 147 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -51,7 +51,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
 ### Features
 
 ### Bug Fixes
-
+
+* [\#3346](https://github.com/cosmos/ibc-go/pull/3346) Properly handle ordered channels in `UnreceivedPackets` query.
+
 ## [v7.0.0](https://github.com/cosmos/ibc-go/releases/tag/v6.1.0) - 2023-03-17 
 
 ### Dependencies

diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go
@@ -3,7 +3,6 @@ package types
 import (
 	"math/big"
 
-	errorsmod "cosmossdk.io/errors"
 	sdkmath "cosmossdk.io/math"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
@@ -43,7 +42,7 @@ func (a TransferAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.Accep
 		}
 
 		if !isAllowedAddress(ctx, msgTransfer.Receiver, allocation.AllowList) {
-			return authz.AcceptResponse{}, errorsmod.Wrap(sdkerrors.ErrInvalidAddress, "not allowed receiver address for transfer")
+			return authz.AcceptResponse{}, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "not allowed receiver address for transfer")
 		}
 
 		// If the spend limit is set to the MaxUint256 sentinel value, do not subtract the amount from the spend limit.
@@ -53,7 +52,7 @@ func (a TransferAuthorization) Accept(ctx sdk.Context, msg sdk.Msg) (authz.Accep
 
 		limitLeft, isNegative := allocation.SpendLimit.SafeSub(msgTransfer.Token)
 		if isNegative {
-			return authz.AcceptResponse{}, errorsmod.Wrapf(sdkerrors.ErrInsufficientFunds, "requested amount is more than spend limit")
+			return authz.AcceptResponse{}, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "requested amount is more than spend limit")
 		}
 
 		if limitLeft.IsZero() {

diff --git a/modules/core/04-channel/keeper/grpc_query.go b/modules/core/04-channel/keeper/grpc_query.go
@@ -398,18 +398,55 @@ func (q Keeper) UnreceivedPackets(c context.Context, req *types.QueryUnreceivedP
 
 	ctx := sdk.UnwrapSDKContext(c)
 
-	unreceivedSequences := []uint64{}
+	channel, found := q.GetChannel(sdk.UnwrapSDKContext(c), req.PortId, req.ChannelId)
+	if !found {
+		return nil, status.Error(
+			codes.NotFound,
+			sdkerrors.Wrapf(types.ErrChannelNotFound, "port-id: %s, channel-id %s", req.PortId, req.ChannelId).Error(),
+		)
+	}
 
-	for i, seq := range req.PacketCommitmentSequences {
-		if seq == 0 {
-			return nil, status.Errorf(codes.InvalidArgument, "packet sequence %d cannot be 0", i)
-		}
+	var unreceivedSequences []uint64
+	switch channel.Ordering {
+	case types.UNORDERED:
+		for i, seq := range req.PacketCommitmentSequences {
+			// filter for invalid sequences to ensure they are not included in the response value.
+			if seq == 0 {
+				return nil, status.Errorf(codes.InvalidArgument, "packet sequence %d cannot be 0", i)
+			}
 
-		// if packet receipt exists on the receiving chain, then packet has already been received
-		if _, found := q.GetPacketReceipt(ctx, req.PortId, req.ChannelId, seq); !found {
-			unreceivedSequences = append(unreceivedSequences, seq)
+			// if the packet receipt does not exist, then it is unreceived
+			if _, found := q.GetPacketReceipt(ctx, req.PortId, req.ChannelId, seq); !found {
+				unreceivedSequences = append(unreceivedSequences, seq)
+			}
+		}
+	case types.ORDERED:
+		nextSequenceRecv, found := q.GetNextSequenceRecv(ctx, req.PortId, req.ChannelId)
+		if !found {
+			return nil, status.Error(
+				codes.NotFound,
+				sdkerrors.Wrapf(
+					types.ErrSequenceReceiveNotFound,
+					"destination port: %s, destination channel: %s", req.PortId, req.ChannelId,
+				).Error(),
+			)
 		}
 
+		for i, seq := range req.PacketCommitmentSequences {
+			// filter for invalid sequences to ensure they are not included in the response value.
+			if seq == 0 {
+				return nil, status.Errorf(codes.InvalidArgument, "packet sequence %d cannot be 0", i)
+			}
+
+			// Any sequence greater than or equal to the next sequence to be received is not received.
+			if seq >= nextSequenceRecv {
+				unreceivedSequences = append(unreceivedSequences, seq)
+			}
+		}
+	default:
+		return nil, status.Error(
+			codes.InvalidArgument,
+			sdkerrors.Wrapf(types.ErrInvalidChannelOrdering, "channel order %s is not supported", channel.Ordering.String()).Error())
 	}
 
 	selfHeight := clienttypes.GetSelfHeight(ctx)

diff --git a/modules/core/04-channel/keeper/grpc_query_test.go b/modules/core/04-channel/keeper/grpc_query_test.go
@@ -1112,7 +1112,7 @@ func (suite *KeeperTestSuite) TestQueryPacketAcknowledgements() {
 func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
 	var (
 		req    *types.QueryUnreceivedPacketsRequest
-		expSeq = []uint64{}
+		expSeq = []uint64(nil)
 	)
 
 	testCases := []struct {
@@ -1158,6 +1158,46 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
 			},
 			false,
 		},
+		{
+			"invalid seq, ordered channel",
+			func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				path.SetChannelOrdered()
+				suite.coordinator.Setup(path)
+
+				req = &types.QueryUnreceivedPacketsRequest{
+					PortId:                    path.EndpointA.ChannelConfig.PortID,
+					ChannelId:                 path.EndpointA.ChannelID,
+					PacketCommitmentSequences: []uint64{0},
+				}
+			},
+			false,
+		},
+		{
+			"channel not found",
+			func() {
+				req = &types.QueryUnreceivedPacketsRequest{
+					PortId:    "invalid-port-id",
+					ChannelId: "invalid-channel-id",
+				}
+			},
+			false,
+		},
+		{
+			"basic success empty packet commitments",
+			func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				suite.coordinator.Setup(path)
+
+				expSeq = []uint64(nil)
+				req = &types.QueryUnreceivedPacketsRequest{
+					PortId:                    path.EndpointA.ChannelConfig.PortID,
+					ChannelId:                 path.EndpointA.ChannelID,
+					PacketCommitmentSequences: []uint64{},
+				}
+			},
+			true,
+		},
 		{
 			"basic success unreceived packet commitments",
 			func() {
@@ -1183,7 +1223,7 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
 
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetPacketReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, 1)
 
-				expSeq = []uint64{}
+				expSeq = []uint64(nil)
 				req = &types.QueryUnreceivedPacketsRequest{
 					PortId:                    path.EndpointA.ChannelConfig.PortID,
 					ChannelId:                 path.EndpointA.ChannelID,
@@ -1197,7 +1237,7 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
 			func() {
 				path := ibctesting.NewPath(suite.chainA, suite.chainB)
 				suite.coordinator.Setup(path)
-				expSeq = []uint64{} // reset
+				expSeq = []uint64(nil) // reset
 				packetCommitments := []uint64{}
 
 				// set packet receipt for every other sequence
@@ -1219,6 +1259,60 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
 			},
 			true,
 		},
+		{
+			"basic success empty packet commitments, ordered channel",
+			func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				path.SetChannelOrdered()
+				suite.coordinator.Setup(path)
+
+				expSeq = []uint64(nil)
+				req = &types.QueryUnreceivedPacketsRequest{
+					PortId:                    path.EndpointA.ChannelConfig.PortID,
+					ChannelId:                 path.EndpointA.ChannelID,
+					PacketCommitmentSequences: []uint64{},
+				}
+			},
+			true,
+		},
+		{
+			"basic success unreceived packet commitments, ordered channel",
+			func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				path.SetChannelOrdered()
+				suite.coordinator.Setup(path)
+
+				// Note: NextSequenceRecv is set to 1 on channel creation.
+				expSeq = []uint64{1}
+				req = &types.QueryUnreceivedPacketsRequest{
+					PortId:                    path.EndpointA.ChannelConfig.PortID,
+					ChannelId:                 path.EndpointA.ChannelID,
+					PacketCommitmentSequences: []uint64{1},
+				}
+			},
+			true,
+		},
+		{
+			"basic success multiple unreceived packet commitments, ordered channel",
+			func() {
+				path := ibctesting.NewPath(suite.chainA, suite.chainB)
+				path.SetChannelOrdered()
+				suite.coordinator.Setup(path)
+
+				// Exercise scenario from issue #1532. NextSequenceRecv is 5, packet commitments provided are 2, 7, 9, 10.
+				// Packet sequence 2 is already received so only sequences 7, 9, 10 should be considered unreceived.
+				expSeq = []uint64{7, 9, 10}
+				packetCommitments := []uint64{2, 7, 9, 10}
+				suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, 5)
+
+				req = &types.QueryUnreceivedPacketsRequest{
+					PortId:                    path.EndpointA.ChannelConfig.PortID,
+					ChannelId:                 path.EndpointA.ChannelID,
+					PacketCommitmentSequences: packetCommitments,
+				}
+			},
+			true,
+		},
 	}
 
 	for _, tc := range testCases {