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

hermes is not compatible with ethermint #1267

Closed
5 tasks
devashishdxt opened this issue Aug 9, 2021 · 2 comments · Fixed by #1295
Closed
5 tasks

hermes is not compatible with ethermint #1267

devashishdxt opened this issue Aug 9, 2021 · 2 comments · Fixed by #1295
Labels
I: dependencies Internal: related to dependencies I: logic Internal: related to the relaying logic
Milestone
09.2021

Comments

@devashishdxt
Copy link

Crate

relayer

Summary

hermes is not compatible with https://github.com/tharsis/ethermint

Problem Definition

  • Why do we need this feature?
    To use hermes with chains based on ethermint.

  • What problems may be addressed by introducing this feature?
    hermes will no longer be non-compatible with ethermint

  • Are there any disadvantages of including this feature?
    No

Proposal

To be able to use hermes with ethermint, we need two things added in hermes:

  1. Adding key generation algorithm using eth-secp256k1 (Adding key generation algorithm for "eth_secp256k1" #1071)
  2. Currently, the code in relayer assumes that all the accounts are of type BaseAccount (here). In case of ethermint, this account is of type EthAccount (proto). This can be done in code by checking type_url value of prost::Any type returned by QueryAccountRequest.

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate milestone (priority) applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@devashishdxt devashishdxt mentioned this issue Aug 9, 2021
Closed
@xlab
Copy link

xlab commented Aug 11, 2021

Hello! We at Injective are running our own chain that is Ethermint-based, so we needed all these patches to support Hermes:

  1. Support for eth_secp256k1 in keyring to add correct keys: InjectiveLabs@28e7c65 (it uses hd_path.coin_type to detect the correct flow)

  2. Support for EthAccount implemented for the query client: InjectiveLabs@3903602 (it depends on is_ethermint config option now), also signer sets the correct PubKey type, based on the config option.

  3. Important to use Secp256k1 + Keccak256 for Ethermint transactions, instead of sha256, implemented there as alternative flow when config option (is_ethermint) is set for the chain: InjectiveLabs@507e730

Those changes allow to have support for Ethermint chains, while being compatible with gaia and others.

I've also rewrote E2E to use our own node and my fork:
https://github.com/InjectiveLabs/ibc-rs/commits/injective-chain - contains latest work that runs E2E pipeline.

I have no time to properly organize my work, so feel free to iterate on it for this Issue. ✌️

tomtau added a commit to tomtau/ibc-rs that referenced this issue Aug 17, 2021
@tomtau
added Ethermint support (fixes informalsystems#1267 informalsystems#1071
98acd57 
)

- collected changes from informalsystems#1267 (comment)
- EthAccount definition was directly pasted into the proto library
(as different chains the same proto definition, but under a different package path)
- added a new configuration option that allows specifying the address derivation
as well as the proto type of public keys
(e.g. "/injective.crypto.v1beta1.ethsecp256k1.PubKey"
or "/ethermint.crypto.v1alpha1.ethsecp256k1.PubKey")
@tomtau tomtau mentioned this issue Aug 17, 2021
Merged
5 tasks
tomtau added a commit to tomtau/ibc-rs that referenced this issue Aug 17, 2021
@tomtau
added Ethermint support (fixes informalsystems#1267 informalsystems#1071
a75f69d 
)

- collected changes from informalsystems#1267 (comment)
- EthAccount definition was directly pasted into the proto library
(as different chains the same proto definition, but under a different package path)
- added a new configuration option that allows specifying the address derivation
as well as the proto type of public keys
(e.g. "/injective.crypto.v1beta1.ethsecp256k1.PubKey"
or "/ethermint.crypto.v1alpha1.ethsecp256k1.PubKey")
@adizere adizere added this to the 08.2021 milestone Aug 17, 2021
@adizere adizere added I: dependencies Internal: related to dependencies I: logic Internal: related to the relaying logic labels Aug 17, 2021
tomtau added a commit to tomtau/ibc-rs that referenced this issue Aug 23, 2021
@tomtau
added Ethermint support (fixes informalsystems#1267 informalsystems#1071
b5990f2 
)

- collected changes from informalsystems#1267 (comment)
- EthAccount definition was directly pasted into the proto library
(as different chains the same proto definition, but under a different package path)
- added a new configuration option that allows specifying the address derivation
as well as the proto type of public keys
(e.g. "/injective.crypto.v1beta1.ethsecp256k1.PubKey"
or "/ethermint.crypto.v1alpha1.ethsecp256k1.PubKey")
@adizere adizere modified the milestones: 08.2021, 09.2021 Sep 6, 2021
adizere pushed a commit that referenced this issue Sep 10, 2021
@tomtau @romac
Ethermint support (#1295)
8544658 
* added Ethermint support (fixes #1267 #1071)

- collected changes from #1267 (comment)
- EthAccount definition was directly pasted into the proto library
(as different chains the same proto definition, but under a different package path)
- added a new configuration option that allows specifying the address derivation
as well as the proto type of public keys
(e.g. "/injective.crypto.v1beta1.ethsecp256k1.PubKey"
or "/ethermint.crypto.v1alpha1.ethsecp256k1.PubKey")

* added a comment for eth address and change query_account return type back to BaseAccount

* check the public key type in ethermint address generation

* added a check on `sign_msg`

* added comments + reordered example config

* added links with information for testing Ethermint

* adjusted a comment for `EthAccount`

Co-authored-by: Romain Ruetschi <romain@informal.systems>
@charleenfei
Copy link

🎉 🎉

hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this issue Sep 13, 2022
@tomtau @romac
Ethermint support (informalsystems#1295)
5651fa3 
* added Ethermint support (fixes informalsystems#1267 informalsystems#1071)

- collected changes from informalsystems#1267 (comment)
- EthAccount definition was directly pasted into the proto library
(as different chains the same proto definition, but under a different package path)
- added a new configuration option that allows specifying the address derivation
as well as the proto type of public keys
(e.g. "/injective.crypto.v1beta1.ethsecp256k1.PubKey"
or "/ethermint.crypto.v1alpha1.ethsecp256k1.PubKey")

* added a comment for eth address and change query_account return type back to BaseAccount

* check the public key type in ethermint address generation

* added a check on `sign_msg`

* added comments + reordered example config

* added links with information for testing Ethermint

* adjusted a comment for `EthAccount`

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
No one assigned
Labels
I: dependencies Internal: related to dependencies I: logic Internal: related to the relaying logic
Projects
None yet
Milestone
09.2021
Development

Successfully merging a pull request may close this issue.

Ethermint support tomtau/ibc-rs
4 participants
@xlab @adizere @charleenfei @devashishdxt