MetaMask / metamask-extension

:globe_with_meridians: :electric_plug: The MetaMask browser extension enables browsing Ethereum blockchain enabled websites
https://metamask.io
Other
11.94k stars 4.88k forks source link

Fix prompt for backing SRP #13277

Open EHaracic opened 2 years ago

EHaracic commented 2 years ago

User Story

As a user, I want to only get the prompt for backing my SRP once I have non-zero token balances

Figma/Notion links:

Notion: https://www.notion.so/As-a-user-I-want-to-only-get-the-prompt-for-backing-my-SRP-once-I-have-non-zero-token-balances-70f7780fe14f470aa8454704b55cf76b

Context:

Currently, we show this prompt once the user has non-zero funds for the base token (e.g ETH) or once they add a token to their list.

As it obscures part of the UI, prevents the user from interacting with what's behind it and cannot be closed, we should update it to also check if the added token has a non-zero balance and, if yes, only then show the prompt.

We should also exclude testnet assets from this non-zero balance detection as those tokens don't have value.

We should also exclude hardware wallet accounts from this non-zero balance detection as those keys are not generated with MetaMask Secret Recovery Phrase so backing it up won’t make the user more safe.

image

Acceptance Criteria

  1. Don't show the banner when the users only have balance on testnets;
  2. Don't show the banner when the users have added new tokens with zero balance (and don't have any other non-zero balance in their wallet).
  3. Don’t show the banner when the users only have balance on hardware accounts.
  4. The banner is show in all networks even if the user doesn't have balance on that specific network (but meets one of other criterias needed for the warning to be shown).

Open question(s)

EHaracic commented 2 years ago

@danjm will provide account with the assets for testing

trisboyd commented 2 years ago

If this hasn't already been addressed (or someone is not in the process) I'll take a stab at this.

danjm commented 2 years ago

@Trisboyd You can take this ticket. Comment here if you run into any issues.

@EHaracic You can unassign this from your team. @Trisboyd, a community contributor, will take this one

EHaracic commented 2 years ago

@danjm Done.

bschorchit commented 2 years ago

Hey @Trisboyd, were you able to make progress on this ticket?

trisboyd commented 2 years ago

Hey Barbara, unfortunately no. I ran into a bug where I can't get a build of the extension on my pc (Windows 10) so examining solutions in real time is impossible. I brought this up with Brad Decker and he said someone from the team is addressing the issue, but it hasn't been resolved yet.

It would probably be a good idea to give someone else this ticket and I'll pick up another when the build issue is fixed.

Thanks! -Tristan Boyd

On Fri, May 20, 2022 at 9:09 AM Bárbara Schorchit @.***> wrote:

Hey @Trisboyd https://github.com/Trisboyd, were you able to make progress on this ticket?

— Reply to this email directly, view it on GitHub https://github.com/MetaMask/metamask-extension/issues/13277#issuecomment-1133020207, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQOSNNJFXGNT6ULXXEJUDOTVK6TKFANCNFSM5LV2WWVA . You are receiving this because you were mentioned.Message ID: @.***>

bschorchit commented 2 years ago

No worries, @Trisboyd, thank you for trying to contribute here! Hopefully that issue gets fixed soon.

I'll assign this back to @EHaracic

EHaracic commented 2 years ago

@danjm @bschorchit We have some questions related to the QA testing which we can discuss on the next meeting

EHaracic commented 2 years ago

@bschorchit @danjm Moved to the Pending Consensys until we get the hardware wallet for the QA testing.

mirjanaKukic commented 2 years ago

How to reproduce issue (all these options are with Remind me later to backup SRP option while creating wallet) :

  1. Create new wallet, switch to Ropsten (account with 0ETH), send some amount from another wallet and warning will appear but it shouldn't
  2. Switch to Mainnet, import token (for example UST token) and warning will appear but it shouldn't
  3. Connect to hardware wallet from MM, import one hardware account and import UST token and warning will appear but it shouldn't
  4. When I'm on Mainnet and have some amount on account, warning will appear