From b786836db61de22fa290d94ac3f12fe5342aecc7 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 2 Oct 2018 16:22:35 -0700 Subject: feat: Add OrderMatcher contract that takes spread in multiple assets by calling `matchOrders` followed by `fillOrder` --- contracts/extensions/compiler.json | 2 +- .../contracts/OrderMatcher/MixinAssets.sol | 192 ++++++++++++ .../contracts/OrderMatcher/MixinMatchOrders.sol | 323 +++++++++++++++++++++ .../contracts/OrderMatcher/OrderMatcher.sol | 37 +++ .../contracts/OrderMatcher/interfaces/IAssets.sol | 43 +++ .../OrderMatcher/interfaces/IMatchOrders.sol | 43 +++ .../OrderMatcher/interfaces/IOrderMatcher.sol | 31 ++ .../contracts/OrderMatcher/libs/LibConstants.sol | 53 ++++ .../contracts/OrderMatcher/mixins/MAssets.sol | 71 +++++ contracts/protocol/src/artifacts/index.ts | 2 + 10 files changed, 796 insertions(+), 1 deletion(-) create mode 100644 contracts/extensions/contracts/OrderMatcher/MixinAssets.sol create mode 100644 contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol create mode 100644 contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol create mode 100644 contracts/extensions/contracts/OrderMatcher/interfaces/IAssets.sol create mode 100644 contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol create mode 100644 contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol create mode 100644 contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol create mode 100644 contracts/extensions/contracts/OrderMatcher/mixins/MAssets.sol diff --git a/contracts/extensions/compiler.json b/contracts/extensions/compiler.json index e6ed0c215..1e21e6e6a 100644 --- a/contracts/extensions/compiler.json +++ b/contracts/extensions/compiler.json @@ -18,5 +18,5 @@ } } }, - "contracts": ["BalanceThresholdFilter", "DutchAuction", "Forwarder"] + "contracts": ["BalanceThresholdFilter", "DutchAuction", "Forwarder", "OrderMatcher"] } diff --git a/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol b/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol new file mode 100644 index 000000000..a2bc95e4b --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol @@ -0,0 +1,192 @@ +/* + + 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.4.24; + +import "../../utils/LibBytes/LibBytes.sol"; +import "../../utils/Ownable/Ownable.sol"; +import "../../tokens/ERC20Token/IERC20Token.sol"; +import "../../tokens/ERC721Token/IERC721Token.sol"; +import "./mixins/MAssets.sol"; +import "./libs/LibConstants.sol"; + + +contract MixinAssets is + MAssets, + Ownable, + LibConstants +{ + using LibBytes for bytes; + + /// @dev Withdraws assets from this contract. The contract requires a ZRX balance in order to + /// function optimally, and this function allows the ZRX to be withdrawn by owner. It may also be + /// used to withdraw assets that were accidentally sent to this contract. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to withdraw. + function withdrawAsset( + bytes assetData, + uint256 amount + ) + external + onlyOwner + { + transferAssetToSender(assetData, amount); + } + + /// @dev Approves or disapproves an AssetProxy to spend asset. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to approve for respective proxy. + function approveAssetProxy( + bytes assetData, + uint256 amount + ) + external + onlyOwner + { + bytes4 proxyId = assetData.readBytes4(0); + + if (proxyId == ERC20_DATA_ID) { + approveERC20Token(assetData, amount); + } else if (proxyId == ERC721_DATA_ID) { + approveERC721Token(assetData, amount); + } else { + revert("UNSUPPORTED_ASSET_PROXY"); + } + } + + /// @dev Transfers given amount of asset to sender. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to transfer to sender. + function transferAssetToSender( + bytes memory assetData, + uint256 amount + ) + internal + { + bytes4 proxyId = assetData.readBytes4(0); + + if (proxyId == ERC20_DATA_ID) { + transferERC20Token(assetData, amount); + } else if (proxyId == ERC721_DATA_ID) { + transferERC721Token(assetData, amount); + } else { + revert("UNSUPPORTED_ASSET_PROXY"); + } + } + + /// @dev Decodes ERC20 assetData and transfers given amount to sender. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to transfer to sender. + function transferERC20Token( + bytes memory assetData, + uint256 amount + ) + internal + { + address token = assetData.readAddress(16); + + // Transfer tokens. + // We do a raw call so we can check the success separate + // from the return data. + bool success = token.call(abi.encodeWithSelector( + ERC20_TRANSFER_SELECTOR, + msg.sender, + amount + )); + require( + success, + "TRANSFER_FAILED" + ); + + // Check return data. + // If there is no return data, we assume the token incorrectly + // does not return a bool. In this case we expect it to revert + // on failure, which was handled above. + // If the token does return data, we require that it is a single + // value that evaluates to true. + assembly { + if returndatasize { + success := 0 + if eq(returndatasize, 32) { + // First 64 bytes of memory are reserved scratch space + returndatacopy(0, 0, 32) + success := mload(0) + } + } + } + require( + success, + "TRANSFER_FAILED" + ); + } + + /// @dev Decodes ERC721 assetData and transfers given amount to sender. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to transfer to sender. + function transferERC721Token( + bytes memory assetData, + uint256 amount + ) + internal + { + require( + amount == 1, + "INVALID_AMOUNT" + ); + // Decode asset data. + address token = assetData.readAddress(16); + uint256 tokenId = assetData.readUint256(36); + + // Perform transfer. + IERC721Token(token).transferFrom( + address(this), + msg.sender, + tokenId + ); + } + + /// @dev Sets approval for ERC20 AssetProxy. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to approve for respective proxy. + function approveERC20Token( + bytes memory assetData, + uint256 amount + ) + internal + { + address token = assetData.readAddress(16); + require( + IERC20Token(token).approve(ERC20_PROXY_ADDRESS, amount), + "APPROVAL_FAILED" + ); + } + + /// @dev Sets approval for ERC721 AssetProxy. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to approve for respective proxy. + function approveERC721Token( + bytes memory assetData, + uint256 amount + ) + internal + { + address token = assetData.readAddress(16); + bool approval = amount >= 1; + IERC721Token(token).setApprovalForAll(ERC721_PROXY_ADDRESS, approval); + } +} diff --git a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol new file mode 100644 index 000000000..1c7cce675 --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol @@ -0,0 +1,323 @@ +/* + + 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.4.24; + +import "../../utils/Ownable/Ownable.sol"; +import "./libs/LibConstants.sol"; + + +contract MixinMatchOrders is + Ownable, + LibConstants +{ + // The below assembly in the fallback function is functionaly equivalent to the following Solidity code: + /* + /// @dev Match two complementary orders that have a profitable spread. + /// Each order is filled at their respective price point. However, the calculations are + /// carried out as though the orders are both being filled at the right order's price point. + /// The profit made by the left order is then used to fill the right order as much as possible. + /// This results in a spread being taken in terms of both assets. The spread is held within this contract. + /// @param leftOrder First order to match. + /// @param rightOrder Second order to match. + /// @param leftSignature Proof that order was created by the left maker. + /// @param rightSignature Proof that order was created by the right maker. + function matchOrders( + LibOrder.Order memory leftOrder, + LibOrder.Order memory rightOrder, + bytes memory leftSignature, + bytes memory rightSignature + ) + public + onlyOwner + { + // Match orders, maximally filling `leftOrder` + LibFillResults.MatchedFillResults memory matchedFillResults = EXCHANGE.matchOrders( + leftOrder, + rightOrder, + leftSignature, + rightSignature + ); + + // If a spread was taken, use the spread to fill remaining amount of `rightOrder` + // Only attempt to fill `rightOrder` if a spread was taken and if not already completely filled + uint256 leftMakerAssetSpreadAmount = matchedFillResults.leftMakerAssetSpreadAmount; + if (leftMakerAssetSpreadAmount > 0 && matchedFillResults.right.takerAssetFilledAmount < rightOrder.takerAssetAmount) { + // The `assetData` fields of the `rightOrder` could have been null for the `matchOrders` call. We reassign them before calling `fillOrder`. + rightOrder.makerAssetData = leftOrder.takerAssetData; + rightOrder.takerAssetData = leftOrder.makerAssetData; + + // We do not need to pass in a signature since it was already validated in the `matchOrders` call + EXCHANGE.fillOrder( + rightOrder, + leftMakerAssetSpreadAmount, + "" + ); + } + } + */ + // solhint-disable-next-line payable-fallback + function () + external + { + assembly { + // The first 4 bytes of calldata holds the function selector + // `matchOrders` selector = 0x3c28d861 + if eq( + and(calldataload(0), 0xffffffff00000000000000000000000000000000000000000000000000000000), + 0x3c28d86100000000000000000000000000000000000000000000000000000000 + ) { + + // Load address of `owner` + let owner := sload(owner_slot) + + // Revert if `msg.sender` != `owner` + if iszero(eq(owner, caller)) { + // Revert with `Error("ONLY_CONTRACT_OWNER")` + mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) + mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) + mstore(64, 0x000000134f4e4c595f434f4e54524143545f4f574e4552000000000000000000) + mstore(96, 0) + revert(0, 100) + } + + // Load address of Exchange contract + let exchange := sload(EXCHANGE_slot) + + // Copy calldata to memory + // The calldata should be identical to the the ABIv2 encoded data required to call `EXCHANGE.matchOrders` + calldatacopy(0, 0, calldatasize()) + + // At this point, calldata and memory have the following layout: + + // | Offset | Length | Contents | + // |---------------------------|---------|-------------------------------------------- | + // | 0 | 4 | `matchOrders` function selector | + // | | 4 * 32 | function parameters: | + // | 4 | | 1. offset to leftOrder (*) | + // | 36 | | 2. offset to rightOrder (*) | + // | 68 | | 3. offset to leftSignature (*) | + // | 100 | | 4. offset to rightSignature (*) | + // | | 12 * 32 | leftOrder: | + // | 132 | | 1. senderAddress | + // | 164 | | 2. makerAddress | + // | 196 | | 3. takerAddress | + // | 228 | | 4. feeRecipientAddress | + // | 260 | | 5. makerAssetAmount | + // | 292 | | 6. takerAssetAmount | + // | 324 | | 7. makerFeeAmount | + // | 356 | | 8. takerFeeAmount | + // | 388 | | 9. expirationTimeSeconds | + // | 420 | | 10. salt | + // | 452 | | 11. offset to leftMakerAssetData (*) | + // | 484 | | 12. offset to leftTakerAssetData (*) | + // | | 12 * 32 | rightOrder: | + // | 516 | | 1. senderAddress | + // | 548 | | 2. makerAddress | + // | 580 | | 3. takerAddress | + // | 612 | | 4. feeRecipientAddress | + // | 644 | | 5. makerAssetAmount | + // | 676 | | 6. takerAssetAmount | + // | 708 | | 7. makerFeeAmount | + // | 740 | | 8. takerFeeAmount | + // | 772 | | 9. expirationTimeSeconds | + // | 804 | | 10. salt | + // | 836 | | 11. offset to rightMakerAssetData (*) | + // | 868 | | 12. offset to rightTakerAssetData (*) | + // | 900 | 32 | leftMakerAssetData Length | + // | 932 | a | leftMakerAssetData Contents | + // | 932 + a | 32 | leftTakerAssetData Length | + // | 964 + a | b | leftTakerAssetData Contents | + // | 964 + a + b | 32 | rightMakerAssetData Length | + // | 996 + a + b | c | rightMakerAssetData Contents | + // | 996 + a + b + c | 32 | rightTakerAssetData Length | + // | 1028 + a + b + c | d | rightTakerAssetData Contents | + // | 1028 + a + b + c + d | 32 | leftSignature Length | + // | 1060 + a + b + c + d | e | leftSignature Contents | + // | 1060 + a + b + c + d + e | 32 | rightSignature Length | + // | 1092 + a + b + c + d + e | f | rightSignature Contents | + + // Call `EXCHANGE.matchOrders` + let matchOrdersSuccess := call( + gas, // forward all gas + exchange, // call address of Exchange contract + 0, // transfer 0 wei + 0, // input starts at 0 + calldatasize(), // length of input + 0, // write output over output + 288 // length of output is 288 bytes + ) + + if iszero(matchOrdersSuccess) { + // Revert with reason if `matchOrders` call was unsuccessful + revert(0, returndatasize()) + } + + // After calling `matchOrders`, the relevant parts of memory are: + + // | Offset | Length | Contents | + // |---------------------------|---------|-------------------------------------------- | + // | | 9 * 32 | matchedFillResults | + // | 0 | | 1. left.makerAssetFilledAmount | + // | 32 | | 2. left.takerAssetFilledAmount | + // | 64 | | 3. left.makerFeePaid | + // | 96 | | 4. left.takerFeePaid | + // | 128 | | 5. right.makerAssetFilledAmount | + // | 160 | | 6. right.takerAssetFilledAmount | + // | 192 | | 7. right.makerFeePaid | + // | 224 | | 8. right.takerFeePaid | + // | 256 | | 9. leftMakerAssetSpreadAmount | + // | | 12 * 32 | rightOrder: | + // | 516 | | 1. senderAddress | + // | 548 | | 2. makerAddress | + // | 580 | | 3. takerAddress | + // | 612 | | 4. feeRecipientAddress | + // | 644 | | 5. makerAssetAmount | + // | 676 | | 6. takerAssetAmount | + // | 708 | | 7. makerFeeAmount | + // | 740 | | 8. takerFeeAmount | + // | 772 | | 9. expirationTimeSeconds | + // | 804 | | 10. salt | + // | 836 | | 11. offset to rightMakerAssetData (*) | + // | 868 | | 12. offset to rightTakerAssetData (*) | + + let rightOrderStart := add(calldataload(36), 4) + + // Only call `fillOrder` if a spread was taken and `rightOrder` has not been completely filled + if and( + gt(mload(256), 0), // gt(leftMakerAssetSpreadAmount, 0) + lt(mload(160), calldataload(add(rightOrderStart, 160))) // lt(rightOrderTakerAssetFilledAmount, rightOrderTakerAssetAmount) + ) { + + // We want the following layout in memory before calling `fillOrder`: + + // | Offset | Length | Contents | + // |---------------------------|---------|-------------------------------------------- | + // | 416 | 4 | `fillOrder` function selector | + // | | 3 * 32 | function parameters: | + // | 420 | | 1. offset to rightOrder (*) | + // | 452 | | 2. takerAssetFillAmount | + // | 484 | | 3. offset to rightSignature (*) | + // | | 12 * 32 | rightOrder: | + // | 516 | | 1. senderAddress | + // | 548 | | 2. makerAddress | + // | 580 | | 3. takerAddress | + // | 612 | | 4. feeRecipientAddress | + // | 644 | | 5. makerAssetAmount | + // | 676 | | 6. takerAssetAmount | + // | 708 | | 7. makerFeeAmount | + // | 740 | | 8. takerFeeAmount | + // | 772 | | 9. expirationTimeSeconds | + // | 804 | | 10. salt | + // | 836 | | 11. offset to rightMakerAssetData (*) | + // | 868 | | 12. offset to rightTakerAssetData (*) | + // | 900 | 32 | rightMakerAssetData Length | + // | 932 | a | rightMakerAssetData Contents | + // | 932 + a | 32 | rightTakerAssetData Length | + // | 964 | b | rightTakerAssetData Contents | + // | 964 + b | 32 | rightSigature Length (always 0) | + + // We assume that `leftOrder.makerAssetData == rightOrder.takerAssetData` and `leftOrder.takerAssetData == rightOrder.makerAssetData` + // `EXCHANGE.matchOrders` already makes this assumption, so it is likely + // that the `rightMakerAssetData` and `rightTakerAssetData` in calldata are empty + + let leftOrderStart := add(calldataload(4), 4) + + // Calculate locations of `leftMakerAssetData` and `leftTakerAssetData` in calldata + let leftMakerAssetDataStart := add( + leftOrderStart, + calldataload(add(leftOrderStart, 320)) // offset to `leftMakerAssetData` + ) + let leftTakerAssetDataStart := add( + leftOrderStart, + calldataload(add(leftOrderStart, 352)) // offset to `leftTakerAssetData` + ) + + // Load lengths of `leftMakerAssetData` and `leftTakerAssetData` + let leftMakerAssetDataLen := calldataload(leftMakerAssetDataStart) + let leftTakerAssetDataLen := calldataload(leftTakerAssetDataStart) + + // Write offset to `rightMakerAssetData` + mstore(add(rightOrderStart, 320), 384) + + // Write offset to `rightTakerAssetData` + let rightTakerAssetDataOffset := add(416, leftTakerAssetDataLen) + mstore(add(rightOrderStart, 352), rightTakerAssetDataOffset) + + // Copy `leftTakerAssetData` from calldata onto `rightMakerAssetData` in memory + calldatacopy( + add(rightOrderStart, 384), // `rightMakerAssetDataStart` + leftTakerAssetDataStart, + add(leftTakerAssetDataLen, 32) + ) + + // Copy `leftMakerAssetData` from calldata onto `rightTakerAssetData` in memory + calldatacopy( + add(rightOrderStart, rightTakerAssetDataOffset), // `rightTakerAssetDataStart` + leftMakerAssetDataStart, + add(leftMakerAssetDataLen, 32) + ) + + // Write length of signature (always 0 since signature was previously validated) + let rightSignatureStart := add( + add(rightOrderStart, rightTakerAssetDataOffset), // `rightTakerAssetDataStart` + add(leftMakerAssetDataLen, 32) + ) + mstore(rightSignatureStart, 0) + + let cdStart := sub(rightOrderStart, 100) + + // `fillOrder` selector = 0xb4be83d5 + mstore(cdStart, 0xb4be83d500000000000000000000000000000000000000000000000000000000) + + // Write offset to `rightOrder` + mstore(add(cdStart, 4), 96) + + // Write `takerAssetFillAmount` + mstore(add(cdStart, 36), mload(256)) + + // Write offset to `rightSignature` + mstore(add(cdStart, 68), sub(rightSignatureStart, add(cdStart, 4))) + + let fillOrderSuccess := call( + gas, // forward all gas + exchange, // call address of Exchange contract + 0, // transfer 0 wei + cdStart, // start of input + sub(add(rightSignatureStart, 32), cdStart), // length of input is end - start + 0, // write output over input + 128 // length of output is 128 bytes + ) + + if fillOrderSuccess { + return(0, 0) + } + + // Revert with reason if `fillOrder` call was unsuccessful + revert(0, returndatasize()) + } + + // Return if `matchOrders` call successful and `fillOrder` was not called + return(0, 0) + } + + // Revert if undefined function is called + revert(0, 0) + } + } +} diff --git a/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol b/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol new file mode 100644 index 000000000..b6f1566cf --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol @@ -0,0 +1,37 @@ +/* + + 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.4.24; + +import "../../utils/Ownable/Ownable.sol"; +import "./libs/LibConstants.sol"; +import "./MixinMatchOrders.sol"; +import "./MixinAssets.sol"; + + +// solhint-disable no-empty-blocks +contract OrderMatcher is + MixinMatchOrders, + MixinAssets +{ + constructor (address _exchange) + public + LibConstants(_exchange) + Ownable() + {} +} diff --git a/contracts/extensions/contracts/OrderMatcher/interfaces/IAssets.sol b/contracts/extensions/contracts/OrderMatcher/interfaces/IAssets.sol new file mode 100644 index 000000000..a0b3aa4c7 --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/interfaces/IAssets.sol @@ -0,0 +1,43 @@ +/* + + 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.4.24; + + +contract IAssets { + + /// @dev Withdraws assets from this contract. The contract requires a ZRX balance in order to + /// function optimally, and this function allows the ZRX to be withdrawn by owner. It may also be + /// used to withdraw assets that were accidentally sent to this contract. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to withdraw. + function withdrawAsset( + bytes assetData, + uint256 amount + ) + external; + + /// @dev Approves or disapproves an AssetProxy to spend asset. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to approve for respective proxy. + function approveAssetProxy( + bytes assetData, + uint256 amount + ) + external; +} diff --git a/contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol new file mode 100644 index 000000000..455259238 --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol @@ -0,0 +1,43 @@ +/* + + 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.4.24; +pragma experimental ABIEncoderV2; + +import "../../../protocol/Exchange/libs/LibOrder.sol"; + + +contract IMatchOrders { + + /// @dev Match two complementary orders that have a profitable spread. + /// Each order is filled at their respective price point. However, the calculations are + /// carried out as though the orders are both being filled at the right order's price point. + /// The profit made by the left order is then used to fill the right order as much as possible. + /// This results in a spread being taken in terms of both assets. The spread is held within this contract. + /// @param leftOrder First order to match. + /// @param rightOrder Second order to match. + /// @param leftSignature Proof that order was created by the left maker. + /// @param rightSignature Proof that order was created by the right maker. + function matchOrders( + LibOrder.Order memory leftOrder, + LibOrder.Order memory rightOrder, + bytes memory leftSignature, + bytes memory rightSignature + ) + public; +} diff --git a/contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol b/contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol new file mode 100644 index 000000000..ed4aee925 --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol @@ -0,0 +1,31 @@ +/* + + 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.4.24; + +import "../../../utils/Ownable/IOwnable.sol"; +import "./IMatchOrders.sol"; +import "./IAssets.sol"; + + +// solhint-disable no-empty-blocks +contract IOrderMatcher is + IOwnable, + IMatchOrders, + IAssets +{} diff --git a/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol b/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol new file mode 100644 index 000000000..a7c170b42 --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol @@ -0,0 +1,53 @@ +/* + + 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.4.24; + +import "../../../protocol/Exchange/interfaces/IExchange.sol"; + + +contract LibConstants { + + bytes4 constant internal ERC20_TRANSFER_SELECTOR = bytes4(keccak256("transfer(address,uint256)")); + bytes4 constant internal ERC20_DATA_ID = bytes4(keccak256("ERC20Token(address)")); + bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256)")); + + // solhint-disable var-name-mixedcase + IExchange internal EXCHANGE; + address internal ERC20_PROXY_ADDRESS; + address internal ERC721_PROXY_ADDRESS; + // solhint-enable var-name-mixedcase + + constructor (address _exchange) + public + { + EXCHANGE = IExchange(_exchange); + + ERC20_PROXY_ADDRESS = EXCHANGE.getAssetProxy(ERC20_DATA_ID); + require( + ERC20_PROXY_ADDRESS != address(0), + "UNREGISTERED_ASSET_PROXY" + ); + + ERC721_PROXY_ADDRESS = EXCHANGE.getAssetProxy(ERC721_DATA_ID); + require( + ERC721_PROXY_ADDRESS != address(0), + "UNREGISTERED_ASSET_PROXY" + ); + } +} diff --git a/contracts/extensions/contracts/OrderMatcher/mixins/MAssets.sol b/contracts/extensions/contracts/OrderMatcher/mixins/MAssets.sol new file mode 100644 index 000000000..32cfddf1c --- /dev/null +++ b/contracts/extensions/contracts/OrderMatcher/mixins/MAssets.sol @@ -0,0 +1,71 @@ +/* + + 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.4.24; + +import "../interfaces/IAssets.sol"; + + +contract MAssets is + IAssets +{ + /// @dev Transfers given amount of asset to sender. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to transfer to sender. + function transferAssetToSender( + bytes memory assetData, + uint256 amount + ) + internal; + + /// @dev Decodes ERC20 assetData and transfers given amount to sender. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to transfer to sender. + function transferERC20Token( + bytes memory assetData, + uint256 amount + ) + internal; + + /// @dev Decodes ERC721 assetData and transfers given amount to sender. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to transfer to sender. + function transferERC721Token( + bytes memory assetData, + uint256 amount + ) + internal; + + /// @dev Sets approval for ERC20 AssetProxy. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to approve for respective proxy. + function approveERC20Token( + bytes memory assetData, + uint256 amount + ) + internal; + + /// @dev Sets approval for ERC721 AssetProxy. + /// @param assetData Byte array encoded for the respective asset proxy. + /// @param amount Amount of asset to approve for respective proxy. + function approveERC721Token( + bytes memory assetData, + uint256 amount + ) + internal; +} diff --git a/contracts/protocol/src/artifacts/index.ts b/contracts/protocol/src/artifacts/index.ts index 1d53ceb04..2a9de78d2 100644 --- a/contracts/protocol/src/artifacts/index.ts +++ b/contracts/protocol/src/artifacts/index.ts @@ -6,6 +6,7 @@ import * as ERC721Proxy from '../../generated-artifacts/ERC721Proxy.json'; import * as Exchange from '../../generated-artifacts/Exchange.json'; import * as MixinAuthorizable from '../../generated-artifacts/MixinAuthorizable.json'; import * as MultiAssetProxy from '../../generated-artifacts/MultiAssetProxy.json'; +import * as OrderMatcher from '../../generated-artifacts/OrderMatcher.json'; import * as OrderValidator from '../../generated-artifacts/OrderValidator.json'; import * as TestAssetProxyDispatcher from '../../generated-artifacts/TestAssetProxyDispatcher.json'; import * as TestAssetProxyOwner from '../../generated-artifacts/TestAssetProxyOwner.json'; @@ -20,6 +21,7 @@ export const artifacts = { Exchange: Exchange as ContractArtifact, MixinAuthorizable: MixinAuthorizable as ContractArtifact, MultiAssetProxy: MultiAssetProxy as ContractArtifact, + OrderMatcher: OrderMatcher as ContractArtifact, OrderValidator: OrderValidator as ContractArtifact, TestAssetProxyDispatcher: TestAssetProxyDispatcher as ContractArtifact, TestAssetProxyOwner: TestAssetProxyOwner as ContractArtifact, -- cgit v1.2.3 From 2fa8b8d1d054015bfac49401c4f6eb5af9309603 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 17 Oct 2018 13:10:51 -0700 Subject: Add OrderMatcher tests --- contracts/extensions/test/extensions/forwarder.ts | 3 +- contracts/protocol/src/wrappers/index.ts | 1 + contracts/protocol/tsconfig.json | 1 + contracts/test-utils/src/constants.ts | 1 + .../contracts/test/extensions/order_matcher.ts | 770 +++++++++++++++++++++ 5 files changed, 774 insertions(+), 2 deletions(-) create mode 100644 packages/contracts/test/extensions/order_matcher.ts diff --git a/contracts/extensions/test/extensions/forwarder.ts b/contracts/extensions/test/extensions/forwarder.ts index 4027f493d..69939ed04 100644 --- a/contracts/extensions/test/extensions/forwarder.ts +++ b/contracts/extensions/test/extensions/forwarder.ts @@ -48,7 +48,6 @@ describe(ContractName.Forwarder, () => { let owner: string; let takerAddress: string; let feeRecipientAddress: string; - let otherAddress: string; let defaultMakerAssetAddress: string; let zrxAssetData: string; let wethAssetData: string; @@ -78,7 +77,7 @@ describe(ContractName.Forwarder, () => { before(async () => { await blockchainLifecycle.startAsync(); const accounts = await web3Wrapper.getAvailableAddressesAsync(); - const usedAddresses = ([owner, makerAddress, takerAddress, feeRecipientAddress, otherAddress] = accounts); + const usedAddresses = ([owner, makerAddress, takerAddress, feeRecipientAddress] = accounts); const txHash = await web3Wrapper.sendTransactionAsync({ from: accounts[0], to: accounts[0], value: 0 }); const transaction = await web3Wrapper.getTransactionByHashAsync(txHash); diff --git a/contracts/protocol/src/wrappers/index.ts b/contracts/protocol/src/wrappers/index.ts index ac951d269..13f16f44f 100644 --- a/contracts/protocol/src/wrappers/index.ts +++ b/contracts/protocol/src/wrappers/index.ts @@ -3,6 +3,7 @@ export * from '../../generated-wrappers/erc20_proxy'; export * from '../../generated-wrappers/erc721_proxy'; export * from '../../generated-wrappers/exchange'; export * from '../../generated-wrappers/mixin_authorizable'; +export * from '../../generated-wrappers/order_matcher'; export * from '../../generated-wrappers/order_validator'; export * from '../../generated-wrappers/test_asset_proxy_dispatcher'; export * from '../../generated-wrappers/test_asset_proxy_owner'; diff --git a/contracts/protocol/tsconfig.json b/contracts/protocol/tsconfig.json index 989d3ef2b..7e4ce5eb4 100644 --- a/contracts/protocol/tsconfig.json +++ b/contracts/protocol/tsconfig.json @@ -13,6 +13,7 @@ "./generated-artifacts/Exchange.json", "./generated-artifacts/MixinAuthorizable.json", "./generated-artifacts/MultiAssetProxy.json", + "./generated-artifacts/OrderMatcher.json", "./generated-artifacts/OrderValidator.json", "./generated-artifacts/TestAssetProxyDispatcher.json", "./generated-artifacts/TestAssetProxyOwner.json", diff --git a/contracts/test-utils/src/constants.ts b/contracts/test-utils/src/constants.ts index d2c3ab512..f631dc81a 100644 --- a/contracts/test-utils/src/constants.ts +++ b/contracts/test-utils/src/constants.ts @@ -29,6 +29,7 @@ export const constants = { MAX_TOKEN_TRANSFERFROM_GAS: 80000, MAX_TOKEN_APPROVE_GAS: 60000, MAX_TRANSFER_FROM_GAS: 150000, + MAX_MATCH_ORDERS_GAS: 400000, DUMMY_TOKEN_NAME: '', DUMMY_TOKEN_SYMBOL: '', DUMMY_TOKEN_DECIMALS: new BigNumber(18), diff --git a/packages/contracts/test/extensions/order_matcher.ts b/packages/contracts/test/extensions/order_matcher.ts new file mode 100644 index 000000000..a3958894b --- /dev/null +++ b/packages/contracts/test/extensions/order_matcher.ts @@ -0,0 +1,770 @@ +import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { assetDataUtils } from '@0xproject/order-utils'; +import { RevertReason } from '@0xproject/types'; +import { BigNumber } from '@0xproject/utils'; +import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import * as chai from 'chai'; +import { LogWithDecodedArgs } from 'ethereum-types'; +import * as _ from 'lodash'; + +import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; +import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; +import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; +import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; +import { ExchangeContract, ExchangeFillEventArgs } from '../../generated-wrappers/exchange'; +import { OrderMatcherContract } from '../../generated-wrappers/order_matcher'; +import { artifacts } from '../../src/artifacts'; +import { + expectContractCreationFailedAsync, + expectTransactionFailedAsync, + sendTransactionResult, +} from '../utils/assertions'; +import { chaiSetup } from '../utils/chai_setup'; +import { constants } from '../utils/constants'; +import { ERC20Wrapper } from '../utils/erc20_wrapper'; +import { ExchangeWrapper } from '../utils/exchange_wrapper'; +import { LogDecoder } from '../utils/log_decoder'; +import { OrderFactory } from '../utils/order_factory'; +import { ERC20BalancesByOwner } from '../utils/types'; +import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; + +const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +chaiSetup.configure(); +const expect = chai.expect; +// tslint:disable:no-unnecessary-type-assertion +describe('OrderMatcher', () => { + let makerAddressLeft: string; + let makerAddressRight: string; + let owner: string; + let takerAddress: string; + let feeRecipientAddressLeft: string; + let feeRecipientAddressRight: string; + + let erc20TokenA: DummyERC20TokenContract; + let erc20TokenB: DummyERC20TokenContract; + let zrxToken: DummyERC20TokenContract; + let exchange: ExchangeContract; + let erc20Proxy: ERC20ProxyContract; + let erc721Proxy: ERC721ProxyContract; + let orderMatcher: OrderMatcherContract; + + let erc20BalancesByOwner: ERC20BalancesByOwner; + let exchangeWrapper: ExchangeWrapper; + let erc20Wrapper: ERC20Wrapper; + let orderFactoryLeft: OrderFactory; + let orderFactoryRight: OrderFactory; + + let leftMakerAssetData: string; + let leftTakerAssetData: string; + let defaultERC20MakerAssetAddress: string; + let defaultERC20TakerAssetAddress: string; + + before(async () => { + await blockchainLifecycle.startAsync(); + }); + after(async () => { + await blockchainLifecycle.revertAsync(); + }); + before(async () => { + // Create accounts + const accounts = await web3Wrapper.getAvailableAddressesAsync(); + // Hack(albrow): Both Prettier and TSLint insert a trailing comma below + // but that is invalid syntax as of TypeScript version >= 2.8. We don't + // have the right fine-grained configuration options in TSLint, + // Prettier, or TypeScript, to reconcile this, so we will just have to + // wait for them to sort it out. We disable TSLint and Prettier for + // this part of the code for now. This occurs several times in this + // file. See https://github.com/prettier/prettier/issues/4624. + // prettier-ignore + const usedAddresses = ([ + owner, + makerAddressLeft, + makerAddressRight, + takerAddress, + feeRecipientAddressLeft, + // tslint:disable-next-line:trailing-comma + feeRecipientAddressRight + ] = _.slice(accounts, 0, 6)); + // Create wrappers + erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); + // Deploy ERC20 token & ERC20 proxy + const numDummyErc20ToDeploy = 3; + [erc20TokenA, erc20TokenB, zrxToken] = await erc20Wrapper.deployDummyTokensAsync( + numDummyErc20ToDeploy, + constants.DUMMY_TOKEN_DECIMALS, + ); + erc20Proxy = await erc20Wrapper.deployProxyAsync(); + await erc20Wrapper.setBalancesAndAllowancesAsync(); + // Deploy ERC721 proxy + erc721Proxy = await ERC721ProxyContract.deployFrom0xArtifactAsync(artifacts.ERC721Proxy, provider, txDefaults); + // Depoy exchange + exchange = await ExchangeContract.deployFrom0xArtifactAsync( + artifacts.Exchange, + provider, + txDefaults, + assetDataUtils.encodeERC20AssetData(zrxToken.address), + ); + exchangeWrapper = new ExchangeWrapper(exchange, provider); + await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); + await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); + // Authorize ERC20 trades by exchange + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Deploy OrderMatcher + orderMatcher = await OrderMatcherContract.deployFrom0xArtifactAsync( + artifacts.OrderMatcher, + provider, + txDefaults, + exchange.address, + ); + // Set default addresses + defaultERC20MakerAssetAddress = erc20TokenA.address; + defaultERC20TakerAssetAddress = erc20TokenB.address; + leftMakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress); + leftTakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress); + // Set OrderMatcher balances and allowances + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenA.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenB.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync( + leftMakerAssetData, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync( + leftTakerAssetData, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Create default order parameters + const defaultOrderParamsLeft = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressLeft, + exchangeAddress: exchange.address, + makerAssetData: leftMakerAssetData, + takerAssetData: leftTakerAssetData, + feeRecipientAddress: feeRecipientAddressLeft, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + }; + const defaultOrderParamsRight = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressRight, + exchangeAddress: exchange.address, + makerAssetData: leftTakerAssetData, + takerAssetData: leftMakerAssetData, + feeRecipientAddress: feeRecipientAddressRight, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + }; + const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)]; + orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft); + const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; + orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight); + }); + beforeEach(async () => { + await blockchainLifecycle.startAsync(); + }); + afterEach(async () => { + await blockchainLifecycle.revertAsync(); + }); + describe('constructor', () => { + it('should revert if assetProxy is unregistered', async () => { + const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( + artifacts.Exchange, + provider, + txDefaults, + constants.NULL_BYTES, + ); + return expectContractCreationFailedAsync( + (OrderMatcherContract.deployFrom0xArtifactAsync( + artifacts.OrderMatcher, + provider, + txDefaults, + exchangeInstance.address, + ) as any) as sendTransactionResult, + RevertReason.UnregisteredAssetProxy, + ); + }); + }); + describe('matchOrders', () => { + beforeEach(async () => { + erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); + }); + it('should revert if not claled by owner', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: takerAddress, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.OnlyContractOwner, + ); + }); + it('should transfer the correct amounts when orders completely fill each other', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + }); + it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal(initialLeftMakerAssetTakerBalance); + }); + it('should transfer the correct amounts when left order is completely filled and right order would be partially filled', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it('should not call fillOrder when rightOrder is completely filled after matchOrders call', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + const logDecoder = new LogDecoder(web3Wrapper); + const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + ); + const fillLogs = _.filter( + txReceipt.logs, + log => (log as LogWithDecodedArgs).event === 'Fill', + ); + expect(fillLogs.length).to.be.equal(2); + }); + it('should only take a spread in rightMakerAsset if entire leftMakerAssetSpread amount can be used to fill rightOrder after matchOrders call', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.9), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(990), 18), + }); + const initialLeftMakerAssetSpreadAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1.09), 18); + const leftTakerAssetSpreadAmount = initialLeftMakerAssetSpreadAmount + .times(signedOrderRight.makerAssetAmount) + .dividedToIntegerBy(signedOrderRight.takerAssetAmount); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderLeft.takerAssetAmount.plus(leftTakerAssetSpreadAmount), + amountBoughtByRightMaker: signedOrderLeft.makerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: constants.ZERO_AMOUNT, + leftTakerAssetSpreadAmount, + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it("should succeed if rightOrder's makerAssetData and takerAssetData are not provided", async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + signedOrderRight.makerAssetData = constants.NULL_BYTES; + signedOrderRight.takerAssetData = constants.NULL_BYTES; + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it('should revert with the correct reason if matchOrders call reverts', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + signedOrderRight.signature = `0xff${signedOrderRight.signature.slice(4)}`; + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.InvalidOrderSignature, + ); + }); + it('should revert with the correct reason if fillOrder call reverts', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Matcher will not have enough allowance to fill rightOrder + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, constants.ZERO_AMOUNT, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.TransferFailed, + ); + }); + }); + describe('withdrawAsset', () => { + it('should allow owner to withdraw ERC20 tokens', async () => { + const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { + from: owner, + }), + ); + const newBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(newBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should allow owner to withdraw ERC721 tokens', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const tokenId = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await erc721Token.mint.sendTransactionAsync(orderMatcher.address, tokenId, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, tokenId); + const withdrawAmount = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.withdrawAsset.sendTransactionAsync(assetData, withdrawAmount, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const erc721Owner = await erc721Token.ownerOf.callAsync(tokenId); + expect(erc721Owner).to.be.equal(owner); + }); + it('should revert if not called by owner', async () => { + const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); + await expectTransactionFailedAsync( + orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { + from: takerAddress, + }), + RevertReason.OnlyContractOwner, + ); + }); + }); + describe('approveAssetProxy', () => { + it('should be able to set an allowance for ERC20 tokens', async () => { + const allowance = new BigNumber(55465465426546); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, allowance, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newAllowance = await erc20TokenA.allowance.callAsync(orderMatcher.address, erc20Proxy.address); + expect(newAllowance).to.be.bignumber.equal(allowance); + }); + it('should be able to approve an ERC721 token by passing in allowance = 1', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); + const allowance = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); + expect(isApproved).to.be.equal(true); + }); + it('should be able to approve an ERC721 token by passing in allowance > 1', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); + const allowance = new BigNumber(2); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); + expect(isApproved).to.be.equal(true); + }); + it('should revert if not called by owner', async () => { + const approval = new BigNumber(1); + await expectTransactionFailedAsync( + orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, approval, { + from: takerAddress, + }), + RevertReason.OnlyContractOwner, + ); + }); + }); +}); +// tslint:disable:max-file-line-count +// tslint:enable:no-unnecessary-type-assertion -- cgit v1.2.3 From a6ec2a8c549b9493c3ebd96bed58bb8016e5f2be Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 13 Nov 2018 12:49:54 -0800 Subject: Update dependency paths --- packages/contracts/test/extensions/order_matcher.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/contracts/test/extensions/order_matcher.ts b/packages/contracts/test/extensions/order_matcher.ts index a3958894b..8e1f633c2 100644 --- a/packages/contracts/test/extensions/order_matcher.ts +++ b/packages/contracts/test/extensions/order_matcher.ts @@ -1,8 +1,8 @@ -import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils } from '@0xproject/order-utils'; -import { RevertReason } from '@0xproject/types'; -import { BigNumber } from '@0xproject/utils'; -import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils } from '@0x/order-utils'; +import { RevertReason } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import * as _ from 'lodash'; @@ -208,7 +208,7 @@ describe('OrderMatcher', () => { beforeEach(async () => { erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); }); - it('should revert if not claled by owner', async () => { + it('should revert if not called by owner', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), -- cgit v1.2.3 From b6f4c5c7da7d0afe65c0e3dba97aa64e11934595 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 9 Dec 2018 15:03:45 -0800 Subject: Fix build and add back tests --- contracts/core/test/extensions/order_matcher.ts | 772 +++++++++++++++++++++ .../contracts/OrderMatcher/MixinAssets.sol | 4 +- .../contracts/OrderMatcher/MixinMatchOrders.sol | 2 +- .../contracts/OrderMatcher/OrderMatcher.sol | 2 +- 4 files changed, 776 insertions(+), 4 deletions(-) create mode 100644 contracts/core/test/extensions/order_matcher.ts diff --git a/contracts/core/test/extensions/order_matcher.ts b/contracts/core/test/extensions/order_matcher.ts new file mode 100644 index 000000000..e61d0f8e0 --- /dev/null +++ b/contracts/core/test/extensions/order_matcher.ts @@ -0,0 +1,772 @@ +import { + chaiSetup, + constants, + ERC20BalancesByOwner, + expectContractCreationFailedAsync, + expectTransactionFailedAsync, + LogDecoder, + OrderFactory, + provider, + sendTransactionResult, + txDefaults, + web3Wrapper, +} from '@0x/contracts-test-utils'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils } from '@0x/order-utils'; +import { RevertReason } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import { Web3Wrapper } from '@0x/web3-wrapper'; +import * as chai from 'chai'; +import { LogWithDecodedArgs } from 'ethereum-types'; +import * as _ from 'lodash'; + +import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; +import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; +import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; +import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; +import { ExchangeContract, ExchangeFillEventArgs } from '../../generated-wrappers/exchange'; +import { OrderMatcherContract } from '../../generated-wrappers/order_matcher'; +import { artifacts } from '../../src/artifacts'; +import { ERC20Wrapper } from '../utils/erc20_wrapper'; +import { ExchangeWrapper } from '../utils/exchange_wrapper'; + +const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +chaiSetup.configure(); +const expect = chai.expect; +// tslint:disable:no-unnecessary-type-assertion +describe('OrderMatcher', () => { + let makerAddressLeft: string; + let makerAddressRight: string; + let owner: string; + let takerAddress: string; + let feeRecipientAddressLeft: string; + let feeRecipientAddressRight: string; + + let erc20TokenA: DummyERC20TokenContract; + let erc20TokenB: DummyERC20TokenContract; + let zrxToken: DummyERC20TokenContract; + let exchange: ExchangeContract; + let erc20Proxy: ERC20ProxyContract; + let erc721Proxy: ERC721ProxyContract; + let orderMatcher: OrderMatcherContract; + + let erc20BalancesByOwner: ERC20BalancesByOwner; + let exchangeWrapper: ExchangeWrapper; + let erc20Wrapper: ERC20Wrapper; + let orderFactoryLeft: OrderFactory; + let orderFactoryRight: OrderFactory; + + let leftMakerAssetData: string; + let leftTakerAssetData: string; + let defaultERC20MakerAssetAddress: string; + let defaultERC20TakerAssetAddress: string; + + before(async () => { + await blockchainLifecycle.startAsync(); + }); + after(async () => { + await blockchainLifecycle.revertAsync(); + }); + before(async () => { + // Create accounts + const accounts = await web3Wrapper.getAvailableAddressesAsync(); + // Hack(albrow): Both Prettier and TSLint insert a trailing comma below + // but that is invalid syntax as of TypeScript version >= 2.8. We don't + // have the right fine-grained configuration options in TSLint, + // Prettier, or TypeScript, to reconcile this, so we will just have to + // wait for them to sort it out. We disable TSLint and Prettier for + // this part of the code for now. This occurs several times in this + // file. See https://github.com/prettier/prettier/issues/4624. + // prettier-ignore + const usedAddresses = ([ + owner, + makerAddressLeft, + makerAddressRight, + takerAddress, + feeRecipientAddressLeft, + // tslint:disable-next-line:trailing-comma + feeRecipientAddressRight + ] = _.slice(accounts, 0, 6)); + // Create wrappers + erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); + // Deploy ERC20 token & ERC20 proxy + const numDummyErc20ToDeploy = 3; + [erc20TokenA, erc20TokenB, zrxToken] = await erc20Wrapper.deployDummyTokensAsync( + numDummyErc20ToDeploy, + constants.DUMMY_TOKEN_DECIMALS, + ); + erc20Proxy = await erc20Wrapper.deployProxyAsync(); + await erc20Wrapper.setBalancesAndAllowancesAsync(); + // Deploy ERC721 proxy + erc721Proxy = await ERC721ProxyContract.deployFrom0xArtifactAsync(artifacts.ERC721Proxy, provider, txDefaults); + // Depoy exchange + exchange = await ExchangeContract.deployFrom0xArtifactAsync( + artifacts.Exchange, + provider, + txDefaults, + assetDataUtils.encodeERC20AssetData(zrxToken.address), + ); + exchangeWrapper = new ExchangeWrapper(exchange, provider); + await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); + await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); + // Authorize ERC20 trades by exchange + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Deploy OrderMatcher + orderMatcher = await OrderMatcherContract.deployFrom0xArtifactAsync( + artifacts.OrderMatcher, + provider, + txDefaults, + exchange.address, + ); + // Set default addresses + defaultERC20MakerAssetAddress = erc20TokenA.address; + defaultERC20TakerAssetAddress = erc20TokenB.address; + leftMakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress); + leftTakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress); + // Set OrderMatcher balances and allowances + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenA.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenB.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync( + leftMakerAssetData, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync( + leftTakerAssetData, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Create default order parameters + const defaultOrderParamsLeft = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressLeft, + exchangeAddress: exchange.address, + makerAssetData: leftMakerAssetData, + takerAssetData: leftTakerAssetData, + feeRecipientAddress: feeRecipientAddressLeft, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + }; + const defaultOrderParamsRight = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressRight, + exchangeAddress: exchange.address, + makerAssetData: leftTakerAssetData, + takerAssetData: leftMakerAssetData, + feeRecipientAddress: feeRecipientAddressRight, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + }; + const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)]; + orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft); + const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; + orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight); + }); + beforeEach(async () => { + await blockchainLifecycle.startAsync(); + }); + afterEach(async () => { + await blockchainLifecycle.revertAsync(); + }); + describe('constructor', () => { + it('should revert if assetProxy is unregistered', async () => { + const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( + artifacts.Exchange, + provider, + txDefaults, + constants.NULL_BYTES, + ); + return expectContractCreationFailedAsync( + (OrderMatcherContract.deployFrom0xArtifactAsync( + artifacts.OrderMatcher, + provider, + txDefaults, + exchangeInstance.address, + ) as any) as sendTransactionResult, + RevertReason.UnregisteredAssetProxy, + ); + }); + }); + describe('matchOrders', () => { + beforeEach(async () => { + erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); + }); + it('should revert if not called by owner', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: takerAddress, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.OnlyContractOwner, + ); + }); + it('should transfer the correct amounts when orders completely fill each other', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + }); + it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal(initialLeftMakerAssetTakerBalance); + }); + it('should transfer the correct amounts when left order is completely filled and right order would be partially filled', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it('should not call fillOrder when rightOrder is completely filled after matchOrders call', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + const logDecoder = new LogDecoder(web3Wrapper, artifacts); + const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + ); + const fillLogs = _.filter( + txReceipt.logs, + log => (log as LogWithDecodedArgs).event === 'Fill', + ); + expect(fillLogs.length).to.be.equal(2); + }); + it('should only take a spread in rightMakerAsset if entire leftMakerAssetSpread amount can be used to fill rightOrder after matchOrders call', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.9), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(990), 18), + }); + const initialLeftMakerAssetSpreadAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1.09), 18); + const leftTakerAssetSpreadAmount = initialLeftMakerAssetSpreadAmount + .times(signedOrderRight.makerAssetAmount) + .dividedToIntegerBy(signedOrderRight.takerAssetAmount); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderLeft.takerAssetAmount.plus(leftTakerAssetSpreadAmount), + amountBoughtByRightMaker: signedOrderLeft.makerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: constants.ZERO_AMOUNT, + leftTakerAssetSpreadAmount, + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it("should succeed if rightOrder's makerAssetData and takerAssetData are not provided", async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + signedOrderRight.makerAssetData = constants.NULL_BYTES; + signedOrderRight.takerAssetData = constants.NULL_BYTES; + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it('should revert with the correct reason if matchOrders call reverts', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + signedOrderRight.signature = `0xff${signedOrderRight.signature.slice(4)}`; + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.InvalidOrderSignature, + ); + }); + it('should revert with the correct reason if fillOrder call reverts', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Matcher will not have enough allowance to fill rightOrder + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, constants.ZERO_AMOUNT, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.TransferFailed, + ); + }); + }); + describe('withdrawAsset', () => { + it('should allow owner to withdraw ERC20 tokens', async () => { + const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { + from: owner, + }), + ); + const newBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(newBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should allow owner to withdraw ERC721 tokens', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const tokenId = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await erc721Token.mint.sendTransactionAsync(orderMatcher.address, tokenId, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, tokenId); + const withdrawAmount = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.withdrawAsset.sendTransactionAsync(assetData, withdrawAmount, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const erc721Owner = await erc721Token.ownerOf.callAsync(tokenId); + expect(erc721Owner).to.be.equal(owner); + }); + it('should revert if not called by owner', async () => { + const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); + await expectTransactionFailedAsync( + orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { + from: takerAddress, + }), + RevertReason.OnlyContractOwner, + ); + }); + }); + describe('approveAssetProxy', () => { + it('should be able to set an allowance for ERC20 tokens', async () => { + const allowance = new BigNumber(55465465426546); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, allowance, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newAllowance = await erc20TokenA.allowance.callAsync(orderMatcher.address, erc20Proxy.address); + expect(newAllowance).to.be.bignumber.equal(allowance); + }); + it('should be able to approve an ERC721 token by passing in allowance = 1', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); + const allowance = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); + expect(isApproved).to.be.equal(true); + }); + it('should be able to approve an ERC721 token by passing in allowance > 1', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + artifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); + const allowance = new BigNumber(2); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); + expect(isApproved).to.be.equal(true); + }); + it('should revert if not called by owner', async () => { + const approval = new BigNumber(1); + await expectTransactionFailedAsync( + orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, approval, { + from: takerAddress, + }), + RevertReason.OnlyContractOwner, + ); + }); + }); +}); +// tslint:disable:max-file-line-count +// tslint:enable:no-unnecessary-type-assertion diff --git a/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol b/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol index a2bc95e4b..43e19c19e 100644 --- a/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol +++ b/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol @@ -18,8 +18,8 @@ pragma solidity 0.4.24; -import "../../utils/LibBytes/LibBytes.sol"; -import "../../utils/Ownable/Ownable.sol"; +import "@0x/contracts-utils/contracts/utils/LibBytes/LibBytes.sol"; +import "@0x/contracts-utils/contracts/utils/Ownable/Ownable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; import "../../tokens/ERC721Token/IERC721Token.sol"; import "./mixins/MAssets.sol"; diff --git a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol index 1c7cce675..ffe037ee7 100644 --- a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol +++ b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol @@ -18,8 +18,8 @@ pragma solidity 0.4.24; -import "../../utils/Ownable/Ownable.sol"; import "./libs/LibConstants.sol"; +import "@0x/contracts-utils/contracts/utils/Ownable/Ownable.sol"; contract MixinMatchOrders is diff --git a/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol b/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol index b6f1566cf..7136c8c82 100644 --- a/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol +++ b/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol @@ -18,7 +18,7 @@ pragma solidity 0.4.24; -import "../../utils/Ownable/Ownable.sol"; +import "@0x/contracts-utils/contracts/utils/Ownable/Ownable.sol"; import "./libs/LibConstants.sol"; import "./MixinMatchOrders.sol"; import "./MixinAssets.sol"; -- cgit v1.2.3 From 0a5ecec3e222ae0c5e70e0d3092e57df5dfb75cd Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 12 Dec 2018 16:44:28 -0800 Subject: update comments --- contracts/core/test/extensions/order_matcher.ts | 12 ++++++------ contracts/extensions/contracts/OrderMatcher/MixinAssets.sol | 7 +++++-- .../extensions/contracts/OrderMatcher/MixinMatchOrders.sol | 5 +++-- .../contracts/OrderMatcher/interfaces/IMatchOrders.sol | 2 +- .../contracts/OrderMatcher/interfaces/IOrderMatcher.sol | 2 +- .../extensions/contracts/OrderMatcher/libs/LibConstants.sol | 2 +- 6 files changed, 17 insertions(+), 13 deletions(-) diff --git a/contracts/core/test/extensions/order_matcher.ts b/contracts/core/test/extensions/order_matcher.ts index e61d0f8e0..4ea95bc49 100644 --- a/contracts/core/test/extensions/order_matcher.ts +++ b/contracts/core/test/extensions/order_matcher.ts @@ -11,6 +11,7 @@ import { txDefaults, web3Wrapper, } from '@0x/contracts-test-utils'; +import { artifacts as tokenArtifacts, DummyERC20TokenContract, DummyERC721TokenContract } from '@0x/contracts-tokens'; import { BlockchainLifecycle } from '@0x/dev-utils'; import { assetDataUtils } from '@0x/order-utils'; import { RevertReason } from '@0x/types'; @@ -20,8 +21,6 @@ import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import * as _ from 'lodash'; -import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; -import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; import { ExchangeContract, ExchangeFillEventArgs } from '../../generated-wrappers/exchange'; @@ -444,7 +443,7 @@ describe('OrderMatcher', () => { signedOrderLeft.signature, signedOrderRight.signature, ); - const logDecoder = new LogDecoder(web3Wrapper, artifacts); + const logDecoder = new LogDecoder(web3Wrapper, { ...artifacts, ...tokenArtifacts }); const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( await web3Wrapper.sendTransactionAsync({ data, @@ -457,6 +456,7 @@ describe('OrderMatcher', () => { txReceipt.logs, log => (log as LogWithDecodedArgs).event === 'Fill', ); + // Only 2 Fill logs should exist for `matchOrders` call. `fillOrder` should not have been called and should not have emitted a Fill event. expect(fillLogs.length).to.be.equal(2); }); it('should only take a spread in rightMakerAsset if entire leftMakerAssetSpread amount can be used to fill rightOrder after matchOrders call', async () => { @@ -680,7 +680,7 @@ describe('OrderMatcher', () => { }); it('should allow owner to withdraw ERC721 tokens', async () => { const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - artifacts.DummyERC721Token, + tokenArtifacts.DummyERC721Token, provider, txDefaults, constants.DUMMY_TOKEN_NAME, @@ -725,7 +725,7 @@ describe('OrderMatcher', () => { }); it('should be able to approve an ERC721 token by passing in allowance = 1', async () => { const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - artifacts.DummyERC721Token, + tokenArtifacts.DummyERC721Token, provider, txDefaults, constants.DUMMY_TOKEN_NAME, @@ -742,7 +742,7 @@ describe('OrderMatcher', () => { }); it('should be able to approve an ERC721 token by passing in allowance > 1', async () => { const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - artifacts.DummyERC721Token, + tokenArtifacts.DummyERC721Token, provider, txDefaults, constants.DUMMY_TOKEN_NAME, diff --git a/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol b/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol index 43e19c19e..323998705 100644 --- a/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol +++ b/contracts/extensions/contracts/OrderMatcher/MixinAssets.sol @@ -20,8 +20,8 @@ pragma solidity 0.4.24; import "@0x/contracts-utils/contracts/utils/LibBytes/LibBytes.sol"; import "@0x/contracts-utils/contracts/utils/Ownable/Ownable.sol"; -import "../../tokens/ERC20Token/IERC20Token.sol"; -import "../../tokens/ERC721Token/IERC721Token.sol"; +import "@0x/contracts-tokens/contracts/tokens/ERC20Token/IERC20Token.sol"; +import "@0x/contracts-tokens/contracts/tokens/ERC721Token/IERC721Token.sol"; import "./mixins/MAssets.sol"; import "./libs/LibConstants.sol"; @@ -98,6 +98,7 @@ contract MixinAssets is ) internal { + // 4 byte id + 12 0 bytes before ABI encoded token address. address token = assetData.readAddress(16); // Transfer tokens. @@ -149,7 +150,9 @@ contract MixinAssets is "INVALID_AMOUNT" ); // Decode asset data. + // 4 byte id + 12 0 bytes before ABI encoded token address. address token = assetData.readAddress(16); + // 4 byte id + 32 byte ABI encoded token address before token id. uint256 tokenId = assetData.readUint256(36); // Perform transfer. diff --git a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol index ffe037ee7..866523190 100644 --- a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol +++ b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol @@ -230,7 +230,7 @@ contract MixinMatchOrders is // | 932 | a | rightMakerAssetData Contents | // | 932 + a | 32 | rightTakerAssetData Length | // | 964 | b | rightTakerAssetData Contents | - // | 964 + b | 32 | rightSigature Length (always 0) | + // | 964 + b | 32 | rightSignature Length (always 0) | // We assume that `leftOrder.makerAssetData == rightOrder.takerAssetData` and `leftOrder.takerAssetData == rightOrder.makerAssetData` // `EXCHANGE.matchOrders` already makes this assumption, so it is likely @@ -280,6 +280,7 @@ contract MixinMatchOrders is ) mstore(rightSignatureStart, 0) + // function selector (4 bytes) + 3 params (3 * 32 bytes) must be stored before `rightOrderStart` let cdStart := sub(rightOrderStart, 100) // `fillOrder` selector = 0xb4be83d5 @@ -288,7 +289,7 @@ contract MixinMatchOrders is // Write offset to `rightOrder` mstore(add(cdStart, 4), 96) - // Write `takerAssetFillAmount` + // Write `takerAssetFillAmount`, which will be the `leftMakerAssetSpreadAmount` received from the `matchOrders` call mstore(add(cdStart, 36), mload(256)) // Write offset to `rightSignature` diff --git a/contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol index 455259238..19bcbb326 100644 --- a/contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol +++ b/contracts/extensions/contracts/OrderMatcher/interfaces/IMatchOrders.sol @@ -19,7 +19,7 @@ pragma solidity 0.4.24; pragma experimental ABIEncoderV2; -import "../../../protocol/Exchange/libs/LibOrder.sol"; +import "@0x/contracts-libs/contracts/libs/LibOrder.sol"; contract IMatchOrders { diff --git a/contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol b/contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol index ed4aee925..9b6ea26d8 100644 --- a/contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol +++ b/contracts/extensions/contracts/OrderMatcher/interfaces/IOrderMatcher.sol @@ -18,7 +18,7 @@ pragma solidity 0.4.24; -import "../../../utils/Ownable/IOwnable.sol"; +import "@0x/contract-utils/contracts/utils/Ownable/IOwnable.sol"; import "./IMatchOrders.sol"; import "./IAssets.sol"; diff --git a/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol b/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol index a7c170b42..43bbdeec2 100644 --- a/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol +++ b/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol @@ -18,7 +18,7 @@ pragma solidity 0.4.24; -import "../../../protocol/Exchange/interfaces/IExchange.sol"; +import "@0x/contracts-interfaces/contracts/protocol/Exchange/IExchange.sol"; contract LibConstants { -- cgit v1.2.3 From 6b5b8fe8e05e497b6f4db4d3b0aa675d0bbdff50 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 12 Dec 2018 17:26:46 -0800 Subject: Fix build after rebase --- contracts/core/test/extensions/order_matcher.ts | 772 -------------------- contracts/extensions/package.json | 2 +- contracts/extensions/src/artifacts/index.ts | 2 + contracts/extensions/src/wrappers/index.ts | 1 + .../extensions/test/extensions/order_matcher.ts | 780 +++++++++++++++++++++ contracts/extensions/tsconfig.json | 3 +- contracts/protocol/src/artifacts/index.ts | 2 - contracts/protocol/src/wrappers/index.ts | 1 - contracts/protocol/tsconfig.json | 1 - .../contracts/test/extensions/order_matcher.ts | 770 -------------------- 10 files changed, 786 insertions(+), 1548 deletions(-) delete mode 100644 contracts/core/test/extensions/order_matcher.ts create mode 100644 contracts/extensions/test/extensions/order_matcher.ts delete mode 100644 packages/contracts/test/extensions/order_matcher.ts diff --git a/contracts/core/test/extensions/order_matcher.ts b/contracts/core/test/extensions/order_matcher.ts deleted file mode 100644 index 4ea95bc49..000000000 --- a/contracts/core/test/extensions/order_matcher.ts +++ /dev/null @@ -1,772 +0,0 @@ -import { - chaiSetup, - constants, - ERC20BalancesByOwner, - expectContractCreationFailedAsync, - expectTransactionFailedAsync, - LogDecoder, - OrderFactory, - provider, - sendTransactionResult, - txDefaults, - web3Wrapper, -} from '@0x/contracts-test-utils'; -import { artifacts as tokenArtifacts, DummyERC20TokenContract, DummyERC721TokenContract } from '@0x/contracts-tokens'; -import { BlockchainLifecycle } from '@0x/dev-utils'; -import { assetDataUtils } from '@0x/order-utils'; -import { RevertReason } from '@0x/types'; -import { BigNumber } from '@0x/utils'; -import { Web3Wrapper } from '@0x/web3-wrapper'; -import * as chai from 'chai'; -import { LogWithDecodedArgs } from 'ethereum-types'; -import * as _ from 'lodash'; - -import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; -import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; -import { ExchangeContract, ExchangeFillEventArgs } from '../../generated-wrappers/exchange'; -import { OrderMatcherContract } from '../../generated-wrappers/order_matcher'; -import { artifacts } from '../../src/artifacts'; -import { ERC20Wrapper } from '../utils/erc20_wrapper'; -import { ExchangeWrapper } from '../utils/exchange_wrapper'; - -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -chaiSetup.configure(); -const expect = chai.expect; -// tslint:disable:no-unnecessary-type-assertion -describe('OrderMatcher', () => { - let makerAddressLeft: string; - let makerAddressRight: string; - let owner: string; - let takerAddress: string; - let feeRecipientAddressLeft: string; - let feeRecipientAddressRight: string; - - let erc20TokenA: DummyERC20TokenContract; - let erc20TokenB: DummyERC20TokenContract; - let zrxToken: DummyERC20TokenContract; - let exchange: ExchangeContract; - let erc20Proxy: ERC20ProxyContract; - let erc721Proxy: ERC721ProxyContract; - let orderMatcher: OrderMatcherContract; - - let erc20BalancesByOwner: ERC20BalancesByOwner; - let exchangeWrapper: ExchangeWrapper; - let erc20Wrapper: ERC20Wrapper; - let orderFactoryLeft: OrderFactory; - let orderFactoryRight: OrderFactory; - - let leftMakerAssetData: string; - let leftTakerAssetData: string; - let defaultERC20MakerAssetAddress: string; - let defaultERC20TakerAssetAddress: string; - - before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - // Create accounts - const accounts = await web3Wrapper.getAvailableAddressesAsync(); - // Hack(albrow): Both Prettier and TSLint insert a trailing comma below - // but that is invalid syntax as of TypeScript version >= 2.8. We don't - // have the right fine-grained configuration options in TSLint, - // Prettier, or TypeScript, to reconcile this, so we will just have to - // wait for them to sort it out. We disable TSLint and Prettier for - // this part of the code for now. This occurs several times in this - // file. See https://github.com/prettier/prettier/issues/4624. - // prettier-ignore - const usedAddresses = ([ - owner, - makerAddressLeft, - makerAddressRight, - takerAddress, - feeRecipientAddressLeft, - // tslint:disable-next-line:trailing-comma - feeRecipientAddressRight - ] = _.slice(accounts, 0, 6)); - // Create wrappers - erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); - // Deploy ERC20 token & ERC20 proxy - const numDummyErc20ToDeploy = 3; - [erc20TokenA, erc20TokenB, zrxToken] = await erc20Wrapper.deployDummyTokensAsync( - numDummyErc20ToDeploy, - constants.DUMMY_TOKEN_DECIMALS, - ); - erc20Proxy = await erc20Wrapper.deployProxyAsync(); - await erc20Wrapper.setBalancesAndAllowancesAsync(); - // Deploy ERC721 proxy - erc721Proxy = await ERC721ProxyContract.deployFrom0xArtifactAsync(artifacts.ERC721Proxy, provider, txDefaults); - // Depoy exchange - exchange = await ExchangeContract.deployFrom0xArtifactAsync( - artifacts.Exchange, - provider, - txDefaults, - assetDataUtils.encodeERC20AssetData(zrxToken.address), - ); - exchangeWrapper = new ExchangeWrapper(exchange, provider); - await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); - await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); - // Authorize ERC20 trades by exchange - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - // Deploy OrderMatcher - orderMatcher = await OrderMatcherContract.deployFrom0xArtifactAsync( - artifacts.OrderMatcher, - provider, - txDefaults, - exchange.address, - ); - // Set default addresses - defaultERC20MakerAssetAddress = erc20TokenA.address; - defaultERC20TakerAssetAddress = erc20TokenB.address; - leftMakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress); - leftTakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress); - // Set OrderMatcher balances and allowances - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20TokenA.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20TokenB.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync( - leftMakerAssetData, - constants.INITIAL_ERC20_ALLOWANCE, - ), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync( - leftTakerAssetData, - constants.INITIAL_ERC20_ALLOWANCE, - ), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - // Create default order parameters - const defaultOrderParamsLeft = { - ...constants.STATIC_ORDER_PARAMS, - makerAddress: makerAddressLeft, - exchangeAddress: exchange.address, - makerAssetData: leftMakerAssetData, - takerAssetData: leftTakerAssetData, - feeRecipientAddress: feeRecipientAddressLeft, - makerFee: constants.ZERO_AMOUNT, - takerFee: constants.ZERO_AMOUNT, - }; - const defaultOrderParamsRight = { - ...constants.STATIC_ORDER_PARAMS, - makerAddress: makerAddressRight, - exchangeAddress: exchange.address, - makerAssetData: leftTakerAssetData, - takerAssetData: leftMakerAssetData, - feeRecipientAddress: feeRecipientAddressRight, - makerFee: constants.ZERO_AMOUNT, - takerFee: constants.ZERO_AMOUNT, - }; - const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)]; - orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft); - const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; - orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight); - }); - beforeEach(async () => { - await blockchainLifecycle.startAsync(); - }); - afterEach(async () => { - await blockchainLifecycle.revertAsync(); - }); - describe('constructor', () => { - it('should revert if assetProxy is unregistered', async () => { - const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( - artifacts.Exchange, - provider, - txDefaults, - constants.NULL_BYTES, - ); - return expectContractCreationFailedAsync( - (OrderMatcherContract.deployFrom0xArtifactAsync( - artifacts.OrderMatcher, - provider, - txDefaults, - exchangeInstance.address, - ) as any) as sendTransactionResult, - RevertReason.UnregisteredAssetProxy, - ); - }); - }); - describe('matchOrders', () => { - beforeEach(async () => { - erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); - }); - it('should revert if not called by owner', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - }); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await expectTransactionFailedAsync( - web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: takerAddress, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - RevertReason.OnlyContractOwner, - ); - }); - it('should transfer the correct amounts when orders completely fill each other', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - }); - it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal(initialLeftMakerAssetTakerBalance); - }); - it('should transfer the correct amounts when left order is completely filled and right order would be partially filled', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), - leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - // Match signedOrderLeft with signedOrderRight - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), - ); - }); - it('should not call fillOrder when rightOrder is completely filled after matchOrders call', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - }); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - const logDecoder = new LogDecoder(web3Wrapper, { ...artifacts, ...tokenArtifacts }); - const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - ); - const fillLogs = _.filter( - txReceipt.logs, - log => (log as LogWithDecodedArgs).event === 'Fill', - ); - // Only 2 Fill logs should exist for `matchOrders` call. `fillOrder` should not have been called and should not have emitted a Fill event. - expect(fillLogs.length).to.be.equal(2); - }); - it('should only take a spread in rightMakerAsset if entire leftMakerAssetSpread amount can be used to fill rightOrder after matchOrders call', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.9), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(990), 18), - }); - const initialLeftMakerAssetSpreadAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1.09), 18); - const leftTakerAssetSpreadAmount = initialLeftMakerAssetSpreadAmount - .times(signedOrderRight.makerAssetAmount) - .dividedToIntegerBy(signedOrderRight.takerAssetAmount); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderLeft.takerAssetAmount.plus(leftTakerAssetSpreadAmount), - amountBoughtByRightMaker: signedOrderLeft.makerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: constants.ZERO_AMOUNT, - leftTakerAssetSpreadAmount, - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - // Match signedOrderLeft with signedOrderRight - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), - ); - }); - it("should succeed if rightOrder's makerAssetData and takerAssetData are not provided", async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), - leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - // Match signedOrderLeft with signedOrderRight - signedOrderRight.makerAssetData = constants.NULL_BYTES; - signedOrderRight.takerAssetData = constants.NULL_BYTES; - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), - ); - }); - it('should revert with the correct reason if matchOrders call reverts', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - }); - signedOrderRight.signature = `0xff${signedOrderRight.signature.slice(4)}`; - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await expectTransactionFailedAsync( - web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - RevertReason.InvalidOrderSignature, - ); - }); - it('should revert with the correct reason if fillOrder call reverts', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), - }); - // Matcher will not have enough allowance to fill rightOrder - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, constants.ZERO_AMOUNT, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await expectTransactionFailedAsync( - web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - RevertReason.TransferFailed, - ); - }); - }); - describe('withdrawAsset', () => { - it('should allow owner to withdraw ERC20 tokens', async () => { - const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { - from: owner, - }), - ); - const newBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - expect(newBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); - }); - it('should allow owner to withdraw ERC721 tokens', async () => { - const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - tokenArtifacts.DummyERC721Token, - provider, - txDefaults, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - ); - const tokenId = new BigNumber(1); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc721Token.mint.sendTransactionAsync(orderMatcher.address, tokenId, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, tokenId); - const withdrawAmount = new BigNumber(1); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.withdrawAsset.sendTransactionAsync(assetData, withdrawAmount, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const erc721Owner = await erc721Token.ownerOf.callAsync(tokenId); - expect(erc721Owner).to.be.equal(owner); - }); - it('should revert if not called by owner', async () => { - const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); - await expectTransactionFailedAsync( - orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { - from: takerAddress, - }), - RevertReason.OnlyContractOwner, - ); - }); - }); - describe('approveAssetProxy', () => { - it('should be able to set an allowance for ERC20 tokens', async () => { - const allowance = new BigNumber(55465465426546); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, allowance, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newAllowance = await erc20TokenA.allowance.callAsync(orderMatcher.address, erc20Proxy.address); - expect(newAllowance).to.be.bignumber.equal(allowance); - }); - it('should be able to approve an ERC721 token by passing in allowance = 1', async () => { - const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - tokenArtifacts.DummyERC721Token, - provider, - txDefaults, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - ); - const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); - const allowance = new BigNumber(1); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); - expect(isApproved).to.be.equal(true); - }); - it('should be able to approve an ERC721 token by passing in allowance > 1', async () => { - const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - tokenArtifacts.DummyERC721Token, - provider, - txDefaults, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - ); - const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); - const allowance = new BigNumber(2); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); - expect(isApproved).to.be.equal(true); - }); - it('should revert if not called by owner', async () => { - const approval = new BigNumber(1); - await expectTransactionFailedAsync( - orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, approval, { - from: takerAddress, - }), - RevertReason.OnlyContractOwner, - ); - }); - }); -}); -// tslint:disable:max-file-line-count -// tslint:enable:no-unnecessary-type-assertion diff --git a/contracts/extensions/package.json b/contracts/extensions/package.json index aa5cef462..d0caa030e 100644 --- a/contracts/extensions/package.json +++ b/contracts/extensions/package.json @@ -32,7 +32,7 @@ "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol" }, "config": { - "abis": "generated-artifacts/@(BalanceThresholdFilter|DutchAuction|Forwarder).json" + "abis": "generated-artifacts/@(BalanceThresholdFilter|DutchAuction|Forwarder|OrderMatcher).json" }, "repository": { "type": "git", diff --git a/contracts/extensions/src/artifacts/index.ts b/contracts/extensions/src/artifacts/index.ts index ebf0b8050..a8bd34b4e 100644 --- a/contracts/extensions/src/artifacts/index.ts +++ b/contracts/extensions/src/artifacts/index.ts @@ -3,9 +3,11 @@ import { ContractArtifact } from 'ethereum-types'; import * as BalanceThresholdFilter from '../../generated-artifacts/BalanceThresholdFilter.json'; import * as DutchAuction from '../../generated-artifacts/DutchAuction.json'; import * as Forwarder from '../../generated-artifacts/Forwarder.json'; +import * as OrderMatcher from '../../generated-artifacts/OrderMatcher.json'; export const artifacts = { BalanceThresholdFilter: BalanceThresholdFilter as ContractArtifact, DutchAuction: DutchAuction as ContractArtifact, Forwarder: Forwarder as ContractArtifact, + OrderMatcher: OrderMatcher as ContractArtifact, }; diff --git a/contracts/extensions/src/wrappers/index.ts b/contracts/extensions/src/wrappers/index.ts index 8a8122caa..4d075ee13 100644 --- a/contracts/extensions/src/wrappers/index.ts +++ b/contracts/extensions/src/wrappers/index.ts @@ -1,3 +1,4 @@ export * from '../../generated-wrappers/balance_threshold_filter'; export * from '../../generated-wrappers/dutch_auction'; export * from '../../generated-wrappers/forwarder'; +export * from '../../generated-wrappers/order_matcher'; diff --git a/contracts/extensions/test/extensions/order_matcher.ts b/contracts/extensions/test/extensions/order_matcher.ts new file mode 100644 index 000000000..45002b324 --- /dev/null +++ b/contracts/extensions/test/extensions/order_matcher.ts @@ -0,0 +1,780 @@ +import { + artifacts as protocolArtifacts, + ERC20ProxyContract, + ERC20Wrapper, + ERC721ProxyContract, + ExchangeContract, + ExchangeFillEventArgs, + ExchangeWrapper, +} from '@0x/contracts-protocol'; +import { + chaiSetup, + constants, + ERC20BalancesByOwner, + expectContractCreationFailedAsync, + expectTransactionFailedAsync, + LogDecoder, + OrderFactory, + provider, + sendTransactionResult, + txDefaults, + web3Wrapper, +} from '@0x/contracts-test-utils'; +import { artifacts as tokenArtifacts, DummyERC20TokenContract, DummyERC721TokenContract } from '@0x/contracts-tokens'; +import { BlockchainLifecycle } from '@0x/dev-utils'; +import { assetDataUtils } from '@0x/order-utils'; +import { RevertReason } from '@0x/types'; +import { BigNumber } from '@0x/utils'; +import { Web3Wrapper } from '@0x/web3-wrapper'; +import * as chai from 'chai'; +import { LogWithDecodedArgs } from 'ethereum-types'; +import * as _ from 'lodash'; + +import { OrderMatcherContract } from '../../generated-wrappers/order_matcher'; +import { artifacts } from '../../src/artifacts'; + +const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); +chaiSetup.configure(); +const expect = chai.expect; +// tslint:disable:no-unnecessary-type-assertion +describe('OrderMatcher', () => { + let makerAddressLeft: string; + let makerAddressRight: string; + let owner: string; + let takerAddress: string; + let feeRecipientAddressLeft: string; + let feeRecipientAddressRight: string; + + let erc20TokenA: DummyERC20TokenContract; + let erc20TokenB: DummyERC20TokenContract; + let zrxToken: DummyERC20TokenContract; + let exchange: ExchangeContract; + let erc20Proxy: ERC20ProxyContract; + let erc721Proxy: ERC721ProxyContract; + let orderMatcher: OrderMatcherContract; + + let erc20BalancesByOwner: ERC20BalancesByOwner; + let exchangeWrapper: ExchangeWrapper; + let erc20Wrapper: ERC20Wrapper; + let orderFactoryLeft: OrderFactory; + let orderFactoryRight: OrderFactory; + + let leftMakerAssetData: string; + let leftTakerAssetData: string; + let defaultERC20MakerAssetAddress: string; + let defaultERC20TakerAssetAddress: string; + + before(async () => { + await blockchainLifecycle.startAsync(); + }); + after(async () => { + await blockchainLifecycle.revertAsync(); + }); + before(async () => { + // Create accounts + const accounts = await web3Wrapper.getAvailableAddressesAsync(); + // Hack(albrow): Both Prettier and TSLint insert a trailing comma below + // but that is invalid syntax as of TypeScript version >= 2.8. We don't + // have the right fine-grained configuration options in TSLint, + // Prettier, or TypeScript, to reconcile this, so we will just have to + // wait for them to sort it out. We disable TSLint and Prettier for + // this part of the code for now. This occurs several times in this + // file. See https://github.com/prettier/prettier/issues/4624. + // prettier-ignore + const usedAddresses = ([ + owner, + makerAddressLeft, + makerAddressRight, + takerAddress, + feeRecipientAddressLeft, + // tslint:disable-next-line:trailing-comma + feeRecipientAddressRight + ] = _.slice(accounts, 0, 6)); + // Create wrappers + erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); + // Deploy ERC20 token & ERC20 proxy + const numDummyErc20ToDeploy = 3; + [erc20TokenA, erc20TokenB, zrxToken] = await erc20Wrapper.deployDummyTokensAsync( + numDummyErc20ToDeploy, + constants.DUMMY_TOKEN_DECIMALS, + ); + erc20Proxy = await erc20Wrapper.deployProxyAsync(); + await erc20Wrapper.setBalancesAndAllowancesAsync(); + // Deploy ERC721 proxy + erc721Proxy = await ERC721ProxyContract.deployFrom0xArtifactAsync( + protocolArtifacts.ERC721Proxy, + provider, + txDefaults, + ); + // Depoy exchange + exchange = await ExchangeContract.deployFrom0xArtifactAsync( + protocolArtifacts.Exchange, + provider, + txDefaults, + assetDataUtils.encodeERC20AssetData(zrxToken.address), + ); + exchangeWrapper = new ExchangeWrapper(exchange, provider); + await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); + await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); + // Authorize ERC20 trades by exchange + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Deploy OrderMatcher + orderMatcher = await OrderMatcherContract.deployFrom0xArtifactAsync( + artifacts.OrderMatcher, + provider, + txDefaults, + exchange.address, + ); + // Set default addresses + defaultERC20MakerAssetAddress = erc20TokenA.address; + defaultERC20TakerAssetAddress = erc20TokenB.address; + leftMakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress); + leftTakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress); + // Set OrderMatcher balances and allowances + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenA.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await erc20TokenB.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync( + leftMakerAssetData, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync( + leftTakerAssetData, + constants.INITIAL_ERC20_ALLOWANCE, + ), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + // Create default order parameters + const defaultOrderParamsLeft = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressLeft, + exchangeAddress: exchange.address, + makerAssetData: leftMakerAssetData, + takerAssetData: leftTakerAssetData, + feeRecipientAddress: feeRecipientAddressLeft, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + }; + const defaultOrderParamsRight = { + ...constants.STATIC_ORDER_PARAMS, + makerAddress: makerAddressRight, + exchangeAddress: exchange.address, + makerAssetData: leftTakerAssetData, + takerAssetData: leftMakerAssetData, + feeRecipientAddress: feeRecipientAddressRight, + makerFee: constants.ZERO_AMOUNT, + takerFee: constants.ZERO_AMOUNT, + }; + const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)]; + orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft); + const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; + orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight); + }); + beforeEach(async () => { + await blockchainLifecycle.startAsync(); + }); + afterEach(async () => { + await blockchainLifecycle.revertAsync(); + }); + describe('constructor', () => { + it('should revert if assetProxy is unregistered', async () => { + const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( + protocolArtifacts.Exchange, + provider, + txDefaults, + constants.NULL_BYTES, + ); + return expectContractCreationFailedAsync( + (OrderMatcherContract.deployFrom0xArtifactAsync( + artifacts.OrderMatcher, + provider, + txDefaults, + exchangeInstance.address, + ) as any) as sendTransactionResult, + RevertReason.UnregisteredAssetProxy, + ); + }); + }); + describe('matchOrders', () => { + beforeEach(async () => { + erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); + }); + it('should revert if not called by owner', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: takerAddress, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.OnlyContractOwner, + ); + }); + it('should transfer the correct amounts when orders completely fill each other', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + }); + it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal(initialLeftMakerAssetTakerBalance); + }); + it('should transfer the correct amounts when left order is completely filled and right order would be partially filled', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it('should not call fillOrder when rightOrder is completely filled after matchOrders call', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + const logDecoder = new LogDecoder(web3Wrapper, { ...artifacts, ...tokenArtifacts }); + const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + ); + const fillLogs = _.filter( + txReceipt.logs, + log => (log as LogWithDecodedArgs).event === 'Fill', + ); + // Only 2 Fill logs should exist for `matchOrders` call. `fillOrder` should not have been called and should not have emitted a Fill event. + expect(fillLogs.length).to.be.equal(2); + }); + it('should only take a spread in rightMakerAsset if entire leftMakerAssetSpread amount can be used to fill rightOrder after matchOrders call', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.9), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(990), 18), + }); + const initialLeftMakerAssetSpreadAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1.09), 18); + const leftTakerAssetSpreadAmount = initialLeftMakerAssetSpreadAmount + .times(signedOrderRight.makerAssetAmount) + .dividedToIntegerBy(signedOrderRight.takerAssetAmount); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderLeft.takerAssetAmount.plus(leftTakerAssetSpreadAmount), + amountBoughtByRightMaker: signedOrderLeft.makerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: constants.ZERO_AMOUNT, + leftTakerAssetSpreadAmount, + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it("should succeed if rightOrder's makerAssetData and takerAssetData are not provided", async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Match signedOrderLeft with signedOrderRight + const expectedTransferAmounts = { + // Left Maker + amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, + amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, + // Right Maker + amountSoldByRightMaker: signedOrderRight.makerAssetAmount, + amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, + // Taker + leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), + leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), + }; + const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + // Match signedOrderLeft with signedOrderRight + signedOrderRight.makerAssetData = constants.NULL_BYTES; + signedOrderRight.takerAssetData = constants.NULL_BYTES; + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await web3Wrapper.awaitTransactionSuccessAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); + const newErc20Balances = await erc20Wrapper.getBalancesAsync(); + expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( + expectedTransferAmounts.amountSoldByRightMaker, + ), + ); + expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByLeftMaker, + ), + ); + expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( + erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( + expectedTransferAmounts.amountBoughtByRightMaker, + ), + ); + expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), + ); + expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( + initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), + ); + }); + it('should revert with the correct reason if matchOrders call reverts', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + }); + signedOrderRight.signature = `0xff${signedOrderRight.signature.slice(4)}`; + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.InvalidOrderSignature, + ); + }); + it('should revert with the correct reason if fillOrder call reverts', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), + }); + // Matcher will not have enough allowance to fill rightOrder + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, constants.ZERO_AMOUNT, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + await expectTransactionFailedAsync( + web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + RevertReason.TransferFailed, + ); + }); + }); + describe('withdrawAsset', () => { + it('should allow owner to withdraw ERC20 tokens', async () => { + const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { + from: owner, + }), + ); + const newBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(newBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('should allow owner to withdraw ERC721 tokens', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + tokenArtifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const tokenId = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await erc721Token.mint.sendTransactionAsync(orderMatcher.address, tokenId, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, tokenId); + const withdrawAmount = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.withdrawAsset.sendTransactionAsync(assetData, withdrawAmount, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const erc721Owner = await erc721Token.ownerOf.callAsync(tokenId); + expect(erc721Owner).to.be.equal(owner); + }); + it('should revert if not called by owner', async () => { + const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); + expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); + await expectTransactionFailedAsync( + orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { + from: takerAddress, + }), + RevertReason.OnlyContractOwner, + ); + }); + }); + describe('approveAssetProxy', () => { + it('should be able to set an allowance for ERC20 tokens', async () => { + const allowance = new BigNumber(55465465426546); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, allowance, { + from: owner, + }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const newAllowance = await erc20TokenA.allowance.callAsync(orderMatcher.address, erc20Proxy.address); + expect(newAllowance).to.be.bignumber.equal(allowance); + }); + it('should be able to approve an ERC721 token by passing in allowance = 1', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + tokenArtifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); + const allowance = new BigNumber(1); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); + expect(isApproved).to.be.equal(true); + }); + it('should be able to approve an ERC721 token by passing in allowance > 1', async () => { + const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( + tokenArtifacts.DummyERC721Token, + provider, + txDefaults, + constants.DUMMY_TOKEN_NAME, + constants.DUMMY_TOKEN_SYMBOL, + ); + const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); + const allowance = new BigNumber(2); + await web3Wrapper.awaitTransactionSuccessAsync( + await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), + constants.AWAIT_TRANSACTION_MINED_MS, + ); + const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); + expect(isApproved).to.be.equal(true); + }); + it('should revert if not called by owner', async () => { + const approval = new BigNumber(1); + await expectTransactionFailedAsync( + orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, approval, { + from: takerAddress, + }), + RevertReason.OnlyContractOwner, + ); + }); + }); +}); +// tslint:disable:max-file-line-count +// tslint:enable:no-unnecessary-type-assertion diff --git a/contracts/extensions/tsconfig.json b/contracts/extensions/tsconfig.json index a303e3f5c..506c283aa 100644 --- a/contracts/extensions/tsconfig.json +++ b/contracts/extensions/tsconfig.json @@ -9,7 +9,8 @@ "files": [ "./generated-artifacts/BalanceThresholdFilter.json", "./generated-artifacts/DutchAuction.json", - "./generated-artifacts/Forwarder.json" + "./generated-artifacts/Forwarder.json", + "./generated-artifacts/OrderMatcher.json" ], "exclude": ["./deploy/solc/solc_bin"] } diff --git a/contracts/protocol/src/artifacts/index.ts b/contracts/protocol/src/artifacts/index.ts index 2a9de78d2..1d53ceb04 100644 --- a/contracts/protocol/src/artifacts/index.ts +++ b/contracts/protocol/src/artifacts/index.ts @@ -6,7 +6,6 @@ import * as ERC721Proxy from '../../generated-artifacts/ERC721Proxy.json'; import * as Exchange from '../../generated-artifacts/Exchange.json'; import * as MixinAuthorizable from '../../generated-artifacts/MixinAuthorizable.json'; import * as MultiAssetProxy from '../../generated-artifacts/MultiAssetProxy.json'; -import * as OrderMatcher from '../../generated-artifacts/OrderMatcher.json'; import * as OrderValidator from '../../generated-artifacts/OrderValidator.json'; import * as TestAssetProxyDispatcher from '../../generated-artifacts/TestAssetProxyDispatcher.json'; import * as TestAssetProxyOwner from '../../generated-artifacts/TestAssetProxyOwner.json'; @@ -21,7 +20,6 @@ export const artifacts = { Exchange: Exchange as ContractArtifact, MixinAuthorizable: MixinAuthorizable as ContractArtifact, MultiAssetProxy: MultiAssetProxy as ContractArtifact, - OrderMatcher: OrderMatcher as ContractArtifact, OrderValidator: OrderValidator as ContractArtifact, TestAssetProxyDispatcher: TestAssetProxyDispatcher as ContractArtifact, TestAssetProxyOwner: TestAssetProxyOwner as ContractArtifact, diff --git a/contracts/protocol/src/wrappers/index.ts b/contracts/protocol/src/wrappers/index.ts index 13f16f44f..ac951d269 100644 --- a/contracts/protocol/src/wrappers/index.ts +++ b/contracts/protocol/src/wrappers/index.ts @@ -3,7 +3,6 @@ export * from '../../generated-wrappers/erc20_proxy'; export * from '../../generated-wrappers/erc721_proxy'; export * from '../../generated-wrappers/exchange'; export * from '../../generated-wrappers/mixin_authorizable'; -export * from '../../generated-wrappers/order_matcher'; export * from '../../generated-wrappers/order_validator'; export * from '../../generated-wrappers/test_asset_proxy_dispatcher'; export * from '../../generated-wrappers/test_asset_proxy_owner'; diff --git a/contracts/protocol/tsconfig.json b/contracts/protocol/tsconfig.json index 7e4ce5eb4..989d3ef2b 100644 --- a/contracts/protocol/tsconfig.json +++ b/contracts/protocol/tsconfig.json @@ -13,7 +13,6 @@ "./generated-artifacts/Exchange.json", "./generated-artifacts/MixinAuthorizable.json", "./generated-artifacts/MultiAssetProxy.json", - "./generated-artifacts/OrderMatcher.json", "./generated-artifacts/OrderValidator.json", "./generated-artifacts/TestAssetProxyDispatcher.json", "./generated-artifacts/TestAssetProxyOwner.json", diff --git a/packages/contracts/test/extensions/order_matcher.ts b/packages/contracts/test/extensions/order_matcher.ts deleted file mode 100644 index 8e1f633c2..000000000 --- a/packages/contracts/test/extensions/order_matcher.ts +++ /dev/null @@ -1,770 +0,0 @@ -import { BlockchainLifecycle } from '@0x/dev-utils'; -import { assetDataUtils } from '@0x/order-utils'; -import { RevertReason } from '@0x/types'; -import { BigNumber } from '@0x/utils'; -import { Web3Wrapper } from '@0x/web3-wrapper'; -import * as chai from 'chai'; -import { LogWithDecodedArgs } from 'ethereum-types'; -import * as _ from 'lodash'; - -import { DummyERC20TokenContract } from '../../generated-wrappers/dummy_erc20_token'; -import { DummyERC721TokenContract } from '../../generated-wrappers/dummy_erc721_token'; -import { ERC20ProxyContract } from '../../generated-wrappers/erc20_proxy'; -import { ERC721ProxyContract } from '../../generated-wrappers/erc721_proxy'; -import { ExchangeContract, ExchangeFillEventArgs } from '../../generated-wrappers/exchange'; -import { OrderMatcherContract } from '../../generated-wrappers/order_matcher'; -import { artifacts } from '../../src/artifacts'; -import { - expectContractCreationFailedAsync, - expectTransactionFailedAsync, - sendTransactionResult, -} from '../utils/assertions'; -import { chaiSetup } from '../utils/chai_setup'; -import { constants } from '../utils/constants'; -import { ERC20Wrapper } from '../utils/erc20_wrapper'; -import { ExchangeWrapper } from '../utils/exchange_wrapper'; -import { LogDecoder } from '../utils/log_decoder'; -import { OrderFactory } from '../utils/order_factory'; -import { ERC20BalancesByOwner } from '../utils/types'; -import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; - -const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -chaiSetup.configure(); -const expect = chai.expect; -// tslint:disable:no-unnecessary-type-assertion -describe('OrderMatcher', () => { - let makerAddressLeft: string; - let makerAddressRight: string; - let owner: string; - let takerAddress: string; - let feeRecipientAddressLeft: string; - let feeRecipientAddressRight: string; - - let erc20TokenA: DummyERC20TokenContract; - let erc20TokenB: DummyERC20TokenContract; - let zrxToken: DummyERC20TokenContract; - let exchange: ExchangeContract; - let erc20Proxy: ERC20ProxyContract; - let erc721Proxy: ERC721ProxyContract; - let orderMatcher: OrderMatcherContract; - - let erc20BalancesByOwner: ERC20BalancesByOwner; - let exchangeWrapper: ExchangeWrapper; - let erc20Wrapper: ERC20Wrapper; - let orderFactoryLeft: OrderFactory; - let orderFactoryRight: OrderFactory; - - let leftMakerAssetData: string; - let leftTakerAssetData: string; - let defaultERC20MakerAssetAddress: string; - let defaultERC20TakerAssetAddress: string; - - before(async () => { - await blockchainLifecycle.startAsync(); - }); - after(async () => { - await blockchainLifecycle.revertAsync(); - }); - before(async () => { - // Create accounts - const accounts = await web3Wrapper.getAvailableAddressesAsync(); - // Hack(albrow): Both Prettier and TSLint insert a trailing comma below - // but that is invalid syntax as of TypeScript version >= 2.8. We don't - // have the right fine-grained configuration options in TSLint, - // Prettier, or TypeScript, to reconcile this, so we will just have to - // wait for them to sort it out. We disable TSLint and Prettier for - // this part of the code for now. This occurs several times in this - // file. See https://github.com/prettier/prettier/issues/4624. - // prettier-ignore - const usedAddresses = ([ - owner, - makerAddressLeft, - makerAddressRight, - takerAddress, - feeRecipientAddressLeft, - // tslint:disable-next-line:trailing-comma - feeRecipientAddressRight - ] = _.slice(accounts, 0, 6)); - // Create wrappers - erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); - // Deploy ERC20 token & ERC20 proxy - const numDummyErc20ToDeploy = 3; - [erc20TokenA, erc20TokenB, zrxToken] = await erc20Wrapper.deployDummyTokensAsync( - numDummyErc20ToDeploy, - constants.DUMMY_TOKEN_DECIMALS, - ); - erc20Proxy = await erc20Wrapper.deployProxyAsync(); - await erc20Wrapper.setBalancesAndAllowancesAsync(); - // Deploy ERC721 proxy - erc721Proxy = await ERC721ProxyContract.deployFrom0xArtifactAsync(artifacts.ERC721Proxy, provider, txDefaults); - // Depoy exchange - exchange = await ExchangeContract.deployFrom0xArtifactAsync( - artifacts.Exchange, - provider, - txDefaults, - assetDataUtils.encodeERC20AssetData(zrxToken.address), - ); - exchangeWrapper = new ExchangeWrapper(exchange, provider); - await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner); - await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner); - // Authorize ERC20 trades by exchange - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20Proxy.addAuthorizedAddress.sendTransactionAsync(exchange.address, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - // Deploy OrderMatcher - orderMatcher = await OrderMatcherContract.deployFrom0xArtifactAsync( - artifacts.OrderMatcher, - provider, - txDefaults, - exchange.address, - ); - // Set default addresses - defaultERC20MakerAssetAddress = erc20TokenA.address; - defaultERC20TakerAssetAddress = erc20TokenB.address; - leftMakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress); - leftTakerAssetData = assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress); - // Set OrderMatcher balances and allowances - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20TokenA.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc20TokenB.setBalance.sendTransactionAsync(orderMatcher.address, constants.INITIAL_ERC20_BALANCE, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync( - leftMakerAssetData, - constants.INITIAL_ERC20_ALLOWANCE, - ), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync( - leftTakerAssetData, - constants.INITIAL_ERC20_ALLOWANCE, - ), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - // Create default order parameters - const defaultOrderParamsLeft = { - ...constants.STATIC_ORDER_PARAMS, - makerAddress: makerAddressLeft, - exchangeAddress: exchange.address, - makerAssetData: leftMakerAssetData, - takerAssetData: leftTakerAssetData, - feeRecipientAddress: feeRecipientAddressLeft, - makerFee: constants.ZERO_AMOUNT, - takerFee: constants.ZERO_AMOUNT, - }; - const defaultOrderParamsRight = { - ...constants.STATIC_ORDER_PARAMS, - makerAddress: makerAddressRight, - exchangeAddress: exchange.address, - makerAssetData: leftTakerAssetData, - takerAssetData: leftMakerAssetData, - feeRecipientAddress: feeRecipientAddressRight, - makerFee: constants.ZERO_AMOUNT, - takerFee: constants.ZERO_AMOUNT, - }; - const privateKeyLeft = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressLeft)]; - orderFactoryLeft = new OrderFactory(privateKeyLeft, defaultOrderParamsLeft); - const privateKeyRight = constants.TESTRPC_PRIVATE_KEYS[accounts.indexOf(makerAddressRight)]; - orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight); - }); - beforeEach(async () => { - await blockchainLifecycle.startAsync(); - }); - afterEach(async () => { - await blockchainLifecycle.revertAsync(); - }); - describe('constructor', () => { - it('should revert if assetProxy is unregistered', async () => { - const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync( - artifacts.Exchange, - provider, - txDefaults, - constants.NULL_BYTES, - ); - return expectContractCreationFailedAsync( - (OrderMatcherContract.deployFrom0xArtifactAsync( - artifacts.OrderMatcher, - provider, - txDefaults, - exchangeInstance.address, - ) as any) as sendTransactionResult, - RevertReason.UnregisteredAssetProxy, - ); - }); - }); - describe('matchOrders', () => { - beforeEach(async () => { - erc20BalancesByOwner = await erc20Wrapper.getBalancesAsync(); - }); - it('should revert if not called by owner', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - }); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await expectTransactionFailedAsync( - web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: takerAddress, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - RevertReason.OnlyContractOwner, - ); - }); - it('should transfer the correct amounts when orders completely fill each other', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - }); - it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal(initialLeftMakerAssetTakerBalance); - }); - it('should transfer the correct amounts when left order is completely filled and right order would be partially filled', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), - leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - // Match signedOrderLeft with signedOrderRight - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), - ); - }); - it('should not call fillOrder when rightOrder is completely filled after matchOrders call', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - }); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - const logDecoder = new LogDecoder(web3Wrapper); - const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - ); - const fillLogs = _.filter( - txReceipt.logs, - log => (log as LogWithDecodedArgs).event === 'Fill', - ); - expect(fillLogs.length).to.be.equal(2); - }); - it('should only take a spread in rightMakerAsset if entire leftMakerAssetSpread amount can be used to fill rightOrder after matchOrders call', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(0.9), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(990), 18), - }); - const initialLeftMakerAssetSpreadAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1.09), 18); - const leftTakerAssetSpreadAmount = initialLeftMakerAssetSpreadAmount - .times(signedOrderRight.makerAssetAmount) - .dividedToIntegerBy(signedOrderRight.takerAssetAmount); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderLeft.takerAssetAmount.plus(leftTakerAssetSpreadAmount), - amountBoughtByRightMaker: signedOrderLeft.makerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: constants.ZERO_AMOUNT, - leftTakerAssetSpreadAmount, - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - // Match signedOrderLeft with signedOrderRight - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), - ); - }); - it("should succeed if rightOrder's makerAssetData and takerAssetData are not provided", async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), - }); - // Match signedOrderLeft with signedOrderRight - const expectedTransferAmounts = { - // Left Maker - amountSoldByLeftMaker: signedOrderLeft.makerAssetAmount, - amountBoughtByLeftMaker: signedOrderLeft.takerAssetAmount, - // Right Maker - amountSoldByRightMaker: signedOrderRight.makerAssetAmount, - amountBoughtByRightMaker: signedOrderRight.takerAssetAmount, - // Taker - leftMakerAssetSpreadAmount: signedOrderLeft.makerAssetAmount.minus(signedOrderRight.takerAssetAmount), - leftTakerAssetSpreadAmount: signedOrderRight.makerAssetAmount.minus(signedOrderLeft.takerAssetAmount), - }; - const initialLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const initialLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - // Match signedOrderLeft with signedOrderRight - signedOrderRight.makerAssetData = constants.NULL_BYTES; - signedOrderRight.takerAssetData = constants.NULL_BYTES; - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await web3Wrapper.awaitTransactionSuccessAsync( - await web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newLeftMakerAssetTakerBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - const newLeftTakerAssetTakerBalance = await erc20TokenB.balanceOf.callAsync(orderMatcher.address); - const newErc20Balances = await erc20Wrapper.getBalancesAsync(); - expect(newErc20Balances[makerAddressLeft][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20MakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20TakerAssetAddress].minus( - expectedTransferAmounts.amountSoldByRightMaker, - ), - ); - expect(newErc20Balances[makerAddressLeft][defaultERC20TakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressLeft][defaultERC20TakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByLeftMaker, - ), - ); - expect(newErc20Balances[makerAddressRight][defaultERC20MakerAssetAddress]).to.be.bignumber.equal( - erc20BalancesByOwner[makerAddressRight][defaultERC20MakerAssetAddress].plus( - expectedTransferAmounts.amountBoughtByRightMaker, - ), - ); - expect(newLeftMakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftMakerAssetTakerBalance.plus(expectedTransferAmounts.leftMakerAssetSpreadAmount), - ); - expect(newLeftTakerAssetTakerBalance).to.be.bignumber.equal( - initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), - ); - }); - it('should revert with the correct reason if matchOrders call reverts', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - }); - signedOrderRight.signature = `0xff${signedOrderRight.signature.slice(4)}`; - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await expectTransactionFailedAsync( - web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - RevertReason.InvalidOrderSignature, - ); - }); - it('should revert with the correct reason if fillOrder call reverts', async () => { - // Create orders to match - const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - }); - const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ - makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(20), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 18), - }); - // Matcher will not have enough allowance to fill rightOrder - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, constants.ZERO_AMOUNT, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const data = exchange.matchOrders.getABIEncodedTransactionData( - signedOrderLeft, - signedOrderRight, - signedOrderLeft.signature, - signedOrderRight.signature, - ); - await expectTransactionFailedAsync( - web3Wrapper.sendTransactionAsync({ - data, - to: orderMatcher.address, - from: owner, - gas: constants.MAX_MATCH_ORDERS_GAS, - }), - RevertReason.TransferFailed, - ); - }); - }); - describe('withdrawAsset', () => { - it('should allow owner to withdraw ERC20 tokens', async () => { - const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { - from: owner, - }), - ); - const newBalance = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - expect(newBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); - }); - it('should allow owner to withdraw ERC721 tokens', async () => { - const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - artifacts.DummyERC721Token, - provider, - txDefaults, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - ); - const tokenId = new BigNumber(1); - await web3Wrapper.awaitTransactionSuccessAsync( - await erc721Token.mint.sendTransactionAsync(orderMatcher.address, tokenId, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, tokenId); - const withdrawAmount = new BigNumber(1); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.withdrawAsset.sendTransactionAsync(assetData, withdrawAmount, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const erc721Owner = await erc721Token.ownerOf.callAsync(tokenId); - expect(erc721Owner).to.be.equal(owner); - }); - it('should revert if not called by owner', async () => { - const erc20AWithdrawAmount = await erc20TokenA.balanceOf.callAsync(orderMatcher.address); - expect(erc20AWithdrawAmount).to.be.bignumber.gt(constants.ZERO_AMOUNT); - await expectTransactionFailedAsync( - orderMatcher.withdrawAsset.sendTransactionAsync(leftMakerAssetData, erc20AWithdrawAmount, { - from: takerAddress, - }), - RevertReason.OnlyContractOwner, - ); - }); - }); - describe('approveAssetProxy', () => { - it('should be able to set an allowance for ERC20 tokens', async () => { - const allowance = new BigNumber(55465465426546); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, allowance, { - from: owner, - }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const newAllowance = await erc20TokenA.allowance.callAsync(orderMatcher.address, erc20Proxy.address); - expect(newAllowance).to.be.bignumber.equal(allowance); - }); - it('should be able to approve an ERC721 token by passing in allowance = 1', async () => { - const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - artifacts.DummyERC721Token, - provider, - txDefaults, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - ); - const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); - const allowance = new BigNumber(1); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); - expect(isApproved).to.be.equal(true); - }); - it('should be able to approve an ERC721 token by passing in allowance > 1', async () => { - const erc721Token = await DummyERC721TokenContract.deployFrom0xArtifactAsync( - artifacts.DummyERC721Token, - provider, - txDefaults, - constants.DUMMY_TOKEN_NAME, - constants.DUMMY_TOKEN_SYMBOL, - ); - const assetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, constants.ZERO_AMOUNT); - const allowance = new BigNumber(2); - await web3Wrapper.awaitTransactionSuccessAsync( - await orderMatcher.approveAssetProxy.sendTransactionAsync(assetData, allowance, { from: owner }), - constants.AWAIT_TRANSACTION_MINED_MS, - ); - const isApproved = await erc721Token.isApprovedForAll.callAsync(orderMatcher.address, erc721Proxy.address); - expect(isApproved).to.be.equal(true); - }); - it('should revert if not called by owner', async () => { - const approval = new BigNumber(1); - await expectTransactionFailedAsync( - orderMatcher.approveAssetProxy.sendTransactionAsync(leftMakerAssetData, approval, { - from: takerAddress, - }), - RevertReason.OnlyContractOwner, - ); - }); - }); -}); -// tslint:disable:max-file-line-count -// tslint:enable:no-unnecessary-type-assertion -- cgit v1.2.3 From e74b24bbdb1f6f53e5437ae3d3b2ebc2aa2adba6 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 17 Dec 2018 12:59:39 -0800 Subject: Update comments and hard code function selector constants --- .../contracts/OrderMatcher/MixinMatchOrders.sol | 100 ++++++++++----------- .../contracts/OrderMatcher/libs/LibConstants.sol | 9 +- 2 files changed, 56 insertions(+), 53 deletions(-) diff --git a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol index 866523190..c691b732f 100644 --- a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol +++ b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol @@ -126,23 +126,23 @@ contract MixinMatchOrders is // | 420 | | 10. salt | // | 452 | | 11. offset to leftMakerAssetData (*) | // | 484 | | 12. offset to leftTakerAssetData (*) | + // | 516 | 32 | leftMakerAssetData Length | + // | 548 | a | leftMakerAssetData Contents | + // | 548 + a | 32 | leftTakerAssetData Length | + // | 580 + a | b | leftTakerAssetData Contents | // | | 12 * 32 | rightOrder: | - // | 516 | | 1. senderAddress | - // | 548 | | 2. makerAddress | - // | 580 | | 3. takerAddress | - // | 612 | | 4. feeRecipientAddress | - // | 644 | | 5. makerAssetAmount | - // | 676 | | 6. takerAssetAmount | - // | 708 | | 7. makerFeeAmount | - // | 740 | | 8. takerFeeAmount | - // | 772 | | 9. expirationTimeSeconds | - // | 804 | | 10. salt | - // | 836 | | 11. offset to rightMakerAssetData (*) | - // | 868 | | 12. offset to rightTakerAssetData (*) | - // | 900 | 32 | leftMakerAssetData Length | - // | 932 | a | leftMakerAssetData Contents | - // | 932 + a | 32 | leftTakerAssetData Length | - // | 964 + a | b | leftTakerAssetData Contents | + // | 580 + a + b | | 1. senderAddress | + // | 612 + a + b | | 2. makerAddress | + // | 644 + a + b | | 3. takerAddress | + // | 676 + a + b | | 4. feeRecipientAddress | + // | 708 + a + b | | 5. makerAssetAmount | + // | 740 + a + b | | 6. takerAssetAmount | + // | 772 + a + b | | 7. makerFeeAmount | + // | 804 + a + b | | 8. takerFeeAmount | + // | 836 + a + b | | 9. expirationTimeSeconds | + // | 868 + a + b | | 10. salt | + // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | + // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | // | 964 + a + b | 32 | rightMakerAssetData Length | // | 996 + a + b | c | rightMakerAssetData Contents | // | 996 + a + b + c | 32 | rightTakerAssetData Length | @@ -159,7 +159,7 @@ contract MixinMatchOrders is 0, // transfer 0 wei 0, // input starts at 0 calldatasize(), // length of input - 0, // write output over output + 0, // write output over input 288 // length of output is 288 bytes ) @@ -183,18 +183,18 @@ contract MixinMatchOrders is // | 224 | | 8. right.takerFeePaid | // | 256 | | 9. leftMakerAssetSpreadAmount | // | | 12 * 32 | rightOrder: | - // | 516 | | 1. senderAddress | - // | 548 | | 2. makerAddress | - // | 580 | | 3. takerAddress | - // | 612 | | 4. feeRecipientAddress | - // | 644 | | 5. makerAssetAmount | - // | 676 | | 6. takerAssetAmount | - // | 708 | | 7. makerFeeAmount | - // | 740 | | 8. takerFeeAmount | - // | 772 | | 9. expirationTimeSeconds | - // | 804 | | 10. salt | - // | 836 | | 11. offset to rightMakerAssetData (*) | - // | 868 | | 12. offset to rightTakerAssetData (*) | + // | 580 + a + b | | 1. senderAddress | + // | 612 + a + b | | 2. makerAddress | + // | 644 + a + b | | 3. takerAddress | + // | 676 + a + b | | 4. feeRecipientAddress | + // | 708 + a + b | | 5. makerAssetAmount | + // | 740 + a + b | | 6. takerAssetAmount | + // | 772 + a + b | | 7. makerFeeAmount | + // | 804 + a + b | | 8. takerFeeAmount | + // | 836 + a + b | | 9. expirationTimeSeconds | + // | 868 + a + b | | 10. salt | + // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | + // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | let rightOrderStart := add(calldataload(36), 4) @@ -208,29 +208,29 @@ contract MixinMatchOrders is // | Offset | Length | Contents | // |---------------------------|---------|-------------------------------------------- | - // | 416 | 4 | `fillOrder` function selector | + // | 480 + a + b | 4 | `fillOrder` function selector | // | | 3 * 32 | function parameters: | - // | 420 | | 1. offset to rightOrder (*) | - // | 452 | | 2. takerAssetFillAmount | - // | 484 | | 3. offset to rightSignature (*) | + // | 484 + a + b | | 1. offset to rightOrder (*) | + // | 516 + a + b | | 2. takerAssetFillAmount | + // | 548 + a + b | | 3. offset to rightSignature (*) | // | | 12 * 32 | rightOrder: | - // | 516 | | 1. senderAddress | - // | 548 | | 2. makerAddress | - // | 580 | | 3. takerAddress | - // | 612 | | 4. feeRecipientAddress | - // | 644 | | 5. makerAssetAmount | - // | 676 | | 6. takerAssetAmount | - // | 708 | | 7. makerFeeAmount | - // | 740 | | 8. takerFeeAmount | - // | 772 | | 9. expirationTimeSeconds | - // | 804 | | 10. salt | - // | 836 | | 11. offset to rightMakerAssetData (*) | - // | 868 | | 12. offset to rightTakerAssetData (*) | - // | 900 | 32 | rightMakerAssetData Length | - // | 932 | a | rightMakerAssetData Contents | - // | 932 + a | 32 | rightTakerAssetData Length | - // | 964 | b | rightTakerAssetData Contents | - // | 964 + b | 32 | rightSignature Length (always 0) | + // | 580 + a + b | | 1. senderAddress | + // | 612 + a + b | | 2. makerAddress | + // | 644 + a + b | | 3. takerAddress | + // | 676 + a + b | | 4. feeRecipientAddress | + // | 708 + a + b | | 5. makerAssetAmount | + // | 740 + a + b | | 6. takerAssetAmount | + // | 772 + a + b | | 7. makerFeeAmount | + // | 804 + a + b | | 8. takerFeeAmount | + // | 836 + a + b | | 9. expirationTimeSeconds | + // | 868 + a + b | | 10. salt | + // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | + // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | + // | 964 + a + b | 32 | rightMakerAssetData Length | + // | 996 + a + b | c | rightMakerAssetData Contents | + // | 996 + a + b + c | 32 | rightTakerAssetData Length | + // | 1028 + a + b + c | d | rightTakerAssetData Contents | + // | 1028 + a + b + c + d | 32 | rightSignature Length (always 0) | // We assume that `leftOrder.makerAssetData == rightOrder.takerAssetData` and `leftOrder.takerAssetData == rightOrder.makerAssetData` // `EXCHANGE.matchOrders` already makes this assumption, so it is likely diff --git a/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol b/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol index 43bbdeec2..bd6a5e0ee 100644 --- a/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol +++ b/contracts/extensions/contracts/OrderMatcher/libs/LibConstants.sol @@ -23,9 +23,12 @@ import "@0x/contracts-interfaces/contracts/protocol/Exchange/IExchange.sol"; contract LibConstants { - bytes4 constant internal ERC20_TRANSFER_SELECTOR = bytes4(keccak256("transfer(address,uint256)")); - bytes4 constant internal ERC20_DATA_ID = bytes4(keccak256("ERC20Token(address)")); - bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256)")); + // bytes4(keccak256("transfer(address,uint256)")) + bytes4 constant internal ERC20_TRANSFER_SELECTOR = 0xa9059cbb; + // bytes4(keccak256("ERC20Token(address)")) + bytes4 constant internal ERC20_DATA_ID = 0xf47261b0; + // bytes4(keccak256("ERC721Token(address,uint256)")) + bytes4 constant internal ERC721_DATA_ID = 0x02571792; // solhint-disable var-name-mixedcase IExchange internal EXCHANGE; -- cgit v1.2.3 From 90fcf59a3257df91653324a48e031bdc080f841b Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 17 Dec 2018 22:07:12 -0800 Subject: Add getOrderInfo check before calling fillOrder --- .../contracts/OrderMatcher/MixinMatchOrders.sol | 322 +++++++++++++-------- .../extensions/test/extensions/order_matcher.ts | 44 ++- 2 files changed, 242 insertions(+), 124 deletions(-) diff --git a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol index c691b732f..b6bc83af9 100644 --- a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol +++ b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol @@ -25,7 +25,7 @@ import "@0x/contracts-utils/contracts/utils/Ownable/Ownable.sol"; contract MixinMatchOrders is Ownable, LibConstants -{ +{ // The below assembly in the fallback function is functionaly equivalent to the following Solidity code: /* /// @dev Match two complementary orders that have a profitable spread. @@ -54,21 +54,33 @@ contract MixinMatchOrders is rightSignature ); - // If a spread was taken, use the spread to fill remaining amount of `rightOrder` - // Only attempt to fill `rightOrder` if a spread was taken and if not already completely filled uint256 leftMakerAssetSpreadAmount = matchedFillResults.leftMakerAssetSpreadAmount; - if (leftMakerAssetSpreadAmount > 0 && matchedFillResults.right.takerAssetFilledAmount < rightOrder.takerAssetAmount) { - // The `assetData` fields of the `rightOrder` could have been null for the `matchOrders` call. We reassign them before calling `fillOrder`. - rightOrder.makerAssetData = leftOrder.takerAssetData; - rightOrder.takerAssetData = leftOrder.makerAssetData; - - // We do not need to pass in a signature since it was already validated in the `matchOrders` call - EXCHANGE.fillOrder( - rightOrder, - leftMakerAssetSpreadAmount, - "" - ); + uint256 rightOrderTakerAssetAmount = rightOrder.takerAssetAmount; + + // Do not attempt to call `fillOrder` if no spread was taken or `rightOrder` has been completely filled + if (leftMakerAssetSpreadAmount == 0 || matchedFillResults.right.takerAssetFilledAmount == rightOrderTakerAssetAmount) { + return; + } + + // The `assetData` fields of the `rightOrder` could have been null for the `matchOrders` call. We reassign them before calling `fillOrder`. + rightOrder.makerAssetData = leftOrder.takerAssetData; + rightOrder.takerAssetData = leftOrder.makerAssetData; + + // Query `rightOrder` info to check if it has been completely filled + // We need to make this check in case the `rightOrder` was partially filled before the `matchOrders` call + OrderInfo memory orderInfo = EXCHANGE.getOrderInfo(rightOrder); + + // Do not attempt to call `fillOrder` if order has been completely filled + if (orderInfo.orderTakerAssetFilledAmount == rightOrderTakerAssetAmount) { + return; } + + // We do not need to pass in a signature since it was already validated in the `matchOrders` call + EXCHANGE.fillOrder( + rightOrder, + leftMakerAssetSpreadAmount, + "" + ); } */ // solhint-disable-next-line payable-fallback @@ -101,7 +113,11 @@ contract MixinMatchOrders is // Copy calldata to memory // The calldata should be identical to the the ABIv2 encoded data required to call `EXCHANGE.matchOrders` - calldatacopy(0, 0, calldatasize()) + calldatacopy( + 0, // copy to memory at 0 + 0, // copy from calldata at 0 + calldatasize() // copy all calldata + ) // At this point, calldata and memory have the following layout: @@ -153,7 +169,7 @@ contract MixinMatchOrders is // | 1092 + a + b + c + d + e | f | rightSignature Contents | // Call `EXCHANGE.matchOrders` - let matchOrdersSuccess := call( + let success := call( gas, // forward all gas exchange, // call address of Exchange contract 0, // transfer 0 wei @@ -163,12 +179,17 @@ contract MixinMatchOrders is 288 // length of output is 288 bytes ) - if iszero(matchOrdersSuccess) { - // Revert with reason if `matchOrders` call was unsuccessful + if iszero(success) { + // Revert with reason if `EXCHANGE.matchOrders` call was unsuccessful + returndatacopy( + 0, // copy to memory at 0 + 0, // copy from return data at 0 + returndatasize() // copy all return data + ) revert(0, returndatasize()) } - // After calling `matchOrders`, the relevant parts of memory are: + // After calling `matchOrders`, the return data is layed out in memory as: // | Offset | Length | Contents | // |---------------------------|---------|-------------------------------------------- | @@ -182,6 +203,67 @@ contract MixinMatchOrders is // | 192 | | 7. right.makerFeePaid | // | 224 | | 8. right.takerFeePaid | // | 256 | | 9. leftMakerAssetSpreadAmount | + + let rightOrderStart := add(calldataload(36), 4) + + // If no spread was taken or if the entire `rightOrderTakerAssetAmount` has been filled, there is no need to call `EXCHANGE.fillOrder`. + if or( + iszero(mload(256)), // iszero(leftMakerAssetSpreadAmount) + eq(mload(160), calldataload(add(rightOrderStart, 160))) // eq(rightOrderTakerAssetFilledAmount, rightOrderTakerAssetAmount) + ) { + return(0, 0) + } + + // We assume that `leftOrder.makerAssetData == rightOrder.takerAssetData` and `leftOrder.takerAssetData == rightOrder.makerAssetData` + // `EXCHANGE.matchOrders` already makes this assumption, so it is likely + // that the `rightMakerAssetData` and `rightTakerAssetData` in calldata are empty + + let leftOrderStart := add(calldataload(4), 4) + + // Calculate locations of `leftMakerAssetData` and `leftTakerAssetData` in calldata + let leftMakerAssetDataStart := add( + leftOrderStart, + calldataload(add(leftOrderStart, 320)) // offset to `leftMakerAssetData` + ) + let leftTakerAssetDataStart := add( + leftOrderStart, + calldataload(add(leftOrderStart, 352)) // offset to `leftTakerAssetData` + ) + + // Load lengths of `leftMakerAssetData` and `leftTakerAssetData` + let leftMakerAssetDataLen := calldataload(leftMakerAssetDataStart) + let leftTakerAssetDataLen := calldataload(leftTakerAssetDataStart) + + // Write offset to `rightMakerAssetData` + mstore(add(rightOrderStart, 320), 384) + + // Write offset to `rightTakerAssetData` + let rightTakerAssetDataOffset := add(416, leftTakerAssetDataLen) + mstore(add(rightOrderStart, 352), rightTakerAssetDataOffset) + + // Copy `leftTakerAssetData` from calldata onto `rightMakerAssetData` in memory + calldatacopy( + add(rightOrderStart, 384), // `rightMakerAssetDataStart` + leftTakerAssetDataStart, + add(leftTakerAssetDataLen, 32) + ) + + // Copy `leftMakerAssetData` from calldata onto `rightTakerAssetData` in memory + calldatacopy( + add(rightOrderStart, rightTakerAssetDataOffset), // `rightTakerAssetDataStart` + leftMakerAssetDataStart, + add(leftMakerAssetDataLen, 32) + ) + + // We must call `EXCHANGE.getOrderInfo(rightOrder)` before calling `EXCHANGE.fillOrder` to ensure that the order + // has not already been entirely filled (which is possible if an order was partially filled before the `matchOrders` call). + // We want the following layout in memory before calling `getOrderInfo`: + + // | Offset | Length | Contents | + // |---------------------------|---------|-------------------------------------------- | + // | 544 + a + b | 4 | `getOrderInfo` function selector | + // | | 3 * 32 | function parameters: | + // | 548 + a + b | | 1. offset to rightOrder (*) | // | | 12 * 32 | rightOrder: | // | 580 + a + b | | 1. senderAddress | // | 612 + a + b | | 2. makerAddress | @@ -195,126 +277,124 @@ contract MixinMatchOrders is // | 868 + a + b | | 10. salt | // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | + // | 964 + a + b | 32 | rightMakerAssetData Length | + // | 996 + a + b | c | rightMakerAssetData Contents | + // | 996 + a + b + c | 32 | rightTakerAssetData Length | + // | 1028 + a + b + c | d | rightTakerAssetData Contents | - let rightOrderStart := add(calldataload(36), 4) + // function selector (4 bytes) + 1 param (32 bytes) must be stored before `rightOrderStart` + let cdStart := sub(rightOrderStart, 36) - // Only call `fillOrder` if a spread was taken and `rightOrder` has not been completely filled - if and( - gt(mload(256), 0), // gt(leftMakerAssetSpreadAmount, 0) - lt(mload(160), calldataload(add(rightOrderStart, 160))) // lt(rightOrderTakerAssetFilledAmount, rightOrderTakerAssetAmount) - ) { - - // We want the following layout in memory before calling `fillOrder`: - - // | Offset | Length | Contents | - // |---------------------------|---------|-------------------------------------------- | - // | 480 + a + b | 4 | `fillOrder` function selector | - // | | 3 * 32 | function parameters: | - // | 484 + a + b | | 1. offset to rightOrder (*) | - // | 516 + a + b | | 2. takerAssetFillAmount | - // | 548 + a + b | | 3. offset to rightSignature (*) | - // | | 12 * 32 | rightOrder: | - // | 580 + a + b | | 1. senderAddress | - // | 612 + a + b | | 2. makerAddress | - // | 644 + a + b | | 3. takerAddress | - // | 676 + a + b | | 4. feeRecipientAddress | - // | 708 + a + b | | 5. makerAssetAmount | - // | 740 + a + b | | 6. takerAssetAmount | - // | 772 + a + b | | 7. makerFeeAmount | - // | 804 + a + b | | 8. takerFeeAmount | - // | 836 + a + b | | 9. expirationTimeSeconds | - // | 868 + a + b | | 10. salt | - // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | - // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | - // | 964 + a + b | 32 | rightMakerAssetData Length | - // | 996 + a + b | c | rightMakerAssetData Contents | - // | 996 + a + b + c | 32 | rightTakerAssetData Length | - // | 1028 + a + b + c | d | rightTakerAssetData Contents | - // | 1028 + a + b + c + d | 32 | rightSignature Length (always 0) | - - // We assume that `leftOrder.makerAssetData == rightOrder.takerAssetData` and `leftOrder.takerAssetData == rightOrder.makerAssetData` - // `EXCHANGE.matchOrders` already makes this assumption, so it is likely - // that the `rightMakerAssetData` and `rightTakerAssetData` in calldata are empty - - let leftOrderStart := add(calldataload(4), 4) - - // Calculate locations of `leftMakerAssetData` and `leftTakerAssetData` in calldata - let leftMakerAssetDataStart := add( - leftOrderStart, - calldataload(add(leftOrderStart, 320)) // offset to `leftMakerAssetData` - ) - let leftTakerAssetDataStart := add( - leftOrderStart, - calldataload(add(leftOrderStart, 352)) // offset to `leftTakerAssetData` - ) + // `getOrderInfo` selector = 0xc75e0a81 + mstore(cdStart, 0xc75e0a8100000000000000000000000000000000000000000000000000000000) - // Load lengths of `leftMakerAssetData` and `leftTakerAssetData` - let leftMakerAssetDataLen := calldataload(leftMakerAssetDataStart) - let leftTakerAssetDataLen := calldataload(leftTakerAssetDataStart) + // Write offset to `rightOrder` + mstore(add(cdStart, 4), 32) - // Write offset to `rightMakerAssetData` - mstore(add(rightOrderStart, 320), 384) + let rightOrderEnd := add( + add(rightOrderStart, 484), + add(leftMakerAssetDataLen, leftTakerAssetDataLen) + ) - // Write offset to `rightTakerAssetData` - let rightTakerAssetDataOffset := add(416, leftTakerAssetDataLen) - mstore(add(rightOrderStart, 352), rightTakerAssetDataOffset) + // Call `EXCHANGE.getOrderInfo(rightOrder)` + success := staticcall( + gas, // forward all gas + exchange, // call address of Exchange contract + cdStart, // start of input + sub(rightOrderEnd, cdStart), // length of input + 0, // write output over old output + 96 // output is 96 bytes + ) - // Copy `leftTakerAssetData` from calldata onto `rightMakerAssetData` in memory - calldatacopy( - add(rightOrderStart, 384), // `rightMakerAssetDataStart` - leftTakerAssetDataStart, - add(leftTakerAssetDataLen, 32) - ) + // After calling `EXCHANGE.getOrderInfo`, the return data is layed out in memory as: - // Copy `leftMakerAssetData` from calldata onto `rightTakerAssetData` in memory - calldatacopy( - add(rightOrderStart, rightTakerAssetDataOffset), // `rightTakerAssetDataStart` - leftMakerAssetDataStart, - add(leftMakerAssetDataLen, 32) - ) + // | Offset | Length | Contents | + // |---------------------------|---------|-------------------------------------------- | + // | | 3 * 32 | orderInfo | + // | 0 | | 1. orderStatus | + // | 32 | | 2. orderHash | + // | 64 | | 3. orderTakerAssetFilledAmount | + + // We do not need to check if the `getOrderInfo` call was successful because it has no possibility of reverting + // If order has been entirely filled, there is no need to call `EXCHANGE.fillOrder` + // eq(orderTakerAssetFilledAmount, rightOrderTakerAssetAmount) + if eq(mload(64), calldataload(add(rightOrderStart, 160))) { + return(0, 0) + } - // Write length of signature (always 0 since signature was previously validated) - let rightSignatureStart := add( - add(rightOrderStart, rightTakerAssetDataOffset), // `rightTakerAssetDataStart` - add(leftMakerAssetDataLen, 32) - ) - mstore(rightSignatureStart, 0) + // We want the following layout in memory before calling `EXCHANGE.fillOrder`: - // function selector (4 bytes) + 3 params (3 * 32 bytes) must be stored before `rightOrderStart` - let cdStart := sub(rightOrderStart, 100) + // | Offset | Length | Contents | + // |---------------------------|---------|-------------------------------------------- | + // | 480 + a + b | 4 | `fillOrder` function selector | + // | | 3 * 32 | function parameters: | + // | 484 + a + b | | 1. offset to rightOrder (*) | + // | 516 + a + b | | 2. takerAssetFillAmount | + // | 548 + a + b | | 3. offset to rightSignature (*) | + // | | 12 * 32 | rightOrder: | + // | 580 + a + b | | 1. senderAddress | + // | 612 + a + b | | 2. makerAddress | + // | 644 + a + b | | 3. takerAddress | + // | 676 + a + b | | 4. feeRecipientAddress | + // | 708 + a + b | | 5. makerAssetAmount | + // | 740 + a + b | | 6. takerAssetAmount | + // | 772 + a + b | | 7. makerFeeAmount | + // | 804 + a + b | | 8. takerFeeAmount | + // | 836 + a + b | | 9. expirationTimeSeconds | + // | 868 + a + b | | 10. salt | + // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | + // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | + // | 964 + a + b | 32 | rightMakerAssetData Length | + // | 996 + a + b | c | rightMakerAssetData Contents | + // | 996 + a + b + c | 32 | rightTakerAssetData Length | + // | 1028 + a + b + c | d | rightTakerAssetData Contents | + // | 1028 + a + b + c + d | 32 | rightSignature Length (always 0) | - // `fillOrder` selector = 0xb4be83d5 - mstore(cdStart, 0xb4be83d500000000000000000000000000000000000000000000000000000000) + // Write length of signature (always 0 since signature was previously validated) + mstore(rightOrderEnd, 0) - // Write offset to `rightOrder` - mstore(add(cdStart, 4), 96) + // function selector (4 bytes) + 3 params (3 * 32 bytes) must be stored before `rightOrderStart` + cdStart := sub(rightOrderStart, 100) - // Write `takerAssetFillAmount`, which will be the `leftMakerAssetSpreadAmount` received from the `matchOrders` call - mstore(add(cdStart, 36), mload(256)) + // `fillOrder` selector = 0xb4be83d5 + mstore(cdStart, 0xb4be83d500000000000000000000000000000000000000000000000000000000) - // Write offset to `rightSignature` - mstore(add(cdStart, 68), sub(rightSignatureStart, add(cdStart, 4))) + // Write offset to `rightOrder` + mstore(add(cdStart, 4), 96) - let fillOrderSuccess := call( - gas, // forward all gas - exchange, // call address of Exchange contract - 0, // transfer 0 wei - cdStart, // start of input - sub(add(rightSignatureStart, 32), cdStart), // length of input is end - start - 0, // write output over input - 128 // length of output is 128 bytes - ) + // Write `takerAssetFillAmount`, which will be the `leftMakerAssetSpreadAmount` received from the `matchOrders` call + mstore(add(cdStart, 36), mload(256)) - if fillOrderSuccess { - return(0, 0) - } - - // Revert with reason if `fillOrder` call was unsuccessful + // Write offset to `rightSignature` + mstore( + add(cdStart, 68), + sub(rightOrderEnd, add(cdStart, 4)) + ) + + // Call `EXCHANGE.fillOrder(rightOrder, leftMakerAssetSpreadAmount, "")` + success := call( + gas, // forward all gas + exchange, // call address of Exchange contract + 0, // transfer 0 wei + cdStart, // start of input + add(sub(rightOrderEnd, cdStart), 32), // length of input is end - start + 0, // write output over input + 0 // do not write output to memory + ) + + if iszero(success) { + // Revert with reason if `EXCHANGE.fillOrder` call was unsuccessful + returndatacopy( + 0, // copy to memory at 0 + 0, // copy from return data at 0 + returndatasize() // copy all return data + ) revert(0, returndatasize()) } - - // Return if `matchOrders` call successful and `fillOrder` was not called + + // Return if `EXCHANGE.fillOrder` did not revert return(0, 0) + } // Revert if undefined function is called diff --git a/contracts/extensions/test/extensions/order_matcher.ts b/contracts/extensions/test/extensions/order_matcher.ts index 45002b324..acb46ced4 100644 --- a/contracts/extensions/test/extensions/order_matcher.ts +++ b/contracts/extensions/test/extensions/order_matcher.ts @@ -435,7 +435,7 @@ describe('OrderMatcher', () => { initialLeftTakerAssetTakerBalance.plus(expectedTransferAmounts.leftTakerAssetSpreadAmount), ); }); - it('should not call fillOrder when rightOrder is completely filled after matchOrders call', async () => { + it('should not call fillOrder when rightOrder is completely filled after matchOrders call and orders were never partially filled', async () => { // Create orders to match const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), @@ -443,7 +443,45 @@ describe('OrderMatcher', () => { }); const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), - takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + const data = exchange.matchOrders.getABIEncodedTransactionData( + signedOrderLeft, + signedOrderRight, + signedOrderLeft.signature, + signedOrderRight.signature, + ); + const logDecoder = new LogDecoder(web3Wrapper, { ...artifacts, ...tokenArtifacts, ...protocolArtifacts }); + const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( + await web3Wrapper.sendTransactionAsync({ + data, + to: orderMatcher.address, + from: owner, + gas: constants.MAX_MATCH_ORDERS_GAS, + }), + ); + const fillLogs = _.filter( + txReceipt.logs, + log => (log as LogWithDecodedArgs).event === 'Fill', + ); + // Only 2 Fill logs should exist for `matchOrders` call. `fillOrder` should not have been called and should not have emitted a Fill event. + expect(fillLogs.length).to.be.equal(2); + }); + it('should not call fillOrder when rightOrder is completely filled after matchOrders call and orders were initially partially filled', async () => { + // Create orders to match + const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(5), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + }); + const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({ + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 18), + }); + await exchangeWrapper.fillOrderAsync(signedOrderLeft, takerAddress, { + takerAssetFillAmount: signedOrderLeft.takerAssetAmount.dividedToIntegerBy(5), + }); + await exchangeWrapper.fillOrderAsync(signedOrderRight, takerAddress, { + takerAssetFillAmount: signedOrderRight.takerAssetAmount.dividedToIntegerBy(5), }); const data = exchange.matchOrders.getABIEncodedTransactionData( signedOrderLeft, @@ -451,7 +489,7 @@ describe('OrderMatcher', () => { signedOrderLeft.signature, signedOrderRight.signature, ); - const logDecoder = new LogDecoder(web3Wrapper, { ...artifacts, ...tokenArtifacts }); + const logDecoder = new LogDecoder(web3Wrapper, { ...artifacts, ...tokenArtifacts, ...protocolArtifacts }); const txReceipt = await logDecoder.getTxWithDecodedLogsAsync( await web3Wrapper.sendTransactionAsync({ data, -- cgit v1.2.3 From 58be23ac9798dfb0c8dd5146aa41872b03dff174 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 19 Dec 2018 14:15:25 -0800 Subject: Remove assembly version of matchOrders --- .../contracts/OrderMatcher/MixinMatchOrders.sol | 420 +++------------------ .../contracts/OrderMatcher/OrderMatcher.sol | 1 + 2 files changed, 52 insertions(+), 369 deletions(-) diff --git a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol index b6bc83af9..f75cecdc1 100644 --- a/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol +++ b/contracts/extensions/contracts/OrderMatcher/MixinMatchOrders.sol @@ -17,8 +17,11 @@ */ pragma solidity 0.4.24; +pragma experimental ABIEncoderV2; import "./libs/LibConstants.sol"; +import "@0x/contracts-libs/contracts/libs/LibOrder.sol"; +import "@0x/contracts-libs/contracts/libs/LibFillResults.sol"; import "@0x/contracts-utils/contracts/utils/Ownable/Ownable.sol"; @@ -26,379 +29,58 @@ contract MixinMatchOrders is Ownable, LibConstants { - // The below assembly in the fallback function is functionaly equivalent to the following Solidity code: - /* - /// @dev Match two complementary orders that have a profitable spread. - /// Each order is filled at their respective price point. However, the calculations are - /// carried out as though the orders are both being filled at the right order's price point. - /// The profit made by the left order is then used to fill the right order as much as possible. - /// This results in a spread being taken in terms of both assets. The spread is held within this contract. - /// @param leftOrder First order to match. - /// @param rightOrder Second order to match. - /// @param leftSignature Proof that order was created by the left maker. - /// @param rightSignature Proof that order was created by the right maker. - function matchOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - bytes memory leftSignature, - bytes memory rightSignature - ) - public - onlyOwner - { - // Match orders, maximally filling `leftOrder` - LibFillResults.MatchedFillResults memory matchedFillResults = EXCHANGE.matchOrders( - leftOrder, - rightOrder, - leftSignature, - rightSignature - ); - - uint256 leftMakerAssetSpreadAmount = matchedFillResults.leftMakerAssetSpreadAmount; - uint256 rightOrderTakerAssetAmount = rightOrder.takerAssetAmount; - - // Do not attempt to call `fillOrder` if no spread was taken or `rightOrder` has been completely filled - if (leftMakerAssetSpreadAmount == 0 || matchedFillResults.right.takerAssetFilledAmount == rightOrderTakerAssetAmount) { - return; - } - - // The `assetData` fields of the `rightOrder` could have been null for the `matchOrders` call. We reassign them before calling `fillOrder`. - rightOrder.makerAssetData = leftOrder.takerAssetData; - rightOrder.takerAssetData = leftOrder.makerAssetData; - - // Query `rightOrder` info to check if it has been completely filled - // We need to make this check in case the `rightOrder` was partially filled before the `matchOrders` call - OrderInfo memory orderInfo = EXCHANGE.getOrderInfo(rightOrder); - - // Do not attempt to call `fillOrder` if order has been completely filled - if (orderInfo.orderTakerAssetFilledAmount == rightOrderTakerAssetAmount) { - return; - } - - // We do not need to pass in a signature since it was already validated in the `matchOrders` call - EXCHANGE.fillOrder( - rightOrder, - leftMakerAssetSpreadAmount, - "" - ); - } - */ - // solhint-disable-next-line payable-fallback - function () - external + /// @dev Match two complementary orders that have a profitable spread. + /// Each order is filled at their respective price point. However, the calculations are + /// carried out as though the orders are both being filled at the right order's price point. + /// The profit made by the left order is then used to fill the right order as much as possible. + /// This results in a spread being taken in terms of both assets. The spread is held within this contract. + /// @param leftOrder First order to match. + /// @param rightOrder Second order to match. + /// @param leftSignature Proof that order was created by the left maker. + /// @param rightSignature Proof that order was created by the right maker. + function matchOrders( + LibOrder.Order memory leftOrder, + LibOrder.Order memory rightOrder, + bytes memory leftSignature, + bytes memory rightSignature + ) + public + onlyOwner { - assembly { - // The first 4 bytes of calldata holds the function selector - // `matchOrders` selector = 0x3c28d861 - if eq( - and(calldataload(0), 0xffffffff00000000000000000000000000000000000000000000000000000000), - 0x3c28d86100000000000000000000000000000000000000000000000000000000 - ) { - - // Load address of `owner` - let owner := sload(owner_slot) - - // Revert if `msg.sender` != `owner` - if iszero(eq(owner, caller)) { - // Revert with `Error("ONLY_CONTRACT_OWNER")` - mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) - mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) - mstore(64, 0x000000134f4e4c595f434f4e54524143545f4f574e4552000000000000000000) - mstore(96, 0) - revert(0, 100) - } - - // Load address of Exchange contract - let exchange := sload(EXCHANGE_slot) - - // Copy calldata to memory - // The calldata should be identical to the the ABIv2 encoded data required to call `EXCHANGE.matchOrders` - calldatacopy( - 0, // copy to memory at 0 - 0, // copy from calldata at 0 - calldatasize() // copy all calldata - ) - - // At this point, calldata and memory have the following layout: - - // | Offset | Length | Contents | - // |---------------------------|---------|-------------------------------------------- | - // | 0 | 4 | `matchOrders` function selector | - // | | 4 * 32 | function parameters: | - // | 4 | | 1. offset to leftOrder (*) | - // | 36 | | 2. offset to rightOrder (*) | - // | 68 | | 3. offset to leftSignature (*) | - // | 100 | | 4. offset to rightSignature (*) | - // | | 12 * 32 | leftOrder: | - // | 132 | | 1. senderAddress | - // | 164 | | 2. makerAddress | - // | 196 | | 3. takerAddress | - // | 228 | | 4. feeRecipientAddress | - // | 260 | | 5. makerAssetAmount | - // | 292 | | 6. takerAssetAmount | - // | 324 | | 7. makerFeeAmount | - // | 356 | | 8. takerFeeAmount | - // | 388 | | 9. expirationTimeSeconds | - // | 420 | | 10. salt | - // | 452 | | 11. offset to leftMakerAssetData (*) | - // | 484 | | 12. offset to leftTakerAssetData (*) | - // | 516 | 32 | leftMakerAssetData Length | - // | 548 | a | leftMakerAssetData Contents | - // | 548 + a | 32 | leftTakerAssetData Length | - // | 580 + a | b | leftTakerAssetData Contents | - // | | 12 * 32 | rightOrder: | - // | 580 + a + b | | 1. senderAddress | - // | 612 + a + b | | 2. makerAddress | - // | 644 + a + b | | 3. takerAddress | - // | 676 + a + b | | 4. feeRecipientAddress | - // | 708 + a + b | | 5. makerAssetAmount | - // | 740 + a + b | | 6. takerAssetAmount | - // | 772 + a + b | | 7. makerFeeAmount | - // | 804 + a + b | | 8. takerFeeAmount | - // | 836 + a + b | | 9. expirationTimeSeconds | - // | 868 + a + b | | 10. salt | - // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | - // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | - // | 964 + a + b | 32 | rightMakerAssetData Length | - // | 996 + a + b | c | rightMakerAssetData Contents | - // | 996 + a + b + c | 32 | rightTakerAssetData Length | - // | 1028 + a + b + c | d | rightTakerAssetData Contents | - // | 1028 + a + b + c + d | 32 | leftSignature Length | - // | 1060 + a + b + c + d | e | leftSignature Contents | - // | 1060 + a + b + c + d + e | 32 | rightSignature Length | - // | 1092 + a + b + c + d + e | f | rightSignature Contents | - - // Call `EXCHANGE.matchOrders` - let success := call( - gas, // forward all gas - exchange, // call address of Exchange contract - 0, // transfer 0 wei - 0, // input starts at 0 - calldatasize(), // length of input - 0, // write output over input - 288 // length of output is 288 bytes - ) - - if iszero(success) { - // Revert with reason if `EXCHANGE.matchOrders` call was unsuccessful - returndatacopy( - 0, // copy to memory at 0 - 0, // copy from return data at 0 - returndatasize() // copy all return data - ) - revert(0, returndatasize()) - } - - // After calling `matchOrders`, the return data is layed out in memory as: - - // | Offset | Length | Contents | - // |---------------------------|---------|-------------------------------------------- | - // | | 9 * 32 | matchedFillResults | - // | 0 | | 1. left.makerAssetFilledAmount | - // | 32 | | 2. left.takerAssetFilledAmount | - // | 64 | | 3. left.makerFeePaid | - // | 96 | | 4. left.takerFeePaid | - // | 128 | | 5. right.makerAssetFilledAmount | - // | 160 | | 6. right.takerAssetFilledAmount | - // | 192 | | 7. right.makerFeePaid | - // | 224 | | 8. right.takerFeePaid | - // | 256 | | 9. leftMakerAssetSpreadAmount | - - let rightOrderStart := add(calldataload(36), 4) - - // If no spread was taken or if the entire `rightOrderTakerAssetAmount` has been filled, there is no need to call `EXCHANGE.fillOrder`. - if or( - iszero(mload(256)), // iszero(leftMakerAssetSpreadAmount) - eq(mload(160), calldataload(add(rightOrderStart, 160))) // eq(rightOrderTakerAssetFilledAmount, rightOrderTakerAssetAmount) - ) { - return(0, 0) - } - - // We assume that `leftOrder.makerAssetData == rightOrder.takerAssetData` and `leftOrder.takerAssetData == rightOrder.makerAssetData` - // `EXCHANGE.matchOrders` already makes this assumption, so it is likely - // that the `rightMakerAssetData` and `rightTakerAssetData` in calldata are empty - - let leftOrderStart := add(calldataload(4), 4) - - // Calculate locations of `leftMakerAssetData` and `leftTakerAssetData` in calldata - let leftMakerAssetDataStart := add( - leftOrderStart, - calldataload(add(leftOrderStart, 320)) // offset to `leftMakerAssetData` - ) - let leftTakerAssetDataStart := add( - leftOrderStart, - calldataload(add(leftOrderStart, 352)) // offset to `leftTakerAssetData` - ) - - // Load lengths of `leftMakerAssetData` and `leftTakerAssetData` - let leftMakerAssetDataLen := calldataload(leftMakerAssetDataStart) - let leftTakerAssetDataLen := calldataload(leftTakerAssetDataStart) - - // Write offset to `rightMakerAssetData` - mstore(add(rightOrderStart, 320), 384) - - // Write offset to `rightTakerAssetData` - let rightTakerAssetDataOffset := add(416, leftTakerAssetDataLen) - mstore(add(rightOrderStart, 352), rightTakerAssetDataOffset) - - // Copy `leftTakerAssetData` from calldata onto `rightMakerAssetData` in memory - calldatacopy( - add(rightOrderStart, 384), // `rightMakerAssetDataStart` - leftTakerAssetDataStart, - add(leftTakerAssetDataLen, 32) - ) - - // Copy `leftMakerAssetData` from calldata onto `rightTakerAssetData` in memory - calldatacopy( - add(rightOrderStart, rightTakerAssetDataOffset), // `rightTakerAssetDataStart` - leftMakerAssetDataStart, - add(leftMakerAssetDataLen, 32) - ) - - // We must call `EXCHANGE.getOrderInfo(rightOrder)` before calling `EXCHANGE.fillOrder` to ensure that the order - // has not already been entirely filled (which is possible if an order was partially filled before the `matchOrders` call). - // We want the following layout in memory before calling `getOrderInfo`: - - // | Offset | Length | Contents | - // |---------------------------|---------|-------------------------------------------- | - // | 544 + a + b | 4 | `getOrderInfo` function selector | - // | | 3 * 32 | function parameters: | - // | 548 + a + b | | 1. offset to rightOrder (*) | - // | | 12 * 32 | rightOrder: | - // | 580 + a + b | | 1. senderAddress | - // | 612 + a + b | | 2. makerAddress | - // | 644 + a + b | | 3. takerAddress | - // | 676 + a + b | | 4. feeRecipientAddress | - // | 708 + a + b | | 5. makerAssetAmount | - // | 740 + a + b | | 6. takerAssetAmount | - // | 772 + a + b | | 7. makerFeeAmount | - // | 804 + a + b | | 8. takerFeeAmount | - // | 836 + a + b | | 9. expirationTimeSeconds | - // | 868 + a + b | | 10. salt | - // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | - // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | - // | 964 + a + b | 32 | rightMakerAssetData Length | - // | 996 + a + b | c | rightMakerAssetData Contents | - // | 996 + a + b + c | 32 | rightTakerAssetData Length | - // | 1028 + a + b + c | d | rightTakerAssetData Contents | - - // function selector (4 bytes) + 1 param (32 bytes) must be stored before `rightOrderStart` - let cdStart := sub(rightOrderStart, 36) - - // `getOrderInfo` selector = 0xc75e0a81 - mstore(cdStart, 0xc75e0a8100000000000000000000000000000000000000000000000000000000) - - // Write offset to `rightOrder` - mstore(add(cdStart, 4), 32) - - let rightOrderEnd := add( - add(rightOrderStart, 484), - add(leftMakerAssetDataLen, leftTakerAssetDataLen) - ) - - // Call `EXCHANGE.getOrderInfo(rightOrder)` - success := staticcall( - gas, // forward all gas - exchange, // call address of Exchange contract - cdStart, // start of input - sub(rightOrderEnd, cdStart), // length of input - 0, // write output over old output - 96 // output is 96 bytes - ) - - // After calling `EXCHANGE.getOrderInfo`, the return data is layed out in memory as: - - // | Offset | Length | Contents | - // |---------------------------|---------|-------------------------------------------- | - // | | 3 * 32 | orderInfo | - // | 0 | | 1. orderStatus | - // | 32 | | 2. orderHash | - // | 64 | | 3. orderTakerAssetFilledAmount | - - // We do not need to check if the `getOrderInfo` call was successful because it has no possibility of reverting - // If order has been entirely filled, there is no need to call `EXCHANGE.fillOrder` - // eq(orderTakerAssetFilledAmount, rightOrderTakerAssetAmount) - if eq(mload(64), calldataload(add(rightOrderStart, 160))) { - return(0, 0) - } - - // We want the following layout in memory before calling `EXCHANGE.fillOrder`: - - // | Offset | Length | Contents | - // |---------------------------|---------|-------------------------------------------- | - // | 480 + a + b | 4 | `fillOrder` function selector | - // | | 3 * 32 | function parameters: | - // | 484 + a + b | | 1. offset to rightOrder (*) | - // | 516 + a + b | | 2. takerAssetFillAmount | - // | 548 + a + b | | 3. offset to rightSignature (*) | - // | | 12 * 32 | rightOrder: | - // | 580 + a + b | | 1. senderAddress | - // | 612 + a + b | | 2. makerAddress | - // | 644 + a + b | | 3. takerAddress | - // | 676 + a + b | | 4. feeRecipientAddress | - // | 708 + a + b | | 5. makerAssetAmount | - // | 740 + a + b | | 6. takerAssetAmount | - // | 772 + a + b | | 7. makerFeeAmount | - // | 804 + a + b | | 8. takerFeeAmount | - // | 836 + a + b | | 9. expirationTimeSeconds | - // | 868 + a + b | | 10. salt | - // | 900 + a + b | | 11. offset to rightMakerAssetData (*) | - // | 932 + a + b | | 12. offset to rightTakerAssetData (*) | - // | 964 + a + b | 32 | rightMakerAssetData Length | - // | 996 + a + b | c | rightMakerAssetData Contents | - // | 996 + a + b + c | 32 | rightTakerAssetData Length | - // | 1028 + a + b + c | d | rightTakerAssetData Contents | - // | 1028 + a + b + c + d | 32 | rightSignature Length (always 0) | - - // Write length of signature (always 0 since signature was previously validated) - mstore(rightOrderEnd, 0) - - // function selector (4 bytes) + 3 params (3 * 32 bytes) must be stored before `rightOrderStart` - cdStart := sub(rightOrderStart, 100) - - // `fillOrder` selector = 0xb4be83d5 - mstore(cdStart, 0xb4be83d500000000000000000000000000000000000000000000000000000000) - - // Write offset to `rightOrder` - mstore(add(cdStart, 4), 96) - - // Write `takerAssetFillAmount`, which will be the `leftMakerAssetSpreadAmount` received from the `matchOrders` call - mstore(add(cdStart, 36), mload(256)) - - // Write offset to `rightSignature` - mstore( - add(cdStart, 68), - sub(rightOrderEnd, add(cdStart, 4)) - ) + // Match orders, maximally filling `leftOrder` + LibFillResults.MatchedFillResults memory matchedFillResults = EXCHANGE.matchOrders( + leftOrder, + rightOrder, + leftSignature, + rightSignature + ); + + uint256 leftMakerAssetSpreadAmount = matchedFillResults.leftMakerAssetSpreadAmount; + uint256 rightOrderTakerAssetAmount = rightOrder.takerAssetAmount; + + // Do not attempt to call `fillOrder` if no spread was taken or `rightOrder` has been completely filled + if (leftMakerAssetSpreadAmount == 0 || matchedFillResults.right.takerAssetFilledAmount == rightOrderTakerAssetAmount) { + return; + } - // Call `EXCHANGE.fillOrder(rightOrder, leftMakerAssetSpreadAmount, "")` - success := call( - gas, // forward all gas - exchange, // call address of Exchange contract - 0, // transfer 0 wei - cdStart, // start of input - add(sub(rightOrderEnd, cdStart), 32), // length of input is end - start - 0, // write output over input - 0 // do not write output to memory - ) + // The `assetData` fields of the `rightOrder` could have been null for the `matchOrders` call. We reassign them before calling `fillOrder`. + rightOrder.makerAssetData = leftOrder.takerAssetData; + rightOrder.takerAssetData = leftOrder.makerAssetData; - if iszero(success) { - // Revert with reason if `EXCHANGE.fillOrder` call was unsuccessful - returndatacopy( - 0, // copy to memory at 0 - 0, // copy from return data at 0 - returndatasize() // copy all return data - ) - revert(0, returndatasize()) - } - - // Return if `EXCHANGE.fillOrder` did not revert - return(0, 0) - - } + // Query `rightOrder` info to check if it has been completely filled + // We need to make this check in case the `rightOrder` was partially filled before the `matchOrders` call + LibOrder.OrderInfo memory orderInfo = EXCHANGE.getOrderInfo(rightOrder); - // Revert if undefined function is called - revert(0, 0) + // Do not attempt to call `fillOrder` if order has been completely filled + if (orderInfo.orderTakerAssetFilledAmount == rightOrderTakerAssetAmount) { + return; } + + // We do not need to pass in a signature since it was already validated in the `matchOrders` call + EXCHANGE.fillOrder( + rightOrder, + leftMakerAssetSpreadAmount, + "" + ); } } diff --git a/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol b/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol index 7136c8c82..4879b7bca 100644 --- a/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol +++ b/contracts/extensions/contracts/OrderMatcher/OrderMatcher.sol @@ -17,6 +17,7 @@ */ pragma solidity 0.4.24; +pragma experimental ABIEncoderV2; import "@0x/contracts-utils/contracts/utils/Ownable/Ownable.sol"; import "./libs/LibConstants.sol"; -- cgit v1.2.3 From fbfb6eb45e60aa1439162559df16444344322092 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 20 Dec 2018 09:13:19 -0800 Subject: Update CHANGELOG --- contracts/extensions/CHANGELOG.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/contracts/extensions/CHANGELOG.json b/contracts/extensions/CHANGELOG.json index da4d9c2ba..4f2b54988 100644 --- a/contracts/extensions/CHANGELOG.json +++ b/contracts/extensions/CHANGELOG.json @@ -5,6 +5,10 @@ { "note": "Added Balance Threshold Filter", "pr": 1383 + }, + { + "note": "Add OrderMatcher", + "pr": 1117 } ] }, -- cgit v1.2.3