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

Add support Ethermint #1106

Closed
vincentysc opened this issue Apr 1, 2022 · 10 comments
Closed

Add support Ethermint #1106

vincentysc opened this issue Apr 1, 2022 · 10 comments

Comments

@vincentysc
Copy link

vincentysc commented Apr 1, 2022

Hi!

I added the Ethermint support to cosmjs locally with the following functions:

  • generate EVM / Bech32 address from public key type /ethermint.crypto.v1.ethsecp256k1.PubKey
  • sign the transaction with public key type /ethermint.crypto.v1.ethsecp256k1.PubKey

Is it ok to add a public key type flag to identify different decode/signing method in the following files? Any suggestions?

Generate address

In directsecp256k1hdwallet.js: getAccountsWithPrivkeys(), getKeyPair()
In account.js: accountFromAny()

Sign transaction

In pubkey.js: decodePubkey(), decodeSinglePubkey(), encodePubkey()
In signingstargateclient.js: signDirect()

@webmaster128
Copy link
Member

webmaster128 commented Apr 1, 2022

👋 I'm not convinced we should add chain-specific types into any core component of the library. But let's see, maybe we can help here and there …

In account.js: accountFromAny()

Do you need support for your own account type? This was recently implemented in #1102

@vincentysc
Copy link
Author

No problem~~

For generating the address, it's all good. I can do it outside the library and keep the core component library clean.

But for adding a new transaction signing method, Im wondering if I need to copy a set for code to do it because the signing method is shared across the code. Or is there any customize module for implementing it?

@webmaster128
Copy link
Member

Could you commit your local changed and create a draft PR with what you have? Maybe that helps understanding what you are doing and how to best collaborate. I worked on Ethereum client tech before but have no clue about Ethermint.

@vincentysc
Copy link
Author

Sure~

@vincentysc
Copy link
Author

vincentysc commented Apr 1, 2022

Hi, I just created a draft PR #1107. This is a quick draft from me and probably I will re-write and enhance it with pubkey type instead of coin type. Just let me know if there are any improvement or suggestions. Thank you!

@webmaster128
Copy link
Member

Thank you @vincentysc. I looked at your diff last week. That's very helpful. However, I don't think we want to integrate chain-specific functionality at that level. Sign Mode Direct is a Cosmos sign mode with Cosmos pubkeys and not made to add arbitrary secp256k1 signing. I think you can add Ethermint messages and queries in your client code. But if you need Ethereum signing tooling, you better use Ethereum libs for that.

@vincentysc
Copy link
Author

Fully understand. I'll give it a try.

Thank you @webmaster128.

@vincentysc
Copy link
Author

Thank you @vincentysc. I looked at your diff last week. That's very helpful. However, I don't think we want to integrate chain-specific functionality at that level. Sign Mode Direct is a Cosmos sign mode with Cosmos pubkeys and not made to add arbitrary secp256k1 signing. I think you can add Ethermint messages and queries in your client code. But if you need Ethereum signing tooling, you better use Ethereum libs for that.

Hi @webmaster128. Actually my project is running a in-app relayer(ts-relayer) to relay its own packets only. When it is Cosmos chains, use CosmJS at it is right now; On Ethermint based chain, switch to use another library. However, this would require quite some refactoring to the (ts-relayer) as it is highly dependant of CosmJs.

So, is the previous approach is good enough for this project? Do you have any other suggestions in your mind?

@garrettparris
Copy link

bringing this back, having the same issue with cosmjs signing client on an ethermint based chain. IMO, it stops being a chain-specific problem when multiple chains start trying to do this. would support for this to be re-opened so that ethermint developers dont have to maintain their own forks. I know its more of an ethos shift, @webmaster128 thoughts?

@webmaster128
Copy link
Member

See #1351

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants