[#21578] Keycard - Factory reset a Keycard #21914

flexsurfer · 2025-01-10T11:20:04Z

status-im-auto · 2025-01-10T11:26:02Z

Jenkins Builds

Click to see older builds (16)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`150aa98`	#1	2025-01-10 11:26:01	~5 min	`tests`	📄`log`
✔️	`150aa98`	#1	2025-01-10 11:28:09	~7 min	`ios`	📱`ipa` 📲
✔️	`150aa98`	#1	2025-01-10 11:28:14	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`150aa98`	#1	2025-01-10 11:30:12	~9 min	`android`	🤖`apk` 📲

✔️	`9ef178b`	#2	2025-01-14 07:52:34	~4 min	`tests`	📄`log`
✔️	`9ef178b`	#2	2025-01-14 07:54:50	~7 min	`ios`	📱`ipa` 📲
✔️	`9ef178b`	#2	2025-01-14 07:56:38	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`9ef178b`	#2	2025-01-14 07:57:11	~9 min	`android`	🤖`apk` 📲

✔️	`e24261c`	#3	2025-01-16 12:24:47	~4 min	`tests`	📄`log`
✔️	`e24261c`	#3	2025-01-16 12:28:55	~9 min	`android-e2e`	🤖`apk` 📲
✔️	`e24261c`	#3	2025-01-16 12:29:35	~9 min	`android`	🤖`apk` 📲
✔️	`e24261c`	#3	2025-01-16 12:31:29	~11 min	`ios`	📱`ipa` 📲

✔️	`1b6bf2e`	#5	2025-01-17 09:05:07	~4 min	`tests`	📄`log`
✔️	`1b6bf2e`	#5	2025-01-17 09:09:02	~8 min	`android-e2e`	🤖`apk` 📲
✔️	`1b6bf2e`	#5	2025-01-17 09:09:35	~9 min	`android`	🤖`apk` 📲
✔️	`1b6bf2e`	#5	2025-01-17 09:10:54	~10 min	`ios`	📱`ipa` 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`787916c`	#6	2025-01-17 13:30:54	~4 min	`tests`	📄`log`
✔️	`787916c`	#6	2025-01-17 13:33:41	~7 min	`android`	🤖`apk` 📲
✔️	`787916c`	#6	2025-01-17 13:34:31	~8 min	`android-e2e`	🤖`apk` 📲
✔️	`787916c`	#6	2025-01-17 13:36:18	~10 min	`ios`	📱`ipa` 📲

❌	`5ba2af9`	#7	2025-01-20 09:58:33	~1 min	`ios`	📄`log`
✔️	`5ba2af9`	#7	2025-01-20 10:02:05	~4 min	`tests`	📄`log`
✔️	`5ba2af9`	#7	2025-01-20 10:05:04	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`5ba2af9`	#7	2025-01-20 10:06:11	~9 min	`android`	🤖`apk` 📲
✔️	`5ba2af9`	#8	2025-01-20 11:09:50	~10 min	`ios`	📱`ipa` 📲

Parveshdhull

Thank you @flexsurfer for the PR. Looks good to me.

src/status_im/contexts/keycard/create/events.cljs

src/status_im/navigation/screens.cljs

status-im-auto · 2025-01-13T07:48:45Z

88% of end-end tests have passed

Total executed tests: 8
Failed tests: 1
Expected to fail tests: 0
Passed tests: 7

IDs of failed tests: 727230

Failed tests (1)

Click to expand

Rerun failed tests

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230

Device 1: Find `Text` by `xpath`: `//android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='Ether']/../android.widget.TextView[3]`
Device 1: `Text` is `0.315528 ETH`

critical/test_wallet.py:200: in test_wallet_send_asset_from_drawer
    self.errors.verify_no_errors()
base_test_case.py:176: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Eth amount in the shdUaM8M6QcxQ4qn32nQ's wallet is 0.3155 but should be 0.3154

Passed tests (7)

Click to expand

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231

2. test_wallet_balance_mainnet, id: 740490

Class TestWalletMultipleDevice:

1. test_wallet_send_eth, id: 727229

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions Device 1: Steps, video, logs
2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions Device 1: Steps, video, logs

flexsurfer · 2025-01-15T07:19:33Z

hey @pavloburykh lets pause this PR for now, i'll be back soon with updates, thank you

flexsurfer · 2025-01-15T07:35:19Z

thank you @ilmotta all point are valid, I rushed this pull request little bit, it needs more work, also about description you are right i'll change it

flexsurfer · 2025-01-15T09:21:40Z

i've implemented only Factory reset Keycard with other / unknown key pair flow in this PR, so i need to implement both in this PR, will be back soon with the update, and will update a description, thank you for your patience

flexsurfer · 2025-01-15T09:26:54Z

Issue 1. User is locked out after successful reset

After successfully resetting a keycard, the user is still asked to login with the keycard after logout, which prevents the user from logging in because the card was effectively reset.

this is by design, cc @xAlisher

flexsurfer · 2025-01-15T09:28:18Z

Issue 3. Screen "Failed to reset Keycard" not implemented as in Figma

Failed to reset keycard screen doesn't show tips at the bottom.

@xAlisher can we change this in design? it will simplify development, for user there will be no difference, on try again we just move it to screen there he have all the instructions and can try again

flexsurfer · 2025-01-15T09:29:57Z

Issue 5. Bottom sheet to factory reset is different from Figma

Figma: https://www.figma.com/design/YGm3igIOAcwMqUVJWCJ6f1/Keycard?node-id=3039-58204&m=dev

Differences:

Missing card name

Different paragraph text and checkbox text. Which one is right? Maybe Figma is outdated? Which Figma node did you base the implementation on?

it's because i implemented different flow, i don't check if keycard is for current profile, i'll add this condition

flexsurfer · 2025-01-15T11:16:57Z

Issue 1. User is locked out after successful reset
by design, discuss with @xAlisher
Issue 2. Successful reset button shouldn't be "Done"
implement profile flow or change design
Issue 3. Screen "Failed to reset Keycard" not implemented as in Figma
change design
Issue 4. Biometrics are still active after profile migration to keycard
not related to this PR
Issue 5. Bottom sheet to factory reset is different from Figma
by design, this flow is from scan keycard, so it can be any card we don't check if it's profile keycard
ISSUE 6 'It's a different keycard` flow and screen is not implemented
bug

flexsurfer · 2025-01-15T13:01:36Z

Successful reset button shouldn't be "Done" this seems should also be fixed in design, there is no sense to logout user cc @xAlisher

flexsurfer · 2025-01-16T11:37:22Z

@pavloburykh i checked , card is NOT reseted, not empty error screen is shown, so there is nothing to fix it seems, ready for testing

status-im-auto · 2025-01-16T11:50:31Z

100% of end-end tests have passed

Total executed tests: 8
Failed tests: 0
Expected to fail tests: 0
Passed tests: 8

Passed tests (8)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231

2. test_wallet_balance_mainnet, id: 740490

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230

2. test_wallet_send_eth, id: 727229

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestCommunityOneDeviceMerged:

1. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions Device 1: Steps, video, logs
2. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions Device 1: Steps, video, logs

pavloburykh · 2025-01-16T12:10:30Z

@pavloburykh i checked , card is NOT reseted, not empty error screen is shown, so there is nothing to fix it seems, ready for testing

@flexsurfer my bad. I have rechecked and see that it is not reset indeed.

flexsurfer · 2025-01-16T12:20:46Z

@pavloburykh added 'It's a different keycard` screen

flexsurfer · 2025-01-17T08:57:01Z

hey @pavloburykh , after discussing with @xAlisher Facatory reset Keycard with Profile key pair flow was removed, PR is ready for review

pavloburykh · 2025-01-17T09:06:40Z

hey @pavloburykh , after discussing with @xAlisher Facatory reset Keycard with Profile key pair flow was removed, PR is ready for review

Thank you @flexsurfer! Got a question: according to the new design, after successful keycard reset user is redirected to the Keycard home screen with no Keycard linked https://www.figma.com/design/YGm3igIOAcwMqUVJWCJ6f1/Keycard?node-id=5595-70886&t=hAbB2p3cLP39ZmIX-0

But what if I have reseted the keycard with another profile on it and I still have another keycard linked to current profile? Shouldn't we redirect to the Keycard screen showing this linked keycard? This screen has been removed for some reason.

flexsurfer · 2025-01-17T09:10:25Z

hey @pavloburykh sorry for confusion, @xAlisher is working on this and this is a separate issue not related to this PR

status-im-auto · 2025-01-17T14:43:48Z

88% of end-end tests have passed

Total executed tests: 8
Failed tests: 1
Expected to fail tests: 0
Passed tests: 7

IDs of failed tests: 702843

Failed tests (1)

Click to expand

Rerun failed tests

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device 1: Wait for text element `EmojisNumber` to be equal to `1` Device 1: Find `EmojisNumber` by `xpath`: `//[starts-with(@text,'Message AFTER edit 2 (Edited)')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']/../..//[@content-desc='emoji-reaction-2']/android.widget.TextView[2]` `critical/chats/test_public_chat_browsing.py:380: in test_community_message_edit self.errors.verify_no_errors() base_test_case.py:176: in verify_no_errors pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))])) Message reaction is not shown for the sender` Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Passed tests (7)

Click to expand

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230

2. test_wallet_send_eth, id: 727229

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions Device 1: Steps, video, logs
2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions Device 1: Steps, video, logs

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231

2. test_wallet_balance_mainnet, id: 740490

pavloburykh · 2025-01-17T15:37:32Z

Thanks @flexsurfer couple of questions and issues:

Issue 3. Screen "Failed to reset Keycard" not implemented as in Figma
change design

What does change design mean? Issue is still valid. Current implementation misses text which is present in Figma.

pavloburykh · 2025-01-17T15:37:55Z

ISSUE 7 Scanning not a keycard redirects to `Failed to reset keycard` instead of 'This is not a keycard` screen

Steps:

Login profile which is migrated to keycard
Go to Profile - Keycard - Ready to scan
Scan keycard
During second scan use not a keycard
Observe the result

Actual result: user is redirected to Failed to reset keycard

Expected result: user is redirected to 'This is not a keycard`

pavloburykh · 2025-01-17T15:38:09Z

ISSUE 8 Scanning different key card (which is not empty) during Create Profile flow results in stuck on Preparing screen or overwriting keys on this keycard

Steps:

Start Creating Profile by scanning Keycard A
During second (final) scan use Keycard B which is not empty
Observe the result

Actual result:

Sometimes user stucks on Preparing screen
Sometimes profile is created successfully and Keycard B keys are overwritten

Expected result: It's a different key card screen should be shown. Or at least, until this screen is implemented for this flow, we should show Keycard not empty screen.

pavloburykh · 2025-01-17T16:46:42Z

@ilmotta some updates on this PR:

Following discussions between @flexsurfer and @xAlisher, it was decided to completely remove the separate Keycard reset flow for cases where the Keycard contains the current Profile keys (@flexsurfer can expand on this if needed). From now on, the reset flow will be unified, regardless of whether the Keycard holds current Profile keys or Unknown/Other keys. You can find the updated flow here.

Therefore, the following issues are not valid anymore:

Issue 2. Successful reset button shouldn't be "Done" - according to new design it will always be Done now and there is no logout button anymore since it was only for the case when Keycard contained current Profile keys on it.
Issue 5. Bottom sheet to factory reset is different from Figma - designs have been updated, and app is aligned with designs now.

Issues which are valid but should be logged/discussed separately:

Issue 4. Biometrics are still active after profile migration to keycard
Issue 1. User is locked out after successful reset - this one should be discussed separately with @xAlisher. According to the new design after successful migration user is not logged out anymore but redirected to the Keycard home screen Doesn't seem it somehow fixes the problem, as user will still be locked out after performing manual logout. Although, it worth mentioning that in the updated text in Factory reset bottom sheet we explain user all the consequences of resetting Keycard which contains current Profile keys.

IMPORTANT: as a part of recent changes, Alisher has updated Keycard home screen BUT current PR does not reflect these changes and according to @flexsurfer they will be implemented in a separate PR. Current PR uses the old screen.

ilmotta · 2025-01-17T18:00:01Z

Thanks @pavloburykh for the detailed heads up 💯

Issue 1. User is locked out after successful reset - this one should be discussed separately with @xAlisher.

The new text does improve things and it seems to be sufficient for now. As long as we are being upfront and straightforward about the actions that will take place after reset, I think the current UX & copy is sufficient. Over time we will be able to get good feedback from users, and maybe they will have something to say about this part of the flow.

Issue 4. Biometrics are still active after profile migration to keycard

Sure, this one I mentioned originally to @flexsurfer that it is unrelated to this PR. Can be tracked as a separate GH issue 👍🏼

ilmotta

Approving since I don't have anything else to add to the PR code-wise, code LGTM.

ilmotta · 2025-01-17T18:18:11Z

src/status_im/contexts/keycard/factory_reset/view.cljs

+    {:title            (i18n/label :t/keycard-reset-success)
+     :description      :text
+     :description-text (i18n/label :t/keycard-empty-ready)}]
+   [rn/view {:style {:flex 1}}]


Quick one, what is this rn/view for? It doesn't have any content or size, that's why I'm asking. File src/status_im/contexts/keycard/different_card/view.cljs also has this same empty rn/view.

@flexsurfer probably my comment got lost in the myriad of comments in the PR, but could you elaborate on my question above?

oh sorry @ilmotta that true, I missed your comment, this view just pushes next view to the bottom

Ah okay it's really necessary. Thanks for the explanation!

…nstead of 'This is not a keycard` screen

flexsurfer · 2025-01-20T09:57:34Z

thank you @pavloburykh
Issue 3. will be fixed in design
ISSUE 7 fixed
ISSUE 8 lets do it as separate issue

pavloburykh · 2025-01-20T11:32:29Z

@flexsurfer thanks for the PR. Ready for merge now.

flexsurfer added request-manual-qa keycard labels Jan 10, 2025

flexsurfer requested review from seanstrom and Parveshdhull January 10, 2025 11:20

flexsurfer self-assigned this Jan 10, 2025

Parveshdhull approved these changes Jan 10, 2025

View reviewed changes

src/status_im/contexts/keycard/create/events.cljs Outdated Show resolved Hide resolved

src/status_im/navigation/screens.cljs Show resolved Hide resolved

flexsurfer force-pushed the feature/Keycard-Factory-reset-a-Keycard-#21578 branch from 150aa98 to 9ef178b Compare January 14, 2025 07:47

pavloburykh self-assigned this Jan 14, 2025

pavloburykh added the test cases required label Jan 14, 2025

ilmotta self-requested a review January 15, 2025 00:43

This comment was marked as resolved.

Sign in to view

pavloburykh added the Tested - Issues label Jan 15, 2025

flexsurfer removed the Tested - Issues label Jan 16, 2025

flexsurfer force-pushed the feature/Keycard-Factory-reset-a-Keycard-#21578 branch from 9ef178b to e24261c Compare January 16, 2025 12:19

ilmotta approved these changes Jan 17, 2025

View reviewed changes

flexsurfer added 2 commits January 20, 2025 10:45

[#21578] Keycard - Factory reset a Keycard

89efbf4

ISSUE 7 Scanning not a keycard redirects to Failed to reset keycard i…

5ba2af9

…nstead of 'This is not a keycard` screen

flexsurfer force-pushed the feature/Keycard-Factory-reset-a-Keycard-#21578 branch from 787916c to 5ba2af9 Compare January 20, 2025 09:56

pavloburykh added the Tested - OK label Jan 20, 2025

flexsurfer merged commit be13473 into develop Jan 20, 2025
5 checks passed

flexsurfer deleted the feature/Keycard-Factory-reset-a-Keycard-#21578 branch January 20, 2025 11:35

pavloburykh mentioned this pull request Jan 20, 2025

Biometrics are still active after profile migration to keycard #21956

Open

[#21578] Keycard - Factory reset a Keycard #21914

[#21578] Keycard - Factory reset a Keycard #21914

Conversation

flexsurfer commented Jan 10, 2025 • edited Loading

status-im-auto commented Jan 10, 2025 • edited Loading

Jenkins Builds

Parveshdhull left a comment

Choose a reason for hiding this comment

status-im-auto commented Jan 13, 2025

88% of end-end tests have passed

Failed tests (1)

Class TestWalletMultipleDevice:

Passed tests (7)

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestWalletOneDevice:

Class TestWalletMultipleDevice:

Class TestCommunityMultipleDeviceMerged:

Class TestCommunityOneDeviceMerged:

This comment was marked as resolved.

flexsurfer commented Jan 15, 2025 • edited Loading

flexsurfer commented Jan 15, 2025 • edited Loading

This comment was marked as resolved.

flexsurfer commented Jan 15, 2025 • edited Loading

flexsurfer commented Jan 15, 2025

Issue 1. User is locked out after successful reset

flexsurfer commented Jan 15, 2025 • edited Loading

Issue 3. Screen "Failed to reset Keycard" not implemented as in Figma

flexsurfer commented Jan 15, 2025

Issue 5. Bottom sheet to factory reset is different from Figma

flexsurfer commented Jan 15, 2025 • edited Loading

flexsurfer commented Jan 15, 2025

flexsurfer commented Jan 16, 2025

status-im-auto commented Jan 16, 2025

100% of end-end tests have passed

Passed tests (8)

Class TestWalletOneDevice:

Class TestWalletMultipleDevice:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestCommunityMultipleDeviceMerged:

Class TestCommunityOneDeviceMerged:

pavloburykh commented Jan 16, 2025

flexsurfer commented Jan 16, 2025

flexsurfer commented Jan 17, 2025

pavloburykh commented Jan 17, 2025

flexsurfer commented Jan 17, 2025

status-im-auto commented Jan 17, 2025

88% of end-end tests have passed

Failed tests (1)

Class TestCommunityMultipleDeviceMerged:

Passed tests (7)

Class TestWalletMultipleDevice:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestCommunityOneDeviceMerged:

Class TestWalletOneDevice:

pavloburykh commented Jan 17, 2025 • edited Loading

pavloburykh commented Jan 17, 2025

ISSUE 7 Scanning not a keycard redirects to Failed to reset keycard instead of 'This is not a keycard` screen

pavloburykh commented Jan 17, 2025

ISSUE 8 Scanning different key card (which is not empty) during Create Profile flow results in stuck on Preparing screen or overwriting keys on this keycard

pavloburykh commented Jan 17, 2025

ilmotta commented Jan 17, 2025

ilmotta left a comment

Choose a reason for hiding this comment

ilmotta Jan 17, 2025

Choose a reason for hiding this comment

ilmotta Jan 20, 2025

Choose a reason for hiding this comment

flexsurfer Jan 20, 2025

Choose a reason for hiding this comment

ilmotta Jan 20, 2025

Choose a reason for hiding this comment

flexsurfer commented Jan 20, 2025

pavloburykh commented Jan 20, 2025

flexsurfer commented Jan 10, 2025 •

edited

Loading

status-im-auto commented Jan 10, 2025 •

edited

Loading

flexsurfer commented Jan 15, 2025 •

edited

Loading

flexsurfer commented Jan 15, 2025 •

edited

Loading

flexsurfer commented Jan 15, 2025 •

edited

Loading

flexsurfer commented Jan 15, 2025 •

edited

Loading

flexsurfer commented Jan 15, 2025 •

edited

Loading

pavloburykh commented Jan 17, 2025 •

edited

Loading

ISSUE 7 Scanning not a keycard redirects to `Failed to reset keycard` instead of 'This is not a keycard` screen