Rebase and update feedback

Cache the domain separator data with address this
Use the EIP712Types enum for types everywhere
Rename EIP712 struct ExecuteTransaction to ZeroExTransaction

8 files changed, 114 insertions, 114 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index e3f6b2b2b..9bdaa0f8f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -37,28 +37,31 @@ contract MixinTransactions is
     // Address of current transaction signer
     address public currentContextAddress;
 
-    bytes32 constant EXECUTE_TRANSACTION_SCHEMA_HASH = keccak256(
-        "ExecuteTransaction(",
+    // Hash for the EIP712 ZeroEx Transaction Schema
+    bytes32 constant EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked(
+        "ZeroExTransaction(",
         "uint256 salt,",
         "address signer,",
         "bytes data",
         ")"
-    );
+    ));
 
-    function getExecuteTransactionHash(uint256 salt, address signer, bytes data)
+    /// @dev Calculates EIP712 hash of the Transaction.
+    /// @param salt Arbitrary number to ensure uniqueness of transaction hash.
+    /// @param signer Address of transaction signer.
+    /// @param data AbiV2 encoded calldata.
+    /// @return EIP712 hash of the Transaction.
+    function hashZeroExTransaction(uint256 salt, address signer, bytes data)
         internal
         view
-        returns (bytes32 executeTransactionHash)
+        returns (bytes32)
     {
-        executeTransactionHash = createEIP712Message(
-            keccak256(
-                EXECUTE_TRANSACTION_SCHEMA_HASH,
-                salt,
-                bytes32(signer),
-                keccak256(data)
-            )
-        );
-        return executeTransactionHash;
+        return keccak256(abi.encodePacked(
+            EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH,
+            salt,
+            bytes32(signer),
+            keccak256(abi.encodePacked(data))
+        ));
     }
 
     /// @dev Executes an exchange method call in the context of signer.
@@ -80,7 +83,7 @@ contract MixinTransactions is
             REENTRANCY_ILLEGAL
         );
 
-        bytes32 transactionHash = getExecuteTransactionHash(salt, signer, data);
+        bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(salt, signer, data));
 
         // Validate transaction has not been executed
         require(
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibEIP712.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibEIP712.sol
index 7704e3db4..991137f32 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibEIP712.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibEIP712.sol
@@ -19,44 +19,37 @@
 pragma solidity ^0.4.24;
 
 contract LibEIP712 {
-    string public constant EIP191_HEADER = "\x19\x01";
-
-    bytes32 public constant EIP712_DOMAIN_SEPARATOR_NAME_HASH = keccak256("0x Protocol");
-
-    bytes32 public constant EIP712_DOMAIN_SEPARATOR_VERSION_HASH = keccak256("2");
-
-    bytes32 public constant EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(
-        abi.encodePacked(
-            "DomainSeparator(",
-            "string name,",
-            "string version,",
-            "address contract",
-            ")"
-        )
-    );
+    // EIP191 header for EIP712 prefix
+    string constant EIP191_HEADER = "\x19\x01";
+
+    // Hash of the EIP712 Domain Separator Schema
+    bytes32 public constant EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked(
+        "EIP712Domain(",
+        "string name,",
+        "string version,",
+        "address verifyingContract",
+        ")"
+    ));
+
+    // Hash of the EIP712 Domain Separator data
+    bytes32 public EIP712_DOMAIN_HASH;
+
+    constructor ()
+    public
+    {
+        EIP712_DOMAIN_HASH = keccak256(abi.encodePacked(
+            EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH,
+            keccak256(abi.encodePacked("0x Protocol")),
+            keccak256(abi.encodePacked("2")),
+            bytes32(address(this))
+        ));
+    }
 
-    function createEIP712Message(bytes32 hashStruct)
+    function hashEIP712Message(bytes32 hashStruct)
         internal
         view
-        returns (bytes32 message)
+        returns (bytes32)
     {
-        // TODO: EIP712 is not finalized yet
-        // Source: https://github.com/ethereum/EIPs/pull/712
-        // TODO: Cache the Domain Separator
-        message = keccak256(
-            abi.encodePacked(
-                EIP191_HEADER,
-                keccak256(
-                    abi.encodePacked(
-                        EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH,
-                        EIP712_DOMAIN_SEPARATOR_NAME_HASH,
-                        EIP712_DOMAIN_SEPARATOR_VERSION_HASH,
-                        bytes32(address(this))
-                    )
-                ),
-                hashStruct
-            )
-        );
-        return message;
+        return keccak256(abi.encodePacked(EIP191_HEADER, EIP712_DOMAIN_HASH, hashStruct));
     }
-}
\ No newline at end of file
+}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol
index ed532fd59..04374f1d0 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol
@@ -24,8 +24,8 @@ contract LibOrder is
     LibEIP712
 {
 
-    bytes32 constant EIP712_ORDER_SCHEMA_HASH = keccak256(
-        abi.encodePacked(
+    // Hash for the EIP712 Order Schema
+    bytes32 constant EIP712_ORDER_SCHEMA_HASH = keccak256(abi.encodePacked(
             "Order(",
             "address makerAddress,",
             "address takerAddress,",
@@ -40,7 +40,8 @@ contract LibOrder is
             "bytes makerAssetData,",
             "bytes takerAssetData",
             ")"
-    ));
+        )
+    );
 
     // A valid order remains fillable until it is expired, fully filled, or cancelled.
     // An order's state is unaffected by external factors, like account balances.
@@ -86,25 +87,32 @@ contract LibOrder is
         view
         returns (bytes32 orderHash)
     {
-        orderHash = createEIP712Message(
-            keccak256(
-                abi.encodePacked(
-                    EIP712_ORDER_SCHEMA_HASH,
-                    bytes32(order.makerAddress),
-                    bytes32(order.takerAddress),
-                    bytes32(order.feeRecipientAddress),
-                    bytes32(order.senderAddress),
-                    order.makerAssetAmount,
-                    order.takerAssetAmount,
-                    order.makerFee,
-                    order.takerFee,
-                    order.expirationTimeSeconds,
-                    order.salt,
-                    keccak256(abi.encodePacked(order.makerAssetData)),
-                    keccak256(abi.encodePacked(order.takerAssetData))
-                )
-            )
-        );
+        orderHash = hashEIP712Message(hashOrder(order));
         return orderHash;
     }
+
+    /// @dev Calculates EIP712 hash of the order.
+    /// @param order The order structure.
+    /// @return EIP712 hash of the order.
+    function hashOrder(Order memory order)
+        internal
+        pure
+        returns (bytes32)
+    {
+        return keccak256(abi.encodePacked(
+            EIP712_ORDER_SCHEMA_HASH,
+            bytes32(order.makerAddress),
+            bytes32(order.takerAddress),
+            bytes32(order.feeRecipientAddress),
+            bytes32(order.senderAddress),
+            order.makerAssetAmount,
+            order.takerAssetAmount,
+            order.makerFee,
+            order.takerFee,
+            order.expirationTimeSeconds,
+            order.salt,
+            keccak256(abi.encodePacked(order.makerAssetData)),
+            keccak256(abi.encodePacked(order.takerAssetData))
+        ));
+    }
 }
diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts
index bb15a0309..c0a32b83a 100644
--- a/packages/contracts/src/utils/transaction_factory.ts
+++ b/packages/contracts/src/utils/transaction_factory.ts
@@ -1,16 +1,16 @@
-import { crypto, EIP712Schema, EIP712Utils, generatePseudoRandomSalt } from '@0xproject/order-utils';
+import { crypto, EIP712Schema, EIP712Types, EIP712Utils, generatePseudoRandomSalt } from '@0xproject/order-utils';
 import { SignatureType } from '@0xproject/types';
 import * as ethUtil from 'ethereumjs-util';
 
 import { signingUtils } from './signing_utils';
 import { SignedTransaction } from './types';
 
-const EIP712_EXECUTE_TRANSACTION_SCHEMA: EIP712Schema = {
-    name: 'ExecuteTransaction',
+const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = {
+    name: 'ZeroExTransaction',
     parameters: [
-        { name: 'salt', type: 'uint256' },
-        { name: 'signer', type: 'address' },
-        { name: 'data', type: 'bytes' },
+        { name: 'salt', type: EIP712Types.Uint256 },
+        { name: 'signer', type: EIP712Types.Address },
+        { name: 'data', type: EIP712Types.Bytes },
     ],
 };
 
@@ -24,7 +24,7 @@ export class TransactionFactory {
         this._signerBuff = ethUtil.privateToAddress(this._privateKey);
     }
     public newSignedTransaction(data: string, signatureType: SignatureType = SignatureType.EthSign): SignedTransaction {
-        const executeTransactionSchemaHashBuff = EIP712Utils.compileSchema(EIP712_EXECUTE_TRANSACTION_SCHEMA);
+        const executeTransactionSchemaHashBuff = EIP712Utils.compileSchema(EIP712_ZEROEX_TRANSACTION_SCHEMA);
         const salt = generatePseudoRandomSalt();
         const signer = `0x${this._signerBuff.toString('hex')}`;
         const executeTransactionData = {
@@ -33,7 +33,7 @@ export class TransactionFactory {
             data,
         };
         const executeTransactionHashBuff = EIP712Utils.structHash(
-            EIP712_EXECUTE_TRANSACTION_SCHEMA,
+            EIP712_ZEROEX_TRANSACTION_SCHEMA,
             executeTransactionData,
         );
         const txHash = EIP712Utils.createEIP712Message(executeTransactionHashBuff, this._exchangeAddress);
diff --git a/packages/order-utils/src/eip712_utils.ts b/packages/order-utils/src/eip712_utils.ts
index e54ca0e83..00edd2531 100644
--- a/packages/order-utils/src/eip712_utils.ts
+++ b/packages/order-utils/src/eip712_utils.ts
@@ -4,7 +4,7 @@ import * as _ from 'lodash';
 
 import { assetProxyUtils } from './asset_proxy_utils';
 import { crypto } from './crypto';
-import { EIP712Schema } from './types';
+import { EIP712Schema, EIP712Types } from './types';
 
 const EIP191_PREFIX = '\x19\x01';
 const EIP712_DOMAIN_NAME = '0x Protocol';
@@ -12,22 +12,14 @@ const EIP712_DOMAIN_VERSION = '2';
 const EIP712_VALUE_LENGTH = 32;
 
 const EIP712_DOMAIN_SCHEMA: EIP712Schema = {
-    name: 'DomainSeparator',
+    name: 'EIP712Domain',
     parameters: [
-        { name: 'name', type: 'string' },
-        { name: 'version', type: 'string' },
-        { name: 'contract', type: 'address' },
+        { name: 'name', type: EIP712Types.String },
+        { name: 'version', type: EIP712Types.String },
+        { name: 'verifyingContract', type: EIP712Types.Address },
     ],
 };
 
-enum EIP712Types {
-    String = 'string',
-    Bytes = 'bytes',
-    Address = 'address',
-    Bytes32 = 'bytes32',
-    Uint256 = 'uint256',
-}
-
 export const EIP712Utils = {
     /**
      * Compiles the EIP712Schema and returns the hash of the schema.
@@ -67,7 +59,7 @@ export const EIP712Utils = {
         const encodedData = EIP712Utils._encodeData(EIP712_DOMAIN_SCHEMA, {
             name: EIP712_DOMAIN_NAME,
             version: EIP712_DOMAIN_VERSION,
-            contract: exchangeAddress,
+            verifyingContract: exchangeAddress,
         });
         const domainSeparatorHashBuff2 = crypto.solSHA3([domainSeparatorSchemaBuffer, ...encodedData]);
         return domainSeparatorHashBuff2;
@@ -79,18 +71,14 @@ export const EIP712Utils = {
         return encodedType;
     },
     _encodeData(schema: EIP712Schema, data: { [key: string]: any }): any {
-        const encodedTypes = [];
         const encodedValues = [];
         for (const parameter of schema.parameters) {
             const value = data[parameter.name];
             if (parameter.type === EIP712Types.String || parameter.type === EIP712Types.Bytes) {
-                encodedTypes.push(EIP712Types.Bytes32);
                 encodedValues.push(crypto.solSHA3([ethUtil.toBuffer(value)]));
             } else if (parameter.type === EIP712Types.Uint256) {
-                encodedTypes.push(EIP712Types.Uint256);
                 encodedValues.push(value);
             } else if (parameter.type === EIP712Types.Address) {
-                encodedTypes.push(EIP712Types.Address);
                 encodedValues.push(EIP712Utils.pad32Address(value));
             } else {
                 throw new Error(`Unable to encode ${parameter.type}`);
diff --git a/packages/order-utils/src/index.ts b/packages/order-utils/src/index.ts
index caa996621..5122e60fc 100644
--- a/packages/order-utils/src/index.ts
+++ b/packages/order-utils/src/index.ts
@@ -4,7 +4,7 @@ export { orderFactory } from './order_factory';
 export { constants } from './constants';
 export { crypto } from './crypto';
 export { generatePseudoRandomSalt } from './salt';
-export { OrderError, MessagePrefixType, MessagePrefixOpts, EIP712Parameter, EIP712Schema } from './types';
+export { OrderError, MessagePrefixType, MessagePrefixOpts, EIP712Parameter, EIP712Schema, EIP712Types } from './types';
 export { AbstractBalanceAndProxyAllowanceFetcher } from './abstract/abstract_balance_and_proxy_allowance_fetcher';
 export { AbstractOrderFilledCancelledFetcher } from './abstract/abstract_order_filled_cancelled_fetcher';
 export { RemainingFillableCalculator } from './remaining_fillable_calculator';
diff --git a/packages/order-utils/src/order_hash.ts b/packages/order-utils/src/order_hash.ts
index 46cce4b3b..61f6c978f 100644
--- a/packages/order-utils/src/order_hash.ts
+++ b/packages/order-utils/src/order_hash.ts
@@ -10,25 +10,25 @@ import * as _ from 'lodash';
 import { assert } from './assert';
 import { crypto } from './crypto';
 import { EIP712Utils } from './eip712_utils';
-import { EIP712Schema } from './types';
+import { EIP712Schema, EIP712Types } from './types';
 
 const INVALID_TAKER_FORMAT = 'instance.takerAddress is not of a type(s) string';
 
 const EIP712_ORDER_SCHEMA: EIP712Schema = {
     name: 'Order',
     parameters: [
-        { name: 'makerAddress', type: 'address' },
-        { name: 'takerAddress', type: 'address' },
-        { name: 'feeRecipientAddress', type: 'address' },
-        { name: 'senderAddress', type: 'address' },
-        { name: 'makerAssetAmount', type: 'uint256' },
-        { name: 'takerAssetAmount', type: 'uint256' },
-        { name: 'makerFee', type: 'uint256' },
-        { name: 'takerFee', type: 'uint256' },
-        { name: 'expirationTimeSeconds', type: 'uint256' },
-        { name: 'salt', type: 'uint256' },
-        { name: 'makerAssetData', type: 'bytes' },
-        { name: 'takerAssetData', type: 'bytes' },
+        { name: 'makerAddress', type: EIP712Types.Address },
+        { name: 'takerAddress', type: EIP712Types.Address },
+        { name: 'feeRecipientAddress', type: EIP712Types.Address },
+        { name: 'senderAddress', type: EIP712Types.Address },
+        { name: 'makerAssetAmount', type: EIP712Types.Uint256 },
+        { name: 'takerAssetAmount', type: EIP712Types.Uint256 },
+        { name: 'makerFee', type: EIP712Types.Uint256 },
+        { name: 'takerFee', type: EIP712Types.Uint256 },
+        { name: 'expirationTimeSeconds', type: EIP712Types.Uint256 },
+        { name: 'salt', type: EIP712Types.Uint256 },
+        { name: 'makerAssetData', type: EIP712Types.Bytes },
+        { name: 'takerAssetData', type: EIP712Types.Bytes },
     ],
 };
 
diff --git a/packages/order-utils/src/types.ts b/packages/order-utils/src/types.ts
index 858c0e18b..daa4aee11 100644
--- a/packages/order-utils/src/types.ts
+++ b/packages/order-utils/src/types.ts
@@ -26,10 +26,18 @@ export interface MessagePrefixOpts {
 
 export interface EIP712Parameter {
     name: string;
-    type: string;
+    type: EIP712Types;
 }
 
 export interface EIP712Schema {
     name: string;
     parameters: EIP712Parameter[];
 }
+
+export enum EIP712Types {
+    String = 'string',
+    Bytes = 'bytes',
+    Address = 'address',
+    Bytes32 = 'bytes32',
+    Uint256 = 'uint256',
+}