Merge pull request #1729 from 0xProject/feat/coordinator/functionVisibility

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": [
+    "contracts": ["src/Coordinator.sol", "src/registry/CoordinatorRegistry.sol"]
-        "src/Coordinator.sol",
-        "src/registry/CoordinatorRegistry.sol",
-        "test/TestLibs.sol",
-        "test/TestMixins.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.
+    /// @dev Decodes the orders from Exchange calldata representing any fill method.
-    /// @param transaction 0x transaction containing salt, signerAddress, and data.
+    /// @param data Exchange calldata representing a fill method.
-    /// @param orders Array of order structs containing order specifications.
+    /// @return The orders from the Exchange calldata.
-    /// @param txOrigin Required signer of Ethereum transaction calling this function.
+    function decodeOrdersFromFillData(bytes memory data)
-    /// @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
-    )
         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.
+    /// @dev Validates that the feeRecipients of a batch of order have approved a 0x transaction.
-    /// @param data Exchange calldata representing a fill method.
+    /// @param transaction 0x transaction containing salt, signerAddress, and data.
-    /// @return The orders from the Exchange calldata.
+    /// @param orders Array of order structs containing order specifications.
-    function decodeFillDataOrders(bytes memory data)
+    /// @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
+        view;
-        returns (LibOrder.Order[] memory orders);
 }

contracts/coordinator/contracts/test/TestLibs.sol

@@ -1,61 +0,0 @@
-/*
-
-  Copyright 2018 ZeroEx Intl.
-
-  Licensed under the Apache License, Version 2.0 (the "License");
-  you may not use this file except in compliance with the License.
-  You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing, software
-  distributed under the License is distributed on an "AS IS" BASIS,
-  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-  See the License for the specific language governing permissions and
-  limitations under the License.
-
-*/
-
-pragma solidity ^0.5.5;
-pragma experimental "ABIEncoderV2";
-
-import "../src/libs/LibConstants.sol";
-import "../src/libs/LibCoordinatorApproval.sol";
-import "../src/libs/LibZeroExTransaction.sol";
-
-
-// solhint-disable no-empty-blocks
-contract TestLibs is
-    LibConstants,
-    LibCoordinatorApproval,
-    LibZeroExTransaction
-{
-    constructor (address _exchange)
-        public
-        LibConstants(_exchange)
-    {}
-
-    /// @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 publicGetCoordinatorApprovalHash(CoordinatorApproval memory approval)
-        public
-        view
-        returns (bytes32 approvalHash)
-    {
-        approvalHash = getCoordinatorApprovalHash(approval);
-        return approvalHash;
-    }
-
-    /// @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 the Exchange contract.
-    function publicGetTransactionHash(ZeroExTransaction memory transaction)
-        public
-        view
-        returns (bytes32 transactionHash)
-    {
-        transactionHash = getTransactionHash(transaction);
-        return transactionHash;
-    }
-}

contracts/coordinator/contracts/test/TestMixins.sol

@@ -1,37 +0,0 @@
-/*
-
-  Copyright 2018 ZeroEx Intl.
-
-  Licensed under the Apache License, Version 2.0 (the "License");
-  you may not use this file except in compliance with the License.
-  You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-  Unless required by applicable law or agreed to in writing, software
-  distributed under the License is distributed on an "AS IS" BASIS,
-  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-  See the License for the specific language governing permissions and
-  limitations under the License.
-
-*/
-
-pragma solidity ^0.5.5;
-pragma experimental "ABIEncoderV2";
-
-import "../src/libs/LibConstants.sol";
-import "../src/MixinSignatureValidator.sol";
-import "../src/MixinCoordinatorApprovalVerifier.sol";
-
-
-// solhint-disable no-empty-blocks
-contract TestMixins is
-    LibConstants,
-    MixinSignatureValidator,
-    MixinCoordinatorApprovalVerifier
-{
-    constructor (address _exchange)
-        public
-        LibConstants(_exchange)
-    {}
-}

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": {

contracts/coordinator/src/artifacts.ts

@@ -7,11 +7,7 @@ import { ContractArtifact } from 'ethereum-types';
 
 import * as Coordinator from '../generated-artifacts/Coordinator.json';
 import * as CoordinatorRegistry from '../generated-artifacts/CoordinatorRegistry.json';
-import * as TestLibs from '../generated-artifacts/TestLibs.json';
-import * as TestMixins from '../generated-artifacts/TestMixins.json';
 export const artifacts = {
     Coordinator: Coordinator as ContractArtifact,
     CoordinatorRegistry: CoordinatorRegistry as ContractArtifact,
-    TestLibs: TestLibs as ContractArtifact,
-    TestMixins: TestMixins as ContractArtifact,
 };

contracts/coordinator/src/wrappers.ts

@@ -5,5 +5,3 @@
  */
 export * from '../generated-wrappers/coordinator';
 export * from '../generated-wrappers/coordinator_registry';
-export * from '../generated-wrappers/test_libs';
-export * from '../generated-wrappers/test_mixins';

contracts/coordinator/test/coordinator.ts

@@ -435,7 +435,7 @@ describe('Coordinator tests', () => {
     describe('cancels', () => {
         it('cancelOrder call should be successful without an approval', async () => {
             const orders = [await orderFactory.newSignedOrderAsync()];
-            const data = exchangeDataEncoder.encodeOrdersToExchangeData(constants.CANCEL_ORDERS, orders);
+            const data = exchangeDataEncoder.encodeOrdersToExchangeData(constants.CANCEL_ORDER, orders);
             const transaction = makerTransactionFactory.newSignedTransaction(data);
             const transactionReceipt = await web3Wrapper.awaitTransactionSuccessAsync(
                 await coordinatorContract.executeTransaction.sendTransactionAsync(

contracts/coordinator/test/libs.ts

@@ -4,14 +4,14 @@ import { transactionHashUtils } from '@0x/order-utils';
 import { BigNumber } from '@0x/utils';
 import * as chai from 'chai';
 
-import { artifacts, hashUtils, TestLibsContract } from '../src';
+import { artifacts, CoordinatorContract, hashUtils } from '../src';
 
 chaiSetup.configure();
 const expect = chai.expect;
 const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
 
 describe('Libs tests', () => {
-    let testLibs: TestLibsContract;
+    let coordinatorContract: CoordinatorContract;
     const exchangeAddress = addressUtils.generatePseudoRandomAddress();
 
     before(async () => {
@@ -21,8 +21,8 @@ describe('Libs tests', () => {
         await blockchainLifecycle.revertAsync();
     });
     before(async () => {
-        testLibs = await TestLibsContract.deployFrom0xArtifactAsync(
+        coordinatorContract = await CoordinatorContract.deployFrom0xArtifactAsync(
-            artifacts.TestLibs,
+            artifacts.Coordinator,
             provider,
             txDefaults,
             exchangeAddress,
@@ -44,7 +44,7 @@ describe('Libs tests', () => {
                 data: '0x1234',
             };
             const expectedTxHash = transactionHashUtils.getTransactionHashHex(tx);
-            const txHash = await testLibs.publicGetTransactionHash.callAsync(tx);
+            const txHash = await coordinatorContract.getTransactionHash.callAsync(tx);
             expect(expectedTxHash).to.eq(txHash);
         });
     });
@@ -68,11 +68,11 @@ describe('Libs tests', () => {
             };
             const expectedApprovalHash = hashUtils.getApprovalHashHex(
                 signedTx,
-                testLibs.address,
+                coordinatorContract.address,
                 txOrigin,
                 approvalExpirationTimeSeconds,
             );
-            const approvalHash = await testLibs.publicGetCoordinatorApprovalHash.callAsync(approval);
+            const approvalHash = await coordinatorContract.getCoordinatorApprovalHash.callAsync(approval);
             expect(expectedApprovalHash).to.eq(approvalHash);
         });
     });

contracts/coordinator/test/mixins.ts

@@ -16,7 +16,7 @@ import { BigNumber } from '@0x/utils';
 import * as chai from 'chai';
 import * as ethUtil from 'ethereumjs-util';
 
-import { ApprovalFactory, artifacts, constants, exchangeDataEncoder, TestMixinsContract } from '../src';
+import { ApprovalFactory, artifacts, constants, CoordinatorContract, exchangeDataEncoder } from '../src';
 
 chaiSetup.configure();
 const expect = chai.expect;
@@ -26,7 +26,7 @@ describe('Mixins tests', () => {
     let transactionSignerAddress: string;
     let approvalSignerAddress1: string;
     let approvalSignerAddress2: string;
-    let mixins: TestMixinsContract;
+    let mixins: CoordinatorContract;
     let transactionFactory: TransactionFactory;
     let approvalFactory1: ApprovalFactory;
     let approvalFactory2: ApprovalFactory;
@@ -40,8 +40,8 @@ describe('Mixins tests', () => {
         await blockchainLifecycle.revertAsync();
     });
     before(async () => {
-        mixins = await TestMixinsContract.deployFrom0xArtifactAsync(
+        mixins = await CoordinatorContract.deployFrom0xArtifactAsync(
-            artifacts.TestMixins,
+            artifacts.Coordinator,
             provider,
             txDefaults,
             exchangeAddress,
@@ -135,6 +135,70 @@ describe('Mixins tests', () => {
         });
     });
 
+    describe('decodeOrdersFromFillData', () => {
+        for (const fnName of constants.SINGLE_FILL_FN_NAMES) {
+            it(`should correctly decode the orders for ${fnName} data`, async () => {
+                const orders = [defaultOrder];
+                const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
+                const decodedOrders = await mixins.decodeOrdersFromFillData.callAsync(data);
+                const decodedSignedOrders = decodedOrders.map(order => ({
+                    ...order,
+                    exchangeAddress: devConstants.NULL_ADDRESS,
+                    signature: devConstants.NULL_BYTES,
+                }));
+                expect(orders).to.deep.eq(decodedSignedOrders);
+            });
+        }
+        for (const fnName of constants.BATCH_FILL_FN_NAMES) {
+            it(`should correctly decode the orders for ${fnName} data`, async () => {
+                const orders = [defaultOrder, defaultOrder];
+                const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
+                const decodedOrders = await mixins.decodeOrdersFromFillData.callAsync(data);
+                const decodedSignedOrders = decodedOrders.map(order => ({
+                    ...order,
+                    exchangeAddress: devConstants.NULL_ADDRESS,
+                    signature: devConstants.NULL_BYTES,
+                }));
+                expect(orders).to.deep.eq(decodedSignedOrders);
+            });
+        }
+        for (const fnName of constants.MARKET_FILL_FN_NAMES) {
+            it(`should correctly decode the orders for ${fnName} data`, async () => {
+                const orders = [defaultOrder, defaultOrder];
+                const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
+                const decodedOrders = await mixins.decodeOrdersFromFillData.callAsync(data);
+                const decodedSignedOrders = decodedOrders.map(order => ({
+                    ...order,
+                    exchangeAddress: devConstants.NULL_ADDRESS,
+                    signature: devConstants.NULL_BYTES,
+                }));
+                expect(orders).to.deep.eq(decodedSignedOrders);
+            });
+        }
+        for (const fnName of [constants.CANCEL_ORDER, constants.BATCH_CANCEL_ORDERS, constants.CANCEL_ORDERS_UP_TO]) {
+            it(`should correctly decode the orders for ${fnName} data`, async () => {
+                const orders = [defaultOrder, defaultOrder];
+                const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
+                const decodedOrders = await mixins.decodeOrdersFromFillData.callAsync(data);
+                const emptyArray: any[] = [];
+                expect(emptyArray).to.deep.eq(decodedOrders);
+            });
+        }
+        it('should decode an empty array for invalid data', async () => {
+            const data = '0x0123456789';
+            const decodedOrders = await mixins.decodeOrdersFromFillData.callAsync(data);
+            const emptyArray: any[] = [];
+            expect(emptyArray).to.deep.eq(decodedOrders);
+        });
+        it('should revert if data is less than 4 bytes long', async () => {
+            const data = '0x010203';
+            await expectContractCallFailedAsync(
+                mixins.decodeOrdersFromFillData.callAsync(data),
+                RevertReason.LibBytesGreaterOrEqualTo4LengthRequired,
+            );
+        });
+    });
+
     describe('Single order approvals', () => {
         for (const fnName of constants.SINGLE_FILL_FN_NAMES) {
             it(`Should be successful: function=${fnName}, caller=tx_signer, senderAddress=[verifier], approval_sig=[approver1], expiration=[valid]`, async () => {
@@ -148,15 +212,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    transactionSignerAddress,
-                    transaction.signature,
-                    [approvalExpirationTimeSeconds],
-                    [approval.signature],
-                    { from: transactionSignerAddress },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     transactionSignerAddress,
@@ -181,15 +236,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    transactionSignerAddress,
-                    transaction.signature,
-                    [approvalExpirationTimeSeconds],
-                    [approval.signature],
-                    { from: transactionSignerAddress },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     transactionSignerAddress,
@@ -203,15 +249,6 @@ describe('Mixins tests', () => {
                 const orders = [defaultOrder];
                 const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
                 const transaction = transactionFactory.newSignedTransaction(data);
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    approvalSignerAddress1,
-                    transaction.signature,
-                    [],
-                    [],
-                    { from: approvalSignerAddress1 },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     approvalSignerAddress1,
@@ -234,15 +271,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    approvalSignerAddress1,
-                    transaction.signature,
-                    [approvalExpirationTimeSeconds],
-                    [approval.signature],
-                    { from: approvalSignerAddress1 },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     approvalSignerAddress1,
@@ -256,15 +284,6 @@ describe('Mixins tests', () => {
                 const orders = [defaultOrder];
                 const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
                 const transaction = transactionFactory.newSignedTransaction(data);
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    approvalSignerAddress1,
-                    transaction.signature,
-                    [],
-                    [],
-                    { from: approvalSignerAddress1 },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     approvalSignerAddress1,
@@ -288,18 +307,6 @@ describe('Mixins tests', () => {
                     approvalExpirationTimeSeconds,
                 );
                 const signature = `${approval.signature.slice(0, 4)}FFFFFFFF${approval.signature.slice(12)}`;
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [signature],
-                        { from: transactionSignerAddress },
-                    ),
-                    RevertReason.InvalidApprovalSignature,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -323,18 +330,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [approval.signature],
-                        { from: transactionSignerAddress },
-                    ),
-                    RevertReason.ApprovalExpired,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -358,18 +353,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [approval.signature],
-                        { from: approvalSignerAddress2 },
-                    ),
-                    RevertReason.InvalidOrigin,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -401,15 +384,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    transactionSignerAddress,
-                    transaction.signature,
-                    [approvalExpirationTimeSeconds],
-                    [approval.signature],
-                    { from: transactionSignerAddress },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     transactionSignerAddress,
@@ -433,15 +407,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    transactionSignerAddress,
-                    transaction.signature,
-                    [approvalExpirationTimeSeconds],
-                    [approval.signature],
-                    { from: transactionSignerAddress },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     transactionSignerAddress,
@@ -458,15 +423,6 @@ describe('Mixins tests', () => {
                 }));
                 const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
                 const transaction = transactionFactory.newSignedTransaction(data);
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    transactionSignerAddress,
-                    transaction.signature,
-                    [],
-                    [],
-                    { from: transactionSignerAddress },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     transactionSignerAddress,
@@ -487,15 +443,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    transactionSignerAddress,
-                    transaction.signature,
-                    [approvalExpirationTimeSeconds],
-                    [approval.signature],
-                    { from: transactionSignerAddress },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     transactionSignerAddress,
@@ -521,15 +468,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    transactionSignerAddress,
-                    transaction.signature,
-                    [approvalExpirationTimeSeconds, approvalExpirationTimeSeconds],
-                    [approval1.signature, approval2.signature],
-                    { from: transactionSignerAddress },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     transactionSignerAddress,
@@ -543,15 +481,6 @@ describe('Mixins tests', () => {
                 const orders = [defaultOrder, defaultOrder];
                 const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
                 const transaction = transactionFactory.newSignedTransaction(data);
-                await mixins.assertValidTransactionOrdersApproval.callAsync(
-                    transaction,
-                    orders,
-                    approvalSignerAddress1,
-                    transaction.signature,
-                    [],
-                    [],
-                    { from: approvalSignerAddress1 },
-                );
                 await mixins.assertValidCoordinatorApprovals.callAsync(
                     transaction,
                     approvalSignerAddress1,
@@ -572,18 +501,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [approval2.signature],
-                        { from: approvalSignerAddress1 },
-                    ),
-                    RevertReason.InvalidOrigin,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -600,18 +517,6 @@ describe('Mixins tests', () => {
                 const orders = [defaultOrder, defaultOrder];
                 const data = exchangeDataEncoder.encodeOrdersToExchangeData(fnName, orders);
                 const transaction = transactionFactory.newSignedTransaction(data);
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [],
-                        [],
-                        { from: transactionSignerAddress },
-                    ),
-                    RevertReason.InvalidApprovalSignature,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -636,18 +541,6 @@ describe('Mixins tests', () => {
                     approvalExpirationTimeSeconds,
                 );
                 const signature = `${approval.signature.slice(0, 4)}FFFFFFFF${approval.signature.slice(12)}`;
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [signature],
-                        { from: transactionSignerAddress },
-                    ),
-                    RevertReason.InvalidApprovalSignature,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -677,18 +570,6 @@ describe('Mixins tests', () => {
                     approvalExpirationTimeSeconds,
                 );
                 const approvalSignature2 = `${approval2.signature.slice(0, 4)}FFFFFFFF${approval2.signature.slice(12)}`;
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds, approvalExpirationTimeSeconds],
-                        [approval1.signature, approvalSignature2],
-                        { from: transactionSignerAddress },
-                    ),
-                    RevertReason.InvalidApprovalSignature,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -713,18 +594,6 @@ describe('Mixins tests', () => {
                     approvalExpirationTimeSeconds,
                 );
                 const approvalSignature2 = `${approval2.signature.slice(0, 4)}FFFFFFFF${approval2.signature.slice(12)}`;
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        approvalSignerAddress1,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [approvalSignature2],
-                        { from: approvalSignerAddress1 },
-                    ),
-                    RevertReason.InvalidApprovalSignature,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -754,18 +623,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds2,
                 );
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds1, approvalExpirationTimeSeconds2],
-                        [approval1.signature, approval2.signature],
-                        { from: transactionSignerAddress },
-                    ),
-                    RevertReason.ApprovalExpired,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -789,18 +646,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        approvalSignerAddress1,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [approval2.signature],
-                        { from: approvalSignerAddress1 },
-                    ),
-                    RevertReason.ApprovalExpired,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -824,18 +669,6 @@ describe('Mixins tests', () => {
                     transactionSignerAddress,
                     approvalExpirationTimeSeconds,
                 );
-                expectContractCallFailedAsync(
-                    mixins.assertValidTransactionOrdersApproval.callAsync(
-                        transaction,
-                        orders,
-                        transactionSignerAddress,
-                        transaction.signature,
-                        [approvalExpirationTimeSeconds],
-                        [approval1.signature],
-                        { from: approvalSignerAddress2 },
-                    ),
-                    RevertReason.InvalidOrigin,
-                );
                 expectContractCallFailedAsync(
                     mixins.assertValidCoordinatorApprovals.callAsync(
                         transaction,
@@ -851,9 +684,9 @@ describe('Mixins tests', () => {
         }
     });
     describe('cancels', () => {
-        it('should allow the tx signer to call `cancelOrders` without approval', async () => {
+        it('should allow the tx signer to call `cancelOrder` without approval', async () => {
             const orders = [defaultOrder];
-            const data = exchangeDataEncoder.encodeOrdersToExchangeData(constants.CANCEL_ORDERS, orders);
+            const data = exchangeDataEncoder.encodeOrdersToExchangeData(constants.CANCEL_ORDER, orders);
             const transaction = transactionFactory.newSignedTransaction(data);
             await mixins.assertValidCoordinatorApprovals.callAsync(
                 transaction,

contracts/coordinator/test/utils/constants.ts

@@ -5,7 +5,7 @@ export const constants = {
     BATCH_FILL_FN_NAMES: ['batchFillOrders', 'batchFillOrKillOrders', 'batchFillOrdersNoThrow'],
     MARKET_FILL_FN_NAMES: ['marketBuyOrders', 'marketBuyOrdersNoThrow', 'marketSellOrders', 'marketSellOrdersNoThrow'],
     MATCH_ORDERS: 'matchOrders',
-    CANCEL_ORDERS: 'cancelOrders',
+    CANCEL_ORDER: 'cancelOrder',
     BATCH_CANCEL_ORDERS: 'batchCancelOrders',
     CANCEL_ORDERS_UP_TO: 'cancelOrdersUpTo',
     TIME_BUFFER: new BigNumber(1000),

contracts/coordinator/test/utils/exchange_data_encoder.ts

@@ -37,7 +37,7 @@ export const exchangeDataEncoder = {
                 orders[0].signature,
                 orders[1].signature,
             );
-        } else if (fnName === constants.CANCEL_ORDERS) {
+        } else if (fnName === constants.CANCEL_ORDER) {
             data = exchangeInstance.cancelOrder.getABIEncodedTransactionData(orders[0]);
         } else if (fnName === constants.BATCH_CANCEL_ORDERS) {
             data = exchangeInstance.batchCancelOrders.getABIEncodedTransactionData(orders);

contracts/coordinator/tsconfig.json

@@ -2,11 +2,6 @@
     "extends": "../../tsconfig",
     "compilerOptions": { "outDir": "lib", "rootDir": ".", "resolveJsonModule": true },
     "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"],
-    "files": [
+    "files": ["generated-artifacts/Coordinator.json", "generated-artifacts/CoordinatorRegistry.json"],
-        "generated-artifacts/Coordinator.json",
-        "generated-artifacts/CoordinatorRegistry.json",
-        "generated-artifacts/TestLibs.json",
-        "generated-artifacts/TestMixins.json"
-    ],
     "exclude": ["./deploy/solc/solc_bin"]
 }