search

oasisprotocol / sapphire-paratime

The Sapphire ParaTime monorepo.
https://oasisprotocol.org/sapphire
Apache License 2.0
34 stars 24 forks source link

clients/js: added hook for Ethers JsonRpcApiProvider.getSigner #284

Open CedarMist opened 5 months ago

CedarMist commented 5 months ago

When the user calls .getSigner on a wrapped Ethers provider, it will return an unwrapped signer.

This change returns a wrapped signer.

netlify[bot] commented 5 months ago

Deploy Preview for oasisprotocol-sapphire-paratime canceled.

Name Link
Latest commit 6b5f46af01e4260534a829342a032b06fad8c39c
Latest deploy log https://app.netlify.com/sites/oasisprotocol-sapphire-paratime/deploys/65f16db2b26c52000892c8a0
CedarMist commented 4 months ago

Rethinking this, you are absolutely right. Can you please add a test to check whether the getSigner() call works as intended?

I don't have dappwrite or synpress setup in CI so I can't run E2E tests that require metamask.

Will add a test for .getSigner() if it works with Hardhat or with a wallet signed in Ethers, but really need to start testing with metamask otherwise we're missing a lot of coverage.

matevz commented 4 months ago

I don't have dappwrite or synpress setup in CI so I can't run E2E tests that require metamask.

Will add a test for .getSigner() if it works with Hardhat or with a wallet signed in Ethers, but really need to start testing with metamask otherwise we're missing a lot of coverage.

synpress would be ideal, but this can go into another PR. I was just thinking of testing that .getSigner() is defined and that it returns the wrapped instance.

CedarMist commented 4 months ago

I literally can't test this without doing it in the browser.

Error: could not coalesce error (error={ "code": "UNKNOWN_ERROR", "message": "could not coalesce error (error={ \"code\": -32601, \"payload\": { \"method\": \"eth_requestAccounts\", \"params\": [  ] } }, payload={ \"method\": \"eth_requestAccounts\", \"params\": [  ] }, code=UNKNOWN_ERROR, version=6.10.0)" }, payload={ "id": 2, "jsonrpc": "2.0", "method": "eth_accounts", "params": [  ] }, code=UNKNOWN_ERROR, version=6.10.0)

When called like:

const s = await ethers.provider.getSigner();

If I hook the provider in the way recommended by the hardhat docs, then I get the above error when trying to get the signer from hardhat.

// See: https://hardhat.org/hardhat-runner/docs/advanced/building-plugins#extending-the-hardhat-provider
extendProvider(async (provider, config, network) => {
  const networkConfig = config.networks[network];
  const { chainId } = networkConfig;
  const rpcUrl = 'url' in networkConfig ? networkConfig.url : '';
  if (chainId) {
    if (!sapphire.NETWORKS[chainId]) return provider;
  } else {
    if (!/sapphire/i.test(rpcUrl)) return provider;

    console.warn(
      'The Hardhat config for the network with `url`',
      rpcUrl,
      'did not specify `chainId`.',
      'The RPC URL looks like it may be Sapphire, so `@oasisprotocol/sapphire-hardhat` has been activated.',
      'You can prevent this from happening by setting a non-Sapphire `chainId`.',
    );
  }
  return sapphire.wrap(provider);
});

But, that does more reliably wrap the provider, so ... the problem is it's bypassing the internal hardhat getSigner function in via: