Allow passing primaryType to signTypedData/TypedDataEncoder

[tunnckoCore]

Describe the Feature

Currently, it determines it magically - I hate that kind of stuff.
Wasted a few hours to dig into the code until I understand what is happening.

I don't understand why you're doing that. At least allow passing it as last argument to the signTypedData, and if not passed then do your thing. Or at the very least, state it in the docs.

Code Example

Or even better, just allow passing options (or expose similar method) object which is exactly what EIP-712 states, e.g.:

signTypedData({
  types: {
    EIP712Domain: domainTypes,
    ...additionalTypes,
  },
  domain: domainData,
  primaryType: "Foo",
  message: message,
});

or as it's currently but primaryType as last argument:

signTypedData(domain, types, value, primaryType);
signTypedData({ domain, types, message }, primaryType)

[ricmoo]

It isn’t about being magic, it is about needing to be enforced. If you specify if manually, ethers still must calculate it for two reason, 1) to make sure the structure is non-recursive and a DAG, 2) to verify you are actually passing in the correct primary type.

If you pass in unrelated things, you should know about it. This is like a “check for unused variables” check, which is important to discover bugs, since passing unrelated things in is bad, and usually indicates a typo, cut-and-paste issue, etc. Signing data is critical to get right and not have bugs in.

Specifying the primary type (inho) is like having a PNG library that takes in width, height and area; the 3 variables must be checked against each other and is computable. If they ever don’t match, it was a bug and places addition places to cause a bug in the caller.

Keep in mind you are comparing the JSON-RPC api above to the porcelain ethers API. That is a low-level backend API, and only meaningful when using a JSON-RPC backend. Ethers isn’t JSON-RPC-centric. It aims to provide high-level APIs. And I’ve made no secret about my feelings of EIP-712 and its APIs. :p

It also important to know that I am also against magic in the sense of “guessing”. Ethers will never guess if there is any ambiguity. But if the expected type/result/etc. is set in non-ambiguous but can be derived, it is derived.

Hope that helps explain things a bit. I’m moving this to “ideas” though, to continue discussion. :)

[ricmoo]

(as a random aside, that came about during the initial implementation of the feature, which took primary type in as a parameter, and I had a line if (primaryType !== computedPrimaryType) { throw new Error(…); }, which made me ask why it was even passed in. :))

[tunnckoCore]

Thanks. I see where you're coming from. And yet, I still think that it should be possible. If user wants to use it and use it, it will know what it's doing.

A basic Metamask given example:

const domain = [
    { name: "name", type: "string" },
    { name: "version", type: "string" },
    { name: "chainId", type: "uint256" },
    { name: "verifyingContract", type: "address" },
    { name: "salt", type: "bytes32" },
];

const bid = [
    { name: "amount", type: "uint256" },
    { name: "bidder", type: "Identity" },
];

const identity = [
    { name: "userId", type: "uint256" },
    { name: "wallet", type: "address" },
];

it may be obvious to some that bid will be the "primaryType", but hell.. it's not for me 😆 That's where I'm coming from. Or maybe it's just my eyes triggering some anger inside of me LOL... Because, when you think for a bit, it's not that weird because you're writing the code, so you know the relations.

Meh, okay.. 🤪😅

[0xTux]

I'm also facing the same issue. signTypedData doesn't accept the primaryType param and therefore I'm getting TypeError: ambiguous primary types or unused types: error.

[ricmoo]

The code has to compute the primaryType regardless (for safety); all I would do in the function with a passed primary type is assert(passedPrimaryType === computedPrimaryType).

If you feel compelled to pass superfluous data into a any function, you can just always use a comment. :)

Also, please note, the Domain is boiler plate; do not pass it into ethers, since it is required regardless it does it for you. ;)