Convert MixinExchangeCore to use rich errors.

This commit is contained in:
Lawrence Forman
2019-04-02 09:58:33 -04:00
committed by Amir Bandeali
parent 7232bef07b
commit 4954d0a018
3 changed files with 67 additions and 72 deletions

View File

@@ -28,6 +28,7 @@ import "./mixins/MExchangeCore.sol";
import "./mixins/MSignatureValidator.sol"; import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol"; import "./mixins/MTransactions.sol";
import "./mixins/MAssetProxyDispatcher.sol"; import "./mixins/MAssetProxyDispatcher.sol";
import "./mixins/MExchangeRichErrors.sol";
contract MixinExchangeCore is contract MixinExchangeCore is
@@ -39,7 +40,8 @@ contract MixinExchangeCore is
MAssetProxyDispatcher, MAssetProxyDispatcher,
MExchangeCore, MExchangeCore,
MSignatureValidator, MSignatureValidator,
MTransactions MTransactions,
MExchangeRichErrors
{ {
// Mapping of orderHash => amount of takerAsset already bought by maker // Mapping of orderHash => amount of takerAsset already bought by maker
mapping (bytes32 => uint256) public filled; mapping (bytes32 => uint256) public filled;
@@ -68,10 +70,9 @@ contract MixinExchangeCore is
uint256 oldOrderEpoch = orderEpoch[makerAddress][senderAddress]; uint256 oldOrderEpoch = orderEpoch[makerAddress][senderAddress];
// Ensure orderEpoch is monotonically increasing // Ensure orderEpoch is monotonically increasing
require( if (newOrderEpoch <= oldOrderEpoch) {
newOrderEpoch > oldOrderEpoch, rrevert(OrderEpochError(makerAddress, senderAddress, oldOrderEpoch));
"INVALID_NEW_ORDER_EPOCH" }
);
// Update orderEpoch // Update orderEpoch
orderEpoch[makerAddress][senderAddress] = newOrderEpoch; orderEpoch[makerAddress][senderAddress] = newOrderEpoch;
@@ -325,37 +326,38 @@ contract MixinExchangeCore is
view view
{ {
// An order can only be filled if its status is FILLABLE. // An order can only be filled if its status is FILLABLE.
require( if (orderInfo.orderStatus != uint8(OrderStatus.FILLABLE)) {
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE), rrevert(OrderStatusError(
"ORDER_UNFILLABLE" orderInfo.orderHash,
); OrderStatus(orderInfo.orderStatus)
));
}
// 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( if (order.senderAddress != msg.sender) {
order.senderAddress == msg.sender, rrevert(InvalidSenderError(orderInfo.orderHash, msg.sender));
"INVALID_SENDER" }
);
} }
// Validate taker is allowed to fill this order // Validate taker is allowed to fill this order
if (order.takerAddress != address(0)) { if (order.takerAddress != address(0)) {
require( if (order.takerAddress != takerAddress) {
order.takerAddress == takerAddress, rrevert(InvalidTakerError(orderInfo.orderHash, takerAddress));
"INVALID_TAKER" }
);
} }
// Validate Maker signature (check only if first time seen) // Validate Maker signature (check only if first time seen)
if (orderInfo.orderTakerAssetFilledAmount == 0) { if (orderInfo.orderTakerAssetFilledAmount == 0) {
require( if (!isValidSignature(
isValidSignature( orderInfo.orderHash,
orderInfo.orderHash, order.makerAddress,
order.makerAddress, signature)) {
signature rrevert(SignatureError(
), orderInfo.orderHash,
"INVALID_ORDER_SIGNATURE" SignatureErrorCodes.BAD_SIGNATURE
); ));
}
} }
} }
@@ -377,26 +379,24 @@ contract MixinExchangeCore is
{ {
// Revert if fill amount is invalid // Revert if fill amount is invalid
// TODO: reconsider necessity for v2.1 // TODO: reconsider necessity for v2.1
require( if (takerAssetFillAmount == 0) {
takerAssetFillAmount != 0, rrevert(FillError(orderInfo.orderHash, FillErrorCodes.INVALID_TAKER_AMOUNT));
"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
// as an extra defence against potential bugs. // as an extra defence against potential bugs.
require( if (takerAssetFilledAmount > takerAssetFillAmount) {
takerAssetFilledAmount <= takerAssetFillAmount, rrevert(FillError(orderInfo.orderHash, FillErrorCodes.TAKER_OVERPAY));
"TAKER_OVERPAY" }
);
// Make sure order is not overfilled // Make sure order is not overfilled
// NOTE: This assertion should never fail, it is here // NOTE: This assertion should never fail, it is here
// as an extra defence against potential bugs. // as an extra defence against potential bugs.
require( if (safeAdd(orderInfo.orderTakerAssetFilledAmount, takerAssetFilledAmount)
safeAdd(orderInfo.orderTakerAssetFilledAmount, takerAssetFilledAmount) <= order.takerAssetAmount, > order.takerAssetAmount) {
"ORDER_OVERFILL" rrevert(FillError(orderInfo.orderHash, FillErrorCodes.OVERFILL));
); }
// Make sure order is filled at acceptable price. // Make sure order is filled at acceptable price.
// The order has an implied price from the makers perspective: // The order has an implied price from the makers perspective:
@@ -415,12 +415,10 @@ contract MixinExchangeCore is
// order.makerAssetAmount * takerAssetFilledAmount // order.makerAssetAmount * takerAssetFilledAmount
// NOTE: This assertion should never fail, it is here // NOTE: This assertion should never fail, it is here
// as an extra defence against potential bugs. // as an extra defence against potential bugs.
require( if (safeMul(makerAssetFilledAmount, order.takerAssetAmount)
safeMul(makerAssetFilledAmount, order.takerAssetAmount) > safeMul(order.makerAssetAmount, takerAssetFilledAmount)) {
<= rrevert(FillError(orderInfo.orderHash, FillErrorCodes.INVALID_FILL_PRICE));
safeMul(order.makerAssetAmount, takerAssetFilledAmount), }
"INVALID_FILL_PRICE"
);
} }
/// @dev Validates context for cancelOrder. Succeeds or throws. /// @dev Validates context for cancelOrder. Succeeds or throws.
@@ -435,25 +433,22 @@ contract MixinExchangeCore is
{ {
// Ensure order is valid // Ensure order is valid
// An order can only be cancelled if its status is FILLABLE. // An order can only be cancelled if its status is FILLABLE.
require( if (orderInfo.orderStatus != uint8(OrderStatus.FILLABLE)) {
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE), rrevert(OrderStatusError(orderInfo.orderHash, OrderStatus(orderInfo.orderStatus)));
"ORDER_UNFILLABLE" }
);
// Validate sender is allowed to cancel this order // Validate sender is allowed to cancel this order
if (order.senderAddress != address(0)) { if (order.senderAddress != address(0)) {
require( if (order.senderAddress != msg.sender) {
order.senderAddress == msg.sender, rrevert(InvalidSenderError(orderInfo.orderHash, msg.sender));
"INVALID_SENDER" }
);
} }
// Validate transaction signed by maker // Validate transaction signed by maker
address makerAddress = getCurrentContextAddress(); address makerAddress = getCurrentContextAddress();
require( if (order.makerAddress != makerAddress) {
order.makerAddress == makerAddress, rrevert(InvalidMakerError(orderInfo.orderHash, makerAddress));
"INVALID_MAKER" }
);
} }
/// @dev Calculates amounts filled and fees paid by maker and taker. /// @dev Calculates amounts filled and fees paid by maker and taker.

View File

@@ -117,7 +117,7 @@ contract MixinExchangeRichErrors is
); );
} }
function EpochOrderError( function OrderEpochError(
address maker, address maker,
address sender, address sender,
uint256 currentEpoch uint256 currentEpoch
@@ -127,7 +127,7 @@ contract MixinExchangeRichErrors is
returns (bytes memory) returns (bytes memory)
{ {
return abi.encodeWithSelector( return abi.encodeWithSelector(
EPOCH_ORDER_ERROR_SELECTOR, ORDER_EPOCH_ERROR_SELECTOR,
maker, maker,
sender, sender,
currentEpoch currentEpoch

View File

@@ -120,10 +120,10 @@ contract MExchangeRichErrors is
pure pure
returns (bytes memory); returns (bytes memory);
bytes4 internal constant EPOCH_ORDER_ERROR_SELECTOR = bytes4 internal constant ORDER_EPOCH_ERROR_SELECTOR =
bytes4(keccak256("EpochOrderError(address,address,uint256)")); bytes4(keccak256("OrderEpochError(address,address,uint256)"));
function EpochOrderError( function OrderEpochError(
address maker, address maker,
address sender, address sender,
uint256 currentEpoch uint256 currentEpoch