aboutsummaryrefslogtreecommitdiffstats
path: root/packages/contracts/src
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-08-29 04:00:49 +0800
committerGitHub <noreply@github.com>2018-08-29 04:00:49 +0800
commit14fdb71a716cb95bc1f6933db9057d23f3c41909 (patch)
tree9690b583d128f419f292ef78f0a70be260e8babe /packages/contracts/src
parent2eab0e30b753fb33729db53d141c8e22017cadec (diff)
downloaddexon-0x-contracts-14fdb71a716cb95bc1f6933db9057d23f3c41909.tar.gz
dexon-0x-contracts-14fdb71a716cb95bc1f6933db9057d23f3c41909.tar.zst
dexon-0x-contracts-14fdb71a716cb95bc1f6933db9057d23f3c41909.zip
safeGetPartialAmount (#1035)
* Added Test "Should transfer correct amounts when left order is fully filled and values pass isRoundingErrorCeil but fail isRoundingErrorFloor" * Added RoundingError exception to reference function for getPartialAmount * Added RoundingError exception to reference function for getPartialAmount * Added isRoundingErrorCeil to getPartialAmountCeil reference funtion * Computed new values for "Should give right maker a better buy price when correct price is not integral" that does not have a rounding error * Almost all tests for match orders are passing after adding isRoundingErrorCeil check * WIP commit: Added rounding error checks to getPartialAmount * WIP commit: Added rounding error checks to getPartialAmount * Use safe versions of getPartialAmount * Update Exchange internals tests * Run linter * Found new values for "Should transfer correct amounts when right order fill amount deviates from amount derived by `Exchange.fillOrder`" * Fixed merge conflicts * Run all tests * Cleaned up some comments on match Orders tests * Fix tests for geth
Diffstat (limited to 'packages/contracts/src')
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol16
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol16
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol82
-rw-r--r--packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol1
-rw-r--r--packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol36
5 files changed, 128 insertions, 23 deletions
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 be163ec97..64b1f7665 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
@@ -404,16 +404,6 @@ contract MixinExchangeCore is
safeMul(order.makerAssetAmount, takerAssetFilledAmount),
"INVALID_FILL_PRICE"
);
-
- // Validate fill order rounding
- require(
- !isRoundingErrorFloor(
- takerAssetFilledAmount,
- order.takerAssetAmount,
- order.makerAssetAmount
- ),
- "ROUNDING_ERROR"
- );
}
/// @dev Validates context for cancelOrder. Succeeds or throws.
@@ -463,17 +453,17 @@ contract MixinExchangeCore is
{
// Compute proportional transfer amounts
fillResults.takerAssetFilledAmount = takerAssetFilledAmount;
- fillResults.makerAssetFilledAmount = getPartialAmountFloor(
+ fillResults.makerAssetFilledAmount = safeGetPartialAmountFloor(
takerAssetFilledAmount,
order.takerAssetAmount,
order.makerAssetAmount
);
- fillResults.makerFeePaid = getPartialAmountFloor(
+ fillResults.makerFeePaid = safeGetPartialAmountFloor(
takerAssetFilledAmount,
order.takerAssetAmount,
order.makerFee
);
- fillResults.takerFeePaid = getPartialAmountFloor(
+ fillResults.takerFeePaid = safeGetPartialAmountFloor(
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 075a610b5..b4f6bdb26 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
@@ -177,13 +177,13 @@ contract MixinMatchOrders is
{
// Derive maker asset amounts for left & right orders, given store taker assert amounts
uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount);
- uint256 leftMakerAssetAmountRemaining = getPartialAmountFloor(
+ uint256 leftMakerAssetAmountRemaining = safeGetPartialAmountFloor(
leftOrder.makerAssetAmount,
leftOrder.takerAssetAmount,
leftTakerAssetAmountRemaining
);
uint256 rightTakerAssetAmountRemaining = safeSub(rightOrder.takerAssetAmount, rightOrderTakerAssetFilledAmount);
- uint256 rightMakerAssetAmountRemaining = getPartialAmountFloor(
+ uint256 rightMakerAssetAmountRemaining = safeGetPartialAmountFloor(
rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount,
rightTakerAssetAmountRemaining
@@ -205,7 +205,7 @@ contract MixinMatchOrders is
matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount;
// Round down to ensure the maker's exchange rate does not exceed the price specified by the order.
// We favor the maker when the exchange rate must be rounded.
- matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor(
+ matchedFillResults.left.makerAssetFilledAmount = safeGetPartialAmountFloor(
leftOrder.makerAssetAmount,
leftOrder.takerAssetAmount,
matchedFillResults.left.takerAssetFilledAmount
@@ -217,7 +217,7 @@ contract MixinMatchOrders is
matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount;
// Round up to ensure the maker's exchange rate does not exceed the price specified by the order.
// We favor the maker when the exchange rate must be rounded.
- matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil(
+ matchedFillResults.right.takerAssetFilledAmount = safeGetPartialAmountCeil(
rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount,
matchedFillResults.right.makerAssetFilledAmount
@@ -231,24 +231,24 @@ contract MixinMatchOrders is
);
// Compute fees for left order
- matchedFillResults.left.makerFeePaid = getPartialAmountFloor(
+ matchedFillResults.left.makerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.left.makerAssetFilledAmount,
leftOrder.makerAssetAmount,
leftOrder.makerFee
);
- matchedFillResults.left.takerFeePaid = getPartialAmountFloor(
+ matchedFillResults.left.takerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.left.takerAssetFilledAmount,
leftOrder.takerAssetAmount,
leftOrder.takerFee
);
// Compute fees for right order
- matchedFillResults.right.makerFeePaid = getPartialAmountFloor(
+ matchedFillResults.right.makerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.right.makerAssetFilledAmount,
rightOrder.makerAssetAmount,
rightOrder.makerFee
);
- matchedFillResults.right.takerFeePaid = getPartialAmountFloor(
+ matchedFillResults.right.takerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.right.takerAssetFilledAmount,
rightOrder.takerAssetAmount,
rightOrder.takerFee
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 0e0fba5d2..57fd53f29 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
@@ -26,11 +26,48 @@ contract LibMath is
{
/// @dev Calculates partial value given a numerator and denominator rounded down.
+ /// Reverts if rounding error is >= 0.1%
/// @param numerator Numerator.
/// @param denominator Denominator.
/// @param target Value to calculate partial of.
/// @return Partial value of target rounded down.
- function getPartialAmountFloor(
+ function safeGetPartialAmountFloor(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ internal
+ pure
+ returns (uint256 partialAmount)
+ {
+ require(
+ denominator > 0,
+ "DIVISION_BY_ZERO"
+ );
+
+ require(
+ !isRoundingErrorFloor(
+ numerator,
+ denominator,
+ target
+ ),
+ "ROUNDING_ERROR"
+ );
+
+ partialAmount = safeDiv(
+ safeMul(numerator, target),
+ denominator
+ );
+ return partialAmount;
+ }
+
+ /// @dev Calculates partial value given a numerator and denominator rounded down.
+ /// Reverts if rounding error is >= 0.1%
+ /// @param numerator Numerator.
+ /// @param denominator Denominator.
+ /// @param target Value to calculate partial of.
+ /// @return Partial value of target rounded up.
+ function safeGetPartialAmountCeil(
uint256 numerator,
uint256 denominator,
uint256 target
@@ -43,7 +80,48 @@ contract LibMath is
denominator > 0,
"DIVISION_BY_ZERO"
);
+
+ require(
+ !isRoundingErrorCeil(
+ numerator,
+ denominator,
+ target
+ ),
+ "ROUNDING_ERROR"
+ );
+ // 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 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 down.
+ function getPartialAmountFloor(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ internal
+ pure
+ returns (uint256 partialAmount)
+ {
+ require(
+ denominator > 0,
+ "DIVISION_BY_ZERO"
+ );
+
partialAmount = safeDiv(
safeMul(numerator, target),
denominator
@@ -69,7 +147,7 @@ contract LibMath is
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.
diff --git a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
index 8bfdd2e66..4f4e28302 100644
--- a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
+++ b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
@@ -25,6 +25,7 @@ import "../../protocol/Exchange/interfaces/IExchange.sol";
import "../../protocol/Exchange/libs/LibOrder.sol";
+// solhint-disable no-unused-vars
contract ReentrantERC20Token is
ERC20Token
{
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 da9313e02..27187f8f8 100644
--- a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
+++ b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
@@ -63,6 +63,42 @@ contract TestExchangeInternals is
}
/// @dev Calculates partial value given a numerator and denominator.
+ /// Reverts if rounding error is >= 0.1%
+ /// @param numerator Numerator.
+ /// @param denominator Denominator.
+ /// @param target Value to calculate partial of.
+ /// @return Partial value of target.
+ function publicSafeGetPartialAmountFloor(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ public
+ pure
+ returns (uint256 partialAmount)
+ {
+ return safeGetPartialAmountFloor(numerator, denominator, target);
+ }
+
+ /// @dev Calculates partial value given a numerator and denominator.
+ /// Reverts if rounding error is >= 0.1%
+ /// @param numerator Numerator.
+ /// @param denominator Denominator.
+ /// @param target Value to calculate partial of.
+ /// @return Partial value of target.
+ function publicSafeGetPartialAmountCeil(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ public
+ pure
+ returns (uint256 partialAmount)
+ {
+ return safeGetPartialAmountCeil(numerator, denominator, target);
+ }
+
+ /// @dev Calculates partial value given a numerator and denominator.
/// @param numerator Numerator.
/// @param denominator Denominator.
/// @param target Value to calculate partial of.