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

[14.x] fix: support ethermint's EIP712 implementation #333

Merged
merged 5 commits into from
Oct 2, 2024

Conversation

mtsitrin
Copy link
Contributor

@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 was introduced in MM.

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

@mtsitrin mtsitrin requested a review from a team as a code owner September 9, 2024 10:25
@omritoptix
Copy link

+1

1 similar comment
@testinginprod
Copy link

+1

src/wallet.ts Outdated
@@ -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
Contributor

@legobeat legobeat Sep 10, 2024

Choose a reason for hiding this comment

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

Suggested change
if (verifyingContract && !(isValidHexAddress(verifyingContract) || verifyingContract == 'cosmos')) {
if (verifyingContract && !isValidHexAddress(verifyingContract) && verifyingContract !== 'cosmos') {

Needs adjusting of types as well.

(FWIW: The linting can also be run locally by yarn lint and yarn build, and tests by yarn test)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx. fixed

@legobeat legobeat changed the title fix: support ethermint's EIP712 implementation [14.x] fix: support ethermint's EIP712 implementation Sep 10, 2024
@mtsitrin
Copy link
Contributor Author

@jpuri

@Gudahtt
Copy link
Member

Gudahtt commented Oct 2, 2024

Thanks again for the contribution @mtsitrin ! I've just pushed a few minor changes to make this match the version of this that we merged into the main branch and shipped to production: #334

@Gudahtt Gudahtt merged commit eac3ad7 into MetaMask:14.x Oct 2, 2024
13 checks passed
Gudahtt added a commit that referenced this pull request Oct 2, 2024
* Request validation should not throw if verifyingContract is not defined in typed signature (#328) (#330)

* 14.0.1 (#331)

* [14.x] fix: support ethermint's EIP712 implementation (#333)

* setting cosmos as allowed string for verifyingContract field

* fixed and linter

* readability

* Update condition to match main branch

* Remove duplicate copy of test

---------

Co-authored-by: Jyoti Puri <jyotipuri@gmail.com>
Co-authored-by: Mark Stacey <markjstacey@gmail.com>

* Version 14.0.2 (#339)

* Version 14.0.2

* Fix typo

Co-authored-by: Michele Esposito <34438276+mikesposito@users.noreply.github.com>

---------

Co-authored-by: Michele Esposito <34438276+mikesposito@users.noreply.github.com>

---------

Co-authored-by: Jyoti Puri <jyotipuri@gmail.com>
Co-authored-by: Michael Tsitrin <114929630+mtsitrin@users.noreply.github.com>
Co-authored-by: Michele Esposito <34438276+mikesposito@users.noreply.github.com>
@Gudahtt
Copy link
Member

Gudahtt commented Oct 2, 2024

This has been published as @metamask/eth-json-rpc-middleware@14.0.2

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.

6 participants