From e90ed01105d58c008e31db7a5cfde57716c3c976 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Wed, 18 Jul 2018 16:02:16 -0700 Subject: Add assertValidFillResults --- .../src/2.0.0/forwarder/MixinForwarderCore.sol | 149 ++++++++++++--------- 1 file changed, 88 insertions(+), 61 deletions(-) (limited to 'packages') diff --git a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol index 561507ce4..7a136b018 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol @@ -85,11 +85,12 @@ contract MixinForwarderCore is // Convert ETH to WETH. convertEthToWeth(); + uint256 wethSellAmount; + uint256 zrxBuyAmount; uint256 makerAssetAmountPurchased; - uint256 wethAvailable; if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { // Calculate amount of WETH that won't be spent on ETH fees. - wethAvailable = getPartialAmount( + wethSellAmount = getPartialAmount( PERCENTAGE_DENOMINATOR, safeAdd(PERCENTAGE_DENOMINATOR, feePercentage), msg.value @@ -98,14 +99,14 @@ contract MixinForwarderCore is // ZRX fees are paid with this contract's balance. orderFillResults = marketSellWeth( orders, - wethAvailable, + wethSellAmount, signatures ); // The fee amount must be deducted from the amount transfered back to sender. makerAssetAmountPurchased = safeSub(orderFillResults.makerAssetFilledAmount, orderFillResults.takerFeePaid); } else { // 5% of WETH is reserved for filling feeOrders and paying feeRecipient. - wethAvailable = getPartialAmount( + wethSellAmount = getPartialAmount( MAX_WETH_FILL_PERCENTAGE, PERCENTAGE_DENOMINATOR, msg.value @@ -114,23 +115,24 @@ contract MixinForwarderCore is // ZRX fees are payed with this contract's balance. orderFillResults = marketSellWeth( orders, - wethAvailable, + wethSellAmount, signatures ); // Buy back all ZRX spent on fees. + zrxBuyAmount = orderFillResults.takerFeePaid; feeOrderFillResults = marketBuyZrx( feeOrders, - orderFillResults.takerFeePaid, + zrxBuyAmount, feeSignatures ); makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; } - // Ensure that no extra WETH owned by this contract has been sold. - uint256 totalWethSold = safeAdd(orderFillResults.takerAssetFilledAmount, feeOrderFillResults.takerAssetFilledAmount); - require( - totalWethSold <= msg.value, - "OVERSOLD_WETH" + // 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. @@ -181,6 +183,7 @@ contract MixinForwarderCore is // Convert ETH to WETH. convertEthToWeth(); + uint256 zrxBuyAmount; uint256 makerAssetAmountPurchased; if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { // If the makerAsset is ZRX, it is not necessary to pay fees out of this @@ -201,19 +204,20 @@ contract MixinForwarderCore is signatures ); // Buy back all ZRX spent on fees. + zrxBuyAmount = orderFillResults.takerFeePaid; feeOrderFillResults = marketBuyZrx( feeOrders, - orderFillResults.takerFeePaid, + zrxBuyAmount, feeSignatures ); makerAssetAmountPurchased = orderFillResults.makerAssetFilledAmount; } - // Ensure that no extra WETH owned by this contract has been sold. - uint256 totalWethSold = safeAdd(orderFillResults.takerAssetFilledAmount, feeOrderFillResults.takerAssetFilledAmount); - require( - totalWethSold <= msg.value, - "OVERSOLD_WETH" + // 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. @@ -307,53 +311,76 @@ contract MixinForwarderCore is returns (FillResults memory totalFillResults) { // Do nothing if zrxBuyAmount == 0 - if (zrxBuyAmount > 0) { - - bytes memory zrxAssetData = ZRX_ASSET_DATA; - bytes memory wethAssetData = WETH_ASSET_DATA; - uint256 zrxPurchased = 0; - - uint256 ordersLength = orders.length; - for (uint256 i = 0; i < ordersLength; i++) { - - // All of these are ZRX/WETH, so we can drop the respective assetData from calldata. - orders[i].makerAssetData = zrxAssetData; - orders[i].takerAssetData = wethAssetData; - - // Calculate the remaining amount of ZRX to buy. - uint256 remainingZrxBuyAmount = safeSub(zrxBuyAmount, zrxPurchased); - - // Convert the remaining amount of ZRX to buy into remaining amount - // of WETH to sell, assuming entire amount can be sold in the current order. - uint256 remainingWethSellAmount = getPartialAmount( - orders[i].takerAssetAmount, - safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees - remainingZrxBuyAmount - ); - - // Attempt to sell the remaining amount of WETH. - FillResults memory singleFillResult = EXCHANGE.fillOrderNoThrow( - orders[i], - safeAdd(remainingWethSellAmount, 1), - signatures[i] - ); - - // Update amounts filled and fees paid by maker and taker. - addFillResults(totalFillResults, singleFillResult); - zrxPurchased = safeSub(totalFillResults.makerAssetFilledAmount, totalFillResults.takerFeePaid); - - // Stop execution if the entire amount of ZRX has been bought. - if (zrxPurchased >= zrxBuyAmount) { - break; - } - } + if (zrxBuyAmount == 0) { + return totalFillResults; + } + + bytes memory zrxAssetData = ZRX_ASSET_DATA; + bytes memory wethAssetData = WETH_ASSET_DATA; + uint256 zrxPurchased = 0; + + uint256 ordersLength = orders.length; + for (uint256 i = 0; i < ordersLength; i++) { + + // All of these are ZRX/WETH, so we can drop the respective assetData from calldata. + orders[i].makerAssetData = zrxAssetData; + orders[i].takerAssetData = wethAssetData; + + // Calculate the remaining amount of ZRX to buy. + uint256 remainingZrxBuyAmount = safeSub(zrxBuyAmount, zrxPurchased); + + // Convert the remaining amount of ZRX to buy into remaining amount + // of WETH to sell, assuming entire amount can be sold in the current order. + uint256 remainingWethSellAmount = getPartialAmount( + orders[i].takerAssetAmount, + safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees + remainingZrxBuyAmount + ); - // Ensure that all ZRX spent while filling primary orders has been repurchased. - require( - zrxPurchased >= zrxBuyAmount, - "COMPLETE_FILL_FAILED" + // Attempt to sell the remaining amount of WETH. + FillResults memory singleFillResult = EXCHANGE.fillOrderNoThrow( + orders[i], + safeAdd(remainingWethSellAmount, 1), + signatures[i] ); + + // Update amounts filled and fees paid by maker and taker. + addFillResults(totalFillResults, singleFillResult); + zrxPurchased = safeSub(totalFillResults.makerAssetFilledAmount, totalFillResults.takerFeePaid); + + // Stop execution if the entire amount of ZRX has been bought. + if (zrxPurchased >= zrxBuyAmount) { + break; + } } + return totalFillResults; } + + /// @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" + ); + } } -- cgit v1.2.3