From fec21c44e8cc9143f1e187e4bd8c3f6bbb6cb751 Mon Sep 17 00:00:00 2001 From: jiexi Date: Wed, 25 Sep 2024 10:51:54 -0700 Subject: [PATCH] Replace `ScopeObject.scopes` with `ScopeObject.references` (#27403) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Replaces `scopes` with `references` on `ScopeObject` [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27403?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/3301 ## **Manual testing steps** ``` const EXTENSION_ID = 'nonfpcflonapegmnfeafnddgdniflbnk'; const extensionPort = chrome.runtime.connect(EXTENSION_ID) extensionPort.onMessage.addListener((msg) => console.log('extensionPort on message', msg)) extensionPort.postMessage({ type: 'caip-x', data: { "jsonrpc": "2.0", method: 'wallet_createSession', params: { requiredScopes: { 'eip155': { references: ['1', '59144'], methods: [ 'eth_sendTransaction', 'eth_getBalance', 'eth_subscribe' ], notifications: ['eth_subscription'], } }, optionalScopes: { }, sessionProperties: { 'caip154-mandatory': 'true', }, }, } }) ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] 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](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). 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. --- app/scripts/lib/multichain-api/scope/scope.ts | 4 ++-- .../multichain-api/scope/transform.test.ts | 8 +++---- .../lib/multichain-api/scope/transform.ts | 20 +++++++++------- .../multichain-api/scope/validation.test.ts | 24 +++---------------- .../lib/multichain-api/scope/validation.ts | 19 ++++++--------- .../wallet-createSession/handler.test.js | 2 +- app/scripts/metamask-controller.js | 2 +- test/e2e/run-api-specs-multichain.ts | 2 +- 8 files changed, 30 insertions(+), 51 deletions(-) diff --git a/app/scripts/lib/multichain-api/scope/scope.ts b/app/scripts/lib/multichain-api/scope/scope.ts index e3042d86bbe9..b5fda26bdda4 100644 --- a/app/scripts/lib/multichain-api/scope/scope.ts +++ b/app/scripts/lib/multichain-api/scope/scope.ts @@ -53,7 +53,7 @@ export const KnownNotifications: Record = export type ExternalScope = CaipChainId | CaipReference; export type ExternalScopeString = CaipChainId | CaipReference; export type ExternalScopeObject = ScopeObject & { - scopes?: CaipChainId[]; + references?: CaipReference[]; }; export type ExternalScopesObject = Record< ExternalScopeString, @@ -63,7 +63,7 @@ export type ExternalScopesObject = Record< // These non-prefixed types represent CAIP-217 Scope and // ScopeObject as defined by the spec but without // namespace-only Scopes (except for "wallet") and without -// the `scopes` array of CAIP Chain IDs on ScopeObject. +// the `references` array of CAIP References on the ScopeObject. // These deviations from the spec are necessary as MetaMask // does not support wildcarded Scopes, i.e. Scopes that only // specify a namespace but no specific reference. diff --git a/app/scripts/lib/multichain-api/scope/transform.test.ts b/app/scripts/lib/multichain-api/scope/transform.test.ts index fbb0618a6a53..0a027e617f51 100644 --- a/app/scripts/lib/multichain-api/scope/transform.test.ts +++ b/app/scripts/lib/multichain-api/scope/transform.test.ts @@ -20,17 +20,17 @@ describe('Scope Transform', () => { }); describe('scopeString is namespace scoped', () => { - it('returns the scope as is when `scopes` is not defined', () => { + it('returns the scope as is when `references` is not defined', () => { expect(flattenScope('eip155', validScopeObject)).toStrictEqual({ eip155: validScopeObject, }); }); - it('returns one scope per `scopes` element with `scopes` excluded from the scopeObject', () => { + it('returns one scope per `references` element with `references` excluded from the scopeObject', () => { expect( flattenScope('eip155', { ...validScopeObject, - scopes: ['eip155:1', 'eip155:5', 'eip155:64'], + references: ['1', '5', '64'], }), ).toStrictEqual({ 'eip155:1': validScopeObject, @@ -247,7 +247,7 @@ describe('Scope Transform', () => { eip155: { ...validScopeObject, methods: ['a', 'b'], - scopes: ['eip155:1', 'eip155:5'], + references: ['1', '5'], }, 'eip155:1': { ...validScopeObject, diff --git a/app/scripts/lib/multichain-api/scope/transform.ts b/app/scripts/lib/multichain-api/scope/transform.ts index 881957c3c83b..ba1ffe1a4999 100644 --- a/app/scripts/lib/multichain-api/scope/transform.ts +++ b/app/scripts/lib/multichain-api/scope/transform.ts @@ -1,10 +1,11 @@ -import { CaipChainId, isCaipChainId } from '@metamask/utils'; +import { CaipReference } from '@metamask/utils'; import { ExternalScopeObject, ExternalScopesObject, ScopeString, ScopeObject, ScopesObject, + parseScopeString, } from './scope'; // DRY THIS @@ -14,9 +15,9 @@ function unique(list: T[]): T[] { /** * Flattens a ScopeString and ScopeObject into a separate - * ScopeString and ScopeObject for each scope in the `scopes` value - * if defined. Returns the ScopeString and ScopeObject unmodified if - * it cannot be flattened + * ScopeString and ScopeObject for each reference in the `references` + * value if defined. Returns the ScopeString and ScopeObject + * unmodified if it cannot be flattened * * @param scopeString - The string representing the scopeObject * @param scopeObject - The object that defines the scope @@ -26,16 +27,17 @@ export const flattenScope = ( scopeString: string, scopeObject: ExternalScopeObject, ): ScopesObject => { - const { scopes, ...restScopeObject } = scopeObject; - const isChainScoped = isCaipChainId(scopeString); + const { references, ...restScopeObject } = scopeObject; + const { namespace, reference } = parseScopeString(scopeString); - if (isChainScoped || !scopes) { + // Scope is already a CAIP-2 ID and has no references to flatten + if (reference || !references) { return { [scopeString]: scopeObject }; } const scopeMap: ScopesObject = {}; - scopes.forEach((nestedScopeString: CaipChainId) => { - scopeMap[nestedScopeString] = restScopeObject; + references.forEach((nestedReference: CaipReference) => { + scopeMap[`${namespace}:${nestedReference}`] = restScopeObject; }); return scopeMap; }; diff --git a/app/scripts/lib/multichain-api/scope/validation.test.ts b/app/scripts/lib/multichain-api/scope/validation.test.ts index 67294a3f508a..e2b3fa4d7f7d 100644 --- a/app/scripts/lib/multichain-api/scope/validation.test.ts +++ b/app/scripts/lib/multichain-api/scope/validation.test.ts @@ -45,29 +45,11 @@ describe('Scope Validation', () => { ], [ false, - 'the scopeString is a CAIP chainId but scopes is nonempty', + 'the scopeString is a CAIP chainId but references is nonempty', 'eip155:1', { ...validScopeObject, - scopes: ['eip155:5'], - }, - ], - [ - false, - 'the scopeString is a CAIP namespace but scopes contains CAIP chainIds for a different namespace', - 'eip155:1', - { - ...validScopeObject, - scopes: ['eip155:5', 'bip122:000000000019d6689c085ae165831e93'], - }, - ], - [ - true, - 'the scopeString is a CAIP namespace and scopes contains CAIP chainIds for only the same namespace', - 'eip155', - { - ...validScopeObject, - scopes: ['eip155:5', 'eip155:64'], + references: ['5'], }, ], [ @@ -138,7 +120,7 @@ describe('Scope Validation', () => { 'only expected properties are defined', validScopeString, { - scopes: [], + references: [], methods: [], notifications: [], accounts: [], diff --git a/app/scripts/lib/multichain-api/scope/validation.ts b/app/scripts/lib/multichain-api/scope/validation.ts index ee131b4b1329..95d6dd3a5a62 100644 --- a/app/scripts/lib/multichain-api/scope/validation.ts +++ b/app/scripts/lib/multichain-api/scope/validation.ts @@ -1,4 +1,4 @@ -import { KnownCaipNamespace, parseCaipChainId } from '@metamask/utils'; +import { isCaipReference, KnownCaipNamespace } from '@metamask/utils'; import { toHex } from '@metamask/controller-utils'; import { validateAddEthereumChainParams } from '../../rpc-method-middleware/handlers/ethereum-chain-utils'; import { @@ -19,7 +19,7 @@ export const isValidScope = ( } const { - scopes, + references, methods, notifications, accounts, @@ -33,20 +33,15 @@ export const isValidScope = ( } // These assume that the namespace has a notion of chainIds - if (reference && scopes && scopes.length > 0) { + if (reference && references && references.length > 0) { return false; } - if (namespace && scopes) { - const areScopesValid = scopes.every((scope) => { - try { - return parseCaipChainId(scope).namespace === namespace; - } catch (e) { - console.log(e); - return false; - } + if (namespace && references) { + const areReferencesValid = references.every((nestedReference) => { + return isCaipReference(nestedReference); }); - if (!areScopesValid) { + if (!areReferencesValid) { return false; } } diff --git a/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js b/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js index 6d07bb4cfe75..cc51c23dc116 100644 --- a/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js +++ b/app/scripts/lib/multichain-api/wallet-createSession/handler.test.js @@ -38,7 +38,7 @@ const baseRequest = { params: { requiredScopes: { eip155: { - scopes: ['eip155:1', 'eip155:137'], + references: ['1', '137'], methods: [ 'eth_sendTransaction', 'eth_signTransaction', diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 18883ff4a6db..b0677c00888a 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -6221,7 +6221,7 @@ export default class MetamaskController extends EventEmitter { this.networkController, ), updateNetwork: this.networkController.updateNetwork.bind( - this.networkController + this.networkController, ), setActiveNetwork: async (networkClientId) => { await this.networkController.setActiveNetwork(networkClientId); diff --git a/test/e2e/run-api-specs-multichain.ts b/test/e2e/run-api-specs-multichain.ts index 4f04ef7c10e5..fb8901acdbc0 100644 --- a/test/e2e/run-api-specs-multichain.ts +++ b/test/e2e/run-api-specs-multichain.ts @@ -61,7 +61,7 @@ async function main() { name: 'requiredScopes', value: { eip155: { - scopes: ['eip155:1'], + references: ['1'], methods: ['eth_sendTransaction', 'eth_getBalance'], notifications: [], },