Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 4 commits into from
May 18, 2023
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dddb591
fix: handle ordered packets in `UnreceivedPackets` query
DimitrisJim Mar 30, 2023
775e3cd
add changelog
crodriguezvega May 17, 2023
f4daeea
add import
crodriguezvega May 17, 2023
2472ba9
use sdkerrors
crodriguezvega May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 2 additions & 3 deletions modules/apps/transfer/types/transfer_authorization.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package types
import (
"math/big"

errorsmod "cosmossdk.io/errors"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reverted this change that came in with this backport PR, since using the errors package was introduced in v7.1.x

sdkmath "cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -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.
Expand All @@ -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() {
Expand Down
53 changes: 45 additions & 8 deletions modules/core/04-channel/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
100 changes: 97 additions & 3 deletions modules/core/04-channel/keeper/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,7 @@ func (suite *KeeperTestSuite) TestQueryPacketAcknowledgements() {
func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
var (
req *types.QueryUnreceivedPacketsRequest
expSeq = []uint64{}
expSeq = []uint64(nil)
)

testCases := []struct {
Expand Down Expand Up @@ -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() {
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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 {
Expand Down