Skip to content

Commit 8a6feca

Browse files
mergify[bot]DimitrisJimcrodriguezvega
authored
Handle ordered packets in UnreceivedPackets query. (backport #3346) (#3607)
* fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <cian@interchain.io> (cherry picked from commit c77f80f) * fix conflicts * fix links * add changelog --------- Co-authored-by: Jim Fasarakis-Hilliard <d.f.hilliard@gmail.com> Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
1 parent 171ed72 commit 8a6feca

File tree

5 files changed

+146
-13
lines changed

5 files changed

+146
-13
lines changed

‎.github/PULL_REQUEST_TEMPLATE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ write a little note why.
4343

4444
- [ ] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#pr-targeting)).
4545
- [ ] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
46-
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/10-structure.md).
46+
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/11-structure.md).
4747
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/master/CONTRIBUTING.md#testing).
4848
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`).
4949
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ All notable changes to this project will be documented in this file.
4949

5050
### Bug Fixes
5151

52+
* [\#3346](https://github.com/cosmos/ibc-go/pull/3346) Properly handle ordered channels in `UnreceivedPackets` query.
53+
5254
## [v4.3.0](https://github.com/cosmos/ibc-go/releases/tag/v4.3.0) - 2023-01-24
5355

5456
### Dependencies

‎docs/ibc/integration.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func NewApp(...args) *App {
139139
140140
### Module Managers
141141
142-
In order to use IBC, we need to add the new modules to the module `Manager` and to the `SimulationManager` in case your application supports [simulations](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/13-simulator.md).
142+
In order to use IBC, we need to add the new modules to the module `Manager` and to the `SimulationManager` in case your application supports [simulations](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/14-simulator.md).
143143
144144
```go
145145
// app.go

‎modules/core/04-channel/keeper/grpc_query.go‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -397,18 +397,55 @@ func (q Keeper) UnreceivedPackets(c context.Context, req *types.QueryUnreceivedP
397397

398398
ctx := sdk.UnwrapSDKContext(c)
399399

400-
unreceivedSequences := []uint64{}
400+
channel, found := q.GetChannel(sdk.UnwrapSDKContext(c), req.PortId, req.ChannelId)
401+
if !found {
402+
return nil, status.Error(
403+
codes.NotFound,
404+
sdkerrors.Wrapf(types.ErrChannelNotFound, "port-id: %s, channel-id %s", req.PortId, req.ChannelId).Error(),
405+
)
406+
}
401407

402-
for i, seq := range req.PacketCommitmentSequences {
403-
if seq == 0 {
404-
return nil, status.Errorf(codes.InvalidArgument, "packet sequence %d cannot be 0", i)
405-
}
408+
var unreceivedSequences []uint64
409+
switch channel.Ordering {
410+
case types.UNORDERED:
411+
for i, seq := range req.PacketCommitmentSequences {
412+
// filter for invalid sequences to ensure they are not included in the response value.
413+
if seq == 0 {
414+
return nil, status.Errorf(codes.InvalidArgument, "packet sequence %d cannot be 0", i)
415+
}
406416

407-
// if packet receipt exists on the receiving chain, then packet has already been received
408-
if _, found := q.GetPacketReceipt(ctx, req.PortId, req.ChannelId, seq); !found {
409-
unreceivedSequences = append(unreceivedSequences, seq)
417+
// if the packet receipt does not exist, then it is unreceived
418+
if _, found := q.GetPacketReceipt(ctx, req.PortId, req.ChannelId, seq); !found {
419+
unreceivedSequences = append(unreceivedSequences, seq)
420+
}
421+
}
422+
case types.ORDERED:
423+
nextSequenceRecv, found := q.GetNextSequenceRecv(ctx, req.PortId, req.ChannelId)
424+
if !found {
425+
return nil, status.Error(
426+
codes.NotFound,
427+
sdkerrors.Wrapf(
428+
types.ErrSequenceReceiveNotFound,
429+
"destination port: %s, destination channel: %s", req.PortId, req.ChannelId,
430+
).Error(),
431+
)
410432
}
411433

434+
for i, seq := range req.PacketCommitmentSequences {
435+
// filter for invalid sequences to ensure they are not included in the response value.
436+
if seq == 0 {
437+
return nil, status.Errorf(codes.InvalidArgument, "packet sequence %d cannot be 0", i)
438+
}
439+
440+
// Any sequence greater than or equal to the next sequence to be received is not received.
441+
if seq >= nextSequenceRecv {
442+
unreceivedSequences = append(unreceivedSequences, seq)
443+
}
444+
}
445+
default:
446+
return nil, status.Error(
447+
codes.InvalidArgument,
448+
sdkerrors.Wrapf(types.ErrInvalidChannelOrdering, "channel order %s is not supported", channel.Ordering.String()).Error())
412449
}
413450

414451
selfHeight := clienttypes.GetSelfHeight(ctx)

‎modules/core/04-channel/keeper/grpc_query_test.go‎

Lines changed: 97 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ func (suite *KeeperTestSuite) TestQueryPacketAcknowledgements() {
11101110
func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
11111111
var (
11121112
req *types.QueryUnreceivedPacketsRequest
1113-
expSeq = []uint64{}
1113+
expSeq = []uint64(nil)
11141114
)
11151115

11161116
testCases := []struct {
@@ -1156,6 +1156,46 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
11561156
},
11571157
false,
11581158
},
1159+
{
1160+
"invalid seq, ordered channel",
1161+
func() {
1162+
path := ibctesting.NewPath(suite.chainA, suite.chainB)
1163+
path.SetChannelOrdered()
1164+
suite.coordinator.Setup(path)
1165+
1166+
req = &types.QueryUnreceivedPacketsRequest{
1167+
PortId: path.EndpointA.ChannelConfig.PortID,
1168+
ChannelId: path.EndpointA.ChannelID,
1169+
PacketCommitmentSequences: []uint64{0},
1170+
}
1171+
},
1172+
false,
1173+
},
1174+
{
1175+
"channel not found",
1176+
func() {
1177+
req = &types.QueryUnreceivedPacketsRequest{
1178+
PortId: "invalid-port-id",
1179+
ChannelId: "invalid-channel-id",
1180+
}
1181+
},
1182+
false,
1183+
},
1184+
{
1185+
"basic success empty packet commitments",
1186+
func() {
1187+
path := ibctesting.NewPath(suite.chainA, suite.chainB)
1188+
suite.coordinator.Setup(path)
1189+
1190+
expSeq = []uint64(nil)
1191+
req = &types.QueryUnreceivedPacketsRequest{
1192+
PortId: path.EndpointA.ChannelConfig.PortID,
1193+
ChannelId: path.EndpointA.ChannelID,
1194+
PacketCommitmentSequences: []uint64{},
1195+
}
1196+
},
1197+
true,
1198+
},
11591199
{
11601200
"basic success unreceived packet commitments",
11611201
func() {
@@ -1181,7 +1221,7 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
11811221

11821222
suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetPacketReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, 1)
11831223

1184-
expSeq = []uint64{}
1224+
expSeq = []uint64(nil)
11851225
req = &types.QueryUnreceivedPacketsRequest{
11861226
PortId: path.EndpointA.ChannelConfig.PortID,
11871227
ChannelId: path.EndpointA.ChannelID,
@@ -1195,7 +1235,7 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
11951235
func() {
11961236
path := ibctesting.NewPath(suite.chainA, suite.chainB)
11971237
suite.coordinator.Setup(path)
1198-
expSeq = []uint64{} // reset
1238+
expSeq = []uint64(nil) // reset
11991239
packetCommitments := []uint64{}
12001240

12011241
// set packet receipt for every other sequence
@@ -1217,6 +1257,60 @@ func (suite *KeeperTestSuite) TestQueryUnreceivedPackets() {
12171257
},
12181258
true,
12191259
},
1260+
{
1261+
"basic success empty packet commitments, ordered channel",
1262+
func() {
1263+
path := ibctesting.NewPath(suite.chainA, suite.chainB)
1264+
path.SetChannelOrdered()
1265+
suite.coordinator.Setup(path)
1266+
1267+
expSeq = []uint64(nil)
1268+
req = &types.QueryUnreceivedPacketsRequest{
1269+
PortId: path.EndpointA.ChannelConfig.PortID,
1270+
ChannelId: path.EndpointA.ChannelID,
1271+
PacketCommitmentSequences: []uint64{},
1272+
}
1273+
},
1274+
true,
1275+
},
1276+
{
1277+
"basic success unreceived packet commitments, ordered channel",
1278+
func() {
1279+
path := ibctesting.NewPath(suite.chainA, suite.chainB)
1280+
path.SetChannelOrdered()
1281+
suite.coordinator.Setup(path)
1282+
1283+
// Note: NextSequenceRecv is set to 1 on channel creation.
1284+
expSeq = []uint64{1}
1285+
req = &types.QueryUnreceivedPacketsRequest{
1286+
PortId: path.EndpointA.ChannelConfig.PortID,
1287+
ChannelId: path.EndpointA.ChannelID,
1288+
PacketCommitmentSequences: []uint64{1},
1289+
}
1290+
},
1291+
true,
1292+
},
1293+
{
1294+
"basic success multiple unreceived packet commitments, ordered channel",
1295+
func() {
1296+
path := ibctesting.NewPath(suite.chainA, suite.chainB)
1297+
path.SetChannelOrdered()
1298+
suite.coordinator.Setup(path)
1299+
1300+
// Exercise scenario from issue #1532. NextSequenceRecv is 5, packet commitments provided are 2, 7, 9, 10.
1301+
// Packet sequence 2 is already received so only sequences 7, 9, 10 should be considered unreceived.
1302+
expSeq = []uint64{7, 9, 10}
1303+
packetCommitments := []uint64{2, 7, 9, 10}
1304+
suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetNextSequenceRecv(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, 5)
1305+
1306+
req = &types.QueryUnreceivedPacketsRequest{
1307+
PortId: path.EndpointA.ChannelConfig.PortID,
1308+
ChannelId: path.EndpointA.ChannelID,
1309+
PacketCommitmentSequences: packetCommitments,
1310+
}
1311+
},
1312+
true,
1313+
},
12201314
}
12211315

12221316
for _, tc := range testCases {

0 commit comments

Comments
 (0)