Ethers.js: Add support for `eth_signTransaction`.

[MicahZoltu]

If an app is using its own infrastructure for transaction submission, but something like MetaMask for transaction signing then it needs to be able to call eth_signTransaction rather than eth_sendTransaction.

My specific use case is that I have a library that separates out signing and submission, since signing may be done with something like a hardware wallet, MetaMask, a remote node, in-memory private keys, etc. This way the library can be used with any of those things without having to have divergent code paths for each scenario. It simply does signing as one step, then transaction submission as a separate step.

When I try to use ethers to abstract communication with the remote node (e.g., MetaMask), I have to use provider.send('eth_signTransaction', [ transaction ]). The problem with this is that it means I have to do all of the work that ethers normally does for me (and already has robust logic for) around ensuring that the transaction object is well formed prior to submission. This includes converting all number fields from BigNumber into strings, for example.

[ricmoo]

This makes sense to add to JsonRpcSigner I think. I’ve been considering adding it to the abstract class, Signer as a required method. The main issue with this method, is that it does not automatically populate all the fields, so can easily be used incorrectly, most importantly, the provider chain ID cannot be determined.

Let me mull over this for the general case, and research what the JSON-RPC does with omitted properties.

[ricmoo]

There does not seem to be an eth_signTransaction method specified in any of the API references I can find. Do you have a link to an example?

https://github.com/ethereum/wiki/wiki/JSON-RPC

[MicahZoltu]

I _think_ geth removed it, but it still exists on Parity and I _believe_ exists on MetaMask.

https://wiki.parity.io/JSONRPC-eth-module#eth_signtransaction
It appears MetaMask doesn't have it yet, but there is a lot of demand for it:
https://github.com/MetaMask/metamask-extension/issues/3475

[ricmoo]

I will probably hold off until it is better established then. The eth_signTypedData is becoming a mess because various apps implemented it while the spec was still half-baked, and now it's hard to support things in a standard way. Same thing happened with eth_sign vs personal_sign.

If you call it manually using the JsonRpcProvider.prototype.send method, you can call the JsonRpcProvider.hexlifyTransaction method to sanitize all the values that get passed along. it's a static method used for things that inherit JsonRpcProvider that need to sanitize values.

[PhABC]

@ricmoo is eth_signTypedData still too messy to add support for it in ethers? Seems to have been somewhat stable for the last few months.

[ricmoo]

@PhABC that is a good questions, we had a meeting that included a quick discussion on this during DevCon4. In its current incarnation though, it hadn't convinced us that it was useful or addressed the problems it was trying to. It also adds complexity, with yet another encoding format that is hyper-specific to it.

But I haven't looked at it since then. I'll try to catch up this week on it.

It would be easy enough, for the time being to implement encodeTypedData() as a recipe, and then just use wallet.signingKey.signDigest(keccak256(encodeTypedData(types, data))). Once added to ethers, it will be fairly hard to drop support though, so I would prefer the recipe route for now, until the community at large has agreed it is something they want.

[PhABC]

Does signDigest not prepend the infamous \x19Ethereum Signed Message:\n32? I'll try it out in the coming weeks when I get there :). Thanks for the info!

[ricmoo]

The SigningKey.prototype.signDigest signs the raw digest. The Wallet.prototype.signMessage signs the prefixed message. A Wallet's Signing Key is available at wallet.signingKey. :)

See examples here: https://blog.ricmoo.com/verifying-messages-in-solidity-50a94f82b2ca

[PhABC]

@ricmoo

it would be good to add signDigest() in the Wallet class directly imo, like signMessage(), since as of now signingKey is private.

As of now, I just did:

signerSigningKey = new SigningKey(signerWallet.privateKey)
eip712sig = signerSigningKey.signDigest(hash)

[paulxuca]

Hiya 👋

We're currently trying to use the 0x protocol and they currently use eth_signTypedData across many of their APIs. Is there anyway we could bring support for eth_signTypedData into ethers!

Thanks a ton!

[julien51]

Has anyone found a way to do that when the provider is using Metamask or another webe3 wallet? (it looks lke @PhABC has control over the private keys...)

[ricmoo]

There is no way to sign a digest on a Signer like MetaMask, since that would be far to easy to use to steal all of a user’s crypto assets and ether.

I could simply compute the hash of a transaction, request a signing of it, and then staple the signature to the transaction and it would work, without the owner even knowing what the transaction was. :s

There is no safe way to sign a digest. This is why signing messages requires a prefix, to ensure it is not a valid transaction.

MetaMask does have a signTypedData, I believe, but it is not yet stable (as far as I know) and still has several quirks, but I believe you can use something to the effect of provider.send(“eth_signTypedData_v4”, ...). If anyone knows the exact syntax, please feel free to comment. :)

[julien51]

Sorry for the confusion here. Adding a prefix is certainly required! I would be fine with ethers.js doing that.

[julien51]

I guess I should clarify my question: how to use ethers to sign a typed message using EIP712 when the user has their own provider (MetaMask or other.)?

[ricmoo]

If you just a hash signed in a way that is verifiable, you can use the signer.signMessage method, which will prefix it before signing. To sign a hash you must arrayify it first (note: The string "0x1234" is 6 characters long, the array [ 0x12, 0x23 ] is 2 bytes long).

Check out this article on signing messages: https://blog.ricmoo.com/verifying-messages-in-solidity-50a94f82b2ca

[ricmoo]

I do not currently support EIP712 in ethers. I'm actually kinda hoping it doesn't take off, as the implementation is rather complex and the use cases it handles are quite limited. I have another EIP I will be publishing soon that I think better addresses the concerns, allows for more functionality, and is more flexible, with similar implementation complexity.

That said, I may have to make an EIP-712 signer extension, at least in the v5 experimental package, since there are some things using it today.

To use it, you will need to call the MetaMask signTypedData directly, but I'm not sure on the syntax, and because it was implemented at various stages of the EIP's maturity, there are many versions available. It should still be considered experimental. Hopefully someone else here can post the syntax to call it using the provider.send("eth_signTypedData_v4", [ params ]) syntax. :)

[PhABC]

Any ETA for that EIP @ricmoo ? Currently planning on using EIP-712, but happy to use something else if the alternative is better :).

[ricmoo]

As a side note, this has been implemented for suite some time, using signer.signTransaction. It does not automatically populate fields like sendTransaction does (such as nonce and gasLimit), so those need to be manually specified if you need them in the signed transaction.

Also note that many Signers do not support signing an arbitrary transaction for security reasons. For example, I do not think MetaMask permits it (but maybe that has changed?).

There is a separate issue to track EIP-712 too. :)

So, I'm closing this issue, but please feel free to re-open if you feel something has not been addressed.

Thanks! :)