Here’s maybe a way to do general array hashing: signTypedData for arrays by cag · Pull Request #54 · MetaMask/eth-sig-util · GitHub
Arrays of atomic types have been encoded as a hash of its contents, where its contents are word-aligned according to the type.
Arrays of dynamic types have been encoded as a hash of each element’s hash.
Arrays of structs retain their encoding as a hash of its contents’ concatenated hashStructs.
Arrays of arrays are encoded recursively, as a hash of the contained arrays’ hash encodings concatenated.
Additionally, how should clients handle missing values in an eth_signTypedData structure? eth-sig-util currently skips over fields which are missing, but it uses the same type hash, meaning a struct like Foo(uint256 bar,bytes32 baz) may only have hashStruct(typeHash(Foo) . baz) if bar is undefined on the message. In the PR, because it was simpler to do so in the rewrite, I’ve made every field specified in the typeHash required, but Dan wanted to know if there was a preference for the optional behavior.
Note that this is different from requiring fields like string name to be specified in the EIP712Domain struct. I’m asking about the behavior once the type has been specified whether or not those fields specified should be considered optional.
Personally, I think this should be an error, since using the Foo example, how do you know whether second part of the hashStruct payload refers to bar or baz?
I have a feeling that maybe asking to limit the scope of EIP-712 and introducing this in an EIP extension may be the way to go, as I may have heard of some hardware vendors claiming EIP-712 support, where for them it might actually mean something along the lines of “flat structures containing primitive types” support (I think somebody might have mentioned that being a EIP finalization target). If so, it would be good to get the opinion of relevant stakeholders in this discussion, as I would guess it wouldn’t be feasible to upgrade existing hardware that’s out there claiming EIP 712 support, but I haven’t read through the original discussion thread on Github yet, as it’s pretty long…
It could be that it’s just the crypto primitives being implemented in hardware though, and that software support for this may be added after the fact, in which making this a patch to the EIP could make sense instead.
I’ve done some looking into whether or not this extension can be added to hardware, and it seems that for the majority of the cases, hardware support is for building blocks, and support just has to be added on the Ethereum app level in the case of Ledger or at the firmware level in the case of Trezor. Since this is the case, and because the recent discussion seems to imply dynamic and nested types will be part of the final spec for this EIP, I’ll write an update to this EIP accordingly.
I’ve written a PR to ERC-712 here.
In addition to the changes quoted above, I’ve made all specified struct members mandatory except for structs. The reason why structs are not mandatory is because I’ve realized that the standard intended for recursive types to be possible. As an example, consider:
struct Node {
bytes data;
Node next;
}
This was a valid type in the standard, but it wasn’t clear how to encode instances of this type. With mandatory struct members, this type would not be possible to express. I’ve taken the liberty to state that given the example, the next field may either be encoded recursively as its structHash, or as bytes32(0) when expressing its absence from an instance.