From aa61f332730fe69c94242e185ca77be28a5b5adf Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 10:53:39 -0600 Subject: [PATCH 01/12] address partially mutated state bug --- app/scripts/migrations/137.test.ts | 2 +- app/scripts/migrations/137.ts | 106 ++++++++++++++++++++--------- 2 files changed, 76 insertions(+), 32 deletions(-) diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index 778f85265c2f..bae7ea06608c 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -518,7 +518,7 @@ describe('migration #137', () => { }); const currentScope = `eip155:${chainId}`; - it('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does not have its own network client', async () => { + it.only('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does not have its own network client', async () => { const oldStorage = { meta: { version: oldVersion }, data: { diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 3b7f806e9b2c..cda364057f00 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -74,6 +74,9 @@ export async function migrate( } function transformState(state: Record) { + // **Step 1: Validate all subjects first** + + // Validate the existence and types of controllers if ( !hasProperty(state, 'PermissionController') || !isObject(state.PermissionController) @@ -119,32 +122,36 @@ function transformState(state: Record) { SelectedNetworkController: { domains }, } = state; + // Validate required properties if (!isObject(subjects)) { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.PermissionController.subjects is ${typeof subjects}`, ), ); return state; } + if (!selectedNetworkClientId || typeof selectedNetworkClientId !== 'string') { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.NetworkController.selectedNetworkClientId is ${typeof selectedNetworkClientId}`, + `Migration ${version}: invalid selectedNetworkClientId "${selectedNetworkClientId}"`, ), ); return state; } + if (!isObject(networkConfigurationsByChainId)) { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof networkConfigurationsByChainId}`, ), ); return state; } + if (!isObject(domains)) { - global.sentry?.captureException( + global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.SelectedNetworkController.domains is ${typeof domains}`, ), @@ -152,12 +159,62 @@ function transformState(state: Record) { return state; } + // Validate all subjects before making any changes + for (const [origin, subject] of Object.entries(subjects)) { + if (!isObject(subject)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, + ), + ); + return state; + } + + const { permissions } = subject as { + permissions: Record; + }; + if (!isObject(permissions)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof permissions}`, + ), + ); + return state; + } + } + +interface MetaMaskState { + PermissionController: { + subjects: Record; + }; + NetworkController: { + selectedNetworkClientId: string; + networkConfigurationsByChainId: Record; + }; + SelectedNetworkController: { + domains: Record; + }; +} + + const newState: MetaMaskState = cloneDeep(state) as MetaMaskState; + + const { + PermissionController: { subjects: newSubjects }, + NetworkController: { + selectedNetworkClientId: newSelectedNetworkClientId, + networkConfigurationsByChainId: newNetworkConfigurationsByChainId, + }, + SelectedNetworkController: { domains: newDomains }, + } = newState; + + // **Step 3: Perform all mutations on the cloned state** + const getChainIdForNetworkClientId = ( networkClientId: string, propertyName: string, ): string | undefined => { for (const [chainId, networkConfiguration] of Object.entries( - networkConfigurationsByChainId, + newNetworkConfigurationsByChainId, )) { if (!isObject(networkConfiguration)) { global.sentry?.captureException( @@ -202,34 +259,17 @@ function transformState(state: Record) { }; const currentChainId = getChainIdForNetworkClientId( - selectedNetworkClientId, + newSelectedNetworkClientId, 'selectedNetworkClientId', ); if (!currentChainId) { return state; } - for (const [origin, subject] of Object.entries(subjects)) { - if (!isObject(subject)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, - ), - ); - continue; - } - + for (const [origin, subject] of Object.entries(newSubjects)) { const { permissions } = subject as { permissions: Record; }; - if (!isObject(permissions)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof permissions}`, - ), - ); - continue; - } let basePermission: PermissionConstraint | undefined; @@ -239,8 +279,9 @@ function transformState(state: Record) { Array.isArray(permissions[PermissionNames.eth_accounts].caveats) ) { ethAccounts = - (permissions[PermissionNames.eth_accounts].caveats?.[0] - ?.value as string[]) ?? []; + (permissions[PermissionNames.eth_accounts].caveats?.[0]?.value as + | string[] + | undefined) ?? []; basePermission = permissions[PermissionNames.eth_accounts]; } delete permissions[PermissionNames.eth_accounts]; @@ -251,8 +292,9 @@ function transformState(state: Record) { Array.isArray(permissions[PermissionNames.permittedChains].caveats) ) { chainIds = - (permissions[PermissionNames.permittedChains].caveats?.[0] - ?.value as string[]) ?? []; + (permissions[PermissionNames.permittedChains].caveats?.[0]?.value as + | string[] + | undefined) ?? []; basePermission ??= permissions[PermissionNames.permittedChains]; } delete permissions[PermissionNames.permittedChains]; @@ -264,7 +306,7 @@ function transformState(state: Record) { if (chainIds.length === 0) { chainIds = [currentChainId]; - const networkClientIdForOrigin = domains[origin]; + const networkClientIdForOrigin = newDomains[origin]; if ( networkClientIdForOrigin && typeof networkClientIdForOrigin === 'string' @@ -311,8 +353,10 @@ function transformState(state: Record) { }, ], }; + permissions[Caip25EndowmentPermissionName] = caip25Permission; } - return state; + // **Step 4: Return either original state or fully transformed state** + return newState; } From 344a35237dd288f960f3d2794c891c45062afa3b Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 12:52:17 -0600 Subject: [PATCH 02/12] fixing --- app/scripts/migrations/137.test.ts | 39 +++- app/scripts/migrations/137.ts | 355 ++++++++++++++--------------- 2 files changed, 214 insertions(+), 180 deletions(-) diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index bae7ea06608c..bc7c9bb6bbb8 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -518,7 +518,7 @@ describe('migration #137', () => { }); const currentScope = `eip155:${chainId}`; - it.only('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does not have its own network client', async () => { + it('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does not have its own network client', async () => { const oldStorage = { meta: { version: oldVersion }, data: { @@ -1261,4 +1261,41 @@ describe('migration #137', () => { }, }); }); + + it('returns original state if an error occurs during mutation', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + PermissionController: { + subjects: { + 'test.com': { + permissions: { + eth_accounts: { + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef'], + }, + ], + parentCapability: 'eth_accounts', + }, + }, + }, + }, + }, + NetworkController: { + selectedNetworkClientId: 'invalid-network-client-id', // This will cause an error + networkConfigurationsByChainId: {}, + }, + SelectedNetworkController: { + domains: {}, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + + expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(expect.any(Error)); + expect(newStorage.data).toStrictEqual(oldStorage.data); + }); }); diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index cda364057f00..59823450963d 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -52,6 +52,21 @@ const BUILT_IN_NETWORKS: ReadonlyMap = new Map([ const snapsPrefixes = ['npm:', 'local:'] as const; +interface MetaMaskState { + [key: string]: unknown; + + PermissionController: { + subjects: Record; + }; + NetworkController: { + selectedNetworkClientId: string; + networkConfigurationsByChainId: Record; + }; + SelectedNetworkController: { + domains: Record; + }; +} + /** * This migration transforms `eth_accounts` and `permittedChains` permissions into * an equivalent CAIP-25 permission. @@ -69,45 +84,41 @@ export async function migrate( ): Promise { const versionedData = cloneDeep(originalVersionedData); versionedData.meta.version = version; - transformState(versionedData.data); + + const newState = transformState(versionedData.data); + versionedData.data = newState as Record; return versionedData; } function transformState(state: Record) { - // **Step 1: Validate all subjects first** - - // Validate the existence and types of controllers if ( !hasProperty(state, 'PermissionController') || !isObject(state.PermissionController) ) { - global.sentry?.captureException?.( - new Error( - `Migration ${version}: typeof state.PermissionController is ${typeof state.PermissionController}`, - ), + console.warn( + `Migration ${version}: typeof state.PermissionController is ${typeof state.PermissionController}`, ); return state; } + // TODO do we need to have the SelectedNetworkController in order to migrate? if ( - !hasProperty(state, 'NetworkController') || - !isObject(state.NetworkController) + !hasProperty(state, 'SelectedNetworkController') || + !isObject(state.SelectedNetworkController) ) { - global.sentry?.captureException?.( - new Error( - `Migration ${version}: typeof state.NetworkController is ${typeof state.NetworkController}`, - ), + console.warn( + `Migration ${version}: typeof state.SelectedNetworkController is ${typeof state.SelectedNetworkController}`, ); return state; } if ( - !hasProperty(state, 'SelectedNetworkController') || - !isObject(state.SelectedNetworkController) + !hasProperty(state, 'NetworkController') || + !isObject(state.NetworkController) ) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.SelectedNetworkController is ${typeof state.SelectedNetworkController}`, + `Migration ${version}: typeof state.NetworkController is ${typeof state.NetworkController}`, ), ); return state; @@ -122,7 +133,6 @@ function transformState(state: Record) { SelectedNetworkController: { domains }, } = state; - // Validate required properties if (!isObject(subjects)) { global.sentry?.captureException?.( new Error( @@ -132,14 +142,14 @@ function transformState(state: Record) { return state; } - if (!selectedNetworkClientId || typeof selectedNetworkClientId !== 'string') { - global.sentry?.captureException?.( - new Error( - `Migration ${version}: invalid selectedNetworkClientId "${selectedNetworkClientId}"`, - ), - ); - return state; - } + // if (!selectedNetworkClientId || typeof selectedNetworkClientId !== 'string') { + // global.sentry?.captureException?.( + // new Error( + // `Migration ${version}: invalid selectedNetworkClientId "${selectedNetworkClientId}"`, + // ), + // ); + // return state; + // } if (!isObject(networkConfigurationsByChainId)) { global.sentry?.captureException?.( @@ -159,7 +169,6 @@ function transformState(state: Record) { return state; } - // Validate all subjects before making any changes for (const [origin, subject] of Object.entries(subjects)) { if (!isObject(subject)) { global.sentry?.captureException?.( @@ -183,180 +192,168 @@ function transformState(state: Record) { } } -interface MetaMaskState { - PermissionController: { - subjects: Record; - }; - NetworkController: { - selectedNetworkClientId: string; - networkConfigurationsByChainId: Record; - }; - SelectedNetworkController: { - domains: Record; - }; -} - - const newState: MetaMaskState = cloneDeep(state) as MetaMaskState; - - const { - PermissionController: { subjects: newSubjects }, - NetworkController: { - selectedNetworkClientId: newSelectedNetworkClientId, - networkConfigurationsByChainId: newNetworkConfigurationsByChainId, - }, - SelectedNetworkController: { domains: newDomains }, - } = newState; - - // **Step 3: Perform all mutations on the cloned state** - - const getChainIdForNetworkClientId = ( - networkClientId: string, - propertyName: string, - ): string | undefined => { - for (const [chainId, networkConfiguration] of Object.entries( - newNetworkConfigurationsByChainId, - )) { - if (!isObject(networkConfiguration)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, - ), - ); - return undefined; - } - if (!Array.isArray(networkConfiguration.rpcEndpoints)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, - ), - ); - return undefined; - } - for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { - if (!isObject(rpcEndpoint)) { + const newState = cloneDeep(state) as MetaMaskState; + + try { + const { + PermissionController: { subjects: newSubjects }, + NetworkController: { + selectedNetworkClientId: newSelectedNetworkClientId, + networkConfigurationsByChainId: newNetworkConfigurationsByChainId, + }, + SelectedNetworkController: { domains: newDomains }, + } = newState; + + const getChainIdForNetworkClientId = ( + networkClientId: string, + propertyName: string, + ): string | undefined => { + for (const [chainId, networkConfiguration] of Object.entries( + newNetworkConfigurationsByChainId, + )) { + if (!isObject(networkConfiguration)) { global.sentry?.captureException( new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, ), ); return undefined; } - if (rpcEndpoint.networkClientId === networkClientId) { - return chainId; + if (!Array.isArray(networkConfiguration.rpcEndpoints)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, + ), + ); + return undefined; + } + for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { + if (!isObject(rpcEndpoint)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, + ), + ); + return undefined; + } + if (rpcEndpoint.networkClientId === networkClientId) { + return chainId; + } } } - } - const builtInChainId = BUILT_IN_NETWORKS.get(networkClientId); - if (!builtInChainId) { - global.sentry?.captureException( - new Error( - `Migration ${version}: No chainId found for ${propertyName} "${networkClientId}"`, - ), - ); - } - return builtInChainId; - }; - - const currentChainId = getChainIdForNetworkClientId( - newSelectedNetworkClientId, - 'selectedNetworkClientId', - ); - if (!currentChainId) { - return state; - } - - for (const [origin, subject] of Object.entries(newSubjects)) { - const { permissions } = subject as { - permissions: Record; + const builtInChainId = BUILT_IN_NETWORKS.get(networkClientId); + if (!builtInChainId) { + global.sentry?.captureException( + new Error( + `Migration ${version}: No chainId found for ${propertyName} "${networkClientId}"`, + ), + ); + } + return builtInChainId; }; - let basePermission: PermissionConstraint | undefined; - - let ethAccounts: string[] = []; - if ( - isObject(permissions[PermissionNames.eth_accounts]) && - Array.isArray(permissions[PermissionNames.eth_accounts].caveats) - ) { - ethAccounts = - (permissions[PermissionNames.eth_accounts].caveats?.[0]?.value as - | string[] - | undefined) ?? []; - basePermission = permissions[PermissionNames.eth_accounts]; - } - delete permissions[PermissionNames.eth_accounts]; - - let chainIds: string[] = []; - if ( - isObject(permissions[PermissionNames.permittedChains]) && - Array.isArray(permissions[PermissionNames.permittedChains].caveats) - ) { - chainIds = - (permissions[PermissionNames.permittedChains].caveats?.[0]?.value as - | string[] - | undefined) ?? []; - basePermission ??= permissions[PermissionNames.permittedChains]; + const currentChainId = getChainIdForNetworkClientId( + newSelectedNetworkClientId, + 'selectedNetworkClientId', + ); + if (!currentChainId) { + return state; } - delete permissions[PermissionNames.permittedChains]; - if (ethAccounts.length === 0 || !basePermission) { - continue; - } + // Perform mutations on cloned state + for (const [origin, subject] of Object.entries(newSubjects)) { + const { permissions } = subject; - if (chainIds.length === 0) { - chainIds = [currentChainId]; + let basePermission: PermissionConstraint | undefined; - const networkClientIdForOrigin = newDomains[origin]; + let ethAccounts: string[] = []; if ( - networkClientIdForOrigin && - typeof networkClientIdForOrigin === 'string' + isObject(permissions[PermissionNames.eth_accounts]) && + Array.isArray(permissions[PermissionNames.eth_accounts].caveats) ) { - const chainIdForOrigin = getChainIdForNetworkClientId( - networkClientIdForOrigin, - 'networkClientIdForOrigin', - ); - if (chainIdForOrigin) { - chainIds = [chainIdForOrigin]; + ethAccounts = + (permissions[PermissionNames.eth_accounts].caveats?.[0]?.value as + | string[] + | undefined) ?? []; + basePermission = permissions[PermissionNames.eth_accounts]; + } + delete permissions[PermissionNames.eth_accounts]; + + let chainIds: string[] = []; + if ( + isObject(permissions[PermissionNames.permittedChains]) && + Array.isArray(permissions[PermissionNames.permittedChains].caveats) + ) { + chainIds = + (permissions[PermissionNames.permittedChains].caveats?.[0]?.value as + | string[] + | undefined) ?? []; + basePermission ??= permissions[PermissionNames.permittedChains]; + } + delete permissions[PermissionNames.permittedChains]; + + if (ethAccounts.length === 0 || !basePermission) { + continue; + } + + if (chainIds.length === 0) { + chainIds = [currentChainId]; + + const networkClientIdForOrigin = newDomains[origin]; + if ( + networkClientIdForOrigin && + typeof networkClientIdForOrigin === 'string' + ) { + const chainIdForOrigin = getChainIdForNetworkClientId( + networkClientIdForOrigin, + 'networkClientIdForOrigin', + ); + if (chainIdForOrigin) { + chainIds = [chainIdForOrigin]; + } } } - } - const isSnap = snapsPrefixes.some((prefix) => origin.startsWith(prefix)); - const scopes: InternalScopesObject = {}; - const scopeStrings: CaipChainId[] = isSnap - ? [] - : chainIds.map( - (chainId) => `eip155:${hexToBigInt(chainId).toString(10)}`, - ); - scopeStrings.push('wallet:eip155'); + const isSnap = snapsPrefixes.some((prefix) => origin.startsWith(prefix)); + const scopes: InternalScopesObject = {}; + const scopeStrings: CaipChainId[] = isSnap + ? [] + : chainIds.map( + (chainId) => `eip155:${hexToBigInt(chainId).toString(10)}`, + ); + scopeStrings.push('wallet:eip155'); - scopeStrings.forEach((scopeString) => { - const caipAccounts = ethAccounts.map( - (account) => `${scopeString}:${account}`, - ); - scopes[scopeString] = { - accounts: caipAccounts, - }; - }); - - const caip25Permission: Caip25Permission = { - ...basePermission, - parentCapability: Caip25EndowmentPermissionName, - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: scopes, - isMultichainOrigin: false, + scopeStrings.forEach((scopeString) => { + const caipAccounts = ethAccounts.map( + (account) => `${scopeString}:${account}`, + ); + scopes[scopeString] = { + accounts: caipAccounts, + }; + }); + + const caip25Permission: Caip25Permission = { + ...basePermission, + parentCapability: Caip25EndowmentPermissionName, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: scopes, + isMultichainOrigin: false, + }, }, - }, - ], - }; + ], + }; - permissions[Caip25EndowmentPermissionName] = caip25Permission; - } + permissions[Caip25EndowmentPermissionName] = caip25Permission; + } - // **Step 4: Return either original state or fully transformed state** - return newState; + return newState; + } catch (error) { + global.sentry?.captureException?.(error); + return state; + } } From 1b91f9092c20cd7f10c2ad74a676fcef411020ab Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 14:51:18 -0600 Subject: [PATCH 03/12] fix up --- app/scripts/migrations/137.ts | 381 +++++++++++++++++----------------- 1 file changed, 191 insertions(+), 190 deletions(-) diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 59823450963d..982cb7347b5f 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -52,21 +52,6 @@ const BUILT_IN_NETWORKS: ReadonlyMap = new Map([ const snapsPrefixes = ['npm:', 'local:'] as const; -interface MetaMaskState { - [key: string]: unknown; - - PermissionController: { - subjects: Record; - }; - NetworkController: { - selectedNetworkClientId: string; - networkConfigurationsByChainId: Record; - }; - SelectedNetworkController: { - domains: Record; - }; -} - /** * This migration transforms `eth_accounts` and `permittedChains` permissions into * an equivalent CAIP-25 permission. @@ -101,75 +86,92 @@ function transformState(state: Record) { return state; } - // TODO do we need to have the SelectedNetworkController in order to migrate? if ( - !hasProperty(state, 'SelectedNetworkController') || - !isObject(state.SelectedNetworkController) + !hasProperty(state, 'NetworkController') || + !isObject(state.NetworkController) ) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: typeof state.NetworkController is ${typeof state.NetworkController}`, + ), + ); + return state; + } + + if (!hasProperty(state, 'SelectedNetworkController')) { console.warn( `Migration ${version}: typeof state.SelectedNetworkController is ${typeof state.SelectedNetworkController}`, ); - return state; + state.SelectedNetworkController = { + domains: {}, + }; } - if ( - !hasProperty(state, 'NetworkController') || - !isObject(state.NetworkController) - ) { + // const { + // PermissionController: { subjects }, + // NetworkController: { + // selectedNetworkClientId, + // networkConfigurationsByChainId, + // }, + // SelectedNetworkController: { domains }, + // } = state; + + // legitimate state corruption error, would need to fix once we see this error hit + if (!isObject(state.PermissionController.subjects)) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.NetworkController is ${typeof state.NetworkController}`, + `Migration ${version}: typeof state.PermissionController.subjects is ${typeof state + .PermissionController.subjects}`, ), ); return state; } - const { - PermissionController: { subjects }, - NetworkController: { - selectedNetworkClientId, - networkConfigurationsByChainId, - }, - SelectedNetworkController: { domains }, - } = state; - - if (!isObject(subjects)) { + // legitimate state corruption error, would need to fix once we see this error hit + if ( + !state.NetworkController.selectedNetworkClientId || + typeof state.NetworkController.selectedNetworkClientId !== 'string' + ) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.PermissionController.subjects is ${typeof subjects}`, + `Migration ${version}: invalid selectedNetworkClientId "${state.NetworkController.selectedNetworkClientId}"`, ), ); return state; } - // if (!selectedNetworkClientId || typeof selectedNetworkClientId !== 'string') { - // global.sentry?.captureException?.( - // new Error( - // `Migration ${version}: invalid selectedNetworkClientId "${selectedNetworkClientId}"`, - // ), - // ); - // return state; - // } - - if (!isObject(networkConfigurationsByChainId)) { + // legitimate state corruption error, would need to fix once we see this error hit + if ( + !hasProperty(state.NetworkController, 'networkConfigurationsByChainId') || + !isObject(state.NetworkController.networkConfigurationsByChainId) + ) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof networkConfigurationsByChainId}`, + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof state + .NetworkController.networkConfigurationsByChainId}`, ), ); return state; } - if (!isObject(domains)) { + // legitimate state corruption error, would need to fix once we see this error hit + if ( + !hasProperty(state.SelectedNetworkController as any, 'domains') || + !isObject((state.SelectedNetworkController as any).domains) + ) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.SelectedNetworkController.domains is ${typeof domains}`, + `Migration ${version}: typeof state.SelectedNetworkController.domains is ${typeof ( + state.SelectedNetworkController as any + ).domains}`, ), ); return state; } - for (const [origin, subject] of Object.entries(subjects)) { + for (const [origin, subject] of Object.entries( + state.PermissionController.subjects, + )) { if (!isObject(subject)) { global.sentry?.captureException?.( new Error( @@ -192,168 +194,167 @@ function transformState(state: Record) { } } - const newState = cloneDeep(state) as MetaMaskState; - - try { - const { - PermissionController: { subjects: newSubjects }, - NetworkController: { - selectedNetworkClientId: newSelectedNetworkClientId, - networkConfigurationsByChainId: newNetworkConfigurationsByChainId, - }, - SelectedNetworkController: { domains: newDomains }, - } = newState; - - const getChainIdForNetworkClientId = ( - networkClientId: string, - propertyName: string, - ): string | undefined => { - for (const [chainId, networkConfiguration] of Object.entries( - newNetworkConfigurationsByChainId, - )) { - if (!isObject(networkConfiguration)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, - ), - ); - return undefined; - } - if (!Array.isArray(networkConfiguration.rpcEndpoints)) { + const newState = cloneDeep(state); + + // if ( + // !hasProperty(newState, 'PermissionController') || + // !isObject(newState.PermissionController) || + // !hasProperty(newState.PermissionController, 'subjects') || + // !isObject(newState.PermissionController.subjects) + // ) { + // return state; + // } + + const { + PermissionController: { subjects: newSubjects }, + } = newState; + + const getChainIdForNetworkClientId = ( + networkClientId: string, + propertyName: string, + ): string | undefined => { + for (const [chainId, networkConfiguration] of Object.entries( + state.NetworkController.networkConfigurationsByChainId, + )) { + if (!isObject(networkConfiguration)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, + ), + ); + return undefined; + } + if (!Array.isArray(networkConfiguration.rpcEndpoints)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, + ), + ); + return undefined; + } + for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { + if (!isObject(rpcEndpoint)) { global.sentry?.captureException( new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, ), ); return undefined; } - for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { - if (!isObject(rpcEndpoint)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, - ), - ); - return undefined; - } - if (rpcEndpoint.networkClientId === networkClientId) { - return chainId; - } + if (rpcEndpoint.networkClientId === networkClientId) { + return chainId; } } + } - const builtInChainId = BUILT_IN_NETWORKS.get(networkClientId); - if (!builtInChainId) { - global.sentry?.captureException( - new Error( - `Migration ${version}: No chainId found for ${propertyName} "${networkClientId}"`, - ), - ); - } - return builtInChainId; - }; - - const currentChainId = getChainIdForNetworkClientId( - newSelectedNetworkClientId, - 'selectedNetworkClientId', - ); - if (!currentChainId) { - return state; + const builtInChainId = BUILT_IN_NETWORKS.get(networkClientId); + if (!builtInChainId) { + global.sentry?.captureException( + new Error( + `Migration ${version}: No chainId found for ${propertyName} "${networkClientId}"`, + ), + ); } + return builtInChainId; + }; - // Perform mutations on cloned state - for (const [origin, subject] of Object.entries(newSubjects)) { - const { permissions } = subject; + const currentChainId = getChainIdForNetworkClientId( + selectedNetworkClientId, + 'selectedNetworkClientId', + ); + if (!currentChainId) { + return state; + } - let basePermission: PermissionConstraint | undefined; + // Perform mutations on cloned state + for (const [origin, subject] of Object.entries(newSubjects)) { + const { permissions } = subject; + + let basePermission: PermissionConstraint | undefined; + + let ethAccounts: string[] = []; + if ( + isObject(permissions[PermissionNames.eth_accounts]) && + Array.isArray(permissions[PermissionNames.eth_accounts].caveats) + ) { + ethAccounts = + (permissions[PermissionNames.eth_accounts].caveats?.[0]?.value as + | string[] + | undefined) ?? []; + basePermission = permissions[PermissionNames.eth_accounts]; + } + delete permissions[PermissionNames.eth_accounts]; + + let chainIds: string[] = []; + if ( + isObject(permissions[PermissionNames.permittedChains]) && + Array.isArray(permissions[PermissionNames.permittedChains].caveats) + ) { + chainIds = + (permissions[PermissionNames.permittedChains].caveats?.[0]?.value as + | string[] + | undefined) ?? []; + basePermission ??= permissions[PermissionNames.permittedChains]; + } + delete permissions[PermissionNames.permittedChains]; - let ethAccounts: string[] = []; - if ( - isObject(permissions[PermissionNames.eth_accounts]) && - Array.isArray(permissions[PermissionNames.eth_accounts].caveats) - ) { - ethAccounts = - (permissions[PermissionNames.eth_accounts].caveats?.[0]?.value as - | string[] - | undefined) ?? []; - basePermission = permissions[PermissionNames.eth_accounts]; - } - delete permissions[PermissionNames.eth_accounts]; + if (ethAccounts.length === 0 || !basePermission) { + continue; + } + + if (chainIds.length === 0) { + chainIds = [currentChainId]; - let chainIds: string[] = []; + const networkClientIdForOrigin = newDomains[origin]; if ( - isObject(permissions[PermissionNames.permittedChains]) && - Array.isArray(permissions[PermissionNames.permittedChains].caveats) + networkClientIdForOrigin && + typeof networkClientIdForOrigin === 'string' ) { - chainIds = - (permissions[PermissionNames.permittedChains].caveats?.[0]?.value as - | string[] - | undefined) ?? []; - basePermission ??= permissions[PermissionNames.permittedChains]; - } - delete permissions[PermissionNames.permittedChains]; - - if (ethAccounts.length === 0 || !basePermission) { - continue; - } - - if (chainIds.length === 0) { - chainIds = [currentChainId]; - - const networkClientIdForOrigin = newDomains[origin]; - if ( - networkClientIdForOrigin && - typeof networkClientIdForOrigin === 'string' - ) { - const chainIdForOrigin = getChainIdForNetworkClientId( - networkClientIdForOrigin, - 'networkClientIdForOrigin', - ); - if (chainIdForOrigin) { - chainIds = [chainIdForOrigin]; - } + const chainIdForOrigin = getChainIdForNetworkClientId( + networkClientIdForOrigin, + 'networkClientIdForOrigin', + ); + if (chainIdForOrigin) { + chainIds = [chainIdForOrigin]; } } + } - const isSnap = snapsPrefixes.some((prefix) => origin.startsWith(prefix)); - const scopes: InternalScopesObject = {}; - const scopeStrings: CaipChainId[] = isSnap - ? [] - : chainIds.map( - (chainId) => `eip155:${hexToBigInt(chainId).toString(10)}`, - ); - scopeStrings.push('wallet:eip155'); - - scopeStrings.forEach((scopeString) => { - const caipAccounts = ethAccounts.map( - (account) => `${scopeString}:${account}`, + const isSnap = snapsPrefixes.some((prefix) => origin.startsWith(prefix)); + const scopes: InternalScopesObject = {}; + const scopeStrings: CaipChainId[] = isSnap + ? [] + : chainIds.map( + (chainId) => `eip155:${hexToBigInt(chainId).toString(10)}`, ); - scopes[scopeString] = { - accounts: caipAccounts, - }; - }); - - const caip25Permission: Caip25Permission = { - ...basePermission, - parentCapability: Caip25EndowmentPermissionName, - caveats: [ - { - type: Caip25CaveatType, - value: { - requiredScopes: {}, - optionalScopes: scopes, - isMultichainOrigin: false, - }, - }, - ], - }; + scopeStrings.push('wallet:eip155'); - permissions[Caip25EndowmentPermissionName] = caip25Permission; - } + scopeStrings.forEach((scopeString) => { + const caipAccounts = ethAccounts.map( + (account) => `${scopeString}:${account}`, + ); + scopes[scopeString] = { + accounts: caipAccounts, + }; + }); + + const caip25Permission: Caip25Permission = { + ...basePermission, + parentCapability: Caip25EndowmentPermissionName, + caveats: [ + { + type: Caip25CaveatType, + value: { + requiredScopes: {}, + optionalScopes: scopes, + isMultichainOrigin: false, + }, + }, + ], + }; - return newState; - } catch (error) { - global.sentry?.captureException?.(error); - return state; + permissions[Caip25EndowmentPermissionName] = caip25Permission; } + + return newState; } From dcda8655abc024d3e3e9f96c5970fbcc802b6a2e Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 15:37:52 -0600 Subject: [PATCH 04/12] resolving typing, but stuff is broken --- app/scripts/migrations/137.ts | 165 +++++++++++++++++++--------------- 1 file changed, 92 insertions(+), 73 deletions(-) diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 982cb7347b5f..aebf066c0742 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -107,44 +107,61 @@ function transformState(state: Record) { }; } - // const { - // PermissionController: { subjects }, - // NetworkController: { - // selectedNetworkClientId, - // networkConfigurationsByChainId, - // }, - // SelectedNetworkController: { domains }, - // } = state; + const { + // PermissionController: { subjects }, + NetworkController: { + selectedNetworkClientId, + networkConfigurationsByChainId, + }, + } = state; + + // // legitimate state corruption error, would need to fix once we see this error hit + // if (!isObject(subjects)) { + // global.sentry?.captureException?.( + // new Error( + // `Migration ${version}: typeof state.PermissionController.subjects is ${typeof state + // .PermissionController.subjects}`, + // ), + // ); + // return state; + // } - // legitimate state corruption error, would need to fix once we see this error hit - if (!isObject(state.PermissionController.subjects)) { - global.sentry?.captureException?.( - new Error( - `Migration ${version}: typeof state.PermissionController.subjects is ${typeof state - .PermissionController.subjects}`, - ), - ); - return state; - } + // // legitimate state corruption error, would need to fix once we see this error hit + // for (const [origin, subject] of Object.entries(subjects)) { + // if (!isObject(subject)) { + // global.sentry?.captureException?.( + // new Error( + // `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, + // ), + // ); + // return state; + // } + + // if ( + // !hasProperty(subject, 'permissions') || + // !isObject(subject.permissions) + // ) { + // global.sentry?.captureException?.( + // new Error( + // `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof subject.permissions}`, + // ), + // ); + // return state; + // } + // } // legitimate state corruption error, would need to fix once we see this error hit - if ( - !state.NetworkController.selectedNetworkClientId || - typeof state.NetworkController.selectedNetworkClientId !== 'string' - ) { + if (!selectedNetworkClientId || typeof selectedNetworkClientId !== 'string') { global.sentry?.captureException?.( new Error( - `Migration ${version}: invalid selectedNetworkClientId "${state.NetworkController.selectedNetworkClientId}"`, + `Migration ${version}: invalid selectedNetworkClientId "${selectedNetworkClientId}"`, ), ); return state; } // legitimate state corruption error, would need to fix once we see this error hit - if ( - !hasProperty(state.NetworkController, 'networkConfigurationsByChainId') || - !isObject(state.NetworkController.networkConfigurationsByChainId) - ) { + if (!isObject(networkConfigurationsByChainId)) { global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof state @@ -169,52 +186,26 @@ function transformState(state: Record) { return state; } - for (const [origin, subject] of Object.entries( - state.PermissionController.subjects, - )) { - if (!isObject(subject)) { - global.sentry?.captureException?.( - new Error( - `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, - ), - ); - return state; - } - - const { permissions } = subject as { - permissions: Record; - }; - if (!isObject(permissions)) { - global.sentry?.captureException?.( - new Error( - `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof permissions}`, - ), - ); - return state; - } - } - const newState = cloneDeep(state); - // if ( - // !hasProperty(newState, 'PermissionController') || - // !isObject(newState.PermissionController) || - // !hasProperty(newState.PermissionController, 'subjects') || - // !isObject(newState.PermissionController.subjects) - // ) { - // return state; - // } - const { PermissionController: { subjects: newSubjects }, - } = newState; + } = newState as { + PermissionController: { + subjects: Record; + }; + }; + + const { domains } = state.SelectedNetworkController as { + domains: Record; + }; const getChainIdForNetworkClientId = ( networkClientId: string, propertyName: string, ): string | undefined => { for (const [chainId, networkConfiguration] of Object.entries( - state.NetworkController.networkConfigurationsByChainId, + networkConfigurationsByChainId, )) { if (!isObject(networkConfiguration)) { global.sentry?.captureException( @@ -268,33 +259,58 @@ function transformState(state: Record) { // Perform mutations on cloned state for (const [origin, subject] of Object.entries(newSubjects)) { + if (!isObject(subject)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, + ), + ); + return state; + } + + if ( + !hasProperty(subject, 'permissions') || + !isObject(subject.permissions) + ) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof subject.permissions}`, + ), + ); + return state; + } + const { permissions } = subject; let basePermission: PermissionConstraint | undefined; let ethAccounts: string[] = []; + const ethAccountsPermission = permissions[PermissionNames.eth_accounts]; if ( - isObject(permissions[PermissionNames.eth_accounts]) && - Array.isArray(permissions[PermissionNames.eth_accounts].caveats) + isObject(ethAccountsPermission) && + hasProperty(ethAccountsPermission, 'caveats') && + Array.isArray(ethAccountsPermission.caveats) ) { ethAccounts = - (permissions[PermissionNames.eth_accounts].caveats?.[0]?.value as + (ethAccountsPermission.caveats?.[0]?.value as | string[] | undefined) ?? []; - basePermission = permissions[PermissionNames.eth_accounts]; + // basePermission = ethAccountsPermission; } delete permissions[PermissionNames.eth_accounts]; let chainIds: string[] = []; + const permittedChainsPermission = permissions[PermissionNames.permittedChains]; if ( - isObject(permissions[PermissionNames.permittedChains]) && - Array.isArray(permissions[PermissionNames.permittedChains].caveats) + isObject(permittedChainsPermission) && + hasProperty(permittedChainsPermission, 'caveats') && + Array.isArray(permittedChainsPermission.caveats) ) { chainIds = - (permissions[PermissionNames.permittedChains].caveats?.[0]?.value as + (permittedChainsPermission.caveats?.[0]?.value as | string[] | undefined) ?? []; - basePermission ??= permissions[PermissionNames.permittedChains]; + // basePermission ??= permissions[PermissionNames.permittedChains]; } delete permissions[PermissionNames.permittedChains]; @@ -305,7 +321,7 @@ function transformState(state: Record) { if (chainIds.length === 0) { chainIds = [currentChainId]; - const networkClientIdForOrigin = newDomains[origin]; + const networkClientIdForOrigin = domains[origin]; if ( networkClientIdForOrigin && typeof networkClientIdForOrigin === 'string' @@ -339,7 +355,10 @@ function transformState(state: Record) { }); const caip25Permission: Caip25Permission = { - ...basePermission, + // ...basePermission, + id: '1', // for now + invoker: '1', // for now + date: new Date().getTime(), // for now parentCapability: Caip25EndowmentPermissionName, caveats: [ { From 0861ed976132e45527a190729ef35020ef9b0b3d Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 13 Jan 2025 17:31:58 -0600 Subject: [PATCH 05/12] tests passing --- app/scripts/migrations/137.test.ts | 253 ++++++++++------------------- app/scripts/migrations/137.ts | 188 ++++++++++----------- 2 files changed, 172 insertions(+), 269 deletions(-) diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index bc7c9bb6bbb8..a422bb9942c0 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -104,25 +104,6 @@ describe('migration #137', () => { expect(newStorage.data).toStrictEqual(oldStorage.data); }); - it('does nothing if SelectedNetworkController state is missing', async () => { - const oldStorage = { - meta: { version: oldVersion }, - data: { - PermissionController: {}, - NetworkController: {}, - }, - }; - - const newStorage = await migrate(oldStorage); - - expect(sentryCaptureExceptionMock).toHaveBeenCalledWith( - new Error( - `Migration ${version}: typeof state.SelectedNetworkController is undefined`, - ), - ); - expect(newStorage.data).toStrictEqual(oldStorage.data); - }); - it('does nothing if SelectedNetworkController state is not an object', async () => { const oldStorage = { meta: { version: oldVersion }, @@ -516,8 +497,72 @@ describe('migration #137', () => { domains: {}, }, }); + const basePermission = { + id: '1', + date: 2, + invoker: 'test.com', + parentCapability: PermissionNames.eth_accounts, + }; const currentScope = `eip155:${chainId}`; + it('skips eth_accounts and permittedChains permissions when they are missing an id', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + ...baseData(), + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + [PermissionNames.eth_accounts]: { + invoker: 'test.com', + parentCapability: PermissionNames.eth_accounts, + date: 2, + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef', '0x999'], + }, + ], + }, + [PermissionNames.permittedChains]: { + invoker: 'test.com', + parentCapability: PermissionNames.permittedChains, + date: 2, + caveats: [ + { + type: 'restrictNetworkSwitching', + value: ['0xa', '0x64'], + }, + ], + }, + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + ...baseData(), + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + }, + }, + }, + }, + }); + }); + it('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does not have its own network client', async () => { const oldStorage = { meta: { version: oldVersion }, @@ -531,6 +576,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -556,6 +602,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -588,7 +635,7 @@ describe('migration #137', () => { }); }); - it('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does have its own network client that cannot be resolved', async () => { + it('replaces the eth_accounts permission with a CAIP-25 permission using the globally selected chain id value for the currently selected chain id when the origin does have its own network client that cannot be resolved', async () => { const oldStorage = { meta: { version: oldVersion }, data: { @@ -606,6 +653,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -643,6 +691,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -693,6 +742,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -723,6 +773,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -768,6 +819,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -793,6 +845,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -850,6 +903,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -893,6 +947,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -938,6 +993,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.permittedChains]: { + ...basePermission, caveats: [ { type: 'restrictNetworkSwitching', @@ -982,6 +1038,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -990,6 +1047,7 @@ describe('migration #137', () => { ], }, [PermissionNames.permittedChains]: { + ...basePermission, caveats: [ { type: 'restrictNetworkSwitching', @@ -1015,6 +1073,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -1063,6 +1122,7 @@ describe('migration #137', () => { 'test.com': { permissions: { [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -1075,6 +1135,7 @@ describe('migration #137', () => { 'test2.com': { permissions: { [PermissionNames.eth_accounts]: { + ...basePermission, caveats: [ { type: 'restrictReturnedAccounts', @@ -1097,6 +1158,7 @@ describe('migration #137', () => { 'test.com': { permissions: { 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -1121,6 +1183,7 @@ describe('migration #137', () => { 'test2.com': { permissions: { 'endowment:caip25': { + ...basePermission, parentCapability: 'endowment:caip25', caveats: [ { @@ -1148,154 +1211,4 @@ describe('migration #137', () => { }); }, ); - - it('skips malformed subjects while migrating valid ones', async () => { - const baseData = () => ({ - PermissionController: { - subjects: {}, - }, - SelectedNetworkController: { - domains: {}, - }, - NetworkController: { - selectedNetworkClientId: 'mainnet', - networkConfigurationsByChainId: {}, - }, - }); - const oldStorage = { - meta: { version: oldVersion }, - data: { - ...baseData(), - PermissionController: { - subjects: { - 'test.com': { - permissions: { - eth_accounts: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0xdeadbeef'], - }, - ], - parentCapability: 'eth_accounts', - }, - }, - }, - 'malformed.com': 'invalid-subject', // This should be skipped - 'valid.com': { - permissions: { - eth_accounts: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0x123'], - }, - ], - parentCapability: 'eth_accounts', - }, - }, - }, - }, - }, - }, - }; - - const newStorage = await migrate(oldStorage); - - expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(expect.any(Error)); - - expect(newStorage.data).toStrictEqual({ - ...baseData(), - PermissionController: { - subjects: { - 'test.com': { - permissions: { - 'endowment:caip25': { - parentCapability: 'endowment:caip25', - caveats: [ - { - type: 'authorizedScopes', - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0xdeadbeef'], - }, - 'eip155:1': { - accounts: ['eip155:1:0xdeadbeef'], - }, - }, - isMultichainOrigin: false, - }, - }, - ], - }, - }, - }, - 'malformed.com': 'invalid-subject', - 'valid.com': { - permissions: { - 'endowment:caip25': { - parentCapability: 'endowment:caip25', - caveats: [ - { - type: 'authorizedScopes', - value: { - requiredScopes: {}, - optionalScopes: { - 'wallet:eip155': { - accounts: ['wallet:eip155:0x123'], - }, - 'eip155:1': { - accounts: ['eip155:1:0x123'], - }, - }, - isMultichainOrigin: false, - }, - }, - ], - }, - }, - }, - }, - }, - }); - }); - - it('returns original state if an error occurs during mutation', async () => { - const oldStorage = { - meta: { version: oldVersion }, - data: { - PermissionController: { - subjects: { - 'test.com': { - permissions: { - eth_accounts: { - caveats: [ - { - type: 'restrictReturnedAccounts', - value: ['0xdeadbeef'], - }, - ], - parentCapability: 'eth_accounts', - }, - }, - }, - }, - }, - NetworkController: { - selectedNetworkClientId: 'invalid-network-client-id', // This will cause an error - networkConfigurationsByChainId: {}, - }, - SelectedNetworkController: { - domains: {}, - }, - }, - }; - - const newStorage = await migrate(oldStorage); - - expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(expect.any(Error)); - expect(newStorage.data).toStrictEqual(oldStorage.data); - }); }); diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index aebf066c0742..0b331a70cf20 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -52,6 +52,24 @@ const BUILT_IN_NETWORKS: ReadonlyMap = new Map([ const snapsPrefixes = ['npm:', 'local:'] as const; +function isPermissionConstraint(obj: unknown): obj is PermissionConstraint { + return ( + isObject(obj) && + obj !== null && + hasProperty(obj, 'caveats') && + Array.isArray(obj.caveats) && + obj.caveats.length > 0 && + hasProperty(obj, 'date') && + typeof obj.date === 'number' && + hasProperty(obj, 'id') && + typeof obj.id === 'string' && + hasProperty(obj, 'invoker') && + typeof obj.invoker === 'string' && + hasProperty(obj, 'parentCapability') && + typeof obj.parentCapability === 'string' + ); +} + /** * This migration transforms `eth_accounts` and `permittedChains` permissions into * an equivalent CAIP-25 permission. @@ -75,128 +93,109 @@ export async function migrate( return versionedData; } -function transformState(state: Record) { +function transformState(oldState: Record) { + const newState = cloneDeep(oldState); if ( - !hasProperty(state, 'PermissionController') || - !isObject(state.PermissionController) + !hasProperty(newState, 'PermissionController') || + !isObject(newState.PermissionController) ) { - console.warn( - `Migration ${version}: typeof state.PermissionController is ${typeof state.PermissionController}`, + global.sentry?.captureException?.( + new Error( + `Migration ${version}: typeof state.PermissionController is ${typeof newState.PermissionController}`, + ), ); - return state; + return oldState; } if ( - !hasProperty(state, 'NetworkController') || - !isObject(state.NetworkController) + !hasProperty(newState, 'NetworkController') || + !isObject(newState.NetworkController) ) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.NetworkController is ${typeof state.NetworkController}`, + `Migration ${version}: typeof state.NetworkController is ${typeof newState.NetworkController}`, ), ); - return state; + return newState; } - if (!hasProperty(state, 'SelectedNetworkController')) { + if (!hasProperty(newState, 'SelectedNetworkController')) { console.warn( - `Migration ${version}: typeof state.SelectedNetworkController is ${typeof state.SelectedNetworkController}`, + `Migration ${version}: typeof state.SelectedNetworkController is ${typeof newState.SelectedNetworkController}`, ); - state.SelectedNetworkController = { + newState.SelectedNetworkController = { domains: {}, }; } + if (!isObject(newState.SelectedNetworkController)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: typeof state.SelectedNetworkController is ${typeof newState.SelectedNetworkController}`, + ), + ); + return oldState; + } + const { - // PermissionController: { subjects }, NetworkController: { selectedNetworkClientId, networkConfigurationsByChainId, }, - } = state; - - // // legitimate state corruption error, would need to fix once we see this error hit - // if (!isObject(subjects)) { - // global.sentry?.captureException?.( - // new Error( - // `Migration ${version}: typeof state.PermissionController.subjects is ${typeof state - // .PermissionController.subjects}`, - // ), - // ); - // return state; - // } - - // // legitimate state corruption error, would need to fix once we see this error hit - // for (const [origin, subject] of Object.entries(subjects)) { - // if (!isObject(subject)) { - // global.sentry?.captureException?.( - // new Error( - // `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, - // ), - // ); - // return state; - // } - - // if ( - // !hasProperty(subject, 'permissions') || - // !isObject(subject.permissions) - // ) { - // global.sentry?.captureException?.( - // new Error( - // `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof subject.permissions}`, - // ), - // ); - // return state; - // } - // } - - // legitimate state corruption error, would need to fix once we see this error hit + PermissionController: { subjects }, + } = newState; + + if (!isObject(subjects)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: typeof state.PermissionController.subjects is ${typeof subjects}`, + ), + ); + return oldState; + } + if (!selectedNetworkClientId || typeof selectedNetworkClientId !== 'string') { global.sentry?.captureException?.( new Error( - `Migration ${version}: invalid selectedNetworkClientId "${selectedNetworkClientId}"`, + `Migration ${version}: typeof state.NetworkController.selectedNetworkClientId is ${typeof selectedNetworkClientId}`, ), ); - return state; + return oldState; } - // legitimate state corruption error, would need to fix once we see this error hit if (!isObject(networkConfigurationsByChainId)) { global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof state + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId is ${typeof newState .NetworkController.networkConfigurationsByChainId}`, ), ); - return state; + return oldState; } - // legitimate state corruption error, would need to fix once we see this error hit if ( - !hasProperty(state.SelectedNetworkController as any, 'domains') || - !isObject((state.SelectedNetworkController as any).domains) + !hasProperty(newState.SelectedNetworkController, 'domains') || + !isObject(newState.SelectedNetworkController.domains) ) { + const domains = newState.SelectedNetworkController.domains; global.sentry?.captureException?.( new Error( - `Migration ${version}: typeof state.SelectedNetworkController.domains is ${typeof ( - state.SelectedNetworkController as any - ).domains}`, + `Migration ${version}: typeof state.SelectedNetworkController.domains is ${typeof domains}`, ), ); - return state; + return oldState; } - const newState = cloneDeep(state); - - const { - PermissionController: { subjects: newSubjects }, - } = newState as { - PermissionController: { - subjects: Record; - }; - }; + if (!isObject(newState.PermissionController)) { + global.sentry?.captureException?.( + new Error( + `Migration ${version}: typeof state.PermissionController is ${typeof newState.PermissionController}`, + ), + ); + return oldState; + } - const { domains } = state.SelectedNetworkController as { + const { domains } = newState.SelectedNetworkController as { domains: Record; }; @@ -254,18 +253,18 @@ function transformState(state: Record) { 'selectedNetworkClientId', ); if (!currentChainId) { - return state; + return oldState; } - // Perform mutations on cloned state - for (const [origin, subject] of Object.entries(newSubjects)) { + // perform mutations on the cloned state + for (const [origin, subject] of Object.entries(subjects)) { if (!isObject(subject)) { global.sentry?.captureException?.( new Error( `Migration ${version}: Invalid subject for origin "${origin}" of type ${typeof subject}`, ), ); - return state; + return oldState; } if ( @@ -277,7 +276,7 @@ function transformState(state: Record) { `Migration ${version}: Invalid permissions for origin "${origin}" of type ${typeof subject.permissions}`, ), ); - return state; + return oldState; } const { permissions } = subject; @@ -286,31 +285,25 @@ function transformState(state: Record) { let ethAccounts: string[] = []; const ethAccountsPermission = permissions[PermissionNames.eth_accounts]; - if ( - isObject(ethAccountsPermission) && - hasProperty(ethAccountsPermission, 'caveats') && - Array.isArray(ethAccountsPermission.caveats) - ) { + if (isPermissionConstraint(ethAccountsPermission)) { ethAccounts = - (ethAccountsPermission.caveats?.[0]?.value as - | string[] - | undefined) ?? []; - // basePermission = ethAccountsPermission; + (ethAccountsPermission.caveats?.[0]?.value as string[] | undefined) ?? + []; + basePermission = ethAccountsPermission; } + delete permissions[PermissionNames.eth_accounts]; let chainIds: string[] = []; - const permittedChainsPermission = permissions[PermissionNames.permittedChains]; - if ( - isObject(permittedChainsPermission) && - hasProperty(permittedChainsPermission, 'caveats') && - Array.isArray(permittedChainsPermission.caveats) - ) { + const permittedChainsPermission = + permissions[PermissionNames.permittedChains]; + if (isPermissionConstraint(permittedChainsPermission)) { chainIds = (permittedChainsPermission.caveats?.[0]?.value as | string[] | undefined) ?? []; - // basePermission ??= permissions[PermissionNames.permittedChains]; + + basePermission ??= permittedChainsPermission; } delete permissions[PermissionNames.permittedChains]; @@ -355,10 +348,7 @@ function transformState(state: Record) { }); const caip25Permission: Caip25Permission = { - // ...basePermission, - id: '1', // for now - invoker: '1', // for now - date: new Date().getTime(), // for now + ...basePermission, parentCapability: Caip25EndowmentPermissionName, caveats: [ { From f8c0379418e7ac5199d527c20d57280aa423cb09 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 Jan 2025 10:07:45 -0600 Subject: [PATCH 06/12] move networkConfiguration validation --- app/scripts/migrations/137.ts | 60 ++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 0b331a70cf20..bab19b78b304 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -173,6 +173,45 @@ function transformState(oldState: Record) { return oldState; } + for (const [chainId, networkConfiguration] of Object.entries( + networkConfigurationsByChainId, + )) { + if (!isObject(networkConfiguration)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, + ), + ); + continue; + } + if (!hasProperty(networkConfiguration, 'rpcEndpoints')) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, + ), + ); + continue; + } + + if (!Array.isArray(networkConfiguration.rpcEndpoints)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, + ), + ); + continue; + } + for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { + if (!isObject(rpcEndpoint)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, + ), + ); + } + } + } + if ( !hasProperty(newState.SelectedNetworkController, 'domains') || !isObject(newState.SelectedNetworkController.domains) @@ -207,29 +246,14 @@ function transformState(oldState: Record) { networkConfigurationsByChainId, )) { if (!isObject(networkConfiguration)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, - ), - ); - return undefined; + continue; } if (!Array.isArray(networkConfiguration.rpcEndpoints)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, - ), - ); - return undefined; + continue; } for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { if (!isObject(rpcEndpoint)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, - ), - ); - return undefined; + continue; } if (rpcEndpoint.networkClientId === networkClientId) { return chainId; From 0950b222b55ae2664b03eeacb0ed10d3b655902f Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 Jan 2025 10:17:36 -0600 Subject: [PATCH 07/12] dont short circuit networkConfigurations validation loop --- app/scripts/migrations/137.ts | 54 ++++++++++------------------------- 1 file changed, 15 insertions(+), 39 deletions(-) diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index bab19b78b304..1fe7995c5c2b 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -173,45 +173,6 @@ function transformState(oldState: Record) { return oldState; } - for (const [chainId, networkConfiguration] of Object.entries( - networkConfigurationsByChainId, - )) { - if (!isObject(networkConfiguration)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, - ), - ); - continue; - } - if (!hasProperty(networkConfiguration, 'rpcEndpoints')) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, - ), - ); - continue; - } - - if (!Array.isArray(networkConfiguration.rpcEndpoints)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, - ), - ); - continue; - } - for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { - if (!isObject(rpcEndpoint)) { - global.sentry?.captureException( - new Error( - `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, - ), - ); - } - } - } - if ( !hasProperty(newState.SelectedNetworkController, 'domains') || !isObject(newState.SelectedNetworkController.domains) @@ -246,13 +207,28 @@ function transformState(oldState: Record) { networkConfigurationsByChainId, )) { if (!isObject(networkConfiguration)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"] is ${typeof networkConfiguration}`, + ), + ); continue; } if (!Array.isArray(networkConfiguration.rpcEndpoints)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints is ${typeof networkConfiguration.rpcEndpoints}`, + ), + ); continue; } for (const rpcEndpoint of networkConfiguration.rpcEndpoints) { if (!isObject(rpcEndpoint)) { + global.sentry?.captureException( + new Error( + `Migration ${version}: typeof state.NetworkController.networkConfigurationsByChainId["${chainId}"].rpcEndpoints[] is ${typeof rpcEndpoint}`, + ), + ); continue; } if (rpcEndpoint.networkClientId === networkClientId) { From 16e9b7b4588f1e839652d29217d1ca9484c69cc7 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 Jan 2025 10:34:40 -0600 Subject: [PATCH 08/12] add test for malformed networkConfigurations --- app/scripts/migrations/137.test.ts | 122 +++++++++++++++++++++++++++++ app/scripts/migrations/137.ts | 7 ++ 2 files changed, 129 insertions(+) diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index a422bb9942c0..ac7fdf163552 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -563,6 +563,128 @@ describe('migration #137', () => { }); }); + it('resolves the network client id for the origin even if there are other malformed network configurations', async () => { + const oldStorage = { + meta: { version: oldVersion }, + data: { + ...baseData(), + NetworkController: { + selectedNetworkClientId: 'mainnet', + networkConfigurationsByChainId: { + '0xInvalid': 'invalid-network-configuration', + '0x1': { + rpcEndpoints: [ + { + networkClientId: 'mainnet', + }, + ], + }, + '0xa': { + rpcEndpoints: [ + { + networkClientId: 'bar', + }, + ], + }, + }, + }, + SelectedNetworkController: { + domains: { + 'test.com': 'bar', + }, + }, + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + [PermissionNames.eth_accounts]: { + ...basePermission, + caveats: [ + { + type: 'restrictReturnedAccounts', + value: ['0xdeadbeef', '0x999'], + }, + ], + }, + }, + }, + }, + }, + }, + }; + + const newStorage = await migrate(oldStorage); + expect(newStorage.data).toStrictEqual({ + ...baseData(), + NetworkController: { + selectedNetworkClientId: 'mainnet', + networkConfigurationsByChainId: { + '0xInvalid': 'invalid-network-configuration', + '0x1': { + rpcEndpoints: [ + { + networkClientId: 'mainnet', + }, + ], + }, + '0xa': { + rpcEndpoints: [ + { + networkClientId: 'bar', + }, + ], + }, + }, + }, + PermissionController: { + subjects: { + 'test.com': { + permissions: { + unrelated: { + foo: 'bar', + }, + 'endowment:caip25': { + ...basePermission, + parentCapability: 'endowment:caip25', + caveats: [ + { + type: 'authorizedScopes', + value: { + isMultichainOrigin: false, + requiredScopes: {}, + optionalScopes: { + 'eip155:10': { + accounts: [ + 'eip155:10:0xdeadbeef', + 'eip155:10:0x999', + ], + }, + 'wallet:eip155': { + accounts: [ + 'wallet:eip155:0xdeadbeef', + 'wallet:eip155:0x999', + ], + }, + }, + }, + }, + ], + }, + }, + }, + }, + }, + SelectedNetworkController: { + domains: { + 'test.com': 'bar', + }, + }, + }); + }); + it('replaces the eth_accounts permission with a CAIP-25 permission using the eth_accounts value for the currently selected chain id when the origin does not have its own network client', async () => { const oldStorage = { meta: { version: oldVersion }, diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 1fe7995c5c2b..4d2f194bab77 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -145,6 +145,8 @@ function transformState(oldState: Record) { PermissionController: { subjects }, } = newState; + console.log('subjects', subjects); + if (!isObject(subjects)) { global.sentry?.captureException?.( new Error( @@ -199,6 +201,8 @@ function transformState(oldState: Record) { domains: Record; }; + console.log('domains', domains); + const getChainIdForNetworkClientId = ( networkClientId: string, propertyName: string, @@ -206,6 +210,8 @@ function transformState(oldState: Record) { for (const [chainId, networkConfiguration] of Object.entries( networkConfigurationsByChainId, )) { + console.log('chainId', chainId); + console.log('networkConfiguration', networkConfiguration); if (!isObject(networkConfiguration)) { global.sentry?.captureException( new Error( @@ -232,6 +238,7 @@ function transformState(oldState: Record) { continue; } if (rpcEndpoint.networkClientId === networkClientId) { + console.log('returning chainId', chainId); return chainId; } } From 2c554d1dacc710e1d42705069e0dfd634babcb08 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 Jan 2025 10:36:40 -0600 Subject: [PATCH 09/12] cleanup --- app/scripts/migrations/137.test.ts | 2 +- app/scripts/migrations/137.ts | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index ac7fdf163552..d590d3251767 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -563,7 +563,7 @@ describe('migration #137', () => { }); }); - it('resolves the network client id for the origin even if there are other malformed network configurations', async () => { + it('resolves a chainId for the origin even if there are other malformed network configurations', async () => { const oldStorage = { meta: { version: oldVersion }, data: { diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 4d2f194bab77..1fe7995c5c2b 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -145,8 +145,6 @@ function transformState(oldState: Record) { PermissionController: { subjects }, } = newState; - console.log('subjects', subjects); - if (!isObject(subjects)) { global.sentry?.captureException?.( new Error( @@ -201,8 +199,6 @@ function transformState(oldState: Record) { domains: Record; }; - console.log('domains', domains); - const getChainIdForNetworkClientId = ( networkClientId: string, propertyName: string, @@ -210,8 +206,6 @@ function transformState(oldState: Record) { for (const [chainId, networkConfiguration] of Object.entries( networkConfigurationsByChainId, )) { - console.log('chainId', chainId); - console.log('networkConfiguration', networkConfiguration); if (!isObject(networkConfiguration)) { global.sentry?.captureException( new Error( @@ -238,7 +232,6 @@ function transformState(oldState: Record) { continue; } if (rpcEndpoint.networkClientId === networkClientId) { - console.log('returning chainId', chainId); return chainId; } } From 53224d8d6e376c208aec452443ead771f4ae87ed Mon Sep 17 00:00:00 2001 From: Alex Donesky Date: Tue, 14 Jan 2025 11:50:02 -0600 Subject: [PATCH 10/12] Update app/scripts/migrations/137.test.ts Co-authored-by: jiexi --- app/scripts/migrations/137.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index d590d3251767..5c56f8f4092f 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -505,7 +505,7 @@ describe('migration #137', () => { }; const currentScope = `eip155:${chainId}`; - it('skips eth_accounts and permittedChains permissions when they are missing an id', async () => { + it('skips eth_accounts and permittedChains permissions when they are missing metadata', async () => { const oldStorage = { meta: { version: oldVersion }, data: { From e1b700671e105bc15064ee35eea21db41ba10bfd Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 Jan 2025 11:55:02 -0600 Subject: [PATCH 11/12] basePermission -> baseEthAccountsPermissionMetadata --- app/scripts/migrations/137.test.ts | 42 +++++++++++++++--------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/app/scripts/migrations/137.test.ts b/app/scripts/migrations/137.test.ts index 5c56f8f4092f..69735752f4db 100644 --- a/app/scripts/migrations/137.test.ts +++ b/app/scripts/migrations/137.test.ts @@ -497,7 +497,7 @@ describe('migration #137', () => { domains: {}, }, }); - const basePermission = { + const baseEthAccountsPermissionMetadata = { id: '1', date: 2, invoker: 'test.com', @@ -601,7 +601,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -647,7 +647,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -698,7 +698,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -724,7 +724,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -775,7 +775,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -813,7 +813,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -864,7 +864,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -895,7 +895,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -941,7 +941,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -967,7 +967,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -1025,7 +1025,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -1069,7 +1069,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -1115,7 +1115,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.permittedChains]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictNetworkSwitching', @@ -1160,7 +1160,7 @@ describe('migration #137', () => { foo: 'bar', }, [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -1169,7 +1169,7 @@ describe('migration #137', () => { ], }, [PermissionNames.permittedChains]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictNetworkSwitching', @@ -1195,7 +1195,7 @@ describe('migration #137', () => { foo: 'bar', }, 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -1244,7 +1244,7 @@ describe('migration #137', () => { 'test.com': { permissions: { [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -1257,7 +1257,7 @@ describe('migration #137', () => { 'test2.com': { permissions: { [PermissionNames.eth_accounts]: { - ...basePermission, + ...baseEthAccountsPermissionMetadata, caveats: [ { type: 'restrictReturnedAccounts', @@ -1280,7 +1280,7 @@ describe('migration #137', () => { 'test.com': { permissions: { 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { @@ -1305,7 +1305,7 @@ describe('migration #137', () => { 'test2.com': { permissions: { 'endowment:caip25': { - ...basePermission, + ...baseEthAccountsPermissionMetadata, parentCapability: 'endowment:caip25', caveats: [ { From c7a9704a98850392f1fe291ab7fa9155eac14c97 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 14 Jan 2025 12:06:16 -0600 Subject: [PATCH 12/12] fix lint --- app/scripts/migrations/137.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/scripts/migrations/137.ts b/app/scripts/migrations/137.ts index 1fe7995c5c2b..e25ce81d03ed 100644 --- a/app/scripts/migrations/137.ts +++ b/app/scripts/migrations/137.ts @@ -177,7 +177,7 @@ function transformState(oldState: Record) { !hasProperty(newState.SelectedNetworkController, 'domains') || !isObject(newState.SelectedNetworkController.domains) ) { - const domains = newState.SelectedNetworkController.domains; + const { domains } = newState.SelectedNetworkController; global.sentry?.captureException?.( new Error( `Migration ${version}: typeof state.SelectedNetworkController.domains is ${typeof domains}`, @@ -195,9 +195,7 @@ function transformState(oldState: Record) { return oldState; } - const { domains } = newState.SelectedNetworkController as { - domains: Record; - }; + const { domains } = newState.SelectedNetworkController; const getChainIdForNetworkClientId = ( networkClientId: string,