aboutsummaryrefslogtreecommitdiffstats
path: root/packages
diff options
context:
space:
mode:
authorGreg Hysen <greg.hysen@gmail.com>2018-05-16 07:11:20 +0800
committerGreg Hysen <greg.hysen@gmail.com>2018-05-19 08:01:06 +0800
commit93087324d9b57c87d48d0b7b3cbb28437927ffeb (patch)
tree439814824780234caf29b700545508493dd96bc9 /packages
parent061facdcceddbc68620ae710c1f2fb2c99e4d3f3 (diff)
downloaddexon-sol-tools-93087324d9b57c87d48d0b7b3cbb28437927ffeb.tar
dexon-sol-tools-93087324d9b57c87d48d0b7b3cbb28437927ffeb.tar.gz
dexon-sol-tools-93087324d9b57c87d48d0b7b3cbb28437927ffeb.tar.bz2
dexon-sol-tools-93087324d9b57c87d48d0b7b3cbb28437927ffeb.tar.lz
dexon-sol-tools-93087324d9b57c87d48d0b7b3cbb28437927ffeb.tar.xz
dexon-sol-tools-93087324d9b57c87d48d0b7b3cbb28437927ffeb.tar.zst
dexon-sol-tools-93087324d9b57c87d48d0b7b3cbb28437927ffeb.zip
Throw if the left or right orders do not compute the correct fill results. I like this better than just logging an error and failing silently.
Diffstat (limited to 'packages')
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol36
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol2
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol2
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol6
-rw-r--r--packages/contracts/test/exchange/match_orders.ts18
5 files changed, 27 insertions, 37 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
index 9ea44beab..9f9ad6a52 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
@@ -78,19 +78,14 @@ contract MixinMatchOrders is
validateMatchOrThrow(leftOrder, rightOrder);
// Compute proportional fill amounts
- uint8 matchedFillResultsStatus;
- ( matchedFillResultsStatus,
- matchedFillResults
- ) = calculateMatchedFillResults(
+ matchedFillResults = calculateMatchedFillResults(
leftOrder,
rightOrder,
leftOrderInfo.orderStatus,
rightOrderInfo.orderStatus,
leftOrderInfo.orderFilledAmount,
- rightOrderInfo.orderFilledAmount);
- if (matchedFillResultsStatus != uint8(Status.SUCCESS)) {
- return matchedFillResults;
- }
+ rightOrderInfo.orderFilledAmount
+ );
// Validate fill contexts
validateFillOrRevert(
@@ -231,7 +226,6 @@ contract MixinMatchOrders is
/// @param rightOrderStatus Order status of right order.
/// @param leftOrderFilledAmount Amount of left order already filled.
/// @param rightOrderFilledAmount Amount of right order already filled.
- /// @return status Return status of calculating fill amounts. Returns Status.SUCCESS on success.
/// @param matchedFillResults Amounts to fill and fees to pay by maker and taker of matched orders.
function calculateMatchedFillResults(
Order memory leftOrder,
@@ -242,10 +236,7 @@ contract MixinMatchOrders is
uint256 rightOrderFilledAmount
)
internal
- returns (
- uint8 status,
- MatchedFillResults memory matchedFillResults
- )
+ returns (MatchedFillResults memory matchedFillResults)
{
// We settle orders at the price point defined by the right order (profit goes to the order taker)
// The constraint can be either on the left or on the right.
@@ -286,15 +277,17 @@ contract MixinMatchOrders is
}
// Calculate fill results for left order
+ uint8 status;
(status, matchedFillResults.left) = calculateFillResults(
leftOrder,
leftOrderStatus,
leftOrderFilledAmount,
leftOrderAmountToFill
);
- if (status != uint8(Status.SUCCESS)) {
- return (status, matchedFillResults);
- }
+ require(
+ status == uint8(Status.SUCCESS),
+ FAILED_TO_CALCULATE_FILL_RESULTS_FOR_LEFT_ORDER
+ );
// Calculate fill results for right order
(status, matchedFillResults.right) = calculateFillResults(
@@ -303,9 +296,10 @@ contract MixinMatchOrders is
rightOrderFilledAmount,
rightOrderAmountToFill
);
- if (status != uint8(Status.SUCCESS)) {
- return (status, matchedFillResults);
- }
+ require(
+ status == uint8(Status.SUCCESS),
+ FAILED_TO_CALCULATE_FILL_RESULTS_FOR_RIGHT_ORDER
+ );
// Calculate amount given to taker
matchedFillResults.takerFillAmount = safeSub(
@@ -316,7 +310,7 @@ contract MixinMatchOrders is
// Validate the fill results
validateMatchOrThrow(matchedFillResults);
- // Return status & fill results
- return (status, matchedFillResults);
+ // Return fill results
+ return matchedFillResults;
}
}
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 5ef0a52c5..84f621f64 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
@@ -55,4 +55,6 @@ contract LibExchangeErrors {
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.";
}
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 6bae03ff2..27d65c69f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
@@ -61,7 +61,7 @@ contract LibMath is
require(
!isRoundingError(numerator, denominator, target),
ROUNDING_ERROR_ON_PARTIAL_AMOUNT
- );
+ );
return getPartialAmount(numerator, denominator, target);
}
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 60a266250..d8b9a22c3 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol
@@ -59,7 +59,6 @@ contract MMatchOrders is
/// @param rightOrderStatus Order status of right order.
/// @param leftOrderFilledAmount Amount of left order already filled.
/// @param rightOrderFilledAmount Amount of right order already filled.
- /// @return status Return status of calculating fill amounts. Returns Status.SUCCESS on success.
/// @param matchedFillResults Amounts to fill and fees to pay by maker and taker of matched orders.
function calculateMatchedFillResults(
LibOrder.Order memory leftOrder,
@@ -70,8 +69,5 @@ contract MMatchOrders is
uint256 rightOrderFilledAmount
)
internal
- returns (
- uint8 status,
- LibFillResults.MatchedFillResults memory matchedFillResults
- );
+ returns (LibFillResults.MatchedFillResults memory matchedFillResults);
}
diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts
index 6ba88b5fb..74e4f6760 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -647,7 +647,7 @@ describe('matchOrders', () => {
);
});
- it('Should not transfer any amounts if left order is not fillable', async () => {
+ it('Should throw if left order is not fillable', async () => {
// Create orders to match
const signedOrderLeft = orderFactoryLeft.newSignedOrder({
makerAddress: makerAddressLeft,
@@ -668,13 +668,12 @@ describe('matchOrders', () => {
// Cancel left order
await exchangeWrapper.cancelOrderAsync(signedOrderLeft, signedOrderLeft.makerAddress);
// Match orders
- await exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress);
- // Verify balances did not change
- const newBalances = await erc20Wrapper.getBalancesAsync();
- expect(newBalances).to.be.deep.equal(erc20BalancesByOwner);
+ return expect(
+ exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
+ ).to.be.rejectedWith(constants.REVERT);
});
- it('Should not transfer any amounts if right order is not fillable', async () => {
+ it('Should throw if right order is not fillable', async () => {
// Create orders to match
const signedOrderLeft = orderFactoryLeft.newSignedOrder({
makerAddress: makerAddressLeft,
@@ -695,10 +694,9 @@ describe('matchOrders', () => {
// Cancel right order
await exchangeWrapper.cancelOrderAsync(signedOrderRight, signedOrderRight.makerAddress);
// Match orders
- await exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress);
- // Verify balances did not change
- const newBalances = await erc20Wrapper.getBalancesAsync();
- expect(newBalances).to.be.deep.equal(erc20BalancesByOwner);
+ return expect(
+ exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress),
+ ).to.be.rejectedWith(constants.REVERT);
});
it('should throw if there is not a positive spread', async () => {