Handle ordered packets in UnreceivedPackets query. #3346

DimitrisJim · 2023-03-27T07:18:13Z

Description

closes: #1532

Commit Message / Changelog Entry

fix: handle ordered packets in `UnreceivedPackets` query.

see the guidelines for commit messages. (view raw markdown for examples)

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

Targeted PR against correct branch (see CONTRIBUTING.md).
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Code follows the module structure standards and Go style guide.
Wrote unit and integration tests.
Updated relevant documentation (docs/) or specification (x/<module>/spec/).
Added relevant godoc comments.
Provide a commit message to be used for the changelog entry in the PR description for review.
Re-reviewed Files changed in the Github PR explorer.
Review Codecov Report in the comment section below once CI passes.

codecov-commenter · 2023-03-27T07:25:40Z

Codecov Report

Merging #3346 (5ef8854) into main (ae27fa5) will decrease coverage by 0.07%.
The diff coverage is 60.97%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3346      +/-   ##
==========================================
- Coverage   78.66%   78.59%   -0.07%     
==========================================
  Files         181      181              
  Lines       12511    12546      +35     
==========================================
+ Hits         9842     9861      +19     
- Misses       2238     2252      +14     
- Partials      431      433       +2

Impacted Files	Coverage Δ
modules/core/04-channel/keeper/grpc_query.go	`85.03% <60.97%> (-3.22%)`	⬇️

... and 1 file with indirect coverage changes

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

…r-only-works-for-unordered-channels

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

charleenfei · 2023-03-27T14:12:12Z

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

+				// are present. 2 is received so 7, 9, 10 should be 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)


maybe the NextSequenceRecv parameter and the expSeq/packetCommitments variables can be set up as well generally as variables for the test which are then malleated per test?

expSeq is already set up as so (If i'm understanding the comment correctly) and the rationale I got was that it's done since we need to assert its value against the expected in the main test body. Let me know if I got something wrong here.

ah okay, i was just seeing the packet commitment sequences getting declared/set on the tests as well as the sequence number, so i was thinking to just set them as variables instead for readability -- but i am not very strongly opinionated, i agree that expSeq is the most important as we are testing for that value as the expected one. thanks!

chatton

Looks great nice job! Just had a few suggestions 👍

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

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

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

crodriguezvega

Nice work, @DimitrisJim!

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

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

DimitrisJim · 2023-03-28T07:31:53Z

Test failure appears to be #3264.

chatton

LGTM nice work 👍

charleenfei

nice work, lgtm :)

…r-only-works-for-unordered-channels

DimitrisJim · 2023-03-29T11:54:20Z

Thanks all for the thorough reviews!

colin-axner

Thank you @DimitrisJim! Everything looks great to me. I left several code improvement suggestions, most of which are modifying existing code/comments. Perfectly fine with if they are not all applied and/or modified. Thanks for adding all the tests!

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

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

colin-axner

ACK, thanks @DimitrisJim!

…r-only-works-for-unordered-channels

…3606) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts * fix links * add changelog --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…3607) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts * fix links * add changelog --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…3608) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts * add changelog --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…3609) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts * fix version of golangci * add exclude rule * add exclude rule * add changelog --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…3610) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts * add changelog --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…3611) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix dead links and version of golangci * fix conflicts * add exclude rule * add changelod --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…3612) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * add changelog * add import * use sdkerrors --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…3613) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts * add changelog * fix version golangci --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

…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]>

…3629) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

) (cosmos#3607) * fix: handle ordered packets in `UnreceivedPackets` query Co-authored-by: Cian Hatton <[email protected]> (cherry picked from commit c77f80f) * fix conflicts * fix links * add changelog --------- Co-authored-by: Jim Fasarakis-Hilliard <[email protected]> Co-authored-by: Carlos Rodriguez <[email protected]>

fix: handle ordered packets in UnreceivedPackets query.

0fbb2a6

DimitrisJim requested review from colin-axner, AdityaSripal and damiannolan as code owners March 27, 2023 07:18

DimitrisJim linked an issue Mar 27, 2023 that may be closed by this pull request

UnreceivedPackets grpc query handler only works for unordered channels #1532

Closed

3 tasks

DimitrisJim commented Mar 27, 2023

View reviewed changes

modules/core/04-channel/keeper/grpc_query.go Outdated Show resolved Hide resolved

Merge branch 'main' into jim/1532-unreceivedpackets-grpc-query-handle…

8a992b5

…r-only-works-for-unordered-channels

charleenfei reviewed Mar 27, 2023

View reviewed changes

modules/core/04-channel/keeper/grpc_query_test.go Show resolved Hide resolved

charleenfei reviewed Mar 27, 2023

View reviewed changes

chatton reviewed Mar 27, 2023

View reviewed changes

modules/core/04-channel/keeper/grpc_query.go Outdated Show resolved Hide resolved

modules/core/04-channel/keeper/grpc_query_test.go Outdated Show resolved Hide resolved

modules/core/04-channel/keeper/grpc_query.go Outdated Show resolved Hide resolved

damiannolan reviewed Mar 27, 2023

View reviewed changes

modules/core/04-channel/keeper/grpc_query.go Outdated Show resolved Hide resolved

crodriguezvega reviewed Mar 27, 2023

View reviewed changes

modules/core/04-channel/keeper/grpc_query_test.go Outdated Show resolved Hide resolved

modules/core/04-channel/keeper/grpc_query_test.go Outdated Show resolved Hide resolved

modules/core/04-channel/keeper/grpc_query.go Show resolved Hide resolved

address review comments.

c4bb39e

chatton approved these changes Mar 28, 2023

View reviewed changes

charleenfei approved these changes Mar 28, 2023

View reviewed changes

DimitrisJim added 2 commits March 28, 2023 19:36

Merge branch 'main' into jim/1532-unreceivedpackets-grpc-query-handle…

c2944a3

…r-only-works-for-unordered-channels

Merge branch 'main' into jim/1532-unreceivedpackets-grpc-query-handle…

9af0d6e

…r-only-works-for-unordered-channels

colin-axner approved these changes Mar 30, 2023

View reviewed changes

DimitrisJim added 2 commits March 30, 2023 14:35

address review comments.

92c9d76

comment for the issue.

5ef8854

DimitrisJim mentioned this pull request Mar 30, 2023

Prefer declaration over initialization in grpc query handlers #3376

Closed

3 tasks

DimitrisJim requested a review from colin-axner March 30, 2023 11:56

DimitrisJim commented Mar 30, 2023

View reviewed changes

modules/core/04-channel/keeper/grpc_query_test.go Show resolved Hide resolved

colin-axner approved these changes Mar 30, 2023

View reviewed changes

Merge branch 'main' into jim/1532-unreceivedpackets-grpc-query-handle…

502cfeb

…r-only-works-for-unordered-channels

crodriguezvega merged commit c77f80f into main Mar 30, 2023

crodriguezvega deleted the jim/1532-unreceivedpackets-grpc-query-handler-only-works-for-unordered-channels branch March 30, 2023 18:55

colin-axner mentioned this pull request May 22, 2023

Add e2e test for clearing packet queues #3626

Open

3 tasks

crodriguezvega added the backport-to-v4.1.x label May 23, 2023

mergify bot mentioned this pull request May 23, 2023

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

Merged

This was referenced May 25, 2023

fix: huckleberry on v3.4.0 (backport) crescent-network/ibc-go#3

Merged

build: bump ibc-go to v3.4.0-crescent-v4-3 for huckleberry patch crescent-network/crescent#155

Merged

BrandonWeng mentioned this pull request May 26, 2023

Huckleberry patch for v3.0.0 sei-protocol/sei-ibc-go#22

Merged

ulbqb mentioned this pull request May 30, 2023

Handle ordered packets in UnreceivedPackets query. Finschia/ibc-go#15

Merged

5 tasks

faddat mentioned this pull request Jun 23, 2023

v7.2.0 for sdk 50 #3948

Closed

9 tasks

ulbqb mentioned this pull request Jul 31, 2023

Bump up to v4.3.1 Finschia/ibc-go#19

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle ordered packets in UnreceivedPackets query. #3346

Handle ordered packets in UnreceivedPackets query. #3346

DimitrisJim commented Mar 27, 2023 •

edited by crodriguezvega

Loading

codecov-commenter commented Mar 27, 2023 •

edited

Loading

charleenfei Mar 27, 2023 •

edited

Loading

DimitrisJim Mar 27, 2023 •

edited

Loading

charleenfei Mar 28, 2023

chatton left a comment

crodriguezvega left a comment

DimitrisJim commented Mar 28, 2023

chatton left a comment

charleenfei left a comment

DimitrisJim commented Mar 29, 2023 •

edited

Loading

colin-axner left a comment

colin-axner left a comment

Handle ordered packets in UnreceivedPackets query. #3346

Handle ordered packets in UnreceivedPackets query. #3346

Conversation

DimitrisJim commented Mar 27, 2023 • edited by crodriguezvega Loading

Description

Commit Message / Changelog Entry

codecov-commenter commented Mar 27, 2023 • edited Loading

Codecov Report

charleenfei Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

DimitrisJim Mar 27, 2023 • edited Loading

Choose a reason for hiding this comment

charleenfei Mar 28, 2023

Choose a reason for hiding this comment

chatton left a comment

Choose a reason for hiding this comment

crodriguezvega left a comment

Choose a reason for hiding this comment

DimitrisJim commented Mar 28, 2023

chatton left a comment

Choose a reason for hiding this comment

charleenfei left a comment

Choose a reason for hiding this comment

DimitrisJim commented Mar 29, 2023 • edited Loading

colin-axner left a comment

Choose a reason for hiding this comment

colin-axner left a comment

Choose a reason for hiding this comment

DimitrisJim commented Mar 27, 2023 •

edited by crodriguezvega

Loading

codecov-commenter commented Mar 27, 2023 •

edited

Loading

charleenfei Mar 27, 2023 •

edited

Loading

DimitrisJim Mar 27, 2023 •

edited

Loading

DimitrisJim commented Mar 29, 2023 •

edited

Loading