Skip to content

Commit

Permalink
Replace ScopeObject.scopes with ScopeObject.references (#27403)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **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: MetaMask/MetaMask-planning#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**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **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.
  • Loading branch information
jiexi authored Sep 25, 2024
1 parent 450ec01 commit fec21c4
Show file tree
Hide file tree
Showing 8 changed files with 30 additions and 51 deletions.
4 changes: 2 additions & 2 deletions app/scripts/lib/multichain-api/scope/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export const KnownNotifications: Record<NonWalletKnownCaipNamespace, string[]> =
export type ExternalScope = CaipChainId | CaipReference;
export type ExternalScopeString = CaipChainId | CaipReference;
export type ExternalScopeObject = ScopeObject & {
scopes?: CaipChainId[];
references?: CaipReference[];
};
export type ExternalScopesObject = Record<
ExternalScopeString,
Expand All @@ -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.
Expand Down
8 changes: 4 additions & 4 deletions app/scripts/lib/multichain-api/scope/transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -247,7 +247,7 @@ describe('Scope Transform', () => {
eip155: {
...validScopeObject,
methods: ['a', 'b'],
scopes: ['eip155:1', 'eip155:5'],
references: ['1', '5'],
},
'eip155:1': {
...validScopeObject,
Expand Down
20 changes: 11 additions & 9 deletions app/scripts/lib/multichain-api/scope/transform.ts
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -14,9 +15,9 @@ function unique<T>(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
Expand All @@ -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;
};
Expand Down
24 changes: 3 additions & 21 deletions app/scripts/lib/multichain-api/scope/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
},
],
[
Expand Down Expand Up @@ -138,7 +120,7 @@ describe('Scope Validation', () => {
'only expected properties are defined',
validScopeString,
{
scopes: [],
references: [],
methods: [],
notifications: [],
accounts: [],
Expand Down
19 changes: 7 additions & 12 deletions app/scripts/lib/multichain-api/scope/validation.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -19,7 +19,7 @@ export const isValidScope = (
}

const {
scopes,
references,
methods,
notifications,
accounts,
Expand All @@ -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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const baseRequest = {
params: {
requiredScopes: {
eip155: {
scopes: ['eip155:1', 'eip155:137'],
references: ['1', '137'],
methods: [
'eth_sendTransaction',
'eth_signTransaction',
Expand Down
2 changes: 1 addition & 1 deletion app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/run-api-specs-multichain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ async function main() {
name: 'requiredScopes',
value: {
eip155: {
scopes: ['eip155:1'],
references: ['1'],
methods: ['eth_sendTransaction', 'eth_getBalance'],
notifications: [],
},
Expand Down

0 comments on commit fec21c4

Please sign in to comment.