Update Exchange statuses, revert instead of emmitting event on fill/cancel failures, and remove redundant logic in matchOrders

1 files changed, 44 insertions, 45 deletions

diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 8320e97d6..16492b8ac 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,6 +24,7 @@ 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 { orderUtils } from '../../src/utils/order_utils';
 import { ContractName, ERC20BalancesByOwner, ExchangeStatus } from '../../src/utils/types';
 import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
 
@@ -132,6 +132,7 @@ describe('Exchange core', () => {
             signedOrder = orderFactory.newSignedOrder();
         });
 
+<<<<<<< HEAD
         it('should create an unfillable order', async () => {
             signedOrder = orderFactory.newSignedOrder({
                 makerAssetAmount: new BigNumber(1001),
@@ -164,6 +165,8 @@ describe('Exchange core', () => {
             expect(takerAssetFilledAmountAfter2).to.be.bignumber.equal(takerAssetFilledAmountAfter1);
         });
 
+=======
+>>>>>>> Update Exchange statuses, revert instead of emmitting event on fill/cancel failures, and remove redundant logic in matchOrders
         it('should transfer the correct amounts when makerAssetAmount === takerAssetAmount', async () => {
             signedOrder = orderFactory.newSignedOrder({
                 makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
@@ -550,37 +553,21 @@ describe('Exchange core', () => {
             );
         });
 
-        it('should not change erc20Balances 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)),
             });
-            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 () => {
-            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 +605,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 +627,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,
+            );
+        });
 
-            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);
+        it('should throw if rounding error is greater than 0.1%', async () => {
+            signedOrder = orderFactory.newSignedOrder({
+                makerAssetAmount: new BigNumber(1001),
+                takerAssetAmount: new BigNumber(3),
+            });
+
+            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);
         });
     });