fix: fix granted permissions when adding an existing network via `wallet_addEthereumChain` #29837

jiexi · 2025-01-21T23:22:54Z

Description

Fixes bug with the automatically permittedChains permission grant when triggered via wallet_addEthereumChain in certain scenarios. See ticket for details.

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3814

Manual testing steps

Add a new network to the dapp via wallet_addEthereumChain
After approval, the dapp should also have endowment:permitted-chains permission for the recently added chain via wallet_getPermissions
switch the dapp to a different chain via the wallet UI
revoke dapp permissions via the wallet UI
Add a new network to the dapp via wallet_addEthereumChain using a DIFFERENT rpc endpoint, but for the same chainId added in step 1
After approval, the dapp should also have endowment:permitted-chains permission for the recently added chain via wallet_getPermissions
switch the dapp to a different chain via the wallet UI
revoke dapp permissions via the wallet UI
Add a new network to the dapp via wallet_addEthereumChain using the SAME rpc endpoint in step 1 (i.e. repeat step 1 exactly)
You should get an approval for chain switch, NOT network adding. Approve it.
After approval, the dapp should also have endowment:permitted-chains permission for the recently added chain via wallet_getPermissions

Screenshots/Recordings

Before

After

Pre-merge author checklist

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

…hPermissionControllerConnectedToTestDappWithChains

…um-chain

github-actions · 2025-01-21T23:23:05Z

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.

metamaskbot · 2025-01-22T00:11:58Z

Builds ready [9097ee2]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (2286 ± 92 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	286	2452	1405	901	432
		domContentLoaded	1911	2604	2253	182	88
		load	1926	2703	2286	191	92
		domInteractive	29	130	67	27	13
		backgroundConnect	8	114	35	28	14
		firstReactRender	27	115	68	27	13
		getState	6	84	23	23	11
		initialActions	0	0	0	0	0
		loadScripts	1458	2034	1714	157	76
		setupStore	8	73	22	23	11
		uiStartup	2362	3378	2731	294	141

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 94 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

…um-chain

app/scripts/metamask-controller.js

metamaskbot · 2025-01-22T19:45:40Z

Builds ready [fcc79fc]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1893 ± 92 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	275	2101	1723	460	221
		domContentLoaded	1604	2361	1871	193	93
		load	1616	2372	1893	192	92
		domInteractive	25	124	46	29	14
		backgroundConnect	5	50	19	14	7
		firstReactRender	16	89	45	27	13
		getState	5	67	21	20	9
		initialActions	0	1	0	0	0
		loadScripts	1201	1749	1402	139	67
		setupStore	5	61	13	15	7
		uiStartup	1753	2876	2241	300	144

Bundle size diffs [🚀 Bundle size reduced!]

background: -417 Bytes (-0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

…um-chain

metamaskbot · 2025-01-22T20:26:21Z

Builds ready [11f009f]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1794 ± 92 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1530	2238	1792	189	91
		domContentLoaded	1522	2219	1766	192	92
		load	1539	2241	1794	192	92
		domInteractive	25	95	47	21	10
		backgroundConnect	9	56	24	16	8
		firstReactRender	15	94	53	27	13
		getState	6	64	14	15	7
		initialActions	0	1	0	0	0
		loadScripts	1144	1734	1334	165	79
		setupStore	6	57	14	16	7
		uiStartup	1718	2546	2052	227	109

Bundle size diffs [🚀 Bundle size reduced!]

background: -417 Bytes (-0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

test/e2e/json-rpc/wallet_addEthereumChain.spec.ts

Co-authored-by: Alex Donesky <[email protected]>

test/e2e/json-rpc/wallet_addEthereumChain.spec.ts

…um-chain

…s-add-ethereum-chain' into jl/mmp-3814/fix-grant-permissions-add-ethereum-chain

test/e2e/json-rpc/wallet_addEthereumChain.spec.ts

adonesky1 · 2025-01-22T22:39:00Z

test/e2e/json-rpc/wallet_addEthereumChain.spec.ts

+      );
+    });
+
+    it('automatically switches to the chain when the rpc endpoint being added already exists for the chain', async function () {


Will just highlight the case that (I think) we're missing here, which I've manually tested is, if the rpcEndpoint exists but is not currently the selected rpcEndpoint,, but the chain is selected

Add missing test case here: #29885

app/scripts/lib/rpc-method-middleware/handlers/ethereum-chain-utils.js

…are set

metamaskbot · 2025-01-22T23:24:03Z

Builds ready [6b5c973]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1598 ± 58 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1382	1850	1599	122	59
		domContentLoaded	1376	1838	1580	117	56
		load	1381	1848	1598	120	58
		domInteractive	24	85	36	16	8
		backgroundConnect	5	72	20	19	9
		firstReactRender	16	80	45	25	12
		getState	4	57	12	15	7
		initialActions	0	1	0	0	0
		loadScripts	977	1347	1161	101	48
		setupStore	5	48	12	14	7
		uiStartup	1537	2212	1835	155	75

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 7 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

…um-chain

jiexi · 2025-01-23T16:40:01Z

~~This PR depends on #29864~~

Above PR has been merged and brought into this branch

…um-chain

metamaskbot · 2025-01-23T18:46:34Z

Builds ready [b1b2f8e]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1872 ± 108 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	314	2487	1790	404	194
		domContentLoaded	1590	2402	1837	209	100
		load	1600	2487	1872	225	108
		domInteractive	26	123	41	23	11
		backgroundConnect	9	83	35	24	12
		firstReactRender	16	82	42	24	12
		getState	4	65	17	16	8
		initialActions	0	1	0	0	0
		loadScripts	1126	1862	1358	183	88
		setupStore	6	48	14	13	6
		uiStartup	1838	2919	2128	261	125

Bundle size diffs [🚀 Bundle size reduced!]

background: -28 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

Add missing test case

adonesky1

LGTM

metamaskbot · 2025-01-23T21:49:06Z

Builds ready [a686e2b]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1681 ± 74 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1449	2137	1684	168	81
		domContentLoaded	1438	2033	1655	153	73
		load	1452	2052	1681	155	74
		domInteractive	21	78	35	16	8
		backgroundConnect	7	68	24	19	9
		firstReactRender	16	93	48	28	13
		getState	4	70	18	20	10
		initialActions	0	1	0	0	0
		loadScripts	1018	1491	1214	123	59
		setupStore	6	85	11	17	8
		uiStartup	1651	2397	1960	202	97

Bundle size diffs [🚀 Bundle size reduced!]

background: -28 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: -40.17 KiB (-0.44%)

…um-chain

metamaskbot · 2025-01-30T14:50:31Z

Builds ready [060a6cd]

builds: chrome, firefox
builds (flask): chrome, firefox
builds (MMI): chrome
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Bundle Size Stats
mv2: E2e Actions Stats
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6, 7
- common: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- ui: 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13
- content-script: 0
- offscreen: 0

Page Load Metrics (1752 ± 77 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	230	2101	1613	464	223
		domContentLoaded	1507	2068	1723	158	76
		load	1517	2080	1752	161	77
		domInteractive	25	89	41	18	9
		backgroundConnect	10	92	36	26	13
		firstReactRender	16	103	46	30	14
		getState	5	78	21	22	10
		initialActions	0	1	0	0	0
		loadScripts	1059	1565	1262	135	65
		setupStore	8	89	20	22	11
		uiStartup	1706	2834	2057	280	134

Bundle size diffs [🚀 Bundle size reduced!]

background: -22 Bytes (-0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

jiexi added 6 commits January 21, 2025 13:49

remove approvalFlowId. replace isAddFlow with autoApprove

b7bf2ac

WIP wallet_addEthereumChain specs

65a837b

replace withPermissionControllerConnectedToTestDappWithChain with wit…

15248fb

…hPermissionControllerConnectedToTestDappWithChains

add wallet_addEthereumChain spec

0d44554

lint

493e644

Merge branch 'main' into jl/mmp-3814/fix-grant-permissions-add-ethere…

9097ee2

…um-chain

metamaskbot added the team-wallet-api-platform label Jan 21, 2025

jiexi changed the title ~~fix grant permissions add ethereum chain~~ fix: fix granted permissions when adding an existing network via wallet_addEthereumChain Jan 21, 2025

jiexi marked this pull request as ready for review January 21, 2025 23:40

jiexi added 2 commits January 22, 2025 09:54

remove withPermissionControllerConnectedToTestDappWithTwoChains

d21a6bc

Merge branch 'main' into jl/mmp-3814/fix-grant-permissions-add-ethere…

fcc79fc

…um-chain