[3.0] Add chain ID to EIP 712 domains

contracts/coordinator/CHANGELOG.json

@@ -9,6 +9,18 @@
             {
                 "note": "Make `assertValidTransactionOrdersApproval` internal",
                 "pr": 1729
+            },
+            {
+                "note": "Add chainId to domain separator",
+                "pr": 1742
+            },
+            {
+                "note": "Inherit Exchange domain constants from `exchange-libs` to reduce code duplication",
+                "pr": 1742
+            },
+            {
+                "note": "Update domain separator",
+                "pr": 1742
             }
         ]
     },

contracts/coordinator/contracts/src/Coordinator.sol

@@ -32,8 +32,11 @@ contract Coordinator is
     MixinCoordinatorApprovalVerifier,
     MixinCoordinatorCore
 {
-    constructor (address _exchange)
+    /// @param exchange Address of the 0x Exchange contract.
+    /// @param chainId Chain ID of the network this contract is deployed on.
+    constructor (address exchange, uint256 chainId)
         public
-        LibConstants(_exchange)
+        LibConstants(exchange)
+        LibEIP712Domain(chainId)
     {}
 }

contracts/coordinator/contracts/src/libs/LibConstants.sol

@@ -21,14 +21,16 @@ pragma solidity ^0.5.5;
 import "../interfaces/ITransactions.sol";
 
 
+// solhint-disable var-name-mixedcase
 contract LibConstants {
 
-     // solhint-disable-next-line var-name-mixedcase
+    // The 0x Exchange contract.
     ITransactions internal EXCHANGE;
 
-    constructor (address _exchange)
+    /// @param exchange Address of the 0x Exchange contract.
+    constructor (address exchange)
         public
     {
-        EXCHANGE = ITransactions(_exchange);
+        EXCHANGE = ITransactions(exchange);
     }
 }

contracts/coordinator/contracts/src/libs/LibEIP712Domain.sol

@@ -18,61 +18,46 @@
 
 pragma solidity ^0.5.5;
 
+import "@0x/contracts-exchange-libs/contracts/src/LibEIP712.sol";
+import "@0x/contracts-exchange-libs/contracts/src/LibEIP712ExchangeDomainConstants.sol";
 import "./LibConstants.sol";
 
 
+// solhint-disable var-name-mixedcase
 contract LibEIP712Domain is
-    LibConstants
+    LibConstants,
+    LibEIP712,
+    LibEIP712ExchangeDomainConstants
 {
 
-    // EIP191 header for EIP712 prefix
-    string constant internal EIP191_HEADER = "\x19\x01";
-
     // EIP712 Domain Name value for the Coordinator
     string constant internal EIP712_COORDINATOR_DOMAIN_NAME = "0x Protocol Coordinator";
 
     // EIP712 Domain Version value for the Coordinator
-    string constant internal EIP712_COORDINATOR_DOMAIN_VERSION = "1.0.0";
-
-    // EIP712 Domain Name value for the Exchange
-    string constant internal EIP712_EXCHANGE_DOMAIN_NAME = "0x Protocol";
-
-    // EIP712 Domain Version value for the Exchange
-    string constant internal EIP712_EXCHANGE_DOMAIN_VERSION = "2";
-
-    // 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",
-        ")"
-    ));
+    string constant internal EIP712_COORDINATOR_DOMAIN_VERSION = "2.0.0";
 
     // Hash of the EIP712 Domain Separator data for the Coordinator
-    // solhint-disable-next-line var-name-mixedcase
     bytes32 public EIP712_COORDINATOR_DOMAIN_HASH;
 
     // Hash of the EIP712 Domain Separator data for the Exchange
-    // solhint-disable-next-line var-name-mixedcase
     bytes32 public EIP712_EXCHANGE_DOMAIN_HASH;
 
-    constructor ()
+    /// @param chainId Chain ID of the network this contract is deployed on.
+    constructor (uint256 chainId)
         public
     {
-        EIP712_COORDINATOR_DOMAIN_HASH = keccak256(abi.encodePacked(
-            EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH,
-            keccak256(bytes(EIP712_COORDINATOR_DOMAIN_NAME)),
-            keccak256(bytes(EIP712_COORDINATOR_DOMAIN_VERSION)),
-            uint256(address(this))
-        ));
-
-        EIP712_EXCHANGE_DOMAIN_HASH = keccak256(abi.encodePacked(
-            EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH,
-            keccak256(bytes(EIP712_EXCHANGE_DOMAIN_NAME)),
-            keccak256(bytes(EIP712_EXCHANGE_DOMAIN_VERSION)),
-            uint256(address(EXCHANGE))
-        ));
+        EIP712_COORDINATOR_DOMAIN_HASH = hashEIP712Domain(
+            EIP712_COORDINATOR_DOMAIN_NAME,
+            EIP712_COORDINATOR_DOMAIN_VERSION,
+            chainId,
+            address(this)
+        );
+        EIP712_EXCHANGE_DOMAIN_HASH = hashEIP712Domain(
+            EIP712_EXCHANGE_DOMAIN_NAME,
+            EIP712_EXCHANGE_DOMAIN_VERSION,
+            chainId,
+            address(EXCHANGE)
+        );
     }
 
     /// @dev Calculates EIP712 encoding for a hash struct in the EIP712 domain
@@ -98,34 +83,4 @@ contract LibEIP712Domain is
     {
         return hashEIP712Message(EIP712_EXCHANGE_DOMAIN_HASH, hashStruct);
     }
-
-    /// @dev Calculates EIP712 encoding for a hash struct with a given domain hash.
-    /// @param eip712DomainHash Hash of the domain domain separator data.
-    /// @param hashStruct The EIP712 hash struct.
-    /// @return EIP712 hash applied to the Exchange EIP712 Domain.
-    function hashEIP712Message(bytes32 eip712DomainHash, bytes32 hashStruct)
-        internal
-        pure
-        returns (bytes32 result)
-    {
-        // Assembly for more efficient computing:
-        // keccak256(abi.encodePacked(
-        //     EIP191_HEADER,
-        //     EIP712_DOMAIN_HASH,
-        //     hashStruct
-        // ));
-
-        assembly {
-            // Load free memory pointer
-            let memPtr := mload(64)
-
-            mstore(memPtr, 0x1901000000000000000000000000000000000000000000000000000000000000)  // EIP191 header
-            mstore(add(memPtr, 2), eip712DomainHash)                                            // EIP712 domain hash
-            mstore(add(memPtr, 34), hashStruct)                                                 // Hash of struct
-
-            // Compute hash
-            result := keccak256(memPtr, 66)
-        }
-        return result;
-    }
 }

contracts/coordinator/package.json

@@ -70,7 +70,7 @@
         "@0x/base-contract": "^5.0.4",
         "@0x/contracts-asset-proxy": "^2.1.0",
         "@0x/contracts-erc20": "^2.1.0",
-        "@0x/contracts-exchange": "1.0.2",
+        "@0x/contracts-exchange": "^2.1.0",
         "@0x/contracts-exchange-libs": "^2.1.0",
         "@0x/contracts-utils": "^3.1.0",
         "@0x/order-utils": "^7.1.1",

contracts/coordinator/test/coordinator.ts

@@ -21,7 +21,7 @@ import {
 import { BlockchainLifecycle } from '@0x/dev-utils';
 import { assetDataUtils, orderHashUtils } from '@0x/order-utils';
 import { RevertReason, SignedOrder } from '@0x/types';
-import { BigNumber } from '@0x/utils';
+import { BigNumber, providerUtils } from '@0x/utils';
 import * as chai from 'chai';
 import { LogWithDecodedArgs } from 'ethereum-types';
 
@@ -33,6 +33,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
 web3Wrapper.abiDecoder.addABI(exchangeArtifacts.Exchange.compilerOutput.abi);
 // tslint:disable:no-unnecessary-type-assertion
 describe('Coordinator tests', () => {
+    let chainId: number;
     let makerAddress: string;
     let owner: string;
     let takerAddress: string;
@@ -58,6 +59,7 @@ describe('Coordinator tests', () => {
         await blockchainLifecycle.revertAsync();
     });
     before(async () => {
+        chainId = await providerUtils.getChainIdAsync(provider);
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
         const usedAddresses = ([owner, makerAddress, takerAddress, feeRecipientAddress] = accounts.slice(0, 4));
 
@@ -75,6 +77,7 @@ describe('Coordinator tests', () => {
             provider,
             txDefaults,
             assetDataUtils.encodeERC20AssetData(zrxToken.address),
+            new BigNumber(chainId),
         );
 
         await web3Wrapper.awaitTransactionSuccessAsync(
@@ -92,24 +95,28 @@ describe('Coordinator tests', () => {
             provider,
             txDefaults,
             exchange.address,
+            new BigNumber(chainId),
         );
 
         // Configure order defaults
         const defaultOrderParams = {
             ...devConstants.STATIC_ORDER_PARAMS,
-            exchangeAddress: exchange.address,
             senderAddress: coordinatorContract.address,
             makerAddress,
             feeRecipientAddress,
             makerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenA.address),
             takerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenB.address),
+            domain: {
+                verifyingContractAddress: exchange.address,
+                chainId,
+            },
         };
         const makerPrivateKey = devConstants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddress)];
         const takerPrivateKey = devConstants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(takerAddress)];
         const feeRecipientPrivateKey = devConstants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(feeRecipientAddress)];
         orderFactory = new OrderFactory(makerPrivateKey, defaultOrderParams);
-        makerTransactionFactory = new TransactionFactory(makerPrivateKey, exchange.address);
-        takerTransactionFactory = new TransactionFactory(takerPrivateKey, exchange.address);
+        makerTransactionFactory = new TransactionFactory(makerPrivateKey, exchange.address, chainId);
+        takerTransactionFactory = new TransactionFactory(takerPrivateKey, exchange.address, chainId);
         approvalFactory = new ApprovalFactory(feeRecipientPrivateKey, coordinatorContract.address);
     });
     beforeEach(async () => {

contracts/coordinator/test/libs.ts

@@ -1,7 +1,7 @@
 import { addressUtils, chaiSetup, constants, provider, txDefaults, web3Wrapper } from '@0x/contracts-test-utils';
 import { BlockchainLifecycle } from '@0x/dev-utils';
 import { transactionHashUtils } from '@0x/order-utils';
-import { BigNumber } from '@0x/utils';
+import { BigNumber, providerUtils } from '@0x/utils';
 import * as chai from 'chai';
 
 import { artifacts, CoordinatorContract, hashUtils } from '../src';
@@ -12,6 +12,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
 
 describe('Libs tests', () => {
     let coordinatorContract: CoordinatorContract;
+    let chainId: number;
     const exchangeAddress = addressUtils.generatePseudoRandomAddress();
 
     before(async () => {
@@ -21,11 +22,13 @@ describe('Libs tests', () => {
         await blockchainLifecycle.revertAsync();
     });
     before(async () => {
+        chainId = await providerUtils.getChainIdAsync(provider);
         coordinatorContract = await CoordinatorContract.deployFrom0xArtifactAsync(
             artifacts.Coordinator,
             provider,
             txDefaults,
             exchangeAddress,
+            new BigNumber(chainId),
         );
     });
     beforeEach(async () => {
@@ -38,10 +41,13 @@ describe('Libs tests', () => {
     describe('getTransactionHash', () => {
         it('should return the correct transaction hash', async () => {
             const tx = {
-                verifyingContractAddress: exchangeAddress,
                 salt: new BigNumber(0),
                 signerAddress: constants.NULL_ADDRESS,
                 data: '0x1234',
+                domain: {
+                    verifyingContractAddress: exchangeAddress,
+                    chainId,
+                },
             };
             const expectedTxHash = transactionHashUtils.getTransactionHashHex(tx);
             const txHash = await coordinatorContract.getTransactionHash.callAsync(tx);
@@ -52,11 +58,14 @@ describe('Libs tests', () => {
     describe('getApprovalHash', () => {
         it('should return the correct approval hash', async () => {
             const signedTx = {
-                verifyingContractAddress: exchangeAddress,
                 salt: new BigNumber(0),
                 signerAddress: constants.NULL_ADDRESS,
                 data: '0x1234',
                 signature: '0x5678',
+                domain: {
+                    verifyingContractAddress: exchangeAddress,
+                    chainId,
+                },
             };
             const approvalExpirationTimeSeconds = new BigNumber(0);
             const txOrigin = constants.NULL_ADDRESS;