From a1d89435525795f6f412a823241740f10cbdf1d8 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Thu, 23 Aug 2018 16:00:01 -0700 Subject: Document accetable price check --- .../src/2.0.0/protocol/Exchange/MixinExchangeCore.sol | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'packages/contracts/src/2.0.0') 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 b3671e7e8..e5bd306dd 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol @@ -343,13 +343,27 @@ contract MixinExchangeCore is "BUG_ORDER_OVERFILL" ); - // Make sure order is filled at acceptable price + // Make sure order is filled at acceptable price. + // The order has an implied price from the makers perspective: + // order price = order.makerAssetAmount / order.takerAssetAmount + // i.e. the number of makerAsset maker is paying per takerAsset. The + // maker is guaranteed to get this price or a better (lower) one. The + // actual price maker is getting in this fill is: + // fill price = makerAssetFilledAmount / takerAssetFilledAmount + // We need `fill price <= order price` for the fill to be fair to maker. + // This amounts to: + // makerAssetFilledAmount order.makerAssetAmount + // ------------------------ <= ----------------------- + // takerAssetFilledAmount order.takerAssetAmount + // or, equivalently: + // makerAssetFilledAmount * order.takerAssetAmount <= + // order.makerAssetAmount * takerAssetFilledAmount // 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), + safeMul(order.makerAssetAmount, takerAssetFilledAmount), "BUG_ORDER_FILL_PRICING" ); -- cgit v1.2.3