feat(snaps): Add support for custom network per Snap #26389

Mrtenz · 2024-08-13T13:55:08Z

Description

Like with websites, we want to support a custom network per Snap rather than relying on the globally selected network. For Snaps, the permission for the network to switch to is automatically granted.

Blocked by:

Todo

Revert execution environment URL.
Remove preview version for selected-network-controller.

Manual testing steps

Build the extension with CHAIN_PERMISSIONS=true.
Use wallet_switchEthereumChain in a Snap.
The Snap should now use the new network, without any permission requests.

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.

socket-security · 2024-08-13T13:56:37Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask-previews/[email protected]	Transitive: environment, network	`+8`	2.12 MB	metamaskbot
npm/@types/[email protected]	None	`0`	5.52 kB	types

🚮 Removed packages: npm/@metamask/[email protected]

View full report↗︎

socket-security · 2024-08-13T13:56:40Z

🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎

To accept the risk, merge this PR and you will not be notified again.

Alert	Package	Note	Source	CI
New author	npm/@metamask/[email protected]	New Author: mcmire Previous Author: gudahtt	`package.json`	🚫

View full report↗︎

Next steps

What is new author?

A new npm collaborator published a version of the package for the first time. New collaborators are usually benign additions to a project, but do indicate a change to the security surface area of a package.

Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights.

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

@SocketSecurity ignore npm/@metamask/[email protected]

…rrent network

sonarcloud · 2024-08-15T08:31:44Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

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

See analysis details on SonarCloud

FrederikBolding · 2024-08-15T13:30:28Z

builds.yml

@@ -26,7 +26,7 @@ buildTypes:
      - SEGMENT_WRITE_KEY_REF: SEGMENT_PROD_WRITE_KEY
      - ALLOW_LOCAL_SNAPS: false
      - REQUIRE_SNAPS_ALLOWLIST: true
-      - IFRAME_EXECUTION_ENVIRONMENT_URL: https://execution.metamask.io/iframe/6.5.0/index.html
+      - IFRAME_EXECUTION_ENVIRONMENT_URL: http://localhost:6363


Should be reverted before merge

FrederikBolding · 2024-08-15T13:30:38Z

package.json

@@ -350,7 +350,7 @@
    "@metamask/rpc-errors": "^6.2.1",
    "@metamask/safe-event-emitter": "^3.1.1",
    "@metamask/scure-bip39": "^2.0.3",
-    "@metamask/selected-network-controller": "^15.0.2",
+    "@metamask/selected-network-controller": "npm:@metamask-previews/selected-network-controller@17.0.0-preview-55328a6",


Should be reverted before merge

FrederikBolding · 2024-08-15T13:36:56Z

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

@@ -214,7 +216,11 @@ export async function switchChain(
        permissionedChainIds === undefined ||
        !permissionedChainIds.includes(chainId)
      ) {
-        await requestPermittedChainsPermission([chainId]);
+        if (isSnapId(origin)) {


Can we test this functionality?

FrederikBolding · 2024-08-29T08:43:36Z

app/scripts/metamask-controller.js

+            requestData: {
+              approvedChainIds: chainIds,
+            },
+            approvedPermissions: {
+              [PermissionNames.permittedChains]: {
+                caveats: [
+                  CaveatFactories[CaveatTypes.restrictNetworkSwitching](
+                    chainIds,
+                  ),
+                ],
+              },


Seems a bit strange that we have to specify the chain IDs twice 🤔 Seems like the permission specification strictly uses requestData

FrederikBolding · 2024-08-29T08:43:57Z

app/scripts/migrations/126.ts

+export const version = 126;
+
+/**
+ * This migration adds the `permittedChains` permission to all Snaps that have


Suggested change

* This migration adds the `permittedChains` permission to all Snaps that have

* This migration adds the `endowment:permittedChains` permission to all Snaps that have

As well as others 😄

FrederikBolding · 2024-08-29T08:46:45Z

app/scripts/migrations/126.ts

+      permissionKeys.includes(SnapEndowments.EthereumProvider) &&
+      !permissionKeys.includes(PermissionNames.permittedChains);
+    if (!needsMigration) {
+      return accumulator;


Doesn't this delete any subjects that don't need the migration?

FrederikBolding · 2024-08-29T08:47:33Z

app/scripts/migrations/126.test.ts

+    expect(newStorage.meta).toStrictEqual({ version });
+  });
+
+  it('adds the network endowment to Snaps with the `endowment:ethereum-provider` permission', async () => {


We should test that it doesn't touch Snaps that don't have the permission

metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Aug 13, 2024

metamaskbot added the team-snaps-platform Snaps Platform team label Aug 13, 2024

Mrtenz changed the title ~~Add support for custom network per Snap~~ feat(snaps): Add support for custom network per Snap Aug 14, 2024

Mrtenz added 5 commits August 15, 2024 10:29

Add support for custom network per Snap

2b98184

Remove console.log

751a679

Add migration adding permission to the Snap and setting the Snap's cu…

ffc0b42

…rrent network

Update migration

3905e57

Add another test

7f2494b

Mrtenz force-pushed the mrtenz/snap-networks branch from 38edd7a to 7f2494b Compare August 15, 2024 08:29

FrederikBolding reviewed Aug 15, 2024

View reviewed changes

Fix lint issues

6453f80

FrederikBolding reviewed Aug 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(snaps): Add support for custom network per Snap #26389

feat(snaps): Add support for custom network per Snap #26389

Mrtenz commented Aug 13, 2024 •

edited

Loading

socket-security bot commented Aug 13, 2024 •

edited

Loading

socket-security bot commented Aug 13, 2024 •

edited

Loading

sonarcloud bot commented Aug 15, 2024

FrederikBolding Aug 15, 2024

FrederikBolding Aug 15, 2024

FrederikBolding Aug 15, 2024

FrederikBolding Aug 29, 2024

FrederikBolding Aug 29, 2024 •

edited

Loading

FrederikBolding Aug 29, 2024

FrederikBolding Aug 29, 2024 •

edited

Loading

	* This migration adds the `permittedChains` permission to all Snaps that have
	* This migration adds the `endowment:permittedChains` permission to all Snaps that have

feat(snaps): Add support for custom network per Snap #26389

Are you sure you want to change the base?

feat(snaps): Add support for custom network per Snap #26389

Conversation

Mrtenz commented Aug 13, 2024 • edited Loading

Description

Todo

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

socket-security bot commented Aug 13, 2024 • edited Loading

socket-security bot commented Aug 13, 2024 • edited Loading

Next steps

sonarcloud bot commented Aug 15, 2024

Quality Gate passed

FrederikBolding Aug 15, 2024

Choose a reason for hiding this comment

FrederikBolding Aug 15, 2024

Choose a reason for hiding this comment

FrederikBolding Aug 15, 2024

Choose a reason for hiding this comment

FrederikBolding Aug 29, 2024

Choose a reason for hiding this comment

FrederikBolding Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

FrederikBolding Aug 29, 2024

Choose a reason for hiding this comment

FrederikBolding Aug 29, 2024 • edited Loading

Choose a reason for hiding this comment

Mrtenz commented Aug 13, 2024 •

edited

Loading

socket-security bot commented Aug 13, 2024 •

edited

Loading

socket-security bot commented Aug 13, 2024 •

edited

Loading

FrederikBolding Aug 29, 2024 •

edited

Loading

FrederikBolding Aug 29, 2024 •

edited

Loading