MetaMask / MetaMask/metamask-mobile

Memoize SendMetricsContext value and hoist getAccountTypeSafely computation

Open Beginner friendly
#31,287 1 comment 0 reactions 0 assignees View on GitHub
area-performance Sev3 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: Easy · Fix risk: Simple · Test safety net: Partial
> Owner: `@MetaMask/confirmations`
> File: `app/components/Views/confirmations/context/send-context/send-metrics-context.tsx:90`

### What is this about?

`SendMetricsContextProvider` passes an inline object literal to the provider. Worse, the `accountType` field is computed inline by calling `getAccountTypeSafely(from)` directly in the JSX on every render, so the (try/catch-wrapped) address-type lookup runs on each render in addition to the object identity changing.

**Why it matters**

This provider depends on `useSendContext()` (`asset`, `from`), so it re-renders whenever the Send context changes — including every amount keystroke (see the SendContext finding). Each of those re-renders re-runs `getAccountTypeSafely` and rebuilds the context object, re-rendering its 5 consumers. The lookup is cheap but unnecessary per keystroke, and the fan-out is avoidable.

### Scenario

N/A — see Technical Details.

### Design

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

### Technical Details

**Evidence**

`app/components/Views/confirmations/context/send-context/send-metrics-context.tsx:90`
```tsx

```

**Fix**

Compute `accountType` with `useMemo` keyed on `from`, then wrap the whole value in `useMemo`:
```tsx
const accountType = useMemo(
() => (isEvmAddress(from as string) ? getAccountTypeSafely(from as string) : undefined),
[from],
);
const value = useMemo(
() => ({ accountType, assetListSize, amountInputMethod, amountInputType, assetFilterMethod,
chainId, chainIdCaip, setAssetListSize, setAmountInputMethod, setAmountInputType, setAssetFilterMethod }),
[accountType, assetListSize, amountInputMethod, amountInputType, assetFilterMethod, chainId, chainIdCaip],
);
```
(The `set*` functions are stable `useState` setters.)

### 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/context/send-context/send-metrics-context.test.tsx`.
- Confirm `accountType` lookup is not invoked on amount keystrokes (e.g. spy on `getAddressAccountType`).

### References

- File: `app/components/Views/confirmations/context/send-context/send-metrics-context.tsx:90`
- Source: MetaMask Mobile performance audit — finding `context-send-metrics-inline-value`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations
- Status: **UNVALIDATED**

Contributor guide

Open the contributing guide

Research direction

Start with app/components/Views/confirmations/context/send-context/send-metrics-context.tsx at the provider around line 90, then read the surrounding Send context usage and existing tests. Run yarn jest app/components/Views/confirmations/context/send-context/send-metrics-context.test.tsx; done means the provider value is stable between unrelated renders and the account-type lookup is not invoked on amount keystrokes.

Written by the indexing model from the issue text.

Assessment

Tech stack
react-native, typescript
Domain
frontend, mobile, performance
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.