MetaMask / MetaMask/metamask-mobile

Use stable EMPTY_OBJECT for selectSwapsTransactions fallback

Open Beginner friendly
#31,356 1 comment 0 reactions 0 assignees View on GitHub
area-performance Sev3 size-XS ta-triaged team-confirmations team-transactions
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/selectors/transactionController.ts:294`

### What is this about?

`selectSwapsTransactions` is a plain `createSelector` whose result function returns `transactionControllerState.swapsTransactions ?? {}`. The input is the whole `TransactionController` slice. When `swapsTransactions` is undefined, the `?? {}` creates a NEW empty object literal on every recompute, and because the input is the full controller slice, any transaction-state change recomputes and yields a fresh `{}` reference.

**Why it matters**

Consumers reading swaps transactions re-render whenever ANY transaction-controller field changes (very frequent during pending/confirming transactions), even when there are no swaps. The `?? {}` defeats memoization for all empty-swaps users.

### Scenario

N/A — see Technical Details.

### Design

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

### Technical Details

**Evidence**

`app/selectors/transactionController.ts:294`
```ts
export const selectSwapsTransactions = createSelector(
selectTransactionControllerState,
(transactionControllerState) =>
//@ts-expect-error - This is populated at the app level...
transactionControllerState.swapsTransactions ?? {},
);
```

**Fix**

Define a module-level frozen `EMPTY_OBJECT` and return it for the fallback, and/or use `createDeepEqualSelector`. Minimal mechanical fix:
```ts
const EMPTY_SWAPS = Object.freeze({});
export const selectSwapsTransactions = createSelector(
selectTransactionControllerState,
(transactionControllerState) =>
transactionControllerState.swapsTransactions ?? EMPTY_SWAPS,
);
```

### Threat Modeling Framework

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

### Acceptance Criteria

- `app/selectors/transactionController.test.ts:88` asserts `toStrictEqual({})` for the empty path but not referential stability. Add `expect(selectSwapsTransactions(s1)).toBe(selectSwapsTransactions(s2))` for two states with no swaps. Profiler: confirm swaps consumers stop re-rendering on unrelated transaction updates.

### References

- File: `app/selectors/transactionController.ts:294`
- Source: MetaMask Mobile performance audit — finding `selector-swaps-transactions-empty-object`
- Owner (CODEOWNERS / best-effort): @MetaMask/confirmations
- Status: **UNVALIDATED**

Contributor guide

Open the contributing guide

Research direction

Start in app/selectors/transactionController.ts at selectSwapsTransactions, then read app/selectors/transactionController.test.ts around line 88. Run the selector tests and compare two states without swaps. Done means the empty result remains referentially stable across unrelated transaction-state changes while existing selector behavior stays intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
performance
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.