Split into assertFillable and assertValidFill

This commit is contained in:
Remco Bloemen 2018-08-24 13:41:56 -07:00
parent 3e4493b389
commit 29971f36cf
2 changed files with 43 additions and 29 deletions

View File

@ -91,14 +91,11 @@ contract MixinExchangeCore is
// Fetch order info // Fetch order info
OrderInfo memory orderInfo = getOrderInfo(order); OrderInfo memory orderInfo = getOrderInfo(order);
// An order can only be filled if its status is FILLABLE.
require(
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
"ORDER_UNFILLABLE"
);
// Fetch taker address // Fetch taker address
address takerAddress = getCurrentContextAddress(); address takerAddress = getCurrentContextAddress();
// Assert that the order is fillable by taker
assertFillableOrder(order, orderInfo, taker, signature);
// Get amount of takerAsset to fill // Get amount of takerAsset to fill
uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount); uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount);
@ -110,8 +107,6 @@ contract MixinExchangeCore is
// Validate context // Validate context
assertValidFill( assertValidFill(
order, order,
orderInfo,
takerAddress,
takerAssetFillAmount, takerAssetFillAmount,
takerAssetFilledAmount, takerAssetFilledAmount,
fillResults.makerAssetFilledAmount, fillResults.makerAssetFilledAmount,
@ -266,22 +261,16 @@ contract MixinExchangeCore is
order.takerAssetData order.takerAssetData
); );
} }
/// @dev Validates context for fillOrder. Succeeds or throws. /// @dev Validates context for fillOrder. Succeeds or throws.
/// @param order to be filled. /// @param order to be filled.
/// @param orderInfo OrderStatus, orderHash, and amount already filled of order. /// @param orderInfo OrderStatus, orderHash, and amount already filled of order.
/// @param takerAddress Address of order taker. /// @param takerAddress Address of order taker.
/// @param takerAssetFillAmount Desired amount of order to fill by taker.
/// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
/// @param makerAssetFilledAmount Amount of makerAsset that will be transfered.
/// @param signature Proof that the orders was created by its maker. /// @param signature Proof that the orders was created by its maker.
function assertValidFill( function assertFillableOrder(
Order memory order, Order memory order,
OrderInfo memory orderInfo, OrderInfo memory orderInfo,
address takerAddress, address taker,
uint256 takerAssetFillAmount,
uint256 takerAssetFilledAmount,
uint256 makerAssetFilledAmount,
bytes memory signature bytes memory signature
) )
internal internal
@ -292,13 +281,7 @@ contract MixinExchangeCore is
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE), orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
"ORDER_UNFILLABLE" "ORDER_UNFILLABLE"
); );
// Revert if fill amount is invalid
require(
takerAssetFillAmount != 0,
"INVALID_TAKER_AMOUNT"
);
// Validate sender is allowed to fill this order // Validate sender is allowed to fill this order
if (order.senderAddress != address(0)) { if (order.senderAddress != address(0)) {
require( require(
@ -332,6 +315,29 @@ contract MixinExchangeCore is
"INVALID_ORDER_SIGNATURE" "INVALID_ORDER_SIGNATURE"
); );
} }
}
/// @dev Validates context for fillOrder. Succeeds or throws.
/// @param order to be filled.
/// @param takerAssetFillAmount Desired amount of order to fill by taker.
/// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
/// @param makerAssetFilledAmount Amount of makerAsset that will be transfered.
/// @param signature Proof that the orders was created by its maker.
function assertValidFill(
Order memory order,
uint256 takerAssetFillAmount,
uint256 takerAssetFilledAmount,
uint256 makerAssetFilledAmount,
bytes memory signature
)
internal
view
{
// Revert if fill amount is invalid
require(
takerAssetFillAmount != 0,
"INVALID_TAKER_AMOUNT"
);
// Make sure taker does not pay more than desired amount // Make sure taker does not pay more than desired amount
// NOTE: This assertion should never fail, it is here // NOTE: This assertion should never fail, it is here

View File

@ -61,8 +61,20 @@ contract MixinMatchOrders is
// Fetch taker address // Fetch taker address
address takerAddress = getCurrentContextAddress(); address takerAddress = getCurrentContextAddress();
// Either our context is valid or we revert // Either our context is valid or we revert
assertFillableOrder(
leftOrder,
leftOrderInfo,
takerAddress,
leftSignature
);
assertFillableOrder(
righttOrder,
righttOrderInfo,
takerAddress,
leftSignature
);
assertValidMatch(leftOrder, rightOrder); assertValidMatch(leftOrder, rightOrder);
// Compute proportional fill amounts // Compute proportional fill amounts
@ -76,8 +88,6 @@ contract MixinMatchOrders is
// Validate fill contexts // Validate fill contexts
assertValidFill( assertValidFill(
leftOrder, leftOrder,
leftOrderInfo,
takerAddress,
matchedFillResults.left.takerAssetFilledAmount, matchedFillResults.left.takerAssetFilledAmount,
matchedFillResults.left.takerAssetFilledAmount, matchedFillResults.left.takerAssetFilledAmount,
matchedFillResults.left.makerAssetFilledAmount, matchedFillResults.left.makerAssetFilledAmount,
@ -85,8 +95,6 @@ contract MixinMatchOrders is
); );
assertValidFill( assertValidFill(
rightOrder, rightOrder,
rightOrderInfo,
takerAddress,
matchedFillResults.right.takerAssetFilledAmount, matchedFillResults.right.takerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount, matchedFillResults.right.takerAssetFilledAmount,
matchedFillResults.right.makerAssetFilledAmount, matchedFillResults.right.makerAssetFilledAmount,