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