Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wallet Connect not exposing error.data when custom error is thrown #1000

Closed
makoto opened this issue Apr 29, 2022 · 6 comments · Fixed by #1151
Closed

Wallet Connect not exposing error.data when custom error is thrown #1000

makoto opened this issue Apr 29, 2022 · 6 comments · Fixed by #1151

Comments

@makoto
Copy link

makoto commented Apr 29, 2022

Describe the bug

Wallet Connect does not seem to return the original error from json rpc endpoint when the smart contract throws custom error which was introduced in Solidiy 0.8.4 https://blog.soliditylang.org/2021/04/21/custom-errors/
This leads to ethersjs unable to decode custom error which ccip-read relies on showing ENS offchain record

SDK Version (if relevant)

  • Version 1.7.8

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://offchainexample2.surge.sh/name/1.offchainexample.eth/details
  2. ENS has various record such as ETH/LTC/email/description
  3. Click "Connect" on the left side
  4. Click "Wallet Connect".
  5. Then all the record disappears.
  6. Click "Disconnect"
  7. Then all the record reappear.

Expected behavior

When I put the following console.log at [checkError function of ethers js] (https://github.com/ethers-io/ethers.js/blob/master/packages/providers/src.ts/json-rpc-provider.ts#L52), The provider via Infura (which we connect during read only mode) returned error.data on the left side of screenshot whereas wallet connect doesn't have any data on the right side of the screenshot below.

function checkError(method: string, error: any, params: any): any {
    console.log('***checkError', {
        method, error, params
    })

Screenshots
If applicable, add screenshots to help explain your problem.

Screenshot_2022-04-26_at_20 15 45

@bkrem
Copy link
Member

bkrem commented May 3, 2022

Hi @makoto,

Thank you for the detailed report on this issue regarding the new custom error handling introduced in Solidity v0.8.4, as well as the live reproduction which is a huge help.

@iljadaderko will be looking into a fix for this today, in the meantime is it possible for you to share the source/repo for offchainexample2.surge.sh with us? Being able to run it locally ourselves to validate a possible fix against this concrete example would be the fastest route to patching it.

@makoto
Copy link
Author

makoto commented May 3, 2022

Hi @bkrem . Thanks for taking a look.
You can use ensdomains/ens-app#1468 . The PR has lots of instruction but that's for setting up against local ganache. For pointing to public network, it should be just like this.

git clone https://github.com/ensdomains/ens-app/
cd ens-app
git checkout offchain2
yarn
yarn start

@makoto
Copy link
Author

makoto commented May 9, 2022

Hi @bkrem let me know if there's anything we can help

@xzilja
Copy link
Contributor

xzilja commented May 10, 2022

@makoto Hey, initially we thought that it was a simple addition of data field and made a PR for it here #1006 but after some testing it looks to be more involved. This is on our radar and once some tasks on v2 are finished, I'll get back to it 👍

@makoto
Copy link
Author

makoto commented May 23, 2022

Hi @bkrem just checking on the progress.

@bkrem
Copy link
Member

bkrem commented Jun 11, 2022

Hi @makoto,

Was great meeting you today in person!

I just had a chance to test #1151 against the ens-app offchain branch. It looks like error.data is being passed now and the records are visible after connecting via WalletConnect 🎉

The steps are:

  1. Explicitly add the ethereum-provider prerelease that contains this upcoming fix to the ens-app deps: yarn add @walletconnect/ethereum-provider@next
  2. In web3modal.js, replace the value for providerOptions.walletconnect.package with () => import('@walletconnect/ethereum-provider'),
  3. In setup.js change the provider.removeAllListeners() line to provider.events.removeAllListeners()

Let us know if you're still seeing issues around error forwarding, and sorry again about the wait on this 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants