Skip to content

Commit 52a008d

Browse files
tommasinitommasini
committed
selected account possibly undefined
1 parent ace5c7c commit 52a008d

3 files changed

Lines changed: 385 additions & 0 deletions

File tree

‎app/store/migrations/126.test.ts‎

Lines changed: 286 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,286 @@
1+
import migrate from './126';
2+
import { captureException } from '@sentry/react-native';
3+
4+
jest.mock('@sentry/react-native', () => ({
5+
captureException: jest.fn(),
6+
}));
7+
const mockedCaptureException = jest.mocked(captureException);
8+
9+
const migrationVersion = 126;
10+
11+
interface InternalAccounts {
12+
accounts: Record<string, { id: string; address: string }>;
13+
selectedAccount?: string;
14+
}
15+
16+
const createValidState = (
17+
internalAccounts?: InternalAccounts | unknown,
18+
includeController = true,
19+
) => ({
20+
engine: {
21+
backgroundState: {
22+
...(includeController && {
23+
AccountsController: {
24+
...(internalAccounts !== undefined && {
25+
internalAccounts,
26+
}),
27+
},
28+
}),
29+
},
30+
},
31+
});
32+
33+
const ACCOUNT_1_ID = 'cf8dace4-9439-4bd4-b3a8-88c821c8fcb3';
34+
const ACCOUNT_2_ID = 'e9b38e2b-7d1e-4e5f-8c2a-1a5b3d7e9f01';
35+
36+
const validAccounts = {
37+
[ACCOUNT_1_ID]: { id: ACCOUNT_1_ID, address: '0xabc' },
38+
[ACCOUNT_2_ID]: { id: ACCOUNT_2_ID, address: '0xdef' },
39+
};
40+
41+
describe(`Migration ${migrationVersion}: Fix selectedAccount missing or undefined`, () => {
42+
beforeEach(() => {
43+
jest.clearAllMocks();
44+
});
45+
46+
describe('invalid state structure', () => {
47+
it('returns state if top-level state is invalid', () => {
48+
const result = migrate('invalid');
49+
expect(result).toBe('invalid');
50+
});
51+
52+
it('captures exception and returns state if AccountsController is missing', () => {
53+
const state = createValidState(undefined, false);
54+
const result = migrate(state);
55+
56+
expect(result).toStrictEqual(state);
57+
expect(mockedCaptureException).toHaveBeenCalledWith(
58+
expect.objectContaining({
59+
message: expect.stringContaining(
60+
'AccountsController state is missing or invalid',
61+
),
62+
}),
63+
);
64+
});
65+
66+
it('captures exception and returns state if AccountsController is not an object', () => {
67+
const state = {
68+
engine: {
69+
backgroundState: {
70+
AccountsController: 'invalid',
71+
},
72+
},
73+
};
74+
const result = migrate(state);
75+
76+
expect(result).toStrictEqual(state);
77+
expect(mockedCaptureException).toHaveBeenCalledWith(
78+
expect.objectContaining({
79+
message: expect.stringContaining(
80+
'AccountsController state is missing or invalid',
81+
),
82+
}),
83+
);
84+
});
85+
86+
it('captures exception and returns state if internalAccounts is missing', () => {
87+
const state = createValidState(undefined, true);
88+
const result = migrate(state);
89+
90+
expect(result).toStrictEqual(state);
91+
expect(mockedCaptureException).toHaveBeenCalledWith(
92+
expect.objectContaining({
93+
message: expect.stringContaining(
94+
'internalAccounts is missing or invalid',
95+
),
96+
}),
97+
);
98+
});
99+
});
100+
101+
describe('valid selectedAccount (no-op)', () => {
102+
it('does not modify state when selectedAccount is a valid string', () => {
103+
const state = createValidState({
104+
accounts: validAccounts,
105+
selectedAccount: ACCOUNT_1_ID,
106+
});
107+
const result = migrate(state);
108+
109+
expect(result).toStrictEqual(state);
110+
expect(mockedCaptureException).not.toHaveBeenCalled();
111+
});
112+
});
113+
114+
describe('selectedAccount key is missing (JSON.stringify stripped it)', () => {
115+
it('sets selectedAccount to first account ID when key is absent', () => {
116+
const internalAccounts = {
117+
accounts: validAccounts,
118+
};
119+
// Simulate JSON roundtrip: key was stripped
120+
delete (internalAccounts as Record<string, unknown>).selectedAccount;
121+
122+
const state = createValidState(internalAccounts);
123+
const result = migrate(state) as ReturnType<typeof createValidState>;
124+
const resultAccounts = result.engine.backgroundState
125+
.AccountsController as Record<string, unknown>;
126+
const resultInternal = resultAccounts.internalAccounts as Record<
127+
string,
128+
unknown
129+
>;
130+
131+
expect(resultInternal.selectedAccount).toBe(ACCOUNT_1_ID);
132+
expect(mockedCaptureException).toHaveBeenCalledWith(
133+
expect.objectContaining({
134+
message: expect.stringContaining('key_missing'),
135+
}),
136+
);
137+
});
138+
139+
it('sets selectedAccount to empty string when key is absent and no accounts exist', () => {
140+
const internalAccounts = {
141+
accounts: {},
142+
};
143+
delete (internalAccounts as Record<string, unknown>).selectedAccount;
144+
145+
const state = createValidState(internalAccounts);
146+
const result = migrate(state) as ReturnType<typeof createValidState>;
147+
const resultAccounts = result.engine.backgroundState
148+
.AccountsController as Record<string, unknown>;
149+
const resultInternal = resultAccounts.internalAccounts as Record<
150+
string,
151+
unknown
152+
>;
153+
154+
expect(resultInternal.selectedAccount).toBe('');
155+
expect(mockedCaptureException).toHaveBeenCalledWith(
156+
expect.objectContaining({
157+
message: expect.stringContaining('key_missing'),
158+
}),
159+
);
160+
});
161+
});
162+
163+
describe('selectedAccount is explicitly undefined', () => {
164+
it('sets selectedAccount to first account ID when value is undefined', () => {
165+
const state = createValidState({
166+
accounts: validAccounts,
167+
selectedAccount: undefined,
168+
});
169+
const result = migrate(state) as ReturnType<typeof createValidState>;
170+
const resultAccounts = result.engine.backgroundState
171+
.AccountsController as Record<string, unknown>;
172+
const resultInternal = resultAccounts.internalAccounts as Record<
173+
string,
174+
unknown
175+
>;
176+
177+
expect(resultInternal.selectedAccount).toBe(ACCOUNT_1_ID);
178+
expect(mockedCaptureException).toHaveBeenCalledWith(
179+
expect.objectContaining({
180+
message: expect.stringContaining('selectedAccount is corrupt'),
181+
}),
182+
);
183+
});
184+
});
185+
186+
describe('selectedAccount is wrong type', () => {
187+
it('sets selectedAccount to first account ID when value is a number', () => {
188+
const state = createValidState({
189+
accounts: validAccounts,
190+
selectedAccount: 42,
191+
});
192+
const result = migrate(state) as ReturnType<typeof createValidState>;
193+
const resultAccounts = result.engine.backgroundState
194+
.AccountsController as Record<string, unknown>;
195+
const resultInternal = resultAccounts.internalAccounts as Record<
196+
string,
197+
unknown
198+
>;
199+
200+
expect(resultInternal.selectedAccount).toBe(ACCOUNT_1_ID);
201+
expect(mockedCaptureException).toHaveBeenCalledWith(
202+
expect.objectContaining({
203+
message: expect.stringContaining('wrong_type_number'),
204+
}),
205+
);
206+
});
207+
208+
it('sets selectedAccount to first account ID when value is null', () => {
209+
const state = createValidState({
210+
accounts: validAccounts,
211+
selectedAccount: null,
212+
});
213+
const result = migrate(state) as ReturnType<typeof createValidState>;
214+
const resultAccounts = result.engine.backgroundState
215+
.AccountsController as Record<string, unknown>;
216+
const resultInternal = resultAccounts.internalAccounts as Record<
217+
string,
218+
unknown
219+
>;
220+
221+
expect(resultInternal.selectedAccount).toBe(ACCOUNT_1_ID);
222+
expect(mockedCaptureException).toHaveBeenCalledWith(
223+
expect.objectContaining({
224+
message: expect.stringContaining('wrong_type_object'),
225+
}),
226+
);
227+
});
228+
});
229+
230+
describe('selectedAccount is empty string', () => {
231+
it('does not modify state when selectedAccount is empty string (AccountsController handles this)', () => {
232+
const state = createValidState({
233+
accounts: validAccounts,
234+
selectedAccount: '',
235+
});
236+
const result = migrate(state) as ReturnType<typeof createValidState>;
237+
const resultAccounts = result.engine.backgroundState
238+
.AccountsController as Record<string, unknown>;
239+
const resultInternal = resultAccounts.internalAccounts as Record<
240+
string,
241+
unknown
242+
>;
243+
244+
expect(resultInternal.selectedAccount).toBe(ACCOUNT_1_ID);
245+
expect(mockedCaptureException).toHaveBeenCalledWith(
246+
expect.objectContaining({
247+
message: expect.stringContaining('empty_string'),
248+
}),
249+
);
250+
});
251+
});
252+
253+
describe('fallback to empty string', () => {
254+
it('sets selectedAccount to empty string when accounts have invalid structure', () => {
255+
const state = createValidState({
256+
accounts: { 'bad-id': { notAnId: true } },
257+
selectedAccount: undefined,
258+
});
259+
const result = migrate(state) as ReturnType<typeof createValidState>;
260+
const resultAccounts = result.engine.backgroundState
261+
.AccountsController as Record<string, unknown>;
262+
const resultInternal = resultAccounts.internalAccounts as Record<
263+
string,
264+
unknown
265+
>;
266+
267+
expect(resultInternal.selectedAccount).toBe('');
268+
});
269+
270+
it('sets selectedAccount to empty string when first account id is empty', () => {
271+
const state = createValidState({
272+
accounts: { 'bad-id': { id: '' } },
273+
selectedAccount: undefined,
274+
});
275+
const result = migrate(state) as ReturnType<typeof createValidState>;
276+
const resultAccounts = result.engine.backgroundState
277+
.AccountsController as Record<string, unknown>;
278+
const resultInternal = resultAccounts.internalAccounts as Record<
279+
string,
280+
unknown
281+
>;
282+
283+
expect(resultInternal.selectedAccount).toBe('');
284+
});
285+
});
286+
});

‎app/store/migrations/126.ts‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import { captureException } from '@sentry/react-native';
2+
import { hasProperty, isObject } from '@metamask/utils';
3+
import { ensureValidState } from './util';
4+
5+
const migrationVersion = 126;
6+
7+
/**
8+
* Migration 126: Fix AccountsController selectedAccount that is missing or undefined.
9+
*
10+
* Migration 059 attempted to fix this but used `hasProperty` which returns false
11+
* when the key is entirely absent (the common case after JSON.stringify/JSON.parse
12+
* roundtrip strips undefined values). This migration covers both cases:
13+
* - selectedAccount key is missing from internalAccounts
14+
* - selectedAccount is explicitly undefined
15+
* - selectedAccount is not a string
16+
*
17+
* @param state - The persisted Redux state.
18+
* @returns The migrated Redux state.
19+
*/
20+
export default function migrate(state: unknown) {
21+
if (!ensureValidState(state, migrationVersion)) {
22+
return state;
23+
}
24+
25+
if (
26+
!hasProperty(state.engine.backgroundState, 'AccountsController') ||
27+
!isObject(state.engine.backgroundState.AccountsController)
28+
) {
29+
captureException(
30+
new Error(
31+
`Migration ${migrationVersion}: AccountsController state is missing or invalid: '${typeof state.engine.backgroundState.AccountsController}'`,
32+
),
33+
);
34+
return state;
35+
}
36+
37+
const accountsController = state.engine.backgroundState.AccountsController;
38+
39+
if (
40+
!hasProperty(accountsController, 'internalAccounts') ||
41+
!isObject(accountsController.internalAccounts)
42+
) {
43+
captureException(
44+
new Error(
45+
`Migration ${migrationVersion}: internalAccounts is missing or invalid: '${typeof accountsController.internalAccounts}'`,
46+
),
47+
);
48+
return state;
49+
}
50+
51+
const { internalAccounts } = accountsController;
52+
const hasKey = hasProperty(internalAccounts, 'selectedAccount');
53+
const currentValue = (internalAccounts as Record<string, unknown>)
54+
.selectedAccount;
55+
56+
if (hasKey && typeof currentValue === 'string' && currentValue.length > 0) {
57+
return state;
58+
}
59+
60+
// Report the exact corruption type to Sentry for diagnostics
61+
const corruptionType = !hasKey
62+
? 'key_missing'
63+
: currentValue === undefined
64+
? 'value_undefined'
65+
: typeof currentValue !== 'string'
66+
? `wrong_type_${typeof currentValue}`
67+
: 'empty_string';
68+
69+
captureException(
70+
new Error(
71+
`Migration ${migrationVersion}: selectedAccount is corrupt (${corruptionType}). hasKey=${hasKey}, typeof=${typeof currentValue}, value=${String(currentValue)}`,
72+
),
73+
);
74+
75+
// Try to recover: set to the first account ID if accounts exist
76+
if (
77+
hasProperty(internalAccounts, 'accounts') &&
78+
isObject(internalAccounts.accounts)
79+
) {
80+
const accountIds = Object.keys(internalAccounts.accounts);
81+
if (accountIds.length > 0) {
82+
const firstAccount = internalAccounts.accounts[accountIds[0]];
83+
if (isObject(firstAccount) && hasProperty(firstAccount, 'id')) {
84+
const accountId = firstAccount.id;
85+
if (typeof accountId === 'string' && accountId.length > 0) {
86+
(internalAccounts as Record<string, unknown>).selectedAccount =
87+
accountId;
88+
return state;
89+
}
90+
}
91+
}
92+
}
93+
94+
// Fallback: set to empty string so AccountsController auto-reconciles
95+
(internalAccounts as Record<string, unknown>).selectedAccount = '';
96+
return state;
97+
}

0 commit comments

Comments
 (0)