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

Allow wallet_addEthereumChain for ethereum chains #13405

Conversation

kurushdubash
Copy link
Contributor

Fixes: #13629

Explanation: Metamask allows adding a custom RPC with any of the default chain ids. However, it blocks requests for wallet_addEthereumChain when the chain_id is one of the defaults that come with Metamask. Removing this block would allow wallet_addEthereumChain requests for Ethereum networks. This is a more convenient experience for users versus having to manually add a custom RPC and matches the functionality of manually adding an Ethereum chain.

Manual testing steps:

  • ran yarn tests and verified all tests pass
  • ran yarn build:test and verified all tests pass
  • Manually sent a wallet_addEthereumChain request with an Ethereum chain_id and verified it was successful with no errors

@kurushdubash kurushdubash requested a review from a team as a code owner January 26, 2022 02:52
@github-actions
Copy link
Contributor

github-actions bot commented Jan 26, 2022

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@kurushdubash
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

}),
);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure, id there any reason this check was put in place initially ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not find any documentation or information in commit history as to why that check is there. The same functionality is permitted through manual adding of RPC networks. In both flows, warnings are presented with regards to existing chain id networks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @jpuri, any update on this?

@kurushdubash
Copy link
Contributor Author

Hi @ryanml @jpuri , just wanted to bump this PR. There's been an increasing interest for allowing custom RPC endpoints for eth networks - https://twitter.com/MetaMask/status/1499448226180583429

@kurushdubash kurushdubash requested a review from jpuri April 18, 2022 20:28
@jordanmessina
Copy link

What is the status of this?

@brad-decker
Copy link
Member

@adonesky1 can we possibly fix this PR up, merge it and then rebase your work in #16733 so that we incorporate this community contribution.

@brad-decker
Copy link
Member

@kurushdubash -- @adonesky1 cherry picked your commit into his branch so that you will get credit for your contribution. 🎉 please head over to #16733 for any futher comments on this thread.

@brad-decker brad-decker closed this Dec 2, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2022
@brad-decker
Copy link
Member

@kurushdubash your commit landed as part of #16733 and you now have a shiny "contributor" label on your interactions with the repo:

image

Thanks @adonesky1 for taking the time to carry out my request to carry this commit into the codebase.

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

Successfully merging this pull request may close these issues.

Allow wallet_addEthereumChain for Ethereum networks
4 participants