diff --git a/contracts/coordinator/CHANGELOG.json b/contracts/coordinator/CHANGELOG.json index 0ad71372c0..0efc92c116 100644 --- a/contracts/coordinator/CHANGELOG.json +++ b/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": [ diff --git a/contracts/coordinator/compiler.json b/contracts/coordinator/compiler.json index 8df8f3cd33..b5e9e3e156 100644 --- a/contracts/coordinator/compiler.json +++ b/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"] } diff --git a/contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol b/contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol index 408fa69b4e..bb3be65a73 100644 --- a/contracts/coordinator/contracts/src/MixinCoordinatorApprovalVerifier.sol +++ b/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; - } } diff --git a/contracts/coordinator/contracts/src/interfaces/ICoordinatorApprovalVerifier.sol b/contracts/coordinator/contracts/src/interfaces/ICoordinatorApprovalVerifier.sol index c603e7a4fa..c0f8ccf4e0 100644 --- a/contracts/coordinator/contracts/src/interfaces/ICoordinatorApprovalVerifier.sol +++ b/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); } diff --git a/contracts/coordinator/contracts/src/libs/LibCoordinatorApproval.sol b/contracts/coordinator/contracts/src/libs/LibCoordinatorApproval.sol index 7f9ec9b134..15bd656176 100644 --- a/contracts/coordinator/contracts/src/libs/LibCoordinatorApproval.sol +++ b/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 { diff --git a/contracts/coordinator/contracts/src/libs/LibZeroExTransaction.sol b/contracts/coordinator/contracts/src/libs/LibZeroExTransaction.sol index 8d388a55a2..a5e16f9694 100644 --- a/contracts/coordinator/contracts/src/libs/LibZeroExTransaction.sol +++ b/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) { diff --git a/contracts/coordinator/contracts/src/mixins/MCoordinatorApprovalVerifier.sol b/contracts/coordinator/contracts/src/mixins/MCoordinatorApprovalVerifier.sol index d8e217279b..302350c357 100644 --- a/contracts/coordinator/contracts/src/mixins/MCoordinatorApprovalVerifier.sol +++ b/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; } diff --git a/contracts/coordinator/contracts/test/TestLibs.sol b/contracts/coordinator/contracts/test/TestLibs.sol deleted file mode 100644 index 8c615a5d42..0000000000 --- a/contracts/coordinator/contracts/test/TestLibs.sol +++ /dev/null @@ -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; - } -} diff --git a/contracts/coordinator/contracts/test/TestMixins.sol b/contracts/coordinator/contracts/test/TestMixins.sol deleted file mode 100644 index cdadce171c..0000000000 --- a/contracts/coordinator/contracts/test/TestMixins.sol +++ /dev/null @@ -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) - {} -} diff --git a/contracts/coordinator/package.json b/contracts/coordinator/package.json index 8df2e578b4..afc8a0adc3 100644 --- a/contracts/coordinator/package.json +++ b/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": { diff --git a/contracts/coordinator/src/artifacts.ts b/contracts/coordinator/src/artifacts.ts index e32e231879..8276964486 100644 --- a/contracts/coordinator/src/artifacts.ts +++ b/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, }; diff --git a/contracts/coordinator/src/wrappers.ts b/contracts/coordinator/src/wrappers.ts index d8e6cc9857..a88791dffa 100644 --- a/contracts/coordinator/src/wrappers.ts +++ b/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'; diff --git a/contracts/coordinator/test/coordinator.ts b/contracts/coordinator/test/coordinator.ts index 6215213652..912d48d922 100644 --- a/contracts/coordinator/test/coordinator.ts +++ b/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( diff --git a/contracts/coordinator/test/libs.ts b/contracts/coordinator/test/libs.ts index 85291d3583..8c08f0ec59 100644 --- a/contracts/coordinator/test/libs.ts +++ b/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( - artifacts.TestLibs, + coordinatorContract = await CoordinatorContract.deployFrom0xArtifactAsync( + 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); }); }); diff --git a/contracts/coordinator/test/mixins.ts b/contracts/coordinator/test/mixins.ts index a862791b0f..a500333048 100644 --- a/contracts/coordinator/test/mixins.ts +++ b/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( - artifacts.TestMixins, + mixins = await CoordinatorContract.deployFrom0xArtifactAsync( + 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, diff --git a/contracts/coordinator/test/utils/constants.ts b/contracts/coordinator/test/utils/constants.ts index e83a636418..fd52a86b64 100644 --- a/contracts/coordinator/test/utils/constants.ts +++ b/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), diff --git a/contracts/coordinator/test/utils/exchange_data_encoder.ts b/contracts/coordinator/test/utils/exchange_data_encoder.ts index 784c009671..8622331b6e 100644 --- a/contracts/coordinator/test/utils/exchange_data_encoder.ts +++ b/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); diff --git a/contracts/coordinator/tsconfig.json b/contracts/coordinator/tsconfig.json index 9de6c34a60..ba2c48b6cd 100644 --- a/contracts/coordinator/tsconfig.json +++ b/contracts/coordinator/tsconfig.json @@ -2,11 +2,6 @@ "extends": "../../tsconfig", "compilerOptions": { "outDir": "lib", "rootDir": ".", "resolveJsonModule": true }, "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"], - "files": [ - "generated-artifacts/Coordinator.json", - "generated-artifacts/CoordinatorRegistry.json", - "generated-artifacts/TestLibs.json", - "generated-artifacts/TestMixins.json" - ], + "files": ["generated-artifacts/Coordinator.json", "generated-artifacts/CoordinatorRegistry.json"], "exclude": ["./deploy/solc/solc_bin"] }