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: support ethermint's EIP712 implementation #1

Closed
wants to merge 1 commit into from

Conversation

mtsitrin
Copy link
Owner

@mtsitrin mtsitrin commented Sep 9, 2024

MM is broken since v12.1.1 for cosmos chains that use https://github.com/evmos/ethermint as EVM adapter.

Ethermint uses hard coded "cosmos" string as the VerifyingContract field, which is broken since validateVerifyingContract introduced in MM.

This PR adds support to allow "cosmos" as the VerifyingContract value.

@mtsitrin mtsitrin changed the base branch from main to 14.x September 9, 2024 08:57
@mtsitrin mtsitrin force-pushed the allows_cosmos_for_verifyingContract branch from e760486 to b7e1f17 Compare September 9, 2024 09:03
@mtsitrin mtsitrin force-pushed the allows_cosmos_for_verifyingContract branch from b7e1f17 to 7d7cbc2 Compare September 9, 2024 09:07
@@ -500,7 +500,7 @@ WalletMiddlewareOptions): JsonRpcMiddleware<any, Block> {
*/
function validateVerifyingContract(data: string) {
const { domain: { verifyingContract } = {} } = parseTypedMessage(data);
if (verifyingContract && !isValidHexAddress(verifyingContract)) {
if (verifyingContract && !(isValidHexAddress(verifyingContract) || verifyingContract == 'cosmos')) {
Copy link

Choose a reason for hiding this comment

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

I think it will be better to support any bech32 format for cosmos chain.

I addition can you also plz add comment detailing why we have done this change.

Copy link
Owner Author

@mtsitrin mtsitrin Sep 10, 2024

Choose a reason for hiding this comment

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

thx.
currently we want to solve the hard coded "cosmos" issue, no need for generic bech32 support.
fixed and opened PR to the origin
MetaMask#333

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.

2 participants