[CD] | provider: add nonce and is web3 provider methods

[cbrzn]

adds get nonce of the signer & check if it is connected through a wallet (aka Web3Provider)

this PR updates the interface of the ethereum provider which needs to be updated in ENS once this is merged

[krisbitney]

For me, this name is not intuitive. I would expect isWallet to return true if the signer itself is a wallet (e.g. an instance of ethers.Wallet). It sounds like this method instead distinguishes between web3provider and jsonRpcProvider. Is that correct?

It is possible to interface with a wallet via RPC, like we do with Ganache

And with MetaMask, we would generally want to call sendTransaction, not sendRawTransaction

That is my understanding

[krisbitney]

The reason for using sendTransaction is so that the wallet can handle both signing and sending securely. My understanding is that signing a transaction and then sending it with sendRawTransaction is not secure, and most wallets don't even allow it.

Have you tested this with MetaMask to make sure it works?

[cbrzn]

It sounds like this method instead distinguishes between web3provider and jsonRpcProvider. Is that correct?

that's correct - maybe doing isWeb3Provider would be better?

It is possible to interface with a wallet via RPC, like we do with Ganache

yes, it's possible, but neither infura or alchemy accept sendTransaction because they don't manipulate signing keys. Meaning that currently, we're only able to execute transactions using a private key in testnet, but not in any other chain (we are not able to execute ethereum transactions in a script like this, for example)

And with MetaMask, we would generally want to call sendTransaction, not sendRawTransaction

metamask uses sendTransaction to make sure that the private key used to sign the transaction never leaves the user's device, which in our case it, might not always be that way. the reason is that we accept Wallet object (which has access to these private keys) as a signer (besides a Web3Provider). so effectively with metamask we are still doing sendTransaction

The reason for using sendTransaction is so that the wallet can handle both signing and sending securely. My understanding is that signing a transaction and then sending it with sendRawTransaction is not secure, and most wallets don't even allow it.

while I completely understand your point I think that we're not a wallet but rather a development tool (that probably wallets will eventually use!) and we should allow users to interact and execute transactions in any way they would like to

Have you tested this with MetaMask to make sure it works?

yes! i tested with safe wrapper demo app and worked as expected

[krisbitney]

This is just my understanding of things at the moment.

I never said we should limit how users use the wrapper. I am just saying that I thought signing and sending raw transactions was supposed to be less secure. If so, they shouldn't be used in cases where a more secure option is available. I could be wrong about all of this!

Does Infura or Alchemy provide wallets? I thought they were always providers without signers

I think with MetaMask we can always do eth_sendTransaction, right? Or do we need to use eth_sendRawTransaction?

[cbrzn]

Does Infura or Alchemy provide wallets? I thought they were always providers without signers

they do not, that's why we need to sign it locally

I think with MetaMask we can always do eth_sendTransaction, right? Or do we need to use eth_sendRawTransaction?

you're right! with metamask or any web3provider we will use eth_sendTransaction (and that's what we're doing in the other PR, if the provider is a Web3Provider (currently checked with isWallet method), we do eth_sendTransaction)

just for clarification, when I said I tested with metamask, I meant that the eth_sendTransaction is being done as expected, and with a script (giving the private key) we're doing eth_sendRawTransaction

[krisbitney]

Okay, so to clarify, the Safe wrapper is using a private key to sign transactions but does not have access to an Ethereum provider, so you need to send the transactions using something like Infura?

What if we just use two methods? One would be syntax sugar for signing and sending raw transactions (which is already possible), the other for handling standard transactions. That way we are still calling eth_sendTransaction with wallets that have signers but are not of type Web3Provider.

[cbrzn]

Okay, so to clarify, the Safe wrapper is using a private key to sign transactions but does not have access to an Ethereum provider, so you need to send the transactions using something like Infura?

in a node js script context, we need to pass the private key (and a JSON RPC, like infura or alchemy) - but in the demo app all works fresh and uses eth_sendTransaction (we use the metamask provider)

What if we just use two methods? One would be syntax sugar for signing and sending raw transactions (which is already possible), the other for handling standard transactions. That way we are still calling eth_sendTransaction with wallets that have signers but are not of type Web3Provider.

yes I like this!

[krisbitney]

Awesome, okay. I apologize for making this more complicated!

[cbrzn]

closing this in favor of #45