From b9d8d2d5e3e7aa5f14ae2fd2019460890497bb7e Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Thu, 9 Aug 2018 11:20:06 -0700 Subject: Make marketBuy functions revert if entire amount not filled --- .../src/2.0.0/forwarder/MixinExchangeWrapper.sol | 16 +++++-- .../src/2.0.0/forwarder/MixinForwarderCore.sol | 53 +++------------------- .../contracts/src/2.0.0/forwarder/MixinWeth.sol | 12 +++-- .../2.0.0/forwarder/mixins/MExchangeWrapper.sol | 4 +- .../src/2.0.0/forwarder/mixins/MForwarderCore.sol | 42 ----------------- 5 files changed, 29 insertions(+), 98 deletions(-) delete mode 100644 packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol (limited to 'packages/contracts/src/2.0.0') diff --git a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol index f3aa483c5..4584bb840 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinExchangeWrapper.sol @@ -139,7 +139,7 @@ contract MixinExchangeWrapper is /// @param makerAssetFillAmount Desired amount of makerAsset to buy. /// @param signatures Proofs that orders have been signed by makers. /// @return Amounts filled and fees paid by makers and taker. - function marketBuyWithWeth( + function marketBuyExactAmountWithWeth( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, bytes[] memory signatures @@ -180,10 +180,16 @@ contract MixinExchangeWrapper is addFillResults(totalFillResults, singleFillResults); // Stop execution if the entire amount of makerAsset has been bought - if (totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount) { + uint256 makerAssetFilledAmount = totalFillResults.makerAssetFilledAmount; + if (makerAssetFilledAmount >= makerAssetFillAmount) { break; } } + + require( + makerAssetFilledAmount >= makerAssetFillAmount, + "COMPLETE_FILL_FAILED" + ); return totalFillResults; } @@ -196,7 +202,7 @@ contract MixinExchangeWrapper is /// @param zrxBuyAmount Desired amount of ZRX to buy. /// @param signatures Proofs that orders have been created by makers. /// @return totalFillResults Amounts filled and fees paid by maker and taker. - function marketBuyZrxWithWeth( + function marketBuyExactZrxWithWeth( LibOrder.Order[] memory orders, uint256 zrxBuyAmount, bytes[] memory signatures @@ -248,6 +254,10 @@ contract MixinExchangeWrapper is } } + require( + zrxPurchased >= zrxBuyAmount, + "COMPLETE_FILL_FAILED" + ); return totalFillResults; } } diff --git a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol index 1164ae919..0d313ea91 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol @@ -23,7 +23,7 @@ import "./libs/LibConstants.sol"; import "./mixins/MWeth.sol"; import "./mixins/MAssets.sol"; import "./mixins/MExchangeWrapper.sol"; -import "./mixins/MForwarderCore.sol"; +import "./interfaces/IForwarderCore.sol"; import "../utils/LibBytes/LibBytes.sol"; import "../protocol/Exchange/libs/LibOrder.sol"; import "../protocol/Exchange/libs/LibFillResults.sol"; @@ -37,7 +37,7 @@ contract MixinForwarderCore is MWeth, MAssets, MExchangeWrapper, - MForwarderCore + IForwarderCore { using LibBytes for bytes; @@ -117,7 +117,7 @@ contract MixinForwarderCore is ); // Buy back all ZRX spent on fees. zrxBuyAmount = orderFillResults.takerFeePaid; - feeOrderFillResults = marketBuyZrxWithWeth( + feeOrderFillResults = marketBuyExactZrxWithWeth( feeOrders, zrxBuyAmount, feeSignatures @@ -125,13 +125,6 @@ contract MixinForwarderCore is makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; } - // Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - assertValidFillResults( - orderFillResults, - feeOrderFillResults, - zrxBuyAmount - ); - // Transfer feePercentage of total ETH spent on primary orders to feeRecipient. // Refund remaining ETH to msg.sender. transferEthFeeAndRefund( @@ -180,7 +173,7 @@ contract MixinForwarderCore is if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { // If the makerAsset is ZRX, it is not necessary to pay fees out of this // contracts's ZRX balance because fees are factored into the price of the order. - orderFillResults = marketBuyZrxWithWeth( + orderFillResults = marketBuyExactZrxWithWeth( orders, makerAssetFillAmount, signatures @@ -190,14 +183,14 @@ contract MixinForwarderCore is } else { // Attemp to purchase desired amount of makerAsset. // ZRX fees are payed with this contract's balance. - orderFillResults = marketBuyWithWeth( + orderFillResults = marketBuyExactAmountWithWeth( orders, makerAssetFillAmount, signatures ); // Buy back all ZRX spent on fees. zrxBuyAmount = orderFillResults.takerFeePaid; - feeOrderFillResults = marketBuyZrxWithWeth( + feeOrderFillResults = marketBuyExactZrxWithWeth( feeOrders, zrxBuyAmount, feeSignatures @@ -205,13 +198,6 @@ contract MixinForwarderCore is makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; } - // Ensure that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - assertValidFillResults( - orderFillResults, - feeOrderFillResults, - zrxBuyAmount - ); - // Transfer feePercentage of total ETH spent on primary orders to feeRecipient. // Refund remaining ETH to msg.sender. transferEthFeeAndRefund( @@ -224,31 +210,4 @@ contract MixinForwarderCore is // Transfer purchased assets to msg.sender. transferPurchasedAssetToSender(orders[0].makerAssetData, makerAssetAmountPurchased); } - - /// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - /// @param orderFillResults Amounts filled and fees paid for primary orders. - /// @param feeOrderFillResults Amounts filled and fees paid for fee orders. - /// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders. - function assertValidFillResults( - FillResults memory orderFillResults, - FillResults memory feeOrderFillResults, - uint256 zrxBuyAmount - ) - internal - view - { - // Ensure that all ZRX spent while filling primary orders has been repurchased. - uint256 zrxPurchased = safeSub(feeOrderFillResults.makerAssetFilledAmount, feeOrderFillResults.takerFeePaid); - require( - zrxPurchased >= zrxBuyAmount, - "COMPLETE_FILL_FAILED" - ); - - // Ensure that no extra WETH owned by this contract has been sold. - uint256 wethSold = safeAdd(orderFillResults.takerAssetFilledAmount, feeOrderFillResults.takerAssetFilledAmount); - require( - wethSold <= msg.value, - "OVERSOLD_WETH" - ); - } } diff --git a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol index 8ba236e7f..e07940776 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinWeth.sol @@ -71,12 +71,16 @@ contract MixinWeth is "FEE_PERCENTAGE_TOO_LARGE" ); - // Calculate amount of WETH that hasn't been sold. - uint256 wethRemaining = safeSub( - msg.value, - safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx) + // Ensure that no extra WETH owned by this contract has been sold. + uint256 wethSold = safeAdd(wethSoldExcludingFeeOrders, wethSoldForZrx); + require( + wethSold <= msg.value, + "OVERSOLD_WETH" ); + // Calculate amount of WETH that hasn't been sold. + uint256 wethRemaining = safeSub(msg.value, wethSold); + // Calculate ETH fee to pay to feeRecipient. uint256 ethFee = getPartialAmount( feePercentage, diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol index 5a2def7e5..360dea0e4 100644 --- a/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/forwarder/mixins/MExchangeWrapper.sol @@ -60,7 +60,7 @@ contract MExchangeWrapper { /// @param makerAssetFillAmount Desired amount of makerAsset to buy. /// @param signatures Proofs that orders have been signed by makers. /// @return Amounts filled and fees paid by makers and taker. - function marketBuyWithWeth( + function marketBuyExactAmountWithWeth( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, bytes[] memory signatures @@ -77,7 +77,7 @@ contract MExchangeWrapper { /// @param zrxBuyAmount Desired amount of ZRX to buy. /// @param signatures Proofs that orders have been created by makers. /// @return totalFillResults Amounts filled and fees paid by maker and taker. - function marketBuyZrxWithWeth( + function marketBuyExactZrxWithWeth( LibOrder.Order[] memory orders, uint256 zrxBuyAmount, bytes[] memory signatures diff --git a/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol deleted file mode 100644 index 0f5cd9c66..000000000 --- a/packages/contracts/src/2.0.0/forwarder/mixins/MForwarderCore.sol +++ /dev/null @@ -1,42 +0,0 @@ -/* - - Copyright 2018 ZeroEx Intl. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - -*/ - -pragma solidity 0.4.24; -pragma experimental ABIEncoderV2; - -import "../../protocol/Exchange/libs/LibOrder.sol"; -import "../../protocol/Exchange/libs/LibFillResults.sol"; -import "../interfaces/IForwarderCore.sol"; - - -contract MForwarderCore is - IForwarderCore -{ - - /// @dev Ensures that all ZRX fees have been repurchased and no extra WETH owned by this contract has been sold. - /// @param orderFillResults Amounts filled and fees paid for primary orders. - /// @param feeOrderFillResults Amounts filled and fees paid for fee orders. - /// @param zrxBuyAmount The amount of ZRX that needed to be repurchased after filling primary orders. - function assertValidFillResults( - LibFillResults.FillResults memory orderFillResults, - LibFillResults.FillResults memory feeOrderFillResults, - uint256 zrxBuyAmount - ) - internal - view; -} -- cgit v1.2.3