Implement query for client connections

[andynog]

Closes: #151

Description

Implemented query for client connections. Response is parsed as array of strings (which is the ABCI query result).

---

For contributor use:

• Unit tests written
• Added test to CI if applicable
• Updated CHANGELOG_PENDING.md
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Updated relevant documentation (docs/) and code comments
• Re-reviewed Files changed in the Github PR explorer

[greg-szabo]

Ok, this might be a very high level feedback, but running the command gives me an empty value. (I'm running simd in docker on the side.)

cargo run --bin relayer -- -c relayer/relay/tests/config/fixtures/simple_config.toml query client connections ibc-test ethbridge --height 3 -p false

query client connections error Empty response value

[greg-szabo]

Overall looks good, minor notes and requests here and there.

At the testing code, somehow I expected that the command itself will be tested. We should implement some kind of integration testing for these.

Not in this PR, though, that would be unfair to the scope here.

[greg-szabo]

I just noticed that you implemented the TryFromRaw in the obsolete query.rs. Please clean up the unused code and move things around like you can see in the connection and channel code.

[andynog]

Ok, this might be a very high level feedback, but running the command gives me an empty value. (I'm running simd in docker on the side.)

cargo run --bin relayer -- -c relayer/relay/tests/config/fixtures/simple_config.toml query client connections ibc-test ethbridge --height 3 -p false

query client connections error Empty response value

Using the simapp version in this branch, the command I've used to get the client connections is:

cargo run --bin relayer -- -c ./relayer/relay/tests/config/fixtures/relayer_conf_example.toml query client connections chain_A clientidone -h 4 -p false

the chain_id used is chain_A and client_id clientidone.

[greg-szabo]

Ah cool, thanks! I was a bit lost there. Can you update the comment in the code to reflect that?
I know it's hairsplitting but I'm going to use that comment to write our first tests for testing queries and it helps me tremendously if the comment is correct.

[adizere]

If we supply a height that is too advanced, the reported error is a bit misleading saying something along the lines of proof is unexpectedly empty; ensure height has not been pruned:. See below:

     Running `target/debug/relayer -c ./relayer/relay/tests/config/fixtures/relayer_conf_example.toml query client connections chain_A clientidone -h 1000`
     Options QueryClientConnectionsOptions { client_id: ClientId("clientidone"), height: 1000, proof: true }
query client connections error RPC error: proof is unexpectedly empty; ensure height has not been pruned: invalid request

[andynog]

If we supply a height that is too advanced, the reported error is a bit misleading saying something along the lines of proof is unexpectedly empty; ensure height has not been pruned:. See below:

     Running `target/debug/relayer -c ./relayer/relay/tests/config/fixtures/relayer_conf_example.toml query client connections chain_A clientidone -h 1000`
     Options QueryClientConnectionsOptions { client_id: ClientId("clientidone"), height: 1000, proof: true }
query client connections error RPC error: proof is unexpectedly empty; ensure height has not been pruned: invalid request

Actually noticed this on another issue and provided comments. I think someone mentioned that has to do with pruning I believe but can't find that thread.

[greg-szabo]

Hint: the file modules/src/ics02_client/query.rs should completely disappear after this PR. Ping me if you need any pointers on how to achieve that.

[adizere]

Actually noticed this on another issue and provided comments. I think someone mentioned that has to do with pruning I believe but can't find that thread.

I'm not sure this is about pruning. Pruning would interfere with a query if the supplied height -h would be very old (which was pruned). But in this case I was supplying a height (1000) that does not exist yet (it's a height in the future). At the least, I think the error message is not right.

query client connections error RPC error: proof is unexpectedly empty; ensure height has not been pruned: invalid request

But there might a bug in the validation/handling.

[andynog]

I just noticed that you implemented the TryFromRaw in the obsolete query.rs. Please clean up the unused code and move things around like you can see in the connection and channel code.

This is fixed in 1ca727d

[greg-szabo]

Last drift, I promise.

[codecov-commenter]

Codecov Report

Merging #169 into master will increase coverage by 2.1%.
The diff coverage is 19.6%.

@@           Coverage Diff            @@
##           master    #169     +/-   ##
========================================
+ Coverage    13.6%   15.7%   +2.1%     
========================================
  Files          69      59     -10     
  Lines        3752    3823     +71     
  Branches     1374    1480    +106     
========================================
+ Hits          513     604     +91     
+ Misses       2618    2475    -143     
- Partials      621     744    +123     

Impacted Files	Coverage Δ
modules/src/events.rs	0.0% <ø> (ø)
modules/src/ics02_client/client.rs	0.0% <0.0%> (ø)
modules/src/ics02_client/client_type.rs	47.6% <ø> (ø)
modules/src/ics03_connection/error.rs	25.0% <0.0%> (-8.4%)	⬇️
modules/src/ics03_connection/exported.rs	42.1% <0.0%> (-9.6%)	⬇️
modules/src/ics04_channel/error.rs	23.0% <0.0%> (-2.0%)	⬇️
modules/src/ics04_channel/packet.rs	0.0% <0.0%> (ø)
modules/src/ics07_tendermint/error.rs	33.3% <0.0%> (+33.3%)	⬆️
modules/src/ics07_tendermint/msgs/create_client.rs	0.0% <0.0%> (ø)
modules/src/ics23_commitment/mod.rs	0.0% <0.0%> (ø)
... and 47 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 021cdc4...3d3a2e0. Read the comment docs.