diff --git a/packages/accounts-controller/CHANGELOG.md b/packages/accounts-controller/CHANGELOG.md index 5ebd6da597a..9810571fe8f 100644 --- a/packages/accounts-controller/CHANGELOG.md +++ b/packages/accounts-controller/CHANGELOG.md @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- Add `accountIdByAddress` mapping to state ([#7893](https://github.com/MetaMask/core/pull/7893)) + - This state was added to improve lookup times for an account by address from O(n) to O(1). + - `getAccountByAddress` also leverages this new map, thus, should be slightly faster too. - Add logging capabilities ([#8118](https://github.com/MetaMask/core/pull/8118/)) - Expose missing public `AccountsController` methods through its messenger ([#7976](https://github.com/MetaMask/core/pull/7976/)) - The following actions are now available: diff --git a/packages/accounts-controller/src/AccountsController.test.ts b/packages/accounts-controller/src/AccountsController.test.ts index 741afbd8545..6fe0bd849b8 100644 --- a/packages/accounts-controller/src/AccountsController.test.ts +++ b/packages/accounts-controller/src/AccountsController.test.ts @@ -1,5 +1,8 @@ import { deriveStateFromMetadata } from '@metamask/base-controller'; -import { InfuraNetworkType } from '@metamask/controller-utils'; +import { + InfuraNetworkType, + toChecksumHexAddress, +} from '@metamask/controller-utils'; import type { AccountAssetListUpdatedEventPayload, AccountBalancesUpdatedEventPayload, @@ -67,6 +70,7 @@ const defaultState: AccountsControllerState = { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }; const mockGetKeyringByType = jest.fn(); @@ -143,6 +147,21 @@ const mockAccount4: InternalAccount = { }, }; +const mockAccount5: InternalAccount = { + id: 'mock-id5', + address: '0x4f3e8a2c1d7b6e9f0a5c8d2b1e7f3a6c9d0e4b5a', + options: {}, + methods: [...ETH_EOA_METHODS], + type: EthAccountType.Eoa, + scopes: [EthScope.Eoa], + metadata: { + name: '', + keyring: { type: KeyringTypes.hd }, + importTime: 1691565967600, + lastSelected: 1955565967656, + }, +}; + class MockNormalAccountUUID { readonly #accountIds: Record = {}; @@ -289,14 +308,17 @@ function buildAccountsControllerMessenger( * @param options - The options object. * @param [options.initialState] - The initial state to use for the AccountsController. * @param [options.messenger] - The root messenger to use for creating the AccountsController messenger. + * @param [options.overrideState] - The state to override the initial state with. * @returns An instance of the AccountsController class. */ function setupAccountsController({ initialState = {}, messenger = buildMessenger(), + overrideState = null, }: { initialState?: Partial; messenger?: RootMessenger; + overrideState?: AccountsControllerState | null; }): { accountsController: AccountsController; messenger: RootMessenger; @@ -308,7 +330,7 @@ function setupAccountsController({ const accountsController = new AccountsController({ messenger: accountsControllerMessenger, - state: { ...defaultState, ...initialState }, + state: overrideState ?? { ...defaultState, ...initialState }, }); const triggerMultichainNetworkChange = ( @@ -348,6 +370,35 @@ describe('AccountsController', () => { lastSelected: 22222, }); + describe('constructor', () => { + it('constructs the accountIdByAddress correctly', () => { + const { accountsController } = setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { + [mockAccount.id]: mockAccount, + }, + selectedAccount: mockAccount.id, + }, + }, + }); + + expect(accountsController.state.accountIdByAddress).toStrictEqual({ + [mockAccount.address]: mockAccount.id, + }); + }); + + it('handles empty state', () => { + const { accountsController } = setupAccountsController({ + // @ts-expect-error - We want to test empty state here. + overrideState: {}, + }); + // No accounts, no accountIdByAddress + // Initial state + expect(accountsController.state.accountIdByAddress).toStrictEqual({}); + }); + }); + describe('onSnapStateChange', () => { it('enables an account if the Snap is enabled and not blocked', async () => { const messenger = buildMessenger(); @@ -380,6 +431,9 @@ describe('AccountsController', () => { }, selectedAccount: mockSnapAccount.id, }, + accountIdByAddress: { + [mockSnapAccount.address]: mockSnapAccount.id, + }, }, messenger, }); @@ -422,6 +476,9 @@ describe('AccountsController', () => { }, selectedAccount: mockSnapAccount.id, }, + accountIdByAddress: { + [mockSnapAccount.address]: mockSnapAccount.id, + }, }, messenger, }); @@ -464,6 +521,9 @@ describe('AccountsController', () => { }, selectedAccount: mockSnapAccount.id, }, + accountIdByAddress: { + [mockSnapAccount.address]: mockSnapAccount.id, + }, }, messenger, }); @@ -507,6 +567,9 @@ describe('AccountsController', () => { }, selectedAccount: mockSnapAccount.id, }, + accountIdByAddress: { + [mockSnapAccount.address]: mockSnapAccount.id, + }, }, messenger, }); @@ -552,6 +615,9 @@ describe('AccountsController', () => { }, selectedAccount: mockSnapAccount.id, }, + accountIdByAddress: { + [mockSnapAccount.address]: mockSnapAccount.id, + }, }, messenger, }); @@ -579,6 +645,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -619,6 +686,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -657,6 +725,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -700,6 +769,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount3.address]: mockAccount3.id, + }, }, messenger, }); @@ -769,6 +842,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount4.address]: mockAccount4.id, + }, }, messenger, }); @@ -844,6 +921,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount4.address]: mockAccount4.id, + }, }, messenger, }); @@ -905,6 +986,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -953,6 +1037,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, messenger, }); @@ -1022,6 +1110,11 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2WithCustomName.address]: + mockAccount2WithCustomName.id, + }, }, messenger, }); @@ -1093,6 +1186,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: 'missing', }, + accountIdByAddress: {}, }, messenger, }); @@ -1135,6 +1229,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount3.address]: mockAccount3.id, + }, }, messenger, }); @@ -1170,6 +1268,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -1237,6 +1338,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, messenger, }); @@ -1293,6 +1398,10 @@ describe('AccountsController', () => { }, selectedAccount: 'missing-account', }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, messenger, }); @@ -1357,6 +1466,10 @@ describe('AccountsController', () => { }, selectedAccount: 'missing-account', }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, messenger, }); @@ -1429,6 +1542,10 @@ describe('AccountsController', () => { }, selectedAccount: 'missing-account', }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, messenger, }); @@ -1472,6 +1589,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount3.address]: mockAccount3.id, + }, }, messenger, }); @@ -1549,6 +1670,9 @@ describe('AccountsController', () => { }, selectedAccount: mockInitialAccount.id, }, + accountIdByAddress: { + [mockInitialAccount.address]: mockInitialAccount.id, + }, }, messenger, }); @@ -1627,6 +1751,10 @@ describe('AccountsController', () => { }, selectedAccount: 'unknown', }, + accountIdByAddress: { + [mockExistingAccount1.address]: mockExistingAccount1.id, + [mockExistingAccount2.address]: mockExistingAccount2.id, + }, }, messenger, }); @@ -1669,6 +1797,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -1743,6 +1872,9 @@ describe('AccountsController', () => { }, selectedAccount: account.id, }, + accountIdByAddress: { + [account.address]: account.id, + }, }, messenger, }); @@ -1850,6 +1982,11 @@ describe('AccountsController', () => { }, selectedAccount: mockNewerEvmAccount.id, }, + accountIdByAddress: { + [mockOlderEvmAccount.address]: mockOlderEvmAccount.id, + [mockNewerEvmAccount.address]: mockNewerEvmAccount.id, + [mockBtcAccount.address]: mockBtcAccount.id, + }, }, messenger, }); @@ -1875,6 +2012,10 @@ describe('AccountsController', () => { }, selectedAccount: mockBtcAccount.id, }, + accountIdByAddress: { + [mockOlderEvmAccount.address]: mockOlderEvmAccount.id, + [mockBtcAccount.address]: mockBtcAccount.id, + }, }, messenger, }); @@ -1900,6 +2041,9 @@ describe('AccountsController', () => { }, selectedAccount: mockOlderEvmAccount.id, }, + accountIdByAddress: { + [mockOlderEvmAccount.address]: mockOlderEvmAccount.id, + }, }, messenger, }); @@ -1999,6 +2143,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -2064,6 +2209,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -2117,6 +2263,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -2168,6 +2315,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -2238,6 +2388,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -2310,6 +2461,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -2382,6 +2534,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -2432,6 +2585,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, messenger, }); @@ -2531,6 +2685,10 @@ describe('AccountsController', () => { }, selectedAccount: 'unknown', }, + accountIdByAddress: { + [mockExistingAccount1.address]: mockExistingAccount1.id, + [mockExistingAccount2.address]: mockExistingAccount2.id, + }, }, messenger, }); @@ -2613,6 +2771,9 @@ describe('AccountsController', () => { }, selectedAccount: mockHdAccount.id, }, + accountIdByAddress: { + [mockHdAccount.address]: mockHdAccount.id, + }, }, messenger, }); @@ -2700,6 +2861,9 @@ describe('AccountsController', () => { }, selectedAccount: mockHdAccount.id, }, + accountIdByAddress: { + [mockHdAccount.address]: mockHdAccount.id, + }, }, messenger, }); @@ -2727,6 +2891,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, }); @@ -2737,6 +2902,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }); expect(accountsController.state).toStrictEqual({ @@ -2746,6 +2914,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }); }); @@ -2756,6 +2927,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); @@ -2769,6 +2943,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }); }); }); @@ -2781,6 +2958,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); @@ -2795,6 +2975,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); @@ -2816,6 +2999,11 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + [mockAccount3.address]: mockAccount3.id, + }, }, }); @@ -2855,6 +3043,11 @@ describe('AccountsController', () => { }, selectedAccount: lastSelectedAccount.id, }, + accountIdByAddress: { + [mockOlderEvmAccount.address]: mockOlderEvmAccount.id, + [mockNewerEvmAccount.address]: mockNewerEvmAccount.id, + [mockBtcAccount.address]: mockBtcAccount.id, + }, }, }); @@ -2871,6 +3064,9 @@ describe('AccountsController', () => { }, selectedAccount: mockBtcAccount.id, }, + accountIdByAddress: { + [mockBtcAccount.address]: mockBtcAccount.id, + }, }, }); @@ -2886,6 +3082,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, }); @@ -2948,6 +3145,11 @@ describe('AccountsController', () => { }, selectedAccount: selectedAccount.id, }, + accountIdByAddress: { + [mockOlderEvmAccount.address]: mockOlderEvmAccount.id, + [mockNewerEvmAccount.address]: mockNewerEvmAccount.id, + [mockBtcAccount.address]: mockBtcAccount.id, + }, }, }); @@ -2973,6 +3175,11 @@ describe('AccountsController', () => { }, selectedAccount: mockBtcAccount.id, }, + accountIdByAddress: { + [mockOlderEvmAccount.address]: mockOlderEvmAccount.id, + [mockNewerEvmAccount.address]: mockNewerEvmAccount.id, + [mockBtcAccount.address]: mockBtcAccount.id, + }, }, }); @@ -2991,6 +3198,7 @@ describe('AccountsController', () => { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }, }); @@ -3038,6 +3246,11 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + [mockNonEvmAccount.address]: mockNonEvmAccount.id, + }, }, }); @@ -3134,6 +3347,15 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + [mockErc4337MainnetAccount.address]: mockErc4337MainnetAccount.id, + [mockErc4337TestnetAccount.address]: mockErc4337TestnetAccount.id, + [mockBtcMainnetAccount.address]: mockBtcMainnetAccount.id, + [mockBtcMainnetAccount2.address]: mockBtcMainnetAccount2.id, + [mockBtcTestnetAccount.address]: mockBtcTestnetAccount.id, + }, }, }); expect( @@ -3151,6 +3373,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, }); @@ -3174,6 +3400,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, }); @@ -3225,6 +3455,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockNonEvmAccount.address]: mockNonEvmAccount.id, + }, }, }); @@ -3258,6 +3492,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }; @@ -3287,6 +3524,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, }); @@ -3347,6 +3588,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); accountsController.setAccountName(mockAccount.id, 'new name'); @@ -3367,6 +3611,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); @@ -3386,6 +3633,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); @@ -3417,6 +3667,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccountWithName.address]: mockAccountWithName.id, + }, }, }); @@ -3434,6 +3688,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); accountsController.updateAccountMetadata(mockAccount.id, { @@ -3511,6 +3768,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3550,6 +3810,9 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3598,6 +3861,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); @@ -3615,6 +3881,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, }); @@ -3640,6 +3909,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockNonEvmAccount.address]: mockNonEvmAccount.id, + }, }, }); @@ -3649,6 +3922,23 @@ describe('AccountsController', () => { expect(account).toStrictEqual(mockNonEvmAccount); }); + + it('can handle a checksummed address', () => { + const { accountsController } = setupAccountsController({ + initialState: { + internalAccounts: { + accounts: { [mockAccount5.id]: mockAccount5 }, + selectedAccount: mockAccount5.id, + }, + }, + }); + + const checksummedAddress = toChecksumHexAddress(mockAccount5.address); + const account = + accountsController.getAccountByAddress(checksummedAddress); + + expect(account).toStrictEqual(mockAccount5); + }); }); describe('actions', () => { @@ -3676,6 +3966,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3706,6 +3999,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockNonEvmAccount.address]: mockNonEvmAccount.id, + }, }, messenger, }); @@ -3735,6 +4032,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockNonEvmAccount.address]: mockNonEvmAccount.id, + }, }, messenger, }); @@ -3758,6 +4059,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3786,6 +4090,10 @@ describe('AccountsController', () => { }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + [mockAccount2.address]: mockAccount2.id, + }, }, messenger, }); @@ -3823,6 +4131,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3842,6 +4153,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3867,6 +4181,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3887,6 +4204,9 @@ describe('AccountsController', () => { accounts: { [mockAccount.id]: mockAccount }, selectedAccount: mockAccount.id, }, + accountIdByAddress: { + [mockAccount.address]: mockAccount.id, + }, }, messenger, }); @@ -3965,6 +4285,7 @@ describe('AccountsController', () => { ), ).toMatchInlineSnapshot(` { + "accountIdByAddress": {}, "internalAccounts": { "accounts": {}, "selectedAccount": "", diff --git a/packages/accounts-controller/src/AccountsController.ts b/packages/accounts-controller/src/AccountsController.ts index 33ba8a789d3..5bd67e9a36f 100644 --- a/packages/accounts-controller/src/AccountsController.ts +++ b/packages/accounts-controller/src/AccountsController.ts @@ -44,6 +44,7 @@ import type { MultichainNetworkControllerNetworkDidChangeEvent } from './types'; import type { AccountsControllerStrictState } from './typing'; import type { HdSnapKeyringAccount } from './utils'; import { + constructAccountIdByAddress, getEvmDerivationPathForIndex, getEvmGroupIndexFromAddressIndex, getUUIDFromAddressOfNormalAccount, @@ -70,6 +71,7 @@ export type AccountsControllerState = { accounts: Record; selectedAccount: string; // id of the selected account }; + accountIdByAddress: Record; }; /** @@ -238,6 +240,12 @@ const accountsControllerMetadata = { includeInDebugSnapshot: false, usedInUi: true, }, + accountIdByAddress: { + includeInStateLogs: false, + persist: false, + includeInDebugSnapshot: false, + usedInUi: true, + }, }; const defaultState: AccountsControllerState = { @@ -245,6 +253,7 @@ const defaultState: AccountsControllerState = { accounts: {}, selectedAccount: '', }, + accountIdByAddress: {}, }; /** @@ -309,6 +318,9 @@ export class AccountsController extends BaseController< messenger: AccountsControllerMessenger; state: AccountsControllerState; }) { + const accountIdByAddress = constructAccountIdByAddress( + state?.internalAccounts?.accounts ?? {}, + ); super({ messenger, name: controllerName, @@ -316,6 +328,7 @@ export class AccountsController extends BaseController< state: { ...defaultState, ...state, + accountIdByAddress, }, }); @@ -478,9 +491,23 @@ export class AccountsController extends BaseController< * @returns The account with the specified address, or undefined if not found. */ getAccountByAddress(address: string): InternalAccount | undefined { - return this.listMultichainAccounts().find( - (account) => account.address.toLowerCase() === address.toLowerCase(), - ); + // We need to have a fallback as a cache miss might be attributed to a checksummed address being passed. + let accountId = this.state.accountIdByAddress[address]; + if (!accountId) { + // FIXME: We should not need lower-cased addresses, but some consumers might + // still be using non-normalized addresses. For now we keep it + // for convenience, but we will need to remove this fallback + // at some point. + // NOTE: We should only hit that branch for EVM accounts only. + const lowercasedAddress = address.toLowerCase(); + accountId = this.state.accountIdByAddress[lowercasedAddress]; + if (accountId) { + log( + `Cache missed for account ID: ${accountId}, received address: "${address}", matched address: "${lowercasedAddress}"`, + ); + } + } + return accountId ? this.getAccount(accountId) : undefined; } /** @@ -670,6 +697,7 @@ export class AccountsController extends BaseController< this.#update((state) => { state.internalAccounts.accounts = internalAccounts; + state.accountIdByAddress = constructAccountIdByAddress(internalAccounts); }); log('Accounts synchronized!'); @@ -684,9 +712,13 @@ export class AccountsController extends BaseController< */ loadBackup(backup: AccountsControllerState): void { if (backup.internalAccounts) { + const accountIdByAddress = constructAccountIdByAddress( + backup.internalAccounts.accounts, + ); this.update( (currentState: WritableDraft) => { currentState.internalAccounts = backup.internalAccounts; + currentState.accountIdByAddress = accountIdByAddress; }, ); } @@ -911,11 +943,12 @@ export class AccountsController extends BaseController< this.#update( (state) => { - const { internalAccounts } = state; + const { internalAccounts, accountIdByAddress } = state; for (const patch of [patches.snap, patches.normal]) { for (const account of patch.removed) { delete internalAccounts.accounts[account.id]; + delete accountIdByAddress[account.address]; diff.removed.push(account.id); } @@ -944,6 +977,8 @@ export class AccountsController extends BaseController< }, }; + accountIdByAddress[account.address] = account.id; + diff.added.push(internalAccounts.accounts[account.id]); } } diff --git a/packages/accounts-controller/src/typing.ts b/packages/accounts-controller/src/typing.ts index 1160df39ab3..81d038ac998 100644 --- a/packages/accounts-controller/src/typing.ts +++ b/packages/accounts-controller/src/typing.ts @@ -32,4 +32,5 @@ export type AccountsControllerStrictState = IsAccountControllerState<{ accounts: Record; selectedAccount: InternalAccount['id']; }; + accountIdByAddress: Record; }>; diff --git a/packages/accounts-controller/src/utils.test.ts b/packages/accounts-controller/src/utils.test.ts index b118cc4836a..618494cf268 100644 --- a/packages/accounts-controller/src/utils.test.ts +++ b/packages/accounts-controller/src/utils.test.ts @@ -3,10 +3,12 @@ import type { KeyringObject } from '@metamask/keyring-controller'; import { KeyringTypes } from '@metamask/keyring-controller'; import { + constructAccountIdByAddress, getEvmGroupIndexFromAddressIndex, isNormalKeyringType, isSimpleKeyringType, } from './utils'; +import { createMockInternalAccount } from '../tests/mocks'; describe('utils', () => { describe('isNormalKeyringType', () => { @@ -132,4 +134,21 @@ describe('utils', () => { ); }); }); + + describe('constructAccountIdByAddress', () => { + it('returns the account id by address for a map of accounts', () => { + const account = createMockInternalAccount({ + id: '1', + address: '0x123abc', + }); + + const accountIdByAddress = constructAccountIdByAddress({ + [account.id]: account, + }); + + expect(accountIdByAddress).toStrictEqual({ + '0x123abc': account.id, + }); + }); + }); }); diff --git a/packages/accounts-controller/src/utils.ts b/packages/accounts-controller/src/utils.ts index e2bbcc93fbd..5620031d37b 100644 --- a/packages/accounts-controller/src/utils.ts +++ b/packages/accounts-controller/src/utils.ts @@ -8,6 +8,8 @@ import { sha256 } from 'ethereum-cryptography/sha256'; import type { V4Options } from 'uuid'; import { v4 as uuid } from 'uuid'; +import type { AccountId } from './AccountsController'; + /** * Returns the name of the keyring type. * @@ -206,3 +208,13 @@ export function isHdSnapKeyringAccount( ): account is HdSnapKeyringAccount { return is(account.options, HdSnapKeyringAccountOptionsStruct); } + +export function constructAccountIdByAddress( + accountsMap: Record, +): Record { + const accounts = Object.values(accountsMap); + return accounts.reduce>((acc, account) => { + acc[account.address] = account.id; + return acc; + }, {}); +} diff --git a/packages/profile-metrics-controller/src/ProfileMetricsController.test.ts b/packages/profile-metrics-controller/src/ProfileMetricsController.test.ts index b6890783e76..681314ddd3f 100644 --- a/packages/profile-metrics-controller/src/ProfileMetricsController.test.ts +++ b/packages/profile-metrics-controller/src/ProfileMetricsController.test.ts @@ -106,6 +106,10 @@ describe('ProfileMetricsController', () => { }, selectedAccount: account1.id, }, + accountIdByAddress: { + [account1.address]: account1.id, + [account2.address]: account2.id, + }, }; }, ); @@ -152,6 +156,10 @@ describe('ProfileMetricsController', () => { }, selectedAccount: account1.id, }, + accountIdByAddress: { + [account1.address]: account1.id, + [account2.address]: account2.id, + }, }; }, );