From fed305e46bed9d549238ba0fcce00854135b6abd Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Wed, 12 Jun 2024 15:23:47 -0600 Subject: [PATCH 01/11] Enable `resetMocks` Jest configuration option Jest has a configuration option `resetMocks`. Equivalent to calling `jest.resetAllMocks()` before each test, it helps to ensure that tests are run in isolation by removing fake implementations for each registered mock function, thereby resetting its state. This option is enabled by default for new Jest projects but has been disabled in this repo for a very long time. For some test suites, `jest.resetAllMocks()` (or some variant) has been added to a `beforeEach` or `afterEach` to simulate this option, but overall this is not the case, and some tests were written which assumed that mock functions were not being reset between tests. This commit enables the aforementioned configuration option, fixes tests that fail as a result of this, and removes manual calls to `jest.resetAllMocks()` (or the like). --- jest.config.packages.js | 3 +- jest.config.scripts.js | 4 + .../src/AccountsController.test.ts | 11 - .../src/ApprovalController.test.ts | 8 +- .../src/CurrencyRateController.test.ts | 1 - .../RatesController/RatesController.test.ts | 6 - .../src/TokenListController.test.ts | 1 - .../src/TokenRatesController.test.ts | 4 - .../src/TokensController.test.ts | 1 - .../src/GasFeeController.test.ts | 261 ++++++++++++++---- .../gas-fee-controller/src/gas-util.test.ts | 4 - .../src/LoggingController.test.ts | 37 ++- .../src/LoggingController.ts | 5 + .../src/NameController.test.ts | 14 +- .../name-controller/src/providers/ens.test.ts | 4 - .../src/providers/etherscan.test.ts | 4 - .../src/providers/lens.test.ts | 4 - .../src/providers/token.test.ts | 4 - .../tests/NetworkController.test.ts | 4 - .../src/PermissionController.test.ts | 4 - .../tests/SelectedNetworkController.test.ts | 10 - .../src/SignatureController.test.ts | 1 - .../src/TransactionController.test.ts | 4 - .../src/gas-flows/LineaGasFeeFlow.test.ts | 2 - .../EtherscanRemoteTransactionSource.test.ts | 2 - .../src/helpers/GasFeePoller.test.ts | 1 - .../helpers/IncomingTransactionHelper.test.ts | 4 - .../helpers/MultichainTrackingHelper.test.ts | 2 - .../helpers/PendingTransactionTracker.test.ts | 2 - .../src/utils/etherscan.test.ts | 4 - .../src/utils/gas.test.ts | 2 - .../src/utils/simulation-api.test.ts | 2 - .../src/utils/simulation.test.ts | 1 - .../src/utils/utils.test.ts | 4 - .../src/utils/validation.test.ts | 4 - .../src/UserOperationController.test.ts | 2 - .../PendingUserOperationTracker.test.ts | 1 - .../helpers/SnapSmartContractAccount.test.ts | 2 - .../src/utils/gas-fees.test.ts | 2 - scripts/create-package/commands.test.ts | 4 - scripts/create-package/utils.test.ts | 4 - 41 files changed, 257 insertions(+), 187 deletions(-) diff --git a/jest.config.packages.js b/jest.config.packages.js index 89c610463f1..96bde23bfc7 100644 --- a/jest.config.packages.js +++ b/jest.config.packages.js @@ -108,8 +108,7 @@ module.exports = { // "resetMocks" resets all mocks, including mocked modules, to jest.fn(), // between each test case. - // TODO: Enable - // resetMocks: true, + resetMocks: true, // Reset the module registry before running each individual test // resetModules: false, diff --git a/jest.config.scripts.js b/jest.config.scripts.js index a86dd27b949..fe5792a9b6b 100644 --- a/jest.config.scripts.js +++ b/jest.config.scripts.js @@ -50,6 +50,10 @@ module.exports = { // // A preset that is used as a base for Jest's configuration // preset: 'ts-jest', + // "resetMocks" resets all mocks, including mocked modules, to jest.fn(), + // between each test case. + resetMocks: true, + // "restoreMocks" restores all mocks created using jest.spyOn to their // original implementations, between each test. It does not affect mocked // modules. diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index 8f0bc38b2a5..08d5804902a 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -315,10 +315,6 @@ function setupAccountsController({ } describe('AccountsController', () => { - afterEach(() => { - jest.resetAllMocks(); - }); - describe('onSnapStateChange', () => { it('be used enable an account if the Snap is enabled and not blocked', async () => { const messenger = buildMessenger(); @@ -449,9 +445,6 @@ describe('AccountsController', () => { }); describe('onKeyringStateChange', () => { - afterEach(() => { - jest.clearAllMocks(); - }); it('not update state when only keyring is unlocked without any keyrings', async () => { const messenger = buildMessenger(); const { accountsController } = setupAccountsController({ @@ -1304,10 +1297,6 @@ describe('AccountsController', () => { ); }); - afterEach(() => { - jest.clearAllMocks(); - }); - it('update accounts with normal accounts', async () => { mockUUID.mockReturnValueOnce('mock-id').mockReturnValueOnce('mock-id2'); const messenger = buildMessenger(); diff --git a/packages/approval-controller/src/ApprovalController.test.ts b/packages/approval-controller/src/ApprovalController.test.ts index 11cb6a8b265..fe190895280 100644 --- a/packages/approval-controller/src/ApprovalController.test.ts +++ b/packages/approval-controller/src/ApprovalController.test.ts @@ -2,6 +2,7 @@ import { ControllerMessenger } from '@metamask/base-controller'; import { errorCodes, JsonRpcError } from '@metamask/rpc-errors'; +import { nanoid } from 'nanoid'; import type { AddApprovalOptions, @@ -24,9 +25,9 @@ import { NoApprovalFlowsError, } from './errors'; -jest.mock('nanoid', () => ({ - nanoid: jest.fn(() => 'TestId'), -})); +jest.mock('nanoid'); + +const nanoidMock = jest.mocked(nanoid); const PENDING_APPROVALS_STORE_KEY = 'pendingApprovals'; const APPROVAL_FLOWS_STORE_KEY = 'approvalFlows'; @@ -243,6 +244,7 @@ describe('approval controller', () => { let showApprovalRequest: jest.Mock; beforeEach(() => { + nanoidMock.mockReturnValue('TestId'); jest.spyOn(global.console, 'info').mockImplementation(() => undefined); showApprovalRequest = jest.fn(); diff --git a/packages/assets-controllers/src/CurrencyRateController.test.ts b/packages/assets-controllers/src/CurrencyRateController.test.ts index d322e4be3bf..0dd2e82aa8d 100644 --- a/packages/assets-controllers/src/CurrencyRateController.test.ts +++ b/packages/assets-controllers/src/CurrencyRateController.test.ts @@ -75,7 +75,6 @@ describe('CurrencyRateController', () => { afterEach(() => { clock.restore(); - jest.restoreAllMocks(); }); it('should set default state', () => { diff --git a/packages/assets-controllers/src/RatesController/RatesController.test.ts b/packages/assets-controllers/src/RatesController/RatesController.test.ts index aea62d27be0..17e00f33266 100644 --- a/packages/assets-controllers/src/RatesController/RatesController.test.ts +++ b/packages/assets-controllers/src/RatesController/RatesController.test.ts @@ -90,10 +90,6 @@ function setupRatesController({ describe('RatesController', () => { let clock: sinon.SinonFakeTimers; - afterEach(() => { - jest.resetAllMocks(); - }); - describe('construct', () => { it('constructs the RatesController with default values', () => { const ratesController = setupRatesController({ @@ -116,7 +112,6 @@ describe('RatesController', () => { afterEach(() => { clock.restore(); - jest.restoreAllMocks(); }); it('starts the polling process with default values', async () => { @@ -249,7 +244,6 @@ describe('RatesController', () => { afterEach(() => { clock.restore(); - jest.restoreAllMocks(); }); it('stops the polling process', async () => { diff --git a/packages/assets-controllers/src/TokenListController.test.ts b/packages/assets-controllers/src/TokenListController.test.ts index 790f68b045c..d6f04de76d4 100644 --- a/packages/assets-controllers/src/TokenListController.test.ts +++ b/packages/assets-controllers/src/TokenListController.test.ts @@ -517,7 +517,6 @@ const getRestrictedMessenger = ( describe('TokenListController', () => { afterEach(() => { - jest.restoreAllMocks(); jest.clearAllTimers(); sinon.restore(); }); diff --git a/packages/assets-controllers/src/TokenRatesController.test.ts b/packages/assets-controllers/src/TokenRatesController.test.ts index df8833ff4e7..b42cd28469d 100644 --- a/packages/assets-controllers/src/TokenRatesController.test.ts +++ b/packages/assets-controllers/src/TokenRatesController.test.ts @@ -79,10 +79,6 @@ function buildTokenRatesControllerMessenger( } describe('TokenRatesController', () => { - afterEach(() => { - jest.restoreAllMocks(); - }); - describe('constructor', () => { let clock: sinon.SinonFakeTimers; diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index 2d8fb47cb17..dfa25e6bc6a 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -68,7 +68,6 @@ describe('TokensController', () => { afterEach(() => { sinon.restore(); - jest.resetAllMocks(); }); it('should set default state', async () => { diff --git a/packages/gas-fee-controller/src/GasFeeController.test.ts b/packages/gas-fee-controller/src/GasFeeController.test.ts index 2c4d59b02af..84d25a03dc8 100644 --- a/packages/gas-fee-controller/src/GasFeeController.test.ts +++ b/packages/gas-fee-controller/src/GasFeeController.test.ts @@ -298,7 +298,6 @@ describe('GasFeeController', () => { // eslint-disable-next-line @typescript-eslint/no-floating-promises blockTracker?.destroy(); sinon.restore(); - jest.clearAllMocks(); }); describe('constructor', () => { @@ -735,12 +734,6 @@ describe('GasFeeController', () => { describe('fetchGasFeeEstimates', () => { describe('when on any network supporting legacy gas estimation api', () => { - const defaultConstructorOptions = { - getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), - getCurrentNetworkLegacyGasAPICompatibility: jest - .fn() - .mockReturnValue(true), - }; const mockDetermineGasFeeCalculations = buildMockGasFeeStateLegacy(); beforeEach(() => { @@ -751,7 +744,10 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -792,7 +788,12 @@ describe('GasFeeController', () => { }); it('should update the state with a fetched set of estimates', async () => { - await setupGasFeeController(defaultConstructorOptions); + await setupGasFeeController({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), + }); await gasFeeController.fetchGasFeeEstimates(); @@ -802,7 +803,12 @@ describe('GasFeeController', () => { }); it('should return the same data that it puts into state', async () => { - await setupGasFeeController(defaultConstructorOptions); + await setupGasFeeController({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), + }); const estimateData = await gasFeeController.fetchGasFeeEstimates(); @@ -811,7 +817,10 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a number input', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), getChainId: jest.fn().mockReturnValue(1), }); @@ -826,7 +835,10 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a hexstring input', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), getChainId: jest.fn().mockReturnValue('0x1'), }); @@ -841,7 +853,10 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when nonRPCGasFeeApisDisabled is true', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), state: { ...buildMockGasFeeStateEthGasPrice(), nonRPCGasFeeApisDisabled: true, @@ -859,7 +874,10 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when nonRPCGasFeeApisDisabled is false', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), state: { ...buildMockGasFeeStateEthGasPrice(), nonRPCGasFeeApisDisabled: false, @@ -877,7 +895,10 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly when getChainId returns a numeric string input', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(false), + getCurrentNetworkLegacyGasAPICompatibility: jest + .fn() + .mockReturnValue(true), getChainId: jest.fn().mockReturnValue('1'), }); @@ -892,9 +913,6 @@ describe('GasFeeController', () => { }); describe('when on any network supporting EIP-1559', () => { - const defaultConstructorOptions = { - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - }; const mockDetermineGasFeeCalculations = buildMockGasFeeStateFeeMarket(); beforeEach(() => { @@ -905,7 +923,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), networkControllerState: { networkConfigurations: { 'AAAA-BBBB-CCCC-DDDD': { @@ -946,7 +964,9 @@ describe('GasFeeController', () => { }); it('should update the state with a fetched set of estimates', async () => { - await setupGasFeeController(defaultConstructorOptions); + await setupGasFeeController({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + }); await gasFeeController.fetchGasFeeEstimates(); @@ -956,7 +976,9 @@ describe('GasFeeController', () => { }); it('should return the same data that it puts into state', async () => { - await setupGasFeeController(defaultConstructorOptions); + await setupGasFeeController({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + }); const estimateData = await gasFeeController.fetchGasFeeEstimates(); @@ -965,7 +987,7 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations with a URL that contains the chain ID', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), getChainId: jest.fn().mockReturnValue('0x1'), }); @@ -979,31 +1001,6 @@ describe('GasFeeController', () => { }); }); describe('when passed a networkClientId in options object', () => { - const defaultConstructorOptions = { - getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), - networkControllerState: { - networksMetadata: { - goerli: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - sepolia: { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - 'test-network-client-id': { - EIPS: { - 1559: true, - }, - status: NetworkStatus.Available, - }, - }, - }, - }; const mockDetermineGasFeeCalculations = buildMockGasFeeStateFeeMarket(); beforeEach(() => { @@ -1014,7 +1011,29 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations correctly', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, clientId: '99999', }); @@ -1049,7 +1068,29 @@ describe('GasFeeController', () => { describe("the chainId of the networkClientId matches the globally selected network's chainId", () => { it('should update the globally selected network state with a fetched set of estimates', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, getChainId: jest.fn().mockReturnValue(ChainId.goerli), onNetworkDidChange: jest.fn(), }); @@ -1065,7 +1106,29 @@ describe('GasFeeController', () => { it('should update the gasFeeEstimatesByChainId state with a fetched set of estimates', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, getChainId: jest.fn().mockReturnValue(ChainId.goerli), onNetworkDidChange: jest.fn(), }); @@ -1083,7 +1146,29 @@ describe('GasFeeController', () => { describe("the chainId of the networkClientId does not match the globally selected network's chainId", () => { it('should not update the globally selected network state with a fetched set of estimates', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, getChainId: jest.fn().mockReturnValue(ChainId.mainnet), onNetworkDidChange: jest.fn(), }); @@ -1101,7 +1186,29 @@ describe('GasFeeController', () => { it('should update the gasFeeEstimatesByChainId state with a fetched set of estimates', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, getChainId: jest.fn().mockReturnValue(ChainId.mainnet), onNetworkDidChange: jest.fn(), }); @@ -1117,7 +1224,31 @@ describe('GasFeeController', () => { }); it('should return the same data that it puts into state', async () => { - await setupGasFeeController(defaultConstructorOptions); + await setupGasFeeController({ + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, + }); const estimateData = await gasFeeController.fetchGasFeeEstimates({ networkClientId: 'sepolia', @@ -1128,7 +1259,29 @@ describe('GasFeeController', () => { it('should call determineGasFeeCalculations with a URL that contains the chain ID', async () => { await setupGasFeeController({ - ...defaultConstructorOptions, + getIsEIP1559Compatible: jest.fn().mockResolvedValue(true), + networkControllerState: { + networksMetadata: { + goerli: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + sepolia: { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + 'test-network-client-id': { + EIPS: { + 1559: true, + }, + status: NetworkStatus.Available, + }, + }, + }, }); await gasFeeController.fetchGasFeeEstimates({ diff --git a/packages/gas-fee-controller/src/gas-util.test.ts b/packages/gas-fee-controller/src/gas-util.test.ts index b1e5647be84..d6829e7fc78 100644 --- a/packages/gas-fee-controller/src/gas-util.test.ts +++ b/packages/gas-fee-controller/src/gas-util.test.ts @@ -78,10 +78,6 @@ const INFURA_AUTH_TOKEN_MOCK = 'dGVzdDo='; const INFURA_GAS_API_URL_MOCK = 'https://gas.api.infura.io'; describe('gas utils', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('fetchGasEstimates', () => { it('should fetch external gasFeeEstimates when data is valid', async () => { handleFetchMock.mockResolvedValue(mockEIP1559ApiResponses[0]); diff --git a/packages/logging-controller/src/LoggingController.test.ts b/packages/logging-controller/src/LoggingController.test.ts index 0b8626ef1a9..b76957908e3 100644 --- a/packages/logging-controller/src/LoggingController.test.ts +++ b/packages/logging-controller/src/LoggingController.test.ts @@ -7,10 +7,11 @@ import { LogType } from './logTypes'; import { SigningMethod, SigningStage } from './logTypes/EthSignLog'; jest.mock('uuid', () => { - const actual = jest.requireActual('uuid'); return { - ...actual, - v1: jest.fn(() => actual.v1()), + // We need to use this name as this is what Jest recognizes. + // eslint-disable-next-line @typescript-eslint/naming-convention + __esModule: true, + ...jest.requireActual('uuid'), }; }); @@ -42,9 +43,6 @@ function getRestrictedMessenger( } describe('LoggingController', () => { - afterEach(() => { - jest.clearAllMocks(); - }); it('action: LoggingController:add with generic log', async () => { const unrestricted = getUnrestrictedMessenger(); const messenger = getRestrictedMessenger(unrestricted); @@ -112,6 +110,7 @@ describe('LoggingController', () => { it('action: LoggingController:add prevents possible collision of ids', async () => { const unrestricted = getUnrestrictedMessenger(); const messenger = getRestrictedMessenger(unrestricted); + const uuidV1Spy = jest.spyOn(uuid, 'v1'); const controller = new LoggingController({ messenger, @@ -128,9 +127,7 @@ describe('LoggingController', () => { const { id } = Object.values(controller.state.logs)[0]; - if (jest.isMockFunction(uuid.v1)) { - uuid.v1.mockImplementationOnce(() => id); - } + uuidV1Spy.mockReturnValueOnce(id); expect( // TODO: Either fix this lint violation or explain why it's necessary to ignore. @@ -160,7 +157,27 @@ describe('LoggingController', () => { }), }); - expect(uuid.v1).toHaveBeenCalledTimes(3); + expect(uuidV1Spy).toHaveBeenCalledTimes(3); + }); + + it('action: LoggingController:add does not crash, but throws, if a unique id is never generated', async () => { + const unrestricted = getUnrestrictedMessenger(); + const messenger = getRestrictedMessenger(unrestricted); + jest.spyOn(uuid, 'v1').mockReturnValue('same UUID every time'); + + new LoggingController({ messenger }); + + unrestricted.call('LoggingController:add', { + type: LogType.GenericLog, + data: `Generic log`, + }); + + expect(() => { + unrestricted.call('LoggingController:add', { + type: LogType.GenericLog, + data: `Generic log 2`, + }); + }).toThrow('Endless loop'); }); it('internal method: clear', async () => { diff --git a/packages/logging-controller/src/LoggingController.ts b/packages/logging-controller/src/LoggingController.ts index 150711afd4b..202906d43f2 100644 --- a/packages/logging-controller/src/LoggingController.ts +++ b/packages/logging-controller/src/LoggingController.ts @@ -107,8 +107,13 @@ export class LoggingController extends BaseController< */ #generateId(): string { let id = random(); + let i = 0; while (id in this.state.logs) { + if (i > 1000) { + throw new Error('Endless loop'); + } id = random(); + i += 1; } return id; } diff --git a/packages/name-controller/src/NameController.test.ts b/packages/name-controller/src/NameController.test.ts index d05d3fa80a4..551d7eb2ff9 100644 --- a/packages/name-controller/src/NameController.test.ts +++ b/packages/name-controller/src/NameController.test.ts @@ -34,12 +34,6 @@ const CONTROLLER_ARGS_MOCK = { providers: [], }; -// eslint-disable-next-line jest/prefer-spy-on -console.error = jest.fn(); - -// eslint-disable-next-line jest/prefer-spy-on -Date.now = jest.fn().mockReturnValue(TIME_MOCK * 1000); - /** * Creates a mock name provider. * @@ -76,6 +70,14 @@ function createMockProvider( } describe('NameController', () => { + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(() => { + // do nothing + }); + + jest.spyOn(Date, 'now').mockReturnValue(TIME_MOCK * 1000); + }); + describe('setName', () => { it('creates an entry if new%s', () => { const provider1 = createMockProvider(1); diff --git a/packages/name-controller/src/providers/ens.test.ts b/packages/name-controller/src/providers/ens.test.ts index 5166839e60a..126fb0cdcd7 100644 --- a/packages/name-controller/src/providers/ens.test.ts +++ b/packages/name-controller/src/providers/ens.test.ts @@ -16,10 +16,6 @@ const CONSTRUCTOR_ARGS_MOCK = { } as any; describe('ENSNameProvider', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('getMetadata', () => { it('returns the provider metadata', () => { const metadata = new ENSNameProvider(CONSTRUCTOR_ARGS_MOCK).getMetadata(); diff --git a/packages/name-controller/src/providers/etherscan.test.ts b/packages/name-controller/src/providers/etherscan.test.ts index dd8d89eff6e..78ae2d42e41 100644 --- a/packages/name-controller/src/providers/etherscan.test.ts +++ b/packages/name-controller/src/providers/etherscan.test.ts @@ -14,10 +14,6 @@ const CONTRACT_NAME_2_MOCK = 'TestContractName2'; describe('EtherscanNameProvider', () => { const handleFetchMock = jest.mocked(handleFetch); - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('getMetadata', () => { it('returns the provider metadata', () => { const metadata = new EtherscanNameProvider().getMetadata(); diff --git a/packages/name-controller/src/providers/lens.test.ts b/packages/name-controller/src/providers/lens.test.ts index 729bd504dc4..dc22770dbf3 100644 --- a/packages/name-controller/src/providers/lens.test.ts +++ b/packages/name-controller/src/providers/lens.test.ts @@ -13,10 +13,6 @@ const HANDLE_2_MOCK = 'TestHandle2'; describe('LensNameProvider', () => { const graphqlMock = jest.mocked(graphQL); - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('getMetadata', () => { it('returns the provider metadata', () => { const metadata = new LensNameProvider().getMetadata(); diff --git a/packages/name-controller/src/providers/token.test.ts b/packages/name-controller/src/providers/token.test.ts index 58bf6c172d2..4215e2dfe51 100644 --- a/packages/name-controller/src/providers/token.test.ts +++ b/packages/name-controller/src/providers/token.test.ts @@ -12,10 +12,6 @@ const TOKEN_NAME_MOCK = 'TestTokenName'; describe('TokenNameProvider', () => { const handleFetchMock = jest.mocked(handleFetch); - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('getMetadata', () => { it('returns the provider metadata', () => { const metadata = new TokenNameProvider().getMetadata(); diff --git a/packages/network-controller/tests/NetworkController.test.ts b/packages/network-controller/tests/NetworkController.test.ts index 2b3d924f343..7392a0956a5 100644 --- a/packages/network-controller/tests/NetworkController.test.ts +++ b/packages/network-controller/tests/NetworkController.test.ts @@ -153,10 +153,6 @@ const GENERIC_JSON_RPC_ERROR = rpcErrors.internal( ); describe('NetworkController', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); - afterEach(() => { resetAllWhenMocks(); }); diff --git a/packages/permission-controller/src/PermissionController.test.ts b/packages/permission-controller/src/PermissionController.test.ts index 7f117be97a2..3595af9285b 100644 --- a/packages/permission-controller/src/PermissionController.test.ts +++ b/packages/permission-controller/src/PermissionController.test.ts @@ -774,10 +774,6 @@ function getPermissionMatcher({ } describe('PermissionController', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - describe('constructor', () => { it('initializes a new PermissionController', () => { const controller = getDefaultPermissionController(); diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index a36ebb50fc7..8b8e4c1c0c1 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -203,9 +203,6 @@ const setup = ({ }; describe('SelectedNetworkController', () => { - afterEach(() => { - jest.clearAllMocks(); - }); describe('constructor', () => { it('can be instantiated with default values', () => { const { controller } = setup(); @@ -299,10 +296,6 @@ describe('SelectedNetworkController', () => { }); describe('setNetworkClientIdForDomain', () => { - afterEach(() => { - jest.clearAllMocks(); - }); - describe('when the useRequestQueue is false', () => { it('skips setting the networkClientId for the passed in domain', () => { const { controller } = setup({ @@ -864,9 +857,6 @@ describe('SelectedNetworkController', () => { setTarget: jest.fn(), } as unknown as BlockTrackerProxy; - afterEach(() => { - jest.clearAllMocks(); - }); describe('when toggled from off to on', () => { describe('when domains have permissions', () => { it('sets the target of the existing proxies to the non-proxied networkClient for the globally selected networkClientId', () => { diff --git a/packages/signature-controller/src/SignatureController.test.ts b/packages/signature-controller/src/SignatureController.test.ts index d4eb1e07c43..1cf29e035c3 100644 --- a/packages/signature-controller/src/SignatureController.test.ts +++ b/packages/signature-controller/src/SignatureController.test.ts @@ -173,7 +173,6 @@ describe('SignatureController', () => { }; beforeEach(() => { - jest.resetAllMocks(); jest.spyOn(console, 'info').mockImplementation(() => undefined); addUnapprovedMessageMock.mockResolvedValue(messageIdMock); diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ed7f8b6fb7b..f02b63baa1e 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -855,10 +855,6 @@ describe('TransactionController', () => { ); }); - afterEach(() => { - jest.resetAllMocks(); - }); - describe('constructor', () => { it('sets default state', () => { const { controller } = setupController(); diff --git a/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts b/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts index 154b911bd71..64d84fbdecf 100644 --- a/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts +++ b/packages/transaction-controller/src/gas-flows/LineaGasFeeFlow.test.ts @@ -60,8 +60,6 @@ describe('LineaGasFeeFlow', () => { let request: GasFeeFlowRequest; beforeEach(() => { - jest.resetAllMocks(); - request = { ethQuery: {} as EthQuery, transactionMeta: TRANSACTION_META_MOCK, diff --git a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts index cab6442cd69..1c9bc290fdd 100644 --- a/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts +++ b/packages/transaction-controller/src/helpers/EtherscanRemoteTransactionSource.test.ts @@ -44,8 +44,6 @@ describe('EtherscanRemoteTransactionSource', () => { const randomMock = random as jest.MockedFn; beforeEach(() => { - jest.resetAllMocks(); - clock = sinon.useFakeTimers(); fetchEtherscanTransactionsMock.mockResolvedValue( diff --git a/packages/transaction-controller/src/helpers/GasFeePoller.test.ts b/packages/transaction-controller/src/helpers/GasFeePoller.test.ts index 91248f55464..bd62f1fa2d3 100644 --- a/packages/transaction-controller/src/helpers/GasFeePoller.test.ts +++ b/packages/transaction-controller/src/helpers/GasFeePoller.test.ts @@ -73,7 +73,6 @@ describe('GasFeePoller', () => { const findNetworkClientIdByChainIdMock = jest.fn(); beforeEach(() => { - jest.resetAllMocks(); jest.clearAllTimers(); gasFeeFlowMock = createGasFeeFlowMock(); diff --git a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts index 4afe6c64ed0..837a561210f 100644 --- a/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts +++ b/packages/transaction-controller/src/helpers/IncomingTransactionHelper.test.ts @@ -129,10 +129,6 @@ async function emitBlockTrackerLatestEvent( } describe('IncomingTransactionHelper', () => { - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('on block tracker latest event', () => { // eslint-disable-next-line jest/expect-expect it('handles errors', async () => { diff --git a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts index 69119be8e22..2b5fa6c546a 100644 --- a/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts +++ b/packages/transaction-controller/src/helpers/MultichainTrackingHelper.test.ts @@ -221,8 +221,6 @@ function newMultichainTrackingHelper( describe('MultichainTrackingHelper', () => { beforeEach(() => { - jest.resetAllMocks(); - for (const network of [ 'mainnet', 'goerli', diff --git a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts index 0deb17f8ca8..920e90d75c1 100644 --- a/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts +++ b/packages/transaction-controller/src/helpers/PendingTransactionTracker.test.ts @@ -85,8 +85,6 @@ describe('PendingTransactionTracker', () => { } beforeEach(() => { - jest.resetAllMocks(); - blockTracker = createBlockTrackerMock(); failTransaction = jest.fn(); diff --git a/packages/transaction-controller/src/utils/etherscan.test.ts b/packages/transaction-controller/src/utils/etherscan.test.ts index 9a54e575b44..6fd0d7e0ac1 100644 --- a/packages/transaction-controller/src/utils/etherscan.test.ts +++ b/packages/transaction-controller/src/utils/etherscan.test.ts @@ -34,10 +34,6 @@ const RESPONSE_MOCK: EtherscanTransactionResponse = { describe('Etherscan', () => { const handleFetchMock = jest.mocked(handleFetch); - beforeEach(() => { - jest.resetAllMocks(); - }); - describe('getEtherscanApiHost', () => { it('returns Etherscan API host for supported network', () => { expect(getEtherscanApiHost(CHAIN_IDS.GOERLI)).toBe( diff --git a/packages/transaction-controller/src/utils/gas.test.ts b/packages/transaction-controller/src/utils/gas.test.ts index 53e66f73e81..b5dfebdf155 100644 --- a/packages/transaction-controller/src/utils/gas.test.ts +++ b/packages/transaction-controller/src/utils/gas.test.ts @@ -93,8 +93,6 @@ describe('gas', () => { } beforeEach(() => { - jest.resetAllMocks(); - updateGasRequest = JSON.parse(JSON.stringify(UPDATE_GAS_REQUEST_MOCK)); }); diff --git a/packages/transaction-controller/src/utils/simulation-api.test.ts b/packages/transaction-controller/src/utils/simulation-api.test.ts index 8ff5603051c..aafccb5282f 100644 --- a/packages/transaction-controller/src/utils/simulation-api.test.ts +++ b/packages/transaction-controller/src/utils/simulation-api.test.ts @@ -65,8 +65,6 @@ describe('Simulation API Utils', () => { } beforeEach(() => { - jest.resetAllMocks(); - fetchMock = jest.spyOn(global, 'fetch') as jest.MockedFunction< typeof fetch >; diff --git a/packages/transaction-controller/src/utils/simulation.test.ts b/packages/transaction-controller/src/utils/simulation.test.ts index 4d138b51ab2..60a742de491 100644 --- a/packages/transaction-controller/src/utils/simulation.test.ts +++ b/packages/transaction-controller/src/utils/simulation.test.ts @@ -266,7 +266,6 @@ describe('Simulation Utils', () => { const simulateTransactionsMock = jest.mocked(simulateTransactions); beforeEach(() => { - jest.resetAllMocks(); jest.spyOn(Interface.prototype, 'encodeFunctionData').mockReturnValue(''); }); diff --git a/packages/transaction-controller/src/utils/utils.test.ts b/packages/transaction-controller/src/utils/utils.test.ts index 14e3374ac45..8d7f1986b0e 100644 --- a/packages/transaction-controller/src/utils/utils.test.ts +++ b/packages/transaction-controller/src/utils/utils.test.ts @@ -25,10 +25,6 @@ const TRANSACTION_PARAMS_MOCK: TransactionParams = { }; describe('utils', () => { - afterEach(() => { - jest.clearAllMocks(); - }); - describe('normalizeTransactionParams', () => { it('normalizes properties', () => { const normalized = util.normalizeTransactionParams( diff --git a/packages/transaction-controller/src/utils/validation.test.ts b/packages/transaction-controller/src/utils/validation.test.ts index 265f3e954d5..e26175022bd 100644 --- a/packages/transaction-controller/src/utils/validation.test.ts +++ b/packages/transaction-controller/src/utils/validation.test.ts @@ -4,10 +4,6 @@ import { TransactionEnvelopeType } from '../types'; import { validateTxParams } from './validation'; describe('validation', () => { - afterEach(() => { - jest.clearAllMocks(); - }); - describe('validateTxParams', () => { it('should throw if no from address', () => { // TODO: Replace `any` with type diff --git a/packages/user-operation-controller/src/UserOperationController.test.ts b/packages/user-operation-controller/src/UserOperationController.test.ts index 6e9554d6957..e41af0e540f 100644 --- a/packages/user-operation-controller/src/UserOperationController.test.ts +++ b/packages/user-operation-controller/src/UserOperationController.test.ts @@ -192,8 +192,6 @@ describe('UserOperationController', () => { ); beforeEach(() => { - jest.resetAllMocks(); - jest.spyOn(BundlerHelper, 'Bundler').mockReturnValue(bundlerMock); jest .spyOn(PendingUserOperationTrackerHelper, 'PendingUserOperationTracker') diff --git a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.test.ts b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.test.ts index b3c16aa3585..2285f2cd90e 100644 --- a/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.test.ts +++ b/packages/user-operation-controller/src/helpers/PendingUserOperationTracker.test.ts @@ -121,7 +121,6 @@ describe('PendingUserOperationTracker', () => { } beforeEach(() => { - jest.resetAllMocks(); jest.spyOn(BundlerHelper, 'Bundler').mockReturnValue(bundlerMock); messengerMock.call.mockReturnValue({ diff --git a/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts b/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts index 960ba8b7180..11475aa0386 100644 --- a/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts +++ b/packages/user-operation-controller/src/helpers/SnapSmartContractAccount.test.ts @@ -89,8 +89,6 @@ describe('SnapSmartContractAccount', () => { let signMock: jest.MockedFn; beforeEach(() => { - jest.resetAllMocks(); - messengerMock = createMessengerMock(); prepareMock = jest.fn(); patchMock = jest.fn(); diff --git a/packages/user-operation-controller/src/utils/gas-fees.test.ts b/packages/user-operation-controller/src/utils/gas-fees.test.ts index 3d7e1fb5661..25cb1b1a0d2 100644 --- a/packages/user-operation-controller/src/utils/gas-fees.test.ts +++ b/packages/user-operation-controller/src/utils/gas-fees.test.ts @@ -34,8 +34,6 @@ describe('gas-fees', () => { let request: jest.Mocked; beforeEach(() => { - jest.resetAllMocks(); - request = cloneDeep(UPDATE_GAS_FEES_REQUEST_MOCK); jest diff --git a/scripts/create-package/commands.test.ts b/scripts/create-package/commands.test.ts index 97153222fbe..e557dd51506 100644 --- a/scripts/create-package/commands.test.ts +++ b/scripts/create-package/commands.test.ts @@ -13,10 +13,6 @@ jest.mock('./utils', () => ({ jest.useFakeTimers().setSystemTime(new Date('2023-01-02')); describe('create-package/commands', () => { - afterEach(() => { - jest.resetAllMocks(); - }); - describe('createPackageHandler', () => { it('should create the expected package', async () => { (utils.readMonorepoFiles as jest.Mock).mockResolvedValue({ diff --git a/scripts/create-package/utils.test.ts b/scripts/create-package/utils.test.ts index 263b52f2ec0..af903f8edec 100644 --- a/scripts/create-package/utils.test.ts +++ b/scripts/create-package/utils.test.ts @@ -29,10 +29,6 @@ jest.mock('./fs-utils', () => ({ })); describe('create-package/utils', () => { - afterEach(() => { - jest.resetAllMocks(); - }); - describe('readMonorepoFiles', () => { const tsConfig = JSON.stringify({ references: [{ path: '../packages/foo' }], From 9829a60153ef474c1c013a5632f7063c8b89f406 Mon Sep 17 00:00:00 2001 From: jiexi Date: Tue, 18 Jun 2024 11:39:49 -0700 Subject: [PATCH 02/11] Release 162.0.0 (#4431) Releases `queued-request-controller` methods array constructor param to callback constructor param update --------- Co-authored-by: Alex Donesky --- package.json | 2 +- packages/queued-request-controller/CHANGELOG.md | 10 +++++++++- packages/queued-request-controller/package.json | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index 368207873a6..b1f2ea738af 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "161.0.0", + "version": "162.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { diff --git a/packages/queued-request-controller/CHANGELOG.md b/packages/queued-request-controller/CHANGELOG.md index d714aae7a93..565ddfe8500 100644 --- a/packages/queued-request-controller/CHANGELOG.md +++ b/packages/queued-request-controller/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.0.0] + +### Changed + +- **BREAKING:** `QueuedRequestController` constructor no longer accepts the `methodsRequiringNetworkSwitch` array param. It's now replaced with the `shouldRequestSwitchNetwork` function param which should return true when a request requires the globally selected network to match that of the dapp from which the request originated. ([#4423](https://github.com/MetaMask/core/pull/4423)) +- **BREAKING:** `createQueuedRequestMiddleware` no longer accepts the `methodsWithConfirmation` array typed param. It's now replaced with the `shouldEnqueueRequest` function typed param which should return true when a request should be handled by the `QueuedRequestController`. ([#4423](https://github.com/MetaMask/core/pull/4423)) + ## [0.12.0] ### Changed @@ -200,7 +207,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Initial release -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/queued-request-controller@0.12.0...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/queued-request-controller@1.0.0...HEAD +[1.0.0]: https://github.com/MetaMask/core/compare/@metamask/queued-request-controller@0.12.0...@metamask/queued-request-controller@1.0.0 [0.12.0]: https://github.com/MetaMask/core/compare/@metamask/queued-request-controller@0.11.0...@metamask/queued-request-controller@0.12.0 [0.11.0]: https://github.com/MetaMask/core/compare/@metamask/queued-request-controller@0.10.0...@metamask/queued-request-controller@0.11.0 [0.10.0]: https://github.com/MetaMask/core/compare/@metamask/queued-request-controller@0.9.0...@metamask/queued-request-controller@0.10.0 diff --git a/packages/queued-request-controller/package.json b/packages/queued-request-controller/package.json index a6e33594cd5..8b2b1233831 100644 --- a/packages/queued-request-controller/package.json +++ b/packages/queued-request-controller/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/queued-request-controller", - "version": "0.12.0", + "version": "1.0.0", "description": "Includes a controller and middleware that implements a request queue", "keywords": [ "MetaMask", From 44ab192a61bce012eb9941d2c66f3f7e41d9f62c Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 19 Jun 2024 13:35:23 +0100 Subject: [PATCH 03/11] refactor: update notification package exports (#4437) ## Explanation it seems we were inconsistent and forgot to correctly handle default exports and controller exports ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- .../NotificationServicesController.test.ts | 3 +-- .../NotificationServicesController.ts | 2 +- .../src/NotificationServicesController/index.ts | 3 +++ .../NotificationServicesPushController.test.ts | 2 +- .../NotificationServicesPushController.ts | 2 +- .../src/NotificationServicesPushController/index.ts | 3 +++ .../src/controllers/authentication/index.ts | 3 +++ .../src/controllers/user-storage/encryption/index.ts | 1 + .../src/controllers/user-storage/index.ts | 4 +++- packages/profile-sync-controller/src/sdk/encryption.ts | 4 ++-- 10 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts index ae3b18b9ee7..3a637e46629 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.test.ts @@ -25,8 +25,7 @@ import { mockMarkNotificationsAsRead, } from './__fixtures__/mockServices'; import { waitFor } from './__fixtures__/test-utils'; -import { - NotificationServicesController, +import NotificationServicesController, { defaultState, } from './NotificationServicesController'; import type { diff --git a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts index 308f434cc79..ef77e0b8741 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/NotificationServicesController.ts @@ -236,7 +236,7 @@ type FeatureAnnouncementEnv = { /** * Controller that enables wallet notifications and feature announcements */ -export class NotificationServicesController extends BaseController< +export default class NotificationServicesController extends BaseController< typeof controllerName, NotificationServicesControllerState, NotificationServicesControllerMessenger diff --git a/packages/notification-services-controller/src/NotificationServicesController/index.ts b/packages/notification-services-controller/src/NotificationServicesController/index.ts index 1fd0ee9ba0c..163646e27ad 100644 --- a/packages/notification-services-controller/src/NotificationServicesController/index.ts +++ b/packages/notification-services-controller/src/NotificationServicesController/index.ts @@ -1,3 +1,6 @@ +import Controller from './NotificationServicesController'; + +export { Controller }; export * from './NotificationServicesController'; export * as Types from './types'; export * as Mocks from './__fixtures__'; diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts index 5ac2f0c9d6e..333a056d594 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.test.ts @@ -1,7 +1,7 @@ import { ControllerMessenger } from '@metamask/base-controller'; import type { AuthenticationController } from '@metamask/profile-sync-controller'; -import { NotificationServicesPushController } from './NotificationServicesPushController'; +import NotificationServicesPushController from './NotificationServicesPushController'; import type { AllowedActions, AllowedEvents, diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts index 018396ebb40..4c49aa93709 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/NotificationServicesPushController.ts @@ -115,7 +115,7 @@ type ControllerConfig = { * * @augments {BaseController} */ -export class NotificationServicesPushController extends BaseController< +export default class NotificationServicesPushController extends BaseController< typeof controllerName, NotificationServicesPushControllerState, NotificationServicesPushControllerMessenger diff --git a/packages/notification-services-controller/src/NotificationServicesPushController/index.ts b/packages/notification-services-controller/src/NotificationServicesPushController/index.ts index ecff150083e..012dd290255 100644 --- a/packages/notification-services-controller/src/NotificationServicesPushController/index.ts +++ b/packages/notification-services-controller/src/NotificationServicesPushController/index.ts @@ -1,3 +1,6 @@ +import Controller from './NotificationServicesPushController'; + +export { Controller }; export * from './NotificationServicesPushController'; export * as Types from './types'; export * as Utils from './utils'; diff --git a/packages/profile-sync-controller/src/controllers/authentication/index.ts b/packages/profile-sync-controller/src/controllers/authentication/index.ts index 8ce30b898d2..ece95fff72b 100644 --- a/packages/profile-sync-controller/src/controllers/authentication/index.ts +++ b/packages/profile-sync-controller/src/controllers/authentication/index.ts @@ -1,2 +1,5 @@ +import Controller from './AuthenticationController'; + +export { Controller }; export * from './AuthenticationController'; export * as Mocks from './__fixtures__'; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/encryption/index.ts b/packages/profile-sync-controller/src/controllers/user-storage/encryption/index.ts index 3582e3b9e2a..90cdb5f5f2e 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/encryption/index.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/encryption/index.ts @@ -1,4 +1,5 @@ import Encryption from './encryption'; +export { Encryption }; export * from './encryption'; export default Encryption; diff --git a/packages/profile-sync-controller/src/controllers/user-storage/index.ts b/packages/profile-sync-controller/src/controllers/user-storage/index.ts index 69a3f0b2661..01c28fababb 100644 --- a/packages/profile-sync-controller/src/controllers/user-storage/index.ts +++ b/packages/profile-sync-controller/src/controllers/user-storage/index.ts @@ -1,4 +1,6 @@ +import Controller from './UserStorageController'; + +export { Controller }; export * from './UserStorageController'; export * from './encryption'; - export * as Mocks from './__fixtures__'; diff --git a/packages/profile-sync-controller/src/sdk/encryption.ts b/packages/profile-sync-controller/src/sdk/encryption.ts index 24c2d3043f8..4070e4f2532 100644 --- a/packages/profile-sync-controller/src/sdk/encryption.ts +++ b/packages/profile-sync-controller/src/sdk/encryption.ts @@ -162,8 +162,8 @@ class EncryptorDecryptor { } } -const encryption = new EncryptorDecryptor(); -export default encryption; +export const Encryption = new EncryptorDecryptor(); +export default Encryption; /** * Create a SHA-256 hash from a given string. From 5d381971a70707b1632e4fc93339c68d3964a967 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 19 Jun 2024 14:10:04 +0100 Subject: [PATCH 04/11] Release 163.0.0 (#4436) ## Explanation Releases `profile-sync-controller` and from `0.0.0` to `0.1.0` ## References N/A ## Changelog ### `@metamask/profile-sync-controller` - **ADDED**: Initial release of SDK; Authentication Controller; User Storage Controller. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- package.json | 2 +- packages/notification-services-controller/package.json | 4 +--- packages/profile-sync-controller/CHANGELOG.md | 5 ++++- packages/profile-sync-controller/package.json | 2 +- yarn.lock | 4 +--- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index b1f2ea738af..b0d19a9938e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "162.0.0", + "version": "163.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { diff --git a/packages/notification-services-controller/package.json b/packages/notification-services-controller/package.json index cdd2c988be3..95762bb0504 100644 --- a/packages/notification-services-controller/package.json +++ b/packages/notification-services-controller/package.json @@ -45,7 +45,6 @@ "@metamask/base-controller": "^6.0.0", "@metamask/controller-utils": "^11.0.0", "@metamask/keyring-controller": "^17.1.0", - "@metamask/profile-sync-controller": "^0.0.0", "bignumber.js": "^4.1.0", "contentful": "^10.3.6", "firebase": "^10.11.0", @@ -67,8 +66,7 @@ "typescript": "~4.9.5" }, "peerDependencies": { - "@metamask/keyring-controller": "^17.0.0", - "@metamask/profile-sync-controller": "^0.0.0" + "@metamask/keyring-controller": "^17.0.0" }, "engines": { "node": "^18.18 || >=20" diff --git a/packages/profile-sync-controller/CHANGELOG.md b/packages/profile-sync-controller/CHANGELOG.md index 8fcf72c699c..695bec498cc 100644 --- a/packages/profile-sync-controller/CHANGELOG.md +++ b/packages/profile-sync-controller/CHANGELOG.md @@ -7,8 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.1.0] + ### Added - Initial release -[Unreleased]: https://github.com/MetaMask/core/ +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/profile-sync-controller@0.1.0...HEAD +[0.1.0]: https://github.com/MetaMask/core/releases/tag/@metamask/profile-sync-controller@0.1.0 diff --git a/packages/profile-sync-controller/package.json b/packages/profile-sync-controller/package.json index 5095bf124fe..346d998909f 100644 --- a/packages/profile-sync-controller/package.json +++ b/packages/profile-sync-controller/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/profile-sync-controller", - "version": "0.0.0", + "version": "0.1.0", "description": "The profile sync helps developers synchronize data across multiple clients and devices in a privacy-preserving way. All data saved in the user storage database is encrypted client-side to preserve privacy. The user storage provides a modular design, giving developers the flexibility to construct and manage their storage spaces in a way that best suits their needs", "keywords": [ "MetaMask", diff --git a/yarn.lock b/yarn.lock index b3347414e8f..d4dccf087ed 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3254,7 +3254,6 @@ __metadata: "@metamask/base-controller": ^6.0.0 "@metamask/controller-utils": ^11.0.0 "@metamask/keyring-controller": ^17.1.0 - "@metamask/profile-sync-controller": ^0.0.0 "@types/jest": ^27.4.1 "@types/readable-stream": ^2.3.0 bignumber.js: ^4.1.0 @@ -3272,7 +3271,6 @@ __metadata: uuid: ^8.3.2 peerDependencies: "@metamask/keyring-controller": ^17.0.0 - "@metamask/profile-sync-controller": ^0.0.0 languageName: unknown linkType: soft @@ -3465,7 +3463,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/profile-sync-controller@^0.0.0, @metamask/profile-sync-controller@workspace:packages/profile-sync-controller": +"@metamask/profile-sync-controller@workspace:packages/profile-sync-controller": version: 0.0.0-use.local resolution: "@metamask/profile-sync-controller@workspace:packages/profile-sync-controller" dependencies: From 92a10382caa3e7ac60394d220399e7f90fc7c051 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 19 Jun 2024 15:19:15 +0100 Subject: [PATCH 05/11] refactor: add back profile sync dependency (#4439) ## Explanation This was removed as it was blocking publishing of profile sync controller. Adding here so it unblocks the next release for this controller: https://github.com/MetaMask/core/pull/4438 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- packages/notification-services-controller/package.json | 4 +++- yarn.lock | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/notification-services-controller/package.json b/packages/notification-services-controller/package.json index 95762bb0504..e78c728189b 100644 --- a/packages/notification-services-controller/package.json +++ b/packages/notification-services-controller/package.json @@ -45,6 +45,7 @@ "@metamask/base-controller": "^6.0.0", "@metamask/controller-utils": "^11.0.0", "@metamask/keyring-controller": "^17.1.0", + "@metamask/profile-sync-controller": "^0.1.0", "bignumber.js": "^4.1.0", "contentful": "^10.3.6", "firebase": "^10.11.0", @@ -66,7 +67,8 @@ "typescript": "~4.9.5" }, "peerDependencies": { - "@metamask/keyring-controller": "^17.0.0" + "@metamask/keyring-controller": "^17.0.0", + "@metamask/profile-sync-controller": "^0.1.0" }, "engines": { "node": "^18.18 || >=20" diff --git a/yarn.lock b/yarn.lock index d4dccf087ed..7d3f5cedb5a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3254,6 +3254,7 @@ __metadata: "@metamask/base-controller": ^6.0.0 "@metamask/controller-utils": ^11.0.0 "@metamask/keyring-controller": ^17.1.0 + "@metamask/profile-sync-controller": ^0.1.0 "@types/jest": ^27.4.1 "@types/readable-stream": ^2.3.0 bignumber.js: ^4.1.0 @@ -3271,6 +3272,7 @@ __metadata: uuid: ^8.3.2 peerDependencies: "@metamask/keyring-controller": ^17.0.0 + "@metamask/profile-sync-controller": ^0.1.0 languageName: unknown linkType: soft @@ -3463,7 +3465,7 @@ __metadata: languageName: unknown linkType: soft -"@metamask/profile-sync-controller@workspace:packages/profile-sync-controller": +"@metamask/profile-sync-controller@^0.1.0, @metamask/profile-sync-controller@workspace:packages/profile-sync-controller": version: 0.0.0-use.local resolution: "@metamask/profile-sync-controller@workspace:packages/profile-sync-controller" dependencies: From 26a1c6e745acb303fb1cab0efac582de0d9569a5 Mon Sep 17 00:00:00 2001 From: Prithpal Sooriya Date: Wed, 19 Jun 2024 15:37:19 +0100 Subject: [PATCH 06/11] Release 164.0.0 (#4438) ## Explanation Releases `notification-services-controller` from `0.0.0` to `0.1.0`. ## References N/A ## Changelog ### `@metamask/notification-services-controller` - **ADDED**: Initial Release of Notification Services Controller; Notification Services Push Controller. ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- package.json | 2 +- packages/notification-services-controller/CHANGELOG.md | 5 ++++- packages/notification-services-controller/package.json | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index b0d19a9938e..feeca510a7a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "163.0.0", + "version": "164.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { diff --git a/packages/notification-services-controller/CHANGELOG.md b/packages/notification-services-controller/CHANGELOG.md index 8fcf72c699c..524b205b1e3 100644 --- a/packages/notification-services-controller/CHANGELOG.md +++ b/packages/notification-services-controller/CHANGELOG.md @@ -7,8 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.1.0] + ### Added - Initial release -[Unreleased]: https://github.com/MetaMask/core/ +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/notification-services-controller@0.1.0...HEAD +[0.1.0]: https://github.com/MetaMask/core/releases/tag/@metamask/notification-services-controller@0.1.0 diff --git a/packages/notification-services-controller/package.json b/packages/notification-services-controller/package.json index e78c728189b..8933f8d3d0f 100644 --- a/packages/notification-services-controller/package.json +++ b/packages/notification-services-controller/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/notification-services-controller", - "version": "0.0.0", + "version": "0.1.0", "description": "Manages New MetaMask decentralized Notification system", "keywords": [ "MetaMask", From ae56018583a206cd9e450f10b3039022aa5f8b23 Mon Sep 17 00:00:00 2001 From: jiexi Date: Wed, 19 Jun 2024 10:32:48 -0700 Subject: [PATCH 07/11] test: cleanup/fix `SelectedNetworkController` specs (#4409) Cleans up / fixes some `SelectedNetworkController` specs Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2626 No consumer facing changes - [x] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate --------- Co-authored-by: Alex Donesky --- .../src/QueuedRequestController.ts | 2 +- .../tests/SelectedNetworkController.test.ts | 626 +++++++++--------- 2 files changed, 306 insertions(+), 322 deletions(-) diff --git a/packages/queued-request-controller/src/QueuedRequestController.ts b/packages/queued-request-controller/src/QueuedRequestController.ts index 291210a8120..63358b20040 100644 --- a/packages/queued-request-controller/src/QueuedRequestController.ts +++ b/packages/queued-request-controller/src/QueuedRequestController.ts @@ -197,7 +197,7 @@ export class QueuedRequestController extends BaseController< ) { const origin = path[1]; this.#flushQueueForOrigin(origin); - // When a domain is removed from SelectedNetworkController, its because of revoke permissions. + // When a domain is removed from SelectedNetworkController, its because of revoke permissions or the useRequestQueue flag was toggled off. // Rather than subscribe to the permissions controller event in addition to the selectedNetworkController ones, we simplify it and just handle remove on this event alone. if (op === 'remove' && origin === this.#originOfCurrentBatch) { this.#clearPendingConfirmations(); diff --git a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts index 8b8e4c1c0c1..2914024b3f4 100644 --- a/packages/selected-network-controller/tests/SelectedNetworkController.test.ts +++ b/packages/selected-network-controller/tests/SelectedNetworkController.test.ts @@ -203,6 +203,13 @@ const setup = ({ }; describe('SelectedNetworkController', () => { +<<<<<<< HEAD +======= + afterEach(() => { + jest.clearAllMocks(); + }); + +>>>>>>> bcf496075 (test: cleanup/fix `SelectedNetworkController` specs (#4409)) describe('constructor', () => { it('can be instantiated with default values', () => { const { controller } = setup(); @@ -210,6 +217,7 @@ describe('SelectedNetworkController', () => { domains: {}, }); }); + it('can be instantiated with a state', () => { const { controller } = setup({ state: { @@ -220,108 +228,146 @@ describe('SelectedNetworkController', () => { domains: { networkClientId: 'goerli' }, }); }); - }); - describe('networkController:stateChange', () => { - describe('when useRequestQueuePreference is false', () => { - describe('when a networkClient is deleted from the network controller state', () => { - it('does not update the networkClientId for domains which were previously set to the deleted networkClientId', () => { - const { controller, messenger } = setup({ - state: { - // normally there would not be any domains in state if useRequestQueuePreference is false - domains: { - metamask: 'goerli', - 'example.com': 'test-network-client-id', - 'test.com': 'test-network-client-id', - }, + describe('when useRequestQueuePreference is true', () => { + it('should set networkClientId for domains not already in state', async () => { + const { controller } = setup({ + state: { + domains: { + 'existingdomain.com': 'initialNetworkId', }, - }); + }, + getSubjectNames: ['newdomain.com'], + useRequestQueuePreference: true, + }); - messenger.publish( - 'NetworkController:stateChange', - { - selectedNetworkClientId: 'goerli', - networkConfigurations: {}, - networksMetadata: {}, + expect(controller.state.domains).toStrictEqual({ + 'newdomain.com': 'mainnet', + 'existingdomain.com': 'initialNetworkId', + }); + }); + + it('should not modify domains already in state', async () => { + const { controller } = setup({ + state: { + domains: { + 'existingdomain.com': 'initialNetworkId', }, - [ - { - op: 'remove', - path: ['networkConfigurations', 'test-network-client-id'], - }, - ], - ); - expect(controller.state.domains).toStrictEqual({ - metamask: 'goerli', - 'example.com': 'test-network-client-id', - 'test.com': 'test-network-client-id', - }); + }, + getSubjectNames: ['existingdomain.com'], + useRequestQueuePreference: true, + }); + + expect(controller.state.domains).toStrictEqual({ + 'existingdomain.com': 'initialNetworkId', }); }); }); - describe('when useRequestQueuePreference is true', () => { - describe('when a networkClient is deleted from the network controller state', () => { - it('updates the networkClientId for domains which were previously set to the deleted networkClientId', () => { - const { controller, messenger } = setup({ - state: { - domains: { - metamask: 'goerli', - 'example.com': 'test-network-client-id', - 'test.com': 'test-network-client-id', - }, + describe('when useRequestQueuePreference is false', () => { + it('should not set networkClientId for new domains', async () => { + const { controller } = setup({ + state: { + domains: { + 'existingdomain.com': 'initialNetworkId', }, - useRequestQueuePreference: true, - }); + }, + getSubjectNames: ['newdomain.com'], + }); - messenger.publish( - 'NetworkController:stateChange', - { - selectedNetworkClientId: 'goerli', - networkConfigurations: {}, - networksMetadata: {}, + expect(controller.state.domains).toStrictEqual({ + 'existingdomain.com': 'initialNetworkId', + }); + }); + + it('should not modify domains already in state', async () => { + const { controller } = setup({ + state: { + domains: { + 'existingdomain.com': 'initialNetworkId', }, - [ - { - op: 'remove', - path: ['networkConfigurations', 'test-network-client-id'], - }, - ], - ); - expect(controller.state.domains['example.com']).toBe('goerli'); - expect(controller.state.domains['test.com']).toBe('goerli'); + }, + getSubjectNames: ['existingdomain.com'], + }); + + expect(controller.state.domains).toStrictEqual({ + 'existingdomain.com': 'initialNetworkId', }); }); }); }); - describe('setNetworkClientIdForDomain', () => { - describe('when the useRequestQueue is false', () => { - it('skips setting the networkClientId for the passed in domain', () => { - const { controller } = setup({ + describe('networkController:stateChange', () => { + describe('when a networkClient is deleted from the network controller state', () => { + it('does not update state when useRequestQueuePreference is false', () => { + const { controller, messenger } = setup({ + state: { + domains: {}, + }, + }); + + messenger.publish( + 'NetworkController:stateChange', + { + selectedNetworkClientId: 'goerli', + networkConfigurations: {}, + networksMetadata: {}, + }, + [ + { + op: 'remove', + path: ['networkConfigurations', 'test-network-client-id'], + }, + ], + ); + expect(controller.state.domains).toStrictEqual({}); + }); + + it('updates the networkClientId for domains which were previously set to the deleted networkClientId when useRequestQueuePreference is true', () => { + const { controller, messenger } = setup({ state: { domains: { - '1.com': 'mainnet', - '2.com': 'mainnet', - '3.com': 'mainnet', + metamask: 'goerli', + 'example.com': 'test-network-client-id', + 'test.com': 'test-network-client-id', }, }, + useRequestQueuePreference: true, }); - const domains = ['1.com', '2.com', '3.com']; - const networkClientIds = ['1', '2', '3']; - domains.forEach((domain, i) => - controller.setNetworkClientIdForDomain(domain, networkClientIds[i]), + messenger.publish( + 'NetworkController:stateChange', + { + selectedNetworkClientId: 'goerli', + networkConfigurations: {}, + networksMetadata: {}, + }, + [ + { + op: 'remove', + path: ['networkConfigurations', 'test-network-client-id'], + }, + ], ); + expect(controller.state.domains['example.com']).toBe('goerli'); + expect(controller.state.domains['test.com']).toBe('goerli'); + }); + }); + }); - expect(controller.state.domains).toStrictEqual({ - '1.com': 'mainnet', - '2.com': 'mainnet', - '3.com': 'mainnet', - }); + describe('setNetworkClientIdForDomain', () => { + it('does not update state when the useRequestQueuePreference is false', () => { + const { controller } = setup({ + state: { + domains: {}, + }, }); + + controller.setNetworkClientIdForDomain('1.com', '1'); + expect(controller.state.domains).toStrictEqual({}); }); - describe('when the useRequestQueue is true', () => { + + describe('when useRequestQueuePreference is true', () => { it('should throw an error when passed "metamask" as domain arg', () => { const { controller } = setup({ useRequestQueuePreference: true }); expect(() => { @@ -331,6 +377,7 @@ describe('SelectedNetworkController', () => { ); expect(controller.state.domains.metamask).toBeUndefined(); }); + describe('when the requesting domain is a snap (starts with "npm:" or "local:"', () => { it('skips setting the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ @@ -361,6 +408,7 @@ describe('SelectedNetworkController', () => { }); }); }); + describe('when the requesting domain has existing permissions', () => { it('sets the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ @@ -406,7 +454,7 @@ describe('SelectedNetworkController', () => { }); describe('when the requesting domain does not have permissions', () => { - it('throw an error and does not set the networkClientId for the passed in domain', () => { + it('throws an error and does not set the networkClientId for the passed in domain', () => { const { controller, mockHasPermissions } = setup({ state: { domains: {} }, useRequestQueuePreference: true, @@ -427,46 +475,29 @@ describe('SelectedNetworkController', () => { }); describe('getNetworkClientIdForDomain', () => { - describe('when the useRequestQueue is false', () => { - it('returns the selectedNetworkClientId from the NetworkController if not no networkClientId is set for requested domain', () => { - const { controller } = setup(); - expect(controller.getNetworkClientIdForDomain('example.com')).toBe( - 'mainnet', - ); - }); - it('returns the selectedNetworkClientId from the NetworkController if a networkClientId is set for the requested domain', () => { - const { controller } = setup(); - const networkClientId = 'network3'; - controller.setNetworkClientIdForDomain('example.com', networkClientId); - expect(controller.getNetworkClientIdForDomain('example.com')).toBe( - 'mainnet', - ); - }); - it('returns the networkClientId for the metamask domain when passed "metamask"', () => { - const { controller } = setup(); - const result = controller.getNetworkClientIdForDomain('metamask'); - expect(result).toBe('mainnet'); - }); + it('returns the selectedNetworkClientId from the NetworkController when useRequestQueuePreference is false', () => { + const { controller } = setup(); + expect(controller.getNetworkClientIdForDomain('example.com')).toBe( + 'mainnet', + ); }); - describe('when the useRequestQueue is true', () => { - it('returns the networkClientId for the passed in domain, when a networkClientId has been set for the requested domain', () => { + describe('when useRequestQueuePreference is true', () => { + it('returns the networkClientId from state when a networkClientId has been set for the requested domain', () => { const { controller } = setup({ - state: { domains: {} }, + state: { + domains: { + 'example.com': '1', + }, + }, useRequestQueuePreference: true, }); - const networkClientId1 = 'network5'; - const networkClientId2 = 'network6'; - controller.setNetworkClientIdForDomain('example.com', networkClientId1); - controller.setNetworkClientIdForDomain('test.com', networkClientId2); - const result1 = controller.getNetworkClientIdForDomain('example.com'); - const result2 = controller.getNetworkClientIdForDomain('test.com'); - expect(result1).toBe(networkClientId1); - expect(result2).toBe(networkClientId2); + const result = controller.getNetworkClientIdForDomain('example.com'); + expect(result).toBe('1'); }); - it('returns the selectedNetworkClientId from the NetworkController when no networkClientId has been set for the domain requested', () => { + it('returns the selectedNetworkClientId from the NetworkController when no networkClientId has been set for the requested domain', () => { const { controller } = setup({ state: { domains: {} }, useRequestQueuePreference: true, @@ -479,111 +510,78 @@ describe('SelectedNetworkController', () => { }); describe('getProviderAndBlockTracker', () => { - describe('when the domain already has a cached networkProxy in the domainProxyMap', () => { - it('returns the cached proxy provider and block tracker', () => { - const mockProxyProvider = { - setTarget: jest.fn(), - } as unknown as ProviderProxy; - const mockProxyBlockTracker = { - setTarget: jest.fn(), - } as unknown as BlockTrackerProxy; - - const domainProxyMap = new Map([ - [ - 'example.com', - { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }, - ], - [ - 'test.com', - { - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }, - ], - ]); - const { controller } = setup({ - state: { - domains: {}, + it('returns the cached proxy provider and block tracker when the domain already has a cached networkProxy in the domainProxyMap', () => { + const mockProxyProvider = { + setTarget: jest.fn(), + } as unknown as ProviderProxy; + const mockProxyBlockTracker = { + setTarget: jest.fn(), + } as unknown as BlockTrackerProxy; + + const domainProxyMap = new Map([ + [ + 'example.com', + { + provider: mockProxyProvider, + blockTracker: mockProxyBlockTracker, }, - useRequestQueuePreference: true, - domainProxyMap, - }); + ], + [ + 'test.com', + { + provider: mockProxyProvider, + blockTracker: mockProxyBlockTracker, + }, + ], + ]); + const { controller } = setup({ + state: { + domains: {}, + }, + useRequestQueuePreference: true, + domainProxyMap, + }); - const result = controller.getProviderAndBlockTracker('example.com'); - expect(result).toStrictEqual({ - provider: mockProxyProvider, - blockTracker: mockProxyBlockTracker, - }); + const result = controller.getProviderAndBlockTracker('example.com'); + expect(result).toStrictEqual({ + provider: mockProxyProvider, + blockTracker: mockProxyBlockTracker, }); }); - describe('when the domain does not have a cached networkProxy in the domainProxyMap', () => { - describe('when the useRequestQueue preference is true', () => { - describe('when the domain has permissions', () => { - it('calls to NetworkController:getNetworkClientById and creates a new proxy provider and block tracker with the non-proxied globally selected network client', () => { - const { controller, messenger } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: true, - }); - jest.spyOn(messenger, 'call'); - - const result = controller.getProviderAndBlockTracker('example.com'); - expect(result).toBeDefined(); - // unfortunately checking which networkController method is called is the best - // proxy (no pun intended) for checking that the correct instance of the networkClient is used - expect(messenger.call).toHaveBeenCalledWith( - 'NetworkController:getNetworkClientById', - 'mainnet', - ); - }); - it('throws an error if the globally selected network client is not initialized', () => { - const { controller, mockGetSelectedNetworkClient } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: false, - }); - mockGetSelectedNetworkClient.mockReturnValue(undefined); - expect(() => - controller.getProviderAndBlockTracker('example.com'), - ).toThrow('Selected network not initialized'); - }); - }); - describe('when the domain does not have permissions', () => { - it('calls to NetworkController:getSelectedNetworkClient and creates a new proxy provider and block tracker with the proxied globally selected network client', () => { - const { controller, messenger, mockHasPermissions } = setup({ - state: { - domains: {}, - }, - useRequestQueuePreference: true, - }); - jest.spyOn(messenger, 'call'); - mockHasPermissions.mockReturnValue(false); - const result = controller.getProviderAndBlockTracker('example.com'); - expect(result).toBeDefined(); - // unfortunately checking which networkController method is called is the best - // proxy (no pun intended) for checking that the correct instance of the networkClient is used - expect(messenger.call).not.toHaveBeenCalledWith( - 'NetworkController:getNetworkClientById', - 'mainnet', - ); + + describe('when the domain does not have a cached networkProxy in the domainProxyMap and useRequestQueuePreference is true', () => { + describe('when the domain has permissions', () => { + it('calls to NetworkController:getNetworkClientById and creates a new proxy provider and block tracker with the non-proxied globally selected network client', () => { + const { controller, messenger, mockHasPermissions } = setup({ + state: { + domains: {}, + }, + useRequestQueuePreference: true, }); + jest.spyOn(messenger, 'call'); + mockHasPermissions.mockReturnValue(true); + + const result = controller.getProviderAndBlockTracker('example.com'); + expect(result).toBeDefined(); + // unfortunately checking which networkController method is called is the best + // proxy (no pun intended) for checking that the correct instance of the networkClient is used + expect(messenger.call).toHaveBeenCalledWith( + 'NetworkController:getNetworkClientById', + 'mainnet', + ); }); }); - describe('when the useRequestQueue preference is false', () => { + + describe('when the domain does not have permissions', () => { it('calls to NetworkController:getSelectedNetworkClient and creates a new proxy provider and block tracker with the proxied globally selected network client', () => { - const { controller, messenger } = setup({ + const { controller, messenger, mockHasPermissions } = setup({ state: { domains: {}, }, - useRequestQueuePreference: false, + useRequestQueuePreference: true, }); jest.spyOn(messenger, 'call'); - + mockHasPermissions.mockReturnValue(false); const result = controller.getProviderAndBlockTracker('example.com'); expect(result).toBeDefined(); // unfortunately checking which networkController method is called is the best @@ -592,8 +590,42 @@ describe('SelectedNetworkController', () => { 'NetworkController:getSelectedNetworkClient', ); }); + + it('throws an error if the globally selected network client is not initialized', () => { + const { controller, mockGetSelectedNetworkClient } = setup({ + state: { + domains: {}, + }, + useRequestQueuePreference: false, + }); + mockGetSelectedNetworkClient.mockReturnValue(undefined); + expect(() => + controller.getProviderAndBlockTracker('example.com'), + ).toThrow('Selected network not initialized'); + }); + }); + }); + + describe('when the domain does not have a cached networkProxy in the domainProxyMap and useRequestQueuePreference is false', () => { + it('calls to NetworkController:getSelectedNetworkClient and creates a new proxy provider and block tracker with the proxied globally selected network client', () => { + const { controller, messenger } = setup({ + state: { + domains: {}, + }, + useRequestQueuePreference: false, + }); + jest.spyOn(messenger, 'call'); + + const result = controller.getProviderAndBlockTracker('example.com'); + expect(result).toBeDefined(); + // unfortunately checking which networkController method is called is the best + // proxy (no pun intended) for checking that the correct instance of the networkClient is used + expect(messenger.call).toHaveBeenCalledWith( + 'NetworkController:getSelectedNetworkClient', + ); }); }); + // TODO - improve these tests by using a full NetworkController and doing more robust behavioral testing describe('when the domain is a snap (starts with "npm:" or "local:")', () => { it('returns a proxied globally selected networkClient and does not create a new proxy in the domainProxyMap', () => { @@ -614,7 +646,23 @@ describe('SelectedNetworkController', () => { ); expect(result).toBeDefined(); }); + + it('throws an error if the globally selected network client is not initialized', () => { + const { controller, mockGetSelectedNetworkClient } = setup({ + state: { + domains: {}, + }, + useRequestQueuePreference: false, + }); + const snapDomain = 'npm:@metamask/bip32-example-snap'; + mockGetSelectedNetworkClient.mockReturnValue(undefined); + + expect(() => controller.getProviderAndBlockTracker(snapDomain)).toThrow( + 'Selected network not initialized', + ); + }); }); + describe('when the domain is a "metamask"', () => { it('returns a proxied globally selected networkClient and does not create a new proxy in the domainProxyMap', () => { const { controller, domainProxyMap, messenger } = setup({ @@ -633,6 +681,7 @@ describe('SelectedNetworkController', () => { 'NetworkController:getSelectedNetworkClient', ); }); + it('throws an error if the globally selected network client is not initialized', () => { const { controller, mockGetSelectedNetworkClient } = setup({ state: { @@ -649,51 +698,59 @@ describe('SelectedNetworkController', () => { }); }); - describe('When a permission is added or removed', () => { - it('should add new domain to domains list on permission add if #useRequestQueuePreference is true', async () => { - const { controller, messenger } = setup({ - useRequestQueuePreference: true, + describe('PermissionController:stateChange', () => { + describe('on permission add', () => { + it('should add new domain to domains list when useRequestQueuePreference is true', async () => { + const { controller, messenger } = setup({ + useRequestQueuePreference: true, + }); + const mockPermission = { + parentCapability: 'eth_accounts', + id: 'example.com', + date: Date.now(), + caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], + }; + + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, + [ + { + op: 'add', + path: ['subjects', 'example.com', 'permissions'], + value: mockPermission, + }, + ], + ); + + const { domains } = controller.state; + expect(domains['example.com']).toBeDefined(); }); - const mockPermission = { - parentCapability: 'eth_accounts', - id: 'example.com', - date: Date.now(), - caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], - }; - - messenger.publish('PermissionController:stateChange', { subjects: {} }, [ - { - op: 'add', - path: ['subjects', 'example.com', 'permissions'], - value: mockPermission, - }, - ]); - const { domains } = controller.state; - expect(domains['example.com']).toBeDefined(); - }); + it('should not add new domain to domains list when useRequestQueuePreference is false', async () => { + const { controller, messenger } = setup({}); + const mockPermission = { + parentCapability: 'eth_accounts', + id: 'example.com', + date: Date.now(), + caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], + }; - it('should not add new domain to domains list on permission add if #useRequestQueuePreference is false', async () => { - const { controller, messenger } = setup({ - useRequestQueuePreference: false, - }); - const mockPermission = { - parentCapability: 'eth_accounts', - id: 'example.com', - date: Date.now(), - caveats: [{ type: 'restrictToAccounts', value: ['0x...'] }], - }; - - messenger.publish('PermissionController:stateChange', { subjects: {} }, [ - { - op: 'add', - path: ['subjects', 'example.com', 'permissions'], - value: mockPermission, - }, - ]); + messenger.publish( + 'PermissionController:stateChange', + { subjects: {} }, + [ + { + op: 'add', + path: ['subjects', 'example.com', 'permissions'], + value: mockPermission, + }, + ], + ); - const { domains } = controller.state; - expect(domains['example.com']).toBeUndefined(); + const { domains } = controller.state; + expect(domains['example.com']).toBeUndefined(); + }); }); describe('on permission removal', () => { @@ -774,82 +831,13 @@ describe('SelectedNetworkController', () => { }); }); - describe('Constructor checks for domains in permissions', () => { - describe('when useRequestQueuePreference is true', () => { - it('should set networkClientId for domains not already in state', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, - }, - getSubjectNames: ['newdomain.com'], - useRequestQueuePreference: true, - }); - - expect(controller.state.domains).toStrictEqual({ - 'newdomain.com': 'mainnet', - 'existingdomain.com': 'initialNetworkId', - }); - }); - - it('should not modify domains already in state', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, - }, - getSubjectNames: ['existingdomain.com'], - useRequestQueuePreference: true, - }); - - expect(controller.state.domains).toStrictEqual({ - 'existingdomain.com': 'initialNetworkId', - }); - }); - }); - - describe('when useRequestQueuePreference is false', () => { - it('should not set networkClientId for new domains', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, - }, - getSubjectNames: ['newdomain.com'], - }); - - expect(controller.state.domains).toStrictEqual({ - 'existingdomain.com': 'initialNetworkId', - }); - }); - - it('should not modify domains already in state', async () => { - const { controller } = setup({ - state: { - domains: { - 'existingdomain.com': 'initialNetworkId', - }, - }, - getSubjectNames: ['existingdomain.com'], - }); - - expect(controller.state.domains).toStrictEqual({ - 'existingdomain.com': 'initialNetworkId', - }); - }); - }); - }); - // because of the opacity of the networkClient and proxy implementations, // its impossible to make valuable assertions around which networkClient proxies - // should be targeted when the useRequestQueue state is toggled on and off: + // should be targeted when the useRequestQueuePreference state is toggled on and off: // When toggled on, the networkClient for the globally selected networkClientId should be used - **not** the NetworkController's proxy of this networkClient. // When toggled off, the NetworkControllers proxy of the globally selected networkClient should be used // TODO - improve these tests by using a full NetworkController and doing more robust behavioral testing - describe('On useRequestQueue toggle state change', () => { + describe('onPreferencesStateChange', () => { const mockProxyProvider = { setTarget: jest.fn(), } as unknown as ProviderProxy; @@ -883,10 +871,7 @@ describe('SelectedNetworkController', () => { messenger, } = setup({ state: { - domains: { - 'example.com': 'foo', - 'test.com': 'bar', - }, + domains: {}, }, useRequestQueuePreference: false, domainProxyMap, @@ -908,6 +893,7 @@ describe('SelectedNetworkController', () => { expect(mockProxyBlockTracker.setTarget).toHaveBeenCalledTimes(2); }); }); + describe('when domains do not have permissions', () => { it('does not change the target of the existing proxy', () => { const domainProxyMap = new Map([ @@ -928,10 +914,7 @@ describe('SelectedNetworkController', () => { ]); const { mockHasPermissions, triggerPreferencesStateChange } = setup({ state: { - domains: { - 'example.com': 'foo', - 'test.com': 'bar', - }, + domains: {}, }, useRequestQueuePreference: false, domainProxyMap, @@ -946,6 +929,7 @@ describe('SelectedNetworkController', () => { }); }); }); + describe('when toggled from on to off', () => { it('sets the target of the existing proxies to the proxied globally selected networkClient', () => { const domainProxyMap = new Map([ From e2b05d9dd76e691d18c52750cfb919f4a47a0e61 Mon Sep 17 00:00:00 2001 From: Shane Date: Wed, 19 Jun 2024 14:43:13 -0400 Subject: [PATCH 08/11] fix: disable some transaction controller integration tests (#4429) ## Explanation Please see https://github.com/MetaMask/MetaMask-planning/issues/2668 ## References See: https://github.com/MetaMask/MetaMask-planning/issues/2668 Unblocks: https://github.com/MetaMask/core/pull/4327 ## Changelog No user facing changes ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate --------- Co-authored-by: Jiexi Luan Co-authored-by: Derek Brans --- packages/transaction-controller/jest.config.js | 4 ++-- .../src/TransactionControllerIntegration.test.ts | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/transaction-controller/jest.config.js b/packages/transaction-controller/jest.config.js index 3369de0f1df..9ff351e9c16 100644 --- a/packages/transaction-controller/jest.config.js +++ b/packages/transaction-controller/jest.config.js @@ -19,8 +19,8 @@ module.exports = merge(baseConfig, { global: { branches: 93.65, functions: 98.38, - lines: 98.84, - statements: 98.85, + lines: 98.8, + statements: 98.81, }, }, diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index e683f48243e..4b87353e0d7 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -266,7 +266,8 @@ describe('TransactionController Integration', () => { transactionController.destroy(); }); - it('should submit all approved transactions in state', async () => { + // eslint-disable-next-line jest/no-disabled-tests + it.skip('should submit all approved transactions in state', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( InfuraNetworkType.goerli, @@ -804,7 +805,8 @@ describe('TransactionController Integration', () => { }); describe('when transactions are added concurrently with different networkClientIds but on the same chainId', () => { - it('should add each transaction with consecutive nonces', async () => { + // eslint-disable-next-line jest/no-disabled-tests + it.skip('should add each transaction with consecutive nonces', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( InfuraNetworkType.goerli, @@ -913,7 +915,8 @@ describe('TransactionController Integration', () => { }); describe('when transactions are added concurrently with the same networkClientId', () => { - it('should add each transaction with consecutive nonces', async () => { + // eslint-disable-next-line jest/no-disabled-tests + it.skip('should add each transaction with consecutive nonces', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( InfuraNetworkType.goerli, @@ -1191,7 +1194,8 @@ describe('TransactionController Integration', () => { describe('startIncomingTransactionPolling', () => { // TODO(JL): IncomingTransactionHelper doesn't populate networkClientId on the generated tx object. Should it?.. - it('should add incoming transactions to state with the correct chainId for the given networkClientId on the next block', async () => { + // eslint-disable-next-line jest/no-disabled-tests + it.skip('should add incoming transactions to state with the correct chainId for the given networkClientId on the next block', async () => { mockNetwork({ networkClientConfiguration: buildInfuraNetworkClientConfiguration( InfuraNetworkType.mainnet, @@ -1617,7 +1621,8 @@ describe('TransactionController Integration', () => { }); describe('updateIncomingTransactions', () => { - it('should add incoming transactions to state with the correct chainId for the given networkClientId without waiting for the next block', async () => { + // eslint-disable-next-line jest/no-disabled-tests + it.skip('should add incoming transactions to state with the correct chainId for the given networkClientId without waiting for the next block', async () => { const selectedAddress = ETHERSCAN_TRANSACTION_BASE_MOCK.to; const selectedAccountMock = createMockInternalAccount({ address: selectedAddress, From 63639e36b5020c858b2467fac8b98d484f670294 Mon Sep 17 00:00:00 2001 From: Derek Brans Date: Thu, 20 Jun 2024 09:00:19 -0400 Subject: [PATCH 09/11] docs: document TransactionStatus enum (#4380) ## Explanation Met with @matthewwalsh0 who took the time to explain each status. ## References ## Changelog ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- packages/transaction-controller/src/types.ts | 70 ++++++++++++++++---- 1 file changed, 57 insertions(+), 13 deletions(-) diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index c84593ab406..9cbc4e0cd72 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -450,39 +450,83 @@ export type SendFlowHistoryEntry = { }; /** - * The status of the transaction. Each status represents the state of the transaction internally - * in the wallet. Some of these correspond with the state of the transaction on the network, but - * some are wallet-specific. + * Represents the status of a transaction within the wallet. + * Each status reflects the state of the transaction internally, + * with some statuses corresponding to the transaction's state on the network. + * + * The typical transaction lifecycle follows this state machine: + * unapproved -> approved -> signed -> submitted -> FINAL_STATE + * where FINAL_STATE is one of: confirmed, failed, dropped, or rejected. */ export enum TransactionStatus { + /** + * The initial state of a transaction before user approval. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - approved = 'approved', - /** @deprecated Determined by the clients using the transaction type. No longer used. */ + unapproved = 'unapproved', + + /** + * The transaction has been approved by the user but is not yet signed. + * This status is usually brief but may be longer for scenarios like hardware wallet usage. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - cancelled = 'cancelled', + approved = 'approved', + + /** + * The transaction is signed and in the process of being submitted to the network. + * This status is typically short-lived but can be longer for certain cases, such as smart transactions. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - confirmed = 'confirmed', + signed = 'signed', + + /** + * The transaction has been submitted to the network and is awaiting confirmation. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - dropped = 'dropped', + submitted = 'submitted', + + /** + * The transaction has been successfully executed and confirmed on the blockchain. + * This is a final state. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - failed = 'failed', + confirmed = 'confirmed', + + /** + * The transaction encountered an error during execution on the blockchain and failed. + * This is a final state. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - rejected = 'rejected', + failed = 'failed', + + /** + * The transaction was superseded by another transaction, resulting in its dismissal. + * This is a final state. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - signed = 'signed', + dropped = 'dropped', + + /** + * The transaction was rejected by the user and not processed further. + * This is a final state. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - submitted = 'submitted', + rejected = 'rejected', + + /** + * @deprecated This status is no longer used. + */ // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/naming-convention - unapproved = 'unapproved', + cancelled = 'cancelled', } /** From 1921526da1a0d694efe2d06d65ab597fee621621 Mon Sep 17 00:00:00 2001 From: Monte Lai Date: Thu, 20 Jun 2024 21:18:32 +0800 Subject: [PATCH 10/11] fix: update tokens controllers to use selectedAccountId instead of selectedAddress (#4219) ## Explanation This PR updates the `selectedAccount` to `selectedAccountId` in the token controllers ## References Fixes https://github.com/MetaMask/accounts-planning/issues/381 ## Changelog ### `@metamask/assets-controllers` - **BREAKING**: `TokenBalancesController` update `PreferencesConrtollerGetStateAction` to `AccountsControllerGetSelectedAccountAction` - **BREAKING**: `TokenDetectionController` change `selectedAddress` to `selectedAccountId` - **ADDED**: `TokenDetectionController` add `getAccountAction` - **BREAKING**: `TokenRatesController` change `selectedAddress` to `selectedAccountId` - **BREAKING**: `onPreferencesStateChange` arg removed and `getInternalAccount` and `onSelectedAccountChange` added in `TokenRatesController` - **ADDED**: `getAccountAction` added in`TokensController` - **BREAKING**: Changed `selectedAddress` to `selectedAccountId` and `PreferencesControllerStateChangeEvent` to `AccountsControllerSelectedEvmAccountChangeEvent` in the `TokensController` - **ADDED**: `getAccountAction` added in`TokensController` ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've highlighted breaking changes using the "BREAKING" category above as appropriate --- .../src/TokenBalancesController.test.ts | 309 ++++---- .../src/TokenBalancesController.ts | 14 +- .../src/TokenDetectionController.test.ts | 700 ++++++++++++------ .../src/TokenDetectionController.ts | 55 +- .../src/TokenRatesController.test.ts | 106 +-- .../src/TokenRatesController.ts | 75 +- .../src/TokensController.test.ts | 406 +++++++--- .../src/TokensController.ts | 106 ++- 8 files changed, 1138 insertions(+), 633 deletions(-) diff --git a/packages/assets-controllers/src/TokenBalancesController.test.ts b/packages/assets-controllers/src/TokenBalancesController.test.ts index 1d722b421ca..49390c64fe3 100644 --- a/packages/assets-controllers/src/TokenBalancesController.test.ts +++ b/packages/assets-controllers/src/TokenBalancesController.test.ts @@ -1,8 +1,10 @@ import { ControllerMessenger } from '@metamask/base-controller'; import { toHex } from '@metamask/controller-utils'; +import type { InternalAccount } from '@metamask/keyring-api'; import BN from 'bn.js'; import { flushPromises } from '../../../tests/helpers'; +import { createMockInternalAccount } from '../../accounts-controller/src/tests/mocks'; import type { AllowedActions, AllowedEvents, @@ -31,19 +33,63 @@ function getMessenger( ): TokenBalancesControllerMessenger { return controllerMessenger.getRestricted({ name: controllerName, - allowedActions: ['PreferencesController:getState'], + allowedActions: ['AccountsController:getSelectedAccount'], allowedEvents: ['TokensController:stateChange'], }); } -describe('TokenBalancesController', () => { - let controllerMessenger: ControllerMessenger; - let messenger: TokenBalancesControllerMessenger; +const setupController = ({ + config, + mock, +}: { + config?: Partial[0]>; + mock: { + getBalanceOf?: BN; + selectedAccount: InternalAccount; + }; +}): { + controller: TokenBalancesController; + messenger: TokenBalancesControllerMessenger; + mockSelectedAccount: jest.Mock; + mockGetERC20BalanceOf: jest.Mock; + triggerTokensStateChange: (state: TokensControllerState) => Promise; +} => { + const controllerMessenger = new ControllerMessenger< + AllowedActions, + AllowedEvents + >(); + const messenger = getMessenger(controllerMessenger); + + const mockSelectedAccount = jest.fn().mockReturnValue(mock.selectedAccount); + const mockGetERC20BalanceOf = jest.fn().mockReturnValue(mock.getBalanceOf); + + controllerMessenger.registerActionHandler( + 'AccountsController:getSelectedAccount', + mockSelectedAccount, + ); + + const controller = new TokenBalancesController({ + getERC20BalanceOf: mockGetERC20BalanceOf, + messenger, + ...config, + }); + + const triggerTokensStateChange = async (state: TokensControllerState) => { + controllerMessenger.publish('TokensController:stateChange', state, []); + }; + return { + controller, + messenger, + mockSelectedAccount, + mockGetERC20BalanceOf, + triggerTokensStateChange, + }; +}; + +describe('TokenBalancesController', () => { beforeEach(() => { jest.useFakeTimers(); - controllerMessenger = new ControllerMessenger(); - messenger = getMessenger(controllerMessenger); }); afterEach(() => { @@ -51,23 +97,16 @@ describe('TokenBalancesController', () => { }); it('should set default state', () => { - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - getERC20BalanceOf: jest.fn(), - messenger, + const { controller } = setupController({ + mock: { + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + }, }); expect(controller.state).toStrictEqual({ contractBalances: {} }); }); it('should poll and update balances in the right interval', async () => { - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); const updateBalancesSpy = jest.spyOn( TokenBalancesController.prototype, 'updateBalances', @@ -76,7 +115,7 @@ describe('TokenBalancesController', () => { new TokenBalancesController({ interval: 10, getERC20BalanceOf: jest.fn(), - messenger, + messenger: getMessenger(new ControllerMessenger()), }); await flushPromises(); @@ -90,16 +129,16 @@ describe('TokenBalancesController', () => { it('should update balances if enabled', async () => { const address = '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0'; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - disabled: false, - tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], - interval: 10, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller } = setupController({ + config: { + disabled: false, + tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], + interval: 10, + }, + mock: { + getBalanceOf: new BN(1), + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + }, }); await controller.updateBalances(); @@ -111,16 +150,16 @@ describe('TokenBalancesController', () => { it('should not update balances if disabled', async () => { const address = '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0'; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - disabled: true, - tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], - interval: 10, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller } = setupController({ + config: { + disabled: true, + tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], + interval: 10, + }, + mock: { + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + getBalanceOf: new BN(1), + }, }); await controller.updateBalances(); @@ -130,16 +169,16 @@ describe('TokenBalancesController', () => { it('should update balances if controller is manually enabled', async () => { const address = '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0'; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - disabled: true, - tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], - interval: 10, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller } = setupController({ + config: { + disabled: true, + tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], + interval: 10, + }, + mock: { + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + getBalanceOf: new BN(1), + }, }); await controller.updateBalances(); @@ -156,16 +195,16 @@ describe('TokenBalancesController', () => { it('should not update balances if controller is manually disabled', async () => { const address = '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0'; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - disabled: false, - tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], - interval: 10, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller } = setupController({ + config: { + disabled: false, + tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], + interval: 10, + }, + mock: { + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + getBalanceOf: new BN(1), + }, }); await controller.updateBalances(); @@ -184,20 +223,17 @@ describe('TokenBalancesController', () => { it('should update balances if tokens change and controller is manually enabled', async () => { const address = '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0'; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - disabled: true, - tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], - interval: 10, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller, triggerTokensStateChange } = setupController({ + config: { + disabled: true, + tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], + interval: 10, + }, + mock: { + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + getBalanceOf: new BN(1), + }, }); - const triggerTokensStateChange = async (state: TokensControllerState) => { - controllerMessenger.publish('TokensController:stateChange', state, []); - }; await controller.updateBalances(); @@ -222,20 +258,17 @@ describe('TokenBalancesController', () => { it('should not update balances if tokens change and controller is manually disabled', async () => { const address = '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0'; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - disabled: false, - tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], - interval: 10, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller, triggerTokensStateChange } = setupController({ + config: { + disabled: false, + tokens: [{ address, decimals: 18, symbol: 'EOS', aggregators: [] }], + interval: 10, + }, + mock: { + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + getBalanceOf: new BN(1), + }, }); - const triggerTokensStateChange = async (state: TokensControllerState) => { - controllerMessenger.publish('TokensController:stateChange', state, []); - }; await controller.updateBalances(); @@ -261,14 +294,14 @@ describe('TokenBalancesController', () => { }); it('should clear previous interval', async () => { - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - interval: 1337, - getERC20BalanceOf: jest.fn(), - messenger, + const { controller } = setupController({ + config: { + interval: 1337, + }, + mock: { + selectedAccount: createMockInternalAccount({ address: '0x1234' }), + getBalanceOf: new BN(1), + }, }); const mockClearTimeout = jest.spyOn(global, 'clearTimeout'); @@ -291,15 +324,17 @@ describe('TokenBalancesController', () => { aggregators: [], }, ]; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress }), - ); - const controller = new TokenBalancesController({ - interval: 1337, - tokens, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller } = setupController({ + config: { + interval: 1337, + tokens, + }, + mock: { + selectedAccount: createMockInternalAccount({ + address: selectedAddress, + }), + getBalanceOf: new BN(1), + }, }); expect(controller.state.contractBalances).toStrictEqual({}); @@ -314,9 +349,6 @@ describe('TokenBalancesController', () => { it('should handle `getERC20BalanceOf` error case', async () => { const errorMsg = 'Failed to get balance'; const address = '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0'; - const getERC20BalanceOfStub = jest - .fn() - .mockReturnValue(Promise.reject(new Error(errorMsg))); const tokens: Token[] = [ { address, @@ -326,17 +358,21 @@ describe('TokenBalancesController', () => { }, ]; - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({}), - ); - const controller = new TokenBalancesController({ - interval: 1337, - tokens, - getERC20BalanceOf: getERC20BalanceOfStub, - messenger, + const { controller, mockGetERC20BalanceOf } = setupController({ + config: { + interval: 1337, + tokens, + }, + mock: { + selectedAccount: createMockInternalAccount({ + address, + }), + }, }); + // @ts-expect-error Testing error case + mockGetERC20BalanceOf.mockReturnValueOnce(new Error(errorMsg)); + expect(controller.state.contractBalances).toStrictEqual({}); await controller.updateBalances(); @@ -344,8 +380,7 @@ describe('TokenBalancesController', () => { expect(tokens[0].hasBalanceError).toBe(true); expect(controller.state.contractBalances[address]).toBe(toHex(0)); - getERC20BalanceOfStub.mockReturnValue(new BN(1)); - + mockGetERC20BalanceOf.mockReturnValueOnce(new BN(1)); await controller.updateBalances(); expect(tokens[0].hasBalanceError).toBe(false); @@ -354,18 +389,18 @@ describe('TokenBalancesController', () => { }); it('should update balances when tokens change', async () => { - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - getERC20BalanceOf: jest.fn(), - interval: 1337, - messenger, + const { controller, triggerTokensStateChange } = setupController({ + config: { + interval: 1337, + }, + mock: { + selectedAccount: createMockInternalAccount({ + address: '0x1234', + }), + getBalanceOf: new BN(1), + }, }); - const triggerTokensStateChange = async (state: TokensControllerState) => { - controllerMessenger.publish('TokensController:stateChange', state, []); - }; + const updateBalancesSpy = jest.spyOn(controller, 'updateBalances'); await triggerTokensStateChange({ @@ -383,18 +418,18 @@ describe('TokenBalancesController', () => { }); it('should update token balances when detected tokens are added', async () => { - controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - jest.fn().mockReturnValue({ selectedAddress: '0x1234' }), - ); - const controller = new TokenBalancesController({ - interval: 1337, - getERC20BalanceOf: jest.fn().mockReturnValue(new BN(1)), - messenger, + const { controller, triggerTokensStateChange } = setupController({ + config: { + interval: 1337, + }, + mock: { + selectedAccount: createMockInternalAccount({ + address: '0x1234', + }), + getBalanceOf: new BN(1), + }, }); - const triggerTokensStateChange = async (state: TokensControllerState) => { - controllerMessenger.publish('TokensController:stateChange', state, []); - }; + expect(controller.state.contractBalances).toStrictEqual({}); await triggerTokensStateChange({ diff --git a/packages/assets-controllers/src/TokenBalancesController.ts b/packages/assets-controllers/src/TokenBalancesController.ts index 323544f8135..a1b58a43408 100644 --- a/packages/assets-controllers/src/TokenBalancesController.ts +++ b/packages/assets-controllers/src/TokenBalancesController.ts @@ -1,3 +1,4 @@ +import { type AccountsControllerGetSelectedAccountAction } from '@metamask/accounts-controller'; import { type RestrictedControllerMessenger, type ControllerGetStateAction, @@ -5,7 +6,6 @@ import { BaseController, } from '@metamask/base-controller'; import { safelyExecute, toHex } from '@metamask/controller-utils'; -import type { PreferencesControllerGetStateAction } from '@metamask/preferences-controller'; import type { AssetsContractController } from './AssetsContractController'; import type { Token } from './TokenRatesController'; @@ -56,7 +56,7 @@ export type TokenBalancesControllerGetStateAction = ControllerGetStateAction< export type TokenBalancesControllerActions = TokenBalancesControllerGetStateAction; -export type AllowedActions = PreferencesControllerGetStateAction; +export type AllowedActions = AccountsControllerGetSelectedAccountAction; export type TokenBalancesControllerStateChangeEvent = ControllerStateChangeEvent< @@ -201,16 +201,18 @@ export class TokenBalancesController extends BaseController< if (this.#disabled) { return; } - - const { selectedAddress } = this.messagingSystem.call( - 'PreferencesController:getState', + const selectedInternalAccount = this.messagingSystem.call( + 'AccountsController:getSelectedAccount', ); const newContractBalances: ContractBalances = {}; for (const token of this.#tokens) { const { address } = token; try { - const balance = await this.#getERC20BalanceOf(address, selectedAddress); + const balance = await this.#getERC20BalanceOf( + address, + selectedInternalAccount.address, + ); newContractBalances[address] = toHex(balance); token.hasBalanceError = false; } catch (error) { diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 1012531be9b..2e23ceb5fd9 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -27,6 +27,7 @@ import nock from 'nock'; import * as sinon from 'sinon'; import { advanceTime } from '../../../tests/helpers'; +import { createMockInternalAccount } from '../../accounts-controller/src/tests/mocks'; import { formatAggregatorNames } from './assetsUtil'; import { TOKEN_END_POINT_API } from './token-service'; import type { @@ -144,6 +145,7 @@ function buildTokenDetectionControllerMessenger( return controllerMessenger.getRestricted({ name: controllerName, allowedActions: [ + 'AccountsController:getAccount', 'AccountsController:getSelectedAccount', 'KeyringController:getState', 'NetworkController:getNetworkClientById', @@ -155,7 +157,7 @@ function buildTokenDetectionControllerMessenger( 'PreferencesController:getState', ], allowedEvents: [ - 'AccountsController:selectedAccountChange', + 'AccountsController:selectedEvmAccountChange', 'KeyringController:lock', 'KeyringController:unlock', 'NetworkController:networkDidChange', @@ -166,6 +168,8 @@ function buildTokenDetectionControllerMessenger( } describe('TokenDetectionController', () => { + const defaultSelectedAccount = createMockInternalAccount(); + beforeEach(async () => { nock(TOKEN_END_POINT_API) .get(getTokensPath(ChainId.mainnet)) @@ -207,6 +211,10 @@ describe('TokenDetectionController', () => { await withController( { isKeyringUnlocked: false, + options: {}, + mocks: { + getSelectedAccount: defaultSelectedAccount, + }, }, async ({ controller }) => { const mockTokens = sinon.stub(controller, 'detectTokens'); @@ -225,6 +233,10 @@ describe('TokenDetectionController', () => { await withController( { isKeyringUnlocked: false, + options: {}, + mocks: { + getSelectedAccount: defaultSelectedAccount, + }, }, async ({ controller, triggerKeyringUnlock }) => { const mockTokens = sinon.stub(controller, 'detectTokens'); @@ -259,16 +271,24 @@ describe('TokenDetectionController', () => { }); it('should poll and detect tokens on interval while on supported networks', async () => { - await withController(async ({ controller }) => { - const mockTokens = sinon.stub(controller, 'detectTokens'); - controller.setIntervalLength(10); + await withController( + { + options: {}, + mocks: { + getSelectedAccount: defaultSelectedAccount, + }, + }, + async ({ controller }) => { + const mockTokens = sinon.stub(controller, 'detectTokens'); + controller.setIntervalLength(10); - await controller.start(); + await controller.start(); - expect(mockTokens.calledOnce).toBe(true); - await advanceTime({ clock, duration: 15 }); - expect(mockTokens.calledTwice).toBe(true); - }); + expect(mockTokens.calledOnce).toBe(true); + await advanceTime({ clock, duration: 15 }); + expect(mockTokens.calledTwice).toBe(true); + }, + ); }); it('should not autodetect while not on supported networks', async () => { @@ -280,6 +300,9 @@ describe('TokenDetectionController', () => { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, }, + mocks: { + getSelectedAccount: defaultSelectedAccount, + }, }, async ({ controller, mockNetworkState }) => { mockNetworkState({ @@ -297,12 +320,17 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ controller, mockTokenListGetState, callActionSpy }) => { @@ -333,7 +361,7 @@ describe('TokenDetectionController', () => { [sampleTokenA], { chainId: ChainId.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }, ); }, @@ -344,12 +372,17 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -397,7 +430,7 @@ describe('TokenDetectionController', () => { [sampleTokenA], { chainId: '0x89', - selectedAddress, + selectedAddress: selectedAccount.address, }, ); }, @@ -409,14 +442,19 @@ describe('TokenDetectionController', () => { [sampleTokenA.address]: new BN(1), [sampleTokenB.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); const interval = 100; await withController( { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, interval, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ controller, mockTokenListGetState, callActionSpy }) => { @@ -459,7 +497,7 @@ describe('TokenDetectionController', () => { [sampleTokenA, sampleTokenB], { chainId: ChainId.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }, ); }, @@ -470,12 +508,17 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -526,6 +569,9 @@ describe('TokenDetectionController', () => { options: { getBalancesInSingleCall: mockGetBalancesInSingleCall, }, + mocks: { + getSelectedAccount: defaultSelectedAccount, + }, }, async ({ controller, mockTokenListGetState, callActionSpy }) => { mockTokenListGetState({ @@ -573,19 +619,24 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const firstSelectedAddress = - '0x0000000000000000000000000000000000000001'; - const secondSelectedAddress = - '0x0000000000000000000000000000000000000002'; + const firstSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + const secondSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000002', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress: firstSelectedAddress, + }, + mocks: { + getSelectedAccount: firstSelectedAccount, }, }, async ({ + mockGetAccount, mockTokenListGetState, triggerSelectedAccountChange, callActionSpy, @@ -610,9 +661,8 @@ describe('TokenDetectionController', () => { }, }); - triggerSelectedAccountChange({ - address: secondSelectedAddress, - } as InternalAccount); + mockGetAccount(secondSelectedAccount); + triggerSelectedAccountChange(secondSelectedAccount); await advanceTime({ clock, duration: 1 }); expect(callActionSpy).toHaveBeenCalledWith( @@ -620,7 +670,7 @@ describe('TokenDetectionController', () => { [sampleTokenA], { chainId: ChainId.mainnet, - selectedAddress: secondSelectedAddress, + selectedAddress: secondSelectedAccount.address, }, ); }, @@ -631,13 +681,17 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, }, }, async ({ @@ -666,7 +720,7 @@ describe('TokenDetectionController', () => { }); triggerSelectedAccountChange({ - address: selectedAddress, + address: selectedAccount.address, } as InternalAccount); await advanceTime({ clock, duration: 1 }); @@ -682,16 +736,20 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const firstSelectedAddress = - '0x0000000000000000000000000000000000000001'; - const secondSelectedAddress = - '0x0000000000000000000000000000000000000002'; + const firstSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + const secondSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000002', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress: firstSelectedAddress, + }, + mocks: { + getSelectedAccount: firstSelectedAccount, }, isKeyringUnlocked: false, }, @@ -721,7 +779,7 @@ describe('TokenDetectionController', () => { }); triggerSelectedAccountChange({ - address: secondSelectedAddress, + address: secondSelectedAccount.address, } as InternalAccount); await advanceTime({ clock, duration: 1 }); @@ -739,16 +797,20 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const firstSelectedAddress = - '0x0000000000000000000000000000000000000001'; - const secondSelectedAddress = - '0x0000000000000000000000000000000000000002'; + const firstSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + const secondSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000002', + }); await withController( { options: { disabled: true, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress: firstSelectedAddress, + }, + mocks: { + getSelectedAccount: firstSelectedAccount, }, }, async ({ @@ -777,7 +839,7 @@ describe('TokenDetectionController', () => { }); triggerSelectedAccountChange({ - address: secondSelectedAddress, + address: secondSelectedAccount.address, } as InternalAccount); await advanceTime({ clock, duration: 1 }); @@ -805,21 +867,27 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const firstSelectedAddress = - '0x0000000000000000000000000000000000000001'; - const secondSelectedAddress = - '0x0000000000000000000000000000000000000002'; + const firstSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + const secondSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000002', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress: firstSelectedAddress, + }, + mocks: { + getSelectedAccount: firstSelectedAccount, }, }, async ({ + mockGetAccount, mockTokenListGetState, triggerPreferencesStateChange, + triggerSelectedAccountChange, callActionSpy, }) => { mockTokenListGetState({ @@ -844,17 +912,18 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress: secondSelectedAddress, useTokenDetection: true, }); + mockGetAccount(secondSelectedAccount); + triggerSelectedAccountChange(secondSelectedAccount); await advanceTime({ clock, duration: 1 }); - expect(callActionSpy).toHaveBeenCalledWith( + expect(callActionSpy).toHaveBeenLastCalledWith( 'TokensController:addDetectedTokens', [sampleTokenA], { chainId: ChainId.mainnet, - selectedAddress: secondSelectedAddress, + selectedAddress: secondSelectedAccount.address, }, ); }, @@ -865,20 +934,26 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, }, }, async ({ + mockGetAccount, mockTokenListGetState, triggerPreferencesStateChange, callActionSpy, }) => { + mockGetAccount(selectedAccount); mockTokenListGetState({ ...getDefaultTokenListState(), tokensChainsCache: { @@ -901,14 +976,12 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress, useTokenDetection: false, }); await advanceTime({ clock, duration: 1 }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress, useTokenDetection: true, }); await advanceTime({ clock, duration: 1 }); @@ -918,7 +991,7 @@ describe('TokenDetectionController', () => { [sampleTokenA], { chainId: ChainId.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }, ); }, @@ -929,23 +1002,30 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const firstSelectedAddress = - '0x0000000000000000000000000000000000000001'; - const secondSelectedAddress = - '0x0000000000000000000000000000000000000002'; + const firstSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + const secondSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000002', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress: firstSelectedAddress, + }, + mocks: { + getSelectedAccount: firstSelectedAccount, }, }, async ({ + mockGetAccount, mockTokenListGetState, + triggerSelectedAccountChange, triggerPreferencesStateChange, callActionSpy, }) => { + mockGetAccount(firstSelectedAccount); mockTokenListGetState({ ...getDefaultTokenListState(), tokenList: { @@ -963,9 +1043,10 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress: secondSelectedAddress, useTokenDetection: false, }); + mockGetAccount(secondSelectedAccount); + triggerSelectedAccountChange(secondSelectedAccount); await advanceTime({ clock, duration: 1 }); expect(callActionSpy).not.toHaveBeenCalledWith( @@ -979,13 +1060,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -1010,7 +1096,6 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress, useTokenDetection: true, }); await advanceTime({ clock, duration: 1 }); @@ -1021,113 +1106,124 @@ describe('TokenDetectionController', () => { }, ); }); + }); - describe('when keyring is locked', () => { - it('should not detect new tokens after switching between accounts', async () => { - const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ - [sampleTokenA.address]: new BN(1), - }); - const firstSelectedAddress = - '0x0000000000000000000000000000000000000001'; - const secondSelectedAddress = - '0x0000000000000000000000000000000000000002'; - await withController( - { - options: { - disabled: false, - getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress: firstSelectedAddress, - }, - isKeyringUnlocked: false, + describe('when keyring is locked', () => { + it('should not detect new tokens after switching between accounts', async () => { + const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ + [sampleTokenA.address]: new BN(1), + }); + const firstSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + const secondSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000002', + }); + await withController( + { + options: { + disabled: false, + getBalancesInSingleCall: mockGetBalancesInSingleCall, }, - async ({ - mockTokenListGetState, - triggerPreferencesStateChange, - callActionSpy, - }) => { - mockTokenListGetState({ - ...getDefaultTokenListState(), - tokenList: { - [sampleTokenA.address]: { - name: sampleTokenA.name, - symbol: sampleTokenA.symbol, - decimals: sampleTokenA.decimals, - address: sampleTokenA.address, - occurrences: 1, - aggregators: sampleTokenA.aggregators, - iconUrl: sampleTokenA.image, - }, + mocks: { + getSelectedAccount: firstSelectedAccount, + getAccount: firstSelectedAccount, + }, + isKeyringUnlocked: false, + }, + async ({ + mockGetAccount, + mockTokenListGetState, + triggerPreferencesStateChange, + triggerSelectedAccountChange, + callActionSpy, + }) => { + mockTokenListGetState({ + ...getDefaultTokenListState(), + tokenList: { + [sampleTokenA.address]: { + name: sampleTokenA.name, + symbol: sampleTokenA.symbol, + decimals: sampleTokenA.decimals, + address: sampleTokenA.address, + occurrences: 1, + aggregators: sampleTokenA.aggregators, + iconUrl: sampleTokenA.image, }, - }); + }, + }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: secondSelectedAddress, - useTokenDetection: true, - }); - await advanceTime({ clock, duration: 1 }); + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + useTokenDetection: true, + }); + mockGetAccount(secondSelectedAccount); + triggerSelectedAccountChange(secondSelectedAccount); + await advanceTime({ clock, duration: 1 }); - expect(callActionSpy).not.toHaveBeenCalledWith( - 'TokensController:addDetectedTokens', - ); - }, - ); - }); + expect(callActionSpy).not.toHaveBeenCalledWith( + 'TokensController:addDetectedTokens', + ); + }, + ); + }); - it('should not detect new tokens after enabling token detection', async () => { - const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ - [sampleTokenA.address]: new BN(1), - }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; - await withController( - { - options: { - disabled: false, - getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, - }, - isKeyringUnlocked: false, + it('should not detect new tokens after enabling token detection', async () => { + const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ + [sampleTokenA.address]: new BN(1), + }); + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + await withController( + { + options: { + disabled: false, + getBalancesInSingleCall: mockGetBalancesInSingleCall, }, - async ({ - mockTokenListGetState, - triggerPreferencesStateChange, - callActionSpy, - }) => { - mockTokenListGetState({ - ...getDefaultTokenListState(), - tokenList: { - [sampleTokenA.address]: { - name: sampleTokenA.name, - symbol: sampleTokenA.symbol, - decimals: sampleTokenA.decimals, - address: sampleTokenA.address, - occurrences: 1, - aggregators: sampleTokenA.aggregators, - iconUrl: sampleTokenA.image, - }, + isKeyringUnlocked: false, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, + }, + async ({ + mockTokenListGetState, + triggerPreferencesStateChange, + callActionSpy, + }) => { + mockTokenListGetState({ + ...getDefaultTokenListState(), + tokenList: { + [sampleTokenA.address]: { + name: sampleTokenA.name, + symbol: sampleTokenA.symbol, + decimals: sampleTokenA.decimals, + address: sampleTokenA.address, + occurrences: 1, + aggregators: sampleTokenA.aggregators, + iconUrl: sampleTokenA.image, }, - }); + }, + }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress, - useTokenDetection: false, - }); - await advanceTime({ clock, duration: 1 }); + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + useTokenDetection: false, + }); + await advanceTime({ clock, duration: 1 }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress, - useTokenDetection: true, - }); - await advanceTime({ clock, duration: 1 }); + triggerPreferencesStateChange({ + ...getDefaultPreferencesState(), + useTokenDetection: true, + }); + await advanceTime({ clock, duration: 1 }); - expect(callActionSpy).not.toHaveBeenCalledWith( - 'TokensController:addDetectedTokens', - ); - }, - ); - }); + expect(callActionSpy).not.toHaveBeenCalledWith( + 'TokensController:addDetectedTokens', + ); + }, + ); }); }); @@ -1136,21 +1232,28 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const firstSelectedAddress = - '0x0000000000000000000000000000000000000001'; - const secondSelectedAddress = - '0x0000000000000000000000000000000000000002'; + const firstSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); + const secondSelectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000002', + }); await withController( { options: { disabled: true, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress: firstSelectedAddress, + }, + mocks: { + getAccount: firstSelectedAccount, + getSelectedAccount: firstSelectedAccount, }, }, async ({ + mockGetAccount, mockTokenListGetState, triggerPreferencesStateChange, + triggerSelectedAccountChange, callActionSpy, }) => { mockTokenListGetState({ @@ -1170,9 +1273,10 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress: secondSelectedAddress, useTokenDetection: true, }); + mockGetAccount(secondSelectedAccount); + triggerSelectedAccountChange(secondSelectedAccount); await advanceTime({ clock, duration: 1 }); expect(callActionSpy).not.toHaveBeenCalledWith( @@ -1186,13 +1290,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: true, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -1217,14 +1326,12 @@ describe('TokenDetectionController', () => { triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress, useTokenDetection: false, }); await advanceTime({ clock, duration: 1 }); triggerPreferencesStateChange({ ...getDefaultPreferencesState(), - selectedAddress, useTokenDetection: true, }); await advanceTime({ clock, duration: 1 }); @@ -1253,13 +1360,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -1298,7 +1410,7 @@ describe('TokenDetectionController', () => { [sampleTokenA], { chainId: '0x89', - selectedAddress, + selectedAddress: selectedAccount.address, }, ); }, @@ -1309,13 +1421,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -1360,13 +1477,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -1407,15 +1529,20 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, }, isKeyringUnlocked: false, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, + }, }, async ({ mockTokenListGetState, @@ -1457,13 +1584,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: true, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, }, }, async ({ @@ -1516,13 +1648,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ @@ -1561,7 +1698,7 @@ describe('TokenDetectionController', () => { [sampleTokenA], { chainId: ChainId.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }, ); }, @@ -1572,13 +1709,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ @@ -1607,15 +1749,20 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, }, isKeyringUnlocked: false, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, }, async ({ mockTokenListGetState, @@ -1655,13 +1802,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: true, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ @@ -1711,13 +1863,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ controller, mockTokenListGetState }) => { @@ -1777,13 +1934,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ @@ -1802,7 +1964,7 @@ describe('TokenDetectionController', () => { }); await controller.detectTokens({ networkClientId: NetworkType.goerli, - selectedAddress, + selectedAddress: selectedAccount.address, }); expect(callActionSpy).not.toHaveBeenCalledWith( 'TokensController:addDetectedTokens', @@ -1821,13 +1983,18 @@ describe('TokenDetectionController', () => { {}, ), ); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ @@ -1841,7 +2008,7 @@ describe('TokenDetectionController', () => { }); await controller.detectTokens({ networkClientId: NetworkType.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }); expect(callActionSpy).toHaveBeenLastCalledWith( 'TokensController:addDetectedTokens', @@ -1854,7 +2021,7 @@ describe('TokenDetectionController', () => { }; }), { - selectedAddress, + selectedAddress: selectedAccount.address, chainId: ChainId.mainnet, }, ); @@ -1866,13 +2033,18 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); await withController( { options: { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ controller, mockTokenListGetState, callActionSpy }) => { @@ -1898,7 +2070,7 @@ describe('TokenDetectionController', () => { await controller.detectTokens({ networkClientId: NetworkType.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }); expect(callActionSpy).toHaveBeenCalledWith( @@ -1906,7 +2078,7 @@ describe('TokenDetectionController', () => { [sampleTokenA], { chainId: ChainId.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }, ); }, @@ -1917,7 +2089,9 @@ describe('TokenDetectionController', () => { const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ [sampleTokenA.address]: new BN(1), }); - const selectedAddress = '0x0000000000000000000000000000000000000001'; + const selectedAccount = createMockInternalAccount({ + address: '0x0000000000000000000000000000000000000001', + }); const mockTrackMetaMetricsEvent = jest.fn(); await withController( @@ -1926,7 +2100,10 @@ describe('TokenDetectionController', () => { disabled: false, getBalancesInSingleCall: mockGetBalancesInSingleCall, trackMetaMetricsEvent: mockTrackMetaMetricsEvent, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, }, }, async ({ controller, mockTokenListGetState }) => { @@ -1952,7 +2129,7 @@ describe('TokenDetectionController', () => { await controller.detectTokens({ networkClientId: NetworkType.mainnet, - selectedAddress, + selectedAddress: selectedAccount.address, }); expect(mockTrackMetaMetricsEvent).toHaveBeenCalledWith({ @@ -1971,6 +2148,85 @@ describe('TokenDetectionController', () => { }, ); }); + + it('does not trigger `TokensController:addDetectedTokens` action when selectedAccount is not found', async () => { + const mockGetBalancesInSingleCall = jest.fn().mockResolvedValue({ + [sampleTokenA.address]: new BN(1), + }); + + const mockTrackMetaMetricsEvent = jest.fn(); + + await withController( + { + options: { + disabled: false, + getBalancesInSingleCall: mockGetBalancesInSingleCall, + trackMetaMetricsEvent: mockTrackMetaMetricsEvent, + }, + }, + async ({ + controller, + mockGetAccount, + mockTokenListGetState, + callActionSpy, + }) => { + // @ts-expect-error forcing an undefined value + mockGetAccount(undefined); + mockTokenListGetState({ + ...getDefaultTokenListState(), + tokensChainsCache: { + '0x1': { + timestamp: 0, + data: { + [sampleTokenA.address]: { + name: sampleTokenA.name, + symbol: sampleTokenA.symbol, + decimals: sampleTokenA.decimals, + address: sampleTokenA.address, + occurrences: 1, + aggregators: sampleTokenA.aggregators, + iconUrl: sampleTokenA.image, + }, + }, + }, + }, + }); + + await controller.detectTokens({ + networkClientId: NetworkType.mainnet, + }); + + expect(callActionSpy).toHaveBeenLastCalledWith( + 'TokensController:addDetectedTokens', + [ + { + address: '0x514910771AF9Ca656af840dff83E8264EcF986CA', + aggregators: [ + 'Paraswap', + 'PMM', + 'AirswapLight', + '0x', + 'Bancor', + 'CoinGecko', + 'Zapper', + 'Kleros', + 'Zerion', + 'CMC', + '1inch', + ], + decimals: 18, + image: + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + isERC721: false, + name: 'Chainlink', + symbol: 'LINK', + }, + ], + { chainId: '0x1', selectedAddress: '' }, + ); + }, + ); + }); }); }); @@ -1990,6 +2246,7 @@ function getTokensPath(chainId: Hex) { type WithControllerCallback = ({ controller, + mockGetAccount, mockGetSelectedAccount, mockKeyringGetState, mockTokensGetState, @@ -2007,6 +2264,7 @@ type WithControllerCallback = ({ triggerNetworkDidChange, }: { controller: TokenDetectionController; + mockGetAccount: (internalAccount: InternalAccount) => void; mockGetSelectedAccount: (address: string) => void; mockKeyringGetState: (state: KeyringControllerState) => void; mockTokensGetState: (state: TokensControllerState) => void; @@ -2034,6 +2292,10 @@ type WithControllerOptions = { options?: Partial[0]>; isKeyringUnlocked?: boolean; messenger?: ControllerMessenger; + mocks?: { + getAccount?: InternalAccount; + getSelectedAccount?: InternalAccount; + }; }; type WithControllerArgs = @@ -2053,16 +2315,25 @@ async function withController( ...args: WithControllerArgs ): Promise { const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]]; - const { options, isKeyringUnlocked, messenger } = rest; + const { options, isKeyringUnlocked, messenger, mocks } = rest; const controllerMessenger = messenger ?? new ControllerMessenger(); + const mockGetAccount = jest.fn(); + controllerMessenger.registerActionHandler( + 'AccountsController:getAccount', + mockGetAccount.mockReturnValue( + mocks?.getAccount ?? createMockInternalAccount({ address: '0x1' }), + ), + ); + const mockGetSelectedAccount = jest.fn(); controllerMessenger.registerActionHandler( 'AccountsController:getSelectedAccount', - mockGetSelectedAccount.mockReturnValue({ - address: '0x1', - } as InternalAccount), + mockGetSelectedAccount.mockReturnValue( + mocks?.getSelectedAccount ?? + createMockInternalAccount({ address: '0x1' }), + ), ); const mockKeyringState = jest.fn(); controllerMessenger.registerActionHandler( @@ -2140,6 +2411,9 @@ async function withController( try { return await fn({ controller, + mockGetAccount: (internalAccount: InternalAccount) => { + mockGetAccount.mockReturnValue(internalAccount); + }, mockGetSelectedAccount: (address: string) => { mockGetSelectedAccount.mockReturnValue({ address } as InternalAccount); }, @@ -2195,7 +2469,7 @@ async function withController( }, triggerSelectedAccountChange: (account: InternalAccount) => { controllerMessenger.publish( - 'AccountsController:selectedAccountChange', + 'AccountsController:selectedEvmAccountChange', account, ); }, diff --git a/packages/assets-controllers/src/TokenDetectionController.ts b/packages/assets-controllers/src/TokenDetectionController.ts index e4329998679..64572b9a439 100644 --- a/packages/assets-controllers/src/TokenDetectionController.ts +++ b/packages/assets-controllers/src/TokenDetectionController.ts @@ -1,6 +1,7 @@ import type { AccountsControllerGetSelectedAccountAction, - AccountsControllerSelectedAccountChangeEvent, + AccountsControllerGetAccountAction, + AccountsControllerSelectedEvmAccountChangeEvent, } from '@metamask/accounts-controller'; import type { RestrictedControllerMessenger, @@ -105,6 +106,7 @@ export type TokenDetectionControllerActions = export type AllowedActions = | AccountsControllerGetSelectedAccountAction + | AccountsControllerGetAccountAction | NetworkControllerGetNetworkClientByIdAction | NetworkControllerGetNetworkConfigurationByNetworkClientId | NetworkControllerGetStateAction @@ -121,7 +123,7 @@ export type TokenDetectionControllerEvents = TokenDetectionControllerStateChangeEvent; export type AllowedEvents = - | AccountsControllerSelectedAccountChangeEvent + | AccountsControllerSelectedEvmAccountChangeEvent | NetworkControllerNetworkDidChangeEvent | TokenListStateChange | KeyringControllerLockEvent @@ -153,7 +155,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< > { #intervalId?: ReturnType; - #selectedAddress: string; + #selectedAccountId: string; #networkClientId: NetworkClientId; @@ -190,19 +192,16 @@ export class TokenDetectionController extends StaticIntervalPollingController< * @param options.messenger - The controller messaging system. * @param options.disabled - If set to true, all network requests are blocked. * @param options.interval - Polling interval used to fetch new token rates - * @param options.selectedAddress - Vault selected address * @param options.getBalancesInSingleCall - Gets the balances of a list of tokens for the given address. * @param options.trackMetaMetricsEvent - Sets options for MetaMetrics event tracking. */ constructor({ - selectedAddress, interval = DEFAULT_INTERVAL, disabled = true, getBalancesInSingleCall, trackMetaMetricsEvent, messenger, }: { - selectedAddress?: string; interval?: number; disabled?: boolean; getBalancesInSingleCall: AssetsContractController['getBalancesInSingleCall']; @@ -231,10 +230,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< this.#disabled = disabled; this.setIntervalLength(interval); - this.#selectedAddress = - selectedAddress ?? - this.messagingSystem.call('AccountsController:getSelectedAccount') - .address; + this.#selectedAccountId = this.#getSelectedAccount().id; const { chainId, networkClientId } = this.#getCorrectChainIdAndNetworkClientId(); @@ -291,34 +287,32 @@ export class TokenDetectionController extends StaticIntervalPollingController< 'PreferencesController:stateChange', // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/no-misused-promises - async ({ selectedAddress: newSelectedAddress, useTokenDetection }) => { - const isSelectedAddressChanged = - this.#selectedAddress !== newSelectedAddress; + async ({ useTokenDetection }) => { + const selectedAccount = this.#getSelectedAccount(); const isDetectionChangedFromPreferences = this.#isDetectionEnabledFromPreferences !== useTokenDetection; - this.#selectedAddress = newSelectedAddress; this.#isDetectionEnabledFromPreferences = useTokenDetection; - if (isSelectedAddressChanged || isDetectionChangedFromPreferences) { + if (isDetectionChangedFromPreferences) { await this.#restartTokenDetection({ - selectedAddress: this.#selectedAddress, + selectedAddress: selectedAccount.address, }); } }, ); this.messagingSystem.subscribe( - 'AccountsController:selectedAccountChange', + 'AccountsController:selectedEvmAccountChange', // TODO: Either fix this lint violation or explain why it's necessary to ignore. // eslint-disable-next-line @typescript-eslint/no-misused-promises - async ({ address: newSelectedAddress }) => { - const isSelectedAddressChanged = - this.#selectedAddress !== newSelectedAddress; - if (isSelectedAddressChanged) { - this.#selectedAddress = newSelectedAddress; + async (selectedAccount) => { + const isSelectedAccountIdChanged = + this.#selectedAccountId !== selectedAccount.id; + if (isSelectedAccountIdChanged) { + this.#selectedAccountId = selectedAccount.id; await this.#restartTokenDetection({ - selectedAddress: this.#selectedAddress, + selectedAddress: selectedAccount.address, }); } }, @@ -493,7 +487,7 @@ export class TokenDetectionController extends StaticIntervalPollingController< } const addressAgainstWhichToDetect = - selectedAddress ?? this.#selectedAddress; + selectedAddress ?? this.#getSelectedAddress(); const { chainId, networkClientId: selectedNetworkClientId } = this.#getCorrectChainIdAndNetworkClientId(networkClientId); const chainIdAgainstWhichToDetect = chainId; @@ -637,6 +631,19 @@ export class TokenDetectionController extends StaticIntervalPollingController< } }); } + + #getSelectedAccount() { + return this.messagingSystem.call('AccountsController:getSelectedAccount'); + } + + #getSelectedAddress() { + // If the address is not defined (or empty), we fallback to the currently selected account's address + const account = this.messagingSystem.call( + 'AccountsController:getAccount', + this.#selectedAccountId, + ); + return account?.address || ''; + } } export default TokenDetectionController; diff --git a/packages/assets-controllers/src/TokenRatesController.test.ts b/packages/assets-controllers/src/TokenRatesController.test.ts index b42cd28469d..4ba07c9cb55 100644 --- a/packages/assets-controllers/src/TokenRatesController.test.ts +++ b/packages/assets-controllers/src/TokenRatesController.test.ts @@ -1,3 +1,4 @@ +import { createMockInternalAccount } from '@metamask/accounts-controller/src/tests/mocks'; import type { AddApprovalRequest } from '@metamask/approval-controller'; import { ControllerMessenger } from '@metamask/base-controller'; import { @@ -7,16 +8,13 @@ import { toChecksumHexAddress, toHex, } from '@metamask/controller-utils'; +import type { InternalAccount } from '@metamask/keyring-api'; import type { NetworkClientId, NetworkState, } from '@metamask/network-controller'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; import type { NetworkClientConfiguration } from '@metamask/network-controller/src/types'; -import { - getDefaultPreferencesState, - type PreferencesState, -} from '@metamask/preferences-controller'; import type { Hex } from '@metamask/utils'; import { add0x } from '@metamask/utils'; import assert from 'assert'; @@ -45,6 +43,9 @@ import { getDefaultTokensState } from './TokensController'; import type { TokensControllerState } from './TokensController'; const defaultSelectedAddress = '0x0000000000000000000000000000000000000001'; +const defaultSelectedAccount = createMockInternalAccount({ + address: defaultSelectedAddress, +}); const mockTokenAddress = '0x0000000000000000000000000000000000000010'; const defaultSelectedNetworkClientId = 'AAAA-BBBB-CCCC-DDDD'; @@ -68,12 +69,13 @@ function buildTokenRatesControllerMessenger( 'TokensController:getState', 'NetworkController:getNetworkClientById', 'NetworkController:getState', - 'PreferencesController:getState', + 'AccountsController:getAccount', + 'AccountsController:getSelectedAccount', ], allowedEvents: [ - 'PreferencesController:stateChange', 'TokensController:stateChange', 'NetworkController:stateChange', + 'AccountsController:selectedEvmAccountChange', ], }); } @@ -988,6 +990,9 @@ describe('TokenRatesController', () => { it('should update exchange rates when selected address changes', async () => { const alternateSelectedAddress = '0x0000000000000000000000000000000000000002'; + const alternateSelectedAccount = createMockInternalAccount({ + address: alternateSelectedAddress, + }); await withController( { options: { @@ -1014,69 +1019,26 @@ describe('TokenRatesController', () => { }, }, }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, triggerSelectedAccountChange }) => { await controller.start(); const updateExchangeRatesSpy = jest .spyOn(controller, 'updateExchangeRates') .mockResolvedValue(); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: alternateSelectedAddress, - }); + triggerSelectedAccountChange(alternateSelectedAccount); expect(updateExchangeRatesSpy).toHaveBeenCalledTimes(1); }, ); }); - - it('should not update exchange rates when preferences state changes without selected address changing', async () => { - await withController( - { - options: { - interval: 100, - }, - mockTokensControllerState: { - allTokens: { - '0x1': { - [defaultSelectedAddress]: [ - { - address: '0x02', - decimals: 0, - symbol: '', - aggregators: [], - }, - { - address: '0x03', - decimals: 0, - symbol: '', - aggregators: [], - }, - ], - }, - }, - }, - }, - async ({ controller, triggerPreferencesStateChange }) => { - await controller.start(); - const updateExchangeRatesSpy = jest - .spyOn(controller, 'updateExchangeRates') - .mockResolvedValue(); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: defaultSelectedAddress, - openSeaEnabled: false, - }); - - expect(updateExchangeRatesSpy).not.toHaveBeenCalled(); - }, - ); - }); }); describe('when polling is inactive', () => { - it('should not update exchange rates when selected address changes', async () => { + it('does not update exchange rates when selected account changes', async () => { const alternateSelectedAddress = '0x0000000000000000000000000000000000000002'; + const alternateSelectedAccount = createMockInternalAccount({ + address: alternateSelectedAddress, + }); await withController( { options: { @@ -1103,14 +1065,11 @@ describe('TokenRatesController', () => { }, }, }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, triggerSelectedAccountChange }) => { const updateExchangeRatesSpy = jest .spyOn(controller, 'updateExchangeRates') .mockResolvedValue(); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: alternateSelectedAddress, - }); + triggerSelectedAccountChange(alternateSelectedAccount); expect(updateExchangeRatesSpy).not.toHaveBeenCalled(); }, @@ -2299,12 +2258,12 @@ describe('TokenRatesController', () => { */ type WithControllerCallback = ({ controller, - triggerPreferencesStateChange, + triggerSelectedAccountChange, triggerTokensStateChange, triggerNetworkStateChange, }: { controller: TokenRatesController; - triggerPreferencesStateChange: (state: PreferencesState) => void; + triggerSelectedAccountChange: (state: InternalAccount) => void; triggerTokensStateChange: (state: TokensControllerState) => void; triggerNetworkStateChange: (state: NetworkState) => void; }) => Promise | ReturnValue; @@ -2373,13 +2332,16 @@ async function withController( }), ); - const mockPreferencesState = jest.fn(); + const mockGetSelectedAccount = jest.fn(); controllerMessenger.registerActionHandler( - 'PreferencesController:getState', - mockPreferencesState.mockReturnValue({ - ...getDefaultPreferencesState(), - selectedAddress: defaultSelectedAddress, - }), + 'AccountsController:getSelectedAccount', + mockGetSelectedAccount.mockReturnValue(defaultSelectedAccount), + ); + + const mockGetAccount = jest.fn(); + controllerMessenger.registerActionHandler( + 'AccountsController:getAccount', + mockGetAccount.mockReturnValue(defaultSelectedAccount), ); const controller = new TokenRatesController({ @@ -2390,13 +2352,13 @@ async function withController( try { return await fn({ controller, - triggerPreferencesStateChange: (state: PreferencesState) => { + triggerSelectedAccountChange: (account: InternalAccount) => { controllerMessenger.publish( - 'PreferencesController:stateChange', - state, - [], + 'AccountsController:selectedEvmAccountChange', + account, ); }, + triggerTokensStateChange: (state: TokensControllerState) => { controllerMessenger.publish('TokensController:stateChange', state, []); }, diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index c7380171fb0..534ca176bf8 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -1,3 +1,8 @@ +import type { + AccountsControllerGetAccountAction, + AccountsControllerGetSelectedAccountAction, + AccountsControllerSelectedEvmAccountChangeEvent, +} from '@metamask/accounts-controller'; import type { ControllerGetStateAction, ControllerStateChangeEvent, @@ -9,6 +14,7 @@ import { FALL_BACK_VS_CURRENCY, toHex, } from '@metamask/controller-utils'; +import type { InternalAccount } from '@metamask/keyring-api'; import type { NetworkClientId, NetworkControllerGetNetworkClientByIdAction, @@ -16,10 +22,6 @@ import type { NetworkControllerStateChangeEvent, } from '@metamask/network-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; -import type { - PreferencesControllerGetStateAction, - PreferencesControllerStateChangeEvent, -} from '@metamask/preferences-controller'; import { createDeferredPromise, type Hex } from '@metamask/utils'; import { isEqual } from 'lodash'; @@ -103,15 +105,16 @@ export type AllowedActions = | TokensControllerGetStateAction | NetworkControllerGetNetworkClientByIdAction | NetworkControllerGetStateAction - | PreferencesControllerGetStateAction; + | AccountsControllerGetAccountAction + | AccountsControllerGetSelectedAccountAction; /** * The external events available to the {@link TokenRatesController}. */ export type AllowedEvents = - | PreferencesControllerStateChangeEvent | TokensControllerStateChangeEvent - | NetworkControllerStateChangeEvent; + | NetworkControllerStateChangeEvent + | AccountsControllerSelectedEvmAccountChangeEvent; /** * The name of the {@link TokenRatesController}. @@ -235,7 +238,7 @@ export class TokenRatesController extends StaticIntervalPollingController< #inProcessExchangeRateUpdates: Record<`${Hex}:${string}`, Promise> = {}; - #selectedAddress: string; + #selectedAccountId: string; #disabled: boolean; @@ -289,36 +292,17 @@ export class TokenRatesController extends StaticIntervalPollingController< this.#chainId = currentChainId; this.#ticker = currentTicker; - this.#selectedAddress = this.#getSelectedAddress(); + this.#selectedAccountId = this.#getSelectedAccount().id; const { allTokens, allDetectedTokens } = this.#getTokensControllerState(); this.#allTokens = allTokens; this.#allDetectedTokens = allDetectedTokens; - this.#subscribeToPreferencesStateChange(); - this.#subscribeToTokensStateChange(); this.#subscribeToNetworkStateChange(); - } - #subscribeToPreferencesStateChange() { - this.messagingSystem.subscribe( - 'PreferencesController:stateChange', - // TODO: Either fix this lint violation or explain why it's necessary to ignore. - // eslint-disable-next-line @typescript-eslint/no-misused-promises - async (selectedAddress: string) => { - if (this.#selectedAddress !== selectedAddress) { - this.#selectedAddress = selectedAddress; - if (this.#pollState === PollState.Active) { - await this.updateExchangeRates(); - } - } - }, - ({ selectedAddress }) => { - return selectedAddress; - }, - ); + this.#subscribeToAccountChange(); } #subscribeToTokensStateChange() { @@ -372,6 +356,22 @@ export class TokenRatesController extends StaticIntervalPollingController< ); } + #subscribeToAccountChange() { + this.messagingSystem.subscribe( + 'AccountsController:selectedEvmAccountChange', + // TODO: Either fix this lint violation or explain why it's necessary to ignore. + // eslint-disable-next-line @typescript-eslint/no-misused-promises + async (selectedAccount) => { + if (this.#selectedAccountId !== selectedAccount.id) { + this.#selectedAccountId = selectedAccount.id; + if (this.#pollState === PollState.Active) { + await this.updateExchangeRates(); + } + } + }, + ); + } + /** * Get the user's tokens for the given chain. * @@ -379,9 +379,14 @@ export class TokenRatesController extends StaticIntervalPollingController< * @returns The list of tokens addresses for the current chain */ #getTokenAddresses(chainId: Hex): Hex[] { - const tokens = this.#allTokens[chainId]?.[this.#selectedAddress] || []; + const selectedAccount = this.messagingSystem.call( + 'AccountsController:getAccount', + this.#selectedAccountId, + ); + const selectedAddress = selectedAccount?.address ?? ''; + const tokens = this.#allTokens[chainId]?.[selectedAddress] || []; const detectedTokens = - this.#allDetectedTokens[chainId]?.[this.#selectedAddress] || []; + this.#allDetectedTokens[chainId]?.[selectedAddress] || []; return [ ...new Set( @@ -423,12 +428,12 @@ export class TokenRatesController extends StaticIntervalPollingController< this.#pollState = PollState.Inactive; } - #getSelectedAddress(): string { - const { selectedAddress } = this.messagingSystem.call( - 'PreferencesController:getState', + #getSelectedAccount(): InternalAccount { + const selectedAccount = this.messagingSystem.call( + 'AccountsController:getSelectedAccount', ); - return selectedAddress; + return selectedAccount; } #getChainIdAndTicker(): { diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index dfa25e6bc6a..3d30994470b 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -13,18 +13,18 @@ import { convertHexToDecimal, InfuraNetworkType, } from '@metamask/controller-utils'; +import type { InternalAccount } from '@metamask/keyring-api'; import type { NetworkClientConfiguration, NetworkClientId, } from '@metamask/network-controller'; import { defaultState as defaultNetworkState } from '@metamask/network-controller'; -import type { PreferencesState } from '@metamask/preferences-controller'; -import { getDefaultPreferencesState } from '@metamask/preferences-controller'; import nock from 'nock'; import * as sinon from 'sinon'; import { v1 as uuidV1 } from 'uuid'; import { FakeProvider } from '../../../tests/fake-provider'; +import { createMockInternalAccount } from '../../accounts-controller/src/tests/mocks'; import type { ExtractAvailableAction, ExtractAvailableEvent, @@ -39,12 +39,17 @@ import { TOKEN_END_POINT_API } from './token-service'; import type { Token } from './TokenRatesController'; import { TokensController } from './TokensController'; import type { + AllowedActions, + AllowedEvents, TokensControllerMessenger, TokensControllerState, } from './TokensController'; jest.mock('@ethersproject/contracts'); -jest.mock('uuid'); +jest.mock('uuid', () => ({ + ...jest.requireActual('uuid'), + v1: jest.fn(), +})); jest.mock('./Standards/ERC20Standard'); jest.mock('./Standards/NftStandards/ERC1155/ERC1155Standard'); @@ -58,6 +63,10 @@ const uuidV1Mock = jest.mocked(uuidV1); const ERC20StandardMock = jest.mocked(ERC20Standard); const ERC1155StandardMock = jest.mocked(ERC1155Standard); +const defaultMockInternalAccount = createMockInternalAccount({ + address: '0x1', +}); + describe('TokensController', () => { beforeEach(() => { uuidV1Mock.mockReturnValue('9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'); @@ -264,33 +273,36 @@ describe('TokensController', () => { }); it('should add token by selected address', async () => { + const firstAddress = '0x123'; + const firstAccount = createMockInternalAccount({ + address: firstAddress, + }); + const secondAddress = '0x321'; + const secondAccount = createMockInternalAccount({ + address: secondAddress, + }); await withController( - async ({ controller, triggerPreferencesStateChange }) => { + { + mocks: { + getAccount: firstAccount, + getSelectedAccount: firstAccount, + }, + }, + async ({ controller, triggerSelectedAccountChange }) => { ContractMock.mockReturnValue( buildMockEthersERC721Contract({ supportsInterface: false }), ); - const firstAddress = '0x123'; - const secondAddress = '0x321'; - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: firstAddress, - }); + triggerSelectedAccountChange(firstAccount); await controller.addToken({ address: '0x01', symbol: 'bar', decimals: 2, }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: secondAddress, - }); + triggerSelectedAccountChange(secondAccount); expect(controller.state.tokens).toHaveLength(0); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: firstAddress, - }); + triggerSelectedAccountChange(firstAccount); expect(controller.state.tokens[0]).toStrictEqual({ address: '0x01', decimals: 2, @@ -406,26 +418,33 @@ describe('TokensController', () => { }); it('should remove token by selected address', async () => { + const firstAddress = '0x123'; + const firstAccount = createMockInternalAccount({ + address: firstAddress, + }); + const secondAddress = '0x321'; + const secondAccount = createMockInternalAccount({ + address: secondAddress, + }); await withController( - async ({ controller, triggerPreferencesStateChange }) => { + { + mocks: { + getAccount: firstAccount, + getSelectedAccount: firstAccount, + }, + }, + async ({ controller, triggerSelectedAccountChange }) => { ContractMock.mockReturnValue( buildMockEthersERC721Contract({ supportsInterface: false }), ); - const firstAddress = '0x123'; - const secondAddress = '0x321'; - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: firstAddress, - }); + + triggerSelectedAccountChange(firstAccount); await controller.addToken({ address: '0x02', symbol: 'baz', decimals: 2, }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: secondAddress, - }); + triggerSelectedAccountChange(secondAccount); await controller.addToken({ address: '0x01', symbol: 'bar', @@ -435,10 +454,7 @@ describe('TokensController', () => { controller.ignoreTokens(['0x01']); expect(controller.state.tokens).toHaveLength(0); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: firstAddress, - }); + triggerSelectedAccountChange(firstAccount); expect(controller.state.tokens[0]).toStrictEqual({ address: '0x02', decimals: 2, @@ -518,17 +534,19 @@ describe('TokensController', () => { }); it('should remove a token from the ignoredTokens/allIgnoredTokens lists if re-added as part of a bulk addTokens add', async () => { + const selectedAddress = '0x0001'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); await withController( - async ({ - controller, - triggerPreferencesStateChange, - changeNetwork, - }) => { - const selectedAddress = '0x0001'; - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress, - }); + { + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, + }, + async ({ controller, triggerSelectedAccountChange, changeNetwork }) => { + triggerSelectedAccountChange(selectedAccount); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await controller.addToken({ address: '0x01', @@ -565,17 +583,19 @@ describe('TokensController', () => { }); it('should be able to clear the ignoredTokens list', async () => { + const selectedAddress = '0x0001'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); await withController( - async ({ - controller, - triggerPreferencesStateChange, - changeNetwork, - }) => { - const selectedAddress = '0x0001'; - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress, - }); + { + mocks: { + getSelectedAccount: selectedAccount, + getAccount: selectedAccount, + }, + }, + async ({ controller, triggerSelectedAccountChange, changeNetwork }) => { + triggerSelectedAccountChange(selectedAccount); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await controller.addToken({ address: '0x01', @@ -602,18 +622,23 @@ describe('TokensController', () => { }); it('should ignore tokens by [chainID][accountAddress]', async () => { + const selectedAddress1 = '0x0001'; + const selectedAccount1 = createMockInternalAccount({ + address: selectedAddress1, + }); + const selectedAddress2 = '0x0002'; + const selectedAccount2 = createMockInternalAccount({ + address: selectedAddress2, + }); await withController( - async ({ - controller, - triggerPreferencesStateChange, - changeNetwork, - }) => { - const selectedAddress1 = '0x0001'; - const selectedAddress2 = '0x0002'; - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: selectedAddress1, - }); + { + mocks: { + getSelectedAccount: selectedAccount1, + getAccount: selectedAccount1, + }, + }, + async ({ controller, triggerSelectedAccountChange, changeNetwork }) => { + triggerSelectedAccountChange(selectedAccount1); changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); await controller.addToken({ address: '0x01', @@ -637,10 +662,7 @@ describe('TokensController', () => { controller.ignoreTokens(['0x02']); expect(controller.state.ignoredTokens).toStrictEqual(['0x02']); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: selectedAddress2, - }); + triggerSelectedAccountChange(selectedAccount2); expect(controller.state.ignoredTokens).toHaveLength(0); await controller.addToken({ @@ -888,7 +910,9 @@ describe('TokensController', () => { symbol: 'LINK', decimals: 18, }); - changeNetwork({ selectedNetworkClientId: InfuraNetworkType.goerli }); + changeNetwork({ + selectedNetworkClientId: InfuraNetworkType.goerli, + }); await expect(addTokenPromise).rejects.toThrow( 'TokensController Error: Switched networks while adding token', @@ -968,12 +992,17 @@ describe('TokensController', () => { }); it('should add tokens to the correct chainId/selectedAddress on which they were detected even if its not the currently configured chainId/selectedAddress', async () => { + const CONFIGURED_ADDRESS = '0xConfiguredAddress'; + const configuredAccount = createMockInternalAccount({ + address: CONFIGURED_ADDRESS, + }); await withController( - async ({ - controller, - changeNetwork, - triggerPreferencesStateChange, - }) => { + { + mocks: { + getAccount: configuredAccount, + }, + }, + async ({ controller, changeNetwork, triggerSelectedAccountChange }) => { ContractMock.mockReturnValue( buildMockEthersERC721Contract({ supportsInterface: false }), ); @@ -981,14 +1010,11 @@ describe('TokensController', () => { // The currently configured chain + address const CONFIGURED_CHAIN = ChainId.sepolia; const CONFIGURED_NETWORK_CLIENT_ID = InfuraNetworkType.sepolia; - const CONFIGURED_ADDRESS = '0xConfiguredAddress'; + changeNetwork({ selectedNetworkClientId: CONFIGURED_NETWORK_CLIENT_ID, }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: CONFIGURED_ADDRESS, - }); + triggerSelectedAccountChange(configuredAccount); // A different chain + address const OTHER_CHAIN = '0xOtherChainId'; @@ -1571,7 +1597,6 @@ describe('TokensController', () => { buildMockEthersERC721Contract({ supportsInterface: false }), ); uuidV1Mock.mockReturnValue(requestId); - await controller.watchAsset({ asset, type: 'ERC20' }); expect(controller.state.tokens).toHaveLength(1); @@ -1722,7 +1747,6 @@ describe('TokensController', () => { buildMockEthersERC721Contract({ supportsInterface: false }), ); uuidV1Mock.mockReturnValue(requestId); - await expect( controller.watchAsset({ asset, type: 'ERC20' }), ).rejects.toThrow(errorMessage); @@ -1844,15 +1868,23 @@ describe('TokensController', () => { describe('when PreferencesController:stateChange is published', () => { it('should update tokens list when set address changes', async () => { + const selectedAccount = createMockInternalAccount({ address: '0x1' }); + const selectedAccount2 = createMockInternalAccount({ + address: '0x2', + }); await withController( - async ({ controller, triggerPreferencesStateChange }) => { + { + mocks: { + getAccount: selectedAccount, + getSelectedAccount: selectedAccount, + }, + }, + async ({ controller, triggerSelectedAccountChange }) => { ContractMock.mockReturnValue( buildMockEthersERC721Contract({ supportsInterface: false }), ); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: '0x1', - }); + + triggerSelectedAccountChange(selectedAccount); await controller.addToken({ address: '0x01', symbol: 'A', @@ -1863,10 +1895,7 @@ describe('TokensController', () => { symbol: 'B', decimals: 5, }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: '0x2', - }); + triggerSelectedAccountChange(selectedAccount2); expect(controller.state.tokens).toStrictEqual([]); await controller.addToken({ @@ -1874,10 +1903,7 @@ describe('TokensController', () => { symbol: 'C', decimals: 6, }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: '0x1', - }); + triggerSelectedAccountChange(selectedAccount); expect(controller.state.tokens).toStrictEqual([ { address: '0x01', @@ -1901,10 +1927,7 @@ describe('TokensController', () => { }, ]); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress: '0x2', - }); + triggerSelectedAccountChange(selectedAccount2); expect(controller.state.tokens).toStrictEqual([ { address: '0x03', @@ -2011,6 +2034,9 @@ describe('TokensController', () => { describe('Clearing nested lists', () => { it('should clear nest allTokens under chain ID and selected address when an added token is ignored', async () => { const selectedAddress = '0x1'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); const tokenAddress = '0x01'; const dummyTokens = [ { @@ -2026,7 +2052,9 @@ describe('TokensController', () => { { options: { chainId: ChainId.mainnet, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, }, }, async ({ controller }) => { @@ -2042,6 +2070,9 @@ describe('TokensController', () => { it('should clear nest allIgnoredTokens under chain ID and selected address when an ignored token is re-added', async () => { const selectedAddress = '0x1'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); const tokenAddress = '0x01'; const dummyTokens = [ { @@ -2057,7 +2088,9 @@ describe('TokensController', () => { { options: { chainId: ChainId.mainnet, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, }, }, async ({ controller }) => { @@ -2074,6 +2107,9 @@ describe('TokensController', () => { it('should clear nest allDetectedTokens under chain ID and selected address when an detected token is added to tokens list', async () => { const selectedAddress = '0x1'; + const selectedAccount = createMockInternalAccount({ + address: selectedAddress, + }); const tokenAddress = '0x01'; const dummyTokens = [ { @@ -2089,7 +2125,9 @@ describe('TokensController', () => { { options: { chainId: ChainId.mainnet, - selectedAddress, + }, + mocks: { + getSelectedAccount: selectedAccount, }, }, async ({ controller }) => { @@ -2159,6 +2197,117 @@ describe('TokensController', () => { }); }); }); + + describe('when selectedAccountId is not set or account not found', () => { + describe('detectTokens', () => { + it('updates the token states to empty arrays if the selectedAccountId account is undefined', async () => { + await withController(async ({ controller, changeNetwork }) => { + ContractMock.mockReturnValue( + buildMockEthersERC721Contract({ supportsInterface: false }), + ); + changeNetwork({ selectedNetworkClientId: InfuraNetworkType.sepolia }); + + expect(controller.state.tokens).toStrictEqual([]); + expect(controller.state.ignoredTokens).toStrictEqual([]); + expect(controller.state.detectedTokens).toStrictEqual([]); + }); + }); + }); + + describe('addToken', () => { + it('handles undefined selected account', async () => { + await withController(async ({ controller, getAccountHandler }) => { + getAccountHandler.mockReturnValue(undefined); + const contractAddresses = Object.keys(contractMaps); + const erc721ContractAddresses = contractAddresses.filter( + (contractAddress) => contractMaps[contractAddress].erc721 === true, + ); + const address = erc721ContractAddresses[0]; + const { symbol, decimals } = contractMaps[address]; + + await controller.addToken({ address, symbol, decimals }); + + expect(controller.state.tokens).toStrictEqual([ + { + address, + aggregators: [], + decimals, + image: + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x9c8ff314c9bc7f6e59a9d9225fb22946427edc03.png', + isERC721: true, + name: undefined, + symbol, + }, + ]); + }); + }); + }); + + describe('addDetectedTokens', () => { + it('handles an undefined selected account', async () => { + await withController(async ({ controller, getAccountHandler }) => { + getAccountHandler.mockReturnValue(undefined); + const mockToken = { + address: '0x01', + symbol: 'barA', + decimals: 2, + aggregators: [], + }; + await controller.addDetectedTokens([mockToken]); + expect(controller.state.detectedTokens[0]).toStrictEqual({ + ...mockToken, + image: undefined, + isERC721: undefined, + name: undefined, + }); + }); + }); + }); + + describe('watchAsset', () => { + it('handles undefined selected account', async () => { + await withController( + async ({ controller, approvalController, getAccountHandler }) => { + const requestId = '12345'; + const addAndShowApprovalRequestSpy = jest + .spyOn(approvalController, 'addAndShowApprovalRequest') + .mockResolvedValue(undefined); + const asset = buildToken(); + ContractMock.mockReturnValue( + buildMockEthersERC721Contract({ supportsInterface: false }), + ); + uuidV1Mock.mockReturnValue(requestId); + getAccountHandler.mockReturnValue(undefined); + await controller.watchAsset({ asset, type: 'ERC20' }); + + expect(controller.state.tokens).toHaveLength(1); + expect(controller.state.tokens).toStrictEqual([ + { + address: '0x000000000000000000000000000000000000dEaD', + aggregators: [], + decimals: 12, + image: 'image', + isERC721: false, + name: undefined, + symbol: 'TOKEN', + }, + ]); + expect(addAndShowApprovalRequestSpy).toHaveBeenCalledTimes(1); + expect(addAndShowApprovalRequestSpy).toHaveBeenCalledWith({ + id: requestId, + origin: ORIGIN_METAMASK, + type: ApprovalType.WatchAsset, + requestData: { + id: requestId, + interactingAddress: '', // this is the default value if account is not found + asset, + }, + }); + }, + ); + }); + }); + }); }); type WithControllerCallback = ({ @@ -2166,7 +2315,7 @@ type WithControllerCallback = ({ changeNetwork, messenger, approvalController, - triggerPreferencesStateChange, + triggerSelectedAccountChange, }: { controller: TokensController; changeNetwork: (networkControllerState: { @@ -2174,9 +2323,16 @@ type WithControllerCallback = ({ }) => void; messenger: UnrestrictedMessenger; approvalController: ApprovalController; - triggerPreferencesStateChange: (state: PreferencesState) => void; + triggerSelectedAccountChange: (internalAccount: InternalAccount) => void; + getAccountHandler: jest.Mock; + getSelectedAccountHandler: jest.Mock; }) => Promise | ReturnValue; +type WithControllerMockArgs = { + getAccount?: InternalAccount; + getSelectedAccount?: InternalAccount; +}; + type WithControllerArgs = | [WithControllerCallback] | [ @@ -2186,6 +2342,7 @@ type WithControllerArgs = NetworkClientId, NetworkClientConfiguration >; + mocks?: WithControllerMockArgs; }, WithControllerCallback, ]; @@ -2200,17 +2357,22 @@ type WithControllerArgs = * @param args.mockNetworkClientConfigurationsByNetworkClientId - Used to construct * mock versions of network clients and ultimately mock the * `NetworkController:getNetworkClientById` action. + * @param args.mocks - Move values for actions to be mocked. * @returns A collection of test controllers and mocks. */ async function withController( ...args: WithControllerArgs ): Promise { const [ - { options = {}, mockNetworkClientConfigurationsByNetworkClientId = {} }, + { + options = {}, + mockNetworkClientConfigurationsByNetworkClientId = {}, + mocks = {} as WithControllerMockArgs, + }, fn, ] = args.length === 2 ? args : [{}, args[0]]; - const messenger: UnrestrictedMessenger = new ControllerMessenger(); + const messenger = new ControllerMessenger(); const approvalControllerMessenger = messenger.getRestricted({ name: 'ApprovalController', @@ -2228,16 +2390,34 @@ async function withController( allowedActions: [ 'ApprovalController:addRequest', 'NetworkController:getNetworkClientById', + 'AccountsController:getAccount', + 'AccountsController:getSelectedAccount', ], allowedEvents: [ 'NetworkController:networkDidChange', - 'PreferencesController:stateChange', + 'AccountsController:selectedEvmAccountChange', 'TokenListController:stateChange', ], }); + + const getAccountHandler = jest.fn(); + messenger.registerActionHandler( + 'AccountsController:getAccount', + getAccountHandler.mockReturnValue( + mocks?.getAccount ?? defaultMockInternalAccount, + ), + ); + + const getSelectedAccountHandler = jest.fn(); + messenger.registerActionHandler( + 'AccountsController:getSelectedAccount', + getSelectedAccountHandler.mockReturnValue( + mocks?.getSelectedAccount ?? defaultMockInternalAccount, + ), + ); + const controller = new TokensController({ chainId: ChainId.mainnet, - selectedAddress: '0x1', // The tests assume that this is set, but they shouldn't make that // assumption. But we have to do this due to a bug in TokensController // where the provider can possibly be `undefined` if `networkClientId` is @@ -2247,8 +2427,12 @@ async function withController( ...options, }); - const triggerPreferencesStateChange = (state: PreferencesState) => { - messenger.publish('PreferencesController:stateChange', state, []); + const triggerSelectedAccountChange = (internalAccount: InternalAccount) => { + getAccountHandler.mockReturnValue(internalAccount); + messenger.publish( + 'AccountsController:selectedEvmAccountChange', + internalAccount, + ); }; const changeNetwork = ({ @@ -2275,7 +2459,9 @@ async function withController( changeNetwork, messenger, approvalController, - triggerPreferencesStateChange, + triggerSelectedAccountChange, + getAccountHandler, + getSelectedAccountHandler, }); } diff --git a/packages/assets-controllers/src/TokensController.ts b/packages/assets-controllers/src/TokensController.ts index b90df2f7816..9345f8b3342 100644 --- a/packages/assets-controllers/src/TokensController.ts +++ b/packages/assets-controllers/src/TokensController.ts @@ -1,5 +1,10 @@ import { Contract } from '@ethersproject/contracts'; import { Web3Provider } from '@ethersproject/providers'; +import type { + AccountsControllerGetAccountAction, + AccountsControllerGetSelectedAccountAction, + AccountsControllerSelectedEvmAccountChangeEvent, +} from '@metamask/accounts-controller'; import type { AddApprovalRequest } from '@metamask/approval-controller'; import type { RestrictedControllerMessenger, @@ -19,6 +24,7 @@ import { isValidHexAddress, safelyExecute, } from '@metamask/controller-utils'; +import type { InternalAccount } from '@metamask/keyring-api'; import { abiERC721 } from '@metamask/metamask-eth-abis'; import type { NetworkClientId, @@ -27,10 +33,6 @@ import type { NetworkState, Provider, } from '@metamask/network-controller'; -import type { - PreferencesControllerStateChangeEvent, - PreferencesState, -} from '@metamask/preferences-controller'; import { rpcErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; import { Mutex } from 'async-mutex'; @@ -136,7 +138,9 @@ export type TokensControllerAddDetectedTokensAction = { */ export type AllowedActions = | AddApprovalRequest - | NetworkControllerGetNetworkClientByIdAction; + | NetworkControllerGetNetworkClientByIdAction + | AccountsControllerGetAccountAction + | AccountsControllerGetSelectedAccountAction; export type TokensControllerStateChangeEvent = ControllerStateChangeEvent< typeof controllerName, @@ -147,8 +151,8 @@ export type TokensControllerEvents = TokensControllerStateChangeEvent; export type AllowedEvents = | NetworkControllerNetworkDidChangeEvent - | PreferencesControllerStateChangeEvent - | TokenListStateChange; + | TokenListStateChange + | AccountsControllerSelectedEvmAccountChangeEvent; /** * The messenger of the {@link TokensController}. @@ -184,7 +188,7 @@ export class TokensController extends BaseController< #chainId: Hex; - #selectedAddress: string; + #selectedAccountId: string; #provider: Provider | undefined; @@ -194,20 +198,17 @@ export class TokensController extends BaseController< * Tokens controller options * @param options - Constructor options. * @param options.chainId - The chain ID of the current network. - * @param options.selectedAddress - Vault selected address * @param options.provider - Network provider. * @param options.state - Initial state to set on this controller. * @param options.messenger - The controller messenger. */ constructor({ chainId: initialChainId, - selectedAddress, provider, state, messenger, }: { chainId: Hex; - selectedAddress: string; provider: Provider | undefined; state?: Partial; messenger: TokensControllerMessenger; @@ -226,7 +227,7 @@ export class TokensController extends BaseController< this.#provider = provider; - this.#selectedAddress = selectedAddress; + this.#selectedAccountId = this.#getSelectedAccount().id; this.#abortController = new AbortController(); @@ -236,8 +237,8 @@ export class TokensController extends BaseController< ); this.messagingSystem.subscribe( - 'PreferencesController:stateChange', - this.#onPreferenceControllerStateChange.bind(this), + 'AccountsController:selectedEvmAccountChange', + this.#onSelectedAccountChange.bind(this), ); this.messagingSystem.subscribe( @@ -273,29 +274,28 @@ export class TokensController extends BaseController< this.#abortController.abort(); this.#abortController = new AbortController(); this.#chainId = chainId; + const selectedAddress = this.#getSelectedAddress(); this.update((state) => { - state.tokens = allTokens[chainId]?.[this.#selectedAddress] || []; - state.ignoredTokens = - allIgnoredTokens[chainId]?.[this.#selectedAddress] || []; + state.tokens = allTokens[chainId]?.[selectedAddress] || []; + state.ignoredTokens = allIgnoredTokens[chainId]?.[selectedAddress] || []; state.detectedTokens = - allDetectedTokens[chainId]?.[this.#selectedAddress] || []; + allDetectedTokens[chainId]?.[selectedAddress] || []; }); } /** - * Handles the state change of the preference controller. - * @param preferencesState - The new state of the preference controller. - * @param preferencesState.selectedAddress - The current selected address of the preference controller. + * Handles the selected account change in the accounts controller. + * @param selectedAccount - The new selected account */ - #onPreferenceControllerStateChange({ selectedAddress }: PreferencesState) { + #onSelectedAccountChange(selectedAccount: InternalAccount) { const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; - this.#selectedAddress = selectedAddress; + this.#selectedAccountId = selectedAccount.id; this.update((state) => { - state.tokens = allTokens[this.#chainId]?.[selectedAddress] ?? []; + state.tokens = allTokens[this.#chainId]?.[selectedAccount.address] ?? []; state.ignoredTokens = - allIgnoredTokens[this.#chainId]?.[selectedAddress] ?? []; + allIgnoredTokens[this.#chainId]?.[selectedAccount.address] ?? []; state.detectedTokens = - allDetectedTokens[this.#chainId]?.[selectedAddress] ?? []; + allDetectedTokens[this.#chainId]?.[selectedAccount.address] ?? []; }); } @@ -357,7 +357,6 @@ export class TokensController extends BaseController< networkClientId?: NetworkClientId; }): Promise { const chainId = this.#chainId; - const selectedAddress = this.#selectedAddress; const releaseLock = await this.#mutex.acquire(); const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; let currentChainId = chainId; @@ -368,9 +367,10 @@ export class TokensController extends BaseController< ).configuration.chainId; } - const accountAddress = interactingAddress || selectedAddress; - const isInteractingWithWalletAccount = accountAddress === selectedAddress; - + const accountAddress = + this.#getAddressOrSelectedAddress(interactingAddress); + const isInteractingWithWalletAccount = + this.#isInteractingWithWallet(accountAddress); try { address = toChecksumHexAddress(address); const tokens = allTokens[currentChainId]?.[accountAddress] || []; @@ -578,10 +578,10 @@ export class TokensController extends BaseController< ) { const releaseLock = await this.#mutex.acquire(); - // Get existing tokens for the chain + account const chainId = detectionDetails?.chainId ?? this.#chainId; + // Previously selectedAddress could be an empty string. This is to preserve the behaviour const accountAddress = - detectionDetails?.selectedAddress ?? this.#selectedAddress; + detectionDetails?.selectedAddress ?? this.#getSelectedAddress(); const { allTokens, allDetectedTokens, allIgnoredTokens } = this.state; let newTokens = [...(allTokens?.[chainId]?.[accountAddress] ?? [])]; @@ -648,9 +648,11 @@ export class TokensController extends BaseController< // We may be detecting tokens on a different chain/account pair than are currently configured. // Re-point `tokens` and `detectedTokens` to keep them referencing the current chain/account. - newTokens = newAllTokens?.[this.#chainId]?.[this.#selectedAddress] || []; + const selectedAddress = this.#getSelectedAddress(); + + newTokens = newAllTokens?.[this.#chainId]?.[selectedAddress] || []; newDetectedTokens = - newAllDetectedTokens?.[this.#chainId]?.[this.#selectedAddress] || []; + newAllDetectedTokens?.[this.#chainId]?.[selectedAddress] || []; this.update((state) => { state.tokens = newTokens; @@ -806,6 +808,9 @@ export class TokensController extends BaseController< throw rpcErrors.invalidParams(`Invalid address "${asset.address}"`); } + const selectedAddress = + this.#getAddressOrSelectedAddress(interactingAddress); + // Validate contract if (await this.#detectIsERC721(asset.address, networkClientId)) { @@ -906,7 +911,7 @@ export class TokensController extends BaseController< id: this.#generateRandomId(), time: Date.now(), type, - interactingAddress: interactingAddress || this.#selectedAddress, + interactingAddress: selectedAddress, }; await this.#requestApproval(suggestedAssetMeta); @@ -951,7 +956,9 @@ export class TokensController extends BaseController< } = params; const { allTokens, allIgnoredTokens, allDetectedTokens } = this.state; - const userAddressToAddTokens = interactingAddress ?? this.#selectedAddress; + const userAddressToAddTokens = + this.#getAddressOrSelectedAddress(interactingAddress); + const chainIdToAddTokens = interactingChainId ?? this.#chainId; let newAllTokens = allTokens; @@ -1013,6 +1020,20 @@ export class TokensController extends BaseController< return { newAllTokens, newAllIgnoredTokens, newAllDetectedTokens }; } + #getAddressOrSelectedAddress(address: string | undefined): string { + if (address) { + return address; + } + + return this.#getSelectedAddress(); + } + + #isInteractingWithWallet(address: string | undefined) { + const selectedAddress = this.#getSelectedAddress(); + + return selectedAddress === address; + } + /** * Removes all tokens from the ignored list. */ @@ -1044,6 +1065,19 @@ export class TokensController extends BaseController< true, ); } + + #getSelectedAccount() { + return this.messagingSystem.call('AccountsController:getSelectedAccount'); + } + + #getSelectedAddress() { + // If the address is not defined (or empty), we fallback to the currently selected account's address + const account = this.messagingSystem.call( + 'AccountsController:getAccount', + this.#selectedAccountId, + ); + return account?.address || ''; + } } export default TokensController; From c9a94192a23e89e122b819202206dee4ae1510c9 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Fri, 21 Jun 2024 16:30:42 -0600 Subject: [PATCH 11/11] Remove infinite loop detection from logging-controller --- .../src/LoggingController.test.ts | 20 ------------------- .../src/LoggingController.ts | 5 ----- 2 files changed, 25 deletions(-) diff --git a/packages/logging-controller/src/LoggingController.test.ts b/packages/logging-controller/src/LoggingController.test.ts index b76957908e3..3f092f56b4d 100644 --- a/packages/logging-controller/src/LoggingController.test.ts +++ b/packages/logging-controller/src/LoggingController.test.ts @@ -160,26 +160,6 @@ describe('LoggingController', () => { expect(uuidV1Spy).toHaveBeenCalledTimes(3); }); - it('action: LoggingController:add does not crash, but throws, if a unique id is never generated', async () => { - const unrestricted = getUnrestrictedMessenger(); - const messenger = getRestrictedMessenger(unrestricted); - jest.spyOn(uuid, 'v1').mockReturnValue('same UUID every time'); - - new LoggingController({ messenger }); - - unrestricted.call('LoggingController:add', { - type: LogType.GenericLog, - data: `Generic log`, - }); - - expect(() => { - unrestricted.call('LoggingController:add', { - type: LogType.GenericLog, - data: `Generic log 2`, - }); - }).toThrow('Endless loop'); - }); - it('internal method: clear', async () => { const unrestricted = getUnrestrictedMessenger(); const messenger = getRestrictedMessenger(unrestricted); diff --git a/packages/logging-controller/src/LoggingController.ts b/packages/logging-controller/src/LoggingController.ts index 202906d43f2..150711afd4b 100644 --- a/packages/logging-controller/src/LoggingController.ts +++ b/packages/logging-controller/src/LoggingController.ts @@ -107,13 +107,8 @@ export class LoggingController extends BaseController< */ #generateId(): string { let id = random(); - let i = 0; while (id in this.state.logs) { - if (i > 1000) { - throw new Error('Endless loop'); - } id = random(); - i += 1; } return id; }