status-mobile icon indicating copy to clipboard operation
status-mobile copied to clipboard

Metamask address support

Open vkjr opened this issue 1 year ago • 9 comments

fixes #19986

Summary

This PR adds support for metamask addresses, they now can be scanned from QR codes and converted to Status addresses. Small additional changes:

  • functions to manipulate addresses moved to utils.address namespace
  • few more meaningful renamings
  • tests added

Review notes

Metamask stores addresses in QR codes with the following format: ethereum:0x38cf6E0Ba4C4530735616e1Ee7ff5FbCB726fBd2@0xa

ethereum: - prefix described in https://github.com/ethereum/EIPs/issues/67 0xa - chain ID suffix described in EIP-155

Testing notes

Bunch of qr codes for quick testing:

status-qr-multichain status-qr-profile metamask-qr-@0xa4b1 metamask-qr-@0xa metamask-qr-@0x1

Platforms

  • Android
  • iOS

Areas that maybe impacted

vkjr avatar Jul 22 '24 13:07 vkjr

Jenkins Builds

Click to see older builds (43)
:grey_question: Commit :hash: Finished (UTC) Duration Platform Result
:x: 98b6322b #1 2024-07-22 13:25:36 ~3 min tests :page_facing_up:log
:heavy_check_mark: 98b6322b #1 2024-07-22 13:31:17 ~8 min android-e2e :robot:apk :calling:
:heavy_check_mark: 98b6322b #1 2024-07-22 13:31:26 ~8 min android :robot:apk :calling:
:heavy_check_mark: 98b6322b #1 2024-07-22 13:34:37 ~12 min ios :iphone:ipa :calling:
:x: 13d2ffb1 #2 2024-07-22 16:14:57 ~2 min tests :page_facing_up:log
:heavy_check_mark: 13d2ffb1 #2 2024-07-22 16:18:39 ~6 min android-e2e :robot:apk :calling:
:heavy_check_mark: 13d2ffb1 #2 2024-07-22 16:19:13 ~6 min android :robot:apk :calling:
:x: e9f4f814 #3 2024-07-22 16:23:53 ~2 min tests :page_facing_up:log
:heavy_check_mark: e9f4f814 #3 2024-07-22 16:28:39 ~7 min android :robot:apk :calling:
:heavy_check_mark: e9f4f814 #3 2024-07-22 16:29:03 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: e9f4f814 #3 2024-07-22 16:31:29 ~10 min ios :iphone:ipa :calling:
:x: 704ed012 #4 2024-07-23 11:10:28 ~3 min tests :page_facing_up:log
:heavy_check_mark: 704ed012 #4 2024-07-23 11:13:10 ~6 min android-e2e :robot:apk :calling:
:heavy_check_mark: 704ed012 #4 2024-07-23 11:14:47 ~7 min android :robot:apk :calling:
:heavy_check_mark: 704ed012 #4 2024-07-23 11:19:10 ~12 min ios :iphone:ipa :calling:
:x: 89b769c9 #5 2024-07-23 11:38:29 ~2 min tests :page_facing_up:log
:heavy_check_mark: 89b769c9 #5 2024-07-23 11:42:24 ~6 min android :robot:apk :calling:
:heavy_check_mark: 89b769c9 #5 2024-07-23 11:43:23 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 89b769c9 #5 2024-07-23 11:46:25 ~10 min ios :iphone:ipa :calling:
:heavy_check_mark: 8a38102c #6 2024-07-24 13:13:27 ~4 min tests :page_facing_up:log
:heavy_check_mark: 8a38102c #6 2024-07-24 13:16:53 ~8 min android :robot:apk :calling:
:heavy_check_mark: 8a38102c #6 2024-07-24 13:17:03 ~8 min android-e2e :robot:apk :calling:
:heavy_check_mark: 8a38102c #6 2024-07-24 13:20:34 ~12 min ios :iphone:ipa :calling:
:heavy_check_mark: fb352d79 #7 2024-07-30 15:30:29 ~4 min tests :page_facing_up:log
:heavy_check_mark: fb352d79 #7 2024-07-30 15:32:25 ~6 min android-e2e :robot:apk :calling:
:heavy_check_mark: fb352d79 #7 2024-07-30 15:32:51 ~7 min android :robot:apk :calling:
:heavy_check_mark: fb352d79 #7 2024-07-30 15:39:28 ~13 min ios :iphone:ipa :calling:
:x: 92cb0bf2 #8 2024-07-30 16:20:26 ~2 min tests :page_facing_up:log
:heavy_check_mark: 92cb0bf2 #8 2024-07-30 16:24:46 ~6 min android :robot:apk :calling:
:heavy_check_mark: 92cb0bf2 #8 2024-07-30 16:24:57 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 92cb0bf2 #8 2024-07-30 16:34:14 ~16 min ios :iphone:ipa :calling:
:x: 1e6b0f1c #9 2024-08-23 12:48:20 ~3 min tests :page_facing_up:log
:heavy_check_mark: 1e6b0f1c #9 2024-08-23 12:53:26 ~8 min android-e2e :robot:apk :calling:
:heavy_check_mark: 1e6b0f1c #9 2024-08-23 12:53:34 ~8 min android :robot:apk :calling:
:heavy_check_mark: 1e6b0f1c #9 2024-08-23 12:55:11 ~10 min ios :iphone:ipa :calling:
:x: 4250ada3 #10 2024-08-26 13:52:10 ~2 min tests :page_facing_up:log
:heavy_check_mark: 4250ada3 #10 2024-08-26 13:56:42 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 4250ada3 #10 2024-08-26 13:59:06 ~9 min android :robot:apk :calling:
:heavy_check_mark: 4250ada3 #10 2024-08-26 13:59:48 ~10 min ios :iphone:ipa :calling:
:heavy_check_mark: 79c59656 #11 2024-08-28 13:28:52 ~5 min tests :page_facing_up:log
:heavy_check_mark: 79c59656 #11 2024-08-28 13:31:04 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 79c59656 #11 2024-08-28 13:31:45 ~8 min android :robot:apk :calling:
:heavy_check_mark: 79c59656 #11 2024-08-28 13:59:31 ~35 min ios :iphone:ipa :calling:
:grey_question: Commit :hash: Finished (UTC) Duration Platform Result
:heavy_check_mark: b7558111 #12 2024-08-29 11:22:36 ~4 min tests :page_facing_up:log
:heavy_check_mark: b7558111 #12 2024-08-29 11:25:11 ~7 min android :robot:apk :calling:
:heavy_check_mark: b7558111 #12 2024-08-29 11:25:36 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: b7558111 #12 2024-08-29 11:29:44 ~11 min ios :iphone:ipa :calling:
:heavy_check_mark: 9fb32551 #13 2024-08-29 11:41:14 ~4 min tests :page_facing_up:log
:heavy_check_mark: 9fb32551 #13 2024-08-29 11:43:34 ~6 min android-e2e :robot:apk :calling:
:heavy_check_mark: 9fb32551 #13 2024-08-29 11:45:28 ~8 min android :robot:apk :calling:
:heavy_check_mark: 9fb32551 #13 2024-08-29 11:47:37 ~10 min ios :iphone:ipa :calling:

status-im-auto avatar Jul 22 '24 13:07 status-im-auto

It's WIP or not? 🤔

J-Son89 avatar Jul 23 '24 21:07 J-Son89

@J-Son89, not anymore)

vkjr avatar Jul 24 '24 11:07 vkjr

57% of end-end tests have passed

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

Failed tests (3)

Click to expand
  • Rerun failed tests

  • Class TestCommunityMultipleDeviceMerged:

    1. test_community_message_edit, id: 702843

    Device 2: Looking for a message by text: Message AFTER edit 2 (Edited)
    Device 2: Find `ChatElementByText` by `xpath`: `//*[starts-with(@text,'Message AFTER edit 2 (Edited)')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']`
    critical/chats/test_public_chat_browsing.py:378: in test_community_message_edit
        self.channel_2.set_reaction(message_text_after_edit)
    ../views/chat_view.py:1053: in set_reaction
        self.chat_element_by_text(message).long_press_until_element_is_shown(element)
    ../views/base_element.py:327: in long_press_until_element_is_shown
        element = self.find_element()
    ../views/chat_view.py:116: in find_element
        self.wait_for_visibility_of_element(20)
    ../views/base_element.py:147: in wait_for_visibility_of_element
        raise TimeoutException(
     Device 2: ChatElementByText by xpath:`//*[starts-with(@text,'Message AFTER edit 2 (Edited)')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']` is not found on the screen after wait_for_visibility_of_element
    



    Device sessions

    Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230

    Device 2: Find Text by xpath: //android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='Ether']/../android.widget.TextView[3]
    Device 2: Text is 0.03939 ETH
    critical/test_wallet.py:190: in test_wallet_send_asset_from_drawer
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.452 but expected to be 0.4521
    



    2. test_wallet_send_eth, id: 727229

    Device 2: Find Text by xpath: //android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='Ether']/../android.widget.TextView[3]
    Device 2: Text is 0.03929 ETH
    critical/test_wallet.py:159: in test_wallet_send_eth
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.4522 but expected to be 0.4523
    



    Passed tests (4)

    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_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    status-im-auto avatar Aug 27 '24 08:08 status-im-auto

    @vkjr Thank you for the PR. Here are found issues:

    ISSUE 1: User navigated to overview wallet page when MetaMask QR is Scanned by Universal Scanner

    Steps:

    1. Open the universal scanner on the app.
    2. Scan a MetaMask QR code (QR that contains "ethereum:" prefix) image

    Actual Result:

    The user is navigated to the wallet overview page after scanning the MetaMask QR code.

    https://github.com/user-attachments/assets/3d98d7d7-34c4-4e7d-95be-543a81dd18fb

    Expected Result:

    The app should recognize the MetaMask QR code and show the drawer with the scanned address. After converting, ethereum:0x38cf6E0Ba4C4530735616e1Ee7ff5FbCB726fBd2 should become 0x38cf6E0Ba4C4530735616e1Ee7ff5FbCB726fBd2

    image

    Devices:

    Pixel 7a, Android 13 iPhone 11 Pro Max, iOS 17

    Additional info:

    The QR with "ethereum" prefix is not recognized on the Send to page as well "Oops this is not QR" toast is shown

    https://github.com/user-attachments/assets/33f3060b-ac33-4f93-be5a-cb855d0c231a

    VolodLytvynenko avatar Aug 28 '24 07:08 VolodLytvynenko

    @VolodLytvynenko , thanks for finding, checking!

    vkjr avatar Aug 28 '24 08:08 vkjr

    Hi @vkjr, the PR looks great. The QR code issue mentioned in this comment seems to be related to the Metamask plugin. The mobile version usually shares other QRs, which can be scanned well using a build of this PR. The issue can be fixed in a follow-up, but if you think it’s better to address it in this PR, feel free to do so. Otherwise, PR can be merged

    VolodLytvynenko avatar Aug 28 '24 09:08 VolodLytvynenko

    @VolodLytvynenko, qr code from metamask plugin contains text ethereum:0xFC6327a092f6232e158a0Dd1d6d967a2e1c65cD5 and it has no suffix, so it is not strictly follow the rules that described in original issue . But I think that we can simply imply that addresses without suffix are belong to mainnet, wdyt? Also @xAlisher, wdyt?

    vkjr avatar Aug 28 '24 13:08 vkjr

    @VolodLytvynenko, qr code from metamask plugin contains text ethereum:0xFC6327a092f6232e158a0Dd1d6d967a2e1c65cD5 and it has no suffix, so it is not strictly follow the rules that described in original issue . But I think that we can simply imply that addresses without suffix are belong to mainnet, wdyt? Also @xAlisher, wdyt?

    @vkjr The best way to address this issue is to ignore the "ethereum:" prefix and only extract the address body, like 0xFC6327a092f6232e158a0Dd1d6d967a2e1c65cD5. In MetaMask, the "ethereum:" prefix doesn’t necessarily indicate the mainnet—it appears even when other networks are selected.

    VolodLytvynenko avatar Aug 28 '24 15:08 VolodLytvynenko

    @VolodLytvynenko, I added support to ethereum addresses without suffixes. Could you please briefly take a look, just to make sure all is good?

    vkjr avatar Aug 29 '24 11:08 vkjr

    71% of end-end tests have passed

    Total executed tests: 7
    Failed tests: 2
    Expected to fail tests: 0
    Passed tests: 5
    
    IDs of failed tests: 727230,727229 
    

    Failed tests (2)

    Click to expand
  • Rerun failed tests

  • Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230

    Device 2: Find `Text` by `xpath`: `//android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='Ether']/../android.widget.TextView[3]`
    Device 2: `Text` is `0.04199 ETH`
    critical/test_wallet.py:190: in test_wallet_send_asset_from_drawer
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.4479 but expected to be 0.448
    



    2. test_wallet_send_eth, id: 727229

    Device 2: Find Text by xpath: //android.view.ViewGroup[@content-desc='container']/android.widget.TextView[@text='Ether']/../android.widget.TextView[3]
    Device 2: Text is 0.04189 ETH
    critical/test_wallet.py:159: in test_wallet_send_eth
        self.errors.verify_no_errors()
    base_test_case.py:191: in verify_no_errors
        pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
     Sender balance is not updated on Etherscan, it is 0.4481 but expected to be 0.4482
    



    Passed tests (5)

    Click to expand

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

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

    1. test_community_message_edit, id: 702843
    Device sessions

    status-im-auto avatar Aug 29 '24 12:08 status-im-auto

    @VolodLytvynenko, are the results of endtoend testing acceptable?

    vkjr avatar Aug 29 '24 13:08 vkjr

    @VolodLytvynenko, are the results of endtoend testing acceptable?

    @vkjr Yes. e2e are ok. PR can be merged. Thank you!

    VolodLytvynenko avatar Aug 29 '24 14:08 VolodLytvynenko

    @VolodLytvynenko, thank you!

    vkjr avatar Aug 29 '24 14:08 vkjr