MetaMask / MetaMask/metamask-mobile

Remove isEqual band-aid on selectPendingApprovals in useApprovalRequest

Open
#31,336 1 comment 0 reactions 0 assignees View on GitHub
area-performance Sev2 size-S ta-triaged team-confirmations
Dominant language
TypeScript
Stars
3k
Forks
1.7k
Avg merge
1d 14h
Merged PRs (30d)
669

Description

> **Performance audit finding** · Severity: **Medium** · Effort: Hard · Fix risk: Risky · Test safety net: Covered (app/components/Views/confirmations/hooks/useApprovalRequest.test.ts)
> Owner: `@MetaMask/confirmations`
> File: `app/components/Views/confirmations/hooks/useApprovalRequest.ts:14`

### What is this about?

`useApprovalRequest` does `useSelector(selectPendingApprovals, isEqual)`. `selectPendingApprovals` is an identity-style reselect selector that simply returns `approvalControllerState?.pendingApprovals` (a slice reference). The `isEqual` deep comparator is a band-aid: it papers over the fact that the slice reference changes on unrelated ApprovalController state updates, forcing a deep object compare on every store dispatch.

**Why it matters**

This hook is used in core confirmation flows and runs `lodash.isEqual` against the entire pending-approvals map on every Redux dispatch app-wide. Deep equality over the approvals object on every store update is avoidable overhead, and `isEqual` masks the real issue (the selector returns a raw slice rather than a stably-shaped projection).

### Scenario

N/A — see Technical Details.

### Design

N/A — internal performance change; no UI/design impact.

### Technical Details

**Evidence**

`app/components/Views/confirmations/hooks/useApprovalRequest.ts:14`
```ts
const pendingApprovals = useSelector(selectPendingApprovals, isEqual);
const pendingApprovalList = Object.values(pendingApprovals ?? {});
const firstPendingApproval = pendingApprovalList[0] as ApprovalRequestType | undefined;
```
Selector (identity passthrough):
`app/selectors/approvalController.ts:8`
```ts
export const selectPendingApprovals = createSelector(
selectApprovalControllerState,
(approvalControllerState) => approvalControllerState?.pendingApprovals,
);
```

**Fix**

The hook only needs the first pending approval. Add a memoized selector (e.g. `selectFirstPendingApproval`) built with `createSelector` that returns `Object.values(pendingApprovals ?? {})[0]`; reselect will then keep the reference stable while inputs are unchanged, allowing default `===` comparison and dropping `isEqual`. This avoids deep comparisons on every dispatch. Mirror the same treatment in the other `selectPendingApprovals` + `isEqual` callsites (`useApprovalFlow.ts`, `PermissionApproval.tsx`, `useMusdConversion.ts`).

### Threat Modeling Framework

N/A — performance-only change; behavior is preserved, no new data flow / trust boundary / attack surface.

### Acceptance Criteria

- - Run `yarn jest app/components/Views/confirmations/hooks/useApprovalRequest.test.ts` and `yarn jest app/selectors/approvalController.test.ts`.
- Add a selector test asserting the projection returns a stable reference for unchanged `pendingApprovals`.

### References

- File: `app/components/Views/confirmations/hooks/useApprovalRequest.ts:14`
- Source: MetaMask Mobile performance audit — finding `redux-useapprovalrequest-isequal-bandaid`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations
- Status: **UNVALIDATED**

Contributor guide

Open the contributing guide

Research direction

Start with app/components/Views/confirmations/hooks/useApprovalRequest.ts and app/selectors/approvalController.ts, then inspect the other selectPendingApprovals callsites named in the issue. Run the listed hook and selector Jest tests; done means the deep comparator is removed, the first-approval projection is covered by a stability test, and the affected callsites retain their behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
frontend, performance
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.