Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ui/pages/confirmations/selectors/accounts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('selectAccountGroupNameByInternalAccount', () => {

beforeEach(() => {
jest.clearAllMocks();
selectAccountGroupNameByInternalAccount.clearCache();
(getAccountGroupsByAddress as unknown as jest.Mock).mockReturnValue(
mockAccountGroups,
);
Expand Down
30 changes: 15 additions & 15 deletions ui/pages/confirmations/selectors/accounts.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,26 @@
import { InternalAccount } from '@metamask/keyring-internal-api';
import { createDeepEqualSelector } from '../../../../shared/lib/selectors/selector-creators';
import { createParameterizedSelector } from '../../../../shared/lib/selectors/selector-creators';
import { getAccountGroupsByAddress } from '../../../selectors/multichain-accounts/account-tree';
import {
AccountGroupWithInternalAccounts,
MultichainAccountsState,
} from '../../../selectors/multichain-accounts/account-tree.types';

export const selectAccountGroupNameByInternalAccount = createDeepEqualSelector(
[
(state: MultichainAccountsState, internalAccount: string | undefined) => {
if (!internalAccount) {
return { groups: [], address: null };
}
const accountSelectorFactory = createParameterizedSelector(20);

return {
groups: getAccountGroupsByAddress(state, [internalAccount]),
address: internalAccount?.toLowerCase(),
};
},
],
({ groups, address }): string | null => {
if (!address || !groups.length) {
export const selectAccountGroupNameByInternalAccount = accountSelectorFactory(
(state: MultichainAccountsState) => state,
(_state: MultichainAccountsState, internalAccount: string | undefined) =>
internalAccount,
(state, internalAccount): string | null => {
if (!internalAccount) {
return null;
}

const groups = getAccountGroupsByAddress(state, [internalAccount]);
const address = internalAccount.toLowerCase();

if (!groups.length) {
return null;
}

Expand Down
3 changes: 1 addition & 2 deletions ui/selectors/approvals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
import { ApprovalType } from '@metamask/controller-utils';
import { createSelector } from 'reselect';
import { Json } from '@metamask/utils';
import { createDeepEqualSelector } from '../../shared/lib/selectors/selector-creators';
import { SMART_TRANSACTION_CONFIRMATION_TYPES } from '../../shared/constants/app';
import { getBooleanFeatureFlag } from '../../shared/lib/remote-feature-flag-utils';
import { getRemoteFeatureFlags } from '../../shared/lib/selectors/remote-feature-flags';
Expand Down Expand Up @@ -89,7 +88,7 @@ const getSkipSmartTransactionStatusPage = createSelector(
* Returns pending approvals sorted by time for use in confirmation navigation.
* Excludes duplicate watch asset approvals as they are combined into a single confirmation.
*/
export const selectPendingApprovalsForNavigation = createDeepEqualSelector(
export const selectPendingApprovalsForNavigation = createSelector(
pendingApprovalsSortedSelector,
getSkipSmartTransactionStatusPage,
(sortedPendingApprovals, skipSmartTransactionStatusPage) =>
Expand Down
30 changes: 15 additions & 15 deletions ui/selectors/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -1348,9 +1348,8 @@ export function getUnapprovedTxCount(state) {
return Object.keys(unapprovedTxs).length;
}

// Deep-equal memo: pendingApprovals map identity can change while approvals list is unchanged.
export const getUnapprovedConfirmations = createDeepEqualSelector(
(state) => state.metamask.pendingApprovals || {},
export const getUnapprovedConfirmations = createSelector(
(state) => state.metamask.pendingApprovals ?? EMPTY_OBJECT,
(pendingApprovals) => Object.values(pendingApprovals),
);

Expand Down Expand Up @@ -2072,43 +2071,44 @@ export const getMetadataContractName = createSelector(

export const getTxData = (state) => state.confirmTransaction.txData;

// Deep-equal memo: tx map lookups; controller-backed maps can get new references with same txs.
export const getUnapprovedTransaction = createDeepEqualSelector(
const unapprovedTransactionSelectorFactory = createParameterizedSelector(20);

export const getUnapprovedTransaction = unapprovedTransactionSelectorFactory(
(state) => getUnapprovedTransactions(state),
(_, transactionId) => transactionId,
(unapprovedTxs, transactionId) =>
Object.values(unapprovedTxs).find(({ id }) => id === transactionId),
);

// Deep-equal memo: find + EMPTY_OBJECT fallback makes output reference unstable without deep memo.
export const getTransaction = createDeepEqualSelector(
const transactionSelectorFactory = createParameterizedSelector(50);

export const getTransaction = transactionSelectorFactory(
getCurrentNetworkTransactions,
(_, transactionId) => transactionId,
(transactions, transactionId) => {
return transactions.find(({ id }) => id === transactionId) || EMPTY_OBJECT;
},
);

// Deep-equal memo: merges txData and tx meta; nested objects often re-created for the same tx.
export const getFullTxData = createDeepEqualSelector(
const fullTxDataSelectorFactory = createParameterizedSelector(20);

export const getFullTxData = fullTxDataSelectorFactory(
getTxData,
(state, transactionId, status) => {
if (status === TransactionStatus.unapproved) {
return getUnapprovedTransaction(state, transactionId) ?? {};
}
return getTransaction(state, transactionId);
},
(_state, _transactionId, _status, customTxParamsData) => customTxParamsData,
(
_state,
_transactionId,
_status,
customTxParamsData,
hexTransactionAmount,
) => ({
customTxParamsData,
_customTxParamsData,
hexTransactionAmount,
}),
(txData, transaction, { customTxParamsData, hexTransactionAmount }) => {
) => hexTransactionAmount,
(txData, transaction, customTxParamsData, hexTransactionAmount) => {
let fullTxData = { ...txData, ...transaction };
if (transaction && transaction.simulationFails) {
fullTxData.simulationFails = { ...transaction.simulationFails };
Expand Down
112 changes: 112 additions & 0 deletions ui/selectors/selectors.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,118 @@ describe('Selectors', () => {
expect(totalUnapprovedCount).toStrictEqual(1);
});

describe('transaction lookup selector memoization', () => {
const state = {
confirmTransaction: {
txData: {
txParams: {
data: '0x',
value: '0x0',
},
},
},
metamask: {
...mockNetworkState({ chainId: CHAIN_IDS.MAINNET }),
pendingApprovals: {},
transactions: [
{
chainId: CHAIN_IDS.MAINNET,
id: 'tx-1',
status: TransactionStatus.submitted,
txParams: { to: '0x1' },
},
{
chainId: CHAIN_IDS.MAINNET,
id: 'tx-2',
status: TransactionStatus.submitted,
txParams: { to: '0x2' },
},
{
chainId: CHAIN_IDS.MAINNET,
id: 'tx-3',
status: TransactionStatus.unapproved,
txParams: { to: '0x3' },
},
{
chainId: CHAIN_IDS.MAINNET,
id: 'tx-4',
status: TransactionStatus.unapproved,
txParams: { to: '0x4' },
},
],
},
};

beforeEach(() => {
selectors.getUnapprovedTransaction.clearCache();
selectors.getUnapprovedTransaction.resetRecomputations();
selectors.getTransaction.clearCache();
selectors.getTransaction.resetRecomputations();
selectors.getFullTxData.clearCache();
selectors.getFullTxData.resetRecomputations();
});

it('caches unapproved transaction lookups per transaction ID', () => {
expect(selectors.getUnapprovedTransaction(state, 'tx-3').id).toBe('tx-3');
expect(selectors.getUnapprovedTransaction(state, 'tx-4').id).toBe('tx-4');
expect(selectors.getUnapprovedTransaction(state, 'tx-3').id).toBe('tx-3');

expect(selectors.getUnapprovedTransaction.recomputations()).toBe(2);
});

it('caches current-network transaction lookups per transaction ID', () => {
expect(selectors.getTransaction(state, 'tx-1').id).toBe('tx-1');
expect(selectors.getTransaction(state, 'tx-2').id).toBe('tx-2');
expect(selectors.getTransaction(state, 'tx-1').id).toBe('tx-1');

expect(selectors.getTransaction.recomputations()).toBe(2);
});

it('caches full transaction data per transaction ID', () => {
expect(
selectors.getFullTxData(
state,
'tx-1',
TransactionStatus.submitted,
'0xdeadbeef',
'0x10',
).txParams,
).toStrictEqual({
data: '0xdeadbeef',
to: '0x1',
value: '0x10',
});
expect(
selectors.getFullTxData(
state,
'tx-2',
TransactionStatus.submitted,
'0xdeadbeef',
'0x10',
).txParams,
).toStrictEqual({
data: '0xdeadbeef',
to: '0x2',
value: '0x10',
});
expect(
selectors.getFullTxData(
state,
'tx-1',
TransactionStatus.submitted,
'0xdeadbeef',
'0x10',
).txParams,
).toStrictEqual({
data: '0xdeadbeef',
to: '0x1',
value: '0x10',
});

expect(selectors.getFullTxData.recomputations()).toBe(2);
});
});

it('#getUseTokenDetection', () => {
const useTokenDetection = selectors.getUseTokenDetection(mockState);
expect(useTokenDetection).toStrictEqual(true);
Expand Down
48 changes: 19 additions & 29 deletions ui/selectors/transactions.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
} from '../../shared/lib/selectors/networks';

import {
createDeepEqualSelector,
createShallowEqualInputAndResultSelector,
createParameterizedShallowEqualSelector,
} from '../../shared/lib/selectors/selector-creators';
Expand Down Expand Up @@ -108,47 +107,38 @@
return getTransactionsByChainId(state, providerConfig.chainId);
};

export const incomingTxListSelectorAllChains = createDeepEqualSelector(
(state) => {
const allNetworkTransactions = getTransactions(state);
const { address: selectedAddress } = getSelectedInternalAccount(state);

return allNetworkTransactions.filter(
export const incomingTxListSelectorAllChains = createSelector(
getTransactions,
getSelectedInternalAccount,
(allNetworkTransactions, { address: selectedAddress }) =>
allNetworkTransactions.filter(
(tx) =>
tx.type === TransactionType.incoming &&
tx.txParams.to === selectedAddress,
);
},
(transactions) => transactions,
),
);

export const getApprovedAndSignedTransactions = createDeepEqualSelector(
(state) => {
// Fetch transactions across all networks to address a nonce management limitation.
// This issue arises when a pending transaction exists on one network, and the user initiates another transaction on a different network.
const transactions = getTransactions(state);

return transactions.filter((transaction) =>
export const getApprovedAndSignedTransactions = createSelector(
getTransactions,
// Fetch transactions across all networks to address a nonce management limitation.
// This issue arises when a pending transaction exists on one network, and the user initiates another transaction on a different network.
(transactions) =>
transactions.filter((transaction) =>
[TransactionStatus.approved, TransactionStatus.signed].includes(
transaction.status,
),
);
},
(transactions) => transactions,
),
);

export const incomingTxListSelector = createDeepEqualSelector(
(state) => {
const currentNetworkTransactions = getCurrentNetworkTransactions(state);
const { address: selectedAddress } = getSelectedInternalAccount(state);

return currentNetworkTransactions.filter(
export const incomingTxListSelector = createSelector(
getCurrentNetworkTransactions,

Check warning on line 134 in ui/selectors/transactions.js

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

'getCurrentNetworkTransactions' is deprecated.

See more on https://sonarcloud.io/project/issues?id=MetaMask_metamask-extension&issues=AZ7WD2fnTn1XnFaF6ZeG&open=AZ7WD2fnTn1XnFaF6ZeG&pullRequest=43606
getSelectedInternalAccount,
(currentNetworkTransactions, { address: selectedAddress }) =>
currentNetworkTransactions.filter(
(tx) =>
tx.type === TransactionType.incoming &&
tx.txParams.to === selectedAddress,
);
},
(transactions) => transactions,
),
);

export const unapprovedPersonalMsgsSelector = (state) =>
Expand Down
Loading