Add overfill and price assertion to assertValidFill

2 files changed, 32 insertions, 2 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 ab5c6e507..e6b2ddf3d 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
@@ -266,6 +266,7 @@ contract MixinExchangeCore is
     /// @param takerAddress Address of order taker.
     /// @param takerAssetFillAmount Desired amount of order to fill by taker.
     /// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
+    /// @param makerAssetFilledAmount Amount of makerAsset that will be transfered.
     /// @param signature Proof that the orders was created by its maker.
     function assertValidFill(
         Order memory order,
@@ -273,6 +274,7 @@ contract MixinExchangeCore is
         address takerAddress,
         uint256 takerAssetFillAmount,
         uint256 takerAssetFilledAmount,
+        uint256 makerAssetFilledAmount,
         bytes memory signature
     )
         internal
@@ -297,7 +299,7 @@ contract MixinExchangeCore is
                 "INVALID_SENDER"
             );
         }
-
+        
         // Validate taker is allowed to fill this order
         if (order.takerAddress != address(0)) {
             require(
@@ -317,7 +319,33 @@ contract MixinExchangeCore is
                 "INVALID_ORDER_SIGNATURE"
             );
         }
-
+        
+        // Make sure taker does not pay more than desired amount
+        // NOTE: This assertion should never fail, it is here
+        //       as an extra defence against potential bugs.
+        require(
+            takerAssetFilledAmount <= takerAssetFilledAmount,
+            "BUG_TAKER_OVERPAY"
+        );
+        
+        // Make sure order is not overfilled
+        // NOTE: This assertion should never fail, it is here
+        //       as an extra defence against potential bugs.
+        require(
+            safeAdd(orderInfo.orderTakerAssetFilledAmount, takerAssetFilledAmount) <= order.takerAssetAmount,
+            "BUG_ORDER_OVERFILL"
+        );
+        
+        // Make sure order is filled at acceptable price
+        // NOTE: This assertion should never fail, it is here
+        //       as an extra defence against potential bugs.
+        require(
+            safeMul(makerAssetFilledAmount, order.takerAssetAmount)
+            <= 
+            safeMul(takerAssetFilledAmount, order.makerAssetAmount),
+            "BUG_ORDER_FILL_PRICING"
+        );
+        
         // Validate fill order rounding
         require(
             !isRoundingError(
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..eccb6a29d 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
@@ -90,6 +90,7 @@ contract MExchangeCore is
     /// @param takerAddress Address of order taker.
     /// @param takerAssetFillAmount Desired amount of order to fill by taker.
     /// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
+    /// @param makerAssetFilledAmount Amount of makerAsset that will be transfered.
     /// @param signature Proof that the orders was created by its maker.
     function assertValidFill(
         LibOrder.Order memory order,
@@ -97,6 +98,7 @@ contract MExchangeCore is
         address takerAddress,
         uint256 takerAssetFillAmount,
         uint256 takerAssetFilledAmount,
+        uint256 makerAssetFilledAmount,
         bytes memory signature
     )
         internal