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

Some (g)RPC queries are missing proper pagination #811

Closed
9 of 16 tasks
ancazamfir opened this issue Apr 9, 2021 · 0 comments · Fixed by #827
Closed
9 of 16 tasks

Some (g)RPC queries are missing proper pagination #811

ancazamfir opened this issue Apr 9, 2021 · 0 comments · Fixed by #827
Assignees
@ancazamfir
Labels
I: rpc Internal: related to (g)RPC
Milestone
04.2021

Comments

@ancazamfir
Copy link
Collaborator

ancazamfir commented Apr 9, 2021 •
edited
Loading

Crate

relayer

Summary of Bug

all (g)RPC queries that return multiple items are missing proper pagination

Version

Steps to Reproduce

  • Send > 100 packets:
    $ hermes tx raw ft-transfer ibc-0 ibc-1 transfer channel-0 9999 15 -n 500
  • Query packet commitments will return less than 500
    hermes query packet unreceived-packets ibc-1 ibc-0 transfer channel-0

Found this while I was debugging why hermes start doesn't clear all packets on start.

Acceptance Criteria

All gRPC queries that return collections should be looked at. Here is a list (needs to be updated after we release v0.2.0:

  • query_clients
  • query_client_connections - proto QueryClientConnectionsRequest doesn't have pagination
  • query_connections
  • query_connection_channels
  • query_channels
  • query_packet_commitments
  • query_unreceived_packets- proto QueryUnreceivedPacketsRequest doesn't have pagination - not needed as we ask for specific sequences
  • query_packet_acknowledgements
  • query_unreceived_acknowledgements - proto QueryUnreceivedAcksRequest doesn't have pagination - not needed as we ask for specific sequences
  • query_consensus_states

This is using tendermint rpc and should also be fixed:

  • query_txs

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@ancazamfir ancazamfir added this to the 04.2021 milestone Apr 9, 2021
@ancazamfir ancazamfir mentioned this issue Apr 9, 2021
Merged
5 tasks
romac added a commit that referenced this issue Apr 9, 2021
@romac
Set pagination limit to u64::MAX for all queries that support it to e…
d7293dd 
…nsure we get all results at once

See #811
romac added a commit that referenced this issue Apr 9, 2021
@ancazamfir @romac
Misbehavior monitoring and handling (#691)
ff0e6e5 
* Split client_def in state, consensus, header. Add sketch for Tx decoding.

* Added misbehaviour message, structures, relayer submission

* tentative try with interim update event from cosmos PR

* trying to decode the header from event

* decode the header in client update event

* Added restart support, move some of the misbehaviour detection in light client, reorg

* fix merge

* add support for potential on-chain state pruning, cleanup

* Update changelog

* sketch for time travelling lunatic attack check

* fmt

* Decode misbehaviour events, added comments

* Update to reflect bad updates in the middle

* cargo fmt

* Cleanup consensus query and allow query for all

* Allow target and trusted heights to be specified in client updates

* Allow update with any existing trusted height

* Disallow create client with same src and dst

* Fix tests

* Added mock misbehaviour

* Fix CI and reorg code

* Cleanup

* Updates after sitdown review

* Update comments

* Flip back the logic in compatible_headers()

* Rename client_misbehaviour to misbehaviour

* review comments

* fmt

* Return UpdateClient::consensus_height by copy instead of reference

* Small cleanup in LightClient::build_misbehaviour

* Small cleanup in chain::cosmos

* Set pagination limit to u64::MAX for all queries that support it to ensure we get all results at once

See #811

* Addressed review comments

Co-authored-by: Romain Ruetschi <romain@informal.systems>
@romac romac changed the title some (g)RPC queries are missing proper pagination Some (g)RPC queries are missing proper pagination Apr 12, 2021
@ancazamfir ancazamfir mentioned this issue Apr 14, 2021
Closed
3 tasks
@romac romac added the I: rpc Internal: related to (g)RPC label Apr 15, 2021
@ancazamfir ancazamfir self-assigned this Apr 15, 2021
@ancazamfir ancazamfir mentioned this issue Apr 15, 2021
Merged
5 tasks
@ancazamfir ancazamfir mentioned this issue May 6, 2021
Merged
26 tasks
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this issue Sep 13, 2022
@ancazamfir @romac
Misbehavior monitoring and handling (informalsystems#691)
e195b50 
* Split client_def in state, consensus, header. Add sketch for Tx decoding.

* Added misbehaviour message, structures, relayer submission

* tentative try with interim update event from cosmos PR

* trying to decode the header from event

* decode the header in client update event

* Added restart support, move some of the misbehaviour detection in light client, reorg

* fix merge

* add support for potential on-chain state pruning, cleanup

* Update changelog

* sketch for time travelling lunatic attack check

* fmt

* Decode misbehaviour events, added comments

* Update to reflect bad updates in the middle

* cargo fmt

* Cleanup consensus query and allow query for all

* Allow target and trusted heights to be specified in client updates

* Allow update with any existing trusted height

* Disallow create client with same src and dst

* Fix tests

* Added mock misbehaviour

* Fix CI and reorg code

* Cleanup

* Updates after sitdown review

* Update comments

* Flip back the logic in compatible_headers()

* Rename client_misbehaviour to misbehaviour

* review comments

* fmt

* Return UpdateClient::consensus_height by copy instead of reference

* Small cleanup in LightClient::build_misbehaviour

* Small cleanup in chain::cosmos

* Set pagination limit to u64::MAX for all queries that support it to ensure we get all results at once

See informalsystems#811

* Addressed review comments

Co-authored-by: Romain Ruetschi <romain@informal.systems>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ancazamfir ancazamfir

Labels
I: rpc Internal: related to (g)RPC
Projects
None yet
Milestone
04.2021
Development

Successfully merging a pull request may close this issue.

Fix pagination for channels cli and reorg the query_txs() informalsystems/hermes
2 participants
@romac @ancazamfir