MetaMask / MetaMask/metamask-mobile
Replace O(n^2) reduce-with-spread over address book in useExistingAddress
- Dominant language
- TypeScript
- Stars
- 3k
- Forks
- 1.7k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 669
Description
> **Performance audit finding** · Severity: **Low** · Effort: Easy · Fix risk: Simple · Test safety net: Covered (app/components/hooks/useExistingAddress.test.ts)
> Owner: `@MetaMask/confirmations (suggested)`
> File: `app/components/hooks/useExistingAddress.ts:20`
### What is this about?
`useExistingAddress` flattens the per-network address book with `Object.values(addressBook).reduce((acc, networkAddressBook) => ({ ...acc, ...networkAddressBook }), {})`. Spreading `...acc` on every iteration reallocates and recopies the entire accumulator each step, making the flatten O(n^2) in the total number of address-book entries.
**Why it matters**
The address book grows with user contacts across networks. For users with many saved addresses this quadratic copy runs whenever `addressBook` changes (inside `useMemo`). While memoized, the per-recompute cost is unnecessarily high and the pattern is a known anti-pattern flagged by the audit.
### Scenario
N/A — see Technical Details.
### Design
N/A — internal performance change; no UI/design impact.
### Technical Details
**Evidence**
`app/components/hooks/useExistingAddress.ts:18`
```ts
const filteredAddressBook = useMemo(
() =>
Object.values(addressBook).reduce(
(acc, networkAddressBook) => ({
...acc,
...networkAddressBook,
}),
{},
),
[addressBook],
);
```
**Fix**
Mutate a single accumulator instead of spreading: `Object.values(addressBook).reduce((acc, n) => Object.assign(acc, n), {})`, or `Object.assign({}, ...Object.values(addressBook))`. Both are O(n) and produce the same flattened map. Keep the `useMemo` and dependency unchanged.
### 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/useExistingAddress.test.ts` to confirm matching/lookup behavior is unchanged.
### References
- File: `app/components/hooks/useExistingAddress.ts:20`
- Source: MetaMask Mobile performance audit — finding `unstablehook-useexistingaddress-reduce-spread`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations (suggested)
- Status: **UNVALIDATED**
Contributor guide
Research direction
Start with app/components/hooks/useExistingAddress.ts:18-20 and inspect the existing useMemo flattening logic. Run yarn jest app/components/hooks/useExistingAddress.test.ts; the work is done when the address book is flattened without repeated accumulator spreading and the existing matching and lookup tests pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react-native, typescript
- Domain
- mobile, performance
- Issue type
- Refactor
- Difficulty
- 1/5
- Estimated time
- Under an hour
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100