wevm / wagmi

Reactive primitives for Ethereum apps
https://wagmi.sh
MIT License
6k stars 1.07k forks source link

Metamask is connected but signer is absent #444

Closed hhff closed 2 years ago

hhff commented 2 years ago

Is there an existing issue for this?

Package Version

0.0.3

Current Behavior

I'm developing primarily against Metamask, and I've noticed that if I return to the project after a day or so (maybe I reset my computer), I find my project in a state where account is present, but signer is null.

For example, in this state, this code...

const Wallet = () => {
  const { data: account } = useAccount();
  const { data: signer } = useSigner();

  console.log('account', account);
  console.log('signer', signer);

Will produce these logs:

image

I have noticed that this case is when Metamask is re-prompting for password:

image

Expected Behavior

I'm under the impression that account and signer should both be present, or both absent; but never partially. (This may be a misunderstanding on my part).

Steps To Reproduce

I'm pretty sure that restarting MacOS will require that Metamask wants you to reconfirm your password.

Link to Minimal Reproducible Example (CodeSandbox, StackBlitz, etc.)

No response

Anything else?

It's most likely that this is expected behavior, given that it seems to be related to Metamask's internals. But my question for this library is -- how do you practically help a user recover from this state? What's wagmi's expected UX here for this case?

THANK YOU SO MUCH FOR THIS LIBRARY IT IS FANTASTIC

benjaminshafii commented 2 years ago

+1 we used to have a working flow with another library (useWallet) but hit into this issues with WAGMI

tmm commented 2 years ago

@hotkartoffel can you link to useWallet?

wagmi listens for when MM (or other injected providers) lock while a page that uses wagmi is open. Unfortunately, if MM is locked and the page is closed, wagmi isn't able to catch the event.

benjaminshafii commented 2 years ago

@tmm https://github.com/aragon/use-wallet

Will also provide:

  1. Two versions of our app w/ wagmi and w/ useWallet
  2. Our public github repo
tmm commented 2 years ago

@hotkartoffel sounds great. please tag me when you post the links.

benjaminshafii commented 2 years ago

Before wagmi

App -> https://daonative-app-db9nowhcd-prologe.vercel.app/nfts/create

How to reproduce:

  1. click on connect
  2. connect with metamask
  3. go to different webpage
  4. lock metamask
  5. Go pack to url
  6. fill in random info and click "Create collection"

Observed behavior: metmask pops up at step 5 and asks you to enter your password

After wagmi

App -> https://daonative-app-myp8jc48k-prologe.vercel.app/nfts/create

How to reproduce:

  1. click on connect
  2. connect with metamask
  3. go to different webpage
  4. lock metamask
  5. Go pack to url
  6. fill in random info and click "Create collection"

Observed behavior: metamask shows you as connected (see top right), and clicking on "create collection" shows N.getSinger not a function

@tmm

Extra infos:

hhff commented 2 years ago

Thank you everyone. It's probably worth noting that I observed this behavior before 0.0.3, also.

If the solution is simply "catch the intersection of !!account && !signer and trigger disconnect() that's AOK. Just want to be sure I'm handling this "the wagmi way".

tmm commented 2 years ago

Opened up https://github.com/tmm/wagmi/pull/447 for this. When the wagmi client attempts to autoconnect and no connection is made (e.g. MM is locked, wallet app disconnected when page is closed, etc.), the account is cleared since it is no longer connected.

hhff commented 2 years ago

Thank you @tmm - will check back when a new release is cut to validate it's working as expected.

hhff commented 2 years ago

Hi @tmm - checking in to see when you're next planning to cut a release? (I know I could install from github but I'm not sure if main is considered stable)

tmm commented 2 years ago

very soon! been delayed with some other work this week.

github-actions[bot] commented 10 months ago

This issue has been locked since it has been closed for more than 14 days.

If you found a concrete bug or regression related to it, please open a new bug report with a reproduction against the latest wagmi version. If you have any other comments you can create a new discussion.