Style changes to atomic order matching

9 files changed, 287 insertions, 304 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index 981c5984e..2c23575c7 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -53,6 +53,20 @@ contract MixinExchangeCore is
 
     ////// Core exchange functions //////
 
+    /// @dev Cancels all orders reated by sender with a salt less than or equal to the specified salt value.
+    /// @param salt Orders created with a salt less or equal to this value will be cancelled.
+    function cancelOrdersUpTo(uint256 salt)
+        external
+    {
+        uint256 newMakerEpoch = salt + 1;  // makerEpoch is initialized to 0, so to cancelUpTo we need salt + 1
+        require(
+            newMakerEpoch > makerEpoch[msg.sender],  // epoch must be monotonically increasing
+            INVALID_NEW_MAKER_EPOCH
+        );
+        makerEpoch[msg.sender] = newMakerEpoch;
+        emit CancelUpTo(msg.sender, newMakerEpoch);
+    }
+
     /// @dev Gets information about an order: status, hash, and amount filled.
     /// @param order Order to gather information on.
     /// @return status Status of order. Statuses are defined in the LibStatus.Status struct.
@@ -64,20 +78,11 @@ contract MixinExchangeCore is
         returns (
             uint8 orderStatus,
             bytes32 orderHash,
-            uint256 takerAssetFilledAmount)
+            uint256 takerAssetFilledAmount
+        )
     {
-        // Compute the order hash and fetch filled amount
+        // Compute the order hash
         orderHash = getOrderHash(order);
-        takerAssetFilledAmount = filled[orderHash];
-
-        // If order.takerAssetAmount is zero, then the order will always
-        // be considered filled because 0 == takerAssetAmount == takerAssetFilledAmount
-        // Instead of distinguishing between unfilled and filled zero taker
-        // amount orders, we choose not to support them.
-        if (order.takerAssetAmount == 0) {
-            orderStatus = uint8(Status.ORDER_INVALID_TAKER_ASSET_AMOUNT);
-            return (orderStatus, orderHash, takerAssetFilledAmount);
-        }
 
         // If order.makerAssetAmount is zero, we also reject the order.
         // While the Exchange contract handles them correctly, they create
@@ -88,15 +93,18 @@ contract MixinExchangeCore is
             return (orderStatus, orderHash, takerAssetFilledAmount);
         }
 
-        // Validate order expiration
-        if (block.timestamp >= order.expirationTimeSeconds) {
-            orderStatus = uint8(Status.ORDER_EXPIRED);
+        // If order.takerAssetAmount is zero, then the order will always
+        // be considered filled because 0 == takerAssetAmount == takerAssetFilledAmount
+        // Instead of distinguishing between unfilled and filled zero taker
+        // amount orders, we choose not to support them.
+        if (order.takerAssetAmount == 0) {
+            orderStatus = uint8(Status.ORDER_INVALID_TAKER_ASSET_AMOUNT);
             return (orderStatus, orderHash, takerAssetFilledAmount);
         }
 
-        // Validate order availability
-        if (takerAssetFilledAmount >= order.takerAssetAmount) {
-            orderStatus = uint8(Status.ORDER_FULLY_FILLED);
+        // Validate order expiration
+        if (block.timestamp >= order.expirationTimeSeconds) {
+            orderStatus = uint8(Status.ORDER_EXPIRED);
             return (orderStatus, orderHash, takerAssetFilledAmount);
         }
 
@@ -110,6 +118,13 @@ contract MixinExchangeCore is
             return (orderStatus, orderHash, takerAssetFilledAmount);
         }
 
+        // Fetch filled amount and validate order availability
+        takerAssetFilledAmount = filled[orderHash];
+        if (takerAssetFilledAmount >= order.takerAssetAmount) {
+            orderStatus = uint8(Status.ORDER_FULLY_FILLED);
+            return (orderStatus, orderHash, takerAssetFilledAmount);
+        }
+
         // All other statuses are ruled out: order is Fillable
         orderStatus = uint8(Status.ORDER_FILLABLE);
         return (orderStatus, orderHash, takerAssetFilledAmount);
@@ -119,18 +134,18 @@ contract MixinExchangeCore is
     /// @param order to be filled.
     /// @param orderStatus Status of order to be filled.
     /// @param orderHash Hash of order to be filled.
-    /// @param takerAssetFilledAmount Amount of order already filled.
-    /// @param signature Proof that the orders was created by its maker.
     /// @param takerAddress Address of order taker.
+    /// @param takerAssetFilledAmount Amount of order already filled.
     /// @param takerAssetFillAmount Desired amount of order to fill by taker.
-    function validateFillOrderContextOrRevert(
+    /// @param signature Proof that the orders was created by its maker.
+    function validateFillOrRevert(
         Order memory order,
         uint8 orderStatus,
         bytes32 orderHash,
-        uint256 takerAssetFilledAmount,
-        bytes memory signature,
         address takerAddress,
-        uint256 takerAssetFillAmount)
+        uint256 takerAssetFilledAmount,
+        uint256 takerAssetFillAmount,
+        bytes memory signature)
         internal
     {
         // Ensure order is valid
@@ -190,23 +205,24 @@ contract MixinExchangeCore is
         pure
         returns (
             uint8 status,
-            FillResults memory fillResults)
+            FillResults memory fillResults
+        )
     {
-        // Fill Amount must be greater than 0
-        if (takerAssetFillAmount <= 0) {
+        // Fill amount must be greater than 0
+        if (takerAssetFillAmount == 0) {
             status = uint8(Status.TAKER_ASSET_FILL_AMOUNT_TOO_LOW);
             return;
         }
 
         // Ensure the order is fillable
         if (orderStatus != uint8(Status.ORDER_FILLABLE)) {
-            status = uint8(orderStatus);
+            status = orderStatus;
             return;
         }
 
         // Compute takerAssetFilledAmount
-        uint256 remainingtakerAssetAmount = safeSub(order.takerAssetAmount, takerAssetFilledAmount);
-        fillResults.takerAssetFilledAmount = min256(takerAssetFillAmount, remainingtakerAssetAmount);
+        uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, takerAssetFilledAmount);
+        fillResults.takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount);
 
         // Validate fill order rounding
         if (isRoundingError(
@@ -242,19 +258,32 @@ contract MixinExchangeCore is
     /// @dev Updates state with results of a fill order.
     /// @param order that was filled.
     /// @param takerAddress Address of taker who filled the order.
+    /// @param takerAssetFilledAmount Amount of order already filled.
     /// @return fillResults Amounts filled and fees paid by maker and taker.
     function updateFilledState(
         Order memory order,
         address takerAddress,
         bytes32 orderHash,
+        uint256 takerAssetFilledAmount,
         FillResults memory fillResults)
         internal
     {
         // Update state
-        filled[orderHash] = safeAdd(filled[orderHash], fillResults.takerAssetFilledAmount);
+        filled[orderHash] = safeAdd(takerAssetFilledAmount, fillResults.takerAssetFilledAmount);
 
         // Log order
-        emitFillEvent(order, takerAddress, orderHash, fillResults);
+        emit Fill(
+            order.makerAddress,
+            takerAddress,
+            order.feeRecipientAddress,
+            fillResults.makerAssetFilledAmount,
+            fillResults.takerAssetFilledAmount,
+            fillResults.makerFeePaid,
+            fillResults.takerFeePaid,
+            orderHash,
+            order.makerAssetData,
+            order.takerAssetData
+        );
     }
 
     /// @dev Fills the input order.
@@ -279,7 +308,15 @@ contract MixinExchangeCore is
         address takerAddress = getCurrentContextAddress();
 
         // Either our context is valid or we revert
-        validateFillOrderContextOrRevert(order, orderStatus, orderHash, takerAssetFilledAmount, signature, takerAddress, takerAssetFillAmount);
+        validateFillOrRevert(
+            order,
+            orderStatus,
+            orderHash,
+            takerAddress,
+            takerAssetFilledAmount,
+            takerAssetFillAmount,
+            signature
+        );
 
         // Compute proportional fill amounts
         uint8 status;
@@ -290,11 +327,16 @@ contract MixinExchangeCore is
         }
 
         // Settle order
-        (fillResults.makerAssetFilledAmount, fillResults.makerFeePaid, fillResults.takerFeePaid) =
-            settleOrder(order, takerAddress, fillResults.takerAssetFilledAmount);
+        settleOrder(order, takerAddress, fillResults);
 
         // Update exchange internal state
-        updateFilledState(order, takerAddress, orderHash, fillResults);
+        updateFilledState(
+            order,
+            takerAddress,
+            orderHash,
+            takerAssetFilledAmount,
+            fillResults
+        );
         return fillResults;
     }
 
@@ -302,7 +344,7 @@ contract MixinExchangeCore is
     /// @param order that was cancelled.
     /// @param orderStatus Status of order that was cancelled.
     /// @param orderHash Hash of order that was cancelled.
-    function validateCancelOrderContextOrRevert(
+    function validateCancelOrRevert(
         Order memory order,
         uint8 orderStatus,
         bytes32 orderHash)
@@ -386,50 +428,12 @@ contract MixinExchangeCore is
         // Fetch current order status
         bytes32 orderHash;
         uint8 orderStatus;
-        uint256 takerAssetFilledAmount;
-        (orderStatus, orderHash, takerAssetFilledAmount) = getOrderInfo(order);
+        (orderStatus, orderHash, ) = getOrderInfo(order);
 
         // Validate context
-        validateCancelOrderContextOrRevert(order, orderStatus, orderHash);
+        validateCancelOrRevert(order, orderStatus, orderHash);
 
         // Perform cancel
         return updateCancelledState(order, orderStatus, orderHash);
     }
-
-    /// @dev Cancels all orders reated by sender with a salt less than or equal to the specified salt value.
-    /// @param salt Orders created with a salt less or equal to this value will be cancelled.
-    function cancelOrdersUpTo(uint256 salt)
-        external
-    {
-        uint256 newMakerEpoch = salt + 1;  // makerEpoch is initialized to 0, so to cancelUpTo we need salt + 1
-        require(
-            newMakerEpoch > makerEpoch[msg.sender],  // epoch must be monotonically increasing
-            INVALID_NEW_MAKER_EPOCH
-        );
-        makerEpoch[msg.sender] = newMakerEpoch;
-        emit CancelUpTo(msg.sender, newMakerEpoch);
-    }
-
-    /// @dev Logs a Fill event with the given arguments.
-    ///      The sole purpose of this function is to get around the stack variable limit.
-    function emitFillEvent(
-        Order memory order,
-        address takerAddress,
-        bytes32 orderHash,
-        FillResults memory fillResults)
-        internal
-    {
-        emit Fill(
-            order.makerAddress,
-            takerAddress,
-            order.feeRecipientAddress,
-            fillResults.makerAssetFilledAmount,
-            fillResults.takerAssetFilledAmount,
-            fillResults.makerFeePaid,
-            fillResults.takerFeePaid,
-            orderHash,
-            order.makerAssetData,
-            order.takerAssetData
-        );
-    }
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
index a333b8221..77ba0a089 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
@@ -36,12 +36,115 @@ contract MixinMatchOrders is
     MMatchOrders,
     MSettlement,
     MTransactions
+{
+
+    /// @dev Match two complementary orders that have a positive spread.
+    ///      Each order is filled at their respective price point. However, the calculations are
+    ///      carried out as though the orders are both being filled at the right order's price point.
+    ///      The profit made by the left order goes to the taker (who matched the two orders).
+    /// @param leftOrder First order to match.
+    /// @param rightOrder Second order to match.
+    /// @param leftSignature Proof that order was created by the left maker.
+    /// @param rightSignature Proof that order was created by the right maker.
+    /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
+    function matchOrders(
+        Order memory leftOrder,
+        Order memory rightOrder,
+        bytes leftSignature,
+        bytes rightSignature)
+        public
+        returns (MatchedFillResults memory matchedFillResults)
     {
+        // Get left status
+        OrderInfo memory leftOrderInfo;
+        (   leftOrderInfo.orderStatus,
+            leftOrderInfo.orderHash,
+            leftOrderInfo.orderFilledAmount
+        ) = getOrderInfo(leftOrder);
+        if (leftOrderInfo.orderStatus != uint8(Status.ORDER_FILLABLE)) {
+            emit ExchangeStatus(uint8(leftOrderInfo.orderStatus), leftOrderInfo.orderHash);
+            return matchedFillResults;
+        }
+
+        // Get right status
+        OrderInfo memory rightOrderInfo;
+        (   rightOrderInfo.orderStatus,
+            rightOrderInfo.orderHash,
+            rightOrderInfo.orderFilledAmount
+        ) = getOrderInfo(rightOrder);
+        if (rightOrderInfo.orderStatus != uint8(Status.ORDER_FILLABLE)) {
+            emit ExchangeStatus(uint8(rightOrderInfo.orderStatus), rightOrderInfo.orderHash);
+            return matchedFillResults;
+        }
+
+        // Fetch taker address
+        address takerAddress = getCurrentContextAddress();
+
+        // Either our context is valid or we revert
+        validateMatchOrThrow(leftOrder, rightOrder);
+
+        // Compute proportional fill amounts
+        uint8 matchedFillAmountsStatus;
+        (   matchedFillAmountsStatus,
+            matchedFillResults
+        ) = calculateMatchedFillResults(
+            leftOrder,
+            rightOrder,
+            leftOrderInfo.orderStatus,
+            rightOrderInfo.orderStatus,
+            leftOrderInfo.orderFilledAmount,
+            rightOrderInfo.orderFilledAmount);
+        if (matchedFillAmountsStatus != uint8(Status.SUCCESS)) {
+            return matchedFillResults;
+        }
+
+        // Validate fill contexts
+        validateFillOrRevert(
+            leftOrder,
+            leftOrderInfo.orderStatus,
+            leftOrderInfo.orderHash,
+            takerAddress,
+            leftOrderInfo.orderFilledAmount,
+            matchedFillResults.left.takerAssetFilledAmount,
+            leftSignature
+        );
+        validateFillOrRevert(
+            rightOrder,
+            rightOrderInfo.orderStatus,
+            rightOrderInfo.orderHash,
+            takerAddress,
+            rightOrderInfo.orderFilledAmount,
+            matchedFillResults.right.takerAssetFilledAmount,
+            rightSignature
+        );
+
+        // Settle matched orders. Succeeds or throws.
+        settleMatchedOrders(leftOrder, rightOrder, matchedFillResults, takerAddress);
+
+        // Update exchange state
+        updateFilledState(
+            leftOrder,
+            takerAddress,
+            leftOrderInfo.orderHash,
+            leftOrderInfo.orderFilledAmount,
+            matchedFillResults.left
+        );
+        updateFilledState(
+            rightOrder,
+            takerAddress,
+            rightOrderInfo.orderHash,
+            rightOrderInfo.orderFilledAmount,
+            matchedFillResults.right
+        );
+
+        // Return results
+        return matchedFillResults;
+    }
 
     /// @dev Validates context for matchOrders. Succeeds or throws.
     /// @param leftOrder First order to match.
     /// @param rightOrder Second order to match.
-    function validateMatchOrdersContextOrRevert(
+    function validateMatchOrThrow(
         Order memory leftOrder,
         Order memory rightOrder)
         internal
@@ -62,7 +165,7 @@ contract MixinMatchOrders is
         // There is a positive spread iff the cost per unit bought (OrderA.MakerAmount/OrderA.TakerAmount) for each order is greater
         // than the profit per unit sold of the matched order (OrderB.TakerAmount/OrderB.MakerAmount).
         // This is satisfied by the equations below:
-        // <leftOrder.makerAssetAmount> / <leftOrder.takerAssetAmount> = <rightOrder.takerAssetAmount> / <rightOrder.makerAssetAmount>
+        // <leftOrder.makerAssetAmount> / <leftOrder.takerAssetAmount> >= <rightOrder.takerAssetAmount> / <rightOrder.makerAssetAmount>
         // AND
         // <rightOrder.makerAssetAmount> / <rightOrder.takerAssetAmount> >= <leftOrder.takerAssetAmount> / <leftOrder.makerAssetAmount>
         // These equations can be combined to get the following:
@@ -75,7 +178,7 @@ contract MixinMatchOrders is
 
     /// @dev Validates matched fill results. Succeeds or throws.
     /// @param matchedFillResults Amounts to fill and fees to pay by maker and taker of matched orders.
-    function validateMatchedOrderFillResultsOrThrow(MatchedFillResults memory matchedFillResults)
+    function validateMatchOrThrow(MatchedFillResults memory matchedFillResults)
         internal
     {
         // The right order must spend at least as much as we're transferring to the left order.
@@ -95,26 +198,6 @@ contract MixinMatchOrders is
         );
     }
 
-    /// @dev Calculates partial value given a numerator and denominator.
-    ///      Throws if there is a rounding error.
-    /// @param numerator Numerator.
-    /// @param denominator Denominator.
-    /// @param target Value to calculate partial of.
-    /// @return Partial value of target.
-    function safeGetPartialAmount(
-        uint256 numerator,
-        uint256 denominator,
-        uint256 target)
-        internal pure
-        returns (uint256 partialAmount)
-    {
-        require(
-            !isRoundingError(numerator, denominator, target),
-            ROUNDING_ERROR_ON_PARTIAL_AMOUNT
-            );
-        return getPartialAmount(numerator, denominator, target);
-    }
-
     /// @dev Calculates fill amounts for the matched orders.
     ///      Each order is filled at their respective price point. However, the calculations are
     ///      carried out as though the orders are both being filled at the right order's price point.
@@ -137,7 +220,8 @@ contract MixinMatchOrders is
         internal
         returns (
             uint8 status,
-            MatchedFillResults memory matchedFillResults)
+            MatchedFillResults memory matchedFillResults
+        )
     {
         // We settle orders at the price point defined by the right order (profit goes to the order taker)
         // The constraint can be either on the left or on the right.
@@ -148,8 +232,8 @@ contract MixinMatchOrders is
         //    <leftTakerAssetAmountRemaining> * <rightOrder.takerAssetAmount> <= <rightTakerAssetAmountRemaining> * <rightOrder.makerAssetAmount>
         uint256 rightTakerAssetAmountRemaining = safeSub(rightOrder.takerAssetAmount, rightOrderFilledAmount);
         uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderFilledAmount);
-        uint256 leftOrderAmountToFill = 0;
-        uint256 rightOrderAmountToFill = 0;
+        uint256 leftOrderAmountToFill;
+        uint256 rightOrderAmountToFill;
         if (
             safeMul(leftTakerAssetAmountRemaining, rightOrder.takerAssetAmount) <=
             safeMul(rightTakerAssetAmountRemaining, rightOrder.makerAssetAmount)
@@ -162,7 +246,8 @@ contract MixinMatchOrders is
             rightOrderAmountToFill = safeGetPartialAmount(
                 rightOrder.takerAssetAmount,
                 rightOrder.makerAssetAmount,
-                leftOrderAmountToFill);
+                leftOrderAmountToFill
+            );
         } else {
             // Right order is the constraint: maximally fill right
             rightOrderAmountToFill = rightTakerAssetAmountRemaining;
@@ -172,136 +257,36 @@ contract MixinMatchOrders is
             leftOrderAmountToFill = safeGetPartialAmount(
                 rightOrder.makerAssetAmount,
                 rightOrder.takerAssetAmount,
-                rightOrderAmountToFill);
+                rightOrderAmountToFill
+            );
         }
 
         // Calculate fill results for left order
-        (   status,
-            matchedFillResults.left
-        ) = calculateFillResults(
+        (status, matchedFillResults.left) = calculateFillResults(
             leftOrder,
             leftOrderStatus,
             leftOrderFilledAmount,
-            leftOrderAmountToFill);
+            leftOrderAmountToFill
+        );
         if (status != uint8(Status.SUCCESS)) {
             return (status, matchedFillResults);
         }
 
         // Calculate fill results for right order
-        (   status,
-            matchedFillResults.right
-        ) = calculateFillResults(
+        (status, matchedFillResults.right) = calculateFillResults(
             rightOrder,
             rightOrderStatus,
             rightOrderFilledAmount,
-            rightOrderAmountToFill);
+            rightOrderAmountToFill
+        );
         if (status != uint8(Status.SUCCESS)) {
             return (status, matchedFillResults);
         }
 
         // Validate the fill results
-        validateMatchedOrderFillResultsOrThrow(matchedFillResults);
+        validateMatchOrThrow(matchedFillResults);
 
         // Return status & fill results
         return (status, matchedFillResults);
     }
-
-    /// @dev Match two complementary orders that have a positive spread.
-    ///      Each order is filled at their respective price point. However, the calculations are
-    ///      carried out as though the orders are both being filled at the right order's price point.
-    ///      The profit made by the left order goes to the taker (who matched the two orders).
-    /// @param leftOrder First order to match.
-    /// @param rightOrder Second order to match.
-    /// @param leftSignature Proof that order was created by the left maker.
-    /// @param rightSignature Proof that order was created by the right maker.
-    /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
-    function matchOrders(
-        Order memory leftOrder,
-        Order memory rightOrder,
-        bytes leftSignature,
-        bytes rightSignature)
-        public
-        returns (MatchedFillResults memory matchedFillResults)
-    {
-        // Get left status
-        OrderInfo memory leftOrderInfo;
-        (   leftOrderInfo.orderStatus,
-            leftOrderInfo.orderHash,
-            leftOrderInfo.orderFilledAmount
-        ) = getOrderInfo(leftOrder);
-        if (leftOrderInfo.orderStatus != uint8(Status.ORDER_FILLABLE)) {
-            emit ExchangeStatus(uint8(leftOrderInfo.orderStatus), leftOrderInfo.orderHash);
-            return matchedFillResults;
-        }
-
-        // Get right status
-        OrderInfo memory rightOrderInfo;
-        (   rightOrderInfo.orderStatus,
-            rightOrderInfo.orderHash,
-            rightOrderInfo.orderFilledAmount
-        ) = getOrderInfo(rightOrder);
-        if (rightOrderInfo.orderStatus != uint8(Status.ORDER_FILLABLE)) {
-            emit ExchangeStatus(uint8(rightOrderInfo.orderStatus), rightOrderInfo.orderHash);
-            return matchedFillResults;
-        }
-
-        // Fetch taker address
-        address takerAddress = getCurrentContextAddress();
-
-        // Either our context is valid or we revert
-        validateMatchOrdersContextOrRevert(leftOrder, rightOrder);
-
-        // Compute proportional fill amounts
-        uint8 matchedFillAmountsStatus;
-        (   matchedFillAmountsStatus,
-            matchedFillResults
-        ) = calculateMatchedFillResults(
-            leftOrder,
-            rightOrder,
-            leftOrderInfo.orderStatus,
-            rightOrderInfo.orderStatus,
-            leftOrderInfo.orderFilledAmount,
-            rightOrderInfo.orderFilledAmount);
-        if (matchedFillAmountsStatus != uint8(Status.SUCCESS)) {
-            return matchedFillResults;
-        }
-
-        // Validate fill contexts
-        validateFillOrderContextOrRevert(
-            leftOrder,
-            leftOrderInfo.orderStatus,
-            leftOrderInfo.orderHash,
-            leftOrderInfo.orderFilledAmount,
-            leftSignature,
-            rightOrder.makerAddress,
-            matchedFillResults.left.takerAssetFilledAmount);
-        validateFillOrderContextOrRevert(
-            rightOrder,
-            rightOrderInfo.orderStatus,
-            rightOrderInfo.orderHash,
-            rightOrderInfo.orderFilledAmount,
-            rightSignature,
-            leftOrder.makerAddress,
-            matchedFillResults.right.takerAssetFilledAmount);
-
-        // Settle matched orders. Succeeds or throws.
-        settleMatchedOrders(leftOrder, rightOrder, matchedFillResults, takerAddress);
-
-        // Update exchange state
-        updateFilledState(
-            leftOrder,
-            rightOrder.makerAddress,
-            leftOrderInfo.orderHash,
-            matchedFillResults.left
-        );
-        updateFilledState(
-            rightOrder,
-            leftOrder.makerAddress,
-            rightOrderInfo.orderHash,
-            matchedFillResults.right
-        );
-
-        // Return results
-        return matchedFillResults;
-    }
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
index cde64ba74..cb96bbc3e 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
@@ -22,11 +22,13 @@ import "./mixins/MSettlement.sol";
 import "./mixins/MAssetProxyDispatcher.sol";
 import "./libs/LibOrder.sol";
 import "./libs/LibMath.sol";
+import "./libs/LibExchangeErrors.sol";
+import "./libs/LibFillResults.sol";
 import "./mixins/MMatchOrders.sol";
-import "./mixins/LibExchangeErrors.sol";
 
 contract MixinSettlement is
     LibMath,
+    LibFillResults,
     LibExchangeErrors,
     MMatchOrders,
     MSettlement,
@@ -58,47 +60,37 @@ contract MixinSettlement is
     /// @dev Settles an order by transferring assets between counterparties.
     /// @param order Order struct containing order specifications.
     /// @param takerAddress Address selling takerAsset and buying makerAsset.
-    /// @param takerAssetFilledAmount The amount of takerAsset that will be transferred to the order's maker.
-    /// @return Amount filled by maker and fees paid by maker/taker.
+    /// @param fillResults Amounts to be filled and fees paid by maker and taker.
     function settleOrder(
         LibOrder.Order memory order,
         address takerAddress,
-        uint256 takerAssetFilledAmount)
+        FillResults memory fillResults)
         internal
-        returns (
-            uint256 makerAssetFilledAmount,
-            uint256 makerFeePaid,
-            uint256 takerFeePaid
-        )
     {
-        makerAssetFilledAmount = getPartialAmount(takerAssetFilledAmount, order.takerAssetAmount, order.makerAssetAmount);
         dispatchTransferFrom(
             order.makerAssetData,
             order.makerAddress,
             takerAddress,
-            makerAssetFilledAmount
+            fillResults.makerAssetFilledAmount
         );
         dispatchTransferFrom(
             order.takerAssetData,
             takerAddress,
             order.makerAddress,
-            takerAssetFilledAmount
+            fillResults.takerAssetFilledAmount
         );
-        makerFeePaid = getPartialAmount(takerAssetFilledAmount, order.takerAssetAmount, order.makerFee);
         dispatchTransferFrom(
             ZRX_PROXY_DATA,
             order.makerAddress,
             order.feeRecipientAddress,
-            makerFeePaid
+            fillResults.makerFeePaid
         );
-        takerFeePaid = getPartialAmount(takerAssetFilledAmount, order.takerAssetAmount, order.takerFee);
         dispatchTransferFrom(
             ZRX_PROXY_DATA,
             takerAddress,
             order.feeRecipientAddress,
-            takerFeePaid
+            fillResults.takerFeePaid
         );
-        return (makerAssetFilledAmount, makerFeePaid, takerFeePaid);
     }
 
     /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
index ba055dd8e..5ef0a52c5 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
@@ -55,5 +55,4 @@ contract LibExchangeErrors {
     string constant NEGATIVE_SPREAD = "Matched orders must have a positive spread.";
     string constant MISCALCULATED_TRANSFER_AMOUNTS = "A miscalculation occurred: the left maker would receive more than the right maker would spend.";
     string constant ROUNDING_ERROR_TRANSFER_AMOUNTS = "A rounding error occurred when calculating transfer amounts for matched orders.";
-    string constant ROUNDING_ERROR_ON_PARTIAL_AMOUNT = "A rounding error occurred when calculating partial transfer amounts.";
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
index b48602d19..6bae03ff2 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
@@ -23,6 +23,7 @@ import "../../../utils/SafeMath/SafeMath.sol";
 contract LibMath is
     SafeMath
 {
+    string constant ROUNDING_ERROR_ON_PARTIAL_AMOUNT = "A rounding error occurred when calculating partial transfer amounts.";
 
     /// @dev Calculates partial value given a numerator and denominator.
     /// @param numerator Numerator.
@@ -44,6 +45,26 @@ contract LibMath is
         return partialAmount;
     }
 
+    /// @dev Calculates partial value given a numerator and denominator.
+    ///      Throws if there is a rounding error.
+    /// @param numerator Numerator.
+    /// @param denominator Denominator.
+    /// @param target Value to calculate partial of.
+    /// @return Partial value of target.
+    function safeGetPartialAmount(
+        uint256 numerator,
+        uint256 denominator,
+        uint256 target)
+        internal pure
+        returns (uint256 partialAmount)
+    {
+        require(
+            !isRoundingError(numerator, denominator, target),
+            ROUNDING_ERROR_ON_PARTIAL_AMOUNT
+            );
+        return getPartialAmount(numerator, denominator, target);
+    }
+
     /// @dev Checks if rounding error > 0.1%.
     /// @param numerator Numerator.
     /// @param denominator Denominator.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
index cfd2678ba..6f7d99c48 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -56,14 +56,10 @@ contract MExchangeCore is
         uint256 makerEpoch
     );
 
-    /// @dev Logs a Fill event with the given arguments.
-    ///      The sole purpose of this function is to get around the stack variable limit.
-    function emitFillEvent(
-        LibOrder.Order memory order,
-        address takerAddress,
-        bytes32 orderHash,
-        LibFillResults.FillResults memory fillResults)
-        internal;
+    /// @dev Cancels all orders reated by sender with a salt less than or equal to the specified salt value.
+    /// @param salt Orders created with a salt less or equal to this value will be cancelled.
+    function cancelOrdersUpTo(uint256 salt)
+        external;
 
     /// @dev Gets information about an order: status, hash, and amount filled.
     /// @param order Order to gather information on.
@@ -76,24 +72,25 @@ contract MExchangeCore is
         returns (
             uint8 orderStatus,
             bytes32 orderHash,
-            uint256 takerAssetFilledAmount);
+            uint256 takerAssetFilledAmount
+        );
 
     /// @dev Validates context for fillOrder. Succeeds or throws.
     /// @param order to be filled.
     /// @param orderStatus Status of order to be filled.
     /// @param orderHash Hash of order to be filled.
-    /// @param takerAssetFilledAmount Amount of order already filled.
-    /// @param signature Proof that the orders was created by its maker.
     /// @param takerAddress Address of order taker.
+    /// @param takerAssetFilledAmount Amount of order already filled.
     /// @param takerAssetFillAmount Desired amount of order to fill by taker.
-    function validateFillOrderContextOrRevert(
+    /// @param signature Proof that the orders was created by its maker.
+    function validateFillOrRevert(
         LibOrder.Order memory order,
         uint8 orderStatus,
         bytes32 orderHash,
-        uint256 takerAssetFilledAmount,
-        bytes memory signature,
         address takerAddress,
-        uint256 takerAssetFillAmount)
+        uint256 takerAssetFilledAmount,
+        uint256 takerAssetFillAmount,
+        bytes memory signature)
         internal;
 
     /// @dev Calculates amounts filled and fees paid by maker and taker.
@@ -112,16 +109,19 @@ contract MExchangeCore is
         pure
         returns (
             uint8 status,
-            LibFillResults.FillResults memory fillResults);
+            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.
+    /// @param takerAssetFilledAmount Amount of order already filled.
     /// @return fillResults Amounts filled and fees paid by maker and taker.
     function updateFilledState(
         LibOrder.Order memory order,
         address takerAddress,
         bytes32 orderHash,
+        uint256 takerAssetFilledAmount,
         LibFillResults.FillResults memory fillResults)
         internal;
 
@@ -141,7 +141,7 @@ contract MExchangeCore is
     /// @param order that was cancelled.
     /// @param orderStatus Status of order that was cancelled.
     /// @param orderHash Hash of order that was cancelled.
-    function validateCancelOrderContextOrRevert(
+    function validateCancelOrRevert(
         LibOrder.Order memory order,
         uint8 orderStatus,
         bytes32 orderHash)
@@ -162,15 +162,12 @@ contract MExchangeCore is
         returns (bool stateUpdated);
 
     /// @dev After calling, the order can not be filled anymore.
-    /// @param order Order struct containing order specifications.
+    ///      Throws if order is invalid or sender does not have permission to cancel.
+    /// @param order Order to cancel. Order must be Status.FILLABLE.
     /// @return True if the order state changed to cancelled.
-    ///         False if the transaction was already cancelled or expired.
+    ///         False if the order was order was in a valid, but
+    ///               unfillable state (see LibStatus.STATUS for order states)
     function cancelOrder(LibOrder.Order memory order)
         public
         returns (bool);
-
-    /// @dev Cancels all orders reated by sender with a salt less than or equal to the specified salt value.
-    /// @param salt Orders created with a salt less or equal to this value will be cancelled.
-    function cancelOrdersUpTo(uint256 salt)
-        external;
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
index 2aedeb3e6..a1e3f28a1 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
@@ -37,32 +37,36 @@ contract MMatchOrders {
         uint256 orderFilledAmount;
     }
 
+    /// @dev Match two complementary orders that have a positive spread.
+    ///      Each order is filled at their respective price point. However, the calculations are
+    ///      carried out as though the orders are both being filled at the right order's price point.
+    ///      The profit made by the left order goes to the taker (who matched the two orders).
+    /// @param leftOrder First order to match.
+    /// @param rightOrder Second order to match.
+    /// @param leftSignature Proof that order was created by the left maker.
+    /// @param rightSignature Proof that order was created by the right maker.
+    /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
+    function matchOrders(
+        LibOrder.Order memory leftOrder,
+        LibOrder.Order memory rightOrder,
+        bytes leftSignature,
+        bytes rightSignature)
+        public
+        returns (MatchedFillResults memory matchedFillResults);
+
     /// @dev Validates context for matchOrders. Succeeds or throws.
     /// @param leftOrder First order to match.
     /// @param rightOrder Second order to match.
-    function validateMatchOrdersContextOrRevert(
+    function validateMatchOrThrow(
         LibOrder.Order memory leftOrder,
         LibOrder.Order memory rightOrder)
         internal;
 
     /// @dev Validates matched fill results. Succeeds or throws.
     /// @param matchedFillResults Amounts to fill and fees to pay by maker and taker of matched orders.
-    function validateMatchedOrderFillResultsOrThrow(MatchedFillResults memory matchedFillResults)
+    function validateMatchOrThrow(MatchedFillResults memory matchedFillResults)
         internal;
 
-    /// @dev Calculates partial value given a numerator and denominator.
-    ///      Throws if there is a rounding error.
-    /// @param numerator Numerator.
-    /// @param denominator Denominator.
-    /// @param target Value to calculate partial of.
-    /// @return Partial value of target.
-    function safeGetPartialAmount(
-        uint256 numerator,
-        uint256 denominator,
-        uint256 target)
-        internal pure
-        returns (uint256 partialAmount);
-
     /// @dev Calculates fill amounts for the matched orders.
     ///      Each order is filled at their respective price point. However, the calculations are
     ///      carried out as though the orders are both being filled at the right order's price point.
@@ -85,22 +89,6 @@ contract MMatchOrders {
         internal
         returns (
             uint8 status,
-            MatchedFillResults memory matchedFillResults);
-
-    /// @dev Match two complementary orders that have a positive spread.
-    ///      Each order is filled at their respective price point. However, the calculations are
-    ///      carried out as though the orders are both being filled at the right order's price point.
-    ///      The profit made by the left order goes to the taker (who matched the two orders).
-    /// @param leftOrder First order to match.
-    /// @param rightOrder Second order to match.
-    /// @param leftSignature Proof that order was created by the left maker.
-    /// @param rightSignature Proof that order was created by the right maker.
-    /// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
-    function matchOrders(
-        LibOrder.Order memory leftOrder,
-        LibOrder.Order memory rightOrder,
-        bytes leftSignature,
-        bytes rightSignature)
-        public
-        returns (MatchedFillResults memory matchedFillResults);
+            MatchedFillResults memory matchedFillResults
+        );
 }
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
index ac38ede95..bd9042f62 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
@@ -20,24 +20,19 @@ pragma solidity ^0.4.23;
 
 import "../libs/LibOrder.sol";
 import "./MMatchOrders.sol";
+import "../libs/LibFillResults.sol";
 
 contract MSettlement {
 
-    /// @dev Settles an order by transfering assets between counterparties.
+    /// @dev Settles an order by transferring assets between counterparties.
     /// @param order Order struct containing order specifications.
     /// @param takerAddress Address selling takerAsset and buying makerAsset.
-    /// @param takerAssetFilledAmount The amount of takerAsset that will be transfered to the order's maker.
-    /// @return Amount filled by maker and fees paid by maker/taker.
+    /// @param fillResults Amounts to be filled and fees paid by maker and taker.
     function settleOrder(
         LibOrder.Order memory order,
         address takerAddress,
-        uint256 takerAssetFilledAmount)
-        internal
-        returns (
-            uint256 makerAssetFilledAmount,
-            uint256 makerFeePaid,
-            uint256 takerFeePaid
-        );
+        LibFillResults.FillResults memory fillResults)
+        internal;
 
     /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient.
     /// @param leftOrder First matched order.
diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts
index 6d36198f2..5b026fce0 100644
--- a/packages/contracts/src/utils/exchange_wrapper.ts
+++ b/packages/contracts/src/utils/exchange_wrapper.ts
@@ -225,12 +225,6 @@ export class ExchangeWrapper {
         const filledAmount = new BigNumber(await this._exchange.filled.callAsync(orderHashHex));
         return filledAmount;
     }
-    private async _getTxWithDecodedExchangeLogsAsync(txHash: string) {
-        const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash);
-        tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address);
-        tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log));
-        return tx;
-    }
     public async getOrderInfoAsync(
         signedOrder: SignedOrder,
     ): Promise<[number /* orderStatus */, string /* orderHash */, BigNumber /* orderTakerAssetAmountFilled */]> {
@@ -251,6 +245,14 @@ export class ExchangeWrapper {
             { from },
         );
         const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash);
+        tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address);
+        tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log));
+        return tx;
+    }
+    private async _getTxWithDecodedExchangeLogsAsync(txHash: string) {
+        const tx = await this._zeroEx.awaitTransactionMinedAsync(txHash);
+        tx.logs = _.filter(tx.logs, log => log.address === this._exchange.address);
+        tx.logs = _.map(tx.logs, log => this._logDecoder.decodeLogOrThrow(log));
         return tx;
     }
 }