MetaMask / MetaMask/metamask-mobile
Memoize useFirstPartyContractNames per-request O(n) contract scan and array
- Dominant language
- TypeScript
- Stars
- 3k
- Forks
- 1.7k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
> **Performance audit finding** · Severity: **Medium** · Effort: Easy · Fix risk: Simple · Test safety net: Covered (app/components/hooks/DisplayName/useFirstPartyContractNames.test.ts)
> Owner: `@MetaMask/confirmations (suggested)`
> File: `app/components/hooks/DisplayName/useFirstPartyContractNames.ts:9`
### What is this about?
`useFirstPartyContractNames` returns `requests.map(...)` with **no `useMemo`**, and for each request it recomputes `Object.keys(FIRST_PARTY_CONTRACT_NAMES)` and then linearly `.find()`s through every first-party contract name comparing lowercased addresses. This is an O(requests × contracts) scan recomputed on every render, plus a fresh array on every render.
```ts
return requests.map((request) => {
...
const contractNames = Object.keys(FIRST_PARTY_CONTRACT_NAMES); // rebuilt per request, per render
const name = contractNames.find(
(contractName) =>
FIRST_PARTY_CONTRACT_NAMES[contractName]?.[chainId]?.toLowerCase() ===
normalizedValue,
);
return name ?? null;
});
```
**Why it matters**
This is a sub-hook aggregated by `useDisplayNames`, which runs per name/address rendered (confirmation screens, simulation rows, transaction lists — often per virtualized row). `FIRST_PARTY_CONTRACT_NAMES` is a static module constant, so both the `Object.keys` and the lowercasing comparison are pure, repeatable work that should be computed once. Returning a fresh array each render also defeats downstream memoization.
### Scenario
N/A — see Technical Details.
### Design
N/A — internal performance change; no UI/design impact.
### Technical Details
**Evidence**
`app/components/hooks/DisplayName/useFirstPartyContractNames.ts:9-27` — the entire body is an unmemoized `requests.map(...)`; `Object.keys(FIRST_PARTY_CONTRACT_NAMES)` (line 18) is recomputed inside the map callback.
**Fix**
- Hoist `const CONTRACT_NAMES = Object.keys(FIRST_PARTY_CONTRACT_NAMES);` to module scope (it never changes), or better, precompute a `Map>` index once at module load for O(1) lookups.
- Wrap the `requests.map(...)` in `useMemo` keyed on `[requests]` so the returned array is stable.
### 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/hooks/DisplayName/useFirstPartyContractNames.test.ts`.
- Add a test rendering twice with identical `requests` and assert the returned array reference is `===` across renders.
### References
- File: `app/components/hooks/DisplayName/useFirstPartyContractNames.ts:9`
- Source: MetaMask Mobile performance audit — finding `unstablehook-usefirstpartycontractnames-on2-lookup-every-render`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations (suggested)
- Status: **UNVALIDATED**
Contributor guide
Research direction
Start with app/components/hooks/DisplayName/useFirstPartyContractNames.ts and its existing test at app/components/hooks/DisplayName/useFirstPartyContractNames.test.ts; run the specified Jest command first. Keep the lookup behavior while avoiding repeated contract-scan work and stabilizing the mapped result for identical requests, then add the requested repeated-render reference test and confirm the suite passes.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react-native, typescript
- Domain
- mobile-dev, performance, testing-qa
- Issue type
- Refactor
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100