aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJacob Evans <dekz@dekz.net>2018-06-05 08:25:45 +0800
committerGitHub <noreply@github.com>2018-06-05 08:25:45 +0800
commit70858603ed37de24ae47978a191b3f188f450182 (patch)
tree22cb18debbcc5aa16eccc23c04bfd86f8d1439eb
parent79472552aae4ef60ae20e26571c750cbeb02c552 (diff)
parent5c44db341fca8926fcf2329f96f0f7f7a5ce16f7 (diff)
downloaddexon-sol-tools-70858603ed37de24ae47978a191b3f188f450182.tar
dexon-sol-tools-70858603ed37de24ae47978a191b3f188f450182.tar.gz
dexon-sol-tools-70858603ed37de24ae47978a191b3f188f450182.tar.bz2
dexon-sol-tools-70858603ed37de24ae47978a191b3f188f450182.tar.lz
dexon-sol-tools-70858603ed37de24ae47978a191b3f188f450182.tar.xz
dexon-sol-tools-70858603ed37de24ae47978a191b3f188f450182.tar.zst
dexon-sol-tools-70858603ed37de24ae47978a191b3f188f450182.zip
Merge pull request #633 from 0xProject/feature/contracts/errors
Update error handling
-rw-r--r--packages/contracts/compiler.json2
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol15
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol14
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol4
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol13
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol37
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol13
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol349
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol160
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol15
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol31
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol16
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol24
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol28
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol79
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol21
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol20
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol34
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol51
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol76
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol22
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol1
-rw-r--r--packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol4
-rw-r--r--packages/contracts/src/utils/match_order_tester.ts1
-rw-r--r--packages/contracts/src/utils/types.ts22
-rw-r--r--packages/contracts/test/asset_proxy/authorizable.ts1
-rw-r--r--packages/contracts/test/exchange/core.ts121
-rw-r--r--packages/contracts/test/exchange/match_orders.ts43
-rw-r--r--packages/contracts/test/exchange/transactions.ts8
-rw-r--r--packages/contracts/test/exchange/wrapper.ts2
30 files changed, 490 insertions, 737 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index ed59069fe..48ba4ffcd 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -4,7 +4,7 @@
"compilerSettings": {
"optimizer": {
"enabled": true,
- "runs": 0
+ "runs": 200
},
"outputSelection": {
"*": {
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index 79824fbbb..2c321e134 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -20,9 +20,9 @@ pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol";
-import "../../tokens/ERC20Token/IERC20Token.sol";
import "./MixinAssetProxy.sol";
import "./MixinAuthorizable.sol";
+import "../../tokens/ERC20Token/IERC20Token.sol";
contract ERC20Proxy is
LibBytes,
@@ -33,11 +33,6 @@ contract ERC20Proxy is
// Id of this proxy.
uint8 constant PROXY_ID = 1;
- // Revert reasons
- string constant INVALID_METADATA_LENGTH = "Metadata must have a length of 21.";
- string constant TRANSFER_FAILED = "Transfer failed.";
- string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id.";
-
/// @dev Internal version of `transferFrom`.
/// @param assetMetadata Encoded byte array.
/// @param from Address to transfer asset from.
@@ -56,12 +51,12 @@ contract ERC20Proxy is
require(
length == 21,
- INVALID_METADATA_LENGTH
+ LENGTH_21_REQUIRED
);
-
+ // TODO: Is this too inflexible in the future?
require(
uint8(assetMetadata[length - 1]) == PROXY_ID,
- PROXY_ID_MISMATCH
+ ASSET_PROXY_ID_MISMATCH
);
// Decode metadata.
@@ -70,7 +65,7 @@ contract ERC20Proxy is
// Transfer tokens.
bool success = IERC20Token(token).transferFrom(from, to, amount);
require(
- success == true,
+ success,
TRANSFER_FAILED
);
}
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index ace3570bc..07e01c774 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -20,9 +20,9 @@ pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol";
-import "../../tokens/ERC721Token/ERC721Token.sol";
import "./MixinAssetProxy.sol";
import "./MixinAuthorizable.sol";
+import "../../tokens/ERC721Token/ERC721Token.sol";
contract ERC721Proxy is
LibBytes,
@@ -33,11 +33,6 @@ contract ERC721Proxy is
// Id of this proxy.
uint8 constant PROXY_ID = 2;
- // Revert reasons
- string constant INVALID_TRANSFER_AMOUNT = "Transfer amount must equal 1.";
- string constant INVALID_METADATA_LENGTH = "Metadata must have a length of 53.";
- string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id.";
-
/// @dev Internal version of `transferFrom`.
/// @param assetMetadata Encoded byte array.
/// @param from Address to transfer asset from.
@@ -56,18 +51,19 @@ contract ERC721Proxy is
require(
length == 53,
- INVALID_METADATA_LENGTH
+ LENGTH_53_REQUIRED
);
+ // TODO: Is this too inflexible in the future?
require(
uint8(assetMetadata[length - 1]) == PROXY_ID,
- PROXY_ID_MISMATCH
+ ASSET_PROXY_ID_MISMATCH
);
// There exists only 1 of each token.
require(
amount == 1,
- INVALID_TRANSFER_AMOUNT
+ INVALID_AMOUNT
);
// Decode metadata
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
index 4bd533148..5fa33cbef 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol
@@ -19,10 +19,10 @@
pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
-import "./mixins/MAssetProxy.sol";
import "./mixins/MAuthorizable.sol";
+import "./mixins/MAssetProxy.sol";
-contract MixinAssetProxy is
+contract MixinAssetProxy is
MAuthorizable,
MAssetProxy
{
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
index 4aaeb66d1..8cb4254c5 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
@@ -19,21 +19,16 @@
pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
-import "./mixins/MAuthorizable.sol";
+import "./libs/LibAssetProxyErrors.sol";
import "../../utils/Ownable/Ownable.sol";
+import "./mixins/MAuthorizable.sol";
contract MixinAuthorizable is
+ LibAssetProxyErrors,
Ownable,
MAuthorizable
{
- // Revert reasons
- string constant SENDER_NOT_AUTHORIZED = "Sender not authorized to call this method.";
- string constant TARGET_NOT_AUTHORIZED = "Target address must be authorized.";
- string constant TARGET_ALREADY_AUTHORIZED = "Target must not already be authorized.";
- string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds.";
- string constant INDEX_ADDRESS_MISMATCH = "Address found at index does not match target address.";
-
/// @dev Only authorized addresses can invoke functions with this modifier.
modifier onlyAuthorized {
require(
@@ -99,7 +94,7 @@ contract MixinAuthorizable is
);
require(
authorities[index] == target,
- INDEX_ADDRESS_MISMATCH
+ AUTHORIZED_ADDRESS_MISMATCH
);
delete authorized[target];
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol
new file mode 100644
index 000000000..e0c7fc796
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol
@@ -0,0 +1,37 @@
+/*
+
+ Copyright 2018 ZeroEx Intl.
+
+ Licensed under the Apache License, Version 2.0 (the "License");
+ you may not use this file except in compliance with the License.
+ You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+
+*/
+
+pragma solidity ^0.4.24;
+
+contract LibAssetProxyErrors {
+ /// Authorizable errors ///
+ string constant SENDER_NOT_AUTHORIZED = "SENDER_NOT_AUTHORIZED"; // Sender not authorized to call this method.
+ string constant TARGET_NOT_AUTHORIZED = "TARGET_NOT_AUTHORIZED"; // Target address not authorized to call this method.
+ string constant TARGET_ALREADY_AUTHORIZED = "TARGET_ALREADY_AUTHORIZED"; // Target address must not already be authorized.
+ string constant INDEX_OUT_OF_BOUNDS = "INDEX_OUT_OF_BOUNDS"; // Specified array index is out of bounds.
+ string constant AUTHORIZED_ADDRESS_MISMATCH = "AUTHORIZED_ADDRESS_MISMATCH"; // Address at index does not match given target address.
+
+ /// AssetProxy errors ///
+ string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // Proxy id in metadata does not match this proxy id.
+ string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1.
+ string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed.
+
+ /// Length validation errors ///
+ string constant LENGTH_21_REQUIRED = "LENGTH_21_REQUIRED"; // Byte array must have a length of 21.
+ string constant LENGTH_53_REQUIRED = "LENGTH_53_REQUIRED"; // Byte array must have a length of 53.
+}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
index d996f933c..8f9342739 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -19,13 +19,13 @@
pragma solidity ^0.4.24;
import "../../utils/Ownable/Ownable.sol";
-import "../AssetProxy/interfaces/IAssetProxy.sol";
import "./libs/LibExchangeErrors.sol";
import "./mixins/MAssetProxyDispatcher.sol";
+import "../AssetProxy/interfaces/IAssetProxy.sol";
contract MixinAssetProxyDispatcher is
- LibExchangeErrors,
Ownable,
+ LibExchangeErrors,
MAssetProxyDispatcher
{
// Mapping from Asset Proxy Id's to their respective Asset Proxy
@@ -45,9 +45,10 @@ contract MixinAssetProxyDispatcher is
onlyOwner
{
// Ensure the existing asset proxy is not unintentionally overwritten
+ address currentAssetProxy = address(assetProxies[assetProxyId]);
require(
- oldAssetProxy == address(assetProxies[assetProxyId]),
- OLD_ASSET_PROXY_MISMATCH
+ oldAssetProxy == currentAssetProxy,
+ ASSET_PROXY_MISMATCH
);
IAssetProxy assetProxy = IAssetProxy(newAssetProxy);
@@ -57,7 +58,7 @@ contract MixinAssetProxyDispatcher is
uint8 newAssetProxyId = assetProxy.getProxyId();
require(
newAssetProxyId == assetProxyId,
- NEW_ASSET_PROXY_MISMATCH
+ ASSET_PROXY_ID_MISMATCH
);
}
@@ -98,7 +99,7 @@ contract MixinAssetProxyDispatcher is
uint256 length = assetMetadata.length;
require(
length > 0,
- GT_ZERO_LENGTH_REQUIRED
+ LENGTH_GREATER_THAN_0_REQUIRED
);
uint8 assetProxyId = uint8(assetMetadata[length - 1]);
IAssetProxy assetProxy = assetProxies[assetProxyId];
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index 1c2420374..42128c92a 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -22,7 +22,6 @@ pragma experimental ABIEncoderV2;
import "./libs/LibFillResults.sol";
import "./libs/LibOrder.sol";
import "./libs/LibMath.sol";
-import "./libs/LibStatus.sol";
import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
import "./mixins/MSettlement.sol";
@@ -30,9 +29,7 @@ import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
contract MixinExchangeCore is
- SafeMath,
LibMath,
- LibStatus,
LibOrder,
LibFillResults,
LibExchangeErrors,
@@ -58,13 +55,21 @@ contract MixinExchangeCore is
function cancelOrdersUpTo(uint256 salt)
external
{
- uint256 newMakerEpoch = salt + 1; // makerEpoch is initialized to 0, so to cancelUpTo we need salt + 1
+ address makerAddress = getCurrentContextAddress();
+
+ // makerEpoch is initialized to 0, so to cancelUpTo we need salt + 1
+ uint256 newMakerEpoch = salt + 1;
+ uint256 oldMakerEpoch = makerEpoch[makerAddress];
+
+ // Ensure makerEpoch is monotonically increasing
require(
- newMakerEpoch > makerEpoch[msg.sender], // epoch must be monotonically increasing
+ newMakerEpoch > oldMakerEpoch,
INVALID_NEW_MAKER_EPOCH
);
- makerEpoch[msg.sender] = newMakerEpoch;
- emit CancelUpTo(msg.sender, newMakerEpoch);
+
+ // Update makerEpoch
+ makerEpoch[makerAddress] = newMakerEpoch;
+ emit CancelUpTo(makerAddress, newMakerEpoch);
}
/// @dev Fills the input order.
@@ -86,29 +91,22 @@ contract MixinExchangeCore is
// Fetch taker address
address takerAddress = getCurrentContextAddress();
- // Either our context is valid or we revert
+ // Get amount of takerAsset to fill
+ uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderInfo.orderTakerAssetFilledAmount);
+ uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount);
+
+ // Validate context
assertValidFill(
order,
- orderInfo.orderStatus,
- orderInfo.orderHash,
+ orderInfo,
takerAddress,
- orderInfo.orderTakerAssetFilledAmount,
takerAssetFillAmount,
+ takerAssetFilledAmount,
signature
);
// Compute proportional fill amounts
- uint8 status;
- (status, fillResults) = calculateFillResults(
- order,
- orderInfo.orderStatus,
- orderInfo.orderTakerAssetFilledAmount,
- takerAssetFillAmount
- );
- if (status != uint8(Status.SUCCESS)) {
- emit ExchangeStatus(uint8(status), orderInfo.orderHash);
- return getNullFillResults();
- }
+ fillResults = calculateFillResults(order, takerAssetFilledAmount);
// Settle order
settleOrder(order, takerAddress, fillResults);
@@ -126,32 +124,28 @@ contract MixinExchangeCore is
/// @dev After calling, the order can not be filled anymore.
/// 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 order was valid, but in an
- /// unfillable state (see LibStatus.STATUS for order states)
+ /// @param order Order to cancel. Order must be OrderStatus.FILLABLE.
function cancelOrder(Order memory order)
public
- returns (bool)
{
// Fetch current order status
OrderInfo memory orderInfo = getOrderInfo(order);
// Validate context
- assertValidCancel(order, orderInfo.orderStatus, orderInfo.orderHash);
+ assertValidCancel(order, orderInfo);
// Perform cancel
- return updateCancelledState(order, orderInfo.orderStatus, orderInfo.orderHash);
+ updateCancelledState(order, orderInfo.orderHash);
}
/// @dev Gets information about an order: status, hash, and amount filled.
/// @param order Order to gather information on.
/// @return OrderInfo Information about the order and its state.
- /// See LibOrder.OrderInfo for a complete description.
+ /// See LibOrder.OrderInfo for a complete description.
function getOrderInfo(Order memory order)
public
view
- returns (LibOrder.OrderInfo memory orderInfo)
+ returns (OrderInfo memory orderInfo)
{
// Compute the order hash
orderInfo.orderHash = getOrderHash(order);
@@ -161,7 +155,7 @@ contract MixinExchangeCore is
// edge cases in the supporting infrastructure because they have
// an 'infinite' price when computed by a simple division.
if (order.makerAssetAmount == 0) {
- orderInfo.orderStatus = uint8(Status.ORDER_INVALID_MAKER_ASSET_AMOUNT);
+ orderInfo.orderStatus = uint8(OrderStatus.INVALID_MAKER_ASSET_AMOUNT);
return orderInfo;
}
@@ -170,148 +164,124 @@ contract MixinExchangeCore is
// Instead of distinguishing between unfilled and filled zero taker
// amount orders, we choose not to support them.
if (order.takerAssetAmount == 0) {
- orderInfo.orderStatus = uint8(Status.ORDER_INVALID_TAKER_ASSET_AMOUNT);
+ orderInfo.orderStatus = uint8(OrderStatus.INVALID_TAKER_ASSET_AMOUNT);
return orderInfo;
}
// Validate order expiration
if (block.timestamp >= order.expirationTimeSeconds) {
- orderInfo.orderStatus = uint8(Status.ORDER_EXPIRED);
+ orderInfo.orderStatus = uint8(OrderStatus.EXPIRED);
return orderInfo;
}
// Check if order has been cancelled
if (cancelled[orderInfo.orderHash]) {
- orderInfo.orderStatus = uint8(Status.ORDER_CANCELLED);
+ orderInfo.orderStatus = uint8(OrderStatus.CANCELLED);
return orderInfo;
}
if (makerEpoch[order.makerAddress] > order.salt) {
- orderInfo.orderStatus = uint8(Status.ORDER_CANCELLED);
+ orderInfo.orderStatus = uint8(OrderStatus.CANCELLED);
return orderInfo;
}
// Fetch filled amount and validate order availability
orderInfo.orderTakerAssetFilledAmount = filled[orderInfo.orderHash];
if (orderInfo.orderTakerAssetFilledAmount >= order.takerAssetAmount) {
- orderInfo.orderStatus = uint8(Status.ORDER_FULLY_FILLED);
+ orderInfo.orderStatus = uint8(OrderStatus.FULLY_FILLED);
return orderInfo;
}
// All other statuses are ruled out: order is Fillable
- orderInfo.orderStatus = uint8(Status.ORDER_FILLABLE);
+ orderInfo.orderStatus = uint8(OrderStatus.FILLABLE);
return orderInfo;
}
- /// @dev Calculates amounts filled and fees paid by maker and taker.
- /// @param order to be filled.
- /// @param orderStatus Status of order to be filled.
+ /// @dev Updates state with results of a fill order.
+ /// @param order that was filled.
+ /// @param takerAddress Address of taker who filled the order.
/// @param orderTakerAssetFilledAmount Amount of order already filled.
- /// @param takerAssetFillAmount Desired amount of order to fill by taker.
- /// @return status Return status of calculating fill amounts. Returns Status.SUCCESS on success.
/// @return fillResults Amounts filled and fees paid by maker and taker.
- function calculateFillResults(
+ function updateFilledState(
Order memory order,
- uint8 orderStatus,
+ address takerAddress,
+ bytes32 orderHash,
uint256 orderTakerAssetFilledAmount,
- uint256 takerAssetFillAmount
+ FillResults memory fillResults
)
- public
- pure
- returns (
- uint8 status,
- FillResults memory fillResults
- )
+ internal
{
- // Fill amount must be greater than 0
- if (takerAssetFillAmount == 0) {
- status = uint8(Status.TAKER_ASSET_FILL_AMOUNT_TOO_LOW);
- return (status, fillResults);
- }
-
- // Ensure the order is fillable
- if (orderStatus != uint8(Status.ORDER_FILLABLE)) {
- status = orderStatus;
- return (status, fillResults);
- }
-
- // Compute takerAssetFilledAmount
- uint256 remainingTakerAssetAmount = safeSub(order.takerAssetAmount, orderTakerAssetFilledAmount);
- uint256 takerAssetFilledAmount = min256(takerAssetFillAmount, remainingTakerAssetAmount);
-
- // Validate fill order rounding
- if (isRoundingError(
- takerAssetFilledAmount,
- order.takerAssetAmount,
- order.makerAssetAmount))
- {
- status = uint8(Status.ROUNDING_ERROR_TOO_LARGE);
- return (status, fillResults);
- }
+ // Update state
+ filled[orderHash] = safeAdd(orderTakerAssetFilledAmount, fillResults.takerAssetFilledAmount);
- // Compute proportional transfer amounts
- // TODO: All three are multiplied by the same fraction. This can
- // potentially be optimized.
- fillResults.takerAssetFilledAmount = takerAssetFilledAmount;
- fillResults.makerAssetFilledAmount = getPartialAmount(
- fillResults.takerAssetFilledAmount,
- order.takerAssetAmount,
- order.makerAssetAmount
- );
- fillResults.makerFeePaid = getPartialAmount(
- fillResults.takerAssetFilledAmount,
- order.takerAssetAmount,
- order.makerFee
- );
- fillResults.takerFeePaid = getPartialAmount(
+ // Log order
+ emit Fill(
+ order.makerAddress,
+ takerAddress,
+ order.feeRecipientAddress,
+ fillResults.makerAssetFilledAmount,
fillResults.takerAssetFilledAmount,
- order.takerAssetAmount,
- order.takerFee
+ fillResults.makerFeePaid,
+ fillResults.takerFeePaid,
+ orderHash,
+ order.makerAssetData,
+ order.takerAssetData
);
+ }
+
+ /// @dev Updates state with results of cancelling an order.
+ /// State is only updated if the order is currently fillable.
+ /// Otherwise, updating state would have no effect.
+ /// @param order that was cancelled.
+ /// @param orderHash Hash of order that was cancelled.
+ function updateCancelledState(
+ Order memory order,
+ bytes32 orderHash
+ )
+ internal
+ {
+ // Perform cancel
+ cancelled[orderHash] = true;
- status = uint8(Status.SUCCESS);
- return (status, fillResults);
+ // Log cancel
+ emit Cancel(
+ order.makerAddress,
+ order.feeRecipientAddress,
+ orderHash,
+ order.makerAssetData,
+ order.takerAssetData
+ );
}
/// @dev Validates context for fillOrder. Succeeds or throws.
/// @param order to be filled.
- /// @param orderStatus Status of order to be filled.
- /// @param orderHash Hash of order to be filled.
+ /// @param orderInfo OrderStatus, orderHash, and amount already filled of order.
/// @param takerAddress Address of order taker.
- /// @param orderTakerAssetFilledAmount Amount of order already filled.
/// @param takerAssetFillAmount Desired amount of order to fill by taker.
+ /// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
/// @param signature Proof that the orders was created by its maker.
function assertValidFill(
Order memory order,
- uint8 orderStatus,
- bytes32 orderHash,
+ OrderInfo memory orderInfo,
address takerAddress,
- uint256 orderTakerAssetFilledAmount,
uint256 takerAssetFillAmount,
+ uint256 takerAssetFilledAmount,
bytes memory signature
)
internal
+ view
{
- // Ensure order is valid
- // An order can only be filled if its status is FILLABLE;
- // however, only invalid statuses result in a throw.
- // See LibStatus for a complete description of order statuses.
+ // An order can only be filled if its status is FILLABLE.
require(
- orderStatus != uint8(Status.ORDER_INVALID_MAKER_ASSET_AMOUNT),
- INVALID_ORDER_MAKER_ASSET_AMOUNT
+ orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
+ ORDER_UNFILLABLE
);
+
+ // Revert if fill amount is invalid
require(
- orderStatus != uint8(Status.ORDER_INVALID_TAKER_ASSET_AMOUNT),
- INVALID_ORDER_TAKER_ASSET_AMOUNT
+ takerAssetFillAmount != 0,
+ INVALID_TAKER_AMOUNT
);
- // Validate Maker signature (check only if first time seen)
- if (orderTakerAssetFilledAmount == 0) {
- require(
- isValidSignature(orderHash, order.makerAddress, signature),
- SIGNATURE_VALIDATION_FAILED
- );
- }
-
// Validate sender is allowed to fill this order
if (order.senderAddress != address(0)) {
require(
@@ -324,76 +294,44 @@ contract MixinExchangeCore is
if (order.takerAddress != address(0)) {
require(
order.takerAddress == takerAddress,
- INVALID_CONTEXT
+ INVALID_TAKER
);
}
- require(
- takerAssetFillAmount > 0,
- GT_ZERO_AMOUNT_REQUIRED
- );
- }
- /// @dev Updates state with results of a fill order.
- /// @param order that was filled.
- /// @param takerAddress Address of taker who filled the order.
- /// @param orderTakerAssetFilledAmount Amount of order already filled.
- /// @return fillResults Amounts filled and fees paid by maker and taker.
- function updateFilledState(
- Order memory order,
- address takerAddress,
- bytes32 orderHash,
- uint256 orderTakerAssetFilledAmount,
- FillResults memory fillResults
- )
- internal
- {
- // Update state
- filled[orderHash] = safeAdd(orderTakerAssetFilledAmount, fillResults.takerAssetFilledAmount);
+ // Validate Maker signature (check only if first time seen)
+ if (orderInfo.orderTakerAssetFilledAmount == 0) {
+ require(
+ isValidSignature(orderInfo.orderHash, order.makerAddress, signature),
+ INVALID_ORDER_SIGNATURE
+ );
+ }
- // Log order
- emit Fill(
- order.makerAddress,
- takerAddress,
- order.feeRecipientAddress,
- fillResults.makerAssetFilledAmount,
- fillResults.takerAssetFilledAmount,
- fillResults.makerFeePaid,
- fillResults.takerFeePaid,
- orderHash,
- order.makerAssetData,
- order.takerAssetData
+ // Validate fill order rounding
+ require(
+ !isRoundingError(
+ takerAssetFilledAmount,
+ order.takerAssetAmount,
+ order.makerAssetAmount
+ ),
+ ROUNDING_ERROR
);
}
/// @dev Validates context for cancelOrder. Succeeds or throws.
- /// @param order that was cancelled.
- /// @param orderStatus Status of order that was cancelled.
- /// @param orderHash Hash of order that was cancelled.
+ /// @param order to be cancelled.
+ /// @param orderInfo OrderStatus, orderHash, and amount already filled of order.
function assertValidCancel(
Order memory order,
- uint8 orderStatus,
- bytes32 orderHash
+ OrderInfo memory orderInfo
)
internal
+ view
{
// Ensure order is valid
- // An order can only be cancelled if its status is FILLABLE;
- // however, only invalid statuses result in a throw.
- // See LibStatus for a complete description of order statuses.
- require(
- orderStatus != uint8(Status.ORDER_INVALID_MAKER_ASSET_AMOUNT),
- INVALID_ORDER_MAKER_ASSET_AMOUNT
- );
+ // An order can only be cancelled if its status is FILLABLE.
require(
- orderStatus != uint8(Status.ORDER_INVALID_TAKER_ASSET_AMOUNT),
- INVALID_ORDER_TAKER_ASSET_AMOUNT
- );
-
- // Validate transaction signed by maker
- address makerAddress = getCurrentContextAddress();
- require(
- order.makerAddress == makerAddress,
- INVALID_CONTEXT
+ orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
+ ORDER_UNFILLABLE
);
// Validate sender is allowed to cancel this order
@@ -403,44 +341,47 @@ contract MixinExchangeCore is
INVALID_SENDER
);
}
+
+ // Validate transaction signed by maker
+ address makerAddress = getCurrentContextAddress();
+ require(
+ order.makerAddress == makerAddress,
+ INVALID_MAKER
+ );
}
- /// @dev Updates state with results of cancelling an order.
- /// State is only updated if the order is currently fillable.
- /// Otherwise, updating state would have no effect.
- /// @param order that was cancelled.
- /// @param orderStatus Status of order that was cancelled.
- /// @param orderHash Hash of order that was cancelled.
- /// @return stateUpdated Returns true only if state was updated.
- function updateCancelledState(
+ /// @dev Calculates amounts filled and fees paid by maker and taker.
+ /// @param order to be filled.
+ /// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
+ /// @return fillResults Amounts filled and fees paid by maker and taker.
+ function calculateFillResults(
Order memory order,
- uint8 orderStatus,
- bytes32 orderHash
+ uint256 takerAssetFilledAmount
)
internal
- returns (bool stateUpdated)
+ pure
+ returns (FillResults memory fillResults)
{
- // Ensure order is fillable (otherwise cancelling does nothing)
- // See LibStatus for a complete description of order statuses.
- if (orderStatus != uint8(Status.ORDER_FILLABLE)) {
- emit ExchangeStatus(uint8(orderStatus), orderHash);
- stateUpdated = false;
- return stateUpdated;
- }
-
- // Perform cancel
- cancelled[orderHash] = true;
- stateUpdated = true;
-
- // Log cancel
- emit Cancel(
- order.makerAddress,
- order.feeRecipientAddress,
- orderHash,
- order.makerAssetData,
- order.takerAssetData
+ // Compute proportional transfer amounts
+ // TODO: All three are multiplied by the same fraction. This can
+ // potentially be optimized.
+ fillResults.takerAssetFilledAmount = takerAssetFilledAmount;
+ fillResults.makerAssetFilledAmount = getPartialAmount(
+ fillResults.takerAssetFilledAmount,
+ order.takerAssetAmount,
+ order.makerAssetAmount
+ );
+ fillResults.makerFeePaid = getPartialAmount(
+ fillResults.takerAssetFilledAmount,
+ order.takerAssetAmount,
+ order.makerFee
+ );
+ fillResults.takerFeePaid = getPartialAmount(
+ fillResults.takerAssetFilledAmount,
+ order.takerAssetAmount,
+ order.takerFee
);
- return stateUpdated;
+ return fillResults;
}
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
index 9d8b521c0..ed76287e0 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
@@ -14,24 +14,19 @@
pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
+import "../../utils/LibBytes/LibBytes.sol";
+import "./libs/LibMath.sol";
+import "./libs/LibOrder.sol";
+import "./libs/LibFillResults.sol";
+import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
import "./mixins/MMatchOrders.sol";
import "./mixins/MSettlement.sol";
import "./mixins/MTransactions.sol";
-import "../../utils/SafeMath/SafeMath.sol";
-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,
LibBytes,
LibMath,
- LibStatus,
- LibOrder,
- LibFillResults,
LibExchangeErrors,
MExchangeCore,
MMatchOrders,
@@ -50,17 +45,22 @@ contract MixinMatchOrders is
/// @return matchedFillResults Amounts filled and fees paid by maker and taker of matched orders.
/// TODO: Make this function external once supported by Solidity (See Solidity Issues #3199, #1603)
function matchOrders(
- Order memory leftOrder,
- Order memory rightOrder,
+ LibOrder.Order memory leftOrder,
+ LibOrder.Order memory rightOrder,
bytes memory leftSignature,
bytes memory rightSignature
)
public
- returns (MatchedFillResults memory matchedFillResults)
+ returns (LibFillResults.MatchedFillResults memory matchedFillResults)
{
+ // We assume that rightOrder.takerAssetData == leftOrder.makerAssetData and rightOrder.makerAssetData == leftOrder.takerAssetData.
+ // If this assumption isn't true, the match will fail at signature validation.
+ rightOrder.makerAssetData = leftOrder.takerAssetData;
+ rightOrder.takerAssetData = leftOrder.makerAssetData;
+
// Get left & right order info
- OrderInfo memory leftOrderInfo = getOrderInfo(leftOrder);
- OrderInfo memory rightOrderInfo = getOrderInfo(rightOrder);
+ LibOrder.OrderInfo memory leftOrderInfo = getOrderInfo(leftOrder);
+ LibOrder.OrderInfo memory rightOrderInfo = getOrderInfo(rightOrder);
// Fetch taker address
address takerAddress = getCurrentContextAddress();
@@ -72,8 +72,6 @@ contract MixinMatchOrders is
matchedFillResults = calculateMatchedFillResults(
leftOrder,
rightOrder,
- leftOrderInfo.orderStatus,
- rightOrderInfo.orderStatus,
leftOrderInfo.orderTakerAssetFilledAmount,
rightOrderInfo.orderTakerAssetFilledAmount
);
@@ -81,19 +79,17 @@ contract MixinMatchOrders is
// Validate fill contexts
assertValidFill(
leftOrder,
- leftOrderInfo.orderStatus,
- leftOrderInfo.orderHash,
+ leftOrderInfo,
takerAddress,
- leftOrderInfo.orderTakerAssetFilledAmount,
+ matchedFillResults.left.takerAssetFilledAmount,
matchedFillResults.left.takerAssetFilledAmount,
leftSignature
);
assertValidFill(
rightOrder,
- rightOrderInfo.orderStatus,
- rightOrderInfo.orderHash,
+ rightOrderInfo,
takerAddress,
- rightOrderInfo.orderTakerAssetFilledAmount,
+ matchedFillResults.right.takerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount,
rightSignature
);
@@ -129,25 +125,12 @@ contract MixinMatchOrders is
/// @param leftOrder First order to match.
/// @param rightOrder Second order to match.
function assertValidMatch(
- Order memory leftOrder,
- Order memory rightOrder
+ LibOrder.Order memory leftOrder,
+ LibOrder.Order memory rightOrder
)
internal
+ pure
{
- // 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 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).
@@ -159,39 +142,7 @@ contract MixinMatchOrders is
require(
safeMul(leftOrder.makerAssetAmount, rightOrder.makerAssetAmount) >=
safeMul(leftOrder.takerAssetAmount, rightOrder.takerAssetAmount),
- NEGATIVE_SPREAD
- );
- }
-
- /// @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 assertValidMatchResults(MatchedFillResults memory matchedFillResults)
- internal
- {
- // 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(
- !isRoundingError(
- matchedFillResults.left.makerAssetFilledAmount,
- amountSpentByLeft,
- 1
- ),
- ROUNDING_ERROR_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(
- !isRoundingError(
- matchedFillResults.right.makerAssetFilledAmount,
- matchedFillResults.left.takerAssetFilledAmount,
- 1
- ),
- ROUNDING_ERROR_TRANSFER_AMOUNTS
+ NEGATIVE_SPREAD_REQUIRED
);
}
@@ -201,21 +152,18 @@ contract MixinMatchOrders is
/// The profit made by the leftOrder order goes to the taker (who matched the two orders).
/// @param leftOrder First order to match.
/// @param rightOrder Second order to match.
- /// @param leftOrderStatus Order status of left order.
- /// @param rightOrderStatus Order status of right order.
- /// @param leftOrderFilledAmount Amount of left order already filled.
- /// @param rightOrderFilledAmount Amount of right order already filled.
+ /// @param leftOrderTakerAssetFilledAmount Amount of left order already filled.
+ /// @param rightOrderTakerAssetFilledAmount Amount of right order already filled.
/// @param matchedFillResults Amounts to fill and fees to pay by maker and taker of matched orders.
function calculateMatchedFillResults(
- Order memory leftOrder,
- Order memory rightOrder,
- uint8 leftOrderStatus,
- uint8 rightOrderStatus,
- uint256 leftOrderFilledAmount,
- uint256 rightOrderFilledAmount
+ LibOrder.Order memory leftOrder,
+ LibOrder.Order memory rightOrder,
+ uint256 leftOrderTakerAssetFilledAmount,
+ uint256 rightOrderTakerAssetFilledAmount
)
internal
- returns (MatchedFillResults memory matchedFillResults)
+ pure
+ returns (LibFillResults.MatchedFillResults memory matchedFillResults)
{
// We settle orders at the exchange rate of the right order.
// The amount saved by the left maker goes to the taker.
@@ -226,71 +174,55 @@ contract MixinMatchOrders is
// <leftTakerAssetAmountRemaining> <= <rightTakerAssetAmountRemaining> * <rightMakerToTakerRatio>
// <leftTakerAssetAmountRemaining> <= <rightTakerAssetAmountRemaining> * <rightOrder.makerAssetAmount> / <rightOrder.takerAssetAmount>
// <leftTakerAssetAmountRemaining> * <rightOrder.takerAssetAmount> <= <rightTakerAssetAmountRemaining> * <rightOrder.makerAssetAmount>
- uint256 rightTakerAssetAmountRemaining = safeSub(rightOrder.takerAssetAmount, rightOrderFilledAmount);
- uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderFilledAmount);
- uint256 leftOrderAmountToFill;
- uint256 rightOrderAmountToFill;
+ uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount);
+ uint256 rightTakerAssetAmountRemaining = safeSub(rightOrder.takerAssetAmount, rightOrderTakerAssetFilledAmount);
+ uint256 leftTakerAssetFilledAmount;
+ uint256 rightTakerAssetFilledAmount;
if (
safeMul(leftTakerAssetAmountRemaining, rightOrder.takerAssetAmount) <=
safeMul(rightTakerAssetAmountRemaining, rightOrder.makerAssetAmount)
) {
// Left order will be fully filled: maximally fill left
- leftOrderAmountToFill = leftTakerAssetAmountRemaining;
+ leftTakerAssetFilledAmount = leftTakerAssetAmountRemaining;
// The right order receives an amount proportional to how much was spent.
// TODO: Can we ensure rounding error is in the correct direction?
- rightOrderAmountToFill = safeGetPartialAmount(
+ rightTakerAssetFilledAmount = getPartialAmount(
rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount,
- leftOrderAmountToFill
+ leftTakerAssetFilledAmount
);
} else {
// Right order will be fully filled: maximally fill right
- rightOrderAmountToFill = rightTakerAssetAmountRemaining;
+ rightTakerAssetFilledAmount = rightTakerAssetAmountRemaining;
// The left order receives an amount proportional to how much was spent.
// TODO: Can we ensure rounding error is in the correct direction?
- leftOrderAmountToFill = safeGetPartialAmount(
+ leftTakerAssetFilledAmount = getPartialAmount(
rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount,
- rightOrderAmountToFill
+ rightTakerAssetFilledAmount
);
}
// Calculate fill results for left order
- uint8 status;
- (status, matchedFillResults.left) = calculateFillResults(
+ matchedFillResults.left = calculateFillResults(
leftOrder,
- leftOrderStatus,
- leftOrderFilledAmount,
- leftOrderAmountToFill
- );
- require(
- status == uint8(Status.SUCCESS),
- FAILED_TO_CALCULATE_FILL_RESULTS_FOR_LEFT_ORDER
+ leftTakerAssetFilledAmount
);
// Calculate fill results for right order
- (status, matchedFillResults.right) = calculateFillResults(
+ matchedFillResults.right = calculateFillResults(
rightOrder,
- rightOrderStatus,
- rightOrderFilledAmount,
- rightOrderAmountToFill
- );
- require(
- status == uint8(Status.SUCCESS),
- FAILED_TO_CALCULATE_FILL_RESULTS_FOR_RIGHT_ORDER
+ rightTakerAssetFilledAmount
);
// Calculate amount given to taker
- matchedFillResults.takerFillAmount = safeSub(
+ matchedFillResults.leftMakerAssetSpreadAmount = safeSub(
matchedFillResults.left.makerAssetFilledAmount,
matchedFillResults.right.takerAssetFilledAmount
);
- // Validate the fill results
- assertValidMatchResults(matchedFillResults);
-
// Return fill results
return matchedFillResults;
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
index 7c03bde75..646d3ed58 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol
@@ -19,17 +19,16 @@
pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
-import "./mixins/MSettlement.sol";
-import "./mixins/MAssetProxyDispatcher.sol";
-import "./libs/LibOrder.sol";
import "./libs/LibMath.sol";
-import "./libs/LibExchangeErrors.sol";
import "./libs/LibFillResults.sol";
+import "./libs/LibOrder.sol";
+import "./libs/LibExchangeErrors.sol";
import "./mixins/MMatchOrders.sol";
+import "./mixins/MSettlement.sol";
+import "./mixins/MAssetProxyDispatcher.sol";
contract MixinSettlement is
LibMath,
- LibFillResults,
LibExchangeErrors,
MMatchOrders,
MSettlement,
@@ -65,7 +64,7 @@ contract MixinSettlement is
function settleOrder(
LibOrder.Order memory order,
address takerAddress,
- FillResults memory fillResults
+ LibFillResults.FillResults memory fillResults
)
internal
{
@@ -104,7 +103,7 @@ contract MixinSettlement is
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder,
address takerAddress,
- MatchedFillResults memory matchedFillResults
+ LibFillResults.MatchedFillResults memory matchedFillResults
)
internal
{
@@ -125,7 +124,7 @@ contract MixinSettlement is
leftOrder.makerAssetData,
leftOrder.makerAddress,
takerAddress,
- matchedFillResults.takerFillAmount
+ matchedFillResults.leftMakerAssetSpreadAmount
);
// Maker fees
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 79dc87af0..48a0c5552 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -18,12 +18,12 @@
pragma solidity ^0.4.24;
+import "../../utils/LibBytes/LibBytes.sol";
+import "./libs/LibExchangeErrors.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
import "./interfaces/IWallet.sol";
import "./interfaces/IValidator.sol";
-import "./libs/LibExchangeErrors.sol";
-import "../../utils/LibBytes/LibBytes.sol";
contract MixinSignatureValidator is
LibBytes,
@@ -31,6 +31,9 @@ contract MixinSignatureValidator is
MSignatureValidator,
MTransactions
{
+ // Personal message headers
+ string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32";
+ string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x41";
// Mapping of hash => signer => signed
mapping (bytes32 => mapping (address => bool)) public preSigned;
@@ -51,7 +54,7 @@ contract MixinSignatureValidator is
{
require(
isValidSignature(hash, signer, signature),
- SIGNATURE_VALIDATION_FAILED
+ INVALID_SIGNATURE
);
preSigned[hash][signer] = true;
}
@@ -85,8 +88,8 @@ contract MixinSignatureValidator is
{
// TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.)
require(
- signature.length >= 1,
- INVALID_SIGNATURE_LENGTH
+ signature.length > 0,
+ LENGTH_GREATER_THAN_0_REQUIRED
);
// Pop last byte off of signature byte array.
@@ -104,7 +107,7 @@ contract MixinSignatureValidator is
// it an explicit option. This aids testing and analysis. It is
// also the initialization value for the enum type.
if (signatureType == SignatureType.Illegal) {
- revert(ILLEGAL_SIGNATURE_TYPE);
+ revert(SIGNATURE_ILLEGAL);
// Always invalid signature.
// Like Illegal, this is always implicitly available and therefore
@@ -113,7 +116,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Invalid) {
require(
signature.length == 0,
- INVALID_SIGNATURE_LENGTH
+ LENGTH_0_REQUIRED
);
isValid = false;
return isValid;
@@ -122,7 +125,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.EIP712) {
require(
signature.length == 65,
- INVALID_SIGNATURE_LENGTH
+ LENGTH_65_REQUIRED
);
v = uint8(signature[0]);
r = readBytes32(signature, 1);
@@ -135,13 +138,13 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.EthSign) {
require(
signature.length == 65,
- INVALID_SIGNATURE_LENGTH
+ LENGTH_65_REQUIRED
);
v = uint8(signature[0]);
r = readBytes32(signature, 1);
s = readBytes32(signature, 33);
recovered = ecrecover(
- keccak256("\x19Ethereum Signed Message:\n32", hash),
+ keccak256(abi.encodePacked(ETH_PERSONAL_MESSAGE, hash)),
v,
r,
s
@@ -160,7 +163,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Caller) {
require(
signature.length == 0,
- INVALID_SIGNATURE_LENGTH
+ LENGTH_0_REQUIRED
);
isValid = signer == msg.sender;
return isValid;
@@ -208,13 +211,13 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Trezor) {
require(
signature.length == 65,
- INVALID_SIGNATURE_LENGTH
+ LENGTH_65_REQUIRED
);
v = uint8(signature[0]);
r = readBytes32(signature, 1);
s = readBytes32(signature, 33);
recovered = ecrecover(
- keccak256("\x19Ethereum Signed Message:\n\x41", hash),
+ keccak256(abi.encodePacked(TREZOR_PERSONAL_MESSAGE, hash)),
v,
r,
s
@@ -233,6 +236,6 @@ contract MixinSignatureValidator is
// that we currently support. In this case returning false
// may lead the caller to incorrectly believe that the
// signature was invalid.)
- revert(UNSUPPORTED_SIGNATURE_TYPE);
+ revert(SIGNATURE_UNSUPPORTED);
}
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index d153dfa5c..30b0102fd 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -17,9 +17,9 @@
*/
pragma solidity ^0.4.24;
+import "./libs/LibExchangeErrors.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
-import "./libs/LibExchangeErrors.sol";
contract MixinTransactions is
LibExchangeErrors,
@@ -50,29 +50,29 @@ contract MixinTransactions is
// Prevent reentrancy
require(
currentContextAddress == address(0),
- REENTRANCY_NOT_ALLOWED
+ REENTRANCY_ILLEGAL
);
// Calculate transaction hash
- bytes32 transactionHash = keccak256(
+ bytes32 transactionHash = keccak256(abi.encodePacked(
address(this),
signer,
salt,
data
- );
+ ));
// Validate transaction has not been executed
require(
!transactions[transactionHash],
- DUPLICATE_TRANSACTION_HASH
+ INVALID_TX_HASH
);
- // TODO: is SignatureType.Caller necessary if we make this check?
+ // Transaction always valid if signer is sender of transaction
if (signer != msg.sender) {
// Validate signature
require(
isValidSignature(transactionHash, signer, signature),
- SIGNATURE_VALIDATION_FAILED
+ INVALID_TX_SIGNATURE
);
// Set the current transaction signer
@@ -83,7 +83,7 @@ contract MixinTransactions is
transactions[transactionHash] = true;
require(
address(this).delegatecall(data),
- TRANSACTION_EXECUTION_FAILED
+ FAILED_EXECUTION
);
// Reset current transaction signer
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
index 15f1a2e0b..0ad0710ce 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
@@ -20,17 +20,15 @@ pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol";
-import "./mixins/MExchangeCore.sol";
import "./libs/LibMath.sol";
import "./libs/LibOrder.sol";
import "./libs/LibFillResults.sol";
import "./libs/LibExchangeErrors.sol";
+import "./mixins/MExchangeCore.sol";
contract MixinWrapperFunctions is
- SafeMath,
LibBytes,
LibMath,
- LibOrder,
LibFillResults,
LibExchangeErrors,
MExchangeCore
@@ -40,7 +38,7 @@ contract MixinWrapperFunctions is
/// @param takerAssetFillAmount Desired amount of takerAsset to sell.
/// @param signature Proof that order has been created by maker.
function fillOrKillOrder(
- Order memory order,
+ LibOrder.Order memory order,
uint256 takerAssetFillAmount,
bytes memory signature)
public
@@ -65,7 +63,7 @@ contract MixinWrapperFunctions is
/// @param signature Proof that order has been created by maker.
/// @return Amounts filled and fees paid by maker and taker.
function fillOrderNoThrow(
- Order memory order,
+ LibOrder.Order memory order,
uint256 takerAssetFillAmount,
bytes memory signature)
public
@@ -264,7 +262,7 @@ contract MixinWrapperFunctions is
/// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders.
/// @param signatures Proofs that orders have been created by makers.
function batchFillOrders(
- Order[] memory orders,
+ LibOrder.Order[] memory orders,
uint256[] memory takerAssetFillAmounts,
bytes[] memory signatures)
public
@@ -283,7 +281,7 @@ contract MixinWrapperFunctions is
/// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders.
/// @param signatures Proofs that orders have been created by makers.
function batchFillOrKillOrders(
- Order[] memory orders,
+ LibOrder.Order[] memory orders,
uint256[] memory takerAssetFillAmounts,
bytes[] memory signatures)
public
@@ -303,7 +301,7 @@ contract MixinWrapperFunctions is
/// @param takerAssetFillAmounts Array of desired amounts of takerAsset to sell in orders.
/// @param signatures Proofs that orders have been created by makers.
function batchFillOrdersNoThrow(
- Order[] memory orders,
+ LibOrder.Order[] memory orders,
uint256[] memory takerAssetFillAmounts,
bytes[] memory signatures)
public
@@ -323,7 +321,7 @@ contract MixinWrapperFunctions is
/// @param signatures Proofs that orders have been created by makers.
/// @return Amounts filled and fees paid by makers and taker.
function marketSellOrders(
- Order[] memory orders,
+ LibOrder.Order[] memory orders,
uint256 takerAssetFillAmount,
bytes[] memory signatures)
public
@@ -366,7 +364,7 @@ contract MixinWrapperFunctions is
/// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts filled and fees paid by makers and taker.
function marketSellOrdersNoThrow(
- Order[] memory orders,
+ LibOrder.Order[] memory orders,
uint256 takerAssetFillAmount,
bytes[] memory signatures)
public
@@ -408,7 +406,7 @@ contract MixinWrapperFunctions is
/// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts filled and fees paid by makers and taker.
function marketBuyOrders(
- Order[] memory orders,
+ LibOrder.Order[] memory orders,
uint256 makerAssetFillAmount,
bytes[] memory signatures)
public
@@ -459,7 +457,7 @@ contract MixinWrapperFunctions is
/// @param signatures Proofs that orders have been signed by makers.
/// @return Amounts filled and fees paid by makers and taker.
function marketBuyOrdersNoThrow(
- Order[] memory orders,
+ LibOrder.Order[] memory orders,
uint256 makerAssetFillAmount,
bytes[] memory signatures)
public
@@ -505,7 +503,7 @@ contract MixinWrapperFunctions is
/// @dev Synchronously cancels multiple orders in a single transaction.
/// @param orders Array of order specifications.
- function batchCancelOrders(Order[] memory orders)
+ function batchCancelOrders(LibOrder.Order[] memory orders)
public
{
for (uint256 i = 0; i < orders.length; i++) {
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol
index fc0157d75..7ca2dd052 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IExchangeCore.sol
@@ -37,17 +37,15 @@ contract IExchangeCore {
function fillOrder(
LibOrder.Order memory order,
uint256 takerAssetFillAmount,
- bytes memory signature)
+ bytes memory signature
+ )
public
returns (LibFillResults.FillResults memory fillResults);
/// @dev After calling, the order can not be filled anymore.
/// @param order Order struct containing order specifications.
- /// @return True if the order state changed to cancelled.
- /// False if the transaction was already cancelled or expired.
function cancelOrder(LibOrder.Order memory order)
- public
- returns (bool);
+ public;
/// @dev Gets information about an order: status, hash, and amount filled.
/// @param order Order to gather information on.
@@ -57,24 +55,4 @@ contract IExchangeCore {
public
view
returns (LibOrder.OrderInfo memory orderInfo);
-
- /// @dev Calculates amounts filled and fees paid by maker and taker.
- /// @param order to be filled.
- /// @param orderStatus Status of order to be filled.
- /// @param orderTakerAssetFilledAmount Amount of order already filled.
- /// @param takerAssetFillAmount Desired amount of order to fill by taker.
- /// @return status Return status of calculating fill amounts. Returns Status.SUCCESS on success.
- /// @return fillResults Amounts filled and fees paid by maker and taker.
- function calculateFillResults(
- LibOrder.Order memory order,
- uint8 orderStatus,
- uint256 orderTakerAssetFilledAmount,
- uint256 takerAssetFillAmount
- )
- public
- pure
- returns (
- uint8 status,
- LibFillResults.FillResults memory fillResults
- );
}
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 7d67e5080..2b4bbeec4 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
@@ -19,43 +19,44 @@
pragma solidity ^0.4.24;
contract LibExchangeErrors {
-
- // Core revert reasons
- string constant GT_ZERO_AMOUNT_REQUIRED = "Amount must be greater than 0.";
- string constant SIGNATURE_VALIDATION_FAILED = "Signature validation failed.";
- string constant INVALID_SENDER = "Invalid `msg.sender`.";
- string constant INVALID_CONTEXT = "Function called in an invalid context.";
- string constant INVALID_NEW_MAKER_EPOCH = "Specified salt must be greater than or equal to existing makerEpoch.";
-
- // Order revert reasons
- string constant INVALID_ORDER_TAKER_ASSET_AMOUNT = "Invalid order taker asset amount: expected a non-zero value.";
- string constant INVALID_ORDER_MAKER_ASSET_AMOUNT = "Invalid order maker asset amount: expected a non-zero value.";
-
- // Transaction revert reasons
- string constant REENTRANCY_NOT_ALLOWED = "`executeTransaction` is not allowed to call itself recursively.";
- string constant DUPLICATE_TRANSACTION_HASH = "Transaction has already been executed.";
- string constant TRANSACTION_EXECUTION_FAILED = "Transaction execution failed.";
-
- // Wrapper revert reasons
- string constant COMPLETE_FILL_FAILED = "Desired fill amount could not be completely filled.";
- string constant ASSET_DATA_MISMATCH = "Asset data must be the same for each order.";
-
- // Asset proxy dispatcher revert reasons
- string constant GT_ZERO_LENGTH_REQUIRED = "Length must be greater than 0.";
- string constant OLD_ASSET_PROXY_MISMATCH = "Old asset proxy does not match asset proxy at given id.";
- string constant NEW_ASSET_PROXY_MISMATCH = "New asset proxy id does not match given id.";
-
- // Signature validator revert reasons
- 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 FAILED_TO_CALCULATE_FILL_RESULTS_FOR_LEFT_ORDER = "Failed to calculate fill results for left order.";
- string constant FAILED_TO_CALCULATE_FILL_RESULTS_FOR_RIGHT_ORDER = "Failed to calculate fill results for right order.";
+ /// Order validation errors ///
+ string constant ORDER_UNFILLABLE = "ORDER_UNFILLABLE"; // Order cannot be filled.
+ string constant INVALID_MAKER = "INVALID_MAKER"; // Invalid makerAddress.
+ string constant INVALID_TAKER = "INVALID_TAKER"; // Invalid takerAddress.
+ string constant INVALID_SENDER = "INVALID_SENDER"; // Invalid `msg.sender`.
+ string constant INVALID_ORDER_SIGNATURE = "INVALID_ORDER_SIGNATURE"; // Signature validation failed.
+ string constant ASSET_DATA_MISMATCH = "ASSET_DATA_MISMATCH"; // Asset data must be the same for each order.
+
+ /// fillOrder validation errors ///
+ string constant INVALID_TAKER_AMOUNT = "INVALID_TAKER_AMOUNT"; // takerAssetFillAmount cannot equal 0.
+ string constant ROUNDING_ERROR = "ROUNDING_ERROR"; // Rounding error greater than 0.1% of takerAssetFillAmount.
+
+ /// Signature validation errors ///
+ string constant INVALID_SIGNATURE = "INVALID_SIGNATURE"; // Signature validation failed.
+ string constant SIGNATURE_ILLEGAL = "SIGNATURE_ILLEGAL"; // Signature type is illegal.
+ string constant SIGNATURE_UNSUPPORTED = "SIGNATURE_UNSUPPORTED"; // Signature type unsupported.
+
+ /// cancelOrdersUptTo errors ///
+ string constant INVALID_NEW_MAKER_EPOCH = "INVALID_NEW_MAKER_EPOCH"; // Specified salt must be greater than or equal to existing makerEpoch.
+
+ /// fillOrKillOrder errors ///
+ string constant COMPLETE_FILL_FAILED = "COMPLETE_FILL_FAILED"; // Desired takerAssetFillAmount could not be completely filled.
+
+ /// matchOrders errors ///
+ string constant NEGATIVE_SPREAD_REQUIRED = "NEGATIVE_SPREAD_REQUIRED"; // Matched orders must have a negative spread.
+
+ /// Transaction errors ///
+ string constant REENTRANCY_ILLEGAL = "REENTRANCY_ILLEGAL"; // Recursive reentrancy is not allowed.
+ string constant INVALID_TX_HASH = "INVALID_TX_HASH"; // Transaction has already been executed.
+ string constant INVALID_TX_SIGNATURE = "INVALID_TX_SIGNATURE"; // Signature validation failed.
+ string constant FAILED_EXECUTION = "FAILED_EXECUTION"; // Transaction execution failed.
+
+ /// registerAssetProxy errors ///
+ string constant ASSET_PROXY_MISMATCH = "ASSET_PROXY_MISMATCH"; // oldAssetProxy proxy does not match currentAssetProxy.
+ string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // newAssetProxyId does not match given assetProxyId.
+
+ /// Length validation errors ///
+ string constant LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0.
+ string constant LENGTH_0_REQUIRED = "LENGTH_1_REQUIRED"; // Byte array must have a length of 1.
+ string constant LENGTH_65_REQUIRED = "LENGTH_66_REQUIRED"; // Byte array must have a length of 66.
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol
index aa54598fa..b7550d6d2 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol
@@ -32,9 +32,9 @@ contract LibFillResults is
}
struct MatchedFillResults {
- LibFillResults.FillResults left;
- LibFillResults.FillResults right;
- uint256 takerFillAmount;
+ FillResults left;
+ FillResults right;
+ uint256 leftMakerAssetSpreadAmount;
}
/// @dev Adds properties of both FillResults instances.
@@ -50,19 +50,4 @@ contract LibFillResults is
totalFillResults.makerFeePaid = safeAdd(totalFillResults.makerFeePaid, singleFillResults.makerFeePaid);
totalFillResults.takerFeePaid = safeAdd(totalFillResults.takerFeePaid, singleFillResults.takerFeePaid);
}
-
- /// @dev Returns a null fill results struct
- function getNullFillResults()
- internal
- pure
- returns (FillResults memory)
- {
- // returns zeroed out FillResults instance
- return FillResults({
- makerAssetFilledAmount: 0,
- takerAssetFilledAmount: 0,
- makerFeePaid: 0,
- takerFeePaid: 0
- });
- }
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
index ea8c138d6..233547b9f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
@@ -45,26 +45,6 @@ contract LibMath is
return partialAmount;
}
- /// @dev Calculates partial value given a numerator and denominator.
- /// Throws if there is a rounding error.
- /// @param numerator Numerator.
- /// @param denominator Denominator.
- /// @param target Value to calculate partial of.
- /// @return Partial value of target.
- function safeGetPartialAmount(
- uint256 numerator,
- uint256 denominator,
- uint256 target)
- internal pure
- returns (uint256 partialAmount)
- {
- require(
- !isRoundingError(numerator, denominator, target),
- ROUNDING_ERROR_ON_PARTIAL_AMOUNT
- );
- return getPartialAmount(numerator, denominator, target);
- }
-
/// @dev Checks if rounding error > 0.1%.
/// @param numerator Numerator.
/// @param denominator Denominator.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol
index ed7f9391d..05ea27ffc 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol
@@ -20,11 +20,11 @@ pragma solidity ^0.4.24;
contract LibOrder {
- bytes32 constant DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(
+ bytes32 constant DOMAIN_SEPARATOR_SCHEMA_HASH = keccak256(abi.encodePacked(
"DomainSeparator(address contract)"
- );
+ ));
- bytes32 constant ORDER_SCHEMA_HASH = keccak256(
+ bytes32 constant ORDER_SCHEMA_HASH = keccak256(abi.encodePacked(
"Order(",
"address makerAddress,",
"address takerAddress,",
@@ -39,7 +39,19 @@ contract LibOrder {
"bytes makerAssetData,",
"bytes takerAssetData,",
")"
- );
+ ));
+
+ // A valid order remains fillable until it is expired, fully filled, or cancelled.
+ // An order's state is unaffected by external factors, like account balances.
+ enum OrderStatus {
+ INVALID, // Default value
+ INVALID_MAKER_ASSET_AMOUNT, // Order does not have a valid maker asset amount
+ INVALID_TAKER_ASSET_AMOUNT, // Order does not have a valid taker asset amount
+ FILLABLE, // Order is fillable
+ EXPIRED, // Order has already expired
+ FULLY_FILLED, // Order is fully filled
+ CANCELLED // Order has been cancelled
+ }
struct Order {
address makerAddress;
@@ -75,11 +87,11 @@ contract LibOrder {
{
// TODO: EIP712 is not finalized yet
// Source: https://github.com/ethereum/EIPs/pull/712
- orderHash = keccak256(
+ orderHash = keccak256(abi.encodePacked(
DOMAIN_SEPARATOR_SCHEMA_HASH,
- keccak256(address(this)),
+ keccak256(abi.encodePacked(address(this))),
ORDER_SCHEMA_HASH,
- keccak256(
+ keccak256(abi.encodePacked(
order.makerAddress,
order.takerAddress,
order.feeRecipientAddress,
@@ -90,10 +102,10 @@ contract LibOrder {
order.takerFee,
order.expirationTimeSeconds,
order.salt,
- keccak256(order.makerAssetData),
- keccak256(order.takerAssetData)
- )
- );
+ keccak256(abi.encodePacked(order.makerAssetData)),
+ keccak256(abi.encodePacked(order.takerAssetData))
+ ))
+ ));
return orderHash;
}
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol
deleted file mode 100644
index f72b7d65f..000000000
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibStatus.sol
+++ /dev/null
@@ -1,51 +0,0 @@
-/*
-
- Copyright 2018 ZeroEx Intl.
-
- Licensed under the Apache License, Version 2.0 (the "License");
- you may not use this file except in compliance with the License.
- You may obtain a copy of the License at
-
- http://www.apache.org/licenses/LICENSE-2.0
-
- Unless required by applicable law or agreed to in writing, software
- distributed under the License is distributed on an "AS IS" BASIS,
- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- See the License for the specific language governing permissions and
- limitations under the License.
-
-*/
-
-pragma solidity ^0.4.24;
-pragma experimental ABIEncoderV2;
-
-contract LibStatus {
-
- // Exchange Status Codes
- enum Status {
- /// 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 ///
- // A valid order remains fillable until it is expired, fully filled, or cancelled.
- // An order's state is unaffected by external factors, like account balances.
- 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
- }
-
- event ExchangeStatus(uint8 indexed statusId, bytes32 indexed orderHash);
-}
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 ae1e50637..de7c4d3af 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -26,7 +26,6 @@ import "../interfaces/IExchangeCore.sol";
contract MExchangeCore is
IExchangeCore
{
-
// Fill event is emitted whenever an order is filled.
event Fill(
address indexed makerAddress,
@@ -56,25 +55,6 @@ contract MExchangeCore is
uint256 makerEpoch
);
- /// @dev Validates context for fillOrder. Succeeds or throws.
- /// @param order to be filled.
- /// @param orderStatus Status of order to be filled.
- /// @param orderHash Hash of order to be filled.
- /// @param takerAddress Address of order taker.
- /// @param orderTakerAssetFilledAmount Amount of order already filled.
- /// @param takerAssetFillAmount Desired amount of order to fill by taker.
- /// @param signature Proof that the orders was created by its maker.
- function assertValidFill(
- LibOrder.Order memory order,
- uint8 orderStatus,
- bytes32 orderHash,
- address takerAddress,
- uint256 orderTakerAssetFilledAmount,
- uint256 takerAssetFillAmount,
- bytes memory signature
- )
- internal;
-
/// @dev Updates state with results of a fill order.
/// @param order that was filled.
/// @param takerAddress Address of taker who filled the order.
@@ -89,29 +69,55 @@ contract MExchangeCore is
)
internal;
- /// @dev Validates context for cancelOrder. Succeeds or throws.
- /// @param order that was cancelled.
- /// @param orderStatus Status of order that was cancelled.
- /// @param orderHash Hash of order that was cancelled.
- function assertValidCancel(
- LibOrder.Order memory order,
- uint8 orderStatus,
- bytes32 orderHash
- )
- internal;
-
/// @dev Updates state with results of cancelling an order.
/// State is only updated if the order is currently fillable.
/// Otherwise, updating state would have no effect.
/// @param order that was cancelled.
- /// @param orderStatus Status of order that was cancelled.
/// @param orderHash Hash of order that was cancelled.
- /// @return stateUpdated Returns true only if state was updated.
function updateCancelledState(
LibOrder.Order memory order,
- uint8 orderStatus,
bytes32 orderHash
)
+ internal;
+
+ /// @dev Validates context for fillOrder. Succeeds or throws.
+ /// @param order to be filled.
+ /// @param orderInfo Status, orderHash, and amount already filled of order.
+ /// @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 signature Proof that the orders was created by its maker.
+ function assertValidFill(
+ LibOrder.Order memory order,
+ LibOrder.OrderInfo memory orderInfo,
+ address takerAddress,
+ uint256 takerAssetFillAmount,
+ uint256 takerAssetFilledAmount,
+ bytes memory signature
+ )
+ internal
+ view;
+
+
+ /// @dev Validates context for cancelOrder. Succeeds or throws.
+ /// @param order to be cancelled.
+ /// @param orderInfo OrderStatus, orderHash, and amount already filled of order.
+ function assertValidCancel(
+ LibOrder.Order memory order,
+ LibOrder.OrderInfo memory orderInfo
+ )
+ internal
+ view;
+
+ /// @dev Calculates amounts filled and fees paid by maker and taker.
+ /// @param order to be filled.
+ /// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
+ /// @return fillResults Amounts filled and fees paid by maker and taker.
+ function calculateFillResults(
+ LibOrder.Order memory order,
+ uint256 takerAssetFilledAmount
+ )
internal
- returns (bool stateUpdated);
+ pure
+ returns (LibFillResults.FillResults memory fillResults);
}
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 7dd608cf2..e52963a26 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
@@ -20,7 +20,6 @@ pragma experimental ABIEncoderV2;
import "../libs/LibOrder.sol";
import "../libs/LibFillResults.sol";
-import "./MExchangeCore.sol";
import "../interfaces/IMatchOrders.sol";
contract MMatchOrders is
@@ -34,12 +33,8 @@ contract MMatchOrders is
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder
)
- internal;
-
- /// @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 assertValidMatchResults(LibFillResults.MatchedFillResults memory matchedFillResults)
- internal;
+ internal
+ pure;
/// @dev Calculates fill amounts for the matched orders.
/// Each order is filled at their respective price point. However, the calculations are
@@ -47,19 +42,16 @@ contract MMatchOrders is
/// The profit made by the leftOrder order goes to the taker (who matched the two orders).
/// @param leftOrder First order to match.
/// @param rightOrder Second order to match.
- /// @param leftOrderStatus Order status of left order.
- /// @param rightOrderStatus Order status of right order.
- /// @param leftOrderFilledAmount Amount of left order already filled.
- /// @param rightOrderFilledAmount Amount of right order already filled.
+ /// @param leftOrderTakerAssetFilledAmount Amount of left order already filled.
+ /// @param rightOrderTakerAssetFilledAmount Amount of right order already filled.
/// @param matchedFillResults Amounts to fill and fees to pay by maker and taker of matched orders.
function calculateMatchedFillResults(
LibOrder.Order memory leftOrder,
LibOrder.Order memory rightOrder,
- uint8 leftOrderStatus,
- uint8 rightOrderStatus,
- uint256 leftOrderFilledAmount,
- uint256 rightOrderFilledAmount
+ uint256 leftOrderTakerAssetFilledAmount,
+ uint256 rightOrderTakerAssetFilledAmount
)
internal
+ pure
returns (LibFillResults.MatchedFillResults memory matchedFillResults);
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
index 50b62e79f..2c403162f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol
@@ -19,7 +19,6 @@
pragma solidity ^0.4.24;
import "../libs/LibOrder.sol";
-import "./MMatchOrders.sol";
import "../libs/LibFillResults.sol";
contract MSettlement {
diff --git a/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol b/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol
index bd01277dd..47ce0dcf3 100644
--- a/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol
+++ b/packages/contracts/src/contracts/current/test/TestLibs/TestLibs.sol
@@ -71,7 +71,7 @@ contract TestLibs is
function getOrderSchemaHash()
public
- view
+ pure
returns (bytes32)
{
return ORDER_SCHEMA_HASH;
@@ -79,7 +79,7 @@ contract TestLibs is
function getDomainSeparatorSchemaHash()
public
- view
+ pure
returns (bytes32)
{
return DOMAIN_SEPARATOR_SCHEMA_HASH;
diff --git a/packages/contracts/src/utils/match_order_tester.ts b/packages/contracts/src/utils/match_order_tester.ts
index 09c0d8083..6170188bc 100644
--- a/packages/contracts/src/utils/match_order_tester.ts
+++ b/packages/contracts/src/utils/match_order_tester.ts
@@ -26,7 +26,6 @@ import {
ContractName,
ERC20BalancesByOwner,
ERC721TokenIdsByOwner,
- ExchangeStatus,
TransferAmountsByMatchOrders as TransferAmounts,
} from '../utils/types';
import { provider, web3Wrapper } from '../utils/web3_wrapper';
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index 6340c4a51..360e1fdbc 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -69,22 +69,14 @@ export interface Token {
swarmHash: string;
}
-export enum ExchangeStatus {
+export enum OrderStatus {
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,
+ INVALID_MAKER_ASSET_AMOUNT,
+ INVALID_TAKER_ASSET_AMOUNT,
+ FILLABLE,
+ EXPIRED,
+ FULLY_FILLED,
+ CANCELLED,
}
export enum ContractName {
diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts
index 87f89c513..e8274acb1 100644
--- a/packages/contracts/test/asset_proxy/authorizable.ts
+++ b/packages/contracts/test/asset_proxy/authorizable.ts
@@ -19,6 +19,7 @@ describe('Authorizable', () => {
let notOwner: string;
let address: string;
let authorizable: MixinAuthorizableContract;
+
before(async () => {
await blockchainLifecycle.startAsync();
});
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 8320e97d6..91ead93f0 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -15,7 +15,6 @@ import { ERC721ProxyContract } from '../../src/contract_wrappers/generated/e_r_c
import {
CancelContractEventArgs,
ExchangeContract,
- ExchangeStatusContractEventArgs,
FillContractEventArgs,
} from '../../src/contract_wrappers/generated/exchange';
import { artifacts } from '../../src/utils/artifacts';
@@ -25,7 +24,8 @@ import { ERC20Wrapper } from '../../src/utils/erc20_wrapper';
import { ERC721Wrapper } from '../../src/utils/erc721_wrapper';
import { ExchangeWrapper } from '../../src/utils/exchange_wrapper';
import { OrderFactory } from '../../src/utils/order_factory';
-import { ContractName, ERC20BalancesByOwner, ExchangeStatus } from '../../src/utils/types';
+import { orderUtils } from '../../src/utils/order_utils';
+import { ContractName, ERC20BalancesByOwner, OrderStatus } from '../../src/utils/types';
import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
chaiSetup.configure();
@@ -131,39 +131,6 @@ describe('Exchange core', () => {
erc20Balances = await erc20Wrapper.getBalancesAsync();
signedOrder = orderFactory.newSignedOrder();
});
-
- it('should create an unfillable order', async () => {
- signedOrder = orderFactory.newSignedOrder({
- makerAssetAmount: new BigNumber(1001),
- takerAssetAmount: new BigNumber(3),
- });
-
- const takerAssetFilledAmountBefore = await exchangeWrapper.getTakerAssetFilledAmountAsync(
- orderHashUtils.getOrderHashHex(signedOrder),
- );
- expect(takerAssetFilledAmountBefore).to.be.bignumber.equal(0);
-
- const fillTakerAssetAmount1 = new BigNumber(2);
- await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
- takerAssetFillAmount: fillTakerAssetAmount1,
- });
-
- const takerAssetFilledAmountAfter1 = await exchangeWrapper.getTakerAssetFilledAmountAsync(
- orderHashUtils.getOrderHashHex(signedOrder),
- );
- expect(takerAssetFilledAmountAfter1).to.be.bignumber.equal(fillTakerAssetAmount1);
-
- const fillTakerAssetAmount2 = new BigNumber(1);
- await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
- takerAssetFillAmount: fillTakerAssetAmount2,
- });
-
- const takerAssetFilledAmountAfter2 = await exchangeWrapper.getTakerAssetFilledAmountAsync(
- orderHashUtils.getOrderHashHex(signedOrder),
- );
- expect(takerAssetFilledAmountAfter2).to.be.bignumber.equal(takerAssetFilledAmountAfter1);
- });
-
it('should transfer the correct amounts when makerAssetAmount === takerAssetAmount', async () => {
signedOrder = orderFactory.newSignedOrder({
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
@@ -550,37 +517,21 @@ describe('Exchange core', () => {
);
});
- it('should not change erc20Balances if an order is expired', async () => {
- signedOrder = orderFactory.newSignedOrder({
- expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)),
- });
- await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
-
- const newBalances = await erc20Wrapper.getBalancesAsync();
- expect(newBalances).to.be.deep.equal(erc20Balances);
- });
-
- it('should log an error event if an order is expired', async () => {
+ it('should throw if an order is expired', async () => {
signedOrder = orderFactory.newSignedOrder({
expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)),
});
-
- const res = await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
- expect(res.logs).to.have.length(1);
- const log = res.logs[0] as LogWithDecodedArgs<ExchangeStatusContractEventArgs>;
- const statusCode = log.args.statusId;
- expect(statusCode).to.be.equal(ExchangeStatus.ORDER_EXPIRED);
+ return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(
+ constants.REVERT,
+ );
});
- it('should log an error event if no value is filled', async () => {
- signedOrder = orderFactory.newSignedOrder({});
+ it('should throw if no value is filled', async () => {
+ signedOrder = orderFactory.newSignedOrder();
await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
-
- const res = await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
- expect(res.logs).to.have.length(1);
- const log = res.logs[0] as LogWithDecodedArgs<ExchangeStatusContractEventArgs>;
- const statusCode = log.args.statusId;
- expect(statusCode).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(
+ constants.REVERT,
+ );
});
});
@@ -618,12 +569,11 @@ describe('Exchange core', () => {
it('should be able to cancel a full order', async () => {
await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress);
- await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
- takerAssetFillAmount: signedOrder.takerAssetAmount.div(2),
- });
-
- const newBalances = await erc20Wrapper.getBalancesAsync();
- expect(newBalances).to.be.deep.equal(erc20Balances);
+ return expect(
+ exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
+ takerAssetFillAmount: signedOrder.takerAssetAmount.div(2),
+ }),
+ ).to.be.rejectedWith(constants.REVERT);
});
it('should log 1 event with correct arguments', async () => {
@@ -641,26 +591,39 @@ describe('Exchange core', () => {
expect(orderHashUtils.getOrderHashHex(signedOrder)).to.be.equal(logArgs.orderHash);
});
- it('should log an error if already cancelled', async () => {
+ it('should throw if already cancelled', async () => {
await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress);
-
- const res = await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress);
- expect(res.logs).to.have.length(1);
- const log = res.logs[0] as LogWithDecodedArgs<ExchangeStatusContractEventArgs>;
- const statusCode = log.args.statusId;
- expect(statusCode).to.be.equal(ExchangeStatus.ORDER_CANCELLED);
+ return expect(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress)).to.be.rejectedWith(
+ constants.REVERT,
+ );
});
- it('should log error if order is expired', async () => {
+ it('should throw if order is expired', async () => {
signedOrder = orderFactory.newSignedOrder({
expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)),
});
+ return expect(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress)).to.be.rejectedWith(
+ constants.REVERT,
+ );
+ });
+
+ it('should throw if rounding error is greater than 0.1%', async () => {
+ signedOrder = orderFactory.newSignedOrder({
+ makerAssetAmount: new BigNumber(1001),
+ takerAssetAmount: new BigNumber(3),
+ });
- const res = await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress);
- expect(res.logs).to.have.length(1);
- const log = res.logs[0] as LogWithDecodedArgs<ExchangeStatusContractEventArgs>;
- const statusCode = log.args.statusId;
- expect(statusCode).to.be.equal(ExchangeStatus.ORDER_EXPIRED);
+ const fillTakerAssetAmount1 = new BigNumber(2);
+ await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
+ takerAssetFillAmount: fillTakerAssetAmount1,
+ });
+
+ const fillTakerAssetAmount2 = new BigNumber(1);
+ return expect(
+ exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
+ takerAssetFillAmount: fillTakerAssetAmount2,
+ }),
+ ).to.be.rejectedWith(constants.REVERT);
});
});
diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts
index 07295d78a..24ee794bc 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -15,7 +15,6 @@ import { ERC721ProxyContract } from '../../src/contract_wrappers/generated/e_r_c
import {
CancelContractEventArgs,
ExchangeContract,
- ExchangeStatusContractEventArgs,
FillContractEventArgs,
} from '../../src/contract_wrappers/generated/exchange';
import { artifacts } from '../../src/utils/artifacts';
@@ -30,8 +29,8 @@ import {
ContractName,
ERC20BalancesByOwner,
ERC721TokenIdsByOwner,
- ExchangeStatus,
OrderInfo,
+ OrderStatus,
} from '../../src/utils/types';
import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
@@ -186,10 +185,10 @@ describe('matchOrders', () => {
);
// Verify left order was fully filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify right order was fully filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
});
it('should transfer the correct amounts when orders completely fill each other and taker doesnt take a profit', async () => {
@@ -227,10 +226,10 @@ describe('matchOrders', () => {
);
// Verify left order was fully filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify right order was fully filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify taker did not take a profit
expect(takerInitialBalances).to.be.deep.equal(
newERC20BalancesByOwner[takerAddress][defaultERC20MakerAssetAddress],
@@ -265,10 +264,10 @@ describe('matchOrders', () => {
);
// Verify left order was fully filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify right order was partially filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FILLABLE);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE);
});
it('should transfer the correct amounts when right order is completely filled and left order is partially filled', async () => {
@@ -299,10 +298,10 @@ describe('matchOrders', () => {
);
// Verify left order was partially filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FILLABLE);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE);
// Verify right order was fully filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
});
it('should transfer the correct amounts when consecutive calls are used to completely fill the left order', async () => {
@@ -338,10 +337,10 @@ describe('matchOrders', () => {
);
// Verify left order was partially filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FILLABLE);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE);
// Verify right order was fully filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Construct second right order
// Note: This order needs makerAssetAmount=90/takerAssetAmount=[anything <= 45] to fully fill the right order.
// However, we use 100/50 to ensure a partial fill as we want to go down the "left fill"
@@ -368,10 +367,10 @@ describe('matchOrders', () => {
);
// Verify left order was fully filled
const leftOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo2.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(leftOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify second right order was partially filled
const rightOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight2);
- expect(rightOrderInfo2.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FILLABLE);
+ expect(rightOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE);
});
it('should transfer the correct amounts when consecutive calls are used to completely fill the right order', async () => {
@@ -408,10 +407,10 @@ describe('matchOrders', () => {
);
// Verify left order was partially filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify right order was fully filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FILLABLE);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE);
// Create second left order
// Note: This order needs makerAssetAmount=96/takerAssetAmount=48 to fully fill the right order.
// However, we use 100/50 to ensure a partial fill as we want to go down the "right fill"
@@ -441,10 +440,10 @@ describe('matchOrders', () => {
);
// Verify second left order was partially filled
const leftOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft2);
- expect(leftOrderInfo2.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FILLABLE);
+ expect(leftOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FILLABLE);
// Verify right order was fully filled
const rightOrderInfo2: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo2.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(rightOrderInfo2.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
});
it('should transfer the correct amounts if fee recipient is the same across both matched orders', async () => {
@@ -790,10 +789,10 @@ describe('matchOrders', () => {
);
// Verify left order was fully filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify right order was fully filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
});
it('should transfer correct amounts when right order maker asset is an ERC721 token', async () => {
@@ -825,10 +824,10 @@ describe('matchOrders', () => {
);
// Verify left order was fully filled
const leftOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderLeft);
- expect(leftOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(leftOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
// Verify right order was fully filled
const rightOrderInfo: OrderInfo = await exchangeWrapper.getOrderInfoAsync(signedOrderRight);
- expect(rightOrderInfo.orderStatus as ExchangeStatus).to.be.equal(ExchangeStatus.ORDER_FULLY_FILLED);
+ expect(rightOrderInfo.orderStatus as OrderStatus).to.be.equal(OrderStatus.FULLY_FILLED);
});
});
}); // tslint:disable-line:max-file-line-count
diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts
index 9af8b522b..f31053ad3 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -18,7 +18,7 @@ import { ExchangeWrapper } from '../../src/utils/exchange_wrapper';
import { OrderFactory } from '../../src/utils/order_factory';
import { orderUtils } from '../../src/utils/order_utils';
import { TransactionFactory } from '../../src/utils/transaction_factory';
-import { ERC20BalancesByOwner, ExchangeStatus, SignedTransaction } from '../../src/utils/types';
+import { ERC20BalancesByOwner, OrderStatus, SignedTransaction } from '../../src/utils/types';
import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
chaiSetup.configure();
@@ -194,9 +194,9 @@ describe('Exchange transactions', () => {
it('should cancel the order when signed by maker and called by sender', async () => {
await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress);
- const res = await exchangeWrapper.fillOrderAsync(signedOrder, senderAddress);
- const newBalances = await erc20Wrapper.getBalancesAsync();
- expect(newBalances).to.deep.equal(erc20Balances);
+ return expect(exchangeWrapper.fillOrderAsync(signedOrder, senderAddress)).to.be.rejectedWith(
+ constants.REVERT,
+ );
});
});
});
diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts
index a158ba8f3..583ec9f91 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -959,7 +959,7 @@ describe('Exchange wrappers', () => {
const takerAssetCancelAmounts = _.map(signedOrders, signedOrder => signedOrder.takerAssetAmount);
await exchangeWrapper.batchCancelOrdersAsync(signedOrders, makerAddress);
- await exchangeWrapper.batchFillOrdersAsync(signedOrders, takerAddress, {
+ await exchangeWrapper.batchFillOrdersNoThrowAsync(signedOrders, takerAddress, {
takerAssetFillAmounts: takerAssetCancelAmounts,
});
const newBalances = await erc20Wrapper.getBalancesAsync();