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

[#21853] fix: check deeplink pending request after wallet connect loaded #21873

Merged
merged 5 commits into from
Jan 15, 2025

Conversation

mohsen-ghafouri
Copy link
Contributor

@mohsen-ghafouri mohsen-ghafouri commented Dec 27, 2024

fixes #21853

Summary

  • "cannot read property" error is displayed on profiles list page
  • Handle pending dapp pair request after wallet connect loaded successfully

Areas that maybe impacted

  • Dapps interactions

Test Note

Regarding the issue discussed in PR #21833, here’s an explanation of how dApp requests are handled and the current limitations:

Handling dApp Requests

  1. Event Listener

    • The app registers an event listener to handle incoming dApp requests.
    • However, this listener is only registered after the wallet screen loads. If a request is sent before the listener is active, it will not be processed.
  2. Deep Link

    • Currently, pairing requests from a dApp can be handled through deep links, where the dApp sends a URI containing the pairing information to the app.
    • However, sign requests are not handled through deep links. This means any signing request (e.g., transactions or message signing) that is initiated via deep link will not work if user is not logged in already.
    • If the app is launched through a deep link for a pairing request, the process works as expected, as shown in the video below.
  3. Push Notification (Not Yet Implemented)

    • Push notifications are intended to handle scenarios where the app is closed, and the request is sent to a different device.
    • The issue in PR #21833 highlights this limitation. Currently, push notifications are not supported for any type of request.

for PR #21833 we can have a separate issue so i can discuss it with @clauxx and check for any solution.

Steps to test

  1. Connect to the Dapp(in my case Uniswap) using universal scanner
  2. Log out
  3. Make any transaction Using Dapp (approve or swap)

Result

Simulator.Screen.Recording.-.iPhone.13.-.2024-12-27.at.17.11.05.mp4

status: ready

@mohsen-ghafouri mohsen-ghafouri self-assigned this Dec 27, 2024
@status-im-auto
Copy link
Member

status-im-auto commented Dec 27, 2024

Jenkins Builds

Click to see older builds (12)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ f5ae111 #1 2024-12-27 13:00:17 ~4 min tests 📄log
✔️ f5ae111 #1 2024-12-27 13:02:48 ~6 min android-e2e 🤖apk 📲
✔️ f5ae111 #1 2024-12-27 13:03:19 ~7 min ios 📱ipa 📲
✔️ f5ae111 #1 2024-12-27 13:03:23 ~7 min android 🤖apk 📲
✔️ 1c22eb2 #2 2025-01-03 09:20:10 ~5 min tests 📄log
✔️ 1c22eb2 #2 2025-01-03 09:22:03 ~6 min ios 📱ipa 📲
✔️ 1c22eb2 #2 2025-01-03 09:23:47 ~8 min android-e2e 🤖apk 📲
✔️ 1c22eb2 #2 2025-01-03 09:24:35 ~9 min android 🤖apk 📲
✔️ e336c80 #3 2025-01-09 15:07:40 ~4 min tests 📄log
✔️ e336c80 #3 2025-01-09 15:09:35 ~6 min android-e2e 🤖apk 📲
✔️ e336c80 #3 2025-01-09 15:10:06 ~6 min ios 📱ipa 📲
✔️ e336c80 #3 2025-01-09 15:11:09 ~8 min android 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 4673f0d #4 2025-01-14 14:37:26 ~4 min tests 📄log
✔️ 4673f0d #4 2025-01-14 14:40:51 ~8 min android 🤖apk 📲
✔️ 4673f0d #4 2025-01-14 14:41:22 ~8 min android-e2e 🤖apk 📲
✔️ 4673f0d #4 2025-01-15 07:16:45 ~16 hr ios 📱ipa 📲
✔️ 435b53c #5 2025-01-15 14:40:44 ~5 min tests 📄log
✔️ 435b53c #5 2025-01-15 14:45:22 ~9 min android-e2e 🤖apk 📲
✔️ 435b53c #5 2025-01-15 14:45:47 ~10 min android 🤖apk 📲
✔️ 435b53c #5 2025-01-15 14:47:17 ~11 min ios 📱ipa 📲

@mohsen-ghafouri mohsen-ghafouri marked this pull request as ready for review December 27, 2024 14:17
@mohsen-ghafouri mohsen-ghafouri requested review from alwx, smohamedjavid and vkjr and removed request for alwx December 27, 2024 14:20
@shivekkhurana shivekkhurana added the wallet-core Issues for mobile wallet team label Jan 7, 2025
@shivekkhurana
Copy link
Contributor

@mohsen-ghafouri Please use wallet-core label on PRs for better visibility.

@mohsen-ghafouri
Copy link
Contributor Author

@shivekkhurana I also checked the event listener to see if it sends pending requests, but it doesn’t work. Only events after registration are being received.

[:effects.wallet-connect/register-event-listener
            [web3-wallet
             constants/wallet-connect-session-request-event
             #(rf/dispatch [:wallet-connect/on-session-request %])]]

@status-im-auto
Copy link
Member

75% of end-end tests have passed

Total executed tests: 8
Failed tests: 2
Expected to fail tests: 0
Passed tests: 6
IDs of failed tests: 727231,740490 

Failed tests (2)

Click to expand
  • Rerun failed tests

  • Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231

    # STEP: Adding new regular account
    Device 1: Find `Button` by `accessibility id`: `add-account`

    critical/test_wallet.py:266: in test_wallet_add_remove_regular_account
        self.wallet_view.add_regular_account(account_name=new_account_name)
    ../views/wallet_view.py:175: in add_regular_account
        self.add_account_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: `add-account` 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
    



    2. test_wallet_balance_mainnet, id: 740490

    ## Sign in (password: qwerty1234)
    Device 1: Getting username card by 'shdUaM8M6QcxQ4qn32nQ'

    critical/test_wallet.py:230: in test_wallet_balance_mainnet
        self.sign_in_view.sign_in(user_name=self.sender_username)
    ../views/sign_in_view.py:134: in sign_in
        self.get_user_profile_by_name(user_name).click()
    ../views/sign_in_view.py:147: in get_user_profile_by_name
        raise NoSuchElementException(msg="User %s is not found!" % username)
     User shdUaM8M6QcxQ4qn32nQ is not found!; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception
    



    Passed tests (6)

    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

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
    Device sessions

    2. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    @pavloburykh
    Copy link
    Contributor

    pavloburykh commented Jan 10, 2025

    @mohsen-ghafouri thanks for the PR. Unfortunately the initial issue is still reproducible.

    ISSUE 1 "cannot read property" error is displayed on profiles list page when generating event on Dapp's side

    Steps:

    1. Connect to any Dapp (I used https://se-sdk-dapp.vercel.app/ for testing)
    2. Logout
    3. While on Profile list/Login screen generate any event on Dapp's side

    Actual result: "cannot read property..." error appears. At the same time, user is able to sign pending event after login.

    Status-debug-logs - 2025-01-10T153439.251.zip

    telegram-cloud-document-2-5190914400411740550.mp4

    @mohsen-ghafouri
    Copy link
    Contributor Author

    Hey @pavloburykh thanks for testing it, could you please check again?

    @mohsen-ghafouri
    Copy link
    Contributor Author

    Hey @clauxx i added an unregister event in last commit to resolve mentioned issue, JFYI if you have any feedback.

    @pavloburykh
    Copy link
    Contributor

    @mohsen-ghafouri thank you for the fixes. PR is ready to be merged.

    for #21833 (comment) we can have a separate issue so i can discuss it with @clauxx and check for any solution.

    I have logged a separate issue on that #21935

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    Status: DONE
    Development

    Successfully merging this pull request may close these issues.

    "cannot read property" error is displayed on profiles list page when the Dapp attempts to send a signature
    6 participants