fix: validate caveat networks before requesting permissions for chain that is not added #29930

ffmcgee725 · 2025-01-27T19:55:23Z

await window.ethereum.request({
 "method": "wallet_requestPermissions",
 "params": [
  {
    
    "eth_accounts": {
        "caveats": [
            {
                "type": "restrictReturnedAccounts",
                "value": []
            }
        ]
    },
    "endowment:permitted-chains": {
        "caveats": [
            {
                "type": "restrictNetworkSwitching",
                "value": ["0xa86a"]
            }
        ]
    }
  }
],
});

Currently when running this request on main branch, the bad thing here being that we are requesting permitted chain that we don't have added on the extension, we get this error on UI (see image below).

Desired behaviour is what we have on current latest build, where running the same exact request with the chain we don't have added, throws a proper error in the console and no prompt in the UI

{
    "code": -32603,
    "message": "endowment:permitted-chains error: Received unrecognized chainId: \"0xa86a\". Please try adding the network first via wallet_addEthereumChain.",
    "data": {
        "cause": {
            "stack": "Error: endowment:permitted-chains error: Received unrecognized chainId: \"0xa86a\". Please try adding the network first via wallet_addEthereumChain.\n  at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-4.js:1:419383\n  at Array.forEach (<anonymous>)\n  at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-4.js:1:419329\n  at Object.validator (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-4.js:1:419532)\n  at j.validateCaveat (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-2.js:1:10699)\n  at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-2.js:1:9684\n  at Array.forEach (<anonymous>)\n  at j.validatePermission (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-2.js:1:9660)\n  at j.validateRequestedPermissions (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-2.js:1:12496)\n  at j.requestPermissions (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-2.js:1:10822)\n  at requestPermissionsForOrigin (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-0.js:1:243117)\n  at implementation (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-2.js:1:30815)\n  at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/background-7.js:1:173218\n  at chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:20623\n  at new Promise (<anonymous>)\n  at _.p (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:20280)\n  at _.m (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:20198)\n  at async _.d (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:19999)\n  at async _.l (chrome-extension://nkbihfbeogaeaoehlefnkodbefgpgknn/common-1.js:1:19825)",
            "message": "endowment:permitted-chains error: Received unrecognized chainId: \"0xa86a\". Please try adding the network first via wallet_addEthereumChain."
        }
    }
}

This pull request fixes this issue.

Description

Related issues

Fixes:

Manual testing steps

Send the request shown on description
If chain is not configured in wallet, exception should be thrown in console

Screenshots/Recordings

Before

After

Screen.Recording.2025-01-27.at.20.39.12.mov

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.

…nsImplementation

metamaskbot · 2025-01-27T20:33:40Z

Builds ready [6ec0db9]

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 (1660 ± 107 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	310	2118	1574	350	168
		domContentLoaded	1407	2106	1636	224	107
		load	1425	2124	1660	223	107
		domInteractive	23	115	48	33	16
		backgroundConnect	8	68	26	18	9
		firstReactRender	15	103	47	31	15
		getState	4	58	18	17	8
		initialActions	0	1	0	0	0
		loadScripts	961	1561	1193	180	86
		setupStore	6	78	17	21	10
		uiStartup	1623	2826	1945	314	151

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

background: 648 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2025-01-27T23:26:25Z

Builds ready [c61fbe8]

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 (1586 ± 50 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	326	1896	1455	380	182
		domContentLoaded	1397	1842	1562	107	52
		load	1405	1853	1586	105	50
		domInteractive	22	73	34	13	6
		backgroundConnect	7	63	25	16	8
		firstReactRender	14	79	37	26	12
		getState	4	53	9	10	5
		initialActions	0	1	0	0	0
		loadScripts	987	1336	1131	80	38
		setupStore	7	44	11	8	4
		uiStartup	1637	2166	1798	129	62

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

background: 648 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2025-01-28T11:32:14Z

Builds ready [b69e5d9]

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 (1714 ± 101 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	551	2204	1630	308	148
		domContentLoaded	1477	2168	1687	202	97
		load	1498	2206	1714	211	101
		domInteractive	23	73	38	18	8
		backgroundConnect	8	61	26	16	8
		firstReactRender	15	109	41	28	13
		getState	4	52	17	16	8
		initialActions	0	1	0	0	0
		loadScripts	1035	1576	1220	153	74
		setupStore	7	85	26	26	12
		uiStartup	1668	3077	2000	389	187

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

background: 648 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2025-01-28T15:48:33Z

Builds ready [de15169]

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 (1710 ± 84 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	321	2133	1637	348	167
		domContentLoaded	1376	2074	1680	161	77
		load	1386	2138	1710	175	84
		domInteractive	22	93	39	18	9
		backgroundConnect	8	64	29	22	10
		firstReactRender	16	98	45	30	14
		getState	4	65	18	20	10
		initialActions	0	1	0	0	0
		loadScripts	953	1496	1220	138	66
		setupStore	8	90	24	27	13
		uiStartup	1700	2484	2026	244	117

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

background: 648 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

jiexi · 2025-01-28T16:48:37Z

Desired behaviour is what we have on current latest build, where running the same exact request with the chain we don't have added, throws a proper error in the console and no prompt in the UI

I believe the endowment:permitted-chains error: Received unrecognized chainId: error is occurring because the chainId in the original request is still component state and is making it into the approveChainIds result even though it does not show up in the checkboxes. IMO, it is the responsibility of the UI here to properly handle the case where it gets passed chainIds that the wallet does not have. I don't believe that we need the validation/filtering logic at the handler level. Thoughts, @adonesky1 ?

metamaskbot · 2025-01-29T15:17:05Z

Builds ready [d2db02b]

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 (1913 ± 90 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	327	2217	1762	499	240
		domContentLoaded	1572	2203	1888	189	91
		load	1578	2218	1913	187	90
		domInteractive	26	106	45	21	10
		backgroundConnect	9	85	30	22	11
		firstReactRender	16	187	52	42	20
		getState	4	83	31	25	12
		initialActions	0	1	0	0	0
		loadScripts	1105	1635	1374	162	78
		setupStore	8	80	21	22	11
		uiStartup	1868	3069	2267	309	149

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

background: 648 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

metamaskbot · 2025-01-30T11:34:49Z

Builds ready [57d2779]

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 (1675 ± 69 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	302	2001	1532	428	206
		domContentLoaded	1429	1930	1652	135	65
		load	1434	1998	1675	144	69
		domInteractive	26	90	43	19	9
		backgroundConnect	8	70	25	20	10
		firstReactRender	16	95	42	26	13
		getState	4	54	12	13	6
		initialActions	0	0	0	0	0
		loadScripts	1010	1446	1202	121	58
		setupStore	7	68	20	22	10
		uiStartup	1681	2246	1944	177	85

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

background: 648 Bytes (0.01%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

ffmcgee725 · 2025-01-30T16:17:38Z

the approach followed in the following PR will make this unnecessary. Closing this.

ffmcgee725 added 5 commits January 27, 2025 18:28

docs: document possible solution

dc2b9c9

docs: further docs

1255f7f

docs: properly document issue and possible solutions

e8e0155

fix: validate caveat networks before proceeding with requestPermissio…

3391efe

…nsImplementation

Merge branch 'main' into jc/request-permissions-unavailable-chain

6ec0db9

ffmcgee725 requested a review from a team January 27, 2025 19:55

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

test: add e2e test

c61fbe8

Merge branch 'main' into jc/request-permissions-unavailable-chain

b69e5d9

ffmcgee725 marked this pull request as ready for review January 28, 2025 10:58

Merge branch 'main' into jc/request-permissions-unavailable-chain

de15169

Merge branch 'main' into jc/request-permissions-unavailable-chain

d2db02b

Merge branch 'main' into jc/request-permissions-unavailable-chain

57d2779

ffmcgee725 closed this Jan 30, 2025

github-actions bot locked and limited conversation to collaborators Jan 30, 2025

ffmcgee725 deleted the jc/request-permissions-unavailable-chain branch January 30, 2025 16:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: validate caveat networks before requesting permissions for chain that is not added #29930

fix: validate caveat networks before requesting permissions for chain that is not added #29930

ffmcgee725 commented Jan 27, 2025 •

edited by github-actions bot

Loading

metamaskbot commented Jan 27, 2025

metamaskbot commented Jan 27, 2025

metamaskbot commented Jan 28, 2025

metamaskbot commented Jan 28, 2025

jiexi commented Jan 28, 2025

metamaskbot commented Jan 29, 2025

metamaskbot commented Jan 30, 2025

ffmcgee725 commented Jan 30, 2025

fix: validate caveat networks before requesting permissions for chain that is not added #29930

fix: validate caveat networks before requesting permissions for chain that is not added #29930

Conversation

ffmcgee725 commented Jan 27, 2025 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

metamaskbot commented Jan 27, 2025

metamaskbot commented Jan 27, 2025

metamaskbot commented Jan 28, 2025

metamaskbot commented Jan 28, 2025

jiexi commented Jan 28, 2025

metamaskbot commented Jan 29, 2025

metamaskbot commented Jan 30, 2025

ffmcgee725 commented Jan 30, 2025

ffmcgee725 commented Jan 27, 2025 •

edited by github-actions bot

Loading