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

[zgorizzo69]

_signTypedData method on ethers does not match with ERC712 solidity code I am using ethers 5.4.6, openzeppelin/contracts 4.3.1 and hardhat ethers 2.0.2

const MinimalForwarderContractFactory = await ethers.getContractFactory(
    'MinimalForwarder',
  ); // comes from @openzeppelin/contracts/metatx/MinimalForwarder.sol
  const forwarder = await MinimalForwarderContractFactory.deploy();
  await forwarder.deployed();
 let { chainId } = await ethers.provider.getNetwork();
 domain = {
      name: 'Testing',
      version: '0.0.1',
      chainId,
      verifyingContract: forwarder.address,
    };
const Transaction = [
  { name: 'from', type: 'address' },
  { name: 'to', type: 'address' },
  { name: 'value', type: 'uint256' },
  { name: 'gas', type: 'uint256' },
  { name: 'nonce', type: 'uint256' },
  { name: 'data', type: 'bytes' },
];

const types = { Transaction };

 let gasLimit = await ethers.provider.estimateGas({
      to: homeFiContract.address,
      from: signers[0].address,
      data: data,
    });

    let message = {
      from: signers[0].address,
      to: homeFiContract.address,
      value: 0,
      gas: gasLimit.toNumber(),
      nonce: 0,
      data: '0x',
    };

    const signature = await signers[0]._signTypedData(domain, types, message);
    const verifiedAddress = ethers.utils.verifyTypedData(
      domain,
      types,
      message,
      signature,
    );
      expect(verifiedAddress).to.equal(signers[0].address); // works !
       const verifiedFromContract = await forwarder.verify(message, signature);
       expect(verifiedFromContract).to.equal(true); // doesn't work !

in the SC the recovered signer is :0x29501274044eb125ef4d5605b5c25630e722dd44 instead of req.from:0xf39fd6e51aad88f6f4ce6ab8827279cfffb92266 which is indeed the passed hardhat signer 0 Do you have an idea why ?

[zgorizzo69]

Found my problem For info Inside minimalForwarder.sol _TYPEHASH = keccak256("ForwardRequest( so the type Transaction must be renamed to ForwardRequest

[EvilJordan]

I want to confirm that in order to verify an EIP-191 signedMessage, to be compatible with the emerging Sign-In with Ethereum standard, ethers.utils.verifyMessage is my friend. What say you, Ricketh of Moos?

[ricmoo]

@EvilJordan That is correct. It uses EIP-191 as best I know. :)

[ricmoo]

This issue is complete and I won't be changing the API in v5, but in v6 the underscore (_) will be dropped, so I'm going to close it now.

Thanks! :)

[midgerate]

@ricmoo does _signTypedData uses eth_signTypedData_v4 to create the signature? I found that v4 signatures also require primaryType as part of data.

My message contains an array that is only supported in v4 and it seems that metamask mobile (with walletconnect) uses v3 by default, unless explicitly mentioned. It works fine on trust wallet.

[ricmoo]

The primaryType is automatically computed from the request. The ethers API only accepts non-computable parameters, so that validation is implicit.

The JsonRpcSigner uses explicitly specified _v4 though, so I would think it would work?

[danirabbani90]

i am also having issue in calling the signTypedData can you help me out it says it is not a function ?

[ricmoo]

@danirabbani90 In v5 it is ._signTypedData is the method.

[SiestaMadokaist]

emm... may I ask what to do with the signature? how do I combine the signature and the transaction so I can submit it to the blockchain?

[kayceejenz]

@ricmoo, I need your help I've been trying to verify a signature signed by another signer with "ethers.utils.verifyTypedData(domain, types, tokenTransferObj, signature);". But It returns a different signer address from the actual signer. Please, what'd be the problem?

[0xdcota]

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.

I feel this should be more clear on the documentation.

[nftgeek]

@ricmoo, I need your help I've been trying to verify a signature signed by another signer with "ethers.utils.verifyTypedData(domain, types, tokenTransferObj, signature);". But It returns a different signer address from the actual signer. Please, what'd be the problem?

You SHOULD NOT include EIP712Domain when signing using _signTypedData() of ethers. You SHOULD include EIP712Domain when recovering using recoverTypedSignature of @metamask/eth-sig-util

Wasted a day to figure this out.

[nftgeek]

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.

I feel this should be more clear on the documentation.

Exactly. @ricmoo I wish you fix this in the next stable release.

[everdimension]

Hi! I'm testing signing messages using the metamask's example app: https://metamask.github.io/test-dapp/

For eth_signTypedData_v4 it sends an object that Ethers cannot sign:

{"domain":{"chainId":"137","name":"Ether Mail","verifyingContract":"0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC","version":"1"},"message":{"contents":"Hello, Bob!","from":{"name":"Cow","wallets":["0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826","0xDeaDbeefdEAdbeefdEadbEEFdeadbeEFdEaDbeeF"]},"to":[{"name":"Bob","wallets":["0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB","0xB0BdaBea57B0BDABeA57b0bdABEA57b0BDabEa57","0xB0B0b0b0b0b0B000000000000000000000000000"]}]},"primaryType":"Mail","types":{"EIP712Domain":[{"name":"name","type":"string"},{"name":"version","type":"string"},{"name":"chainId","type":"uint256"},{"name":"verifyingContract","type":"address"}],"Group":[{"name":"name","type":"string"},{"name":"members","type":"Person[]"}],"Mail":[{"name":"from","type":"Person"},{"name":"to","type":"Person[]"},{"name":"contents","type":"string"}],"Person":[{"name":"name","type":"string"},{"name":"wallets","type":"address[]"}]}}

The types object here has the following fields:

• EIP712Domain
• Group
• Mail
• Person

I am removing the EIP712Domain as suggested in this topic before passing it to signer._signTypedData, but the Group and Mail are also unused and the method crashes with the ambiguous primary types or unused types error

What's the best solution here? Should the message be considered invalid or should ethers ignore unused types instead of crashing?

[ricmoo]

@everdimension You should remove unused types, it is invalid for ethers. It is not possible to ignore unused types, since ethers constructs the dependency graph to determine what the primaryType is; if there are unused types, there are multiple roots in the DAG, and therefore it is impossible to tell which is the primaryType. The ethers API aims to be minimal, which means not needing the same data specified multiple times.

It is bad form anyways to include random data that isn't used, especially when passion it off to be signed. :)

[0xkakashixyz]

@ricmoo, I need your help I've been trying to verify a signature signed by another signer with "ethers.utils.verifyTypedData(domain, types, tokenTransferObj, signature);". But It returns a different signer address from the actual signer. Please, what'd be the problem?

You SHOULD NOT include EIP712Domain when signing using _signTypedData() of ethers. You SHOULD include EIP712Domain when recovering using recoverTypedSignature of @metamask/eth-sig-util

Wasted a day to figure this out.

Thank you for clearing that up!!! It worked after removing EIP712Domain from types

interface ITypes { name: string, type: string }
interface IEIP712 { 
    types: {ForwardRequest: ITypes[]},
    domain: { name: string, version: string, chainId: number, verifyingContract: string},
    primaryType: string 
}
const ForwardRequest: ITypes[] = [
    { name: 'from', type: 'address' },
    { name: 'to', type: 'address' },
    { name: 'value', type: 'uint256' },
    { name: 'gas', type: 'uint256' },
    { name: 'nonce', type: 'uint256' },
    { name: 'data', type: 'bytes' },
  ];
function getMetaTxTypeData(chainId: number, verifyingContract: string): IEIP712 {
    return {
        types: {
            ForwardRequest,
        },
        domain: {
            name: 'RootForwarder',
            version: '0.0.2',
            chainId,
            verifyingContract
        },
        primaryType: 'ForwardRequest'
    }
}