Skip to content

Commit a3c1ac2

Browse files
FrederikBoldingFrederikBolding
authored
fix: Prevent crash in modal version handling cp-13.11.1 (#38382)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Fixes a crash caused by `useMultichainAccountsIntroModal` that entirely bricks Flask. The crash occurs because the version handling doesn't handle our versioning scheme well. This PR changes the logic to use `previousAppVersion` and strips the `prerelease` part of the version (which indicates the build type) for a proper comparison. Also updates the tests for this hook which were flawed as they didn't use the hook in question at all. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/38382?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Fixes a crash when updating Flask ## **Manual testing steps** 1. Force `lastUpdatedFromVersion` to `13.9.0.150` and `previousAppVersion` to `13.9.0-flask.0` 2. See that it crashes without this PR ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <img width="454" height="634" alt="image" src="https://github.com/user-attachments/assets/ea140cfa-66ef-402e-a6cd-3ca3a2bc6402" /> <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Use `previousAppVersion` and strip prerelease parts for version comparison in `useMultichainAccountsIntroModal`; update tests to exercise the hook with provider-backed state. > > - **Hooks** (`ui/hooks/useMultichainAccountsIntroModal.ts`): > - Use `previousAppVersion` instead of `lastUpdatedFromVersion` for upgrade checks. > - Parse and strip prerelease identifiers via `semver.parse` before comparing with `BIP44_ACCOUNTS_INTRODUCTION_VERSION` using `semver.lt`. > - Export `BIP44_ACCOUNTS_INTRODUCTION_VERSION` for external use. > - Maintain display gating by `DEFAULT_ROUTE` and existing state flags; compute and set `showMultichainIntroModal` accordingly. > - **Tests** (`ui/hooks/useMultichainAccountsIntroModal.test.ts`): > - Replace ad-hoc logic with `renderHookWithProvider` to test the actual hook. > - Cover upgrades across thresholds including prerelease versions (e.g., `*-flask.0`) and ensure correct show/hide behavior across routes and states. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 9d7267f. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c87f69b commit a3c1ac2

File tree

2 files changed

+92
-73
lines changed

2 files changed

+92
-73
lines changed

‎ui/hooks/useMultichainAccountsIntroModal.test.ts‎

Lines changed: 79 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,34 @@
1-
import { lt as semverLt } from 'semver';
1+
import { renderHookWithProvider } from '../../test/lib/render-helpers';
2+
import {
3+
useMultichainAccountsIntroModal,
4+
BIP44_ACCOUNTS_INTRODUCTION_VERSION,
5+
} from './useMultichainAccountsIntroModal';
26

3-
// Test the core logic independently of React hooks
4-
describe('BIP-44 Banner Logic', () => {
5-
const BIP44_ACCOUNTS_INTRODUCTION_VERSION = '13.5.0';
6-
7-
// Helper function that mirrors the hook's core logic
8-
const shouldShowBip44Banner = (
7+
describe('useMultichainAccountsIntroModal', () => {
8+
const renderHook = (
99
isUnlocked: boolean,
1010
isMultichainAccountsEnabled: boolean,
11-
hasShownModalBefore: boolean,
11+
hasShownMultichainAccountsIntroModal: boolean,
1212
lastUpdatedAt: number | null,
13-
lastUpdatedFromVersion: string | null,
14-
isMainRoute: boolean,
13+
previousAppVersion: string | null,
14+
pathname: string,
1515
) => {
16-
const isUpgradeFromLowerThanBip44Version = Boolean(
17-
lastUpdatedFromVersion &&
18-
typeof lastUpdatedFromVersion === 'string' &&
19-
semverLt(lastUpdatedFromVersion, BIP44_ACCOUNTS_INTRODUCTION_VERSION),
20-
);
21-
22-
return (
23-
isUnlocked &&
24-
isMultichainAccountsEnabled &&
25-
!hasShownModalBefore &&
26-
lastUpdatedAt !== null && // null = fresh install, timestamp = upgrade
27-
isUpgradeFromLowerThanBip44Version &&
28-
isMainRoute
16+
return renderHookWithProvider(
17+
() => useMultichainAccountsIntroModal(isUnlocked, { pathname }),
18+
{
19+
metamask: {
20+
remoteFeatureFlags: {
21+
enableMultichainAccountsState2: {
22+
enabled: isMultichainAccountsEnabled,
23+
featureVersion: '2',
24+
minimumVersion: BIP44_ACCOUNTS_INTRODUCTION_VERSION,
25+
},
26+
},
27+
hasShownMultichainAccountsIntroModal,
28+
previousAppVersion,
29+
lastUpdatedAt,
30+
},
31+
},
2932
);
3033
};
3134

@@ -39,137 +42,147 @@ describe('BIP-44 Banner Logic', () => {
3942
};
4043

4144
it('shows banner for upgrade from 13.4.0', () => {
42-
const result = shouldShowBip44Banner(
45+
const { result } = renderHook(
4346
baseParams.isUnlocked,
4447
baseParams.isMultichainAccountsEnabled,
4548
baseParams.hasShownModalBefore,
4649
baseParams.lastUpdatedAt,
4750
'13.4.0',
48-
baseParams.isMainRoute,
51+
'/',
4952
);
50-
expect(result).toBe(true);
53+
expect(result.current.showMultichainIntroModal).toBe(true);
54+
});
55+
56+
it('shows banner for upgrade from 13.4.0-flask.0', () => {
57+
const { result } = renderHook(
58+
baseParams.isUnlocked,
59+
baseParams.isMultichainAccountsEnabled,
60+
baseParams.hasShownModalBefore,
61+
baseParams.lastUpdatedAt,
62+
'13.4.0-flask.0',
63+
'/',
64+
);
65+
expect(result.current.showMultichainIntroModal).toBe(true);
5166
});
5267

5368
it('shows banner for upgrade from 12.0.0', () => {
54-
const result = shouldShowBip44Banner(
69+
const { result } = renderHook(
5570
true,
5671
true,
5772
false,
5873
Date.now(),
5974
'12.0.0',
60-
true,
75+
'/',
6176
);
62-
expect(result).toBe(true);
77+
expect(result.current.showMultichainIntroModal).toBe(true);
6378
});
6479

6580
it('shows banner for upgrade from 13.4.9 (just before threshold)', () => {
66-
const result = shouldShowBip44Banner(
81+
const { result } = renderHook(
6782
true,
6883
true,
6984
false,
7085
Date.now(),
7186
'13.4.9',
72-
true,
87+
'/',
7388
);
74-
expect(result).toBe(true);
89+
expect(result.current.showMultichainIntroModal).toBe(true);
7590
});
7691
});
7792

7893
describe('does NOT show banner correctly', () => {
7994
it('does NOT show for upgrade from 13.5.0 (threshold version)', () => {
80-
const result = shouldShowBip44Banner(
95+
const { result } = renderHook(
8196
true,
8297
true,
8398
false,
8499
Date.now(),
85100
'13.5.0',
86-
true,
101+
'/',
87102
);
88-
expect(result).toBe(false);
103+
expect(result.current.showMultichainIntroModal).toBe(false);
89104
});
90105

91-
it('does NOT show for upgrade from 13.7.0', () => {
92-
const result = shouldShowBip44Banner(
106+
it('does NOT show for upgrade from 13.5.0-flask.0 (threshold version)', () => {
107+
const { result } = renderHook(
93108
true,
94109
true,
95110
false,
96111
Date.now(),
97-
'13.7.0',
98-
true,
112+
'13.5.0-flask.0',
113+
'/',
99114
);
100-
expect(result).toBe(false);
115+
expect(result.current.showMultichainIntroModal).toBe(false);
101116
});
102117

103-
it('does NOT show for fresh install (no previous version)', () => {
104-
const result = shouldShowBip44Banner(
118+
it('does NOT show for upgrade from 13.7.0', () => {
119+
const { result } = renderHook(
105120
true,
106121
true,
107122
false,
108123
Date.now(),
109-
null,
110-
true,
124+
'13.7.0',
125+
'/',
111126
);
112-
expect(result).toBe(false);
127+
expect(result.current.showMultichainIntroModal).toBe(false);
128+
});
129+
130+
it('does NOT show for fresh install (no previous version)', () => {
131+
const { result } = renderHook(true, true, false, Date.now(), null, '/');
132+
expect(result.current.showMultichainIntroModal).toBe(false);
113133
});
114134

115135
it('does NOT show when wallet is locked', () => {
116-
const result = shouldShowBip44Banner(
136+
const { result } = renderHook(
117137
false,
118138
true,
119139
false,
120140
Date.now(),
121141
'13.4.0',
122-
true,
142+
'/',
123143
);
124-
expect(result).toBe(false);
144+
expect(result.current.showMultichainIntroModal).toBe(false);
125145
});
126146

127147
it('does NOT show when multichain accounts disabled', () => {
128-
const result = shouldShowBip44Banner(
148+
const { result } = renderHook(
129149
true,
130150
false,
131151
false,
132152
Date.now(),
133153
'13.4.0',
134-
true,
154+
'/',
135155
);
136-
expect(result).toBe(false);
156+
expect(result.current.showMultichainIntroModal).toBe(false);
137157
});
138158

139159
it('does NOT show when modal already shown', () => {
140-
const result = shouldShowBip44Banner(
160+
const { result } = renderHook(
141161
true,
142162
true,
143163
true,
144164
Date.now(),
145165
'13.4.0',
146-
true,
166+
'/',
147167
);
148-
expect(result).toBe(false);
168+
expect(result.current.showMultichainIntroModal).toBe(false);
149169
});
150170

151171
it('does NOT show on non-main route', () => {
152-
const result = shouldShowBip44Banner(
172+
const { result } = renderHook(
153173
true,
154174
true,
155175
false,
156176
Date.now(),
157177
'13.4.0',
158-
false,
178+
'/confirmation/foo',
159179
);
160-
expect(result).toBe(false);
180+
expect(result.current.showMultichainIntroModal).toBe(false);
161181
});
162182

163183
it('does NOT show for fresh install (lastUpdatedAt is null)', () => {
164-
const result = shouldShowBip44Banner(
165-
true,
166-
true,
167-
false,
168-
null,
169-
'13.4.0',
170-
true,
171-
);
172-
expect(result).toBe(false);
184+
const { result } = renderHook(true, true, false, null, '13.4.0', '/');
185+
expect(result.current.showMultichainIntroModal).toBe(false);
173186
});
174187
});
175188
});

‎ui/hooks/useMultichainAccountsIntroModal.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
import { useState, useEffect } from 'react';
2-
import { lt as semverLt } from 'semver';
2+
import { lt as semverLt, parse as semverParse } from 'semver';
33
import { useAppSelector } from '../store/store';
44
import { getIsMultichainAccountsState2Enabled } from '../selectors/multichain-accounts/feature-flags';
5-
import { getLastUpdatedFromVersion } from '../selectors/selectors';
65
import { DEFAULT_ROUTE } from '../helpers/constants/routes';
76

87
// Version threshold for BIP-44 multichain accounts introduction
9-
const BIP44_ACCOUNTS_INTRODUCTION_VERSION = '13.5.0';
8+
export const BIP44_ACCOUNTS_INTRODUCTION_VERSION = '13.5.0';
109

1110
/**
1211
* Hook to manage the multichain accounts intro modal display logic
@@ -33,17 +32,24 @@ export function useMultichainAccountsIntroModal(
3332
);
3433

3534
const lastUpdatedAt = useAppSelector((state) => state.metamask.lastUpdatedAt);
36-
const lastUpdatedFromVersion = useAppSelector(getLastUpdatedFromVersion);
35+
const lastUpdatedFromVersion = useAppSelector(
36+
(state) => state.metamask.previousAppVersion,
37+
);
3738

3839
useEffect(() => {
3940
// Only show modal on the main wallet/home route
4041
const isMainWalletArea = location.pathname === DEFAULT_ROUTE;
4142

43+
const parsedLastVersion = semverParse(lastUpdatedFromVersion);
44+
// Strip prerelease versions as they just indicate build types.
45+
const strippedLastVersion = parsedLastVersion
46+
? `${parsedLastVersion.major}.${parsedLastVersion.minor}.${parsedLastVersion.patch}`
47+
: null;
48+
4249
// Check if this is an upgrade from a version lower than BIP-44 introduction version
4350
const isUpgradeFromLowerThanBip44Version = Boolean(
44-
lastUpdatedFromVersion &&
45-
typeof lastUpdatedFromVersion === 'string' &&
46-
semverLt(lastUpdatedFromVersion, BIP44_ACCOUNTS_INTRODUCTION_VERSION),
51+
strippedLastVersion &&
52+
semverLt(strippedLastVersion, BIP44_ACCOUNTS_INTRODUCTION_VERSION),
4753
);
4854

4955
// Show modal only for upgrades from versions < BIP-44 introduction version

0 commit comments

Comments
 (0)