Add seed phrase validation for keycard and status-go calls #22269

Parveshdhull · 2025-03-10T10:40:05Z

Summary

Add seed phrase validation for keycard and status-go calls

PR don't need to be manually test, as its only adding small validation check for mnemonics. I will test it manually before merging
(done, video 1, video 2)

status: ready

status-im-auto · 2025-03-10T10:44:09Z

Jenkins Builds

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	`3cef433`	#1	2025-03-10 10:44:09	~3 min	`ios`	📄`log`
✔️	`3cef433`	#1	2025-03-10 10:45:01	~4 min	`tests`	📄`log`
✔️	`3cef433`	#1	2025-03-10 10:48:16	~7 min	`android-e2e`	🤖`apk` 📲
✔️	`3cef433`	#1	2025-03-10 10:49:04	~8 min	`android`	🤖`apk` 📲

❌	`4646ca8`	#2	2025-03-10 14:15:37	~2 min	`ios`	📄`log`
✔️	`4646ca8`	#2	2025-03-10 14:21:00	~8 min	`android`	🤖`apk` 📲
✔️	`4646ca8`	#2	2025-03-10 14:21:17	~8 min	`android-e2e`	🤖`apk` 📲
✔️	`4646ca8`	#2	2025-03-10 14:21:49	~8 min	`tests`	📄`log`

flexsurfer

thank you, left a few comments

flexsurfer · 2025-03-10T11:31:36Z

src/keycard/keycard.cljs

-      (generateAndLoadKey mnemonic pin)
-      (then on-success)
-      (catch on-failure)))
+  (native-module/validate-mnemonic


sorry i know i suggested that, but it's better to move validation outside, keycard ns shouldn't know about native-module

I have moved validation outside, but out of 4, only 2 functions are being used. So I can only apply validation there. Please let me know, if it looks ok.

src/keycard/keycard.cljs

ilmotta · 2025-03-11T03:55:15Z

src/status_im/contexts/keycard/effects.cljs

+ (fn [{:keys [mnemonic on-failure] :as args}]
+   (-> mnemonic
+       (native-module/validate-mnemonic)
+       (promesa/then #(keycard/generate-and-load-key


Wouldn't it be better to add the mnemonic validation directly in the function generate-and-load-key? Otherwise, all devs will need to remember to call it before generate-and-load-key. Or another way to frame the question @Parveshdhull, do you think there's any situation where we shouldn't validate the mnemonic before calling generate-and-load-key?

Same question for the function create-account-from-mnemonic, is there any scenario where we don't want to validate the mnemonic? If the answer is no, then it's better to call native-module/validate-mnemonic inside native-module/create-account-from-mnemonic.

hi @ilmotta, Thank you very much for reviewing the PR.

Wouldn't it be better to add the mnemonic validation directly in the function generate-and-load-key?

This is how I originally implemented validation, please check out #22269 (comment)

status-im-auto · 2025-03-11T10:06:47Z

79% of end-end tests have passed

Total executed tests: 14
Failed tests: 3
Expected to fail tests: 0
Passed tests: 11

IDs of failed tests: 741839,702843,741841

Failed tests (3)

Click to expand

Rerun failed tests

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843

Device 1: ## Creating open community
Device 1: Find `Button` by `accessibility id`: `new-communities-button`

Test setup failed: critical/chats/test_public_chat_browsing.py:339: in prepare_devices
    self.home_1.create_community(community_type="open")
../views/home_view.py:450: in create_community
    self.plus_community_button.click()
../views/base_element.py:89: in click
    element = self.find_element()
../views/base_element.py:78: in find_element
    raise NoSuchElementException(
 Device 1: Button by accessibility id: `new-communities-button` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception

Device sessions

Device 1:

Steps, video, logs

Device 2:

Steps, video, logs

Class TestWalletCollectibles:

1. test_wallet_collectibles_balance, id: 741839
Device 1: `Text` is `Double Spike` Device 1: Click system back button `critical/wallet/test_collectibles.py:94: in test_wallet_collectibles_balance self.errors.verify_no_errors() base_test_case.py:179: in verify_no_errors pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))])) Device 1: BVL image doesn't match expected template E Device 1: Glitch Punks image doesn't match expected template`
2. test_wallet_collectible_send_from_expanded_info_view, id: 741841
Device 1: Find `Button` by `accessibility id`: `collectibles-tab` critical/wallet/test_collectibles.py:149: in test_wallet_collectible_send_from_expanded_info_view self.wallet_view.collectibles_tab.click() ../views/base_element.py:89: in click element = self.find_element() ../views/base_element.py:78: in find_element raise NoSuchElementException( Device 1: Button by accessibility id: `collectibles-tab` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception

Passed tests (11)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_swap_flow_mainnet, id: 741555

2. test_wallet_add_remove_regular_account, id: 727231

3. test_wallet_balance_mainnet, id: 740490

4. test_wallet_bridge_flow_mainnet, id: 741612

5. test_wallet_send_flow_mainnet, id: 741554

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

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 TestWalletCollectibles:

1. test_wallet_send_collectible, id: 741840

Parveshdhull added the skip-manual-qa label Mar 10, 2025

Parveshdhull requested review from ilmotta and flexsurfer March 10, 2025 10:40

Parveshdhull self-assigned this Mar 10, 2025

flexsurfer reviewed Mar 10, 2025

View reviewed changes

Add seed phrase validation for keycard and status-go calls

4646ca8

Parveshdhull force-pushed the fix/add-seed-phrase-validation branch from 3cef433 to 4646ca8 Compare March 10, 2025 14:12

flexsurfer approved these changes Mar 10, 2025

View reviewed changes

ilmotta reviewed Mar 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add seed phrase validation for keycard and status-go calls #22269

Add seed phrase validation for keycard and status-go calls #22269

Parveshdhull commented Mar 10, 2025 •

edited

Loading

status-im-auto commented Mar 10, 2025 •

edited

Loading

flexsurfer left a comment

flexsurfer Mar 10, 2025 •

edited

Loading

Parveshdhull Mar 10, 2025 •

edited

Loading

ilmotta Mar 11, 2025

Parveshdhull Mar 11, 2025

status-im-auto commented Mar 11, 2025

Class TestCommunityMultipleDeviceMerged:

Class TestWalletCollectibles:

Class TestWalletOneDevice:

Class TestCommunityOneDeviceMerged:

Class TestWalletMultipleDevice:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestWalletCollectibles:

Add seed phrase validation for keycard and status-go calls #22269

Are you sure you want to change the base?

Add seed phrase validation for keycard and status-go calls #22269

Conversation

Parveshdhull commented Mar 10, 2025 • edited Loading

Summary

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

Jenkins Builds

flexsurfer left a comment

Choose a reason for hiding this comment

flexsurfer Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

Parveshdhull Mar 10, 2025 • edited Loading

Choose a reason for hiding this comment

ilmotta Mar 11, 2025

Choose a reason for hiding this comment

Parveshdhull Mar 11, 2025

Choose a reason for hiding this comment

status-im-auto commented Mar 11, 2025

79% of end-end tests have passed

Failed tests (3)

Class TestCommunityMultipleDeviceMerged:

Class TestWalletCollectibles:

Passed tests (11)

Class TestWalletOneDevice:

Class TestCommunityOneDeviceMerged:

Class TestWalletMultipleDevice:

Class TestOneToOneChatMultipleSharedDevicesNewUi:

Class TestWalletCollectibles:

Parveshdhull commented Mar 10, 2025 •

edited

Loading

status-im-auto commented Mar 10, 2025 •

edited

Loading

flexsurfer Mar 10, 2025 •

edited

Loading

Parveshdhull Mar 10, 2025 •

edited

Loading