From 154285568eff82f61187597cc9d60e7c28adafc5 Mon Sep 17 00:00:00 2001 From: salimtb Date: Tue, 17 Dec 2024 12:21:40 +0100 Subject: [PATCH] fix: Add migration to remove tokens with null decimals and log affected tokens --- app/store/migrations/64.test.ts | 137 ++++++++++++++++++++++++++++++ app/store/migrations/64.ts | 145 ++++++++++++++++++++++++++++++++ 2 files changed, 282 insertions(+) create mode 100644 app/store/migrations/64.test.ts create mode 100644 app/store/migrations/64.ts diff --git a/app/store/migrations/64.test.ts b/app/store/migrations/64.test.ts new file mode 100644 index 00000000000..3b27a1c4563 --- /dev/null +++ b/app/store/migrations/64.test.ts @@ -0,0 +1,137 @@ +import migrate from './64'; +import { captureException, captureMessage } from '@sentry/react-native'; +import { merge } from 'lodash'; +import initialRootState from '../../util/test/initial-root-state'; + +jest.mock('@sentry/react-native', () => ({ + captureException: jest.fn(), + captureMessage: jest.fn(), +})); + +const mockedCaptureException = jest.mocked(captureException); +const mockedCaptureMessage = jest.mocked(captureMessage); + +const createToken = (address: string, decimals: number | null) => ({ + address, + decimals, +}); + +describe('Migration #64', () => { + const migrationVersion = 64; + + beforeEach(() => { + jest.restoreAllMocks(); + jest.resetAllMocks(); + }); + + const invalidStates = [ + { + state: merge({}, initialRootState, { + engine: { backgroundState: { TokensController: null } }, + }), + errorMessage: `Migration ${migrationVersion}: Invalid TokensController state: '${migrationVersion}'`, + scenario: 'TokensController state is null', + }, + { + state: merge({}, initialRootState, { + engine: { backgroundState: { TokensController: { allTokens: null } } }, + }), + errorMessage: `Migration ${migrationVersion}: Missing allTokens property from TokensController: 'object'`, + scenario: 'allTokens is null', + }, + ]; + + it.each(invalidStates)( + 'captures exception if $scenario', + ({ errorMessage, state }) => { + const newState = migrate(state); + + expect(newState).toStrictEqual(state); + expect(mockedCaptureException).toHaveBeenCalledWith(expect.any(Error)); + expect(mockedCaptureException.mock.calls[0][0].message).toBe( + errorMessage, + ); + }, + ); + + it('removes tokens with decimals === null from allTokens, allDetectedTokens, tokens, and detectedTokens', () => { + const oldState = merge({}, initialRootState, { + engine: { + backgroundState: { + TokensController: { + allTokens: { + '1': { + '0xAccount1': [ + createToken('0xToken1', 18), + createToken('0xToken2', null), // Should be removed + ], + }, + }, + allDetectedTokens: { + '1': { + '0xAccount2': [ + createToken('0xToken3', 18), + createToken('0xToken4', null), // Should be removed + ], + }, + }, + tokens: [ + createToken('0xToken5', 18), + createToken('0xToken6', null), // Should be removed + ], + detectedTokens: [ + createToken('0xToken7', 18), + createToken('0xToken8', null), // Should be removed + ], + }, + }, + }, + }); + + const expectedState = merge({}, initialRootState, { + engine: { + backgroundState: { + TokensController: { + allTokens: { + '1': { + '0xAccount1': [createToken('0xToken1', 18)], + }, + }, + allDetectedTokens: { + '1': { + '0xAccount2': [createToken('0xToken3', 18)], + }, + }, + tokens: [createToken('0xToken5', 18)], + detectedTokens: [createToken('0xToken7', 18)], + }, + }, + }, + }); + + const newState = migrate(oldState); + + expect(newState).toStrictEqual(expectedState); + + // Verify that captureMessage was called for each removed token + expect(mockedCaptureMessage).toHaveBeenCalledTimes(4); + expect(mockedCaptureMessage).toHaveBeenCalledWith( + expect.stringContaining( + `Removed token with decimals === null in allTokens`, + ), + ); + expect(mockedCaptureMessage).toHaveBeenCalledWith( + expect.stringContaining( + `Removed token with decimals === null in allDetectedTokens`, + ), + ); + expect(mockedCaptureMessage).toHaveBeenCalledWith( + expect.stringContaining(`Removed token with decimals === null in tokens`), + ); + expect(mockedCaptureMessage).toHaveBeenCalledWith( + expect.stringContaining( + `Removed token with decimals === null in detectedTokens`, + ), + ); + }); +}); diff --git a/app/store/migrations/64.ts b/app/store/migrations/64.ts new file mode 100644 index 00000000000..c7335b4a265 --- /dev/null +++ b/app/store/migrations/64.ts @@ -0,0 +1,145 @@ +import { hasProperty, isObject } from '@metamask/utils'; +import { captureException, captureMessage } from '@sentry/react-native'; +import { ensureValidState } from './util'; + +const migrationVersion = 64; + +/** + * Migration to remove tokens with `decimals === null` from token-related properties + * such as `allTokens`, `allDetectedTokens`, `tokens`, and `detectedTokens` in the + * `TokensController` state. + * + * @param state - The current MetaMask extension state. + * @returns The updated state with tokens having `decimals === null` removed. + */ +export default function migrate(state: unknown) { + // Ensure the state is valid for migration + if (!ensureValidState(state, migrationVersion)) { + return state; + } + + const TokensController = state.engine.backgroundState.TokensController; + + if (!isObject(TokensController)) { + captureException( + new Error( + `Migration ${migrationVersion}: Invalid TokensController state: '${migrationVersion}'`, + ), + ); + return state; + } + + if (!isObject(TokensController.allTokens)) { + captureException( + new Error( + `Migration ${migrationVersion}: Missing allTokens property from TokensController: '${typeof state + .engine.backgroundState.TokensController}'`, + ), + ); + return state; + } + + TokensController.allTokens = transformTokenCollection( + TokensController.allTokens, + 'allTokens', + ); + + if (!isObject(TokensController.allDetectedTokens)) { + captureException( + new Error( + `Migration ${migrationVersion}: Missing allDetectedTokens property from TokensController: '${typeof state + .engine.backgroundState.TokensController}'`, + ), + ); + return state; + } + + TokensController.allDetectedTokens = transformTokenCollection( + TokensController.allDetectedTokens, + 'allDetectedTokens', + ); + + if (!Array.isArray(TokensController.tokens)) { + captureException( + new Error( + `Migration ${migrationVersion}: Missing tokens property from TokensController: '${typeof state + .engine.backgroundState.TokensController}'`, + ), + ); + return state; + } + + TokensController.tokens = TokensController.tokens.filter((token) => + validateAndLogToken(token, 'tokens'), + ); + + if (!Array.isArray(TokensController.detectedTokens)) { + captureException( + new Error( + `Migration ${migrationVersion}: Missing tokens property from TokensController: '${typeof state + .engine.backgroundState.TokensController}'`, + ), + ); + return state; + } + + TokensController.detectedTokens = TokensController.detectedTokens.filter( + (token) => validateAndLogToken(token, 'detectedTokens'), + ); + + return state; +} + +/** + * Transforms a token collection to remove tokens with `decimals === null` and logs their removal. + * + * @param tokenCollection - The token collection to transform. + * @param propertyName - The name of the property being transformed (for logging purposes). + * @returns The updated token collection. + */ +function transformTokenCollection( + tokenCollection: Record, + propertyName: string, +): Record { + const updatedCollection: Record = {}; + + for (const [chainId, accounts] of Object.entries(tokenCollection)) { + if (isObject(accounts)) { + const updatedAccounts: Record = {}; + + for (const [account, tokens] of Object.entries(accounts)) { + if (Array.isArray(tokens)) { + updatedAccounts[account] = tokens.filter((token) => + validateAndLogToken(token, `${propertyName} - chainId: ${chainId}`), + ); + } + } + + updatedCollection[chainId] = updatedAccounts; + } + } + + return updatedCollection; +} + +/** + * Validates a token object and logs its removal if `decimals === null`. + * + * @param token - The token object to validate. + * @param propertyName - The property name or context for logging. + * @returns `true` if the token is valid, `false` otherwise. + */ +function validateAndLogToken(token: unknown, propertyName: string): boolean { + if ( + isObject(token) && + hasProperty(token, 'decimals') && + token.decimals === null && + hasProperty(token, 'address') + ) { + captureMessage( + `Migration ${migrationVersion}: Removed token with decimals === null in ${propertyName}. Address: ${token.address}`, + ); + return false; + } + return true; +}