Broken EIP-712 support: all EIP712Domain fields should be optional

[xJonathanLEI]

Version

N/A

Platform

N/A

Description

According to the spec, all fields in EIP712Domain should be optional:

Protocol designers only need to include the fields that make sense for their signing domain. Unused fields are left out of the struct type.

whereas in ethers-rs currently only salt is:

ethers-rs/ethers-core/src/types/transaction/eip712.rs

Lines 106 to 124 in 916e9a7

	pub struct EIP712Domain {
	/// The user readable name of signing domain, i.e. the name of the DApp or the protocol.
	pub name: String,
	/// The current major version of the signing domain. Signatures from different versions are not
	/// compatible.
	pub version: String,
	/// The EIP-155 chain id. The user-agent should refuse signing if it does not match the
	/// currently active chain.
	pub chain_id: U256,
	/// The address of the contract that will verify the signature.
	pub verifying_contract: Address,
	/// A disambiguating salt for the protocol. This can be used as a domain separator of last
	/// resort.
	pub salt: Option<[u8; 32]>,
	}

The library pre-computes two type hashes for the domain, but in reality all fields can be optional:

ethers-rs/ethers-core/src/types/transaction/eip712.rs

Lines 16 to 32 in 916e9a7

	/// Pre-computed value of the following statement:
	///
	/// `ethers_core::utils::keccak256("EIP712Domain(string name,string version,uint256 chainId,address
	/// verifyingContract)")`
	pub const EIP712_DOMAIN_TYPE_HASH: [u8; 32] = [
	139, 115, 195, 198, 155, 184, 254, 61, 81, 46, 204, 76, 247, 89, 204, 121, 35, 159, 123, 23,
	155, 15, 250, 202, 169, 167, 93, 82, 43, 57, 64, 15,
	];
	/// Pre-computed value of the following statement:
	///
	/// `ethers_core::utils::keccak256("EIP712Domain(string name,string version,uint256 chainId,address
	/// verifyingContract,bytes32 salt)")`
	pub const EIP712_DOMAIN_TYPE_HASH_WITH_SALT: [u8; 32] = [
	216, 124, 214, 239, 121, 212, 226, 185, 94, 21, 206, 138, 191, 115, 45, 181, 30, 199, 113, 241,
	202, 46, 220, 207, 34, 164, 108, 114, 154, 197, 100, 114,
	];

[xJonathanLEI]

I'm working with the Gnosis Safe contract and it only uses chainId and verifyingContract for the domain. I can't get it to work without manually implementing Eip712.

[mattsse]

these are now optional

#1510

#1485

vote close @gakonst