Ethers.js: Add EIP-712 Ancillary Package for v5.

[ricmoo]

This is a feature a few people have requested.

While I'm not a fan of EIP-712 it is a feature that some people are using, so will need to be supported.

With the upcoming release of v5, this will be easier to add as an ancillary package, which means that people who require it will have access to it, but people without the need for it won't be required to pull in the overly complicated and non-trivial package. :)

This is just a short GitHub Issue to help track progress and let people know it is on the roadmap. :)

[pkieltyka]

@ricmoo just FYI, I've implemented this in another package using ethers v5

https://github.com/arcadeum/ethers-eip712

it's quite succinct and works well. feel free to copy/paste it in, or I can submit a PR .. lmk which package its best suited, but perhaps a new one called "typed-data"

[ricmoo]

I've added this functionality to ethers, please try it out. :)

@pkieltyka I've used MetaMasks library to generate fuzz tests to validate my implementation against, but will look into add yours too, to validate theirs. :)

[pi0neerpat]

Awesome! I'm testing this out by converting an existing web3.js typed data message to use this. Having a bit of trouble understanding this error ambiguous primary types or unused types

ethers 5.0.18

I see the message is coming from here, but not sure why I can't have more than one parent type

https://github.com/ethers-io/ethers.js/blob/86146650d83865d83b10867e9592923eada5d7cc/packages/hash/src.ts/typed-data.ts#L214

My code and logs

    const signer = await walletProvider.getSigner();
    console.log(domain);
    console.log(types);
    console.log(value);
    const signature = await signer._signTypedData(domain, types, value);

domain

{
  "name": "kChannels MVP",
  "version": 1,
  "chainId": 1,
  "verifyingContract": "0x1944517aA5c7D02731A7efAa1023d5C998996461",
  "salt": "0x8a102c4640cfefb2eda4ee058d8783d669df911a16140681aeae27bfcb6c93a3"
}

types

{
  "EIP712Domain": [
    {
      "name": "name",
      "type": "string"
    },
    {
      "name": "version",
      "type": "uint256"
    },
    {
      "name": "chainId",
      "type": "uint256"
    },
    {
      "name": "verifyingContract",
      "type": "address"
    },
    {
      "name": "salt",
      "type": "bytes32"
    }
  ],
  "AuthenticationChallenge": [
    {
      "name": "text",
      "type": "string"
    },
    {
      "name": "client_unpredictable_number",
      "type": "uint256"
    },
    {
      "name": "unpredictable_number",
      "type": "uint256"
    },
    {
      "name": "client_ip",
      "type": "string"
    },
    {
      "name": "issued_at",
      "type": "uint256"
    },
    {
      "name": "expires_at",
      "type": "uint256"
    },
    {
      "name": "issuer_signature",
      "type": "string"
    },
    {
      "name": "signing_identity",
      "type": "address"
    }
  ]
}

value (some values redacted)

{
  "text": "Welcome to Kchannels MVP! Please sign this message to authenticate.",
  "client_unpredictable_number": "33215658837458338000",
  "unpredictable_number": "5213154976",
  "client_ip": "REDACTED",
  "issued_at": "1603321565",
  "expires_at": "1603322165",
  "signing_identity": "REDACTED",
  "issuer_signature": "0x9605a5731e...."
}

[ricmoo]

You should not pass the EIP712Domain into ethers. It will compute it for you. It is seeing that as an alternate possible root.

You can’t have more than one root parent since that would mean you are passing in a bunch of unused types. You can pass in nested structures, just not cyclic ones...

I am considering letting the EIP712Domain and if it is present, ignore it as a ca dosage if there are two roots. But that is a bunch of extra validation that will be required too. It’s on my short list of possible changes.

[pkieltyka]

@ricmoo exciting to have this directly inside of ethers v5. How come the signTypedData method is underscored in the prefix? ie. https://github.com/ethers-io/ethers.js/blob/master/packages/tests/src.ts/test-utils.ts#L773 as well https://github.com/ethers-io/ethers.js/blob/master/packages/tests/src.ts/test-utils.ts#L759

[ricmoo]

The underscore is just temporary until people are happy with the API. Then it will be renamed. :)

[pkieltyka]

Neat, thats a cool way to introduce experimental/new features to prod dists

[pi0neerpat]

EIP712Domain into ethers. It will compute it for you

That did the trick!

Now the server (some else's) is rejecting the signature/message pair. Will report back if I get it fully working.

[markdreyer]

Great work on this! I'm trying to use verifyTypedData to verify the signer address, but it returns a different address than what it was signed with. Am I calling it wrong or is there something with the implementation that changed?

ethers v5.0.23
MetaMask v8.1.6 (chrome extension)

const provider = new ethers.providers.Web3Provider(window.ethereum)
const signer = provider.getSigner()
const domain = {
    name: 'My Messaging App',
    version: '1',
    chainId: 5,
    verifyingContract: '0x7753cfAD258eFbC52A9A1452e42fFbce9bE486cb'
};

const types = {
    Message: [
        { name: 'content', type: 'string' }
    ]
};

const message = {
    content: 'a signed message'
};
signer.getAddress().then(walletAddress => {
    signer._signTypedData(domain, types, message)
        .then(signature => {
            let verifiedAddress = ethers.utils.verifyTypedData(domain, types, message, signature)
            if (verifiedAddress !== walletAddress) {
                alert(`Signed by: ${verifiedAddress}\r\nExpected: ${walletAddress}`)
            }
        })
})
// DEBUG
// walletAddress    = 0x5d2D4CcEDdb0C49972A18D6dD18FAb4cAA3a2F31
// signature        = 0x0b4287a61819d1a4198a3db329eaeed2dcab7d5d6e063d346d348687156ecb8813f23dbc0e1ad241be78b3b0d2f330b3315205217f14de9160c1dd94d8a9c7bc1c
//  verifiedAddress = 0x4A2064A9a3732b651FB27D47b8503a4B2B35f065

[ricmoo]

@markdreyer Thanks for trying it out. I'll investigate first thing tomorrow...

[ricmoo]

Actually, couldn't sleep with this untested...

I've verified there is something not working when using MetaMask... Looking into this now. :)

[ricmoo]

I found the bug! The getPayload function, which is what is passed to the JSON-RPC was not including the EIP712Domain type in the types. I've fixed it locally and will test it a bit more in MetaMask.

I may not get a chance to publish it tonight, as the CI takes about an hour to run, but we'll see. Otherwise it will be posted tomorrow morning.

Thanks for finding this. :)

[markdreyer]

Awesome! Thanks for the quick response and fix. No rush on the deployment - this is something experimental I'm working on so it can wait.

[jamesmorgan]

I am also seeing a similar problem to @markdreyer - thanks for sorting out a fix @ricmoo 🙏

[ricmoo]

@markdreyer @jamesmorgan This should be fixed in 5.0.24. Can you try it out and let me know?

[markdreyer]

thanks @ricmoo it's working like a charm now via MetaMask! I appreciate the quick fix.

[ricmoo]

Awesome! Glad to hear it. :)

[James-Unicrypt]

Hey guys, Im having no luck using signMessage() on mobile wallets such as metamask or trust wallet, it verifies to the incorrect address. Really struggling to get ._signTypedData to work on mobile wallets too. Can anyone provide a decent example? I can confirm signMessage is working on desktop through metamask. Problem is mobile wallets. Anyone else struggling with this?

[ricmoo]

@Mark-UNCX Can you create a brand new private key (and remove it afterward) for the mobile version and include the following for signMessage (create the simplest possible code to demonstrate the issue):

• private key
• the message
• the signature
• the address that is being recovered (incorrectly)
• your code

I don't know what mobile could be doing differently, but that is at least some place I can start to try diagnosing the problem. I've not had any issues with signMessage on iOS (Safari), so please also include the version of the OS and what platforms (Chrome, Safari, etc.)

Also, this probably makes more sense to go into a new issue, so this issue can continue tracking the EIP-712 implementation. :)

[jamesmorgan]

Working as expected now - thanks @ricmoo

[livingrock7]

I dont understand, should I remove EIP712Domain: from types and use ^5.0.24
Any help is appreciated thanks!

[ricmoo]

@livingrock7 yupp. :)

[pkieltyka]

I've been using the _signTypedData method in ethers and looking good so far! one tiny thing I noticed is the typedData object param is being double serialized when sending over the wire -- you can drop the JSON.stringify here https://github.com/ethers-io/ethers.js/blob/master/packages/providers/src.ts/json-rpc-provider.ts#L234

[thegostep]

I am encountering an error when using _signTypedData with hardhat.

linked issue here: https://github.com/nomiclabs/hardhat/issues/1199