gigawatson commented 3 years ago

Up to date Chrome v93.0.4577.82 and web wallet v2.16.0.

I signed out of the StacksArt site and was unable to "Connect Wallet" again. So I uninstalled the web wallet, cleared cache/storage, restarted Chrome, and reinstalled the web wallet.

After about 7-8 minutes of waiting for the wallet to accept my seed (just spinning wheel) it finally connected however I am unable to switch to my other accounts as the "Switch Account" option is not there.

And I am still unable to connect to any service (StacksArt, MineCityCoins, etc.)

gigawatson commented 3 years ago

Just tried Firefox wallet v2.17.0 and the "Switch Account" option just disappeared before my eyes.

I am now unable to access all but one account.

gigawatson commented 3 years ago

I tried to "Create an Account" to see what would happen and it revealed the "Switch Account" menu option with my main account and one of my secondary ones. I had to repeat the "Create an Account" process to reveal all of my accounts, one at a time.

I am, however, still unable to "Connect Wallet" to any service.

markmhendrickson commented 3 years ago

This is odd behavior. I suspect the wallet may be struggling to get relevant data from the API, though we'd have to investigate.

Could you post a screen-capture video here of the behavior you see so we can observe just what's happening as well?

andresgalante commented 3 years ago

I am trying to reproduce it, but I can't either in Chrome (2.16) nor FireFox (2.17)

gigawatson commented 3 years ago

Today I'm able to connect again. I didn't manage to video the issue and I'm a bit reluctant to sign out again at the moment. I'll give it a try once the MIA cycle 3 begins.

markmhendrickson commented 3 years ago

As commented in a seemingly related issue (https://github.com/blockstack/stacks-wallet-web/issues/1721#issuecomment-924824435), we'll have to close this out and reopen if we see more reports with available diagnostic information.

markmhendrickson commented 3 years ago

I'm reopening since similar behavior to https://github.com/blockstack/stacks-wallet-web/issues/1732#issuecomment-923179151 (the need to click "Create Account" to see existing ones) has been reported on Twitter as well:

https://twitter.com/AndyCoins/status/1446162920539295744

kyranjamie commented 3 years ago

When creating a new account/stx address, the account's gaia hub is updated, so the wallet is later able to know how many accounts have been previously generated (even if there's no activity). The wallet checks this resource everytime the user logs into their wallet.

If something were to go wrong the account's gaia store, or connection to gaia, the wallet's behaviour is affected. There's no error handling at all.

Blocking requests to hub.blockstack.org/*, I get an infinite spinner when creating an account.

Further, you're unable to even sign in to your wallet, getting an erroneous incorrect password error, when its blocked.

This is an extremely serious bug, and I'm surprised we haven't had more reports. A testament to the availability of our Gaia hub 👏🏼 . This issue highlights the need for https://github.com/blockstack/stacks-wallet-web/issues/1822, rethinking the "vaultReducer" approach to handling wallet data.

Suggestion

We immediately dd error handling, so that checking of an account's gaia hub has no bearing on whether or not the wallet works
- The behaviour of "finding out whether accounts are generated", could instead simply look for whether there's account activity (removing reliance on gaia)
- Curious on product input of the importance of this feature @markmhx
We educate users in the wallet, with ? icon somewhere, that create account is really just seeing the next generated address. Something to avoid confusion whenever the mechanism of checking for "known accounts" doesn't work

kyranjamie commented 3 years ago

Reopening this issue. Even though my PR fixes the most critical bug, I'm not sure if this issue relates entirely to it. This is partly a communication/design issue imo.

If we assume a newly-restored wallet cannot always know what accounts have been made, how can our UI communicate that "Create new account" is needed to regain access to them? @markmhx

markmhendrickson commented 3 years ago

To be clear about where we stand here given your above PR:

If the wallet doesn't connect to Gaia, will it simply show one account (the first) by default and the user can then select "Create Account" to reveal others one by one?

And will these other accounts remain available in the wallet installation as long as the user A) doesn't sign out or B) doesn't uninstall and reinstall?

Or is the behavior worse in the sense that the other accounts disappear each time the user opens the wallet extension or something?

kyranjamie commented 3 years ago

If the wallet doesn't connect to Gaia, will it simply show one account (the first) by default and the user can then select "Create Account" to reveal others one by one?

Yes

And will these other accounts remain available in the wallet installation as long as the user A) doesn't sign out or B) doesn't uninstall and reinstall?

No, if Gaia is down, when returning to the wallet, they'll be back to account 1. In fact, I've just noticed that if you're on account 20, gaia goes down, and then you return to wallet, it'll break with a white screen. Subsequent attempts will then revert to account 1.

My feeling here is that we should only ever touch Gaia when:

A wallet is restored. We can check gaia, to see if there's info on how many accounts they've previously generated
A new account is created. We can update Gaia to let it know which accounts've been generated.
When a user authenticates to a multiplayer app, this should result in an update to the profile.json. The profile is stored on Gaia. (thanks @aulneau)

Right now, we check how many accounts they've generated every time unlock the wallet. Which is a lot of chances for things to go wrong.

Better, might be to persist locally the highest index of accounts they've generated. Then, we've no reliance on an external resource to restore n accounts.

aulneau commented 3 years ago

My feeling here is that we should only ever touch Gaia when:

A wallet is restored. We can check gaia, to see if there's info on how many accounts they've previously A new account is created. We can update Gaia to let it know which accounts've been generated.

You're missing the third one:

When a user authenticates to a multiplayer app, this should result in an update to the profile.json. The profile is stored on Gaia.

markmhendrickson commented 3 years ago

@kyranjamie those changes seem reasonable to me and don't appear to require any UI changes, correct? Should we tackle them first then address any subsequent UI changes that may be needed to clarify "Create Account" vs. "Restore Account" given remaining edge cases?

kyranjamie commented 3 years ago

1732 fixes the most critical issue bug I found.

But some additional UI to describe further what "creating an account" really does, would more directly resolve the issue described by OP.

markmhendrickson commented 3 years ago

I meant to say, can we make these following Gaia interaction changes without having to touch the UI necessarily?

My feeling here is that we should only ever touch Gaia when:

A wallet is restored. We can check gaia, to see if there's info on how many accounts they've previously generated A new account is created. We can update Gaia to let it know which accounts've been generated. When a user authenticates to a multiplayer app, this should result in an update to the profile.json. The profile is stored on Gaia. (thanks @aulneau)

I agree we can improve the UI but want to decouple changes if possible.

markmhendrickson commented 2 years ago

@kyranjamie did you resolve this with https://github.com/hirosystems/stacks-wallet-web/issues/1877?

markmhendrickson commented 2 years ago

@kyranjamie says this remains unresolved and should be addressed by removing the Gaia dependency. However, it seems very rare that this issue occurs in wild for now and isn't critical, so I'm downgrading to P4.

markmhendrickson commented 2 years ago

We're going to tackle this as part of the addition of initial Bitcoin support since we need to revamp how we determine which accounts to display using both Stacks and Bitcoin-based chain data.

markmhendrickson commented 3 months ago

@kyranjamie is this resolved based on our previous work as far as you remember?

leather-io / extension

Fix need to select "Create Account" to restore missing existing accounts #1732

1732 fixes the most critical issue bug I found.