Skip to content

Commit 5664efc

Browse files
TarekAwwadTarekAwwad
authored
Merge pull request #1 from KiFoundation/fix/huckleberry-v3
Fix/huckleberry v3
2 parents f3db49c + 1b18ffa commit 5664efc

File tree

3 files changed

+148
-16
lines changed

3 files changed

+148
-16
lines changed

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

Lines changed: 46 additions & 9 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)
@@ -501,4 +538,4 @@ func validategRPCRequest(portID, channelID string) error {
501538
}
502539

503540
return nil
504-
}
541+
}

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

Lines changed: 98 additions & 4 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 {
@@ -1451,4 +1545,4 @@ func (suite *KeeperTestSuite) TestQueryNextSequenceReceive() {
14511545
}
14521546
})
14531547
}
1454-
}
1548+
}

‎modules/core/keeper/msg_server.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,12 +410,13 @@ func (k Keeper) RecvPacket(goCtx context.Context, msg *channeltypes.MsgRecvPacke
410410
// Cache context so that we may discard state changes from callback if the acknowledgement is unsuccessful.
411411
cacheCtx, writeFn = ctx.CacheContext()
412412
ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer)
413-
// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
414-
// Events from callback are emitted regardless of acknowledgement success
415-
ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
413+
416414
if ack == nil || ack.Success() {
417415
// write application state changes for asynchronous and successful acknowledgements
418416
writeFn()
417+
418+
// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
419+
ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
419420
}
420421

421422
// Set packet acknowledgement only if the acknowledgement is not nil.

0 commit comments

Comments
 (0)