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

Inability to remove eth accounts which created by key pair generation flow or imported by seed phrase #19375

Closed
Tracked by #19466
VolodLytvynenko opened this issue Mar 22, 2024 · 11 comments · Fixed by #20569
Assignees
Labels
high-prio wallet: Keypair wallet-core Issues for mobile wallet team
Milestone

Comments

@VolodLytvynenko
Copy link
Contributor

Steps:

  1. Create an account with a newly generated keypair (see flow here how to generate keypair)
  2. Open current account
  3. Try to remove it

Actual result:

The removing bottom sheet is not shown

bottomsheet.mp4

Expected result:

The removing bottom sheet should be shown with 'remove' and 'cancel' buttons.
https://www.figma.com/file/HKncH4wnDwZDAhc4AryK8Y/Wallet-for-Mobile?type=design&node-id=900-234771&mode=design&t=x8pd1J3e5lSLbdW3-0

ENV:

NIghtly [22 Mar 2024]

@J-Son89
Copy link
Contributor

J-Son89 commented Apr 1, 2024

@VolodLytvynenko I remember this was discussed in the messengers. Is this still needed?
cc @ulisesmac @smohamedjavid

@J-Son89 J-Son89 mentioned this issue Apr 1, 2024
57 tasks
@VolodLytvynenko
Copy link
Contributor Author

@VolodLytvynenko I remember this was discussed in the messengers. Is this still needed? cc @ulisesmac @smohamedjavid

hi @J-Son89 I don't remember discussing it in the messengers. We discussed it once in this PR

@J-Son89
Copy link
Contributor

J-Son89 commented Apr 1, 2024

Ah apologies, perhaps it was a similar topic but not this. Thanks for confirming 🙏

@J-Son89 J-Son89 changed the title Inability to remove ETH account with Generated Keypairs Wallet - Add flow to remove ETH account with Generated Keypairs Apr 3, 2024
@J-Son89 J-Son89 changed the title Wallet - Add flow to remove ETH account with Generated Keypairs Inability to remove ETH account with Generated Keypairs Apr 3, 2024
@J-Son89
Copy link
Contributor

J-Son89 commented Apr 3, 2024

@VolodLytvynenko
Copy link
Contributor Author

hmm are we missing this flow? https://www.figma.com/file/HKncH4wnDwZDAhc4AryK8Y/Wallet-for-Mobile?type=design&node-id=900-233576&mode=design&t=XKuv9zmTEoDoP9y0-4

hi @J-Son89 This bottom sheet with "remove" and "cancel" buttons is shown for all accounts that were not created via generating a new keypair flow

@VolodLytvynenko VolodLytvynenko changed the title Inability to remove ETH account with Generated Keypairs Inability to remove ETH accounts which created by key pair generation flow or imported by seed phrase Apr 26, 2024
@VolodLytvynenko VolodLytvynenko changed the title Inability to remove ETH accounts which created by key pair generation flow or imported by seed phrase Inability to remove eth accounts which created by key pair generation flow or imported by seed phrase Apr 30, 2024
@churik churik added wallet-core Issues for mobile wallet team T:Mobile Wallet Settings and removed E:Mobile Wallet March wallet-core Issues for mobile wallet team labels Jun 20, 2024
@churik churik added this to the 2.30.0 Beta milestone Jun 21, 2024
@churik
Copy link
Member

churik commented Jun 21, 2024

@VolodLytvynenko is this issue still relevant?

@VolodLytvynenko
Copy link
Contributor Author

@VolodLytvynenko is this issue still relevant?

@churik still relevant

@ulisesmac
Copy link
Contributor

Hi @saledjenic

The accounts generated when we recover a seed phrase are type seed, currently we only remove the accounts with type generated.

Should we use the same mechanism to remove an account of type seed.

Thank you

@smohamedjavid
Copy link
Member

The same RPC method accounts_deleteAccount should work for all addresses irrespective of key pair type, including watch-only addresses 👍

@saledjenic
Copy link

The same RPC method accounts_deleteAccount should work for all addresses irrespective of key pair type, including watch-only addresses 👍

Correct.

@ulisesmac
Copy link
Contributor

Thank you @smohamedjavid @saledjenic

I'll be taking this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-prio wallet: Keypair wallet-core Issues for mobile wallet team
Development

Successfully merging a pull request may close this issue.

7 participants