Addressed lingering review comments

This commit is contained in:
James Towle 2019-07-08 23:28:43 -05:00 committed by Amir Bandeali
parent 5611cb91a0
commit 1c1d257bd9

View File

@ -127,121 +127,24 @@ contract MixinMatchOrders is
// Maximally fill the orders and pay out profits to the matcher in one or both of the maker assets. // Maximally fill the orders and pay out profits to the matcher in one or both of the maker assets.
if (shouldMaximallyFillOrders) { if (shouldMaximallyFillOrders) {
bool doesLeftMakerAssetProfitExist; _calculateMatchedFillResultsWithMaximalFill(
bool doesRightMakerAssetProfitExist; matchedFillResults,
leftOrder,
// Calculate the maximum fill results for the maker and taker assets. At least one of the orders will be fully filled. rightOrder,
// leftMakerAssetAmountRemaining,
// The maximum that the left maker can possibly buy is the amount that the right order can sell. leftTakerAssetAmountRemaining,
// The maximum that the right maker can possibly buy is the amount that the left order can sell. rightMakerAssetAmountRemaining,
// rightTakerAssetAmountRemaining
// If the left order is fully filled, profit will be paid out in the left maker asset. If the right order is fully filled, );
// the profit will be out in the right maker asset.
//
// There are three cases to consider:
// Case 1.
// If the left maker can buy more than the right maker can sell, then only the right order is fully filled.
// Case 2.
// If the right maker can buy more than the left maker can sell, then only the right order is fully filled.
// Case 3.
// If the right maker can sell the max of what the left maker can buy and the left maker can sell the max of
// what the right maker can buy, then both orders are fully filled.
if (leftTakerAssetAmountRemaining > rightMakerAssetAmountRemaining) {
// Case 1: Right order is fully filled with the profit paid in the left makerAsset
matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining;
matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = rightMakerAssetAmountRemaining;
// Round down to ensure the left maker's exchange rate does not exceed the price specified by the order.
// We favor the left maker when the exchange rate must be rounded and the profit is being paid in the
// left maker asset.
matchedFillResults.left.makerAssetFilledAmount = _safeGetPartialAmountFloor(
leftOrder.makerAssetAmount,
leftOrder.takerAssetAmount,
rightMakerAssetAmountRemaining
);
// Indicate that the profit should be set to the spread denominated in the left maker asset.
doesLeftMakerAssetProfitExist = true;
} else if (rightTakerAssetAmountRemaining > leftMakerAssetAmountRemaining) {
// Case 2: Left order is fully filled with the profit paid in the right makerAsset.
matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
// Round down to ensure the right maker's exchange rate does not exceed the price specified by the order.
// We favor the right maker when the exchange rate must be rounded and the profit is being paid in the
// right maker asset.
matchedFillResults.right.makerAssetFilledAmount = _safeGetPartialAmountFloor(
rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount,
leftMakerAssetAmountRemaining
);
matchedFillResults.right.takerAssetFilledAmount = leftMakerAssetAmountRemaining;
// Indicate that the profit should be set to the spread denominated in the right maker asset.
doesRightMakerAssetProfitExist = true;
} else {
// Case 3: The right and left orders are fully filled
matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining;
matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining;
matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
// Indicate that the profit should be set to the spread denominated in the left and the right maker assets.
doesLeftMakerAssetProfitExist = true;
doesRightMakerAssetProfitExist = true;
}
// Calculate amount given to taker in the left order's maker asset if the left spread will be part of the profit.
if (doesLeftMakerAssetProfitExist) {
matchedFillResults.profitInLeftMakerAsset = _safeSub(
matchedFillResults.left.makerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount
);
}
// Calculate amount given to taker in the right order's maker asset if the right spread will be part of the profit.
if (doesRightMakerAssetProfitExist) {
matchedFillResults.profitInRightMakerAsset = _safeSub(
matchedFillResults.right.makerAssetFilledAmount,
matchedFillResults.left.takerAssetFilledAmount
);
}
} else { } else {
// Calculate fill results for maker and taker assets: at least one order will be fully filled. _calculateMatchedFillResults(
// The maximum amount the left maker can buy is `leftTakerAssetAmountRemaining` matchedFillResults,
// The maximum amount the right maker can sell is `rightMakerAssetAmountRemaining` leftOrder,
// We have two distinct cases for calculating the fill results: rightOrder,
// Case 1. leftMakerAssetAmountRemaining,
// If the left maker can buy more than the right maker can sell, then only the right order is fully filled. leftTakerAssetAmountRemaining,
// If the left maker can buy exactly what the right maker can sell, then both orders are fully filled. rightMakerAssetAmountRemaining,
// Case 2. rightTakerAssetAmountRemaining
// If the left maker cannot buy more than the right maker can sell, then only the left order is fully filled.
if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) {
// Case 1: Right order is fully filled
matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining;
matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = rightMakerAssetAmountRemaining;
// Round down to ensure the maker's exchange rate does not exceed the price specified by the order.
// We favor the maker when the exchange rate must be rounded.
matchedFillResults.left.makerAssetFilledAmount = _safeGetPartialAmountFloor(
leftOrder.makerAssetAmount,
leftOrder.takerAssetAmount,
rightMakerAssetAmountRemaining // matchedFillResults.left.takerAssetFilledAmount
);
} else {
// Case 2: Left order is fully filled
matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
matchedFillResults.right.makerAssetFilledAmount = leftTakerAssetAmountRemaining;
// Round up to ensure the maker's exchange rate does not exceed the price specified by the order.
// We favor the maker when the exchange rate must be rounded.
matchedFillResults.right.takerAssetFilledAmount = _safeGetPartialAmountCeil(
rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount,
leftTakerAssetAmountRemaining // matchedFillResults.right.makerAssetFilledAmount
);
}
// Calculate amount given to taker
matchedFillResults.profitInLeftMakerAsset = _safeSub(
matchedFillResults.left.makerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount
); );
} }
@ -396,8 +299,9 @@ contract MixinMatchOrders is
// Without simulating all of the order matching, this program cannot know how many // Without simulating all of the order matching, this program cannot know how many
// matches there will be. To ensure that batchMatchedFillResults has enough memory // matches there will be. To ensure that batchMatchedFillResults has enough memory
// allocated for the left and the right side, we will allocate enough space for the // allocated for the left and the right side, we will allocate enough space for the
// maximum amount of matches (the left side length added to the right side length). // maximum amount of matches (the left side length added to the right side length
uint256 maxLength = leftOrders.length + rightOrders.length; // minus 1).
uint256 maxLength = leftOrders.length + rightOrders.length - 1;
batchMatchedFillResults.left = new LibFillResults.FillResults[](maxLength); batchMatchedFillResults.left = new LibFillResults.FillResults[](maxLength);
batchMatchedFillResults.right = new LibFillResults.FillResults[](maxLength); batchMatchedFillResults.right = new LibFillResults.FillResults[](maxLength);
@ -483,6 +387,180 @@ contract MixinMatchOrders is
return batchMatchedFillResults; return batchMatchedFillResults;
} }
/// @dev Calculates part of the matched fill results for a given situation using the fill strategy that only
/// awards profit denominated in the left maker asset.
/// @param matchedFillResults The MatchedFillResults struct to update with fill result calculations.
/// @param leftOrder The left order in the order matching situation.
/// @param rightOrder The right order in the order matching situation.
/// @param leftMakerAssetAmountRemaining The amount of the left order maker asset that can still be filled.
/// @param leftTakerAssetAmountRemaining The amount of the left order taker asset that can still be filled.
/// @param rightMakerAssetAmountRemaining The amount of the right order maker asset that can still be filled.
/// @param rightTakerAssetAmountRemaining The amount of the right order taker asset that can still be filled.
function _calculateMatchedFillResults(
MatchedFillResults memory matchedFillResults,
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder,
uint256 leftMakerAssetAmountRemaining,
uint256 leftTakerAssetAmountRemaining,
uint256 rightMakerAssetAmountRemaining,
uint256 rightTakerAssetAmountRemaining
)
internal
pure
{
// Calculate fill results for maker and taker assets: at least one order will be fully filled.
// The maximum amount the left maker can buy is `leftTakerAssetAmountRemaining`
// The maximum amount the right maker can sell is `rightMakerAssetAmountRemaining`
// We have two distinct cases for calculating the fill results:
// Case 1.
// If the left maker can buy more than the right maker can sell, then only the right order is fully filled.
// If the left maker can buy exactly what the right maker can sell, then both orders are fully filled.
// Case 2.
// If the left maker cannot buy more than the right maker can sell, then only the left order is fully filled.
if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) {
// Case 1: Right order is fully filled
_calculateCompleteRightFill(
matchedFillResults,
leftOrder,
rightMakerAssetAmountRemaining,
rightTakerAssetAmountRemaining
);
} else {
// Case 2: Left order is fully filled
matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
matchedFillResults.right.makerAssetFilledAmount = leftTakerAssetAmountRemaining;
// Round up to ensure the maker's exchange rate does not exceed the price specified by the order.
// We favor the maker when the exchange rate must be rounded.
matchedFillResults.right.takerAssetFilledAmount = _safeGetPartialAmountCeil(
rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount,
leftTakerAssetAmountRemaining // matchedFillResults.right.makerAssetFilledAmount
);
}
// Calculate amount given to taker
matchedFillResults.profitInLeftMakerAsset = _safeSub(
matchedFillResults.left.makerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount
);
}
/// @dev Calculates part of the matched fill results for a given situation using the maximal fill order matching
/// strategy.
/// @param matchedFillResults The MatchedFillResults struct to update with fill result calculations.
/// @param leftOrder The left order in the order matching situation.
/// @param rightOrder The right order in the order matching situation.
/// @param leftMakerAssetAmountRemaining The amount of the left order maker asset that can still be filled.
/// @param leftTakerAssetAmountRemaining The amount of the left order taker asset that can still be filled.
/// @param rightMakerAssetAmountRemaining The amount of the right order maker asset that can still be filled.
/// @param rightTakerAssetAmountRemaining The amount of the right order taker asset that can still be filled.
function _calculateMatchedFillResultsWithMaximalFill(
MatchedFillResults memory matchedFillResults,
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder,
uint256 leftMakerAssetAmountRemaining,
uint256 leftTakerAssetAmountRemaining,
uint256 rightMakerAssetAmountRemaining,
uint256 rightTakerAssetAmountRemaining
)
internal
pure
{
bool doesLeftMakerAssetProfitExist;
bool doesRightMakerAssetProfitExist;
// Calculate the maximum fill results for the maker and taker assets. At least one of the orders will be fully filled.
//
// The maximum that the left maker can possibly buy is the amount that the right order can sell.
// The maximum that the right maker can possibly buy is the amount that the left order can sell.
//
// If the left order is fully filled, profit will be paid out in the left maker asset. If the right order is fully filled,
// the profit will be out in the right maker asset.
//
// There are three cases to consider:
// Case 1.
// If the left maker can buy more than or equal to the right maker can sell, then only the right order is fully filled.
// Case 2.
// If the right maker can buy more than or equal to the left maker can sell, then only the right order is fully filled.
// Case 3.
// If the right maker can sell the max of what the left maker can buy and the left maker can sell the max of
// what the right maker can buy, then both orders are fully filled.
if (leftTakerAssetAmountRemaining >= rightMakerAssetAmountRemaining) {
// Case 1: Right order is fully filled with the profit paid in the left makerAsset
_calculateCompleteRightFill(
matchedFillResults,
leftOrder,
rightMakerAssetAmountRemaining,
rightTakerAssetAmountRemaining
);
// Indicate that the profit should be set to the spread denominated in the left maker asset.
doesLeftMakerAssetProfitExist = true;
} else if (rightTakerAssetAmountRemaining >= leftMakerAssetAmountRemaining) {
// Case 2: Left order is fully filled with the profit paid in the right makerAsset.
matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
// Round down to ensure the right maker's exchange rate does not exceed the price specified by the order.
// We favor the right maker when the exchange rate must be rounded and the profit is being paid in the
// right maker asset.
matchedFillResults.right.makerAssetFilledAmount = _safeGetPartialAmountFloor(
rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount,
leftMakerAssetAmountRemaining
);
matchedFillResults.right.takerAssetFilledAmount = leftMakerAssetAmountRemaining;
// Indicate that the profit should be set to the spread denominated in the right maker asset.
doesRightMakerAssetProfitExist = true;
} else {
// Case 3: The right and left orders are fully filled
matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining;
matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining;
matchedFillResults.left.makerAssetFilledAmount = leftMakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = leftTakerAssetAmountRemaining;
// Indicate that the profit should be set to the spread denominated in the left and the right maker assets.
doesLeftMakerAssetProfitExist = true;
doesRightMakerAssetProfitExist = true;
}
// Calculate amount given to taker in the left order's maker asset if the left spread will be part of the profit.
if (doesLeftMakerAssetProfitExist) {
matchedFillResults.profitInLeftMakerAsset = _safeSub(
matchedFillResults.left.makerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount
);
}
// Calculate amount given to taker in the right order's maker asset if the right spread will be part of the profit.
if (doesRightMakerAssetProfitExist) {
matchedFillResults.profitInRightMakerAsset = _safeSub(
matchedFillResults.right.makerAssetFilledAmount,
matchedFillResults.left.takerAssetFilledAmount
);
}
}
function _calculateCompleteRightFill(
MatchedFillResults memory matchedFillResults,
LibOrder.Order memory leftOrder,
uint256 rightMakerAssetAmountRemaining,
uint256 rightTakerAssetAmountRemaining
)
internal
pure
{
matchedFillResults.right.makerAssetFilledAmount = rightMakerAssetAmountRemaining;
matchedFillResults.right.takerAssetFilledAmount = rightTakerAssetAmountRemaining;
matchedFillResults.left.takerAssetFilledAmount = rightMakerAssetAmountRemaining;
// Round down to ensure the left maker's exchange rate does not exceed the price specified by the order.
// We favor the left maker when the exchange rate must be rounded and the profit is being paid in the
// left maker asset.
matchedFillResults.left.makerAssetFilledAmount = _safeGetPartialAmountFloor(
leftOrder.makerAssetAmount,
leftOrder.takerAssetAmount,
rightMakerAssetAmountRemaining
);
}
/// @dev Match two complementary orders that have a profitable spread. /// @dev Match two complementary orders that have a profitable spread.
/// Each order is filled at their respective price point. However, the calculations are /// 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. /// carried out as though the orders are both being filled at the right order's price point.