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

Implemented GRPC client to query account sequence #361

Merged
merged 13 commits into from
Nov 6, 2020

Conversation

andynog
Copy link
Contributor

@andynog andynog commented Nov 4, 2020

Closes: cosmos/ibc-proto-rs#18

Description

This PR has the logic to start using the GRPC service from a Cosmos chain to retrieve account information. The account has the sequence number that is needed to submit a transaction. With this implementation there is no need anymore to manually query the account sequence in order to submit a transaction. Changes implemented:

  • Added logic to proto_compiler to generate the GRPC client (using tonic
  • Added logic to retrieve the GRPC address from the configuration file
  • Added method to Cosmos chain to query_account that leverages the GRPC client
  • Removed the old account sequence parameter from the tx raw command
  • Updated instructions on Relayer CLI README to remove the account sequence instructions

For contributor use:

  • Updated the Unreleased section of CHANGELOG.md with the issue.
  • If applicable: Unit tests written, added test to CI.
  • 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.

@andynog
Copy link
Contributor Author

andynog commented Nov 5, 2020

Just noticed there a lot of file changes but a lot of them are proto files that were updated and after including tonic seems it also formatted the files.

@codecov-io
Copy link

codecov-io commented Nov 5, 2020

Codecov Report

Merging #361 into master will increase coverage by 20.7%.
The diff coverage is 64.5%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    informalsystems/hermes#361      +/-   ##
=========================================
+ Coverage    13.6%   34.4%   +20.7%     
=========================================
  Files          69     144      +75     
  Lines        3752    9085    +5333     
  Branches     1374    3066    +1692     
=========================================
+ Hits          513    3131    +2618     
- Misses       2618    5713    +3095     
+ Partials      621     241     -380     
Impacted Files Coverage Δ
modules/src/events.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/events.rs 0.0% <ø> (ø)
modules/src/ics02_client/msgs.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/raw.rs 0.0% <0.0%> (ø)
modules/src/ics03_connection/error.rs 13.6% <0.0%> (-19.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 266 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 5c3dbe2...2bc8b03. Read the comment docs.

@andynog
Copy link
Contributor Author

andynog commented Nov 5, 2020

There might be an additional change required which is the cosmos address from the key_seed.json. Right now it has a hard coded address. I will look into this tomorrow.

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.

LGTM but will defer to @ancazamfir for the final approbation. Great stuff @andynog!

Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks Andy!!
Please update the changelog and pin the protos to sdk v0.40.0-rc0 (commit 15324920548c2629e51d837bcefc1cbc40797c5d)

@andynog
Copy link
Contributor Author

andynog commented Nov 5, 2020

@ancazamfir, I've merged #365 and there were a lot of merging conflicts that I've resolved to the best of my knowledge. Since there were not only changes related to the proto compiler but also for the tx commands maybe from other commits to master, I had to resolve these conflicts. The only weird thing is that if I run a conn-init tx like I've been testing, using the dev-env I cannot see the connection created and the output of the command is Success client updated: [ ]. So not sure something changed or it's something wrong with the merging.

Could you please test the txs with this branch and see if still makes sense for you ?

Here's what I've tested

cargo run --bin relayer -- -c ./relayer-cli/tests/fixtures/two_chains.toml tx raw conn-init ibc0 ibc1 ibczeroclient ibconeclient ibczeroconn246 -k key
_seed.json

and I get

 Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/relayer -c ./relayer-cli/tests/fixtures/two_chains.toml tx raw conn-init ibc0 ibc1 ibczeroclient ibconeclient ibczeroconn246 -k key_seed.json`
     Message ConnectionOpenInitOptions { dest_chain_config: ChainConfig { id: chain::Id(ibc0), rpc_addr: Tcp { peer_id: None, host: "localhost", port: 26657 }, grpc_addr: "tcp://localhost:9090", account_prefix: "cosmos", key_name: "testkey", store_prefix: "ibc", client_ids: ["cla1", "cla2"], gas: 200000, clock_drift: 5s, trusting_period: 1209600s, trust_threshold: TrustThresholdFraction { numerator: 1, denominator: 3 }, peers: Some(PeersConfig { primary: node::Id(E0DE6C540C65FC9059660A7A8535F70048A94539), light_clients: [LightClientConfig { peer_id: node::Id(E0DE6C540C65FC9059660A7A8535F70048A94539), address: Tcp { peer_id: None, host: "localhost", port: 26657 }, trusted_header_hash: Hash::Sha256(169F8F6318B8FAABDBA128AD1689E238566B69DDBD2B36F1911C0DFCA73FD401), trusted_height: block::Height(7806) }] }) }, src_chain_config: ChainConfig { id: chain::Id(ibc1), rpc_addr: Tcp { peer_id: None, host: "localhost", port: 26557 }, grpc_addr: "tcp://localhost:9091", account_prefix: "cosmos", key_name: "testkey", store_prefix: "ibc", client_ids: ["clb1"], gas: 200000, clock_drift: 5s, trusting_period: 1209600s, trust_threshold: TrustThresholdFraction { numerator: 1, denominator: 3 }, peers: Some(PeersConfig { primary: node::Id(356DB878F398BA707DBE69950067C8E6471D8948), light_clients: [LightClientConfig { peer_id: node::Id(356DB878F398BA707DBE69950067C8E6471D8948), address: Tcp { peer_id: None, host: "localhost", port: 26557 }, trusted_header_hash: Hash::Sha256(CC291E79B2E2068984EB13BBF420B4F4AE95357E25B9BA0A30CA26FF27AF3C75), trusted_height: block::Height(7741) }] }) }, dest_client_id: ClientId("ibczeroclient"), src_client_id: ClientId("ibconeclient"), dest_connection_id: ConnectionId("ibczeroconn246"), src_connection_id: None, signer_seed: "{\"name\":\"user\",\"type\":\"local\",\"address\":\"cosmos1x09nchne8s9wya2nwl0erhmf4qz4uhwkx2gyu9\",\"pubkey\":\"cosmospub1addwnpepqf029w068elsesh3s35chgze5m4exvkmrqjrrgakav85c7y4sl0aq4727f3\",\"mnemonic\":\"[MNEMONIC]t\"}\n" }
     Success client updated: []

Copy link
Collaborator

@ancazamfir ancazamfir 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! Thanks Andy!

@ancazamfir ancazamfir merged commit 6c933e9 into master Nov 6, 2020
@ancazamfir ancazamfir deleted the andy/acct-sequence branch November 6, 2020 11:39
romac pushed a commit that referenced this pull request Nov 24, 2020
* Added logic to generate GRPC client from cosmos.auth proto (#337)

* Adding logic to use GRPC client (#337)

* Grpc client connection retrieves account sequence (#337)

* Removed the account sequence flag from the tx raw commands (#337)

* Removed instructions to query and specify account sequence from tx raw command (#337)

* Logic to fetch GRPC endpoint address from config (#337)

* Fixing tests (#361)

* Logic to use the address from the key seed (#337)

* Added boilerplate code for a keys add command to the relayer (#363)

* Removing key flag from tx cmds

* Adding logic to get key specified in the config

* Logic to get the key specified in the config (#363)

* Removed the -k flag from the tx raw commands (#363)

* More logic to add key command (#363)

* key add command for memory store working (#363)

* Added logic to persist key seed in 'home' folder (#363)

* Changes implemented (#363):
- Added a new test keyring backend to support adding keys to file system (under home folder)
- Refactored logic to add key to be part of the keystore and not the command
- Switched the keybase on a chain to use the test keyring
- Key seed file is saved in the test keystore default folder (/home/andy/.rrly)

* Logic to use the key_name parameter from the config to add key. Removed name parameter from keys add cmd (#363)

* Changed the logic to get the key from the test keyring file store (#363)

* Implemented changes: (#363)
- Clean up remaining key_seed flag for tx cmds
- Refactored keybase to include chain config
- Refactoring keyring methods to use chain config
- Logic to use configured key to sign tx

* Updated the README instructions (#363)

* Disable the 'keys restore' command for now (#363)

* Added 'keys list' command to show key added on a chain (#363)

* Added entry for issue #363 (PR #408)

* Refactored the bound variables to use the full name per comment suggestion (#408)

* Move key retrieval, memo and timeout height inside send_tx

Co-authored-by: Anca Zamfir <zamfiranca@gmail.com>
adizere pushed a commit that referenced this pull request Nov 27, 2020
* Added logic to generate GRPC client from cosmos.auth proto (#337)

* Adding logic to use GRPC client (#337)

* Grpc client connection retrieves account sequence (#337)

* Removed the account sequence flag from the tx raw commands (#337)

* Removed instructions to query and specify account sequence from tx raw command (#337)

* Logic to fetch GRPC endpoint address from config (#337)

* Fixing tests (#361)

* Logic to use the address from the key seed (#337)

* Added boilerplate code for a keys add command to the relayer (#363)

* Removing key flag from tx cmds

* Adding logic to get key specified in the config

* Logic to get the key specified in the config (#363)

* Removed the -k flag from the tx raw commands (#363)

* More logic to add key command (#363)

* key add command for memory store working (#363)

* Added logic to persist key seed in 'home' folder (#363)

* Changes implemented (#363):
- Added a new test keyring backend to support adding keys to file system (under home folder)
- Refactored logic to add key to be part of the keystore and not the command
- Switched the keybase on a chain to use the test keyring
- Key seed file is saved in the test keystore default folder (/home/andy/.rrly)

* Logic to use the key_name parameter from the config to add key. Removed name parameter from keys add cmd (#363)

* Changed the logic to get the key from the test keyring file store (#363)

* Implemented changes: (#363)
- Clean up remaining key_seed flag for tx cmds
- Refactored keybase to include chain config
- Refactoring keyring methods to use chain config
- Logic to use configured key to sign tx

* Updated the README instructions (#363)

* Disable the 'keys restore' command for now (#363)

* Added 'keys list' command to show key added on a chain (#363)

* Added entry for issue #363 (PR #408)

* Refactored the bound variables to use the full name per comment suggestion (#408)

* Move key retrieval, memo and timeout height inside send_tx

* Add the client creation, connection and channel handshake

* remove sleeps

* More error handling, cleanup

* Macro for channel CLIs

* Macro for connection CLIs

* Where src/dst make no sense rename to a/b, also fix a few bugs after last commits

* cleanup

* cargo fmt

* Use Romain's skip-verif until backwards verification is done

* fix CLI bugs

Co-authored-by: Andy Nogueira <me@andynogueira.dev>
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* Added logic to generate GRPC client from cosmos.auth proto (#337)

* Adding logic to use GRPC client (#337)

* Grpc client connection retrieves account sequence (#337)

* Removed the account sequence flag from the tx raw commands (#337)

* Removed instructions to query and specify account sequence from tx raw command (#337)

* Logic to fetch GRPC endpoint address from config (#337)

* Fixing tests (informalsystems#361)

* Logic to use the address from the key seed (#337)

* Update readme

* Fix the tx output strings

* Fix update client

Co-authored-by: Anca Zamfir <zamfiranca@gmail.com>
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* Added logic to generate GRPC client from cosmos.auth proto (#337)

* Adding logic to use GRPC client (#337)

* Grpc client connection retrieves account sequence (#337)

* Removed the account sequence flag from the tx raw commands (#337)

* Removed instructions to query and specify account sequence from tx raw command (#337)

* Logic to fetch GRPC endpoint address from config (#337)

* Fixing tests (informalsystems#361)

* Logic to use the address from the key seed (#337)

* Added boilerplate code for a keys add command to the relayer (informalsystems#363)

* Removing key flag from tx cmds

* Adding logic to get key specified in the config

* Logic to get the key specified in the config (informalsystems#363)

* Removed the -k flag from the tx raw commands (informalsystems#363)

* More logic to add key command (informalsystems#363)

* key add command for memory store working (informalsystems#363)

* Added logic to persist key seed in 'home' folder (informalsystems#363)

* Changes implemented (informalsystems#363):
- Added a new test keyring backend to support adding keys to file system (under home folder)
- Refactored logic to add key to be part of the keystore and not the command
- Switched the keybase on a chain to use the test keyring
- Key seed file is saved in the test keystore default folder (/home/andy/.rrly)

* Logic to use the key_name parameter from the config to add key. Removed name parameter from keys add cmd (informalsystems#363)

* Changed the logic to get the key from the test keyring file store (informalsystems#363)

* Implemented changes: (informalsystems#363)
- Clean up remaining key_seed flag for tx cmds
- Refactored keybase to include chain config
- Refactoring keyring methods to use chain config
- Logic to use configured key to sign tx

* Updated the README instructions (informalsystems#363)

* Disable the 'keys restore' command for now (informalsystems#363)

* Added 'keys list' command to show key added on a chain (informalsystems#363)

* Added entry for issue informalsystems#363 (PR informalsystems#408)

* Refactored the bound variables to use the full name per comment suggestion (informalsystems#408)

* Move key retrieval, memo and timeout height inside send_tx

Co-authored-by: Anca Zamfir <zamfiranca@gmail.com>
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
* Added logic to generate GRPC client from cosmos.auth proto (#337)

* Adding logic to use GRPC client (#337)

* Grpc client connection retrieves account sequence (#337)

* Removed the account sequence flag from the tx raw commands (#337)

* Removed instructions to query and specify account sequence from tx raw command (#337)

* Logic to fetch GRPC endpoint address from config (#337)

* Fixing tests (informalsystems#361)

* Logic to use the address from the key seed (#337)

* Added boilerplate code for a keys add command to the relayer (informalsystems#363)

* Removing key flag from tx cmds

* Adding logic to get key specified in the config

* Logic to get the key specified in the config (informalsystems#363)

* Removed the -k flag from the tx raw commands (informalsystems#363)

* More logic to add key command (informalsystems#363)

* key add command for memory store working (informalsystems#363)

* Added logic to persist key seed in 'home' folder (informalsystems#363)

* Changes implemented (informalsystems#363):
- Added a new test keyring backend to support adding keys to file system (under home folder)
- Refactored logic to add key to be part of the keystore and not the command
- Switched the keybase on a chain to use the test keyring
- Key seed file is saved in the test keystore default folder (/home/andy/.rrly)

* Logic to use the key_name parameter from the config to add key. Removed name parameter from keys add cmd (informalsystems#363)

* Changed the logic to get the key from the test keyring file store (informalsystems#363)

* Implemented changes: (informalsystems#363)
- Clean up remaining key_seed flag for tx cmds
- Refactored keybase to include chain config
- Refactoring keyring methods to use chain config
- Logic to use configured key to sign tx

* Updated the README instructions (informalsystems#363)

* Disable the 'keys restore' command for now (informalsystems#363)

* Added 'keys list' command to show key added on a chain (informalsystems#363)

* Added entry for issue informalsystems#363 (PR informalsystems#408)

* Refactored the bound variables to use the full name per comment suggestion (informalsystems#408)

* Move key retrieval, memo and timeout height inside send_tx

* Add the client creation, connection and channel handshake

* remove sleeps

* More error handling, cleanup

* Macro for channel CLIs

* Macro for connection CLIs

* Where src/dst make no sense rename to a/b, also fix a few bugs after last commits

* cleanup

* cargo fmt

* Use Romain's skip-verif until backwards verification is done

* fix CLI bugs

Co-authored-by: Andy Nogueira <me@andynogueira.dev>
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.

Retrieve account sequence information from a chain using a GRPC client
4 participants