Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EIP712Domain missing chainId #11

Closed
BlinkyStitt opened this issue Nov 6, 2018 · 7 comments
Closed

EIP712Domain missing chainId #11

BlinkyStitt opened this issue Nov 6, 2018 · 7 comments

Comments

@BlinkyStitt
Copy link
Contributor

BlinkyStitt commented Nov 6, 2018

I'm trying to get an order hash from an order and noticed a difference between the official EIP712 example and what is in the spec here.

https://github.com/ethereum/EIPs/blob/master/assets/eip-712/Example.sol

struct EIP712Domain {
    string  name;
    string  version;
    uint256 chainId;
    address verifyingContract;
}

https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#eip712-usage

// Hash of the EIP712 Domain Separator Schema
bytes32 constant internal EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked(
    "EIP712Domain(",
    "string name,",
    "string version,",
    "address verifyingContract",
    ")"
));

bytes32 EIP712_DOMAIN_HASH = keccak256(abi.encodePacked(
    EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH,
    keccak256(bytes("0x Protocol")),
    keccak256(bytes("2")),
    bytes32(address(this))
));

The 0x spec is missing chainId. Is this intentional? It seems like it will be a problem.

Also, what address is the "this" referring to? The only place I see "verifyingContract" used in the spec is in the code I've pasted.

On a related note, are there any examples of valid orders and hashes so I can verify that what I'm doing matches what the protocol expects?

@BlinkyStitt BlinkyStitt changed the title EIP712 docs missing chainId EIP712 usage docs missing chainId Nov 6, 2018
@BlinkyStitt BlinkyStitt changed the title EIP712 usage docs missing chainId EIP712Domain missing chainId Nov 6, 2018
@fabioberger
Copy link
Contributor

@wysenynja thanks for reaching out. The chainId is optional, and not security critical. Both signatures generated using eth_sign and eth_signTypedData already include the chainId in the signature (see: ethereum/EIPs#155).

@BlinkyStitt
Copy link
Contributor Author

BlinkyStitt commented Nov 10, 2018 via email

@fabioberger
Copy link
Contributor

@wysenynja no, it means we do not include the chainId in the 0x EIP712 domain, but that is OK, because it is optional and not security critical.

@BlinkyStitt
Copy link
Contributor Author

Ok. Thanks.

And what about verifyingContract? What address should that be?

And are there any example orders with valid hashes for testing my implementation?

@fabioberger
Copy link
Contributor

Verifying contract should the 0x Exchange contract address. See: https://0xproject.com/wiki#Deployed-Addresses

I would just generate an order and hash it using @0x/order-utils. What exactly are you implementing?

@BlinkyStitt
Copy link
Contributor Author

Thanks. I'm building an orderbook aggregator in Rust as a way to learn Rust. The Rust web3 library doesn't have any helpers for EIP712 though (tomusdrw/rust-web3#169)

@fabioberger
Copy link
Contributor

😢 That's unfortunate. If you do end up implementing it, you should publish it for others to use! Good luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants