fix: MMASSETS-626-new-rpc-no-confirmation by bergarces · Pull Request #13883 · MetaMask/metamask-mobile

bergarces · 2025-03-06T14:52:51Z

Description

When a dapp sends a wallet_addEthereumChain request for an existing network with a different RPC, there should be a confirmation dialog. It currently updates the network without asking for any sort of permission from the user.

Related issues

Fixes: #12850

Manual testing steps

Add Polygon in MetaMask and switch back to Mainnet
Open this dapp on the inapp browser
If required, tap request accounts and connect while having Mainnet as the active chain
Tap Add Polygon Chain
A confirmation should show allowing the user to review the update (this was not happening before)
Go to the list of chains in the wallet and verify the list of RPC urls under the Polygon

Screenshots/Recordings

Before

Check bug report

After

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2025-03-07.at.17.02.02.mp4

Pre-merge author checklist

I’ve followed MetaMask Contributor Docs and MetaMask Mobile 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.

github-actions · 2025-03-06T14:53:04Z

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.

github-actions · 2025-03-07T09:28:34Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 434916e
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/0411dcc3-755f-4388-b14e-1084e8ca3efa

Note

You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

sonarqubecloud · 2025-03-07T09:38:10Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
89.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

adonesky1 · 2025-03-07T16:36:19Z

I'm a bit confused in the video you've linked in the description. There appear to be two modals that pop up, the first one suggests you're adding the network for the first time, and then the second one suggests your updating the default rpcUrl for an existing network configuration. It seems like it should just be the second modal that shows?

bergarces · 2025-03-07T17:06:39Z

I'm a bit confused in the video you've linked in the description. There appear to be two modals that pop up, the first one suggests you're adding the network for the first time, and then the second one suggests your updating the default rpcUrl for an existing network configuration. It seems like it should just be the second modal that shows?

I managed to paste the wrong video. I've updated it with one showing the "Update XXX" modal asking for confirmation.

The first time the modal shows, it's adding the network from the extension list of networks. The second one, shows how a dapp requesting to update the existing network with a new rpc requests permissions. Before this change, no permission was being asked to update the network with a new rpc.

adonesky1 · 2025-03-07T17:44:37Z

Ok nice! Yes the new video looks correct to me!

Cal-L

LGTM

adonesky1

Didn't manually test, but code looks pretty good. Probably could be cleaned up/refactored a bit but won't block on that!

metamaskbot added the mmi DEPRECATED: The product got sunset label Mar 6, 2025

bergarces force-pushed the fix/MMASSETS-626-new-rpc-no-confirmation branch 3 times, most recently from e2a2af5 to d2ca9e2 Compare March 6, 2025 15:46

bergarces commented Mar 6, 2025

View reviewed changes

Comment thread app/components/Approvals/AddChainApproval/AddChainApproval.tsx Outdated

bergarces commented Mar 6, 2025

View reviewed changes

Comment thread app/components/UI/NetworkVerificationInfo/NetworkVerificationInfo.test.tsx Outdated

bergarces commented Mar 6, 2025

View reviewed changes

Comment thread app/components/UI/NetworkVerificationInfo/NetworkVerificationInfo.tsx Outdated

bergarces commented Mar 6, 2025

View reviewed changes

Comment thread app/core/RPCMethods/wallet_addEthereumChain.js Outdated

bergarces commented Mar 6, 2025

View reviewed changes

Comment thread app/core/RPCMethods/wallet_addEthereumChain.js Outdated

bergarces added 9 commits March 6, 2025 18:37

confirmation added for network updates

5e88b82

corrections

81e1f33

remove changes

04d29d3

name change

22f35ba

corrections

4dd8122

tests added

0708f29

more tests

3bbca08

removed only

3302aaf

fix lint

12b59ce

bergarces force-pushed the fix/MMASSETS-626-new-rpc-no-confirmation branch from 147e418 to 12b59ce Compare March 6, 2025 18:37

bergarces marked this pull request as ready for review March 6, 2025 18:38

bergarces requested review from a team as code owners March 6, 2025 18:38

bergarces added 2 commits March 6, 2025 18:50

fix test

1aadc0a

removes property

b2eb562

bergarces commented Mar 6, 2025

View reviewed changes

Comment thread app/components/Approvals/AddChainApproval/AddChainApproval.test.tsx

snapshot

2df9aea

bergarces added team-assets Run Smoke E2E and removed mmi DEPRECATED: The product got sunset labels Mar 6, 2025

gambinish approved these changes Mar 6, 2025

View reviewed changes

bergarces changed the title ~~Fix/MMASSETS-626-new-rpc-no-confirmation~~ fix/MMASSETS-626-new-rpc-no-confirmation Mar 6, 2025

bergarces added this to PR review queue Mar 6, 2025

github-project-automation Bot moved this to Needs dev review in PR review queue Mar 6, 2025

bergarces moved this from Needs dev review to Has approvals, needs CODEOWNER in PR review queue Mar 6, 2025

Prithpal-Sooriya approved these changes Mar 6, 2025

View reviewed changes

bergarces changed the title ~~fix/MMASSETS-626-new-rpc-no-confirmation~~ fix: MMASSETS-626-new-rpc-no-confirmation Mar 6, 2025

bergarces added Run Smoke E2E and removed Run Smoke E2E labels Mar 6, 2025

Merge branch 'main' into fix/MMASSETS-626-new-rpc-no-confirmation

434916e

bergarces added Run Smoke E2E and removed Run Smoke E2E labels Mar 7, 2025

Cal-L approved these changes Mar 7, 2025

View reviewed changes

adonesky1 reviewed Mar 11, 2025

View reviewed changes

Comment thread app/core/RPCMethods/wallet_addEthereumChain.js

adonesky1 reviewed Mar 11, 2025

View reviewed changes

Comment thread app/core/RPCMethods/wallet_addEthereumChain.js

adonesky1 reviewed Mar 11, 2025

View reviewed changes

Comment thread app/core/RPCMethods/wallet_addEthereumChain.js

adonesky1 approved these changes Mar 11, 2025

View reviewed changes

github-project-automation Bot moved this from Has approvals, needs CODEOWNER to Review finalised - Ready to be merged in PR review queue Mar 11, 2025

bergarces added this pull request to the merge queue Mar 11, 2025

Merged via the queue into main with commit cecaaf0 Mar 11, 2025

bergarces deleted the fix/MMASSETS-626-new-rpc-no-confirmation branch March 11, 2025 15:24

github-project-automation Bot moved this from Review finalised - Ready to be merged to Merged, Closed or Archived in PR review queue Mar 11, 2025

github-actions Bot locked and limited conversation to collaborators Mar 11, 2025

metamaskbot added the release-7.43.0 Issue or pull request that will be included in release 7.43.0 label Mar 11, 2025

Uh oh!

Conversation

bergarces commented Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions Bot commented Mar 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Mar 7, 2025 • edited by metamaskbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitrise

Uh oh!

sonarqubecloud Bot commented Mar 7, 2025

Quality Gate passed

Uh oh!

adonesky1 commented Mar 7, 2025

Uh oh!

bergarces commented Mar 7, 2025

Uh oh!

adonesky1 commented Mar 7, 2025

Uh oh!

Cal-L left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adonesky1 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bergarces commented Mar 6, 2025 •

edited

Loading

github-actions Bot commented Mar 7, 2025 •

edited by metamaskbot

Loading