Skip to content

Commit 67b73a5

Browse files
authored
feat: Remove Account Snap Warning (Flask) (#11451)
<!-- 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** 1. What is the reason for the change? - This change adds an extra warning to users when trying to remove an account snap (https://metamask.github.io/snap-simple-keyring/latest/ for example) AND the user has snap accounts in their wallet - Snap accounts are not managed/owned by metamask and therefore cannot be recovered in metamask. User's must manage and backup these accounts in the snaps UI. 2. What is the improvement/solution? - Adds an extra layer of friction when trying to remove an account snap and the user has snap accounts in their wallet - This is a `FLASK ONLY` feature ## **Related issues** Fixes: MetaMask/accounts-planning#155 ## **Manual testing steps** 1. check this branch 2. open `.js.env` 3. ensure that `export METAMASK_BUILD_TYPE=flask` is set to flask (not main) 4. build the app on this branch 5. create/import a wallet 6. inside the mobile browser, open this URL: https://metamask.github.io/snap-simple-keyring/latest/ 7. connect this snap 8. click create account, you can do this as many times as you want. 9. navigate back to the main wallet view 10. NOTICE there should be new accounts in your wallet with the name "Snap Account x" 11. Go to settings 12. navigate to snaps 13. locate the Snap simple keyring settings 14. scroll to the bottom of the page and click the red "Remove" button 15. Notice: new warning should pop up indicating that the user should backup these accounts before removing the snap 16. click continue 17. you should be prompted to type the name of the snap in the text input. 18. YOU SHOULD NOT BE ABLE TO REMOVE THE SNAP WITHOUT TYPING The CORRECT SNAP NAME 19. Once you type the correct snap name, you should be able to remove the snap 20. After clicking remove the snap should be removed. #### Testing a non account snap 1. check this branch 2. open `.js.env` 3. ensure that `export METAMASK_BUILD_TYPE=flask` is set to flask (not main) 4. build the app on this branch 5. create/import a wallet 6. navigate to settings/snaps 7. click on the pre installed Message signing snap 8. click on the remove button at the bottom 9. `THERE SHOULD NOT BE A WARNING` #### Testing removing a snap account without snap accounts in the wallet 1. check this branch 2. open `.js.env` 3. ensure that `export METAMASK_BUILD_TYPE=flask` is set to flask (not main) 4. build the app on this branch 5. create/import a wallet 6. inside the mobile browser, open this URL: https://metamask.github.io/snap-simple-keyring/latest/ 7. connect this snap. Do not create any accounts this time. 8. Go to settings 9. navigate to snaps 10. locate the Snap simple keyring settings 11. scroll to the bottom of the page and click the red "Remove" button 12. There should be no warning pop up and the snap should be removed ## **Screenshots/Recordings** ### Extension Version <img width="367" alt="Screenshot 2024-10-01 at 5 38 58 PM" src="https://github.com/user-attachments/assets/024e03f4-bf38-4062-a040-6eb89b1c39c9"> <img width="363" alt="Screenshot 2024-10-01 at 5 39 14 PM" src="https://github.com/user-attachments/assets/918dff2a-ca80-4bee-8889-28042606fe9f"> <img width="361" alt="Screenshot 2024-10-01 at 5 39 20 PM" src="https://github.com/user-attachments/assets/bf27f6bd-b859-49b8-ba84-3d35c399d77c"> ### **After** with snap accounts: https://github.com/user-attachments/assets/24aa2993-98fa-4d72-bc1f-b7f9c1aeb6ad <img width="825" alt="Screenshot 2024-10-10 at 3 26 46 PM" src="https://github.com/user-attachments/assets/0026de7c-e657-49ad-b6a6-e085ffc05816"> Without snap accounts: https://github.com/user-attachments/assets/7cf42c03-dc6e-40fb-b490-fb0eb4d8ba83 ## **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.
1 parent 475e9bf commit 67b73a5

20 files changed

+1421
-160
lines changed

app/components/Views/AccountActions/AccountActions.tsx

+61-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ import { protectWalletModalVisible } from '../../../actions/user';
3434
import Routes from '../../../constants/navigation/Routes';
3535
import { AccountActionsModalSelectorsIDs } from '../../../../e2e/selectors/Modals/AccountActionsModal.selectors';
3636
import { useMetrics } from '../../../components/hooks/useMetrics';
37-
import { isHardwareAccount } from '../../../util/address';
37+
import {
38+
isHardwareAccount,
39+
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
40+
isSnapAccount,
41+
///: END:ONLY_INCLUDE_IF
42+
} from '../../../util/address';
3843
import { removeAccountsFromPermissions } from '../../../core/Permissions';
3944
import ExtendedKeyringTypes, {
4045
HardwareDeviceTypes,
@@ -189,6 +194,49 @@ const AccountActions = () => {
189194
}
190195
}, [controllers.KeyringController]);
191196

197+
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
198+
199+
/**
200+
* Remove the snap account from the keyring
201+
*/
202+
const removeSnapAccount = useCallback(async () => {
203+
if (selectedAddress) {
204+
await controllers.KeyringController.removeAccount(selectedAddress as Hex);
205+
await removeAccountsFromPermissions([selectedAddress]);
206+
trackEvent(MetaMetricsEvents.ACCOUNT_REMOVED, {
207+
accountType: keyring?.type,
208+
selectedAddress,
209+
});
210+
}
211+
}, [
212+
controllers.KeyringController,
213+
keyring?.type,
214+
selectedAddress,
215+
trackEvent,
216+
]);
217+
218+
const showRemoveSnapAccountAlert = useCallback(() => {
219+
Alert.alert(
220+
strings('accounts.remove_snap_account'),
221+
strings('accounts.remove_snap_account_alert_description'),
222+
[
223+
{
224+
text: strings('accounts.remove_account_alert_cancel_btn'),
225+
style: 'cancel',
226+
},
227+
{
228+
text: strings('accounts.remove_account_alert_remove_btn'),
229+
onPress: async () => {
230+
sheetRef.current?.onCloseBottomSheet(async () => {
231+
await removeSnapAccount();
232+
});
233+
},
234+
},
235+
],
236+
);
237+
}, [removeSnapAccount]);
238+
///: END:ONLY_INCLUDE_IF
239+
192240
/**
193241
* Forget the device if there are no more accounts in the keyring
194242
* @param keyringType - The keyring type
@@ -306,6 +354,18 @@ const AccountActions = () => {
306354
testID={AccountActionsModalSelectorsIDs.REMOVE_HARDWARE_ACCOUNT}
307355
/>
308356
)}
357+
{
358+
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
359+
selectedAddress && isSnapAccount(selectedAddress) && (
360+
<AccountAction
361+
actionTitle={strings('accounts.remove_snap_account')}
362+
iconName={IconName.Close}
363+
onPress={showRemoveSnapAccountAlert}
364+
testID={AccountActionsModalSelectorsIDs.REMOVE_SNAP_ACCOUNT}
365+
/>
366+
)
367+
///: END:ONLY_INCLUDE_IF
368+
}
309369
</View>
310370
<BlockingActionModal
311371
modalVisible={blockingModalVisible}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
2+
export const KEYRING_SNAP_REMOVAL_WARNING = 'keyring-snap-removal-warning';
3+
export const KEYRING_SNAP_REMOVAL_WARNING_CONTINUE =
4+
'keyring-snap-removal-warning-continue';
5+
export const KEYRING_SNAP_REMOVAL_WARNING_CANCEL =
6+
'keyring-snap-removal-warning-cancel';
7+
export const KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT =
8+
'keyring-snap-removal-warning-text-input';
9+
///: END:ONLY_INCLUDE_IF
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
2+
import { StyleSheet } from 'react-native';
3+
import { Theme } from '../../../../util/theme/models';
4+
5+
const styleSheet = (params: { theme: Theme }) => {
6+
const { theme } = params;
7+
const { colors } = theme;
8+
9+
return StyleSheet.create({
10+
bottomSheet: {
11+
flex: 1,
12+
},
13+
container: {
14+
paddingHorizontal: 16,
15+
},
16+
description: {
17+
paddingVertical: 8,
18+
},
19+
buttonContainer: {
20+
paddingTop: 16,
21+
},
22+
input: {
23+
borderWidth: 1,
24+
borderColor: colors.border.default,
25+
borderRadius: 4,
26+
padding: 10,
27+
marginVertical: 10,
28+
},
29+
errorText: {
30+
color: colors.error.default,
31+
},
32+
placeholderText: {
33+
color: colors.text.muted,
34+
},
35+
scrollView: {
36+
flexGrow: 1,
37+
maxHeight: 300,
38+
},
39+
});
40+
};
41+
42+
export default styleSheet;
43+
///: END:ONLY_INCLUDE_IF
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
2+
import React, {
3+
useEffect,
4+
useRef,
5+
useState,
6+
useCallback,
7+
useMemo,
8+
} from 'react';
9+
import { View, TextInput, ScrollView } from 'react-native';
10+
import { NativeViewGestureHandler } from 'react-native-gesture-handler';
11+
import { Snap } from '@metamask/snaps-utils';
12+
import BottomSheet, {
13+
BottomSheetRef,
14+
} from '../../../../component-library/components/BottomSheets/BottomSheet';
15+
import Text, {
16+
TextVariant,
17+
} from '../../../../component-library/components/Texts/Text';
18+
import { InternalAccount } from '@metamask/keyring-api';
19+
import BannerAlert from '../../../../component-library/components/Banners/Banner/variants/BannerAlert';
20+
import { BannerAlertSeverity } from '../../../../component-library/components/Banners/Banner';
21+
import BottomSheetHeader from '../../../../component-library/components/BottomSheets/BottomSheetHeader';
22+
import { useStyles } from '../../../hooks/useStyles';
23+
import stylesheet from './KeyringSnapRemovalWarning.styles';
24+
import { strings } from '../../../../../locales/i18n';
25+
import { KeyringAccountListItem } from '../components/KeyringAccountListItem';
26+
import { getAccountLink } from '@metamask/etherscan-link';
27+
import { useSelector } from 'react-redux';
28+
import { selectProviderConfig } from '../../../../selectors/networkController';
29+
import BottomSheetFooter, {
30+
ButtonsAlignment,
31+
} from '../../../../component-library/components/BottomSheets/BottomSheetFooter';
32+
import {
33+
ButtonProps,
34+
ButtonSize,
35+
ButtonVariants,
36+
} from '../../../../component-library/components/Buttons/Button/Button.types';
37+
import {
38+
KEYRING_SNAP_REMOVAL_WARNING,
39+
KEYRING_SNAP_REMOVAL_WARNING_CANCEL,
40+
KEYRING_SNAP_REMOVAL_WARNING_CONTINUE,
41+
KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT,
42+
} from './KeyringSnapRemovalWarning.constants';
43+
import Logger from '../../../../util/Logger';
44+
45+
interface KeyringSnapRemovalWarningProps {
46+
snap: Snap;
47+
keyringAccounts: InternalAccount[];
48+
onCancel: () => void;
49+
onClose: () => void;
50+
onSubmit: () => void;
51+
}
52+
53+
export default function KeyringSnapRemovalWarning({
54+
snap,
55+
keyringAccounts,
56+
onCancel,
57+
onClose,
58+
onSubmit,
59+
}: KeyringSnapRemovalWarningProps) {
60+
const [showConfirmation, setShowConfirmation] = useState(false);
61+
const [confirmedRemoval, setConfirmedRemoval] = useState(false);
62+
const [confirmationInput, setConfirmationInput] = useState('');
63+
const [error, setError] = useState(false);
64+
const { chainId } = useSelector(selectProviderConfig);
65+
const { styles } = useStyles(stylesheet, {});
66+
const bottomSheetRef = useRef<BottomSheetRef>(null);
67+
68+
useEffect(() => {
69+
setShowConfirmation(keyringAccounts.length === 0);
70+
}, [keyringAccounts]);
71+
72+
const validateConfirmationInput = useCallback(
73+
(input: string): boolean => input === snap.manifest.proposedName,
74+
[snap.manifest.proposedName],
75+
);
76+
77+
const handleConfirmationInputChange = useCallback(
78+
(text: string) => {
79+
setConfirmationInput(text);
80+
setConfirmedRemoval(validateConfirmationInput(text));
81+
},
82+
[validateConfirmationInput],
83+
);
84+
85+
const handleContinuePress = useCallback(() => {
86+
if (!showConfirmation) {
87+
setShowConfirmation(true);
88+
} else if (confirmedRemoval) {
89+
try {
90+
onSubmit();
91+
} catch (e) {
92+
Logger.error(
93+
e as Error,
94+
'KeyringSnapRemovalWarning: error while removing snap',
95+
);
96+
setError(true);
97+
}
98+
}
99+
}, [showConfirmation, confirmedRemoval, onSubmit]);
100+
101+
const cancelButtonProps: ButtonProps = useMemo(
102+
() => ({
103+
variant: ButtonVariants.Secondary,
104+
label: strings(
105+
'app_settings.snaps.snap_settings.remove_account_snap_warning.cancel_button',
106+
),
107+
size: ButtonSize.Lg,
108+
onPress: onCancel,
109+
testID: KEYRING_SNAP_REMOVAL_WARNING_CANCEL,
110+
}),
111+
[onCancel],
112+
);
113+
114+
const continueButtonProps: ButtonProps = useMemo(
115+
() => ({
116+
variant: ButtonVariants.Primary,
117+
label: showConfirmation
118+
? strings(
119+
'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_snap_button',
120+
)
121+
: strings(
122+
'app_settings.snaps.snap_settings.remove_account_snap_warning.continue_button',
123+
),
124+
size: ButtonSize.Lg,
125+
onPress: handleContinuePress,
126+
isDisabled: showConfirmation && !confirmedRemoval,
127+
isDanger: showConfirmation,
128+
testID: KEYRING_SNAP_REMOVAL_WARNING_CONTINUE,
129+
}),
130+
[showConfirmation, confirmedRemoval, handleContinuePress],
131+
);
132+
133+
const buttonPropsArray = useMemo(
134+
() => [cancelButtonProps, continueButtonProps],
135+
[cancelButtonProps, continueButtonProps],
136+
);
137+
138+
const accountListItems = useMemo(
139+
() =>
140+
keyringAccounts.map((account, index) => (
141+
<KeyringAccountListItem
142+
key={index}
143+
account={account}
144+
blockExplorerUrl={getAccountLink(account.address, chainId)}
145+
/>
146+
)),
147+
[keyringAccounts, chainId],
148+
);
149+
150+
return (
151+
<BottomSheet
152+
ref={bottomSheetRef}
153+
isFullscreen={false}
154+
onClose={onClose}
155+
shouldNavigateBack={false}
156+
testID={KEYRING_SNAP_REMOVAL_WARNING}
157+
style={styles.bottomSheet}
158+
>
159+
<View style={styles.container}>
160+
<BottomSheetHeader>
161+
<Text variant={TextVariant.HeadingMD}>
162+
{strings(
163+
'app_settings.snaps.snap_settings.remove_account_snap_warning.title',
164+
)}
165+
</Text>
166+
</BottomSheetHeader>
167+
<BannerAlert
168+
severity={BannerAlertSeverity.Warning}
169+
title={strings(
170+
'app_settings.snaps.snap_settings.remove_account_snap_warning.banner_title',
171+
)}
172+
/>
173+
{showConfirmation ? (
174+
<>
175+
<Text variant={TextVariant.BodyMD} style={styles.description}>
176+
{`${strings(
177+
'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_account_snap_alert_description_1',
178+
)} `}
179+
<Text variant={TextVariant.BodyMDBold}>
180+
{snap.manifest.proposedName}
181+
</Text>
182+
{` ${strings(
183+
'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_account_snap_alert_description_2',
184+
)}`}
185+
</Text>
186+
<TextInput
187+
style={styles.input}
188+
value={confirmationInput}
189+
onChangeText={handleConfirmationInputChange}
190+
testID={KEYRING_SNAP_REMOVAL_WARNING_TEXT_INPUT}
191+
/>
192+
{error && (
193+
<Text variant={TextVariant.BodySM} style={styles.errorText}>
194+
{strings(
195+
'app_settings.snaps.snap_settings.remove_account_snap_warning.remove_snap_error',
196+
{
197+
snapName: snap.manifest.proposedName,
198+
},
199+
)}
200+
</Text>
201+
)}
202+
</>
203+
) : (
204+
<>
205+
<Text variant={TextVariant.BodyMD} style={styles.description}>
206+
{strings(
207+
'app_settings.snaps.snap_settings.remove_account_snap_warning.description',
208+
)}
209+
</Text>
210+
<NativeViewGestureHandler disallowInterruption>
211+
<ScrollView style={styles.scrollView}>
212+
{accountListItems}
213+
</ScrollView>
214+
</NativeViewGestureHandler>
215+
</>
216+
)}
217+
</View>
218+
<BottomSheetFooter
219+
style={styles.buttonContainer}
220+
buttonsAlignment={ButtonsAlignment.Horizontal}
221+
buttonPropsArray={buttonPropsArray}
222+
/>
223+
</BottomSheet>
224+
);
225+
}
226+
///: END:ONLY_INCLUDE_IF
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
///: BEGIN:ONLY_INCLUDE_IF(keyring-snaps)
2+
export { default as KeyringSnapRemovalWarning } from './KeyringSnapRemovalWarning';
3+
///: END:ONLY_INCLUDE_IF

0 commit comments

Comments
 (0)