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

Refactor wallet_addEthereumChain implementation #9541

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

starkbamse
Copy link

@starkbamse starkbamse commented May 4, 2024

This commit updates the wallet_addEthereumChain function to check for the existence of a network configuration based on the RPC URL rather than the chain ID. By doing so, we prevent allow multiple networks to have the same chainId but be from different providers.

Description

  1. On Mobile, the current implementation of checking for matching network entries is done by checking chainId. This is incorrect because it prevents users to add additional RPC url's of the same chain in their Metamask app.
  2. Looks for matching networks by RPC url instead of chainId.

Related issues

Fixes: #9519

Manual testing steps

  1. Add default RPC for binance smart chain
  2. Go to 1rpc.io and add their BSC url.
  3. Metamask shall add the new BSC chain configuration from 1rpc to Metamask.

Screenshots/Recordings

N/A

Pre-merge author checklist

  • I’ve followed MetaMask Coding Standards.
  • I've completed the PR template to the best of my ability
  • I’ve included tests if applicable
  • I’ve documented my code using JSDoc format if applicable
  • I’ve applied the right labels on the PR (see labeling guidelines). Not required for external contributors.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

…h for RPC URL in network configurations

This commit updates the wallet_addEthereumChain function to check for the
existence of a network configuration based on the RPC URL rather than the chain ID. By doing so, we prevent allow multiple networks to have the same chainId but be from different providers.
@starkbamse starkbamse requested a review from a team as a code owner May 4, 2024 15:10
Copy link
Contributor

github-actions bot commented May 4, 2024

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.

@starkbamse starkbamse changed the title #9519 Refactor wallet_addEthereumChain existingEntry to search for RP… #9519 Refactor wallet_addEthereumChain implementation May 4, 2024
@starkbamse starkbamse changed the title #9519 Refactor wallet_addEthereumChain implementation Refactor wallet_addEthereumChain implementation May 4, 2024
@metamaskbot metamaskbot added external-contributor INVALID-PR-TEMPLATE PR's body doesn't match template labels May 4, 2024
@starkbamse starkbamse marked this pull request as draft May 4, 2024 15:14
@starkbamse
Copy link
Author

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

@metamaskbot metamaskbot removed the INVALID-PR-TEMPLATE PR's body doesn't match template label May 4, 2024
@starkbamse starkbamse marked this pull request as ready for review May 4, 2024 15:28
@legobeat legobeat requested a review from a team May 5, 2024 12:42
@starkbamse starkbamse closed this May 5, 2024
@starkbamse starkbamse reopened this May 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Needs dev review
Development

Successfully merging this pull request may close these issues.

[Bug]: Metamask switches chain instead of adding a new chain.
2 participants