From 7aa7e0f06f39bab3680764ab9060419efcc46997 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 12 May 2019 13:28:07 -0700 Subject: [PATCH 1/5] Add cancelOrderNoThrow and batchCancelOrdersNoThrow --- .../contracts/src/MixinWrapperFunctions.sol | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol index 3da63dbd4f..958e8c4492 100644 --- a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol @@ -374,6 +374,19 @@ contract MixinWrapperFunctions is return totalFillResults; } + /// @dev After calling, the order can not be filled anymore. + /// @return True if the order was cancelled successfully. + /// @param order Order to cancel. Order must be OrderStatus.FILLABLE. + function cancelOrderNoThrow(LibOrder.Order memory order) + public + returns (bool didCancel) + { + // bytes4(keccak256("cancelOrder((address,address,address,address,uint256,uint256,uint256,uint256,uint256,uint256,bytes,bytes))")) = 0xd46b02c3 + bytes memory cancelOrderCallData = abi.encodeWithSelector(0xd46b02c3, order); + (didCancel,) = address(this).delegatecall(cancelOrderCallData); + return didCancel; + } + /// @dev Synchronously cancels multiple orders in a single transaction. /// @param orders Array of order specifications. function batchCancelOrders(LibOrder.Order[] memory orders) @@ -386,6 +399,21 @@ contract MixinWrapperFunctions is } } + /// @dev Synchronously cancels multiple orders in a single transaction. + /// @param orders Array of order specifications. + /// @return Bool array containing results of each individual cancellation. + function batchCancelOrdersNoThrow(LibOrder.Order[] memory orders) + public + returns (bool[] memory) + { + uint256 ordersLength = orders.length; + bool[] memory didCancel = new bool[](ordersLength); + for (uint256 i = 0; i != ordersLength; i++) { + didCancel[i] = cancelOrderNoThrow(orders[i]); + } + return didCancel; + } + /// @dev Fetches information for all passed in orders. /// @param orders Array of order specifications. /// @return Array of OrderInfo instances that correspond to each order. From 82a688f38e03d605506d007324324beaed8df921 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 12 May 2019 14:13:02 -0700 Subject: [PATCH 2/5] Use build in abi.encode instead of LibAbiEncoder for fillOrderNoThrow --- .../exchange/contracts/src/MixinWrapperFunctions.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol index 958e8c4492..0412b45cc6 100644 --- a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol @@ -23,7 +23,7 @@ import "@0x/contracts-utils/contracts/src/ReentrancyGuard.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol"; -import "@0x/contracts-exchange-libs/contracts/src/LibAbiEncoder.sol"; +import "@0x/contracts-exchange-libs/contracts/src/LibExchangeSelectors.sol"; import "./mixins/MExchangeCore.sol"; import "./mixins/MWrapperFunctions.sol"; import "./mixins/MExchangeRichErrors.sol"; @@ -74,7 +74,8 @@ contract MixinWrapperFunctions is returns (FillResults memory fillResults) { // ABI encode calldata for `fillOrder` - bytes memory fillOrderCalldata = _abiEncodeFillOrder( + bytes memory fillOrderCalldata = abi.encodeWithSelector( + FILL_ORDER_SELECTOR, order, takerAssetFillAmount, signature @@ -381,8 +382,7 @@ contract MixinWrapperFunctions is public returns (bool didCancel) { - // bytes4(keccak256("cancelOrder((address,address,address,address,uint256,uint256,uint256,uint256,uint256,uint256,bytes,bytes))")) = 0xd46b02c3 - bytes memory cancelOrderCallData = abi.encodeWithSelector(0xd46b02c3, order); + bytes memory cancelOrderCallData = abi.encodeWithSelector(CANCEL_ORDER_SELECTOR, order); (didCancel,) = address(this).delegatecall(cancelOrderCallData); return didCancel; } From d38328203651cb2068fb9febfa059c540eddd4b9 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 21 May 2019 12:01:56 -0500 Subject: [PATCH 3/5] Remove LibAbiEncoder --- .../contracts/src/MixinExchangeWrapper.sol | 7 +- contracts/exchange-libs/compiler.json | 1 - .../contracts/src/LibAbiEncoder.sol | 215 ------------------ .../exchange-libs/contracts/test/TestLibs.sol | 21 +- contracts/exchange-libs/package.json | 2 +- contracts/exchange-libs/src/artifacts.ts | 2 - contracts/exchange-libs/src/wrappers.ts | 1 - contracts/exchange-libs/tsconfig.json | 1 - .../contracts/src/MixinWrapperFunctions.sol | 2 +- .../utils/fill_order_combinatorial_utils.ts | 15 -- 10 files changed, 7 insertions(+), 260 deletions(-) delete mode 100644 contracts/exchange-libs/contracts/src/LibAbiEncoder.sol diff --git a/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol b/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol index 032067627e..33672623da 100644 --- a/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol +++ b/contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol @@ -21,17 +21,17 @@ pragma experimental ABIEncoderV2; import "./libs/LibConstants.sol"; import "./mixins/MExchangeWrapper.sol"; -import "@0x/contracts-exchange-libs/contracts/src/LibAbiEncoder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibOrder.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol"; import "@0x/contracts-exchange-libs/contracts/src/LibMath.sol"; +import "@0x/contracts-exchange-libs/contracts/src/LibExchangeSelectors.sol"; contract MixinExchangeWrapper is - LibAbiEncoder, LibFillResults, LibMath, LibConstants, + LibExchangeSelectors, MExchangeWrapper { /// @dev Fills the input order. @@ -49,7 +49,8 @@ contract MixinExchangeWrapper is returns (FillResults memory fillResults) { // ABI encode calldata for `fillOrder` - bytes memory fillOrderCalldata = _abiEncodeFillOrder( + bytes memory fillOrderCalldata = abi.encodeWithSelector( + FILL_ORDER_SELECTOR, order, takerAssetFillAmount, signature diff --git a/contracts/exchange-libs/compiler.json b/contracts/exchange-libs/compiler.json index 179dd9683c..2ecb45ea12 100644 --- a/contracts/exchange-libs/compiler.json +++ b/contracts/exchange-libs/compiler.json @@ -23,7 +23,6 @@ } }, "contracts": [ - "src/LibAbiEncoder.sol", "src/LibConstants.sol", "src/LibEIP712ExchangeDomain.sol", "src/LibFillResults.sol", diff --git a/contracts/exchange-libs/contracts/src/LibAbiEncoder.sol b/contracts/exchange-libs/contracts/src/LibAbiEncoder.sol deleted file mode 100644 index a83d45560e..0000000000 --- a/contracts/exchange-libs/contracts/src/LibAbiEncoder.sol +++ /dev/null @@ -1,215 +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 "./LibOrder.sol"; - - -contract LibAbiEncoder { - - /// @dev ABI encodes calldata for `fillOrder`. - /// @param order Order struct containing order specifications. - /// @param takerAssetFillAmount Desired amount of takerAsset to sell. - /// @param signature Proof that order has been created by maker. - /// @return ABI encoded calldata for `fillOrder`. - function _abiEncodeFillOrder( - LibOrder.Order memory order, - uint256 takerAssetFillAmount, - bytes memory signature - ) - internal - pure - returns (bytes memory fillOrderCalldata) - { - // We need to call MExchangeCore.fillOrder using a delegatecall in - // assembly so that we can intercept a call that throws. For this, we - // need the input encoded in memory in the Ethereum ABIv2 format [1]. - - // | Area | Offset | Length | Contents | - // | -------- |--------|---------|-------------------------------------------- | - // | Header | 0x00 | 4 | function selector | - // | Params | | 3 * 32 | function parameters: | - // | | 0x00 | | 1. offset to order (*) | - // | | 0x20 | | 2. takerAssetFillAmount | - // | | 0x40 | | 3. offset to signature (*) | - // | Data | | 12 * 32 | order: | - // | | 0x000 | | 1. senderAddress | - // | | 0x020 | | 2. makerAddress | - // | | 0x040 | | 3. takerAddress | - // | | 0x060 | | 4. feeRecipientAddress | - // | | 0x080 | | 5. makerAssetAmount | - // | | 0x0A0 | | 6. takerAssetAmount | - // | | 0x0C0 | | 7. makerFeeAmount | - // | | 0x0E0 | | 8. takerFeeAmount | - // | | 0x100 | | 9. expirationTimeSeconds | - // | | 0x120 | | 10. salt | - // | | 0x140 | | 11. Offset to makerAssetData (*) | - // | | 0x160 | | 12. Offset to takerAssetData (*) | - // | | 0x180 | 32 | makerAssetData Length | - // | | 0x1A0 | ** | makerAssetData Contents | - // | | 0x1C0 | 32 | takerAssetData Length | - // | | 0x1E0 | ** | takerAssetData Contents | - // | | 0x200 | 32 | signature Length | - // | | 0x220 | ** | signature Contents | - - // * Offsets are calculated from the beginning of the current area: Header, Params, Data: - // An offset stored in the Params area is calculated from the beginning of the Params section. - // An offset stored in the Data area is calculated from the beginning of the Data section. - - // ** The length of dynamic array contents are stored in the field immediately preceeding the contents. - - // [1]: https://solidity.readthedocs.io/en/develop/abi-spec.html - - assembly { - - // Areas below may use the following variables: - // 1. Start -- Start of this area in memory - // 2. End -- End of this area in memory. This value may - // be precomputed (before writing contents), - // or it may be computed as contents are written. - // 3. Offset -- Current offset into area. If an area's End - // is precomputed, this variable tracks the - // offsets of contents as they are written. - - /////// Setup Header Area /////// - // Load free memory pointer - fillOrderCalldata := mload(0x40) - // bytes4(keccak256("fillOrder((address,address,address,address,uint256,uint256,uint256,uint256,uint256,uint256,bytes,bytes),uint256,bytes)")) - // = 0xb4be83d5 - // Leave 0x20 bytes to store the length - mstore(add(fillOrderCalldata, 0x20), 0xb4be83d500000000000000000000000000000000000000000000000000000000) - let headerAreaEnd := add(fillOrderCalldata, 0x24) - - /////// Setup Params Area /////// - // This area is preallocated and written to later. - // This is because we need to fill in offsets that have not yet been calculated. - let paramsAreaStart := headerAreaEnd - let paramsAreaEnd := add(paramsAreaStart, 0x60) - let paramsAreaOffset := paramsAreaStart - - /////// Setup Data Area /////// - let dataAreaStart := paramsAreaEnd - let dataAreaEnd := dataAreaStart - - // Offset from the source data we're reading from - let sourceOffset := order - // arrayLenBytes and arrayLenWords track the length of a dynamically-allocated bytes array. - let arrayLenBytes := 0 - let arrayLenWords := 0 - - /////// Write order Struct /////// - // Write memory location of Order, relative to the start of the - // parameter list, then increment the paramsAreaOffset respectively. - mstore(paramsAreaOffset, sub(dataAreaEnd, paramsAreaStart)) - paramsAreaOffset := add(paramsAreaOffset, 0x20) - - // Write values for each field in the order - // It would be nice to use a loop, but we save on gas by writing - // the stores sequentially. - mstore(dataAreaEnd, mload(sourceOffset)) // makerAddress - mstore(add(dataAreaEnd, 0x20), mload(add(sourceOffset, 0x20))) // takerAddress - mstore(add(dataAreaEnd, 0x40), mload(add(sourceOffset, 0x40))) // feeRecipientAddress - mstore(add(dataAreaEnd, 0x60), mload(add(sourceOffset, 0x60))) // senderAddress - mstore(add(dataAreaEnd, 0x80), mload(add(sourceOffset, 0x80))) // makerAssetAmount - mstore(add(dataAreaEnd, 0xA0), mload(add(sourceOffset, 0xA0))) // takerAssetAmount - mstore(add(dataAreaEnd, 0xC0), mload(add(sourceOffset, 0xC0))) // makerFeeAmount - mstore(add(dataAreaEnd, 0xE0), mload(add(sourceOffset, 0xE0))) // takerFeeAmount - mstore(add(dataAreaEnd, 0x100), mload(add(sourceOffset, 0x100))) // expirationTimeSeconds - mstore(add(dataAreaEnd, 0x120), mload(add(sourceOffset, 0x120))) // salt - mstore(add(dataAreaEnd, 0x140), mload(add(sourceOffset, 0x140))) // Offset to makerAssetData - mstore(add(dataAreaEnd, 0x160), mload(add(sourceOffset, 0x160))) // Offset to takerAssetData - dataAreaEnd := add(dataAreaEnd, 0x180) - sourceOffset := add(sourceOffset, 0x180) - - // Write offset to - mstore(add(dataAreaStart, mul(10, 0x20)), sub(dataAreaEnd, dataAreaStart)) - - // Calculate length of - sourceOffset := mload(add(order, 0x140)) // makerAssetData - arrayLenBytes := mload(sourceOffset) - sourceOffset := add(sourceOffset, 0x20) - arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) - - // Write length of - mstore(dataAreaEnd, arrayLenBytes) - dataAreaEnd := add(dataAreaEnd, 0x20) - - // Write contents of - for {let i := 0} lt(i, arrayLenWords) {i := add(i, 1)} { - mstore(dataAreaEnd, mload(sourceOffset)) - dataAreaEnd := add(dataAreaEnd, 0x20) - sourceOffset := add(sourceOffset, 0x20) - } - - // Write offset to - mstore(add(dataAreaStart, mul(11, 0x20)), sub(dataAreaEnd, dataAreaStart)) - - // Calculate length of - sourceOffset := mload(add(order, 0x160)) // takerAssetData - arrayLenBytes := mload(sourceOffset) - sourceOffset := add(sourceOffset, 0x20) - arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) - - // Write length of - mstore(dataAreaEnd, arrayLenBytes) - dataAreaEnd := add(dataAreaEnd, 0x20) - - // Write contents of - for {let i := 0} lt(i, arrayLenWords) {i := add(i, 1)} { - mstore(dataAreaEnd, mload(sourceOffset)) - dataAreaEnd := add(dataAreaEnd, 0x20) - sourceOffset := add(sourceOffset, 0x20) - } - - /////// Write takerAssetFillAmount /////// - mstore(paramsAreaOffset, takerAssetFillAmount) - paramsAreaOffset := add(paramsAreaOffset, 0x20) - - /////// Write signature /////// - // Write offset to paramsArea - mstore(paramsAreaOffset, sub(dataAreaEnd, paramsAreaStart)) - - // Calculate length of signature - sourceOffset := signature - arrayLenBytes := mload(sourceOffset) - sourceOffset := add(sourceOffset, 0x20) - arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) - - // Write length of signature - mstore(dataAreaEnd, arrayLenBytes) - dataAreaEnd := add(dataAreaEnd, 0x20) - - // Write contents of signature - for {let i := 0} lt(i, arrayLenWords) {i := add(i, 1)} { - mstore(dataAreaEnd, mload(sourceOffset)) - dataAreaEnd := add(dataAreaEnd, 0x20) - sourceOffset := add(sourceOffset, 0x20) - } - - // Set length of calldata - mstore(fillOrderCalldata, sub(dataAreaEnd, add(fillOrderCalldata, 0x20))) - - // Increment free memory pointer - mstore(0x40, dataAreaEnd) - } - - return fillOrderCalldata; - } -} diff --git a/contracts/exchange-libs/contracts/test/TestLibs.sol b/contracts/exchange-libs/contracts/test/TestLibs.sol index 1ae1843059..8548e0fbdd 100644 --- a/contracts/exchange-libs/contracts/test/TestLibs.sol +++ b/contracts/exchange-libs/contracts/test/TestLibs.sol @@ -23,7 +23,6 @@ import "../src/LibEIP712ExchangeDomain.sol"; import "../src/LibMath.sol"; import "../src/LibOrder.sol"; import "../src/LibFillResults.sol"; -import "../src/LibAbiEncoder.sol"; // solhint-disable no-empty-blocks @@ -31,31 +30,13 @@ contract TestLibs is LibEIP712ExchangeDomain, LibMath, LibOrder, - LibFillResults, - LibAbiEncoder + LibFillResults { constructor (uint256 chainId) public LibEIP712ExchangeDomain(chainId, address(0)) {} - function abiEncodeFillOrder( - Order memory order, - uint256 takerAssetFillAmount, - bytes memory signature - ) - public - pure - returns (bytes memory fillOrderCalldata) - { - fillOrderCalldata = _abiEncodeFillOrder( - order, - takerAssetFillAmount, - signature - ); - return fillOrderCalldata; - } - function getPartialAmountFloor( uint256 numerator, uint256 denominator, diff --git a/contracts/exchange-libs/package.json b/contracts/exchange-libs/package.json index 5f3e5c7f50..9c3eba6202 100644 --- a/contracts/exchange-libs/package.json +++ b/contracts/exchange-libs/package.json @@ -34,7 +34,7 @@ "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol" }, "config": { - "abis": "./generated-artifacts/@(LibAbiEncoder|LibAssetProxyErrors|LibConstants|LibEIP712ExchangeDomain|LibFillResults|LibMath|LibOrder|LibZeroExTransaction|TestLibs).json", + "abis": "./generated-artifacts/@(LibAssetProxyErrors|LibConstants|LibEIP712ExchangeDomain|LibFillResults|LibMath|LibOrder|LibZeroExTransaction|TestLibs).json", "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually." }, "repository": { diff --git a/contracts/exchange-libs/src/artifacts.ts b/contracts/exchange-libs/src/artifacts.ts index 700d08af69..6b3159f20a 100644 --- a/contracts/exchange-libs/src/artifacts.ts +++ b/contracts/exchange-libs/src/artifacts.ts @@ -5,7 +5,6 @@ */ import { ContractArtifact } from 'ethereum-types'; -import * as LibAbiEncoder from '../generated-artifacts/LibAbiEncoder.json'; import * as LibConstants from '../generated-artifacts/LibConstants.json'; import * as LibEIP712ExchangeDomain from '../generated-artifacts/LibEIP712ExchangeDomain.json'; import * as LibFillResults from '../generated-artifacts/LibFillResults.json'; @@ -14,7 +13,6 @@ import * as LibOrder from '../generated-artifacts/LibOrder.json'; import * as LibZeroExTransaction from '../generated-artifacts/LibZeroExTransaction.json'; import * as TestLibs from '../generated-artifacts/TestLibs.json'; export const artifacts = { - LibAbiEncoder: LibAbiEncoder as ContractArtifact, LibConstants: LibConstants as ContractArtifact, LibFillResults: LibFillResults as ContractArtifact, LibMath: LibMath as ContractArtifact, diff --git a/contracts/exchange-libs/src/wrappers.ts b/contracts/exchange-libs/src/wrappers.ts index 4d9adda8cf..4908d5a01a 100644 --- a/contracts/exchange-libs/src/wrappers.ts +++ b/contracts/exchange-libs/src/wrappers.ts @@ -3,7 +3,6 @@ * Warning: This file is auto-generated by contracts-gen. Don't edit manually. * ----------------------------------------------------------------------------- */ -export * from '../generated-wrappers/lib_abi_encoder'; export * from '../generated-wrappers/lib_constants'; export * from '../generated-wrappers/lib_e_i_p712_exchange_domain'; export * from '../generated-wrappers/lib_fill_results'; diff --git a/contracts/exchange-libs/tsconfig.json b/contracts/exchange-libs/tsconfig.json index 08efc17267..c189082e61 100644 --- a/contracts/exchange-libs/tsconfig.json +++ b/contracts/exchange-libs/tsconfig.json @@ -3,7 +3,6 @@ "compilerOptions": { "outDir": "lib", "rootDir": ".", "resolveJsonModule": true }, "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"], "files": [ - "generated-artifacts/LibAbiEncoder.json", "generated-artifacts/LibConstants.json", "generated-artifacts/LibEIP712ExchangeDomain.json", "generated-artifacts/LibFillResults.json", diff --git a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol index 0412b45cc6..d7f8ccd1b6 100644 --- a/contracts/exchange/contracts/src/MixinWrapperFunctions.sol +++ b/contracts/exchange/contracts/src/MixinWrapperFunctions.sol @@ -33,7 +33,7 @@ contract MixinWrapperFunctions is ReentrancyGuard, LibMath, LibFillResults, - LibAbiEncoder, + LibExchangeSelectors, MExchangeCore, MWrapperFunctions, MExchangeRichErrors diff --git a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts index a3cb1e25f7..3e331c7b31 100644 --- a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts +++ b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts @@ -431,8 +431,6 @@ export class FillOrderCombinatorialUtils { lazyStore, fillRevertReasonIfExists, ); - - await this._abiEncodeFillOrderAndAssertOutcomeAsync(signedOrder, takerAssetFillAmount); } private async _fillOrderAndAssertOutcomeAsync( signedOrder: SignedOrder, @@ -609,19 +607,6 @@ export class FillOrderCombinatorialUtils { 'ZRXAssetBalanceOfFeeRecipient', ); } - private async _abiEncodeFillOrderAndAssertOutcomeAsync( - signedOrder: SignedOrder, - takerAssetFillAmount: BigNumber, - ): Promise { - const params = orderUtils.createFill(signedOrder, takerAssetFillAmount); - const abiDataEncodedByContract = await this.testLibsContract.abiEncodeFillOrder.callAsync( - params.order, - params.takerAssetFillAmount, - params.signature, - ); - const paramsDecodedByClient = this.exchangeWrapper.abiDecodeFillOrder(abiDataEncodedByContract); - expect(paramsDecodedByClient).to.be.deep.equal(params, 'ABIEncodedFillOrderData'); - } private async _getTakerAssetFillAmountAsync( signedOrder: SignedOrder, takerAssetFillAmountScenario: TakerAssetFillAmountScenario, From d9ae1df4c80cb3d9fc11f208baec51273b671ed8 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 21 May 2019 14:53:43 -0500 Subject: [PATCH 4/5] Add tests for cancelOrderNoThrow and batchCancelOrdersNoThrow --- .../exchange/test/utils/exchange_wrapper.ts | 18 +++ contracts/exchange/test/wrapper.ts | 112 ++++++++++++++++++ 2 files changed, 130 insertions(+) diff --git a/contracts/exchange/test/utils/exchange_wrapper.ts b/contracts/exchange/test/utils/exchange_wrapper.ts index a02d8e1f46..a69cd080b4 100644 --- a/contracts/exchange/test/utils/exchange_wrapper.ts +++ b/contracts/exchange/test/utils/exchange_wrapper.ts @@ -54,6 +54,15 @@ export class ExchangeWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } + public async cancelOrderNoThrowAsync( + signedOrder: SignedOrder, + from: string, + ): Promise { + const params = orderUtils.createCancel(signedOrder); + const txHash = await this._exchange.cancelOrderNoThrow.sendTransactionAsync(params.order, { from }); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); + return tx; + } public async fillOrKillOrderAsync( signedOrder: SignedOrder, from: string, @@ -198,6 +207,15 @@ export class ExchangeWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } + public async batchCancelOrdersNoThrowAsync( + orders: SignedOrder[], + from: string, + ): Promise { + const params = formatters.createBatchCancel(orders); + const txHash = await this._exchange.batchCancelOrdersNoThrow.sendTransactionAsync(params.orders, { from }); + const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); + return tx; + } public async cancelOrdersUpToAsync(salt: BigNumber, from: string): Promise { const txHash = await this._exchange.cancelOrdersUpTo.sendTransactionAsync(salt, { from }); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); diff --git a/contracts/exchange/test/wrapper.ts b/contracts/exchange/test/wrapper.ts index 06878fe6aa..bd12c2801a 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -18,11 +18,13 @@ import { OrderStatus, RevertReason, SignedOrder } from '@0x/types'; import { BigNumber, providerUtils } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; +import { LogWithDecodedArgs } from 'ethereum-types'; import * as _ from 'lodash'; import { artifacts, constants as exchangeConstants, + ExchangeCancelEventArgs, ExchangeContract, ExchangeWrapper, ReentrantERC20TokenContract, @@ -441,6 +443,88 @@ describe('Exchange wrappers', () => { }); }); + describe('cancelOrderNoThrow', () => { + it('should return false if not sent by maker', async () => { + const signedOrder = await orderFactory.newSignedOrderAsync(); + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + const didCancel = await exchange.cancelOrderNoThrow.callAsync(signedOrder, { from: takerAddress }); + const isCancelled = await exchange.cancelled.callAsync(orderHash); + expect(didCancel).to.equal(false); + expect(isCancelled).to.equal(false); + }); + + it('should return false if makerAssetAmount is 0', async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber(0), + }); + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + const didCancel = await exchange.cancelOrderNoThrow.callAsync(signedOrder, { from: makerAddress }); + const isCancelled = await exchange.cancelled.callAsync(orderHash); + expect(didCancel).to.equal(false); + expect(isCancelled).to.equal(false); + }); + + it('should return false if takerAssetAmount is 0', async () => { + const signedOrder = await orderFactory.newSignedOrderAsync({ + takerAssetAmount: new BigNumber(0), + }); + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + const didCancel = await exchange.cancelOrderNoThrow.callAsync(signedOrder, { from: makerAddress }); + const isCancelled = await exchange.cancelled.callAsync(orderHash); + expect(didCancel).to.equal(false); + expect(isCancelled).to.equal(false); + }); + + it('should be able to cancel an order', async () => { + const signedOrder = await orderFactory.newSignedOrderAsync(); + await exchangeWrapper.cancelOrderNoThrowAsync(signedOrder, makerAddress); + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHash, OrderStatus.Cancelled); + const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { + takerAssetFillAmount: signedOrder.takerAssetAmount.div(2), + }); + return expect(tx).to.revertWith(expectedError); + }); + + it('should log 1 event with correct arguments if successful', async () => { + const signedOrder = await orderFactory.newSignedOrderAsync(); + const res = await exchangeWrapper.cancelOrderNoThrowAsync(signedOrder, makerAddress); + expect(res.logs).to.have.length(1); + + const log = res.logs[0] as LogWithDecodedArgs; + const logArgs = log.args; + + expect(signedOrder.makerAddress).to.be.equal(logArgs.makerAddress); + expect(signedOrder.makerAddress).to.be.equal(logArgs.senderAddress); + expect(signedOrder.feeRecipientAddress).to.be.equal(logArgs.feeRecipientAddress); + expect(signedOrder.makerAssetData).to.be.equal(logArgs.makerAssetData); + expect(signedOrder.takerAssetData).to.be.equal(logArgs.takerAssetData); + expect(orderHashUtils.getOrderHashHex(signedOrder)).to.be.equal(logArgs.orderHash); + }); + + it('should return false if already cancelled', async () => { + const signedOrder = await orderFactory.newSignedOrderAsync(); + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress); + const isCancelled = await exchange.cancelled.callAsync(orderHash); + expect(isCancelled).to.equal(true); + const didCancel = await exchange.cancelOrderNoThrow.callAsync(signedOrder, { from: makerAddress }); + expect(didCancel).to.equal(false); + }); + + it('should return false if order is expired', async () => { + const currentTimestamp = await getLatestBlockTimestampAsync(); + const signedOrder = await orderFactory.newSignedOrderAsync({ + expirationTimeSeconds: new BigNumber(currentTimestamp).minus(10), + }); + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); + const didCancel = await exchange.cancelOrderNoThrow.callAsync(signedOrder, { from: makerAddress }); + const isCancelled = await exchange.cancelled.callAsync(orderHash); + expect(didCancel).to.equal(false); + expect(isCancelled).to.equal(false); + }); + }); + describe('batch functions', () => { let signedOrders: SignedOrder[]; beforeEach(async () => { @@ -1279,6 +1363,34 @@ describe('Exchange wrappers', () => { const newBalances = await erc20Wrapper.getBalancesAsync(); expect(erc20Balances).to.be.deep.equal(newBalances); }); + it('should revert if a single cancel fails', async () => { + await exchangeWrapper.cancelOrderAsync(signedOrders[1], makerAddress); + const orderHash = orderHashUtils.getOrderHashHex(signedOrders[1]); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHash, OrderStatus.Cancelled); + const tx = exchangeWrapper.batchCancelOrdersAsync(signedOrders, makerAddress); + return expect(tx).to.revertWith(expectedError); + }); + }); + + describe('batchCancelOrdersNoThrow', () => { + it('should be able to cancel multiple signedOrders', async () => { + const takerAssetCancelAmounts = _.map(signedOrders, signedOrder => signedOrder.takerAssetAmount); + await exchangeWrapper.batchCancelOrdersNoThrowAsync(signedOrders, makerAddress); + + await exchangeWrapper.batchFillOrdersNoThrowAsync(signedOrders, takerAddress, { + takerAssetFillAmounts: takerAssetCancelAmounts, + }); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(erc20Balances).to.be.deep.equal(newBalances); + }); + it('should return false for cancelled orders', async () => { + await exchangeWrapper.cancelOrderAsync(signedOrders[1], makerAddress); + const didCancelArray = await exchange.batchCancelOrdersNoThrow.callAsync(signedOrders, { + from: makerAddress, + }); + expect(didCancelArray[0]).to.equal(true); + expect(didCancelArray[1]).to.equal(false); + }); }); describe('getOrdersInfo', () => { From 1e7e56606b9ff05906ee191fdab3187aced0bdc1 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 21 May 2019 16:58:17 -0500 Subject: [PATCH 5/5] Fix linting errors --- .../erc20/contracts/test/UntransferrableDummyERC20Token.sol | 1 + contracts/exchange/test/wrapper.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/contracts/erc20/contracts/test/UntransferrableDummyERC20Token.sol b/contracts/erc20/contracts/test/UntransferrableDummyERC20Token.sol index e1e3b3b68d..5c0e424608 100644 --- a/contracts/erc20/contracts/test/UntransferrableDummyERC20Token.sol +++ b/contracts/erc20/contracts/test/UntransferrableDummyERC20Token.sol @@ -22,6 +22,7 @@ import "./DummyERC20Token.sol"; // solhint-disable no-empty-blocks +// solhint-disable no-unused-vars contract UntransferrableDummyERC20Token is DummyERC20Token { diff --git a/contracts/exchange/test/wrapper.ts b/contracts/exchange/test/wrapper.ts index bd12c2801a..575f48215d 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -34,6 +34,7 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +// tslint:disable:no-unnecessary-type-assertion describe('Exchange wrappers', () => { let chainId: number; let makerAddress: string;