From cba16ef3f2e0f5652be762b37862e7067189cd4b Mon Sep 17 00:00:00 2001 From: David Murdoch <187813+davidmurdoch@users.noreply.github.com> Date: Thu, 12 Sep 2024 17:31:59 -0400 Subject: [PATCH] move `getSelectedInternalAccount` from `selectors.js` to `accounts.ts` --- test/data/mock-state.json | 7 ++ .../interactive-replacement-token-modal.tsx | 2 +- ...ractive-replacement-token-notification.tsx | 3 + ui/ducks/metamask/metamask.js | 2 +- ui/hooks/bridge/useBridging.ts | 1 + ui/hooks/useMultichainSelector.ts | 1 + ui/pages/asset/components/token-buttons.tsx | 1 + .../interactive-replacement-token-page.tsx | 2 +- ui/selectors/accounts.test.ts | 72 +++++++++++++++++++ ui/selectors/accounts.ts | 7 +- ui/selectors/index.js | 1 + ui/selectors/institutional/selectors.ts | 5 +- ui/selectors/multichain.ts | 3 +- ui/selectors/permissions.js | 2 +- ui/selectors/selectors.js | 6 +- ui/selectors/selectors.test.js | 68 +----------------- ui/selectors/transactions.js | 3 +- 17 files changed, 105 insertions(+), 81 deletions(-) diff --git a/test/data/mock-state.json b/test/data/mock-state.json index 4b6a2f506215..6bbeb904f184 100644 --- a/test/data/mock-state.json +++ b/test/data/mock-state.json @@ -415,6 +415,7 @@ "address": "0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc", "id": "cf8dace4-9439-4bd4-b3a8-88c821c8fcb3", "metadata": { + "importTime": 0, "name": "Test Account", "keyring": { "type": "HD Key Tree" @@ -434,6 +435,7 @@ "address": "0xec1adf982415d2ef5ec55899b9bfb8bc0f29251b", "id": "07c2cfec-36c9-46c4-8115-3836d3ac9047", "metadata": { + "importTime": 0, "name": "Test Account 2", "keyring": { "type": "HD Key Tree" @@ -453,6 +455,7 @@ "address": "0xc42edfcc21ed14dda456aa0756c153f7985d8813", "id": "15e69915-2a1a-4019-93b3-916e11fd432f", "metadata": { + "importTime": 0, "name": "Ledger Hardware 2", "keyring": { "type": "Ledger Hardware" @@ -472,6 +475,7 @@ "address": "0xeb9e64b93097bc15f01f13eae97015c57ab64823", "id": "784225f4-d30b-4e77-a900-c8bbce735b88", "metadata": { + "importTime": 0, "name": "Test Account 3", "keyring": { "type": "HD Key Tree" @@ -491,6 +495,7 @@ "address": "0xca8f1F0245530118D0cf14a06b01Daf8f76Cf281", "id": "694225f4-d30b-4e77-a900-c8bbce735b42", "metadata": { + "importTime": 0, "name": "Test Account 4", "keyring": { "type": "Custody test" @@ -510,11 +515,13 @@ "address": "0xb552685e3d2790efd64a175b00d51f02cdafee5d", "id": "c3deeb99-ba0d-4a4e-a0aa-033fc1f79ae3", "metadata": { + "importTime": 0, "name": "Snap Account 1", "keyring": { "type": "Snap Keyring" }, "snap": { + "enabled": true, "id": "snap-id", "name": "snap-name" } diff --git a/ui/components/institutional/interactive-replacement-token-modal/interactive-replacement-token-modal.tsx b/ui/components/institutional/interactive-replacement-token-modal/interactive-replacement-token-modal.tsx index 4526595fa455..17c717a7d179 100644 --- a/ui/components/institutional/interactive-replacement-token-modal/interactive-replacement-token-modal.tsx +++ b/ui/components/institutional/interactive-replacement-token-modal/interactive-replacement-token-modal.tsx @@ -4,7 +4,7 @@ import { ICustodianType } from '@metamask-institutional/types'; import { useI18nContext } from '../../../hooks/useI18nContext'; import { MetaMetricsContext } from '../../../contexts/metametrics'; import { hideModal } from '../../../store/actions'; -import { getSelectedInternalAccount } from '../../../selectors/selectors'; +import { getSelectedInternalAccount } from '../../../selectors/accounts'; import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; import { Box, diff --git a/ui/components/institutional/interactive-replacement-token-notification/interactive-replacement-token-notification.tsx b/ui/components/institutional/interactive-replacement-token-notification/interactive-replacement-token-notification.tsx index 10dc049b8678..7c1d0f60488f 100644 --- a/ui/components/institutional/interactive-replacement-token-notification/interactive-replacement-token-notification.tsx +++ b/ui/components/institutional/interactive-replacement-token-notification/interactive-replacement-token-notification.tsx @@ -56,6 +56,7 @@ const InteractiveReplacementTokenNotification: React.FC< interactiveReplacementToken && Boolean(Object.keys(interactiveReplacementToken).length); + // @ts-expect-error keyring type is wrong maybe? if (!/^Custody/u.test(keyring.type) || !hasInteractiveReplacementToken) { setShowNotification(false); return; @@ -66,6 +67,7 @@ const InteractiveReplacementTokenNotification: React.FC< )) as unknown as string; const custodyAccountDetails = await dispatch( mmiActions.getAllCustodianAccountsWithToken( + // @ts-expect-error keyring type is wrong maybe? keyring.type.split(' - ')[1], token, ), @@ -105,6 +107,7 @@ const InteractiveReplacementTokenNotification: React.FC< interactiveReplacementToken?.oldRefreshToken, isUnlocked, dispatch, + // @ts-expect-error keyring type is wrong maybe? keyring.type, interactiveReplacementToken, mmiActions, diff --git a/ui/ducks/metamask/metamask.js b/ui/ducks/metamask/metamask.js index 98ca4fe1032c..427f604f0096 100644 --- a/ui/ducks/metamask/metamask.js +++ b/ui/ducks/metamask/metamask.js @@ -17,9 +17,9 @@ import { checkNetworkAndAccountSupports1559, getAddressBook, getSelectedNetworkClientId, - getSelectedInternalAccount, getNetworkConfigurationsByChainId, } from '../../selectors/selectors'; +import { getSelectedInternalAccount } from '../../selectors/accounts'; import * as actionConstants from '../../store/actionConstants'; import { updateTransactionGasFees } from '../../store/actions'; import { setCustomGasLimit, setCustomGasPrice } from '../gas/gas.duck'; diff --git a/ui/hooks/bridge/useBridging.ts b/ui/hooks/bridge/useBridging.ts index d11aaeb821a9..0aaa77c5aa4f 100644 --- a/ui/hooks/bridge/useBridging.ts +++ b/ui/hooks/bridge/useBridging.ts @@ -43,6 +43,7 @@ const useBridging = () => { const isMarketingEnabled = useSelector(getDataCollectionForMarketing); const chainId = useSelector(getCurrentChainId); const keyring = useSelector(getCurrentKeyring); + // @ts-expect-error keyring type is wrong maybe? const usingHardwareWallet = isHardwareKeyring(keyring.type); const isBridgeSupported = useSelector(getIsBridgeEnabled); diff --git a/ui/hooks/useMultichainSelector.ts b/ui/hooks/useMultichainSelector.ts index 326ac79bf9cd..9bd979df7e7e 100644 --- a/ui/hooks/useMultichainSelector.ts +++ b/ui/hooks/useMultichainSelector.ts @@ -11,6 +11,7 @@ export function useMultichainSelector< ) { return useSelector((state: TState) => { // We either pass an account or fallback to the currently selected one + // @ts-expect-error state types don't match return selector(state, account || getSelectedInternalAccount(state)); }); } diff --git a/ui/pages/asset/components/token-buttons.tsx b/ui/pages/asset/components/token-buttons.tsx index a07cdaca2d48..ef40288322bd 100644 --- a/ui/pages/asset/components/token-buttons.tsx +++ b/ui/pages/asset/components/token-buttons.tsx @@ -66,6 +66,7 @@ const TokenButtons = ({ ///: BEGIN:ONLY_INCLUDE_IF(build-main,build-beta,build-flask) const keyring = useSelector(getCurrentKeyring); + // @ts-expect-error keyring type is wrong maybe? const usingHardwareWallet = isHardwareKeyring(keyring.type); ///: END:ONLY_INCLUDE_IF diff --git a/ui/pages/institutional/interactive-replacement-token-page/interactive-replacement-token-page.tsx b/ui/pages/institutional/interactive-replacement-token-page/interactive-replacement-token-page.tsx index c7258d1b2e38..5c00eb4ffa10 100644 --- a/ui/pages/institutional/interactive-replacement-token-page/interactive-replacement-token-page.tsx +++ b/ui/pages/institutional/interactive-replacement-token-page/interactive-replacement-token-page.tsx @@ -27,7 +27,7 @@ import { useI18nContext } from '../../../hooks/useI18nContext'; import { useCopyToClipboard } from '../../../hooks/useCopyToClipboard'; import { getMetaMaskAccounts } from '../../../selectors'; import { getInstitutionalConnectRequests } from '../../../ducks/institutional/institutional'; -import { getSelectedInternalAccount } from '../../../selectors/selectors'; +import { getSelectedInternalAccount } from '../../../selectors/accounts'; import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils'; import { SWAPS_CHAINID_DEFAULT_BLOCK_EXPLORER_URL_MAP } from '../../../../shared/constants/swaps'; import { CHAIN_IDS } from '../../../../shared/constants/network'; diff --git a/ui/selectors/accounts.test.ts b/ui/selectors/accounts.test.ts index 61a0059989ba..b67c4a30b9e4 100644 --- a/ui/selectors/accounts.test.ts +++ b/ui/selectors/accounts.test.ts @@ -1,3 +1,5 @@ +import { EthAccountType } from '@metamask/keyring-api'; +import { ETH_EOA_METHODS } from '../../shared/constants/eth-methods'; import { MOCK_ACCOUNTS, MOCK_ACCOUNT_EOA, @@ -5,12 +7,14 @@ import { MOCK_ACCOUNT_BIP122_P2WPKH, MOCK_ACCOUNT_BIP122_P2WPKH_TESTNET, } from '../../test/data/mock-accounts'; +import mockState from '../../test/data/mock-state.json'; import { AccountsState, isSelectedInternalAccountEth, isSelectedInternalAccountBtc, hasCreatedBtcMainnetAccount, hasCreatedBtcTestnetAccount, + getSelectedInternalAccount, } from './accounts'; const MOCK_STATE: AccountsState = { @@ -23,6 +27,74 @@ const MOCK_STATE: AccountsState = { }; describe('Accounts Selectors', () => { + describe('#getSelectedInternalAccount', () => { + it('returns selected internalAccount', () => { + expect( + getSelectedInternalAccount(mockState as AccountsState), + ).toStrictEqual({ + address: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc', + id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3', + metadata: { + name: 'Test Account', + keyring: { + type: 'HD Key Tree', + }, + }, + options: {}, + methods: [ + 'personal_sign', + 'eth_signTransaction', + 'eth_signTypedData_v1', + 'eth_signTypedData_v3', + 'eth_signTypedData_v4', + ], + type: 'eip155:eoa', + }); + }); + + it('returns undefined if selectedAccount is undefined', () => { + expect( + getSelectedInternalAccount({ + metamask: { + internalAccounts: { + accounts: {}, + selectedAccount: '', + }, + }, + }), + ).toBeUndefined(); + }); + + it('returns selectedAccount', () => { + const mockInternalAccount = { + address: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc', + id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3', + metadata: { + importTime: 0, + name: 'Test Account', + keyring: { + type: 'HD Key Tree', + }, + }, + options: {}, + methods: ETH_EOA_METHODS, + type: EthAccountType.Eoa, + }; + expect( + getSelectedInternalAccount({ + metamask: { + internalAccounts: { + accounts: { + [mockInternalAccount.id]: mockInternalAccount, + }, + selectedAccount: mockInternalAccount.id, + }, + }, + }), + ).toStrictEqual(mockInternalAccount); + }); + }); + describe('isSelectedInternalAccountEth', () => { // @ts-expect-error This is missing from the Mocha type definitions it.each([ diff --git a/ui/selectors/accounts.ts b/ui/selectors/accounts.ts index bd33d5af1f89..d69cd130f9aa 100644 --- a/ui/selectors/accounts.ts +++ b/ui/selectors/accounts.ts @@ -8,7 +8,7 @@ import { isBtcMainnetAddress, isBtcTestnetAddress, } from '../../shared/lib/multichain'; -import { getSelectedInternalAccount, getInternalAccounts } from './selectors'; +import { getInternalAccounts } from './selectors'; export type AccountsState = { metamask: AccountsControllerState; @@ -20,6 +20,11 @@ function isBtcAccount(account: InternalAccount) { return Boolean(account && account.type === P2wpkh); } +export function getSelectedInternalAccount(state: AccountsState) { + const accountId = state.metamask.internalAccounts.selectedAccount; + return state.metamask.internalAccounts.accounts[accountId]; +} + export function isSelectedInternalAccountEth(state: AccountsState) { const account = getSelectedInternalAccount(state); const { Eoa, Erc4337 } = EthAccountType; diff --git a/ui/selectors/index.js b/ui/selectors/index.js index 6c65a481a709..290c70fb2a31 100644 --- a/ui/selectors/index.js +++ b/ui/selectors/index.js @@ -7,3 +7,4 @@ export * from './permissions'; export * from './selectors'; export * from './transactions'; export * from './approvals'; +export * from './accounts'; diff --git a/ui/selectors/institutional/selectors.ts b/ui/selectors/institutional/selectors.ts index edaaf5278ae7..05bd13b52509 100644 --- a/ui/selectors/institutional/selectors.ts +++ b/ui/selectors/institutional/selectors.ts @@ -1,5 +1,6 @@ import { toChecksumAddress } from 'ethereumjs-util'; -import { getAccountType, getSelectedInternalAccount } from '../selectors'; +import { getAccountType } from '../selectors'; +import { getSelectedInternalAccount } from '../accounts'; import { getProviderConfig } from '../../ducks/metamask/metamask'; import { hexToDecimal } from '../../../shared/modules/conversion.utils'; // TODO: Remove restricted import @@ -166,6 +167,7 @@ export function getCustodianIconForAddress(state: State, address: string) { export function getIsCustodianSupportedChain(state: State) { try { + // @ts-expect-error state types don't match const selectedAccount = getSelectedInternalAccount(state); const accountType = getAccountType(state); const providerConfig = getProviderConfig(state); @@ -207,6 +209,7 @@ export function getIsCustodianSupportedChain(state: State) { export function getMMIAddressFromModalOrAddress(state: State) { const modalAddress = state?.appState?.modal?.modalState?.props?.address; + // @ts-expect-error state types don't match const selectedAddress = getSelectedInternalAccount(state)?.address; return modalAddress || selectedAddress; diff --git a/ui/selectors/multichain.ts b/ui/selectors/multichain.ts index 533cac2afda0..335f557d5318 100644 --- a/ui/selectors/multichain.ts +++ b/ui/selectors/multichain.ts @@ -24,7 +24,7 @@ import { CHAIN_ID_TO_NETWORK_IMAGE_URL_MAP, TEST_NETWORK_IDS, } from '../../shared/constants/network'; -import { AccountsState } from './accounts'; +import { AccountsState, getSelectedInternalAccount } from './accounts'; import { getCurrentChainId, getCurrentCurrency, @@ -33,7 +33,6 @@ import { getNativeCurrencyImage, getNetworkConfigurationsByChainId, getSelectedAccountCachedBalance, - getSelectedInternalAccount, getShouldShowFiat, getShowFiatInTestnets, } from './selectors'; diff --git a/ui/selectors/permissions.js b/ui/selectors/permissions.js index 4031b8e881a3..00468f2f948d 100644 --- a/ui/selectors/permissions.js +++ b/ui/selectors/permissions.js @@ -10,9 +10,9 @@ import { getInternalAccount, getMetaMaskAccountsOrdered, getOriginOfCurrentTab, - getSelectedInternalAccount, getTargetSubjectMetadata, } from './selectors'; +import { getSelectedInternalAccount } from './accounts'; // selectors diff --git a/ui/selectors/selectors.js b/ui/selectors/selectors.js index 31c262df62c4..efc5066d76ad 100644 --- a/ui/selectors/selectors.js +++ b/ui/selectors/selectors.js @@ -121,6 +121,7 @@ import { getOrderedConnectedAccountsForConnectedDapp, getSubjectMetadata, } from './permissions'; +import { getSelectedInternalAccount } from './accounts'; import { createDeepEqualSelector } from './util'; import { getMultichainBalances, getMultichainNetwork } from './multichain'; @@ -357,11 +358,6 @@ export function getMaybeSelectedInternalAccount(state) { : undefined; } -export function getSelectedInternalAccount(state) { - const accountId = state.metamask.internalAccounts.selectedAccount; - return state.metamask.internalAccounts.accounts[accountId]; -} - export function checkIfMethodIsEnabled(state, methodName) { const internalAccount = getSelectedInternalAccount(state); return Boolean(internalAccount.methods.includes(methodName)); diff --git a/ui/selectors/selectors.test.js b/ui/selectors/selectors.test.js index 342e7d7187c8..a6808429f632 100644 --- a/ui/selectors/selectors.test.js +++ b/ui/selectors/selectors.test.js @@ -1,6 +1,6 @@ import { deepClone } from '@metamask/snaps-utils'; import { ApprovalType } from '@metamask/controller-utils'; -import { EthAccountType, EthMethod } from '@metamask/keyring-api'; +import { EthMethod } from '@metamask/keyring-api'; import { TransactionStatus } from '@metamask/transaction-controller'; import mockState from '../../test/data/mock-state.json'; import { KeyringType } from '../../shared/constants/keyring'; @@ -8,7 +8,6 @@ import { CHAIN_IDS, NETWORK_TYPES } from '../../shared/constants/network'; import { SURVEY_DATE, SURVEY_GMT } from '../helpers/constants/survey'; import { PRIVACY_POLICY_DATE } from '../helpers/constants/privacy-policy'; import { createMockInternalAccount } from '../../test/jest/mocks'; -import { ETH_EOA_METHODS } from '../../shared/constants/eth-methods'; import { getProviderConfig } from '../ducks/metamask/metamask'; import { mockNetworkState } from '../../test/stub/networks'; import * as selectors from './selectors'; @@ -60,49 +59,6 @@ describe('Selectors', () => { }); }); - describe('#getSelectedInternalAccount', () => { - it('returns undefined if selectedAccount is undefined', () => { - expect( - selectors.getSelectedInternalAccount({ - metamask: { - internalAccounts: { - accounts: {}, - selectedAccount: '', - }, - }, - }), - ).toBeUndefined(); - }); - - it('returns selectedAccount', () => { - const mockInternalAccount = { - address: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc', - id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3', - metadata: { - name: 'Test Account', - keyring: { - type: 'HD Key Tree', - }, - }, - options: {}, - methods: ETH_EOA_METHODS, - type: EthAccountType.Eoa, - }; - expect( - selectors.getSelectedInternalAccount({ - metamask: { - internalAccounts: { - accounts: { - [mockInternalAccount.id]: mockInternalAccount, - }, - selectedAccount: mockInternalAccount.id, - }, - }, - }), - ).toStrictEqual(mockInternalAccount); - }); - }); - describe('#checkIfMethodIsEnabled', () => { it('returns true if the method is enabled', () => { expect( @@ -955,28 +911,6 @@ describe('Selectors', () => { }); }); - it('returns selected internalAccount', () => { - expect(selectors.getSelectedInternalAccount(mockState)).toStrictEqual({ - address: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc', - id: 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3', - metadata: { - name: 'Test Account', - keyring: { - type: 'HD Key Tree', - }, - }, - options: {}, - methods: [ - 'personal_sign', - 'eth_signTransaction', - 'eth_signTypedData_v1', - 'eth_signTypedData_v3', - 'eth_signTypedData_v4', - ], - type: 'eip155:eoa', - }); - }); - it('returns selected account', () => { const account = selectors.getSelectedAccount(mockState); expect(account.balance).toStrictEqual('0x346ba7725f412cbfdb'); diff --git a/ui/selectors/transactions.js b/ui/selectors/transactions.js index 2c5fb8fe4b98..3074fd4bfde4 100644 --- a/ui/selectors/transactions.js +++ b/ui/selectors/transactions.js @@ -13,7 +13,8 @@ import txHelper from '../helpers/utils/tx-helper'; import { SmartTransactionStatus } from '../../shared/constants/transaction'; import { hexToDecimal } from '../../shared/modules/conversion.utils'; import { getProviderConfig } from '../ducks/metamask/metamask'; -import { getCurrentChainId, getSelectedInternalAccount } from './selectors'; +import { getCurrentChainId } from './selectors'; +import { getSelectedInternalAccount } from './accounts'; import { hasPendingApprovals, getApprovalRequestsByType } from './approvals'; import { createDeepEqualSelector,