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 locally hosted RPC and Block Explorer Urls with wallet_addEthereumChain #14272

Merged
merged 4 commits into from
Jun 10, 2022

Conversation

MicaiahReid
Copy link
Contributor

@MicaiahReid MicaiahReid commented Mar 30, 2022

Explanation

Currently, adding a chain using the wallet_addEthereumChain RPC method requires that the supplied chain has an array of RPC urls, with at least one of them being an HTTPS url.

This is a problem because locally hosted resources are considered secure, and users may want to programmatically add their own local chains (e.g. Ganache) to MetaMask.

This change instead requires that at least one of the urls is either HTTPS or locally hosted (url.hostname === "localhost" || url.hostname === "127.0.0.1").

The same change is made for the block explorer urls.

Manual testing steps

To test, run MetaMask and run ganache --port 8544 --chain.chainId 1338. Send the wallet_addEthereumChain RPC method to MetaMask with the payload:

{
    chainId: "0x53A",
    chainName: 'Ganache Test',
    nativeCurrency: { symbol: 'ETH', decimals: 18 },
    rpcUrls: [ 'http://127.0.0.1:8544' ],
    blockExplorerUrls: [ 'http://localhost:3000' ]
  }

Verify that the chain is successfully added to MetaMask.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 30, 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.

@MicaiahReid
Copy link
Contributor Author

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

@MicaiahReid MicaiahReid force-pushed the feat/allow-local-rpc-urls branch from 3e03807 to cde3fca Compare April 1, 2022 17:16
@MicaiahReid MicaiahReid marked this pull request as ready for review April 2, 2022 17:49
@MicaiahReid MicaiahReid requested a review from a team as a code owner April 2, 2022 17:49
@MicaiahReid MicaiahReid requested a review from danjm April 2, 2022 17:49
brad-decker
brad-decker previously approved these changes Apr 21, 2022
brad-decker
brad-decker previously approved these changes Apr 22, 2022
@Amphaal
Copy link

Amphaal commented May 8, 2022

Many thanks for this PR @MicaiahReid ! Eager to use this patch ASAP 👍

darkwing
darkwing previously approved these changes May 9, 2022
@MicaiahReid MicaiahReid force-pushed the feat/allow-local-rpc-urls branch from 3713ea5 to f4cfade Compare May 10, 2022 13:11
@PeterYinusa PeterYinusa dismissed stale reviews from darkwing and brad-decker via f4cfade May 10, 2022 16:57
@AS-Pando
Copy link

AS-Pando commented Jun 2, 2022

Just wanted to mention that this feature would really help me out. Right now I'm running a small hardhat instance for testing. I don't seem to have a problem adding my chain from the extension network page (settings/networks/add-network). As MicaiahReid mentioned, I'd rather add it programmatically because I'd like to allow the user to direct my d.app to whatever local chain they have installed.

@MicaiahReid
Copy link
Contributor Author

@danjm Is there anything else needed to merge this? Merging is still blocked for me, but it looks like I've had two approvals?

@joani24

This comment was marked as spam.

darkwing
darkwing previously approved these changes Jun 3, 2022
adonesky1
adonesky1 previously approved these changes Jun 7, 2022
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

In general would ask for tests here, but doesn't seem we currently have any test suites for our RPC middleware methods...

LGTM

@MicaiahReid MicaiahReid force-pushed the feat/allow-local-rpc-urls branch from ea4d508 to 9e80229 Compare June 8, 2022 16:28
@bowensanders bowensanders dismissed stale reviews from adonesky1 and darkwing via 9e80229 June 8, 2022 19:07
@PeterYinusa PeterYinusa merged commit 8930df7 into MetaMask:develop Jun 10, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2022
@BelfordZ
Copy link
Contributor

fixes #14416

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

Successfully merging this pull request may close these issues.

10 participants