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.

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

Fix Client queries after last protobuf updates #309

Merged
merged 5 commits into from
Oct 15, 2020
Merged

Conversation

ancazamfir
Copy link
Collaborator

Closes: cosmos/ibc-rs#106

Description

Fixes the type-urls to be consistent with latest protos which are used in stargate-4.
Added constants for the client related URLs. These may change with fix for #254


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

@ancazamfir
Copy link
Collaborator Author

Tested the client queries manually.

$ rrly -c simple_config.toml query client state ibc0 ibconeclient
    Finished dev [unoptimized + debuginfo] target(s) in 0.31s
     Running `target/debug/relayer -c simple_config.toml query client state ibc0 ibconeclient`
     Options QueryClientStateOptions { client_id: ClientId("ibconeclient"), height: 0, proof: true }
[relayer/src/chain/cosmos.rs:65] "Todo: implement proof verification." = "Todo: implement proof verification."
client state query result:  Tendermint(ClientState { chain_id: "ibc1", trusting_period: 1209600s, unbonding_period: 1814400s, max_clock_drift: 600s, frozen_height: Height { version_number: 0, version_height: 0 }, latest_height: Height { version_number: 0, version_height: 34 }, upgrade_path: "upgrade/upgradedClient", allow_update_after_expiry: false, allow_update_after_misbehaviour: false })

$ rrly -c simple_config.toml query client consensus ibc0 ibconeclient 0 34
    Finished dev [unoptimized + debuginfo] target(s) in 0.17s
     Running `target/debug/relayer -c simple_config.toml query client consensus ibc0 ibconeclient 0 34`
     Options QueryClientConsensusOptions { client_id: ClientId("ibconeclient"), consensus_epoch: 0, consensus_height: 34, height: 0, proof: true }
[relayer/src/chain/cosmos.rs:65] "Todo: implement proof verification." = "Todo: implement proof verification."
client consensus state query result:  Tendermint(ConsensusState { timestamp: Time(2020-10-14T10:07:21.778102Z), root: CommitmentRoot([126, 130, 139, 102, 89, 141, 92, 76, 215, 119, 177, 64, 230, 228, 79, 125, 21, 58, 215, 160, 59, 167, 17, 196, 224, 107, 245, 134, 6, 183, 17, 140]), next_validators_hash: Hash::Sha256(44C5CB324AB86ADEE044B8CC0A569608A85AF549C0487405442843E2D88AF99A) })

@codecov-io
Copy link

codecov-io commented Oct 14, 2020

Codecov Report

Merging #309 into master will increase coverage by 23.8%.
The diff coverage is 61.7%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    informalsystems/hermes#309      +/-   ##
=========================================
+ Coverage    13.6%   37.5%   +23.8%     
=========================================
  Files          69     124      +55     
  Lines        3752    7993    +4241     
  Branches     1374    2770    +1396     
=========================================
+ Hits          513    3001    +2488     
- Misses       2618    4753    +2135     
+ Partials      621     239     -382     
Impacted Files Coverage Δ
modules/src/events.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/events.rs 0.0% <ø> (ø)
modules/src/ics02_client/raw.rs 0.0% <0.0%> (ø)
modules/src/ics03_connection/context.rs 0.0% <0.0%> (ø)
modules/src/ics03_connection/error.rs 16.6% <0.0%> (-16.7%) ⬇️
modules/src/ics04_channel/error.rs 75.0% <ø> (+50.0%) ⬆️
modules/src/ics04_channel/packet.rs 0.0% <0.0%> (ø)
modules/src/ics07_tendermint/client_def.rs 0.0% <0.0%> (ø)
modules/src/ics07_tendermint/error.rs 75.0% <ø> (+75.0%) ⬆️
modules/src/ics18_relayer/error.rs 0.0% <0.0%> (ø)
... and 226 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 c8a7c65...9f66182. Read the comment docs.

modules/src/ics02_client/msgs.rs Outdated Show resolved Hide resolved
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for the last changes.

@ancazamfir ancazamfir requested a review from romac October 15, 2020 08:28
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just minor comments & remarks. Looks ready!

modules/src/ics02_client/context.rs Show resolved Hide resolved
modules/src/ics02_client/error.rs Outdated Show resolved Hide resolved
modules/src/ics02_client/handler/create_client.rs Outdated Show resolved Hide resolved
@adizere adizere mentioned this pull request Oct 15, 2020
6 tasks
Copy link
Member

@romac romac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I just have one question inline below.

modules/src/ics02_client/client_def.rs Show resolved Hide resolved
@romac romac self-requested a review October 15, 2020 11:55
@ancazamfir ancazamfir merged commit a3e2834 into master Oct 15, 2020
@ancazamfir ancazamfir deleted the anca/query_bugs branch October 15, 2020 12:00
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* fix type-urls, introduce constants

* Remove client_type from MsgCreateAnyClient, initial cleanup of tests and mock context

* cleanup

* Remove some unwraps

* Adi's review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants