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

[#21836] Relative derivation paths are not supported by keycard #21888

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

Conversation

flexsurfer
Copy link
Member

fixes #21836

it fixed main issue, but also i fixed path in the about tab for account, but most likely there are more places with the wrong path, but i don't want to mess with it in this PR

@status-im-auto
Copy link
Member

status-im-auto commented Jan 7, 2025

Jenkins Builds

Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 9e1b449 #2 2025-01-07 08:13:16 ~4 min tests 📄log
✔️ 9e1b449 #2 2025-01-07 08:16:54 ~8 min android-e2e 🤖apk 📲
✔️ 9e1b449 #2 2025-01-07 08:17:20 ~8 min android 🤖apk 📲
✔️ 9e1b449 #2 2025-01-07 08:17:35 ~8 min ios 📱ipa 📲

@qfrank
Copy link
Contributor

qfrank commented Jan 7, 2025

Hi @flexsurfer , I just checked relate code in status-go, just like as you said before, m/1 is not equal to m/44/60/0/0/1 . And it seems there's no relations between m/1 to m/44/60/0/0/1 in the view of function Derive since it produce different return value, API GetDerivedAddresses return m/1 or m/2 is designed to ensure compatibility with older version(V1) wallet accounts. It is expected for old wallet account generated in v1. If the main account is generated with v2, we should not see m/1. Relate PR , relate issue

// Derive returns a derived child key at a given path
func (k *ExtendedKey) Derive(path []uint32) (*ExtendedKey, error) {
  var err error
  extKey := k
  for _, i := range path {
    extKey, err = extKey.Child(i)
    if err != nil {
      return nil, ErrDerivingChild
    }
  }

  return extKey, nil
}

Comment on lines +262 to +263
:derivation-path (create-account.utils/normalize-path
@derivation-path)
Copy link
Member

Choose a reason for hiding this comment

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

One other place within this same file/screen might need normalization:

Copy link
Member Author

Choose a reason for hiding this comment

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

good point

Copy link
Member Author

Choose a reason for hiding this comment

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

actually this one is tricky i wouldn't change it now, because path might be edited there

@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
    Test setup failed: critical/test_wallet.py:223: in prepare_devices
        self.sender_username = self.home_view.get_username()
    ../views/home_view.py:528: in get_username
        profile_view.default_username_text.wait_for_element(3)
    ../views/base_element.py:128: in wait_for_element
        raise TimeoutException(
     Device `1`: `Text` by` accessibility id`: `username` is not found on the screen after wait_for_element
    



    2. test_wallet_balance_mainnet, id: 740490

    Device 1: Click until Text by accessibility id: username will be presented
    Device 1: Find ProfileButton by accessibility id: open-profile

    Test setup failed: critical/test_wallet.py:223: in prepare_devices
        self.sender_username = self.home_view.get_username()
    ../views/home_view.py:528: in get_username
        profile_view.default_username_text.wait_for_element(3)
    ../views/base_element.py:128: in wait_for_element
        raise TimeoutException(
     Device `1`: `Text` by` accessibility id`: `username` is not found on the screen after wait_for_element
    



    Passed tests (6)

    Click to expand

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    Class TestWalletMultipleDevice:

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

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

    1. test_community_message_edit, id: 702843
    Device sessions

    @VolodLytvynenko VolodLytvynenko self-assigned this Jan 8, 2025
    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.

    Relative derivation paths are not supported by keycard
    5 participants