aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGreg Hysen <greg.hysen@gmail.com>2018-05-12 02:32:12 +0800
committerGreg Hysen <greg.hysen@gmail.com>2018-05-19 08:01:06 +0800
commitfa7570352ce65dc58df6969c5a24cf3f487e738f (patch)
tree0e4816c7711b9625cc5c9965f9cfecf08d45f2bf
parentf378406d155bbb7c4679904756c897ed2f4388c1 (diff)
downloaddexon-sol-tools-fa7570352ce65dc58df6969c5a24cf3f487e738f.tar
dexon-sol-tools-fa7570352ce65dc58df6969c5a24cf3f487e738f.tar.gz
dexon-sol-tools-fa7570352ce65dc58df6969c5a24cf3f487e738f.tar.bz2
dexon-sol-tools-fa7570352ce65dc58df6969c5a24cf3f487e738f.tar.lz
dexon-sol-tools-fa7570352ce65dc58df6969c5a24cf3f487e738f.tar.xz
dexon-sol-tools-fa7570352ce65dc58df6969c5a24cf3f487e738f.tar.zst
dexon-sol-tools-fa7570352ce65dc58df6969c5a24cf3f487e738f.zip
Added require reasons to MixinMatchOrders and cleaned up some comments.
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol18
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol44
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol8
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol8
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol2
-rw-r--r--packages/contracts/src/utils/types.ts35
6 files changed, 75 insertions, 40 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index 58b5fa6b1..981c5984e 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -53,7 +53,7 @@ contract MixinExchangeCore is
////// Core exchange functions //////
- /// @dev Gets information about an order.
+ /// @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.
/// @return orderHash Keccak-256 EIP712 hash of the order.
@@ -105,14 +105,12 @@ contract MixinExchangeCore is
orderStatus = uint8(Status.ORDER_CANCELLED);
return (orderStatus, orderHash, takerAssetFilledAmount);
}
-
- // Validate order is not cancelled
if (makerEpoch[order.makerAddress] > order.salt) {
orderStatus = uint8(Status.ORDER_CANCELLED);
return (orderStatus, orderHash, takerAssetFilledAmount);
}
- // Order is Fillable
+ // All other statuses are ruled out: order is Fillable
orderStatus = uint8(Status.ORDER_FILLABLE);
return (orderStatus, orderHash, takerAssetFilledAmount);
}
@@ -136,6 +134,8 @@ contract MixinExchangeCore is
internal
{
// Ensure order is valid
+ // An order can only be filled if it is Status.FILLABLE;
+ // however, only invalid statuses result in a throw.
require(
orderStatus != uint8(Status.ORDER_INVALID_MAKER_ASSET_AMOUNT),
INVALID_ORDER_MAKER_ASSET_AMOUNT
@@ -269,7 +269,7 @@ contract MixinExchangeCore is
public
returns (FillResults memory fillResults)
{
- // Fetch current order status
+ // Fetch order info
bytes32 orderHash;
uint8 orderStatus;
uint256 takerAssetFilledAmount;
@@ -309,6 +309,8 @@ contract MixinExchangeCore is
internal
{
// Ensure order is valid
+ // An order can only be cancelled if it is Status.FILLABLE;
+ // however, only invalid statuses result in a throw.
require(
orderStatus != uint8(Status.ORDER_INVALID_MAKER_ASSET_AMOUNT),
INVALID_ORDER_MAKER_ASSET_AMOUNT
@@ -372,9 +374,11 @@ contract MixinExchangeCore is
}
/// @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(Order memory order)
public
returns (bool)
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
index 76b021919..a333b8221 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
@@ -23,6 +23,7 @@ import "./libs/LibMath.sol";
import "./libs/LibOrder.sol";
import "./libs/LibStatus.sol";
import "../../utils/LibBytes/LibBytes.sol";
+import "./libs/LibExchangeErrors.sol";
contract MixinMatchOrders is
SafeMath,
@@ -30,6 +31,7 @@ contract MixinMatchOrders is
LibMath,
LibStatus,
LibOrder,
+ LibExchangeErrors,
MExchangeCore,
MMatchOrders,
MSettlement,
@@ -45,22 +47,29 @@ contract MixinMatchOrders is
internal
{
// The leftOrder maker asset must be the same as the rightOrder taker asset.
- require(areBytesEqual(leftOrder.makerAssetData, rightOrder.takerAssetData));
+ require(
+ areBytesEqual(leftOrder.makerAssetData, rightOrder.takerAssetData),
+ ASSET_MISMATCH_MAKER_TAKER
+ );
// The leftOrder taker asset must be the same as the rightOrder maker asset.
- require(areBytesEqual(leftOrder.takerAssetData, rightOrder.makerAssetData));
+ 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
// 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:
require(
safeMul(leftOrder.makerAssetAmount, rightOrder.makerAssetAmount) >=
- safeMul(leftOrder.takerAssetAmount, rightOrder.takerAssetAmount)
+ safeMul(leftOrder.takerAssetAmount, rightOrder.takerAssetAmount),
+ NEGATIVE_SPREAD
);
}
@@ -69,11 +78,21 @@ contract MixinMatchOrders is
function validateMatchedOrderFillResultsOrThrow(MatchedFillResults memory matchedFillResults)
internal
{
- // The right order must spend at least as much as we're transferring to the left order's maker.
- // If the amount transferred from the right order is greater than what is transferred, it is a rounding error amount.
+ // 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(matchedFillResults.right.makerAssetFilledAmount >= matchedFillResults.left.takerAssetFilledAmount);
- require(!isRoundingError(matchedFillResults.right.makerAssetFilledAmount, matchedFillResults.left.takerAssetFilledAmount, 1));
+ require(
+ !isRoundingError(
+ matchedFillResults.right.makerAssetFilledAmount,
+ matchedFillResults.left.takerAssetFilledAmount,
+ 1),
+ ROUNDING_ERROR_TRANSFER_AMOUNTS
+ );
}
/// @dev Calculates partial value given a numerator and denominator.
@@ -89,7 +108,10 @@ contract MixinMatchOrders is
internal pure
returns (uint256 partialAmount)
{
- require(!isRoundingError(numerator, denominator, target));
+ require(
+ !isRoundingError(numerator, denominator, target),
+ ROUNDING_ERROR_ON_PARTIAL_AMOUNT
+ );
return getPartialAmount(numerator, denominator, target);
}
@@ -136,7 +158,7 @@ contract MixinMatchOrders is
leftOrderAmountToFill = leftTakerAssetAmountRemaining;
// The right order receives an amount proportional to how much was spent.
- // TODO: Ensure rounding error is in the correct direction.
+ // TODO: Can we ensure rounding error is in the correct direction?
rightOrderAmountToFill = safeGetPartialAmount(
rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount,
@@ -146,7 +168,7 @@ contract MixinMatchOrders is
rightOrderAmountToFill = rightTakerAssetAmountRemaining;
// The left order receives an amount proportional to how much was spent.
- // TODO: Ensure rounding error is in the correct direction.
+ // TODO: Can we ensure rounding error is in the correct direction?
leftOrderAmountToFill = safeGetPartialAmount(
rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount,
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
index 3b67051d2..cde64ba74 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
@@ -23,9 +23,11 @@ import "./mixins/MAssetProxyDispatcher.sol";
import "./libs/LibOrder.sol";
import "./libs/LibMath.sol";
import "./mixins/MMatchOrders.sol";
+import "./mixins/LibExchangeErrors.sol";
contract MixinSettlement is
LibMath,
+ LibExchangeErrors,
MMatchOrders,
MSettlement,
MAssetProxyDispatcher
@@ -134,7 +136,11 @@ contract MixinSettlement is
// rightOrder.MakerAsset == leftOrder.TakerAsset
// leftOrder.takerAssetFilledAmount ~ rightOrder.makerAssetFilledAmount
// The change goes to right, not to taker.
- assert(matchedFillResults.right.makerAssetFilledAmount >= matchedFillResults.left.takerAssetFilledAmount);
+ require(
+ matchedFillResults.right.makerAssetFilledAmount >=
+ matchedFillResults.left.takerAssetFilledAmount,
+ MISCALCULATED_TRANSFER_AMOUNTS
+ );
dispatchTransferFrom(
rightOrder.makerAssetData,
rightOrder.makerAddress,
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 bf048e815..ba055dd8e 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
@@ -48,4 +48,12 @@ contract LibExchangeErrors {
string constant INVALID_SIGNATURE_LENGTH = "Invalid signature length.";
string constant ILLEGAL_SIGNATURE_TYPE = "Illegal signature type.";
string constant UNSUPPORTED_SIGNATURE_TYPE = "Unsupported signature type.";
+
+ // Order matching revert reasons
+ string constant ASSET_MISMATCH_MAKER_TAKER = "Left order maker asset is different from right order taker asset.";
+ string constant ASSET_MISMATCH_TAKER_MAKER = "Left order taker asset is different from right order maker asset.";
+ 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/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
index 416cf426d..cfd2678ba 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -65,7 +65,7 @@ contract MExchangeCore is
LibFillResults.FillResults memory fillResults)
internal;
- /// @dev Gets information about an order.
+ /// @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.
/// @return orderHash Keccak-256 EIP712 hash of the order.
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index ce7fbcbe1..0e3b2c9a8 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -75,26 +75,21 @@ export interface Token {
}
export enum ExchangeStatus {
- /// Default Status ///
- INVALID, // General invalid status
-
- /// General Exchange Statuses ///
- SUCCESS, // Indicates a successful operation
- ROUNDING_ERROR_TOO_LARGE, // Rounding error too large
- INSUFFICIENT_BALANCE_OR_ALLOWANCE, // Insufficient balance or allowance for token transfer
- TAKER_ASSET_FILL_AMOUNT_TOO_LOW, // takerAssetFillAmount is <= 0
- INVALID_SIGNATURE, // Invalid signature
- INVALID_SENDER, // Invalid sender
- INVALID_TAKER, // Invalid taker
- INVALID_MAKER, // Invalid maker
-
- /// Order State Statuses ///
- ORDER_INVALID_MAKER_ASSET_AMOUNT, // Order does not have a valid maker asset amount
- ORDER_INVALID_TAKER_ASSET_AMOUNT, // Order does not have a valid taker asset amount
- ORDER_FILLABLE, // Order is fillable
- ORDER_EXPIRED, // Order has already expired
- ORDER_FULLY_FILLED, // Order is fully filled
- ORDER_CANCELLED, // Order has been cancelled
+ INVALID,
+ SUCCESS,
+ ROUNDING_ERROR_TOO_LARGE,
+ INSUFFICIENT_BALANCE_OR_ALLOWANCE,
+ TAKER_ASSET_FILL_AMOUNT_TOO_LOW,
+ INVALID_SIGNATURE,
+ INVALID_SENDER,
+ INVALID_TAKER,
+ INVALID_MAKER,
+ ORDER_INVALID_MAKER_ASSET_AMOUNT,
+ ORDER_INVALID_TAKER_ASSET_AMOUNT,
+ ORDER_FILLABLE,
+ ORDER_EXPIRED,
+ ORDER_FULLY_FILLED,
+ ORDER_CANCELLED,
}
export enum ContractName {