Added require reasons to MixinMatchOrders and cleaned up some comments.

This commit is contained in:
Greg Hysen
2018-05-11 11:32:12 -07:00
parent f378406d15
commit fa7570352c
6 changed files with 75 additions and 40 deletions

View File

@@ -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)

View File

@@ -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,

View File

@@ -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,

View File

@@ -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.";
}

View File

@@ -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.

View File

@@ -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 {