-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Add migration to remove tokens with null decimals and log affect…
…ed tokens
- Loading branch information
Showing
2 changed files
with
282 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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`, | ||
), | ||
); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<string, unknown>, | ||
propertyName: string, | ||
): Record<string, unknown> { | ||
const updatedCollection: Record<string, unknown> = {}; | ||
|
||
for (const [chainId, accounts] of Object.entries(tokenCollection)) { | ||
if (isObject(accounts)) { | ||
const updatedAccounts: Record<string, unknown[]> = {}; | ||
|
||
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; | ||
} |