Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix_: Adding own wallet address as saved addresses #20839

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

smohamedjavid
Copy link
Member

fixes #20702
fixes #20751

Summary

This PR

  • prevents the user from saving their wallet address as the saved address
  • fixes button not capturing taps when the keyboard is open

Review notes

The bug is caused by the validation failure when using a multi-chain prefix. This PR updates the validation method to extract and validate the address.

Platforms

  • Android
  • iOS

Steps to test

  • Open Status
  • Navigate to the Wallet tab or Open share screen
  • Copy the address of your wallet account address with multi-chain prefixes
  • Navigate to Profile > Wallet > Saved addresses and tap on +
  • Paste the address into the input field
  • Verify the error is thrown
  • Copy any of the saved addresses with multi-chain prefixes
  • Paste the address into the input field
  • Verify the error is thrown
  • Verify the paste button and [-] button are tappable when the keyboard is open

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Jul 22, 2024

Jenkins Builds

Click to see older builds (4)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 501fe7a #1 2024-07-22 10:05:56 ~5 min tests 📄log
✔️ 501fe7a #1 2024-07-22 10:06:51 ~6 min android 🤖apk 📲
✔️ 501fe7a #1 2024-07-22 10:07:47 ~7 min android-e2e 🤖apk 📲
✔️ 501fe7a #1 2024-07-22 10:09:32 ~9 min ios 📱ipa 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 0ce2680 #2 2024-07-23 10:48:08 ~4 min tests 📄log
✔️ 0ce2680 #2 2024-07-23 10:51:27 ~8 min android 🤖apk 📲
✔️ 0ce2680 #2 2024-07-23 10:51:40 ~8 min android-e2e 🤖apk 📲
✔️ 0ce2680 #2 2024-07-23 10:57:35 ~14 min ios 📱ipa 📲
✔️ a2ce5e3 #3 2024-07-24 12:27:47 ~6 min android-e2e 🤖apk 📲
✔️ a2ce5e3 #3 2024-07-24 12:29:38 ~8 min tests 📄log
✔️ a2ce5e3 #3 2024-07-24 12:30:40 ~9 min android 🤖apk 📲
✔️ a2ce5e3 #3 2024-07-24 12:31:17 ~10 min ios 📱ipa 📲

Copy link
Contributor

@ulisesmac ulisesmac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @smohamedjavid

Looks great!

@smohamedjavid smohamedjavid force-pushed the fix/saved-addresses-fixes branch from 501fe7a to 0ce2680 Compare July 23, 2024 10:42
@status-im-auto
Copy link
Member

86% of end-end tests have passed

Total executed tests: 7
Failed tests: 0
Expected to fail tests: 1
Passed tests: 6
IDs of expected to fail tests: 727232 

Expected to fail tests (1)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_add_remove_watch_only_account, id: 727232

Device 1: Find `EditBox` by `accessibility id`: `add-address-to-watch`
Device 1: Type `0x8d2413447ff297d30bdc475f6d5cb00254685aae` to `EditBox`

critical/test_wallet.py:249: in test_wallet_add_remove_watch_only_account
    self.wallet_view.add_watch_only_account(address=address_to_watch, account_name=new_account_name)
../views/wallet_view.py:163: in add_watch_only_account
    self.account_has_activity_label.wait_for_visibility_of_element()
../views/base_element.py:147: in wait_for_visibility_of_element
    raise TimeoutException(
 Device 1: Text by accessibility id:`account-has-activity` is not found on the screen after wait_for_visibility_of_element 

[[Missing networks in account address, https://github.com//issues/20166]]

Device sessions

Passed tests (6)

Click to expand

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
Device sessions

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
Device sessions

2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

@VolodLytvynenko VolodLytvynenko self-assigned this Jul 23, 2024
@VolodLytvynenko
Copy link
Contributor

hi @smohamedjavid thank you for PR.

I'm not sure if this is an issue. In my opinion, watch-only addresses should be able to be added as saved . WDYT? However, this is a very low priority issue and it's better not to waste time on it. The PR can be merged. Let me know if this issue is decided to be fixed in this PR, and I can recheck it.

PR_ISSUE 1: Watch-only addresses can't be saved

Steps:

  1. Go to wallet
  2. Add watch only account
  3. Go to Profile -> wallet -> saved addresses
  4. Attempt to save the watch-only address into "saved" list

Actual result:

Watch-only addresses cannot be saved into the "saved" list.

Expected result:

Watch-only addresses should be saved into the "saved" list without validation

@smohamedjavid
Copy link
Member Author

Hey @VolodLytvynenko - Thanks for testing the PR 🙏

I'm not sure if this is an issue. In my opinion, watch-only addresses should be able to be added as saved . WDYT? However, this is a very low priority issue and it's better not to waste time on it. The PR can be merged. Let me know if this issue is decided to be fixed in this PR, and I can recheck it.

IMO, this will create confusion for the users. Watched addresses help you view the assets in that address (moreover, it is treated as an own account except for the ability to transact) where the saved address is NOT.

Watched addresses and saved addresses are shown to the users in the Send to screen. If they see the same address on both, it will create confusion.

I will merge this PR with the existing fix. if you feel it's something we should check, we can initiate a discussion with the design team and get their input as well.

Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
@smohamedjavid smohamedjavid force-pushed the fix/saved-addresses-fixes branch from 0ce2680 to a2ce5e3 Compare July 24, 2024 12:20
@smohamedjavid smohamedjavid merged commit 4989c92 into develop Jul 24, 2024
6 checks passed
@smohamedjavid smohamedjavid deleted the fix/saved-addresses-fixes branch July 24, 2024 14:17
ilmotta pushed a commit that referenced this pull request Jul 28, 2024
This commit:

- prevents the user from saving their wallet address as the saved address
- fixes button not capturing taps when the keyboard is open in saved address flows

Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
ilmotta added a commit that referenced this pull request Jul 30, 2024
Revisions from develop:

- 59ceddb develop origin/develop fix(wallet): fix bridge transactions (#20902)
- 99ccbc3 Cover wallet send events with tests Part 2 #20411 #20533 (#20721)
- 8c2d539 Enabling WalletConnect feature flag (#20906)
- 67c83b1 fix(wallet): remove edit routes button in bridging (#20874)
- 11a84ba feat(wallet): disable complex routing (#20901)
- 1f5bb57 chore(wallet): disable bridging on unsupported tokens (#20846)
- 4586f80 Add toggle in advanced settings for mobile data
- 55c620e fix: create password for small screen (#20645)
- 525609f Wallet Activity: transactions are not sorted by time #20808 (#20862)
- 9065395 chore(settings): Disable telemetry option (#20881)
- d27ab75 fix_:display group message using the new ui (#20787)
- c6a1db6 ci: enable split apks & build only for arm64-v8a (#20683)
- 73777e0 Ensure keycard account can send transaction after upgrading from v1 to v2 #20552 (#20845)
- a6d3fc3 [#20524] fix: the missed keypairs are shown in the key pair list screen (#20888)
- a671c70 fix broken screen and navigation when syncing fails (#20887)
- a45991b 🥅 Filter connected dapps based on testnet mode, reject proposals and requests gracefully (#20799)
- 2e9fa22 feat: wallet router v2 (#20631)
- 737d8c4 rename sub to fix error when requesting to join community (#20868)
- 3aa7e10 Sync process is blocked on Enabled notifications screen (#20883)
- c1d2d44 perf: Fix app freeze after login (#20729)
- 0fed811 e2e: updated testnet switching and added one test into smoke
- 53c35cb fix(wallet): Linear gradient exception on invalid colors for watched account cards (#20854)
- be82365 chore(settings)_: Remove testnet toggle from legacy advanced settings (#20875)
- eae8a65 feat(wallet)_: Add beta info box in activity tab (#20873)
- fe54a25 fix: not clearing network & web3-wallet on logout (#20886)
- 15a4219 Reject wallet-connect request by dragging the modal down (#20763) (#20836)
- 2ffbdac WalletConnect show expired toast (#20857)
- 402eb83 fix Issue with scrolling WalletConnect transaction on Android (#20867)
- ff88049 Fix WalletConnect header alignment on Android (#20860)
- cee2124 WalletConnect no internet edge-cases (#20826)
- 60ad7c8 chore(tests): New match-strict? cljs.test directive (#20825)
- 4989c92 fix_: Adding own address as saved addresses (#20839)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
5 participants