Fix comments, styling, and optimize hashOrder

3 files changed, 33 insertions, 25 deletions

diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
index 88d2da7d7..0814638db 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
@@ -123,10 +123,10 @@ contract MixinTransactions is
         bytes32 dataHash = keccak256(data);
 
         // Assembly for more efficiently computing:
-        // keccak256(abi.encode(
+        // keccak256(abi.encodePacked(
         //     EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH,
         //     salt,
-        //     signerAddress,
+        //     bytes32(signerAddress),
         //     keccak256(data)
         // ));
 
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol
index 1fc41dafd..c9e45189d 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol
@@ -30,7 +30,7 @@ contract LibEIP712 {
     string constant internal EIP712_DOMAIN_VERSION = "2";
 
     // Hash of the EIP712 Domain Separator Schema
-    bytes32 public constant EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked(
+    bytes32 constant internal EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked(
         "EIP712Domain(",
         "string name,",
         "string version,",
@@ -45,11 +45,11 @@ contract LibEIP712 {
     constructor ()
         public
     {
-        EIP712_DOMAIN_HASH = keccak256(abi.encode(
+        EIP712_DOMAIN_HASH = keccak256(abi.encodePacked(
             EIP712_DOMAIN_SEPARATOR_SCHEMA_HASH,
             keccak256(bytes(EIP712_DOMAIN_NAME)),
             keccak256(bytes(EIP712_DOMAIN_VERSION)),
-            address(this)
+            bytes32(address(this))
         ));
     }
 
@@ -59,8 +59,13 @@ contract LibEIP712 {
     function hashEIP712Message(bytes32 hashStruct)
         internal
         view
-        returns (bytes32)
+        returns (bytes32 result)
     {
-        return keccak256(abi.encodePacked(EIP191_HEADER, EIP712_DOMAIN_HASH, hashStruct));
+        result = keccak256(abi.encodePacked(
+            EIP191_HEADER,
+            EIP712_DOMAIN_HASH,
+            hashStruct
+        ));
+        return result;
     }
 }
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol
index 4031ff26b..68f4f5f1b 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol
@@ -103,11 +103,12 @@ contract LibOrder is
         bytes32 takerAssetDataHash = keccak256(order.takerAssetData);
 
         // Assembly for more efficiently computing:
-        // keccak256(abi.encode(
-        //     order.makerAddress,
-        //     order.takerAddress,
-        //     order.feeRecipientAddress,
-        //     order.senderAddress,
+        // 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,
@@ -119,24 +120,26 @@ contract LibOrder is
         // ));
 
         assembly {
+            // Calculate memory addresses that will be swapped out before hashing
+            let pos1 := sub(order, 32)
+            let pos2 := add(order, 320)
+            let pos3 := add(order, 352)
+
             // Backup
-            // solhint-disable-next-line space-after-comma
-            let temp1 := mload(sub(order,  32))
-            let temp2 := mload(add(order, 320))
-            let temp3 := mload(add(order, 352))
+            let temp1 := mload(pos1)
+            let temp2 := mload(pos2)
+            let temp3 := mload(pos3)
             
             // Hash in place
-            // solhint-disable-next-line space-after-comma
-            mstore(sub(order,  32), schemaHash)
-            mstore(add(order, 320), makerAssetDataHash)
-            mstore(add(order, 352), takerAssetDataHash)
-            result := keccak256(sub(order, 32), 416)
+            mstore(pos1, schemaHash)
+            mstore(pos2, makerAssetDataHash)
+            mstore(pos3, takerAssetDataHash)
+            result := keccak256(pos1, 416)
             
             // Restore
-            // solhint-disable-next-line space-after-comma
-            mstore(sub(order,  32), temp1)
-            mstore(add(order, 320), temp2)
-            mstore(add(order, 352), temp3)
+            mstore(pos1, temp1)
+            mstore(pos2, temp2)
+            mstore(pos3, temp3)
         }
         return result;
     }