diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 0950da58cae..796a961d095 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -44,7 +44,7 @@ app/util/walletconnect.js @MetaMask/sdk-devs @MetaMask/mobile-platform # Accounts Team app/core/Encryptor/ @MetaMask/accounts-engineers -app/core/Engine/controller/AccountsController @MetaMask/accounts-engineers +app/core/Engine/controllers/accounts @MetaMask/accounts-engineers # Swaps Team app/components/UI/Swaps @MetaMask/swaps-engineers @MetaMask/mobile-platform diff --git a/app/core/Engine/Engine.ts b/app/core/Engine/Engine.ts index eeb4ddc899c..a23bed16d3e 100644 --- a/app/core/Engine/Engine.ts +++ b/app/core/Engine/Engine.ts @@ -152,8 +152,9 @@ import { AccountsControllerSelectedAccountChangeEvent, AccountsControllerAccountAddedEvent, AccountsControllerAccountRenamedEvent, -} from './controllers/AccountsController/constants'; -import { createAccountsController } from './controllers/AccountsController/utils'; +} from './controllers/accounts/constants'; +import { AccountsControllerMessenger } from '@metamask/accounts-controller'; +import { createAccountsController } from './controllers/accounts/utils'; import { captureException } from '@sentry/react-native'; import { lowerCase } from 'lodash'; import { @@ -339,8 +340,22 @@ export class Engine { }); // Create AccountsController + const accountsControllerMessenger: AccountsControllerMessenger = + this.controllerMessenger.getRestricted({ + name: 'AccountsController', + allowedEvents: [ + 'SnapController:stateChange', + 'KeyringController:accountRemoved', + 'KeyringController:stateChange', + ], + allowedActions: [ + 'KeyringController:getAccounts', + 'KeyringController:getKeyringsByType', + 'KeyringController:getKeyringForAccount', + ], + }); const accountsController = createAccountsController({ - messenger: this.controllerMessenger, + messenger: accountsControllerMessenger, initialState: initialState.AccountsController, }); diff --git a/app/core/Engine/controllers/accounts/README.md b/app/core/Engine/controllers/accounts/README.md new file mode 100644 index 00000000000..24ea735bd59 --- /dev/null +++ b/app/core/Engine/controllers/accounts/README.md @@ -0,0 +1,3 @@ +# Controllers owned by the Accounts team + +This folder contains controllers that are owned by the Accounts team. diff --git a/app/core/Engine/controllers/AccountsController/constants.ts b/app/core/Engine/controllers/accounts/constants.ts similarity index 100% rename from app/core/Engine/controllers/AccountsController/constants.ts rename to app/core/Engine/controllers/accounts/constants.ts diff --git a/app/core/Engine/controllers/AccountsController/utils.test.ts b/app/core/Engine/controllers/accounts/utils.test.ts similarity index 62% rename from app/core/Engine/controllers/AccountsController/utils.test.ts rename to app/core/Engine/controllers/accounts/utils.test.ts index 25f95dd2e14..67b640cb448 100644 --- a/app/core/Engine/controllers/AccountsController/utils.test.ts +++ b/app/core/Engine/controllers/accounts/utils.test.ts @@ -1,10 +1,12 @@ -import { AccountsControllerState } from '@metamask/accounts-controller'; +import { + AccountsControllerMessenger, + AccountsControllerState, +} from '@metamask/accounts-controller'; import { ExtendedControllerMessenger } from '../../../ExtendedControllerMessenger'; import { createAccountsController, defaultAccountsControllerState, } from './utils'; -import { ControllerMessenger } from '../../'; import { withScope } from '@sentry/react-native'; import { AGREED, METRICS_OPT_IN } from '../../../../constants/storage'; import StorageWrapper from '../../../../store/storage-wrapper'; @@ -16,7 +18,24 @@ const mockedWithScope = jest.mocked(withScope); describe('accountControllersUtils', () => { describe('createAccountsController', () => { + let accountsControllerMessenger: AccountsControllerMessenger; + beforeEach(() => { + const globalMessenger = new ExtendedControllerMessenger(); + accountsControllerMessenger = globalMessenger.getRestricted({ + name: 'AccountsController', + allowedEvents: [ + 'SnapController:stateChange', + 'KeyringController:accountRemoved', + 'KeyringController:stateChange', + ], + allowedActions: [ + 'KeyringController:getAccounts', + 'KeyringController:getKeyringsByType', + 'KeyringController:getKeyringForAccount', + ], + }); + // Mock required for Logger StorageWrapper.getItem = jest.fn((key: string) => { switch (key) { case METRICS_OPT_IN: @@ -32,14 +51,12 @@ describe('accountControllersUtils', () => { }); it('AccountsController state should be default state when no initial state is passed in', () => { - const controllerMessenger = new ExtendedControllerMessenger(); const accountsController = createAccountsController({ - messenger: controllerMessenger, + messenger: accountsControllerMessenger, }); expect(accountsController.state).toEqual(defaultAccountsControllerState); }); it('AccountsController state should be initial state when initial state is passed in', () => { - const controllerMessenger = new ExtendedControllerMessenger(); const initialAccountsControllerState: AccountsControllerState = { internalAccounts: { accounts: {}, @@ -47,27 +64,28 @@ describe('accountControllersUtils', () => { }, }; const accountsController = createAccountsController({ - messenger: controllerMessenger, + messenger: accountsControllerMessenger, initialState: initialAccountsControllerState, }); expect(accountsController.state).toEqual(initialAccountsControllerState); }); it('AccountsController name should be AccountsController', () => { - const controllerMessenger = new ExtendedControllerMessenger(); const accountsControllerName = 'AccountsController'; const accountsController = createAccountsController({ - messenger: controllerMessenger, + messenger: accountsControllerMessenger, }); expect(accountsController.name).toEqual(accountsControllerName); }); - it('should catch error when controller fails to initialize', async () => { - const controllerMessenger = - 'controllerMessenger' as unknown as ControllerMessenger; - const accountsController = await createAccountsController({ - messenger: controllerMessenger, - }); + it('should detect and log an error when controller fails to initialize', async () => { + const brokenAccountsControllerMessenger = + 'controllerMessenger' as unknown as AccountsControllerMessenger; + await expect(() => + createAccountsController({ + messenger: brokenAccountsControllerMessenger, + }), + ).toThrow(); + expect(mockedWithScope).toHaveBeenCalledTimes(1); - expect(accountsController).toEqual({}); }); }); }); diff --git a/app/core/Engine/controllers/AccountsController/utils.ts b/app/core/Engine/controllers/accounts/utils.ts similarity index 57% rename from app/core/Engine/controllers/AccountsController/utils.ts rename to app/core/Engine/controllers/accounts/utils.ts index 44325a3bd65..881e86e43ef 100644 --- a/app/core/Engine/controllers/AccountsController/utils.ts +++ b/app/core/Engine/controllers/accounts/utils.ts @@ -3,7 +3,6 @@ import { AccountsControllerMessenger, AccountsControllerState, } from '@metamask/accounts-controller'; -import { ControllerMessenger } from '../../types'; import Logger from '../../../../util/Logger'; // Default AccountsControllerState @@ -25,36 +24,20 @@ export const createAccountsController = ({ messenger, initialState, }: { - messenger: ControllerMessenger; + messenger: AccountsControllerMessenger; initialState?: AccountsControllerState; }): AccountsController => { - let accountsController = {} as AccountsController; - try { - const accountsControllerMessenger: AccountsControllerMessenger = - messenger.getRestricted({ - name: 'AccountsController', - allowedEvents: [ - 'SnapController:stateChange', - 'KeyringController:accountRemoved', - 'KeyringController:stateChange', - ], - allowedActions: [ - 'KeyringController:getAccounts', - 'KeyringController:getKeyringsByType', - 'KeyringController:getKeyringForAccount', - ], - }); - - accountsController = new AccountsController({ - messenger: accountsControllerMessenger, + const accountsController = new AccountsController({ + messenger, state: initialState ?? defaultAccountsControllerState, }); + return accountsController; } catch (error) { // Report error while initializing AccountsController - // TODO: Direct to vault recovery to reset controller states Logger.error(error as Error, 'Failed to initialize AccountsController'); - } - return accountsController; + // TODO: Direct to vault recovery to reset controller states + throw error; + } };