starknet-io / get-starknet

StarkNet wallet <-> dApp bridge
MIT License
154 stars 110 forks source link

Feature Request: Expose Wallet Lock State & Disconnect Event #192

Open RyanTimesTen opened 11 months ago

RyanTimesTen commented 11 months ago

Background

At Dynamic, we have partnered with StarkWare to build a bridge component as seen on https://starkgate.starknet.io/. We aim to create a smooth login experience for end users. We use get-starknet-core to connect users' starknet wallet, whether it's Braavos or ArgentX.

Problem

There are two scenarios that could have improved UX, and they both begin at a state where the user has already connected a starknet wallet:

  1. The user explicitly locks their wallet while it is connected to a dapp and the dapp is open in a browser tab
  2. The user locks their wallet while the dapp is not open (either via a browser restart, or by closing the dapp window and explicitly locking their wallet)

Additionally, when dealing with wallets in a locked state, we want to avoid uninitiated wallet popups – meaning, there should not be any wallet popups without the user explicitly clicking on a connect button.

The starknet window object (accessed via get-starknet-core) has a lot of useful properties and methods like isConnected and isPreauthorized, however these cannot be used for accurately detecting wallet locked state while avoiding popups. The main reason for this is that isConnected is always false until enable is called. However, when enable is called, it will prompt the user to unlock the wallet if it's locked, so we have a chicken-and-egg problem.

Proposed solution

There are two parts, each addressing the problems above, respectively:

  1. Emit an accountsChanged event with undefined account address when the wallet is locked
  2. Add a flag to enable that states the dapp doesn't want to show a popup/UI in case the wallet is locked and/or not preauthorized

Alternate solution

  1. Emit a disconnect event when the wallet is locked
  2. Expose an isLocked (or isUnlocked, whichever is preferred) value that represents the current locked status of the wallet, whether or not enable has been called.

Disconnect event When the user explicitly locks their wallet, the wallet should emit a disconnect event that can be handled via the on method, similar to accountsChanged and networkChanged. This will solve the first UX scenario because in our disconnect event handler, we can disconnect the wallet from our app state.

isLocked There should be a boolean value isLocked accessible via get-starknet-core similar to isConnected that is:

Additionally, this boolean should have a value before calling enable – it should not be necessary to call enable for this property to have the correct value. This will solve the second UX scenario because our SDK can read this value and determine if it needs to remove the wallet from its connected state. For a reproduction of the UX problem we aim to solve, see: https://codesandbox.io/s/purple-glade-l3llkm?file=/src/App.js. The problem is with the automatic popup – since we have to call enable first to check if isConnected is true, there is a wallet connection popup on page load. If we were able to check isLocked, then we would know to simply "disconnect" the user (reset their connection state in our app), and then show them the connect button again.

avimak commented 11 months ago

thanks @RyanTimesTen.

  1. Perhaps we could get by with the existing set of events (account/network changed) and just emit, on the wallet side, an undefined account-address once the user locks the wallet?

  2. How about adding a flag to enable that states the dapp doesn't want to show a popup/UI in case the wallet is locked and/or not preauthorized? This way, you could call it naively once the user chooses a wallet. If it works (e.g. you got an address back), perfect; if not, you'll get undefined for the address and realize that either you aren't preauthorized or the wallet is locked.

RyanTimesTen commented 11 months ago

@avimak

  1. i could see that, although a proper disconnect event is be clearer and more explicit IMO. this event could also be emitted when the user removes the dapp connection from their wallet
  2. that would work
avimak commented 11 months ago
  1. Isn't disconnect/connect on L1 more about network connectivity rather than the dapp<->wallet connection?
  2. I think it's a good solution that covers your needs without exposing sensitive information about the wallet's state.
RyanTimesTen commented 11 months ago
  1. you're right – that's true
  2. 👍
RyanTimesTen commented 11 months ago

i updated the proposal with your suggestions, thank you

avimak commented 11 months ago

@RyanTimesTen wrt silent-mode, pls see https://github.com/starknet-io/get-starknet/pull/194 (RequestAccountsParameters); will be implemented as part of that PR

RyanTimesTen commented 11 months ago

thanks @avimak. both Braavos wallet and ArgentX will need to release updates to support this, right?

avimak commented 11 months ago

yes, cc @janek26