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

Keycard signing for dapp interactions #21785

Open
wants to merge 11 commits into
base: develop
Choose a base branch
from

Conversation

clauxx
Copy link
Member

@clauxx clauxx commented Dec 10, 2024

fixes #21618

Summary

  1. Added support for signing dapp requests with the keycard
  2. Removed WC methods that are not supported by test dapps anymore: eth_sign, eth_signTransaction (previously could only be tested on test dapps and no prod. dapps usage)
  3. Fixed issue where dapp logo is not shown in request sometimes

Steps to test

  • Open Status
  • Log into a keycard profile
  • Connect to dapp
  • Send any request from dapp
  • Slide the authorization slider
  • Sign with the keycard
  • Dapp shows success

status: ready

@status-im-auto
Copy link
Member

status-im-auto commented Dec 10, 2024

Jenkins Builds

Click to see older builds (24)
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 0983e41 #2 2024-12-10 14:49:40 ~4 min tests 📄log
✔️ 0983e41 #3 2024-12-10 14:55:24 ~9 min android 🤖apk 📲
✔️ 0983e41 #3 2024-12-10 14:56:14 ~10 min android-e2e 🤖apk 📲
✔️ 0983e41 #2 2024-12-10 15:01:00 ~15 min ios 📱ipa 📲
4eaab72 #4 2025-01-02 14:45:18 ~3 min tests 📄log
✔️ 4eaab72 #5 2025-01-02 14:47:58 ~6 min android-e2e 🤖apk 📲
✔️ 4eaab72 #4 2025-01-02 14:48:34 ~7 min ios 📱ipa 📲
✔️ 4eaab72 #5 2025-01-02 14:49:50 ~8 min android 🤖apk 📲
✔️ a2aa35c #6 2025-01-06 11:39:59 ~4 min tests 📄log
✔️ a2aa35c #6 2025-01-06 11:42:41 ~7 min ios 📱ipa 📲
✔️ a2aa35c #7 2025-01-06 11:42:56 ~7 min android-e2e 🤖apk 📲
✔️ a2aa35c #7 2025-01-06 11:44:28 ~9 min android 🤖apk 📲
4cc6aed #7 2025-01-09 13:49:37 ~4 min tests 📄log
✔️ 4cc6aed #8 2025-01-09 13:53:30 ~8 min android-e2e 🤖apk 📲
✔️ 4cc6aed #8 2025-01-09 13:54:00 ~8 min android 🤖apk 📲
✔️ 4cc6aed #7 2025-01-09 13:54:51 ~9 min ios 📱ipa 📲
✔️ 3f22541 #8 2025-01-13 12:43:46 ~4 min tests 📄log
✔️ 3f22541 #9 2025-01-13 12:46:02 ~6 min android-e2e 🤖apk 📲
✔️ 3f22541 #9 2025-01-13 12:46:20 ~7 min android 🤖apk 📲
✔️ 3f22541 #8 2025-01-13 12:47:21 ~8 min ios 📱ipa 📲
✔️ 6be8ed4 #9 2025-01-15 10:47:59 ~5 min tests 📄log
✔️ 6be8ed4 #9 2025-01-15 10:50:55 ~8 min ios 📱ipa 📲
✔️ 6be8ed4 #10 2025-01-15 10:51:42 ~8 min android-e2e 🤖apk 📲
✔️ 6be8ed4 #10 2025-01-15 10:52:38 ~9 min android 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 3c8d497 #10 2025-01-17 10:44:49 ~4 min tests 📄log
✔️ 3c8d497 #11 2025-01-17 10:47:00 ~7 min android-e2e 🤖apk 📲
✔️ 3c8d497 #11 2025-01-17 10:48:38 ~8 min android 🤖apk 📲
✔️ 3c8d497 #10 2025-01-17 10:50:30 ~10 min ios 📱ipa 📲
✔️ eeeb781 #11 2025-01-21 13:58:50 ~5 min tests 📄log
✔️ eeeb781 #12 2025-01-21 14:00:03 ~6 min android-e2e 🤖apk 📲
✔️ eeeb781 #12 2025-01-21 14:02:14 ~8 min android 🤖apk 📲
✔️ eeeb781 #11 2025-01-21 14:03:45 ~10 min ios 📱ipa 📲

@flexsurfer
Copy link
Member

this PR should wait for #21753 , because auth on keycard has little bit changed

@VolodLytvynenko VolodLytvynenko self-assigned this Dec 16, 2024
@status-im-auto
Copy link
Member

50% of end-end tests have passed

Total executed tests: 8
Failed tests: 4
Expected to fail tests: 0
Passed tests: 4
IDs of failed tests: 727230,702745,727229,702843 

Failed tests (4)

Click to expand
  • Rerun failed tests

  • Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230
    Test setup failed: critical/test_wallet.py:22: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:310: in create_shared_drivers
        drivers = loop.run_until_complete(start_threads(test_suite_data.current_test.name,
    /usr/lib/python3.10/asyncio/base_events.py:649: in run_until_complete
        return future.result()
    __init__.py:26: in start_threads
        returns[k] = await returns[k]
    /usr/lib/python3.10/concurrent/futures/thread.py:58: in run
        result = self.fn(*self.args, **self.kwargs)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/webdriver.py:257: in __init__
        super().__init__(
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/selenium/webdriver/remote/webdriver.py:206: in __init__
        self.start_session(capabilities)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/webdriver.py:360: in start_session
        raise SessionNotCreatedException(
     A valid W3C session creation response must contain a non-empty "sessionId" entry. Got "{'value': {'error': 'unknown error', 'message': 'failed serving request POST /wd/hub/session', 'stacktrace': ''}}" instead
    



    2. test_wallet_send_eth, id: 727229

    Test setup failed: critical/test_wallet.py:22: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:310: in create_shared_drivers
        drivers = loop.run_until_complete(start_threads(test_suite_data.current_test.name,
    /usr/lib/python3.10/asyncio/base_events.py:649: in run_until_complete
        return future.result()
    __init__.py:26: in start_threads
        returns[k] = await returns[k]
    /usr/lib/python3.10/concurrent/futures/thread.py:58: in run
        result = self.fn(*self.args, **self.kwargs)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/webdriver.py:257: in __init__
        super().__init__(
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/selenium/webdriver/remote/webdriver.py:206: in __init__
        self.start_session(capabilities)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/webdriver.py:360: in start_session
        raise SessionNotCreatedException(
     A valid W3C session creation response must contain a non-empty "sessionId" entry. Got "{'value': {'error': 'unknown error', 'message': 'failed serving request POST /wd/hub/session', 'stacktrace': ''}}" instead
    



    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745

    Device 1: Find SendMessageButton by accessibility id: send-message-button
    Device 1: Tap on found: SendMessageButton

    critical/chats/test_1_1_public_chats.py:286: in test_1_1_chat_non_latin_messages_stack_update_profile_photo
        self.home_2.navigate_back_to_home_view()
    ../views/base_view.py:426: in navigate_back_to_home_view
        while not self.chat_floating_screen.is_element_disappeared(1) \
    ../views/base_element.py:231: in is_element_disappeared
        return self.wait_for_invisibility_of_element(sec)
    ../views/base_element.py:154: in wait_for_invisibility_of_element
        .until(expected_conditions.invisibility_of_element_located((self.by, self.locator)))
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/selenium/webdriver/support/wait.py:86: in until
        value = method(self._driver)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/selenium/webdriver/support/expected_conditions.py:320: in _predicate
        return _element_if_visible(target, visibility=False)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/selenium/webdriver/support/expected_conditions.py:175: in _element_if_visible
        return element if element.is_displayed() == visibility else False
     'dict' object has no attribute 'is_displayed'
    



    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Test setup failed: critical/chats/test_public_chat_browsing.py:307: in prepare_devices
        self.drivers, self.loop = create_shared_drivers(2)
    base_test_case.py:310: in create_shared_drivers
        drivers = loop.run_until_complete(start_threads(test_suite_data.current_test.name,
    /usr/lib/python3.10/asyncio/base_events.py:649: in run_until_complete
        return future.result()
    __init__.py:26: in start_threads
        returns[k] = await returns[k]
    /usr/lib/python3.10/concurrent/futures/thread.py:58: in run
        result = self.fn(*self.args, **self.kwargs)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/webdriver.py:257: in __init__
        super().__init__(
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/selenium/webdriver/remote/webdriver.py:206: in __init__
        self.start_session(capabilities)
    ../../../../status-app-prs@tmp/venv/lib/python3.10/site-packages/appium/webdriver/webdriver.py:360: in start_session
        raise SessionNotCreatedException(
     A valid W3C session creation response must contain a non-empty "sessionId" entry. Got "{'value': {'error': 'unknown error', 'message': 'failed serving request POST /wd/hub/session', 'stacktrace': ''}}" instead
    



    Passed tests (4)

    Click to expand

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231
    2. test_wallet_balance_mainnet, id: 740490

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
    Device sessions

    @VolodLytvynenko VolodLytvynenko removed their assignment Dec 17, 2024
    @clauxx clauxx force-pushed the cl-21618-dapps-keycard-signing branch from 0983e41 to c5e6a89 Compare January 2, 2025 14:40
    @clauxx clauxx requested a review from jakubgs as a code owner January 2, 2025 14:40
    @clauxx clauxx requested a review from flexsurfer January 2, 2025 14:41
    @jakubgs jakubgs removed their request for review January 3, 2025 15:18
    @clauxx clauxx force-pushed the cl-21618-dapps-keycard-signing branch 2 times, most recently from 7b40486 to a2aa35c Compare January 6, 2025 11:35
    @shivekkhurana shivekkhurana added the wallet-core Issues for mobile wallet team label Jan 7, 2025
    @flexsurfer flexsurfer mentioned this pull request Jan 8, 2025
    8 tasks
    @clauxx
    Copy link
    Member Author

    clauxx commented Jan 13, 2025

    @flexsurfer please have a look again so I can move it to testing

    @clauxx
    Copy link
    Member Author

    clauxx commented Jan 13, 2025

    @status-im/mobile-qa this PR is ready for testing. LMK if smth is missing in the description.

    @status-im-auto
    Copy link
    Member

    62% of end-end tests have passed

    Total executed tests: 8
    Failed tests: 3
    Expected to fail tests: 0
    Passed tests: 5
    
    IDs of failed tests: 740490,703133,702843 
    

    Failed tests (3)

    Click to expand
  • Rerun failed tests

  • Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Device 2: Tap on found: Button
    Device 2: Attempt 0 is successful clicking close-activity-center

    Test setup failed: critical/chats/test_public_chat_browsing.py:318: in prepare_devices
        self.home_2.handle_contact_request(self.username_1)
    ../views/home_view.py:388: in handle_contact_request
        chat_element.accept_contact_request()
    ../views/home_view.py:167: in accept_contact_request
        self.handle_cr("accept-contact-request")
    ../views/home_view.py:164: in handle_cr
        ).wait_for_rendering_ended_and_click()
    ../views/base_element.py:154: in wait_for_rendering_ended_and_click
        self.wait_for_visibility_of_element(20)
    ../views/base_element.py:138: in wait_for_visibility_of_element
        raise TimeoutException(
     Device 2: Button by xpath:`//*[contains(@text, 'shRpyeHZw847jhX3uTfT')]/ancestor::*[@content-desc='activity']/*[@content-desc="accept-contact-request"]` is not found on the screen after wait_for_visibility_of_element
    



    Device sessions

    Class TestWalletOneDevice:

    1. test_wallet_balance_mainnet, id: 740490

    Device 1: Find AssetElement by xpath: //android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='USDCoin']
    Device 1: Find AssetElement by xpath: //android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='USDCoin']

    critical/test_wallet.py:236: in test_wallet_balance_mainnet
        real_balance[asset] = self.wallet_view.get_asset(asset).get_amount()
    ../views/wallet_view.py:126: in get_asset
        element.scroll_to_element(down_start_y=0.89, down_end_y=0.8)
    ../views/base_element.py:196: in scroll_to_element
        raise NoSuchElementException(
     Device 1: AssetElement by xpath: `//android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='USDCoin']` 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
    



    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133

    Device 1: Tap on found: Button
    # STEP: Check that removed user is not shown in the list anymore

    critical/chats/test_public_chat_browsing.py:241: in test_restore_multiaccount_with_waku_backup_remove_profile_switch
        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))]))
     zQ3...dWXh5 was not restored as a contact from waku backup!
    E    zQ3...Vacac was not restored as a contact from waku backup!
    E    admin_open was not restored from waku-backup!!
    E    member_open was not restored from waku-backup!!
    E    admin_closed was not restored from waku-backup!!
    E    member_closed was not restored from waku-backup!!
    



    Device sessions

    Passed tests (5)

    Click to expand

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. 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

    @status-im-auto
    Copy link
    Member

    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 2: Tap on found: Button
    Device 2: Attempt 0 is successful clicking close-activity-center

    Test setup failed: critical/chats/test_public_chat_browsing.py:318: in prepare_devices
        self.home_2.handle_contact_request(self.username_1)
    ../views/home_view.py:388: in handle_contact_request
        chat_element.accept_contact_request()
    ../views/home_view.py:167: in accept_contact_request
        self.handle_cr("accept-contact-request")
    ../views/home_view.py:164: in handle_cr
        ).wait_for_rendering_ended_and_click()
    ../views/base_element.py:154: in wait_for_rendering_ended_and_click
        self.wait_for_visibility_of_element(20)
    ../views/base_element.py:138: in wait_for_visibility_of_element
        raise TimeoutException(
     Device 2: Button by xpath:`//*[contains(@text, 'shr6bEiu7JCPA7oPyqwo')]/ancestor::*[@content-desc='activity']/*[@content-desc="accept-contact-request"]` is not found on the screen after wait_for_visibility_of_element
    



    Device sessions

    Passed tests (7)

    Click to expand

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231
    2. test_wallet_balance_mainnet, id: 740490

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
    Device sessions

    Class TestWalletMultipleDevice:

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

    @Horupa-Olena Horupa-Olena self-assigned this Jan 14, 2025
    @Horupa-Olena Horupa-Olena force-pushed the cl-21618-dapps-keycard-signing branch from 3f22541 to 6be8ed4 Compare January 15, 2025 10:42
    @Horupa-Olena
    Copy link

    Horupa-Olena commented Jan 16, 2025

    @clauxx Hi! Thanks for your PR.
    Please check the follow issue:

    Issue 1: Send transaction during interaction with a dApp always return rejection for a keycard account

    Steps to reproduce:

    1. Open Status.
    2. Log into a keycard profile.
    3. Connect to a dApp.
    4. Send a transaction request (e.g., send ETH) from the dApp.
    5. Slide the authorization slider.
    6. Sign with the keycard.

    Expected result:
    The transaction is signed successfully and reflected on the dApp side. Balances are updated, and the recipient receives the funds.

    Actual result:
    The transaction returns a "rejected" status.

    Comment:
    This was tested on both a real dApp and a test dApp. Transaction requests work successfully for regular accounts, but for keycard accounts, they consistently return a rejection.
    For example:it easy repro on https://se-sdk-dapp.vercel.app/ for eth_sendTransaction request

    screen-20250116-174908.mp4

    @clauxx
    Copy link
    Member Author

    clauxx commented Jan 17, 2025

    @Horupa-Olena strange, I tested with the test dapp you linked and with real dapps and it works. I merged the latest develop, cause I know there were some fixes there in the meantime, so maybe that helps. Can you check it again, and maybe share the logs to see what's happening there?

    also, does it only happen on iOS, or only Android?

    @Horupa-Olena
    Copy link

    Horupa-Olena commented Jan 17, 2025

    @Horupa-Olena strange, I tested with the test dapp you linked and with real dapps and it works. I merged the latest develop, cause I know there were some fixes there in the meantime, so maybe that helps. Can you check it again, and maybe share the logs to see what's happening there?

    also, does it only happen on iOS, or only Android?

    Hi @clauxx ! I tested it on Android since my iPhone doesn’t read the keycard. I’ve asked for help from Mobile QA team, but it might take some time.

    Unfortunately, merging from develop didn’t help; the issue is still present.
    Here are the logs from the yesterday
    Status-debug-logs (2).zip
    and today’s
    Status-debug-logs (3).zip

    Interesting, that swap in real dApp is working, problem only with send real dApp and eth_sendTransaction request for test dApp.

    @Horupa-Olena
    Copy link

    Horupa-Olena commented Jan 17, 2025

    also, does it only happen on iOS, or only Android?

    @clauxx It reproduce for both OS. Log for IOS:
    logs (15).zip

    @clauxx
    Copy link
    Member Author

    clauxx commented Jan 17, 2025

    @Horupa-Olena I just tested with both test dapps (one failing on android other one on ios) and uniswap (send,swap) and it worked, both with the debug build and the PR build on android. From looking at the logs, seems like the error might be related to the proxy, but unclear why I don't also get it.

    @friofry does this error say anything to you? This is from the wallet_sendTransactionWithSignature rpc and it shows 0 balance when there actually is a balance for the address.

    2025-01-17T13:10:16.554Z ERROR [status-im.contexts.wallet.wallet-connect.events.session-responses:82] - Failed to sign Wallet Connect request {:error
     {:code -32000,
      :message
      "status-proxy-0.error: INTERNAL_ERROR: insufficient funds, status-proxy-fallback-1.error: insufficient funds for gas * price + value: balance 0, tx cost 96775954653000, overshot 96775954653000"},
    
    

    @pavloburykh pavloburykh self-assigned this Jan 21, 2025
    @Horupa-Olena Horupa-Olena force-pushed the cl-21618-dapps-keycard-signing branch from 3c8d497 to eeeb781 Compare January 21, 2025 13:53
    @VolodLytvynenko
    Copy link
    Contributor

    hi @clauxx take a look found issue.
    Unfortunately, this issue doesn't happen with all users, I will send the seed phrase in DM

    ISSUE 2: "Cannot read property 'off' of null" error is shown after migration to keycard

    Steps:

    1. Restore user using seed phrase
    2. Go to profile -> Keycard -> Migrate current user
    3. Tap log out

    Actual result:

    "Cannot read property 'off' of null" error is shown
    image

    Expected result:

    No error after migration

    Devices:

    • Pixel 7a, Android 13
    • iPhone 11 Pro Max, IOS 17

    Logs

    https://drive.google.com/file/d/1MIlNOr1zfNdRit85rgOkqytmXF4lYtPl/view?usp=drive_link

    @Horupa-Olena
    Copy link

    Horupa-Olena commented Jan 21, 2025

    @clauxx
    Unfortunately, the PR is currently blocked by this bug. Transactions (send, swap) are being rejected even in Nightly, which suggests that WalletConnect might be broken.

    @clauxx
    Copy link
    Member Author

    clauxx commented Jan 23, 2025

    @VolodLytvynenko is this happening on this PR or on develop? I understand why it would happen in develop, so want to make sure.

    @VolodLytvynenko
    Copy link
    Contributor

    @VolodLytvynenko is this happening on this PR or on develop? I understand why it would happen in develop, so want to make sure.

    @clauxx I wasn’t able to reproduce this issue on develop. However, it’s also not consistently reproducible in the current PR. For example, @pavloburykh couldn’t reproduce it, but I was able to reproduce it twice in a row on different devices.

    In summary, I’m not entirely sure if this issue is already present in develop. Perhaps I just couldn’t reproduce it there. I’ll investigate it more thoroughly again and let you know soon

    @VolodLytvynenko
    Copy link
    Contributor

    @clauxx I succeeded in reproducing this issue in the develop. Apologies, the issue is not PR related. Created it separately #21979

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

    Successfully merging this pull request may close these issues.

    🖇️ Integrate Keycard signing flow for dapp transactions
    8 participants