From 92497d7df4b61ee62acfdd58bfb98569ff67214e Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Thu, 23 Aug 2018 11:01:52 -0700
Subject: Fix isRoundingError

---
 .../src/2.0.0/protocol/Exchange/libs/LibMath.sol   | 27 ++++++++++++++--------
 1 file changed, 17 insertions(+), 10 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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..146bb9943 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
@@ -46,7 +46,7 @@ contract LibMath is
         return partialAmount;
     }
 
-    /// @dev Checks if rounding error > 0.1%.
+    /// @dev Checks if rounding error >= 0.1%.
     /// @param numerator Numerator.
     /// @param denominator Denominator.
     /// @param target Value to multiply with numerator/denominator.
@@ -60,16 +60,23 @@ contract LibMath is
         pure
         returns (bool isError)
     {
+        // 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. We want the relative rounding error to be strictly less
+        // than 0.1%.
+        // Let's call `numerator * target % denominator` the remainder.
+        // The ideal value is `numerator * target / denominator`.
+        // The absolute error is `remainder / denominator`.
+        // 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);
-        if (remainder == 0) {
-            return false; // No rounding error.
-        }
-
-        uint256 errPercentageTimes1000000 = safeDiv(
-            safeMul(remainder, 1000000),
-            safeMul(numerator, target)
-        );
-        isError = errPercentageTimes1000000 > 1000;
+        isError = safeMul(1000, remainder) >= safeMul(numerator, target);
         return isError;
     }
 }
-- 
cgit v1.2.3


From e68942ee78eb19c27a96fb0b6b8b05c83b647bcc Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Thu, 23 Aug 2018 12:54:39 -0700
Subject: Handle zero case

---
 .../src/2.0.0/protocol/Exchange/libs/LibMath.sol       | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 146bb9943..758b7ec90 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
@@ -60,14 +60,26 @@ contract LibMath is
         pure
         returns (bool isError)
     {
+        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. We want the relative rounding error to be strictly less
-        // than 0.1%.
-        // Let's call `numerator * target % denominator` the remainder.
+        // 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;
+        }
+        // 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
-- 
cgit v1.2.3


From ab5df342e1dc4add20223fab7128f9323a114b8e Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Wed, 22 Aug 2018 12:22:49 -0700
Subject: Add getPartialAmountCeil

---
 .../src/2.0.0/protocol/Exchange/libs/LibMath.sol   | 35 ++++++++++++++++++++--
 1 file changed, 32 insertions(+), 3 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 758b7ec90..3e70d1b60 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
@@ -29,7 +29,7 @@ contract LibMath is
     /// @param numerator Numerator.
     /// @param denominator Denominator.
     /// @param target Value to calculate partial of.
-    /// @return Partial value of target.
+    /// @return Partial value of target rounded down.
     function getPartialAmount(
         uint256 numerator,
         uint256 denominator,
@@ -45,8 +45,37 @@ contract LibMath is
         );
         return partialAmount;
     }
-
-    /// @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 rounded up.
+    function getPartialAmountCeil(
+        uint256 numerator,
+        uint256 denominator,
+        uint256 target
+    )
+        internal
+        pure
+        returns (uint256 partialAmount)
+    {
+        require(
+            denominator > 0,
+            "DIVISION_BY_ZERO"
+        );
+        
+        // SafeDiv rounds down. To use it to round up we need to add
+        // denominator - 1. This causes anything less than an exact multiple
+        // of denominator to be rounded up.
+        partialAmount = safeDiv(
+            safeAdd(safeMul(numerator, target), safeSub(denominator, 1)),
+            denominator
+        );
+        return partialAmount;
+    }
+    
+    /// @dev Checks if rounding error > 0.1%.
     /// @param numerator Numerator.
     /// @param denominator Denominator.
     /// @param target Value to multiply with numerator/denominator.
-- 
cgit v1.2.3


From 5d70df771b151800b8a04b5569529843701c9fbd Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Wed, 22 Aug 2018 12:22:58 -0700
Subject: Add isRoundingErrorCeil

---
 .../src/2.0.0/protocol/Exchange/libs/LibMath.sol   | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 3e70d1b60..c4aa2abb5 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
@@ -107,6 +107,7 @@ contract LibMath is
         if (target == 0 || numerator == 0) {
             return false;
         }
+        
         // Otherwise, we want the relative rounding error to be strictly
         // less than 0.1%.
         // The relative error is `remainder / numerator * target`.
@@ -120,4 +121,32 @@ contract LibMath is
         isError = safeMul(1000, remainder) >= safeMul(numerator, target);
         return isError;
     }
+    
+    /// @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 isRoundingErrorCeil(
+        uint256 numerator,
+        uint256 denominator,
+        uint256 target
+    )
+        internal
+        pure
+        returns (bool isError)
+    {
+        require(denominator > 0, "DIVISION_BY_ZERO");
+        
+        if (target == 0 || numerator == 0) {
+            return false;
+        }
+        uint256 remainder = mulmod(target, numerator, denominator);
+        if (remainder == 0) {
+            return false;
+        }
+        remainder = safeSub(denominator, remainder);
+        isError = safeMul(1000, remainder) >= safeMul(numerator, target);
+        return isError;
+    }
 }
-- 
cgit v1.2.3


From 50fab9feb3b80b7d5ac1e94df9af68cad4aaabe0 Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Wed, 22 Aug 2018 12:22:58 -0700
Subject: Improve getPartialAmountCeil docs

---
 packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 c4aa2abb5..d123c55a1 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
@@ -65,9 +65,9 @@ contract LibMath is
             "DIVISION_BY_ZERO"
         );
         
-        // SafeDiv rounds down. To use it to round up we need to add
-        // denominator - 1. This causes anything less than an exact multiple
-        // of denominator to be rounded up.
+        // 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
-- 
cgit v1.2.3


From 4219af1430f1cfc105d3521616941b7947fde4e3 Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Thu, 23 Aug 2018 14:22:59 -0700
Subject: Add DIVISION_BY_ZERO to getPartialAmount for consistency

---
 packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 d123c55a1..f4e2f1958 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
@@ -39,6 +39,8 @@ contract LibMath is
         pure
         returns (uint256 partialAmount)
     {
+        require(denominator > 0, "DIVISION_BY_ZERO");
+        
         partialAmount = safeDiv(
             safeMul(numerator, target),
             denominator
@@ -60,10 +62,7 @@ contract LibMath is
         pure
         returns (uint256 partialAmount)
     {
-        require(
-            denominator > 0,
-            "DIVISION_BY_ZERO"
-        );
+        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)
-- 
cgit v1.2.3


From 0fb7617a785b765415cb7f43448c9b8ea905963f Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Thu, 23 Aug 2018 15:18:45 -0700
Subject: Fix incorect modulus

---
 packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 f4e2f1958..1c14dbcae 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
@@ -141,10 +141,8 @@ contract LibMath is
             return false;
         }
         uint256 remainder = mulmod(target, numerator, denominator);
-        if (remainder == 0) {
-            return false;
-        }
-        remainder = safeSub(denominator, remainder);
+        // TODO: safeMod
+        remainder = safeSub(denominator, remainder) % denominator;
         isError = safeMul(1000, remainder) >= safeMul(numerator, target);
         return isError;
     }
-- 
cgit v1.2.3


From 6734f2f1bcdab8f0d50524a26195707da00bd8ed Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Thu, 23 Aug 2018 15:18:55 -0700
Subject: Add docs

---
 packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 1c14dbcae..8a1444af1 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
@@ -74,7 +74,7 @@ contract LibMath is
         return partialAmount;
     }
     
-    /// @dev Checks if rounding error > 0.1%.
+    /// @dev Checks if rounding error >= 0.1% when rounding down.
     /// @param numerator Numerator.
     /// @param denominator Denominator.
     /// @param target Value to multiply with numerator/denominator.
@@ -121,7 +121,7 @@ contract LibMath is
         return isError;
     }
     
-    /// @dev Checks if rounding error > 0.1%.
+    /// @dev Checks if rounding error >= 0.1% when rounding up.
     /// @param numerator Numerator.
     /// @param denominator Denominator.
     /// @param target Value to multiply with numerator/denominator.
@@ -137,9 +137,14 @@ contract LibMath is
     {
         require(denominator > 0, "DIVISION_BY_ZERO");
         
+        // 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;
-- 
cgit v1.2.3


From f6080367fe3a90762afea76a653c5df1315c45fa Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Fri, 24 Aug 2018 14:11:45 -0700
Subject: Disambiguate the operator precedence

---
 packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 8a1444af1..90ec9e2b6 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
@@ -109,9 +109,9 @@ contract LibMath is
         
         // Otherwise, we want the relative rounding error to be strictly
         // less than 0.1%.
-        // The relative error is `remainder / numerator * target`.
+        // The relative error is `remainder / (numerator * target)`.
         // We want the relative error less than 1 / 1000:
-        //        remainder / numerator * denominator  <  1 / 1000
+        //        remainder / (numerator * denominator)  <  1 / 1000
         // or equivalently:
         //        1000 * remainder  <  numerator * target
         // so we have a rounding error iff:
-- 
cgit v1.2.3


From 80291caf7cef16f0b300484755960d92d6750a6a Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Fri, 24 Aug 2018 16:16:44 -0700
Subject: Append -Floor to getPartialAmount and isRoundingError

---
 .../contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol   | 8 ++++----
 .../contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol    | 4 ++--
 .../src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol         | 4 ++--
 packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol   | 8 ++++----
 4 files changed, 12 insertions(+), 12 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 ab5c6e507..f3a0591b2 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
@@ -320,7 +320,7 @@ contract MixinExchangeCore is
 
         // Validate fill order rounding
         require(
-            !isRoundingError(
+            !isRoundingErrorFloor(
                 takerAssetFilledAmount,
                 order.takerAssetAmount,
                 order.makerAssetAmount
@@ -376,17 +376,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 56b309a1b..20f4b1c33 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
@@ -183,7 +183,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
@@ -193,7 +193,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/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
index 86194f461..1b77cfbcb 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
@@ -298,7 +298,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
@@ -350,7 +350,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
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 90ec9e2b6..7d58f70ce 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 rounded down.
-    function getPartialAmount(
+    function getPartialAmountFloor(
         uint256 numerator,
         uint256 denominator,
         uint256 target
@@ -48,7 +48,7 @@ contract LibMath is
         return partialAmount;
     }
     
-    /// @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.
@@ -79,7 +79,7 @@ contract LibMath is
     /// @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
-- 
cgit v1.2.3


From bc686fcbf3a0f9eea0c96107b9880314027f2d02 Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Fri, 24 Aug 2018 16:17:02 -0700
Subject: Stylistic fixes

---
 .../src/2.0.0/protocol/Exchange/libs/LibMath.sol   | 25 +++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 7d58f70ce..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
@@ -39,7 +39,10 @@ contract LibMath is
         pure
         returns (uint256 partialAmount)
     {
-        require(denominator > 0, "DIVISION_BY_ZERO");
+        require(
+            denominator > 0,
+            "DIVISION_BY_ZERO"
+        );
         
         partialAmount = safeDiv(
             safeMul(numerator, target),
@@ -62,13 +65,19 @@ contract LibMath is
         pure
         returns (uint256 partialAmount)
     {
-        require(denominator > 0, "DIVISION_BY_ZERO");
+        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)),
+            safeAdd(
+                safeMul(numerator, target),
+                safeSub(denominator, 1)
+            ),
             denominator
         );
         return partialAmount;
@@ -88,7 +97,10 @@ contract LibMath is
         pure
         returns (bool isError)
     {
-        require(denominator > 0, "DIVISION_BY_ZERO");
+        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
@@ -135,7 +147,10 @@ contract LibMath is
         pure
         returns (bool isError)
     {
-        require(denominator > 0, "DIVISION_BY_ZERO");
+        require(
+            denominator > 0,
+            "DIVISION_BY_ZERO"
+        );
         
         // See the comments in `isRoundingError`.
         if (target == 0 || numerator == 0) {
-- 
cgit v1.2.3


From 11328bd93d498abfcfbfa38ed69db8cb268f12be Mon Sep 17 00:00:00 2001
From: Remco Bloemen <remco@wicked.ventures>
Date: Fri, 24 Aug 2018 16:26:48 -0700
Subject: Skip self-transfers

---
 .../contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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,
-- 
cgit v1.2.3


From 044415e23d3a079e6301d8fb838da60456794c4f Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Mon, 20 Aug 2018 17:37:23 -0700
Subject: Remove redundant sload from getCurrentContextAddress

---
 packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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;
     }
 }
-- 
cgit v1.2.3


From 6f88e9bdbdd8f44c45053b8c17c84411f9499084 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Tue, 21 Aug 2018 16:06:21 -0700
Subject: Add internal fill functions, add reentrancy guard to public functions
 that make external calls

---
 .../2.0.0/protocol/Exchange/MixinExchangeCore.sol  | 86 ++++++++++++++--------
 .../2.0.0/protocol/Exchange/MixinMatchOrders.sol   |  3 +
 .../protocol/Exchange/MixinWrapperFunctions.sol    | 46 +++++++++---
 .../protocol/Exchange/mixins/MExchangeCore.sol     | 13 ++++
 .../protocol/Exchange/mixins/MWrapperFunctions.sol | 40 ++++++++++
 5 files changed, 148 insertions(+), 40 deletions(-)
 create mode 100644 packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 f3a0591b2..d00323b15 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,
@@ -86,43 +88,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();
-
-        // Get amount of takerAsset to fill
-        uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount);
-        uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount);
-
-        // Validate context
-        assertValidFill(
+        fillResults = fillOrderInternal(
             order,
-            orderInfo,
-            takerAddress,
             takerAssetFillAmount,
-            takerAssetFilledAmount,
             signature
         );
-
-        // 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;
     }
 
@@ -203,6 +176,57 @@ 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();
+
+        // Get amount of takerAsset to fill
+        uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount);
+        uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount);
+
+        // Validate context
+        assertValidFill(
+            order,
+            orderInfo,
+            takerAddress,
+            takerAssetFillAmount,
+            takerAssetFilledAmount,
+            signature
+        );
+
+        // 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.
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 20f4b1c33..25220a673 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.
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 1b77cfbcb..ff1cb6995 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
@@ -19,14 +19,17 @@
 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,
@@ -43,17 +46,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;
     }
 
@@ -117,11 +117,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 +144,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 +197,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory takerAssetData = orders[0].takerAssetData;
@@ -210,7 +213,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 +285,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory makerAssetData = orders[0].makerAssetData;
@@ -305,7 +309,7 @@ contract MixinWrapperFunctions is
             );
 
             // Attempt to sell the remaining amount of takerAsset
-            FillResults memory singleFillResults = fillOrder(
+            FillResults memory singleFillResults = fillOrderInternal(
                 orders[i],
                 remainingTakerAssetFillAmount,
                 signatures[i]
@@ -400,4 +404,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/mixins/MExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol
index c165b647c..41832fe4b 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);
+}
-- 
cgit v1.2.3


From c75212bef0b3801ecda09a89d5e30e359dcf1b63 Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Wed, 22 Aug 2018 17:58:47 -0700
Subject: Add nonReentrant modifiers on functions that use
 getCurrentContextAddress only, add lockMutex modifier on functions that make
 external calls

---
 .../2.0.0/protocol/Exchange/MixinExchangeCore.sol  |  4 +++-
 .../2.0.0/protocol/Exchange/MixinMatchOrders.sol   |  2 +-
 .../protocol/Exchange/MixinSignatureValidator.sol  |  3 +++
 .../protocol/Exchange/MixinWrapperFunctions.sol    | 23 ++++++++++------------
 4 files changed, 17 insertions(+), 15 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 d00323b15..f02514fc6 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
@@ -56,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.
@@ -88,7 +89,7 @@ contract MixinExchangeCore is
         bytes memory signature
     )
         public
-        nonReentrant
+        lockMutex
         returns (FillResults memory fillResults)
     {
         fillResults = fillOrderInternal(
@@ -104,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);
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 25220a673..39c47e6f9 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
@@ -50,7 +50,7 @@ contract MixinMatchOrders is
         bytes memory rightSignature
     )
         public
-        nonReentrant
+        lockMutex
         returns (LibFillResults.MatchedFillResults memory matchedFillResults)
     {
         // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData.
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/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
index ff1cb6995..b0474b110 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
@@ -46,7 +46,7 @@ contract MixinWrapperFunctions is
         bytes memory signature
     )
         public
-        nonReentrant
+        lockMutex
         returns (FillResults memory fillResults)
     {
         fillResults = fillOrKillOrderInternal(
@@ -69,6 +69,7 @@ contract MixinWrapperFunctions is
         bytes memory signature
     )
         public
+        nonReentrant
         returns (FillResults memory fillResults)
     {
         // ABI encode calldata for `fillOrder`
@@ -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,7 +111,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        nonReentrant
+        lockMutex
         returns (FillResults memory totalFillResults)
     {
         uint256 ordersLength = orders.length;
@@ -144,7 +138,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        nonReentrant
+        lockMutex
         returns (FillResults memory totalFillResults)
     {
         uint256 ordersLength = orders.length;
@@ -172,6 +166,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         uint256 ordersLength = orders.length;
@@ -197,7 +192,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        nonReentrant
+        lockMutex
         returns (FillResults memory totalFillResults)
     {
         bytes memory takerAssetData = orders[0].takerAssetData;
@@ -242,6 +237,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory takerAssetData = orders[0].takerAssetData;
@@ -285,7 +281,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        nonReentrant
+        lockMutex
         returns (FillResults memory totalFillResults)
     {
         bytes memory makerAssetData = orders[0].makerAssetData;
@@ -338,6 +334,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory makerAssetData = orders[0].makerAssetData;
-- 
cgit v1.2.3


From c28c3db63fb80d5d5e82ccd3b327cb8c408cc6fa Mon Sep 17 00:00:00 2001
From: Amir Bandeali <abandeali1@gmail.com>
Date: Thu, 23 Aug 2018 16:43:46 -0700
Subject: Only use one nonReentrant modifier, remove modifier from
 fillOrderNoThrow variations

---
 .../src/2.0.0/protocol/Exchange/MixinExchangeCore.sol   |  2 +-
 .../src/2.0.0/protocol/Exchange/MixinMatchOrders.sol    |  2 +-
 .../2.0.0/protocol/Exchange/MixinWrapperFunctions.sol   | 17 +++++++----------
 3 files changed, 9 insertions(+), 12 deletions(-)

(limited to 'packages/contracts/src/2.0.0/protocol/Exchange')

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 f02514fc6..291af3792 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
@@ -89,7 +89,7 @@ contract MixinExchangeCore is
         bytes memory signature
     )
         public
-        lockMutex
+        nonReentrant
         returns (FillResults memory fillResults)
     {
         fillResults = fillOrderInternal(
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 39c47e6f9..25220a673 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
@@ -50,7 +50,7 @@ contract MixinMatchOrders is
         bytes memory rightSignature
     )
         public
-        lockMutex
+        nonReentrant
         returns (LibFillResults.MatchedFillResults memory matchedFillResults)
     {
         // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData.
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 b0474b110..39fa724cc 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
@@ -33,7 +33,8 @@ contract MixinWrapperFunctions is
     LibMath,
     LibFillResults,
     LibAbiEncoder,
-    MExchangeCore
+    MExchangeCore,
+    MWrapperFunctions
 {
 
     /// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
@@ -46,7 +47,7 @@ contract MixinWrapperFunctions is
         bytes memory signature
     )
         public
-        lockMutex
+        nonReentrant
         returns (FillResults memory fillResults)
     {
         fillResults = fillOrKillOrderInternal(
@@ -69,7 +70,6 @@ contract MixinWrapperFunctions is
         bytes memory signature
     )
         public
-        nonReentrant
         returns (FillResults memory fillResults)
     {
         // ABI encode calldata for `fillOrder`
@@ -111,7 +111,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        lockMutex
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         uint256 ordersLength = orders.length;
@@ -138,7 +138,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        lockMutex
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         uint256 ordersLength = orders.length;
@@ -166,7 +166,6 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         uint256 ordersLength = orders.length;
@@ -192,7 +191,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        lockMutex
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory takerAssetData = orders[0].takerAssetData;
@@ -237,7 +236,6 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory takerAssetData = orders[0].takerAssetData;
@@ -281,7 +279,7 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        lockMutex
+        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory makerAssetData = orders[0].makerAssetData;
@@ -334,7 +332,6 @@ contract MixinWrapperFunctions is
         bytes[] memory signatures
     )
         public
-        nonReentrant
         returns (FillResults memory totalFillResults)
     {
         bytes memory makerAssetData = orders[0].makerAssetData;
-- 
cgit v1.2.3