Skip to content

Commit 28d715f

Browse files
authored
chore: refactor delete wallet into Authentication service (MetaMask#23882)
<!-- 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** This PR refactors wallet deletion logic by consolidating the `useDeleteWallet` hook into the `Authentication` service class. The refactoring improves code organization, ensures consistent wallet deletion behavior, and simplifies the codebase by removing redundant hook logic. **Reason for the change:** 1. The `useDeleteWallet` hook duplicated logic that should be centralized in the `Authentication` service 2. `resetWalletState` and `deleteUser` were always used together, but there was no enforcement of this pattern 3. Having wallet deletion logic in a hook created unnecessary coupling between React components and core authentication logic 4. The hook pattern was not appropriate for core service operations that don't depend on React lifecycle **Improvement/Solution:** 1. **Moved Logic to Authentication Service**: Consolidated `resetWalletState` and `deleteUser` methods into `app/core/Authentication/Authentication.ts` as protected methods 2. **Created Public API**: Added `deleteWallet()` as the main public method that ensures `resetWalletState()` is always called before `deleteUser()` 3. **Protected Methods**: Made `resetWalletState` and `deleteUser` protected to prevent direct external access and enforce the correct usage pattern through `deleteWallet()` 4. **Updated All Call Sites**: Refactored `usePromptSeedlessRelogin` and `DeleteWalletModal` to use the new `Authentication.deleteWallet()` method 5. **Comprehensive Test Coverage**: Added tests for the new `deleteWallet()` method and maintained test coverage for the protected methods **Key Technical Improvements:** - **Removed Hook**: Deleted `app/components/hooks/DeleteWallet/useDeleteWallet.ts` and related test files - **Centralized Logic**: All wallet deletion logic is now in the `Authentication` service, following the single responsibility principle - **Enforced Usage Pattern**: The protected methods ensure that wallet reset and user deletion always happen in the correct sequence - **Better Encapsulation**: Protected methods prevent accidental misuse and make the API surface clearer - **Consistent Behavior**: All wallet deletion flows now use the same underlying implementation ## **Changelog** CHANGELOG entry: Refactored wallet deletion logic by consolidating useDeleteWallet hook into Authentication service ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Wallet Deletion Scenario: Delete wallet from seedless relogin prompt Given the user encounters a seedless controller error And the error prompt is displayed When the user taps the primary button to delete wallet Then the wallet state should be reset And user data should be deleted And the user should be navigated to onboarding And no errors should occur during the deletion process Scenario: Delete wallet from settings Given the user is viewing the Security Settings screen And the user navigates to the Delete Wallet modal When the user confirms wallet deletion Then the wallet state should be reset And user data should be deleted And cookies should be cleared And the user should be navigated to onboarding And analytics should track the deletion event Scenario: Wallet deletion error handling Given the user attempts to delete the wallet When an error occurs during wallet reset Then the error should be logged And the loading state should be reset And the user should see appropriate error feedback And the app should remain in a stable state ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- Before: Wallet deletion logic was split between a hook and the Authentication service - useDeleteWallet hook managed resetWalletState and deleteUser separately - Components had to call both methods in sequence manually - No enforcement that both methods were always called together --> ### **After** https://github.com/user-attachments/assets/7dcb9cc3-cb52-4c50-8f1c-009fa402d1de <!-- After: Wallet deletion is centralized in Authentication service - Single deleteWallet() method ensures correct sequence - Protected methods prevent misuse - Cleaner component code with single method call --> ## **Pre-merge author checklist** - [x] 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). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] 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] > Centralizes wallet deletion into `Authentication.deleteWallet()` and refactors consumers/tests to use it, removing the old `useDeleteWallet` hook. > > - **Core (Authentication)**: > - Add `Authentication.deleteWallet()` as the public API; implements sequential `resetWalletState` → `deleteUser`. > - `resetWalletState` (protected): clears vault backups, disables/re-enables `EngineClass.disableAutomaticVaultBackup`, creates temp wallet, clears seedless state, resets rewards via `Engine.controllerMessenger`, clears provider token, locks app (no navigation). > - `deleteUser` (protected): dispatches `setExistingUser(false)` and triggers `MetaMetrics.createDataDeletionTask`. > - Also removes `OPTIN_META_METRICS_UI_SEEN` and dispatches `setCompletedOnboarding(false)`. > - **UI**: > - `DeleteWalletModal`: replace hook flow with `Authentication.deleteWallet()`, keep sign-out, clear cookies, dispatch `clearHistory`, track event, then navigate to onboarding. > - `usePromptSeedlessRelogin`: invoke `Authentication.deleteWallet()` and simplify flow; maintain loading/error state and `clearHistory` + sign-out + navigation. > - **Hooks**: > - Remove `app/components/hooks/DeleteWallet/*` (hook and tests). > - **Tests**: > - Add comprehensive tests for `deleteWallet`, `resetWalletState`, and `deleteUser` in `Authentication.test.ts`. > - Update component/hook tests to assert calls to `Authentication.deleteWallet()` and revised action sequences; remove assertions tied to deleted hook/storage calls. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b71e98d. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent e630595 commit 28d715f

9 files changed

Lines changed: 468 additions & 358 deletions

File tree

app/components/UI/DeleteWalletModal/index.test.tsx

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,9 @@ import { createStackNavigator } from '@react-navigation/stack';
99
import { RootState } from '../../../reducers';
1010
import { strings } from '../../../../locales/i18n';
1111
import { ForgotPasswordModalSelectorsIDs } from '../../../../e2e/selectors/Common/ForgotPasswordModal.selectors';
12-
import { SET_COMPLETED_ONBOARDING } from '../../../actions/onboarding';
1312
import { InteractionManager } from 'react-native';
14-
import StorageWrapper from '../../../store/storage-wrapper';
15-
import { OPTIN_META_METRICS_UI_SEEN } from '../../../constants/storage';
1613
import { clearHistory } from '../../../actions/browser';
14+
import { Authentication } from '../../../core/Authentication/Authentication';
1715

1816
const mockInitialState = {
1917
engine: { backgroundState },
@@ -22,12 +20,6 @@ const mockInitialState = {
2220
},
2321
};
2422

25-
jest.mock('../../../store/storage-wrapper', () => ({
26-
getItem: jest.fn(),
27-
setItem: jest.fn(),
28-
removeItem: jest.fn(),
29-
}));
30-
3123
const mockUseDispatch = jest.fn();
3224

3325
jest.mock('react-redux', () => ({
@@ -74,11 +66,10 @@ jest.mock('../../../util/identity/hooks/useAuthentication', () => ({
7466
}),
7567
}));
7668

77-
jest.mock('../../hooks/DeleteWallet', () => ({
78-
useDeleteWallet: () => [
79-
jest.fn(() => Promise.resolve()),
80-
jest.fn(() => Promise.resolve()),
81-
],
69+
jest.mock('../../../core/Authentication/Authentication', () => ({
70+
Authentication: {
71+
deleteWallet: jest.fn(() => Promise.resolve()),
72+
},
8273
}));
8374

8475
const Stack = createStackNavigator();
@@ -153,7 +144,6 @@ describe('DeleteWalletModal', () => {
153144
});
154145

155146
it('signs the user out when deleting the wallet', async () => {
156-
const removeItemSpy = jest.spyOn(StorageWrapper, 'removeItem');
157147
const { getByTestId } = renderComponent(mockInitialState);
158148

159149
fireEvent.press(
@@ -164,11 +154,9 @@ describe('DeleteWalletModal', () => {
164154
);
165155

166156
expect(mockSignOut).toHaveBeenCalled();
167-
expect(removeItemSpy).toHaveBeenCalledWith(OPTIN_META_METRICS_UI_SEEN);
168157
});
169158

170-
it('sets completedOnboarding to false when deleting the wallet', async () => {
171-
const removeItemSpy = jest.spyOn(StorageWrapper, 'removeItem');
159+
it('calls deleteWallet when deleting the wallet', async () => {
172160
const { getByTestId } = renderComponent(mockInitialState);
173161

174162
fireEvent.press(
@@ -178,13 +166,10 @@ describe('DeleteWalletModal', () => {
178166
getByTestId(ForgotPasswordModalSelectorsIDs.YES_RESET_WALLET_BUTTON),
179167
);
180168

181-
expect(mockUseDispatch).toHaveBeenCalledWith(
182-
expect.objectContaining({
183-
type: SET_COMPLETED_ONBOARDING,
184-
completedOnboarding: false,
185-
}),
186-
);
187-
expect(removeItemSpy).toHaveBeenCalledWith(OPTIN_META_METRICS_UI_SEEN);
169+
// Wait for async operations
170+
await Promise.resolve();
171+
172+
expect(Authentication.deleteWallet).toHaveBeenCalled();
188173
});
189174
});
190175

app/components/UI/DeleteWalletModal/index.tsx

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,13 @@ import Icon, {
77
IconColor,
88
} from '../../../component-library/components/Icons/Icon';
99
import { createStyles } from './styles';
10-
import { useDeleteWallet } from '../../hooks/DeleteWallet';
10+
import { Authentication } from '../../../core';
1111
import { strings } from '../../../../locales/i18n';
1212
import { useTheme } from '../../../util/theme';
1313
import Device from '../../../util/device';
1414
import Routes from '../../../constants/navigation/Routes';
1515
import { ForgotPasswordModalSelectorsIDs } from '../../../../e2e/selectors/Common/ForgotPasswordModal.selectors';
1616
import { IMetaMetricsEvent, MetaMetricsEvents } from '../../../core/Analytics';
17-
import { setCompletedOnboarding } from '../../../actions/onboarding';
1817
import { useDispatch, useSelector } from 'react-redux';
1918
import { clearHistory } from '../../../actions/browser';
2019
import CookieManager from '@react-native-cookies/cookies';
@@ -38,8 +37,6 @@ import { useMetrics } from '../../hooks/useMetrics';
3837
import ButtonIcon, {
3938
ButtonIconSizes,
4039
} from '../../../component-library/components/Buttons/ButtonIcon';
41-
import StorageWrapper from '../../../store/storage-wrapper';
42-
import { OPTIN_META_METRICS_UI_SEEN } from '../../../constants/storage';
4340

4441
if (Device.isAndroid() && UIManager.setLayoutAnimationEnabledExperimental) {
4542
UIManager.setLayoutAnimationEnabledExperimental(true);
@@ -62,7 +59,6 @@ const DeleteWalletModal: React.FC = () => {
6259

6360
const [isResetWallet, setIsResetWallet] = useState<boolean>(false);
6461

65-
const [resetWalletState, deleteUser] = useDeleteWallet();
6662
const dispatch = useDispatch();
6763
const isDataCollectionForMarketingEnabled = useSelector(
6864
(state: RootState) => state.security.dataCollectionForMarketing,
@@ -115,10 +111,7 @@ const DeleteWalletModal: React.FC = () => {
115111
dispatch(clearHistory(isEnabled(), isDataCollectionForMarketingEnabled));
116112
signOut();
117113
await CookieManager.clearAll(true);
118-
await resetWalletState();
119-
await deleteUser();
120-
await StorageWrapper.removeItem(OPTIN_META_METRICS_UI_SEEN);
121-
dispatch(setCompletedOnboarding(false));
114+
await Authentication.deleteWallet();
122115
// Track analytics for successful deletion
123116
track(MetaMetricsEvents.RESET_WALLET_CONFIRMED, {});
124117
InteractionManager.runAfterInteractions(() => {

app/components/hooks/DeleteWallet/index.ts

Lines changed: 0 additions & 1 deletion
This file was deleted.

app/components/hooks/DeleteWallet/useDeleteWallet.test.tsx

Lines changed: 0 additions & 199 deletions
This file was deleted.

app/components/hooks/DeleteWallet/useDeleteWallet.ts

Lines changed: 0 additions & 62 deletions
This file was deleted.

0 commit comments

Comments
 (0)