Skip to content

Commit 20b69b3

Browse files
authored
fix: do not display Snap account dialogs for multichain wallet Snaps (MetaMask#23218)
## **Description** Similar fix (technical details on this PR too): - MetaMask/metamask-extension#38061 ## **Changelog** CHANGELOG entry: Prevent any dialogs for multichain wallet Snaps (Solana, Bitcoin, Tron) ## **Related issues** Fixes: - MetaMask#22465 ## **Manual testing steps** ```gherkin Feature: my feature name Scenario: user [verb for user action] Given [describe expected initial app state] When user [verb for user action] Then [describe expected outcome] ``` ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [ ] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Skip Snap account dialogs and naming for preinstalled multichain wallet Snaps under state 2, with E2E-safe fallback and updated tests. > > - **Core/SnapKeyring**: > - Enhance `addAccount` flow to auto-skip dialogs for preinstalled multichain wallet Snaps when multichain accounts state 2 is enabled. > - Introduce `isMultichainWalletSnap` and `isE2E` checks; derive `skipAll` to bypass confirmation, name suggestion, and selection steps. > - Set `accountNameSuggestion` to ``''`` when skipping to avoid race conditions; update `skipApprovalFlow` logic accordingly. > - Preserve legacy behavior in E2E runs by disabling skips. > - **Tests**: > - Mock `isMultichainWalletSnap` and extend scenarios for state 2, verifying dialog skipping and naming behavior. > - Minor adjustments to expectations around flows and account naming/selection. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit fd81303. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 451d0b9 commit 20b69b3

2 files changed

Lines changed: 35 additions & 10 deletions

File tree

app/core/SnapKeyring/SnapKeyring.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ const createSnapKeyringBuilder = () =>
167167
// Mock the isSnapPreinstalled function
168168
jest.mock('./utils/snaps', () => ({
169169
isSnapPreinstalled: jest.fn(),
170+
isMultichainWalletSnap: jest.fn().mockReturnValue(false),
170171
getSnapName: jest.fn().mockReturnValue('Mock Snap Name'),
171172
}));
172173

app/core/SnapKeyring/SnapKeyring.ts

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,14 +10,19 @@ import { SnapKeyringBuilderMessenger } from './types';
1010
import { SnapId } from '@metamask/snaps-sdk';
1111
import { assertIsValidSnapId } from '@metamask/snaps-utils';
1212
import { getUniqueAccountName } from './utils/getUniqueAccountName';
13-
import { getSnapName, isSnapPreinstalled } from './utils/snaps';
13+
import {
14+
getSnapName,
15+
isMultichainWalletSnap,
16+
isSnapPreinstalled,
17+
} from './utils/snaps';
1418
import { endTrace, TraceName } from '../../util/trace';
1519
import { store } from '../../store';
1620
import { MetaMetricsEvents } from '../../core/Analytics/MetaMetrics.events';
1721
import { trackSnapAccountEvent } from '../Analytics/helpers/SnapKeyring/trackSnapAccountEvent';
1822
import { endPerformanceTrace } from '../../core/redux/slices/performance';
1923
import { PerformanceEventNames } from '../redux/slices/performance/constants';
2024
import { areAddressesEqual } from '../../util/address';
25+
import { isE2E } from '../../util/test/utils';
2126
import { isMultichainAccountsState2Enabled } from '../../multichain-accounts/remote-feature-flag';
2227

2328
/**
@@ -246,28 +251,47 @@ class SnapKeyringImpl implements SnapKeyringCallbacks {
246251
) {
247252
assertIsValidSnapId(snapId);
248253

254+
// Preinstalled Snaps can skip some confirmation dialogs.
255+
const isPreinstalled = isSnapPreinstalled(snapId);
256+
257+
// Since the introduction of BIP-44, multichain wallet Snaps will skip them automatically too!
258+
let skipAll =
259+
isMultichainAccountsState2Enabled() &&
260+
isPreinstalled &&
261+
isMultichainWalletSnap(snapId);
262+
// FIXME: We still rely on the old behavior in some e2e, so we do not skip them in this case.
263+
if (isE2E) {
264+
skipAll = false;
265+
}
266+
267+
// If Snap is preinstalled and does not request confirmation, skip the confirmation dialog.
268+
const skipConfirmationDialog =
269+
skipAll || (isPreinstalled && !displayConfirmation);
270+
249271
// Only pre-installed Snaps can skip the account name suggestion dialog.
250272
const skipAccountNameSuggestionDialog =
251-
isSnapPreinstalled(snapId) && !displayAccountNameSuggestion;
273+
skipAll || (isPreinstalled && !displayAccountNameSuggestion);
274+
275+
// Only pre-installed Snaps can skip the account from being selected.
276+
const skipSetSelectedAccountStep =
277+
skipAll || (isPreinstalled && !setSelectedAccount);
278+
252279
const skipApprovalFlow =
253-
isSnapPreinstalled(snapId) &&
254-
skipAccountNameSuggestionDialog &&
255-
!displayConfirmation;
280+
skipConfirmationDialog && skipAccountNameSuggestionDialog;
256281

257282
// First part of the flow, which includes confirmation dialogs (if not skipped).
258283
// Once confirmed, we resume the Snap execution.
259284
const { accountName } = await this.addAccountConfirmations({
260285
snapId,
261-
accountNameSuggestion,
286+
// We do not set the account name suggestion if it's a multichain wallet Snap since the
287+
// current naming could have race conditions with other account creations, and since
288+
// naming is now handled by multichain account groups, we can skip this entirely.
289+
accountNameSuggestion: skipAll ? '' : accountNameSuggestion,
262290
handleUserInput,
263291
skipAccountNameSuggestionDialog,
264292
skipApprovalFlow,
265293
});
266294

267-
// Only pre-installed Snaps can skip the account from being selected.
268-
const skipSetSelectedAccountStep =
269-
isSnapPreinstalled(snapId) && !setSelectedAccount;
270-
271295
// The second part is about selecting the newly created account and showing some other
272296
// confirmation dialogs (or error dialogs if anything goes wrong while persisting the account
273297
// into the state.

0 commit comments

Comments
 (0)