From e706fa76acfbf933479f767749755446cdaf438a Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Fri, 17 Aug 2018 12:11:51 -0700 Subject: Add overfill and price assertion to assertValidFill --- .../2.0.0/protocol/Exchange/MixinExchangeCore.sol | 32 ++++++++++++++++++++-- .../protocol/Exchange/mixins/MExchangeCore.sol | 2 ++ 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 -- cgit v1.2.3