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

Support WalletConnect #532

Merged
merged 10 commits into from
Nov 17, 2022
Merged

Support WalletConnect #532

merged 10 commits into from
Nov 17, 2022

Conversation

aomafarag
Copy link
Contributor

@aomafarag aomafarag commented Nov 10, 2022

Closes #353.

Screenshot from 2022-11-10 17-07-40

Checklist:

  • issue number linked above after pound (#)
    • replace "Closes " with "Contributes to" or other if this PR does not close the issue
  • issue checkboxes are all addressed
  • manually checked my feature / not applicable
  • wrote tests / not applicable
  • attached screenshots / not applicable

!!! IMPORTANT !!!

In order to run this PR, you have to provide our Infura ID (just the ID without the URL) as an environment variable under the name INFURA_PROJECT_ID

Testing:

  • Install a WalletConnect-supported wallet on a smartphone (I opted for MetaMask)
  • This will be used for scanning the QR-code when prompted

@aomafarag
Copy link
Contributor Author

General findings

  • WalletConnect is currently updating from V1 to V2
  • V2 is still in early alpha
  • In V1, a Web3 Provider was supported, which is now deprecated, as WalletConnect is moving towards a universal Web3Modal
  • Web3Modal, however, is currently built using the @web3modal/ethereum chain package, which in turn relies on @wagmi/core (React Hooks)
  • It is mentioned that support for Vue is coming soon on the Web3Modal GitHub repo

Technical findings

  • To initialize a WalletConnectProvider, we must provide either an Infura ID or a RPC map (mapping chain IDs to RPC URLs)
  • My recommendation would be to use the Infura ID to avoid the hassle of having to provide all chain IDs and their respective RPC URLs beforehand
  • I managed to resolve the issue I mentioned in the daily on 14.11.2022 regarding the failure to switch networks
  • That issue was actually related to my recommendation: in the initial implementation, I used a RPC map which didn't work because it had to be provided with all possible RPC URLs beforehand, not just the RPC_URL environment variable
  • Moreover, I tested switching accounts and it worked as well
  • However, I couldn't test our custom network because I couldn't register it on my MetaMask wallet on my iPhone: the RPC endpoint which is established by our simulation was not responding, giving a NSURLErrorCannotConnectToHost error

Suggestions/concerns

  • We need to configure our simulation's RPC endpoint to be recognizable by wallets on mobile phones to be able to test it
  • We need to figure out how we are going to test the WalletConnect wallet with various other wallets, not just MetaMask Mobile.

frontend/lib/wallets/WalletConnect.ts Outdated Show resolved Hide resolved
frontend/package.json Outdated Show resolved Hide resolved
@aomafarag
Copy link
Contributor Author

aomafarag commented Nov 16, 2022

Real-world scenario

Scenario :

  • Set RPC_URL to Alchemy (network: Main-like)
  • Set the network on MetaMask mobile wallet to Kovan
  • Try to connect using the QR-code

Results:

  • Connection is successful
  • Network mismatch is detected
  • Network switch successful upon clicking Switch network to mainnet
  • However, console is flooded periodically with the following error messages:
Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://kovan.infura.io/v3/. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing). Status code: 410.

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://kovan.infura.io/v3/. (Reason: CORS request did not succeed). Status code: (null).

Uncaught Error: PollingBlockTracker - encountered an error while attempting to update latest block:
undefined

I have been trying to find out where in our codebase or in the Web3 Provider's source code are these requests made, but I can't pinpoint the issue. Nevertheless, these errors resolve once the switch is completed.

core/src/rpc.ts Outdated Show resolved Hide resolved
frontend/lib/wallets/WalletConnect.ts Outdated Show resolved Hide resolved
frontend/lib/wallets/WalletConnect.ts Outdated Show resolved Hide resolved
frontend/lib/wallets/WalletConnect.ts Show resolved Hide resolved
frontend/lib/wallets/WalletConnect.ts Show resolved Hide resolved
@valiafetisov
Copy link
Contributor

valiafetisov commented Nov 16, 2022

  • Were you able to execute any transactions with the new wallet, eg on the goerli?
  • Where can I find the description of the problem related to simulations or was it resolved?

@aomafarag
Copy link
Contributor Author

aomafarag commented Nov 16, 2022

  • Were you able to execute any transactions with the new wallet, eg on the goerli?

No, because all auctions on goerli are currently inactive + I don't have funds on goerli

  • Where can I find the description of the problem related to simulations or was it resolved?

The description is as follows:

  • A user scans the WalletConnect QR-code on our website to connect their mobile wallet
  • The user's mobile wallet is initially on, for example, mainnet
  • We detect the network mismatch between our website and the user's mobile wallet, and prompt the user to switch to custom
  • The user clicks on the switch button and a wallet_switchEthereumChain request is sent to their mobile wallet
  • An error is returned that the custom network and its chain ID are not recognizable, and that they should be added first with a wallet_addEthereumChain request

@aomafarag
Copy link
Contributor Author

  • Were you able to execute any transactions with the new wallet, eg on the goerli?

Update: Was able to execute transactions on goerli after applying the fix from #540.

@valiafetisov
Copy link
Contributor

they should be added first with a wallet_addEthereumChain request

Have you tried that?

@aomafarag
Copy link
Contributor Author

aomafarag commented Nov 17, 2022

Have you tried that?

I did, and I got this error:

Network switch error: Request for method 'eth_chainId on https://goerli.infura.io/v3/<INFURA_ID> failed

Context

To execute the wallet_addEthereumChain method, WalletConnect requires providing the parameter rpcUrls: string[] with at least one RPC URL; that's why it's mentioned in the error message.

@valiafetisov
Copy link
Contributor

I'm fine with merging it without network switch or support of the localhost (this one is not even available in production) if the flow in general works for @LukSteib

@aomafarag
Copy link
Contributor Author

What about the errors that are output in the console? (see above)

@valiafetisov
Copy link
Contributor

Do you have it on the main branch as well while switching networks?

@aomafarag
Copy link
Contributor Author

Do you have it on the main branch as well while switching networks?

Interestingly, I only get these errors when I use the RPC_URL of Alchemy with this PR (they do not appear on the main branch). When using the RPC_URL of Infura, none of the errors appear with this PR as well as on the main branch.

@LukSteib
Copy link
Contributor

Tested with Zerion Desktop wallet connected through WalletConnect:

  • Could connect to the wallet via WalletConnect ✔️
  • Zerion doesn't seem to support goerli
  • When switching to goerli via our UI this doesn't work (we show error "Invalid Request") -> Probably correct since Zerion doesn't support it
  • Couldn't test a tx since only mainnet is supported by Zerion

Tested Zerion mobile wallet connected through WalletConnect:

  • same outcome as above ✔️

Tested Rainbow mobile wallet connected through WalletConnect:

  • Could connect to the wallet via WalletConnect ✔️
  • Could switch to goerli via our UI ✔️ (had to accept prompt on phone)
  • Could initiate redo of auction on goerli (had to accept prompt on phone)
  • Tx resulted in error Auction redo error: unsupported transaction type: 0x2 (operation="serializeTransaction", transactionType="0x2", code=UNSUPPORTED_OPERATION, version=transactions/5.6.2)
    • Assumption: Rainbow wallet doesn't support goerli tx. Couldn't find evidence for or against this assumption though via quick research

Tested MetaMask mobile wallet connected through WalletConnect:

  • Could connect to the wallet via WalletConnect ✔️
  • Could switch to goerli via our UI ✔️ (had to accept prompt on phone)
  • Could initiate redo of auction on goerli (had to accept prompt on phone)
  • Successfully executed tx ✔️

Given the above I'd approve this PR

@valiafetisov valiafetisov merged commit fbfbe14 into main Nov 17, 2022
@valiafetisov valiafetisov deleted the support-walletconnect branch November 17, 2022 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Investigate WalletConnect support
3 participants