diff options
Diffstat (limited to 'packages/contracts/src')
16 files changed, 591 insertions, 95 deletions
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol index 218713d3c..a7ff400b9 100644 --- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol @@ -163,7 +163,7 @@ contract MixinExchangeWrapper is // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmount( + uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount @@ -231,7 +231,7 @@ contract MixinExchangeWrapper is // 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( + uint256 remainingWethSellAmount = getPartialAmountFloor( orders[i].takerAssetAmount, safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees remainingZrxBuyAmount diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol index 42cec4d36..14f191879 100644 --- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol +++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol @@ -87,7 +87,7 @@ contract MixinForwarderCore is uint256 makerAssetAmountPurchased; if (orders[0].makerAssetData.equals(ZRX_ASSET_DATA)) { // Calculate amount of WETH that won't be spent on ETH fees. - wethSellAmount = getPartialAmount( + wethSellAmount = getPartialAmountFloor( PERCENTAGE_DENOMINATOR, safeAdd(PERCENTAGE_DENOMINATOR, feePercentage), msg.value @@ -103,7 +103,7 @@ contract MixinForwarderCore is makerAssetAmountPurchased = safeSub(orderFillResults.makerAssetFilledAmount, orderFillResults.takerFeePaid); } else { // 5% of WETH is reserved for filling feeOrders and paying feeRecipient. - wethSellAmount = getPartialAmount( + wethSellAmount = getPartialAmountFloor( MAX_WETH_FILL_PERCENTAGE, PERCENTAGE_DENOMINATOR, msg.value diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol index 93e85e599..5863b522d 100644 --- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol +++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol @@ -82,7 +82,7 @@ contract MixinWeth is uint256 wethRemaining = safeSub(msg.value, wethSold); // Calculate ETH fee to pay to feeRecipient. - uint256 ethFee = getPartialAmount( + uint256 ethFee = getPartialAmountFloor( feePercentage, PERCENTAGE_DENOMINATOR, wethSoldExcludingFeeOrders diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol index e9f882194..80475e6e3 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -83,7 +83,7 @@ contract MixinAssetProxyDispatcher is internal { // Do nothing if no amount should be transferred. - if (amount > 0) { + if (amount > 0 && from != to) { // Ensure assetData length is valid require( assetData.length > 3, diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol index 515606cb9..be163ec97 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol @@ -19,6 +19,7 @@ pragma solidity 0.4.24; pragma experimental ABIEncoderV2; +import "../../utils/ReentrancyGuard/ReentrancyGuard.sol"; import "./libs/LibConstants.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; @@ -30,6 +31,7 @@ import "./mixins/MAssetProxyDispatcher.sol"; contract MixinExchangeCore is + ReentrancyGuard, LibConstants, LibMath, LibOrder, @@ -54,6 +56,7 @@ contract MixinExchangeCore is /// @param targetOrderEpoch Orders created with a salt less or equal to this value will be cancelled. function cancelOrdersUpTo(uint256 targetOrderEpoch) external + nonReentrant { address makerAddress = getCurrentContextAddress(); // If this function is called via `executeTransaction`, we only update the orderEpoch for the makerAddress/msg.sender combination. @@ -86,50 +89,14 @@ contract MixinExchangeCore is bytes memory signature ) public + nonReentrant returns (FillResults memory fillResults) { - // Fetch order info - OrderInfo memory orderInfo = getOrderInfo(order); - - // Fetch taker address - address takerAddress = getCurrentContextAddress(); - - // Assert that the order is fillable by taker - assertFillableOrder( + fillResults = fillOrderInternal( order, - orderInfo, - takerAddress, - signature - ); - - // Get amount of takerAsset to fill - uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount); - uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount); - - // Compute proportional fill amounts - fillResults = calculateFillResults(order, takerAssetFilledAmount); - - // Validate context - assertValidFill( - order, - orderInfo, takerAssetFillAmount, - takerAssetFilledAmount, - fillResults.makerAssetFilledAmount - ); - - // Update exchange internal state - updateFilledState( - order, - takerAddress, - orderInfo.orderHash, - orderInfo.orderTakerAssetFilledAmount, - fillResults + signature ); - - // Settle order - settleOrder(order, takerAddress, fillResults); - return fillResults; } @@ -138,6 +105,7 @@ contract MixinExchangeCore is /// @param order Order to cancel. Order must be OrderStatus.FILLABLE. function cancelOrder(Order memory order) public + nonReentrant { // Fetch current order status OrderInfo memory orderInfo = getOrderInfo(order); @@ -210,6 +178,64 @@ contract MixinExchangeCore is return orderInfo; } + /// @dev Fills the input order. + /// @param order Order struct containing order specifications. + /// @param takerAssetFillAmount Desired amount of takerAsset to sell. + /// @param signature Proof that order has been created by maker. + /// @return Amounts filled and fees paid by maker and taker. + function fillOrderInternal( + Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + internal + returns (FillResults memory fillResults) + { + // Fetch order info + OrderInfo memory orderInfo = getOrderInfo(order); + + // Fetch taker address + address takerAddress = getCurrentContextAddress(); + + // Assert that the order is fillable by taker + assertFillableOrder( + order, + orderInfo, + takerAddress, + signature + ); + + // Get amount of takerAsset to fill + uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount); + uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount); + + // Validate context + assertValidFill( + order, + orderInfo, + takerAssetFillAmount, + takerAssetFilledAmount, + fillResults.makerAssetFilledAmount + ); + + // Compute proportional fill amounts + fillResults = calculateFillResults(order, takerAssetFilledAmount); + + // Update exchange internal state + updateFilledState( + order, + takerAddress, + orderInfo.orderHash, + orderInfo.orderTakerAssetFilledAmount, + fillResults + ); + + // Settle order + settleOrder(order, takerAddress, fillResults); + + return fillResults; + } + /// @dev Updates state with results of a fill order. /// @param order that was filled. /// @param takerAddress Address of taker who filled the order. @@ -381,7 +407,7 @@ contract MixinExchangeCore is // Validate fill order rounding require( - !isRoundingError( + !isRoundingErrorFloor( takerAssetFilledAmount, order.takerAssetAmount, order.makerAssetAmount @@ -437,17 +463,17 @@ contract MixinExchangeCore is { // Compute proportional transfer amounts fillResults.takerAssetFilledAmount = takerAssetFilledAmount; - fillResults.makerAssetFilledAmount = getPartialAmount( + fillResults.makerAssetFilledAmount = getPartialAmountFloor( takerAssetFilledAmount, order.takerAssetAmount, order.makerAssetAmount ); - fillResults.makerFeePaid = getPartialAmount( + fillResults.makerFeePaid = getPartialAmountFloor( takerAssetFilledAmount, order.takerAssetAmount, order.makerFee ); - fillResults.takerFeePaid = getPartialAmount( + fillResults.takerFeePaid = getPartialAmountFloor( takerAssetFilledAmount, order.takerAssetAmount, order.takerFee diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol index c860640c4..bf97557d6 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol @@ -14,6 +14,7 @@ pragma solidity 0.4.24; pragma experimental ABIEncoderV2; +import "../../utils/ReentrancyGuard/ReentrancyGuard.sol"; import "./libs/LibConstants.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; @@ -25,6 +26,7 @@ import "./mixins/MAssetProxyDispatcher.sol"; contract MixinMatchOrders is + ReentrancyGuard, LibConstants, LibMath, MAssetProxyDispatcher, @@ -48,6 +50,7 @@ contract MixinMatchOrders is bytes memory rightSignature ) public + nonReentrant returns (LibFillResults.MatchedFillResults memory matchedFillResults) { // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData. @@ -193,7 +196,7 @@ contract MixinMatchOrders is leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining; // The right order receives an amount proportional to how much was spent. - rightTakerAssetFilledAmount = getPartialAmount( + rightTakerAssetFilledAmount = getPartialAmountFloor( rightOrder.takerAssetAmount, rightOrder.makerAssetAmount, leftTakerAssetFilledAmount @@ -203,7 +206,7 @@ contract MixinMatchOrders is rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining; // The left order receives an amount proportional to how much was spent. - leftTakerAssetFilledAmount = getPartialAmount( + leftTakerAssetFilledAmount = getPartialAmountFloor( rightOrder.makerAssetAmount, rightOrder.takerAssetAmount, rightTakerAssetFilledAmount diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol index f30adcdb8..4eb6a2fa6 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol @@ -19,6 +19,7 @@ pragma solidity 0.4.24; import "../../utils/LibBytes/LibBytes.sol"; +import "../../utils/ReentrancyGuard/ReentrancyGuard.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; import "./interfaces/IWallet.sol"; @@ -26,6 +27,7 @@ import "./interfaces/IValidator.sol"; contract MixinSignatureValidator is + ReentrancyGuard, MSignatureValidator, MTransactions { @@ -69,6 +71,7 @@ contract MixinSignatureValidator is bool approval ) external + nonReentrant { address signerAddress = getCurrentContextAddress(); allowedValidators[signerAddress][validatorAddress] = approval; diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol index 821d30279..4a59b6c0f 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol @@ -155,7 +155,8 @@ contract MixinTransactions is view returns (address) { - address contextAddress = currentContextAddress == address(0) ? msg.sender : currentContextAddress; + address currentContextAddress_ = currentContextAddress; + address contextAddress = currentContextAddress_ == address(0) ? msg.sender : currentContextAddress_; return contextAddress; } } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol index 86194f461..39fa724cc 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol @@ -19,18 +19,22 @@ pragma solidity 0.4.24; pragma experimental ABIEncoderV2; +import "../../utils/ReentrancyGuard/ReentrancyGuard.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibAbiEncoder.sol"; import "./mixins/MExchangeCore.sol"; +import "./mixins/MWrapperFunctions.sol"; contract MixinWrapperFunctions is + ReentrancyGuard, LibMath, LibFillResults, LibAbiEncoder, - MExchangeCore + MExchangeCore, + MWrapperFunctions { /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. @@ -43,17 +47,14 @@ contract MixinWrapperFunctions is bytes memory signature ) public + nonReentrant returns (FillResults memory fillResults) { - fillResults = fillOrder( + fillResults = fillOrKillOrderInternal( order, takerAssetFillAmount, signature ); - require( - fillResults.takerAssetFilledAmount == takerAssetFillAmount, - "COMPLETE_FILL_FAILED" - ); return fillResults; } @@ -88,14 +89,7 @@ contract MixinWrapperFunctions is fillOrderCalldata, // write output over input 128 // output size is 128 bytes ) - switch success - case 0 { - mstore(fillResults, 0) - mstore(add(fillResults, 32), 0) - mstore(add(fillResults, 64), 0) - mstore(add(fillResults, 96), 0) - } - case 1 { + if success { mstore(fillResults, mload(fillOrderCalldata)) mstore(add(fillResults, 32), mload(add(fillOrderCalldata, 32))) mstore(add(fillResults, 64), mload(add(fillOrderCalldata, 64))) @@ -117,11 +111,12 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public + nonReentrant returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; for (uint256 i = 0; i != ordersLength; i++) { - FillResults memory singleFillResults = fillOrder( + FillResults memory singleFillResults = fillOrderInternal( orders[i], takerAssetFillAmounts[i], signatures[i] @@ -143,11 +138,12 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public + nonReentrant returns (FillResults memory totalFillResults) { uint256 ordersLength = orders.length; for (uint256 i = 0; i != ordersLength; i++) { - FillResults memory singleFillResults = fillOrKillOrder( + FillResults memory singleFillResults = fillOrKillOrderInternal( orders[i], takerAssetFillAmounts[i], signatures[i] @@ -195,6 +191,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public + nonReentrant returns (FillResults memory totalFillResults) { bytes memory takerAssetData = orders[0].takerAssetData; @@ -210,7 +207,7 @@ contract MixinWrapperFunctions is uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); // Attempt to sell the remaining amount of takerAsset - FillResults memory singleFillResults = fillOrder( + FillResults memory singleFillResults = fillOrderInternal( orders[i], remainingTakerAssetFillAmount, signatures[i] @@ -282,6 +279,7 @@ contract MixinWrapperFunctions is bytes[] memory signatures ) public + nonReentrant returns (FillResults memory totalFillResults) { bytes memory makerAssetData = orders[0].makerAssetData; @@ -298,14 +296,14 @@ contract MixinWrapperFunctions is // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmount( + uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount ); // Attempt to sell the remaining amount of takerAsset - FillResults memory singleFillResults = fillOrder( + FillResults memory singleFillResults = fillOrderInternal( orders[i], remainingTakerAssetFillAmount, signatures[i] @@ -350,7 +348,7 @@ contract MixinWrapperFunctions is // Convert the remaining amount of makerAsset to buy into remaining amount // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmount( + uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount @@ -400,4 +398,28 @@ contract MixinWrapperFunctions is } return ordersInfo; } + + /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. + /// @param order Order struct containing order specifications. + /// @param takerAssetFillAmount Desired amount of takerAsset to sell. + /// @param signature Proof that order has been created by maker. + function fillOrKillOrderInternal( + LibOrder.Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + internal + returns (FillResults memory fillResults) + { + fillResults = fillOrderInternal( + order, + takerAssetFillAmount, + signature + ); + require( + fillResults.takerAssetFilledAmount == takerAssetFillAmount, + "COMPLETE_FILL_FAILED" + ); + return fillResults; + } } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol index fa09da6ac..0e0fba5d2 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol @@ -25,12 +25,12 @@ contract LibMath is SafeMath { - /// @dev Calculates partial value given a numerator and denominator. + /// @dev Calculates partial value given a numerator and denominator rounded down. /// @param numerator Numerator. /// @param denominator Denominator. /// @param target Value to calculate partial of. - /// @return Partial value of target. - function getPartialAmount( + /// @return Partial value of target rounded down. + function getPartialAmountFloor( uint256 numerator, uint256 denominator, uint256 target @@ -39,19 +39,56 @@ contract LibMath is pure returns (uint256 partialAmount) { + require( + denominator > 0, + "DIVISION_BY_ZERO" + ); + partialAmount = safeDiv( safeMul(numerator, target), denominator ); return partialAmount; } - - /// @dev Checks if rounding error > 0.1%. + + /// @dev Calculates partial value given a numerator and denominator rounded down. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to calculate partial of. + /// @return Partial value of target rounded up. + function getPartialAmountCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + internal + pure + returns (uint256 partialAmount) + { + require( + denominator > 0, + "DIVISION_BY_ZERO" + ); + + // safeDiv computes `floor(a / b)`. We use the identity (a, b integer): + // ceil(a / b) = floor((a + b - 1) / b) + // To implement `ceil(a / b)` using safeDiv. + partialAmount = safeDiv( + safeAdd( + safeMul(numerator, target), + safeSub(denominator, 1) + ), + denominator + ); + return partialAmount; + } + + /// @dev Checks if rounding error >= 0.1% when rounding down. /// @param numerator Numerator. /// @param denominator Denominator. /// @param target Value to multiply with numerator/denominator. /// @return Rounding error is present. - function isRoundingError( + function isRoundingErrorFloor( uint256 numerator, uint256 denominator, uint256 target @@ -60,16 +97,73 @@ contract LibMath is pure returns (bool isError) { - uint256 remainder = mulmod(target, numerator, denominator); - if (remainder == 0) { - return false; // No rounding error. + require( + denominator > 0, + "DIVISION_BY_ZERO" + ); + + // The absolute rounding error is the difference between the rounded + // value and the ideal value. The relative rounding error is the + // absolute rounding error divided by the absolute value of the + // ideal value. This is undefined when the ideal value is zero. + // + // The ideal value is `numerator * target / denominator`. + // Let's call `numerator * target % denominator` the remainder. + // The absolute error is `remainder / denominator`. + // + // When the ideal value is zero, we require the absolute error to + // be zero. Fortunately, this is always the case. The ideal value is + // zero iff `numerator == 0` and/or `target == 0`. In this case the + // remainder and absolute error are also zero. + if (target == 0 || numerator == 0) { + return false; } - - uint256 errPercentageTimes1000000 = safeDiv( - safeMul(remainder, 1000000), - safeMul(numerator, target) + + // Otherwise, we want the relative rounding error to be strictly + // less than 0.1%. + // The relative error is `remainder / (numerator * target)`. + // We want the relative error less than 1 / 1000: + // remainder / (numerator * denominator) < 1 / 1000 + // or equivalently: + // 1000 * remainder < numerator * target + // so we have a rounding error iff: + // 1000 * remainder >= numerator * target + uint256 remainder = mulmod(target, numerator, denominator); + isError = safeMul(1000, remainder) >= safeMul(numerator, target); + return isError; + } + + /// @dev Checks if rounding error >= 0.1% when rounding up. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to multiply with numerator/denominator. + /// @return Rounding error is present. + function isRoundingErrorCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + internal + pure + returns (bool isError) + { + require( + denominator > 0, + "DIVISION_BY_ZERO" ); - isError = errPercentageTimes1000000 > 1000; + + // See the comments in `isRoundingError`. + if (target == 0 || numerator == 0) { + // When either is zero, the ideal value and rounded value are zero + // and there is no rounding error. (Although the relative error + // is undefined.) + return false; + } + // Compute remainder as before + uint256 remainder = mulmod(target, numerator, denominator); + // TODO: safeMod + remainder = safeSub(denominator, remainder) % denominator; + isError = safeMul(1000, remainder) >= safeMul(numerator, target); return isError; } } diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol index 708cb329e..d85913e0f 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol @@ -59,6 +59,19 @@ contract MExchangeCore is uint256 orderEpoch // Orders with specified makerAddress and senderAddress with a salt less than this value are considered cancelled. ); + /// @dev Fills the input order. + /// @param order Order struct containing order specifications. + /// @param takerAssetFillAmount Desired amount of takerAsset to sell. + /// @param signature Proof that order has been created by maker. + /// @return Amounts filled and fees paid by maker and taker. + function fillOrderInternal( + LibOrder.Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + internal + returns (LibFillResults.FillResults memory fillResults); + /// @dev Updates state with results of a fill order. /// @param order that was filled. /// @param takerAddress Address of taker who filled the order. diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol new file mode 100644 index 000000000..e04d4a429 --- /dev/null +++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol @@ -0,0 +1,40 @@ +/* + + 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 "../libs/LibOrder.sol"; +import "../libs/LibFillResults.sol"; +import "../interfaces/IWrapperFunctions.sol"; + + +contract MWrapperFunctions { + + /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled. + /// @param order LibOrder.Order struct containing order specifications. + /// @param takerAssetFillAmount Desired amount of takerAsset to sell. + /// @param signature Proof that order has been created by maker. + function fillOrKillOrderInternal( + LibOrder.Order memory order, + uint256 takerAssetFillAmount, + bytes memory signature + ) + internal + returns (LibFillResults.FillResults memory fillResults); +} diff --git a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol new file mode 100644 index 000000000..8bfdd2e66 --- /dev/null +++ b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol @@ -0,0 +1,182 @@ +/* + + 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 "../../utils/LibBytes/LibBytes.sol"; +import "../../tokens/ERC20Token/ERC20Token.sol"; +import "../../protocol/Exchange/interfaces/IExchange.sol"; +import "../../protocol/Exchange/libs/LibOrder.sol"; + + +contract ReentrantERC20Token is + ERC20Token +{ + + using LibBytes for bytes; + + // solhint-disable-next-line var-name-mixedcase + IExchange internal EXCHANGE; + + bytes internal constant REENTRANCY_ILLEGAL_REVERT_REASON = abi.encodeWithSelector( + bytes4(keccak256("Error(string)")), + "REENTRANCY_ILLEGAL" + ); + + // All of these functions are potentially vulnerable to reentrancy + // We do not test any "noThrow" functions because `fillOrderNoThrow` makes a delegatecall to `fillOrder` + enum ExchangeFunction { + FILL_ORDER, + FILL_OR_KILL_ORDER, + BATCH_FILL_ORDERS, + BATCH_FILL_OR_KILL_ORDERS, + MARKET_BUY_ORDERS, + MARKET_SELL_ORDERS, + MATCH_ORDERS, + CANCEL_ORDER, + CANCEL_ORDERS_UP_TO, + SET_SIGNATURE_VALIDATOR_APPROVAL + } + + uint8 internal currentFunctionId = 0; + + constructor (address _exchange) + public + { + EXCHANGE = IExchange(_exchange); + } + + /// @dev Set the current function that will be called when `transferFrom` is called. + /// @param _currentFunctionId Id that corresponds to function name. + function setCurrentFunction(uint8 _currentFunctionId) + external + { + currentFunctionId = _currentFunctionId; + } + + /// @dev A version of `transferFrom` that attempts to reenter the Exchange contract. + /// @param _from The address of the sender + /// @param _to The address of the recipient + /// @param _value The amount of token to be transferred + function transferFrom( + address _from, + address _to, + uint256 _value + ) + external + returns (bool) + { + // This order would normally be invalid, but it will be used strictly for testing reentrnacy. + // Any reentrancy checks will happen before any other checks that invalidate the order. + LibOrder.Order memory order; + + // Initialize remaining null parameters + bytes memory signature; + LibOrder.Order[] memory orders; + uint256[] memory takerAssetFillAmounts; + bytes[] memory signatures; + bytes memory calldata; + + // Create calldata for function that corresponds to currentFunctionId + if (currentFunctionId == uint8(ExchangeFunction.FILL_ORDER)) { + calldata = abi.encodeWithSelector( + EXCHANGE.fillOrder.selector, + order, + 0, + signature + ); + } else if (currentFunctionId == uint8(ExchangeFunction.FILL_OR_KILL_ORDER)) { + calldata = abi.encodeWithSelector( + EXCHANGE.fillOrKillOrder.selector, + order, + 0, + signature + ); + } else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_ORDERS)) { + calldata = abi.encodeWithSelector( + EXCHANGE.batchFillOrders.selector, + orders, + takerAssetFillAmounts, + signatures + ); + } else if (currentFunctionId == uint8(ExchangeFunction.BATCH_FILL_OR_KILL_ORDERS)) { + calldata = abi.encodeWithSelector( + EXCHANGE.batchFillOrKillOrders.selector, + orders, + takerAssetFillAmounts, + signatures + ); + } else if (currentFunctionId == uint8(ExchangeFunction.MARKET_BUY_ORDERS)) { + calldata = abi.encodeWithSelector( + EXCHANGE.marketBuyOrders.selector, + orders, + 0, + signatures + ); + } else if (currentFunctionId == uint8(ExchangeFunction.MARKET_SELL_ORDERS)) { + calldata = abi.encodeWithSelector( + EXCHANGE.marketSellOrders.selector, + orders, + 0, + signatures + ); + } else if (currentFunctionId == uint8(ExchangeFunction.MATCH_ORDERS)) { + calldata = abi.encodeWithSelector( + EXCHANGE.matchOrders.selector, + order, + order, + signature, + signature + ); + } else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDER)) { + calldata = abi.encodeWithSelector( + EXCHANGE.cancelOrder.selector, + order + ); + } else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDERS_UP_TO)) { + calldata = abi.encodeWithSelector( + EXCHANGE.cancelOrdersUpTo.selector, + 0 + ); + } else if (currentFunctionId == uint8(ExchangeFunction.SET_SIGNATURE_VALIDATOR_APPROVAL)) { + calldata = abi.encodeWithSelector( + EXCHANGE.setSignatureValidatorApproval.selector, + address(0), + false + ); + } + + // Call Exchange function, swallow error + address(EXCHANGE).call(calldata); + + // Revert reason is 100 bytes + bytes memory returnData = new bytes(100); + + // Copy return data + assembly { + returndatacopy(add(returnData, 32), 0, 100) + } + + // Revert if function reverted with REENTRANCY_ILLEGAL error + require(!REENTRANCY_ILLEGAL_REVERT_REASON.equals(returnData)); + + // Transfer will return true if function failed for any other reason + return true; + } +}
\ No newline at end of file diff --git a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol index d9cec9edc..da9313e02 100644 --- a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol +++ b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol @@ -67,7 +67,7 @@ contract TestExchangeInternals is /// @param denominator Denominator. /// @param target Value to calculate partial of. /// @return Partial value of target. - function publicGetPartialAmount( + function publicGetPartialAmountFloor( uint256 numerator, uint256 denominator, uint256 target @@ -76,15 +76,49 @@ contract TestExchangeInternals is pure returns (uint256 partialAmount) { - return getPartialAmount(numerator, denominator, target); + return getPartialAmountFloor(numerator, denominator, target); } - /// @dev Checks if rounding error > 0.1%. + /// @dev Calculates partial value given a numerator and denominator. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to calculate partial of. + /// @return Partial value of target. + function publicGetPartialAmountCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (uint256 partialAmount) + { + return getPartialAmountCeil(numerator, denominator, target); + } + + /// @dev Checks if rounding error >= 0.1%. + /// @param numerator Numerator. + /// @param denominator Denominator. + /// @param target Value to multiply with numerator/denominator. + /// @return Rounding error is present. + function publicIsRoundingErrorFloor( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (bool isError) + { + return isRoundingErrorFloor(numerator, denominator, target); + } + + /// @dev Checks if rounding error >= 0.1%. /// @param numerator Numerator. /// @param denominator Denominator. /// @param target Value to multiply with numerator/denominator. /// @return Rounding error is present. - function publicIsRoundingError( + function publicIsRoundingErrorCeil( uint256 numerator, uint256 denominator, uint256 target @@ -93,7 +127,7 @@ contract TestExchangeInternals is pure returns (bool isError) { - return isRoundingError(numerator, denominator, target); + return isRoundingErrorCeil(numerator, denominator, target); } /// @dev Updates state with results of a fill order. diff --git a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol index 4a99dd9c1..c8c58545f 100644 --- a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol +++ b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol @@ -49,7 +49,7 @@ contract TestLibs is return fillOrderCalldata; } - function publicGetPartialAmount( + function publicGetPartialAmountFloor( uint256 numerator, uint256 denominator, uint256 target @@ -58,7 +58,7 @@ contract TestLibs is pure returns (uint256 partialAmount) { - partialAmount = getPartialAmount( + partialAmount = getPartialAmountFloor( numerator, denominator, target @@ -66,7 +66,41 @@ contract TestLibs is return partialAmount; } - function publicIsRoundingError( + function publicGetPartialAmountCeil( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (uint256 partialAmount) + { + partialAmount = getPartialAmountCeil( + numerator, + denominator, + target + ); + return partialAmount; + } + + function publicIsRoundingErrorFloor( + uint256 numerator, + uint256 denominator, + uint256 target + ) + public + pure + returns (bool isError) + { + isError = isRoundingErrorFloor( + numerator, + denominator, + target + ); + return isError; + } + + function publicIsRoundingErrorCeil( uint256 numerator, uint256 denominator, uint256 target @@ -75,7 +109,7 @@ contract TestLibs is pure returns (bool isError) { - isError = isRoundingError( + isError = isRoundingErrorCeil( numerator, denominator, target diff --git a/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol b/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol new file mode 100644 index 000000000..1dee512d4 --- /dev/null +++ b/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol @@ -0,0 +1,44 @@ +/* + + 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 ReentrancyGuard { + + // Locked state of mutex + bool private locked = false; + + /// @dev Functions with this modifer cannot be reentered. The mutex will be locked + /// before function execution and unlocked after. + modifier nonReentrant() { + // Ensure mutex is unlocked + require( + !locked, + "REENTRANCY_ILLEGAL" + ); + + // Lock mutex before function call + locked = true; + + // Perform function call + _; + + // Unlock mutex after function call + locked = false; + } +} |