api3dao / api3-dao-dashboard

API3 DAO dashboard
api3.eth/
14 stars 14 forks source link

Use v2 wallet connector (Wallet Connect) #418

Closed mcoetzee closed 1 year ago

mcoetzee commented 1 year ago

What does this change?

It bumps the Wallet Connect version to v2, but it wasn't as simple as updating the version from 1 to 2:

// src/wallet-connect.ts
-connectors: w3mConnectors({ version: 1, chains, projectId }),
+connectors: w3mConnectors({ version: 2, chains, projectId }),

It turns out, when using the v2 connector and you construct a smart contract with a signer, it fails when calling a method on the smart contract that loads data from the chain. Note this is when using a wallet (like Trust/Rainbow) via Wallet Connect, MetaMask works fine. To fix the data loading issue, it boils down to using the provider instead:

-ConvenienceFactory.connect(contracts.convenience, signer);
+ConvenienceFactory.connect(contracts.convenience, provider);

And connecting as the signer when creating transactions:

-api3Pool.stake(parsedValue))
+api3Pool.connect(signer).stake(parsedValue))

I managed to reproduce the error and I opened a discussion to get feedback whether this is expected behaviour or not (I suspect it's expected behaviour).

Side Note

The Market had a weird bug with the v2 connector that forces us to upgrade the wagmi dependency to v1 (that uses viem). The DAO Dashboard doesn't have that bug, so I decided to keep the wagmi dependency as is (at least for now).

mcoetzee commented 1 year ago

FWIW while using ethers, if a signer is created without the optional provider data, it can't be used to read from contracts. However, you can explicitly connect it to a provider after creation, after which it can be used to read from contracts. So maybe the signer or the contracts here could have been connected to the provider to avoid the signer/reader distinction.

Ah that is interesting. Will look into it now 👍

mcoetzee commented 1 year ago

FYI @bbenligiray I didn't have success calling the signer's connect function. It gives the error (it's a JsonRpcSigner): Error: cannot alter JSON-RPC Signer connection (operation="connect", code=UNSUPPORTED_OPERATION)

I also tried the approach of using provider.getSigner(address) when connecting the contracts, but that also didn't work (it fails when creating a transaction).

So it seems that the approach in this PR is the best I got.

bbenligiray commented 1 year ago

No further comments from me