diff options
11 files changed, 288 insertions, 306 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; } } diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index a114d92ac..6ba88b5fb 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -41,7 +41,7 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe('matchOrdersAndVerifyBalancesAsync', () => { +describe('matchOrders', () => { let makerAddressLeft: string; let makerAddressRight: string; let owner: string; diff --git a/packages/contracts/test/utils/match_order_tester.ts b/packages/contracts/test/utils/match_order_tester.ts index 91c2ab0a3..4e3459734 100644 --- a/packages/contracts/test/utils/match_order_tester.ts +++ b/packages/contracts/test/utils/match_order_tester.ts @@ -32,7 +32,6 @@ import { TransferAmountsByMatchOrders as TransferAmounts, } from '../../src/utils/types'; import { chaiSetup } from '../utils/chai_setup'; -import { deployer } from '../utils/deployer'; import { provider, web3Wrapper } from '../utils/web3_wrapper'; chaiSetup.configure(); |