crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Reordered fund transfers for matched orders, plus added an extra sanity check to order matching calculations

Browse Source
This commit is contained in:
Greg Hysen
2018-05-11 17:22:10 -07:00
parent 5735095521
commit 1dd7688bdd
3 changed files with 49 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
View File

@@ -84,8 +84,8 @@ contract MixinMatchOrders is
validateMatchOrThrow(leftOrder, rightOrder);
// Compute proportional fill amounts
uint8 matchedFillAmountsStatus;
( matchedFillAmountsStatus,
uint8 matchedFillResultsStatus;
( matchedFillResultsStatus,
matchedFillResults
) = calculateMatchedFillResults(
leftOrder,
@@ -94,7 +94,7 @@ contract MixinMatchOrders is
rightOrderInfo.orderStatus,
leftOrderInfo.orderFilledAmount,
rightOrderInfo.orderFilledAmount);
if (matchedFillAmountsStatus != uint8(Status.SUCCESS)) {
if (matchedFillResultsStatus != uint8(Status.SUCCESS)) {
return matchedFillResults;
}
@@ -181,6 +181,27 @@ contract MixinMatchOrders is
function validateMatchOrThrow(MatchedFillResults memory matchedFillResults)
internal
{
// The left order must spend at least as much as we're sending to the combined
// amounts being sent to the right order and taker
uint256 amountSpentByLeft = safeAdd(
matchedFillResults.right.takerAssetFilledAmount,
matchedFillResults.takerFillAmount
);
require(
matchedFillResults.left.makerAssetFilledAmount >=
amountSpentByLeft,
MISCALCULATED_TRANSFER_AMOUNTS
);
// If the amount transferred from the left order is different than what is transferred, it is a rounding error amount.
// Ensure this difference is negligible by dividing the values with each other. The result should equal to ~1.
require(
!isRoundingError(
matchedFillResults.left.makerAssetFilledAmount,
amountSpentByLeft,
1),
ROUNDING_ERROR_TRANSFER_AMOUNTS
);
// The right order must spend at least as much as we're transferring to the left order.
require(
matchedFillResults.right.makerAssetFilledAmount >=
@@ -283,6 +304,12 @@ contract MixinMatchOrders is
return (status, matchedFillResults);
}
// Calculate amount given to taker
matchedFillResults.takerFillAmount = safeSub(
matchedFillResults.left.makerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount
);
// Validate the fill results
validateMatchOrThrow(matchedFillResults);

50
packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
View File

@@ -105,57 +105,41 @@ contract MixinSettlement is
address takerAddress)
internal
{
// Optimized for:
// * leftOrder.feeRecipient =?= rightOrder.feeRecipient
// Not optimized for:
// * {left, right}.{MakerAsset, TakerAsset} == ZRX
// * {left, right}.maker, takerAddress == {left, right}.feeRecipient
// leftOrder.MakerAsset == rightOrder.TakerAsset
// Taker should be left with a positive balance (the spread)
// Order makers and taker
dispatchTransferFrom(
leftOrder.makerAssetData,
leftOrder.makerAddress,
takerAddress,
matchedFillResults.left.makerAssetFilledAmount);
dispatchTransferFrom(
leftOrder.makerAssetData,
takerAddress,
rightOrder.makerAddress,
matchedFillResults.right.takerAssetFilledAmount);
// rightOrder.MakerAsset == leftOrder.TakerAsset
// leftOrder.takerAssetFilledAmount ~ rightOrder.makerAssetFilledAmount
// The change goes to right, not to taker.
require(
matchedFillResults.right.makerAssetFilledAmount >=
matchedFillResults.left.takerAssetFilledAmount,
MISCALCULATED_TRANSFER_AMOUNTS
matchedFillResults.right.takerAssetFilledAmount
);
dispatchTransferFrom(
rightOrder.makerAssetData,
rightOrder.makerAddress,
leftOrder.makerAddress,
matchedFillResults.right.makerAssetFilledAmount);
matchedFillResults.left.takerAssetFilledAmount
);
dispatchTransferFrom(
leftOrder.makerAssetData,
leftOrder.makerAddress,
takerAddress,
matchedFillResults.takerFillAmount
);
// Maker fees
dispatchTransferFrom(
ZRX_PROXY_DATA,
leftOrder.makerAddress,
leftOrder.feeRecipientAddress,
matchedFillResults.left.makerFeePaid);
matchedFillResults.left.makerFeePaid
);
dispatchTransferFrom(
ZRX_PROXY_DATA,
rightOrder.makerAddress,
rightOrder.feeRecipientAddress,
matchedFillResults.right.makerFeePaid);
matchedFillResults.right.makerFeePaid
);
// Taker fees
// If we assume distinct(left, right, takerAddress) and
// distinct(MakerAsset, TakerAsset, zrx) then the only remaining
// opportunity for optimization is when both feeRecipientAddress' are
// the same.
if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) {
dispatchTransferFrom(
ZRX_PROXY_DATA,
@@ -171,12 +155,14 @@ contract MixinSettlement is
ZRX_PROXY_DATA,
takerAddress,
leftOrder.feeRecipientAddress,
matchedFillResults.left.takerFeePaid);
matchedFillResults.left.takerFeePaid
);
dispatchTransferFrom(
ZRX_PROXY_DATA,
takerAddress,
rightOrder.feeRecipientAddress,
matchedFillResults.right.takerFeePaid);
matchedFillResults.right.takerFeePaid
);
}
}
}

1
packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
View File

@@ -27,6 +27,7 @@ contract MMatchOrders {
struct MatchedFillResults {
LibFillResults.FillResults left;
LibFillResults.FillResults right;
uint256 takerFillAmount;
}
/// This struct exists solely to avoid the stack limit constraint