diff options
author | Greg Hysen <greg.hysen@gmail.com> | 2018-05-19 07:37:50 +0800 |
---|---|---|
committer | Greg Hysen <greg.hysen@gmail.com> | 2018-05-19 08:07:00 +0800 |
commit | d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0 (patch) | |
tree | 01d9f7055cb2bdd4837b8860711ab371599b17e5 /packages/contracts/src | |
parent | 89abd765702313c9c22b41e92729880850e44c92 (diff) | |
download | dexon-0x-contracts-d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0.tar dexon-0x-contracts-d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0.tar.gz dexon-0x-contracts-d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0.tar.bz2 dexon-0x-contracts-d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0.tar.lz dexon-0x-contracts-d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0.tar.xz dexon-0x-contracts-d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0.tar.zst dexon-0x-contracts-d13c08cc0db1c211bd354bcdd15a2071aa1cd0e0.zip |
Style improvements to order matching
Diffstat (limited to 'packages/contracts/src')
4 files changed, 17 insertions, 28 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index b57376fa6..b9738519f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -225,13 +225,13 @@ contract MixinExchangeCore is // Fill amount must be greater than 0 if (takerAssetFillAmount == 0) { status = uint8(Status.TAKER_ASSET_FILL_AMOUNT_TOO_LOW); - return; + return (status, fillResults); } // Ensure the order is fillable if (orderStatus != uint8(Status.ORDER_FILLABLE)) { status = orderStatus; - return; + return (status, fillResults); } // Compute takerAssetFilledAmount diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index f1093631f..ac382d719 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -39,7 +39,7 @@ contract MixinMatchOrders is MTransactions { - /// @dev Match two complementary orders that have a positive spread. + /// @dev Match two complementary orders that have a profitable 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). @@ -52,8 +52,8 @@ contract MixinMatchOrders is function matchOrders( Order memory leftOrder, Order memory rightOrder, - bytes leftSignature, - bytes rightSignature + bytes memory leftSignature, + bytes memory rightSignature ) public returns (MatchedFillResults memory matchedFillResults) @@ -135,19 +135,21 @@ contract MixinMatchOrders is internal { // The leftOrder maker asset must be the same as the rightOrder taker asset. + // TODO: Can we safely assume equality and expect a later failure otherwise? require( areBytesEqual(leftOrder.makerAssetData, rightOrder.takerAssetData), ASSET_MISMATCH_MAKER_TAKER ); // The leftOrder taker asset must be the same as the rightOrder maker asset. + // TODO: Can we safely assume equality and expect a later failure otherwise? require( areBytesEqual(leftOrder.takerAssetData, rightOrder.makerAssetData), ASSET_MISMATCH_TAKER_MAKER ); - // Make sure there is a positive spread. - // There is a positive spread iff the cost per unit bought (OrderA.MakerAmount/OrderA.TakerAmount) for each order is greater + // Make sure there is a profitable spread. + // There is a profitable 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> @@ -163,23 +165,16 @@ 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 assertValidMatch(MatchedFillResults memory matchedFillResults) + function assertValidMatchResults(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 + // 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. 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, @@ -188,12 +183,6 @@ contract MixinMatchOrders is 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 >= - matchedFillResults.left.takerAssetFilledAmount, - MISCALCULATED_TRANSFER_AMOUNTS - ); // If the amount transferred from the right 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( @@ -300,7 +289,7 @@ contract MixinMatchOrders is ); // Validate the fill results - assertValidMatch(matchedFillResults); + assertValidMatchResults(matchedFillResults); // Return fill results return matchedFillResults; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol index 9676ca657..df009d063 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IMatchOrders.sol @@ -23,7 +23,7 @@ import "../libs/LibFillResults.sol"; contract IMatchOrders { - /// @dev Match two complementary orders that have a positive spread. + /// @dev Match two complementary orders that have a profitable 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). @@ -36,8 +36,8 @@ contract IMatchOrders { function matchOrders( LibOrder.Order memory leftOrder, LibOrder.Order memory rightOrder, - bytes leftSignature, - bytes rightSignature + bytes memory leftSignature, + bytes memory rightSignature ) public returns (LibFillResults.MatchedFillResults memory matchedFillResults); 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 904647ec2..7dd608cf2 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol @@ -38,7 +38,7 @@ contract MMatchOrders 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 assertValidMatch(LibFillResults.MatchedFillResults memory matchedFillResults) + function assertValidMatchResults(LibFillResults.MatchedFillResults memory matchedFillResults) internal; /// @dev Calculates fill amounts for the matched orders. |