Fetch consensus state heights using QueryConsensusStateHeights

[romac]

Closes: #2001

Description

Fetch consensus state heights using the more efficient QueryConsensusStateHeights gRPC query instead of fetching all the consensus states themselves using QueryConsensusStates and extracting the heights from the result.

TODO

• Remove the sorting of the results in a few places (indicated with TODOs in the code)
• Do we need deal with the case where QueryConsensusStateHeights is unsupported, and fallback on QueryConsensusStates?

Note

This request is available in the following IBC-Go versions:

• >= v3.1.0
• >= v2.3.0
• >= v1.5.0

---

PR author checklist:

• Added changelog entry, using unclog.
• Added tests: integration (for Hermes) or unit/mock tests (for modules).
• Linked to GitHub issue.
• Updated code comments and documentation (e.g., docs/).
• Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

• Reviewed Files changed in the GitHub PR explorer.
• Manually tested (in case integration/unit/mock tests are absent).

[romac]

Looks like the Osmosis integration test is failing with:

2022-12-20T15:04:23.674764Z  WARN foreign_client.wait_and_build_update_client_with_trusted{client=ibc-2->ibc-1:07-tendermint-0 target_height=2-7}:foreign_client.build_update_client_with_trusted{client=ibc-2->ibc-1:07-tendermint-0 target_height=2-7}:foreign_client.solve_trusted_height{client=ibc-2->ibc-1:07-tendermint-0 target_height=2-7}: ibc_relayer::foreign_client: resolving trusted height from the full list of consensus state heights for target height 2-7; this may take a while

2022-12-20T15:04:23.678283Z ERROR ibc_test_framework::util::suspend: test failure occured. set HANG_ON_FAIL=1 to suspend the test on failure for debugging: connection error

Caused by:
   0: failed during an operation on client '07-tendermint-0' hosted by chain 'ibc-1'
   1: chain ibc-1 is missing trusted state smaller than target height 2-7

Will see if I can test with a newer version of Osmosis

[romac]

The wasmd version we are testing against apparently does not support this query:

2022-12-20T14:43:08.999978Z  WARN foreign_client.wait_and_build_update_client_with_trusted{client=ibc-2->ibc-1:07-tendermint-0 target_height=2-6}:foreign_client.build_update_client_with_trusted{client=ibc-2->ibc-1:07-tendermint-0 target_height=2-6}:foreign_client.solve_trusted_height{client=ibc-2->ibc-1:07-tendermint-0 target_height=2-6}: ibc_relayer::foreign_client: resolving trusted height from the full list of consensus state heights for target height 2-6; this may take a while

2022-12-20T14:43:09.003468Z ERROR ibc_test_framework::util::suspend: test failure occured. set HANG_ON_FAIL=1 to suspend the test on failure for debugging: connection error

Caused by:
   0: failed during an operation on client '07-tendermint-0' hosted by chain 'ibc-1'
   1: failed while querying for client 07-tendermint-0 on chain id ibc-2
   2: gRPC call failed with status: status: Unimplemented, message: "unknown method ConsensusStateHeights for service ibc.core.client.v1.Query", details: [], metadata: MetadataMap { headers: {"content-type": "application/grpc"} }

Will see if I can update it as well, and perhaps add a fallback to the full query.

[romac]

Looks like specifying reverse = true on the consensus states and heights gRPC queries results in an empty response…

[romac]

Looks like specifying reverse = true on the consensus states and heights gRPC queries results in an empty response…

I can reproduce this using simd. We get an empty results when we use limit = u64::MAX in combination with reverse = true:

Query consensus state heights with default parameters

$ simd --node tcp://localhost:27000 query ibc client consensus-state-heights 07-tendermint-0
consensus_state_heights:
- revision_height: "13"
  revision_number: "1"
- revision_height: "15"
  revision_number: "1"
- revision_height: "17"
  revision_number: "1"
- revision_height: "18"
  revision_number: "1"
- revision_height: "19"
  revision_number: "1"
- revision_height: "20"
  revision_number: "1"
pagination:
  next_key: null
  total: "0"

Query consensus state heights in reverse (lexicographic) order

simd --node tcp://localhost:27000 query ibc client consensus-state-heights 07-tendermint-0 --reverse
consensus_state_heights:
- revision_height: "20"
  revision_number: "1"
- revision_height: "19"
  revision_number: "1"
- revision_height: "18"
  revision_number: "1"
- revision_height: "17"
  revision_number: "1"
- revision_height: "15"
  revision_number: "1"
- revision_height: "13"
  revision_number: "1"
pagination:
  next_key: null
  total: "0"

Query consensus state heights in default order with a limit of u64::MAX

simd --node tcp://localhost:27000 query ibc client consensus-state-heights 07-tendermint-0 --limit 18446744073709551615
consensus_state_heights:
- revision_height: "13"
  revision_number: "1"
- revision_height: "15"
  revision_number: "1"
- revision_height: "17"
  revision_number: "1"
- revision_height: "18"
  revision_number: "1"
- revision_height: "19"
  revision_number: "1"
- revision_height: "20"
  revision_number: "1"
pagination:
  next_key: null
  total: "0"

Query consensus state heights in reverse (lexicographic) order with a limit of u64::MAX

simd --node tcp://localhost:27000 query ibc client consensus-state-heights 07-tendermint-0 --reverse --limit 18446744073709551615
consensus_state_heights: []
pagination:
  next_key: MS0yMC9wcm9jZXNzZWRUaW1l
  total: "0"

Workaround

I found that using any limit smaller than u64::MAX yields the expected results.

Proposed solution

Since the results are sorted in lexicographic order instead of numeric order, we cannot rely on reverse to give us the results in the order we want and have to sort them ourselves regardless. It therefore does not make sense to specify reverse = true.

I will also change the default limit to u32::MAX instead of u64::MAX to hopefully avoid such behavior in the future if we make use of more PageRequest options.

[ancazamfir]

great work! thanks @romac