search

ourzora / zdk

MIT License
107 stars 42 forks source link

Shouldn't Wallet, the argument of function approveERC20, have a signer or provider interface? #45

Closed serinuntius closed 3 years ago

serinuntius commented 3 years ago

Hi, Thank you for developing a great protocol.

refs: https://github.com/ourzora/zdk/blob/3451b4fb97b733c64a82abe1212d280daae7691f/src/utils.ts#L581-L589

I think wallet of approveERC20 should be signer or provider.

Because, as you know, For connect, You need Singer | Provider.

スクリーンショット 2021-02-19 11 59 11
sporkspatula commented 3 years ago

Hi @serinuntius great question. Wallet implements the signer abstract class so conforms to the signer | provider parameter. The reason why it must be a signer is because approveERC20 is actually calling a write contract function that will sign a transaction and submit it to the blockchain. Signers are the only class that have the ability to be able to sign transactions and submit them to their provider.

You can think of a provider as a fancy word for an ethereum node or way to connect to the network. They are useful for executing contract view functions that don't require state changes.

I hope this answer is helpful! I am going to close this issue.

serinuntius commented 3 years ago

Hi @sporkspatula. Thank you for the very detailed explanation. I saw your reply and thought it was not your problem, so it's perfectly fine to close the issue. Instead, let me share about the problems I have encountered. And if you know of any solutions, I would appreciate it if you could let me know.

I am using web3-react for front-end development. I couldn't figure out how to use wallet with web3-react, so I was looking for a way to solve the error. Then it occurred to me that approveERC20 might not need wallet.

const { library, chainId } = useWeb3React<Web3Provider>();
const signer = library.getSigner();
await approveERC20(signer, '0x~~~~', '0x~~~', ''); // There is an error on this line
TS2345: Argument of type 'JsonRpcSigner' is not assignable to parameter of type 'Wallet'.   Type 'JsonRpcSigner' is missing the following properties from type 'Wallet': address, _signingKey, _mnemonic, mnemonic, and 3 more.

Is there a way to change the signer to a wallet? Do you really need a wallet?

I was able to approve using signer in the following way.

const erc20 = BaseErc20Factory.connect('0x~~~~', signer);
await erc20.approve(zora.marketAddress, MaxUint256);

I may be using rude English, sorry.