Merge pull request #1729 from 0xProject/feat/coordinator/functionVisi…

…bility

Modify function visibilities in Coordinator contract

contracts/coordinator/CHANGELOG.json

@@ -1,4 +1,17 @@
 [
+    {
+        "version": "2.0.0",
+        "changes": [
+            {
+                "note": "Make `decodeOrdersFromFillData`, `getCoordinatorApprovalHash`, and `getTransactionHash` public",
+                "pr": 1729
+            },
+            {
+                "note": "Make `assertValidTransactionOrdersApproval` internal",
+                "pr": 1729
+            }
+        ]
+    },
     {
         "version": "1.1.0",
         "changes": [

contracts/coordinator/compiler.json

@@ -21,10 +21,5 @@
             }
         }
     },
-    "contracts": [
-        "src/Coordinator.sol",
-        "src/registry/CoordinatorRegistry.sol",
-        "test/TestLibs.sol",
-        "test/TestMixins.sol"
-    ]
+    "contracts": ["src/Coordinator.sol", "src/registry/CoordinatorRegistry.sol"]
 }

contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol

@@ -58,7 +58,7 @@ contract MixinCoordinatorApprovalVerifier is
         view
     {
         // Get the orders from the the Exchange calldata in the 0x transaction
-        LibOrder.Order[] memory orders = decodeFillDataOrders(transaction.data);
+        LibOrder.Order[] memory orders = decodeOrdersFromFillData(transaction.data);
 
         // No approval is required for non-fill methods
         if (orders.length > 0) {
@@ -74,6 +74,57 @@ contract MixinCoordinatorApprovalVerifier is
         }
     }
 
+    /// @dev Decodes the orders from Exchange calldata representing any fill method.
+    /// @param data Exchange calldata representing a fill method.
+    /// @return The orders from the Exchange calldata.
+    function decodeOrdersFromFillData(bytes memory data)
+        public
+        pure
+        returns (LibOrder.Order[] memory orders)
+    {
+        bytes4 selector = data.readBytes4(0);
+        if (
+            selector == FILL_ORDER_SELECTOR ||
+            selector == FILL_ORDER_NO_THROW_SELECTOR ||
+            selector == FILL_OR_KILL_ORDER_SELECTOR
+        ) {
+            // Decode single order
+            (LibOrder.Order memory order) = abi.decode(
+                data.slice(4, data.length),
+                (LibOrder.Order)
+            );
+            orders = new LibOrder.Order[](1);
+            orders[0] = order;
+        } else if (
+            selector == BATCH_FILL_ORDERS_SELECTOR ||
+            selector == BATCH_FILL_ORDERS_NO_THROW_SELECTOR ||
+            selector == BATCH_FILL_OR_KILL_ORDERS_SELECTOR ||
+            selector == MARKET_BUY_ORDERS_SELECTOR ||
+            selector == MARKET_BUY_ORDERS_NO_THROW_SELECTOR ||
+            selector == MARKET_SELL_ORDERS_SELECTOR ||
+            selector == MARKET_SELL_ORDERS_NO_THROW_SELECTOR
+        ) {
+            // Decode all orders
+            // solhint-disable indent
+            (orders) = abi.decode(
+                data.slice(4, data.length),
+                (LibOrder.Order[])
+            );
+        } else if (selector == MATCH_ORDERS_SELECTOR) {
+            // Decode left and right orders
+            (LibOrder.Order memory leftOrder, LibOrder.Order memory rightOrder) = abi.decode(
+                data.slice(4, data.length),
+                (LibOrder.Order, LibOrder.Order)
+            );
+
+            // Create array of orders
+            orders = new LibOrder.Order[](2);
+            orders[0] = leftOrder;
+            orders[1] = rightOrder;
+        }
+        return orders;
+    }
+
     /// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction.
     /// @param transaction 0x transaction containing salt, signerAddress, and data.
     /// @param orders Array of order structs containing order specifications.
@@ -89,7 +140,7 @@ contract MixinCoordinatorApprovalVerifier is
         uint256[] memory approvalExpirationTimeSeconds,
         bytes[] memory approvalSignatures
     )
-        public
+        internal
         view
     {
         // Verify that Ethereum tx signer is the same as the approved txOrigin
@@ -149,55 +200,4 @@ contract MixinCoordinatorApprovalVerifier is
             );
         }
     }
-
-    /// @dev Decodes the orders from Exchange calldata representing any fill method.
-    /// @param data Exchange calldata representing a fill method.
-    /// @return The orders from the Exchange calldata.
-    function decodeFillDataOrders(bytes memory data)
-        internal
-        pure
-        returns (LibOrder.Order[] memory orders)
-    {
-        bytes4 selector = data.readBytes4(0);
-        if (
-            selector == FILL_ORDER_SELECTOR ||
-            selector == FILL_ORDER_NO_THROW_SELECTOR ||
-            selector == FILL_OR_KILL_ORDER_SELECTOR
-        ) {
-            // Decode single order
-            (LibOrder.Order memory order) = abi.decode(
-                data.slice(4, data.length),
-                (LibOrder.Order)
-            );
-            orders = new LibOrder.Order[](1);
-            orders[0] = order;
-        } else if (
-            selector == BATCH_FILL_ORDERS_SELECTOR ||
-            selector == BATCH_FILL_ORDERS_NO_THROW_SELECTOR ||
-            selector == BATCH_FILL_OR_KILL_ORDERS_SELECTOR ||
-            selector == MARKET_BUY_ORDERS_SELECTOR ||
-            selector == MARKET_BUY_ORDERS_NO_THROW_SELECTOR ||
-            selector == MARKET_SELL_ORDERS_SELECTOR ||
-            selector == MARKET_SELL_ORDERS_NO_THROW_SELECTOR
-        ) {
-            // Decode all orders
-            // solhint-disable indent
-            (orders) = abi.decode(
-                data.slice(4, data.length),
-                (LibOrder.Order[])
-            );
-        } else if (selector == MATCH_ORDERS_SELECTOR) {
-            // Decode left and right orders
-            (LibOrder.Order memory leftOrder, LibOrder.Order memory rightOrder) = abi.decode(
-                data.slice(4, data.length),
-                (LibOrder.Order, LibOrder.Order)
-            );
-
-            // Create array of orders
-            orders = new LibOrder.Order[](2);
-            orders[0] = leftOrder;
-            orders[1] = rightOrder;
-        }
-        return orders;
-    }
 }

contracts/coordinator/contracts/src/interfaces/ICoordinatorApprovalVerifier.sol

@@ -42,21 +42,11 @@ contract ICoordinatorApprovalVerifier {
         public
         view;
 
-    /// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction.
-    /// @param transaction 0x transaction containing salt, signerAddress, and data.
-    /// @param orders Array of order structs containing order specifications.
-    /// @param txOrigin Required signer of Ethereum transaction calling this function.
-    /// @param transactionSignature Proof that the transaction has been signed by the signer.
-    /// @param approvalExpirationTimeSeconds Array of expiration times in seconds for which each corresponding approval signature expires.
-    /// @param approvalSignatures Array of signatures that correspond to the feeRecipients of each order.
-    function assertValidTransactionOrdersApproval(
-        LibZeroExTransaction.ZeroExTransaction memory transaction,
-        LibOrder.Order[] memory orders,
-        address txOrigin,
-        bytes memory transactionSignature,
-        uint256[] memory approvalExpirationTimeSeconds,
-        bytes[] memory approvalSignatures
-    )
+    /// @dev Decodes the orders from Exchange calldata representing any fill method.
+    /// @param data Exchange calldata representing a fill method.
+    /// @return The orders from the Exchange calldata.
+    function decodeOrdersFromFillData(bytes memory data)
         public
-        view;
+        pure
+        returns (LibOrder.Order[] memory orders);
 }

contracts/coordinator/contracts/src/libs/LibCoordinatorApproval.sol

@@ -17,6 +17,7 @@
 */
 
 pragma solidity ^0.5.5;
+pragma experimental "ABIEncoderV2";
 
 import "./LibEIP712Domain.sol";
 
@@ -37,16 +38,16 @@ contract LibCoordinatorApproval is
 
     struct CoordinatorApproval {
         address txOrigin;                       // Required signer of Ethereum transaction that is submitting approval.
-        bytes32 transactionHash;                // EIP712 hash of the transaction, using the domain separator of this contract.
+        bytes32 transactionHash;                // EIP712 hash of the transaction.
         bytes transactionSignature;             // Signature of the 0x transaction.
-        uint256 approvalExpirationTimeSeconds;  // Timestamp in seconds for which the signature expires.
+        uint256 approvalExpirationTimeSeconds;  // Timestamp in seconds for which the approval expires.
     }
 
     /// @dev Calculated the EIP712 hash of the Coordinator approval mesasage using the domain separator of this contract.
     /// @param approval Coordinator approval message containing the transaction hash, transaction signature, and expiration of the approval.
     /// @return EIP712 hash of the Coordinator approval message with the domain separator of this contract.
     function getCoordinatorApprovalHash(CoordinatorApproval memory approval)
-        internal
+        public
         view
         returns (bytes32 approvalHash)
     {
@@ -71,9 +72,10 @@ contract LibCoordinatorApproval is
         // Assembly for more efficiently computing:
         // keccak256(abi.encodePacked(
         //     EIP712_COORDINATOR_APPROVAL_SCHEMA_HASH,
+        //     approval.txOrigin,
         //     approval.transactionHash,
         //     keccak256(approval.transactionSignature)
-        //     approval.expiration,
+        //     approval.approvalExpirationTimeSeconds,
         // ));
 
         assembly {

contracts/coordinator/contracts/src/libs/LibZeroExTransaction.sol

@@ -17,6 +17,7 @@
 */
 
 pragma solidity ^0.5.5;
+pragma experimental "ABIEncoderV2";
 
 import "./LibEIP712Domain.sol";
 
@@ -40,11 +41,11 @@ contract LibZeroExTransaction is
         bytes data;             // AbiV2 encoded calldata.
     }
 
-    /// @dev Calculates the EIP712 hash of a 0x transaction using the domain separator of this contract.
+    /// @dev Calculates the EIP712 hash of a 0x transaction using the domain separator of the Exchange contract.
     /// @param transaction 0x transaction containing salt, signerAddress, and data.
     /// @return EIP712 hash of the transaction with the domain separator of this contract.
     function getTransactionHash(ZeroExTransaction memory transaction)
-        internal
+        public
         view
         returns (bytes32 transactionHash)
     {

contracts/coordinator/contracts/src/mixins/MCoordinatorApprovalVerifier.sol

@@ -26,11 +26,21 @@ import "../interfaces/ICoordinatorApprovalVerifier.sol";
 contract MCoordinatorApprovalVerifier is
     ICoordinatorApprovalVerifier
 {
-    /// @dev Decodes the orders from Exchange calldata representing any fill method.
-    /// @param data Exchange calldata representing a fill method.
-    /// @return The orders from the Exchange calldata.
-    function decodeFillDataOrders(bytes memory data)
+    /// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction.
+    /// @param transaction 0x transaction containing salt, signerAddress, and data.
+    /// @param orders Array of order structs containing order specifications.
+    /// @param txOrigin Required signer of Ethereum transaction calling this function.
+    /// @param transactionSignature Proof that the transaction has been signed by the signer.
+    /// @param approvalExpirationTimeSeconds Array of expiration times in seconds for which each corresponding approval signature expires.
+    /// @param approvalSignatures Array of signatures that correspond to the feeRecipients of each order.
+    function assertValidTransactionOrdersApproval(
+        LibZeroExTransaction.ZeroExTransaction memory transaction,
+        LibOrder.Order[] memory orders,
+        address txOrigin,
+        bytes memory transactionSignature,
+        uint256[] memory approvalExpirationTimeSeconds,
+        bytes[] memory approvalSignatures
+    )
         internal
-        pure
-        returns (LibOrder.Order[] memory orders);
+        view;
 }

contracts/coordinator/package.json

@@ -33,7 +33,7 @@
         "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol"
     },
     "config": {
-        "abis": "./generated-artifacts/@(Coordinator|CoordinatorRegistry|TestLibs|TestMixins).json",
+        "abis": "./generated-artifacts/@(Coordinator|CoordinatorRegistry).json",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
     },
     "repository": {