MetaMask / snaps

Extend the functionality of MetaMask using Snaps
https://metamask.io/snaps/
Other
721 stars 554 forks source link

Teardown behaves unpredictably before opened session #959

Closed ohyung1287 closed 1 year ago

ohyung1287 commented 1 year ago

I followed the ether-js example to initialize an ethers instance and test with the provider function return await provider.getBlockNumber() I expect the HTML screen will show the current block number from Snap, but the Snap request not respond and timeout eventually. I have connected Metamask with frontend, and the ethers is working if I call it in the frontend. Does anyone have this issue? I just want to connect my smart contract in Snap and run it. Thanks

FrederikBolding commented 1 year ago

Hi @ohyung1287 - Could you share the source code of your snap please?

ohyung1287 commented 1 year ago

I only changed one line in examples/ethers-js/src/index.js of line 37

case 'address':
      return await provider.getBlockNumber();

and I expect when clicking Get Address button, I can get the latest block number of my connected chain (Ethereum Goerli)

ohyung1287 commented 1 year ago

I have tried running ethers in html directly by adding

  <script
  src="https://cdn.ethers.io/lib/ethers-5.2.umd.min.js"
  type="application/javascript">
</script>

and it works fine. But when I run ethers inside the Snap it never responds so I wonder if anything I did wrong

ohyung1287 commented 1 year ago

Ok, I got it. In the example code provider is initialize in the global but the wallet obj is actually undefined atm

const ethers = require('ethers');
const provider = new ethers.providers.Web3Provider(wallet);
module.exports.onRpcRequest = async ({ request }) => {
  console.log('received request', request);
  const privKey = await wallet.request({
    method: 'snap_getAppKey',
  });
  ...

By moving const provider = new ethers.providers.Web3Provider(wallet); into onRpcRequest solves the issue.

ritave commented 1 year ago

That's weird. @FrederikBolding maybe you have an idea why a wallet be undefined in global scope?

Last time I wrote a snap with ethers, the above snippet would work. Did something change?

FrederikBolding commented 1 year ago

@ritave Wallet doesn't look to be undefined. My guess would be that teardown of timers breaks this: https://github.com/ethers-io/ethers.js/blob/master/packages/providers/src.ts/json-rpc-provider.ts#L414-L424

ritave commented 1 year ago

Thanks Frederik. @ohyung1287 can you confirm that the wallet object itself had actually a value of "undefined"?

ohyung1287 commented 1 year ago

@ritave @FrederikBolding I just double check again, the wallet object is NOT undefined and the provider that initialized in the global is also not. I don't know the reason but if I initialize the provider globally, it'll not respond when I call it in onRpcRequest. The solution for now is just to initialize it inside the onRpcRequest. I'm sorry for misdirecting

ritave commented 1 year ago

Thank you! That helped debug the issue!

Ok, the teardown works as expected - we first start the snap to register exports and then we kill all outgoing connections and timeouts. You should have gotten warnings in the console.log of the background page.

Currently those errors are easily missable. In the future we'd like to expose those warnings in a better way.