-
Notifications
You must be signed in to change notification settings - Fork 405
Feat/support cosmos evm #1814
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
base: main
Are you sure you want to change the base?
Feat/support cosmos evm #1814
Changes from 6 commits
f80d43d
36d2e10
f811448
c640c39
a489a7b
ce7e8b9
2b9d6ac
b303742
d15842c
8ff0e05
eaa0ebf
54b103c
bbffd36
5b4a48a
fada481
03fa926
17ad7ba
5fee884
2949808
e77ae7a
5636a48
4a5fd80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -24,10 +24,21 @@ export function isSecp256k1Pubkey(pubkey: Pubkey): pubkey is Secp256k1Pubkey { | |||||
| return (pubkey as Secp256k1Pubkey).type === "tendermint/PubKeySecp256k1"; | ||||||
| } | ||||||
|
|
||||||
| export interface EthSecp256k1Pubkey extends SinglePubkey { | ||||||
| readonly type: "os/PubKeyEthSecp256k1"; | ||||||
| readonly value: string; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question: is this a compressed or uncompressed encoding of the pubkey? In earlier versions of Ethermint this was uncompressed (but barely documented). Now looking at the code above it seems to be compressed. |
||||||
| } | ||||||
|
|
||||||
| export function isEthSecp256k1Pubkey(pubkey: Pubkey): pubkey is EthSecp256k1Pubkey { | ||||||
| return (pubkey as EthSecp256k1Pubkey).type === "os/PubKeyEthSecp256k1"; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this a typo? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. What does "os/" mean in this context? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe a legacy from evmOS? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems so. That is the earliest example of it I can see publicly within github. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should just use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we also remove type casting in this line? https://github.com/cosmos/cosmjs/blob/main/packages/amino/src/pubkeys.ts#L24 return (pubkey as Secp256k1Pubkey).type === "tendermint/PubKeySecp256k1";There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right! #1847 |
||||||
| } | ||||||
|
|
||||||
| export const pubkeyType = { | ||||||
| /** @see https://github.com/tendermint/tendermint/blob/v0.33.0/crypto/ed25519/ed25519.go#L22 */ | ||||||
| secp256k1: "tendermint/PubKeySecp256k1" as const, | ||||||
| /** @see https://github.com/tendermint/tendermint/blob/v0.33.0/crypto/secp256k1/secp256k1.go#L23 */ | ||||||
| secp256k1: "tendermint/PubKeySecp256k1" as const, | ||||||
| /** @see https://github.com/cosmos/evm/blob/main/crypto/ethsecp256k1/ethsecp256k1.go#L36 */ | ||||||
|
||||||
| /** @see https://github.com/cosmos/evm/blob/main/crypto/ethsecp256k1/ethsecp256k1.go#L36 */ | |
| /** @see https://github.com/cosmos/evm/blob/v1.0.0-rc2/crypto/ethsecp256k1/ethsecp256k1.go#L35-L36 */ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import { encodeSecp256k1Pubkey, makeSignDoc as makeSignDocAmino } from "@cosmjs/amino"; | ||
| /* eslint-disable @typescript-eslint/naming-convention */ | ||
webmaster128 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| import { encodeEthSecp256k1Pubkey, encodeSecp256k1Pubkey, makeSignDoc as makeSignDocAmino } from "@cosmjs/amino"; | ||
| import { sha256 } from "@cosmjs/crypto"; | ||
| import { fromBase64, toHex, toUtf8 } from "@cosmjs/encoding"; | ||
| import { Int53, Uint53 } from "@cosmjs/math"; | ||
|
|
@@ -703,7 +704,12 @@ export class SigningCosmWasmClient extends CosmWasmClient { | |
| if (!accountFromSigner) { | ||
| throw new Error("Failed to retrieve account from signer"); | ||
| } | ||
| const pubkey = encodePubkey(encodeSecp256k1Pubkey(accountFromSigner.pubkey)); | ||
| let pubkey; | ||
| if (accountFromSigner.algo == "eth_secp256k1" || accountFromSigner.algo == "ethsecp256k1" ) { | ||
|
||
| pubkey = encodePubkey(encodeEthSecp256k1Pubkey(accountFromSigner.pubkey)); | ||
| } else { | ||
| pubkey = encodePubkey(encodeSecp256k1Pubkey(accountFromSigner.pubkey)); | ||
| } | ||
| const signMode = SignMode.SIGN_MODE_LEGACY_AMINO_JSON; | ||
| const msgs = messages.map((msg) => this.aminoTypes.toAmino(msg)); | ||
| const signDoc = makeSignDocAmino(msgs, fee, chainId, memo, accountNumber, sequence, timeoutHeight); | ||
|
|
@@ -749,7 +755,12 @@ export class SigningCosmWasmClient extends CosmWasmClient { | |
| if (!accountFromSigner) { | ||
| throw new Error("Failed to retrieve account from signer"); | ||
| } | ||
| const pubkey = encodePubkey(encodeSecp256k1Pubkey(accountFromSigner.pubkey)); | ||
| let pubkey; | ||
| if (accountFromSigner.algo == "eth_secp256k1" || accountFromSigner.algo == "ethsecp256k1" ) { | ||
| pubkey = encodePubkey(encodeEthSecp256k1Pubkey(accountFromSigner.pubkey)); | ||
| } else { | ||
| pubkey = encodePubkey(encodeSecp256k1Pubkey(accountFromSigner.pubkey)); | ||
| } | ||
| const txBody: TxBodyEncodeObject = { | ||
| typeUrl: "/cosmos.tx.v1beta1.TxBody", | ||
| value: { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An "raw Eth Secp256k1 Pubkey" is uncompressed by convention. All Ethereum ecosystem uses uncompressed pubkeys. So either the implementation is wrong but maybe it is a question of findung a better name for the functionI am wrong I guess and compressed pubkeys are used consostently in Cosmos, also evm code. Any thoughts on that matter?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found the codes in cosmos/evm
ethsecp256k1.go, the PubKey() method returns a 33-byte compressed pubkey.But when it derives an address, the Address() method first decompresses the pubkey, then uses the uncompressed pubkey to derive an address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that helps. Sounds like a reasonable approach for the Cosmos Stack