diff options
Diffstat (limited to 'packages')
3 files changed, 49 insertions, 35 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index 77ba0a089..ab4768d02 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -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); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index cb96bbc3e..4911c62b5 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -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 + ); } } } 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 a1e3f28a1..17b6cd92d 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol @@ -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 |