diff --git a/CHANGELOG.md b/CHANGELOG.md index 4243f1abca2..c68c17a50ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes +* [\#3346](https://github.com/cosmos/ibc-go/pull/3346) Properly handle ordered channels in `UnreceivedPackets` query. + ## [v6.1.0](https://github.com/cosmos/ibc-go/releases/tag/v6.1.0) - 2022-12-20 ### Dependencies diff --git a/modules/apps/transfer/types/transfer_authorization.go b/modules/apps/transfer/types/transfer_authorization.go index 9ffc170b13c..72d73e66d56 100644 --- 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" @@ -44,7 +43,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. @@ -54,7 +53,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 index e6bc4ea2ecf..232b27f6f62 100644 --- a/modules/core/04-channel/keeper/grpc_query.go +++ b/modules/core/04-channel/keeper/grpc_query.go @@ -397,18 +397,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 index 4772854a4da..19eca2e5bd7 100644 --- a/modules/core/04-channel/keeper/grpc_query_test.go +++ b/modules/core/04-channel/keeper/grpc_query_test.go @@ -1110,7 +1110,7 @@ func (suite *KeeperTestSuite) TestQueryPacketAcknowledgements() { func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() { var ( req *types.QueryUnreceivedPacketsRequest - expSeq = []uint64{} + expSeq = []uint64(nil) ) testCases := []struct { @@ -1156,6 +1156,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() { @@ -1181,7 +1221,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, @@ -1195,7 +1235,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 @@ -1217,6 +1257,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 {