leather-io / extension

Leather browser extension
https://leather.io
MIT License
294 stars 142 forks source link

Unable to sign into Firefox with password (wasm eval warning) #2002

Open markmhendrickson opened 2 years ago

markmhendrickson commented 2 years ago

Submission remains pending forever here:

Screen Shot 2021-11-27 at 09 41 18

Reported here and seen by yours truly as well: https://discord.com/channels/621759717756370964/625538774230892545/913782930281754664

Screen Shot 2021-11-27 at 09 41 24

Seems similar to https://github.com/hirosystems/stacks-wallet-web/issues/1902 for Chrome

timstackblock commented 2 years ago

https://github.com/hirosystems/stacks-wallet-web/issues/1750

beguene commented 2 years ago

I could not really reproduce the 'infinite' but it can be quite slow sometimes even on chrome. I am working on 2 code fixes which significantly reduce this time and should also fix https://github.com/hirosystems/stacks-wallet-web/issues/1750 and https://github.com/hirosystems/stacks-wallet-web/issues/2014

kyranjamie commented 2 years ago

@beguene will this involve addressing https://github.com/hirosystems/stacks-wallet-web/issues/1877 to remove the request to Gaia on every sign in? Curious what other changes there are to make this faster.

beguene commented 2 years ago

@kyranjamie yes this will address #1877 too

andresgalante commented 2 years ago

The current plan to address this issue is to first

If the above doesn't show enough improvemnts then we'll consider changes on Argon2's settings https://www.notion.so/hiropbc/Argon2-Optimisation-f7132735a496499dbb94b045ccf37b52

beguene commented 2 years ago

@andresgalante Regarding "Add metrics to understand the impact of that change", there is already a metric added by @kyranjamie before, which englobes the decrypt function and the call to Gaia. Now that we removed the call to Gaia on unlock, we can track the same metric which will only measure the decrypt function (which includes argon2 hash). There is no need to add any new metrics I think.

beguene commented 2 years ago

Moving it to backlog as we need to check the analytics. This could be picked up again in a few weeks.

markmhendrickson commented 2 years ago

Which metric should we check here exactly? And is the idea that we need to confirm whether this problem is indeed still happening (commonly) in production?

danieltanfh95 commented 2 years ago

is there anyway I could help to accelerate this? it's blocking users on firefox, making it seem that Stacks/Hiro is very unreliable / rug.

zommuter commented 2 years ago

Can anyone explain what the issue here is? Why does the unlocking take forever if it works at all on Firefox? This is horrible UX and probably one of the major reasons for a lack of users. Here's a screenshot from the profiler, though I guess you already know that the problem lies in decryption-worker.js i.e. src/shared/workers/decryption-worker.ts.

image

Not sure how reliable the profiler's line reference is, that would mean the import argon2, { ArgonType } from 'argon2-browser'; already causes the problems 🤷‍♂️

markmhendrickson commented 2 years ago

@zommuter did you intend to comment on this issue with the same content as here?

@beguene what metrics can we check here exactly to gauge the volume of this problem?

beguene commented 2 years ago

@markmhx you need to check 'start_unlock' and 'complete_unlock' 'complete_unlock' has a 'duration' properties in ms You should also check 'start_unlock that is not followed by a 'complete_unlock'. Those are users that could not complete the unlock or exited before completion.

markmhendrickson commented 2 years ago

Note that the last few comments in particular appear related more closely to https://github.com/hirosystems/stacks-wallet-web/issues/2014 than a wasm eval warning per se.

kyranjamie commented 2 years ago

This issue is still open https://github.com/hirosystems/stacks-wallet-web/issues/1750 and I wonder if some users are running into it: where an error's thrown, and the behaviour is an infinite spinner.

Regarding users giving up, maybe we can fire an event to background script to log an event, when the session is closed.