Update core tests to actually check revert message

3 files changed, 97 insertions, 34 deletions

diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts
index 615e648f3..29489e648 100644
--- a/packages/contracts/src/utils/assertions.ts
+++ b/packages/contracts/src/utils/assertions.ts
@@ -52,6 +52,18 @@ export function expectRevertOrAlwaysFailingTransactionAsync<T>(p: Promise<T>): P
 }
 
 /**
+ * Rejects if the given Promise does not reject with the given revert reason or "always
+ * failing transaction" error.
+ * @param p the Promise which is expected to reject
+ * @param reason a specific revert reason
+ * @returns a new Promise which will reject if the conditions are not met and
+ * otherwise resolve with no value.
+ */
+export function expectRevertReasonOrAlwaysFailingTransactionAsync<T>(p: Promise<T>, reason: string): PromiseLike<void> {
+    return _expectEitherErrorAsync(p, 'always failing transaction', reason);
+}
+
+/**
  * Rejects if the given Promise does not reject with a "revert" or "Contract
  * call failed" error.
  * @param p the Promise which is expected to reject
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index bb8c12088..da8ea588f 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -150,3 +150,35 @@ export interface MatchOrder {
     leftSignature: string;
     rightSignature: string;
 }
+
+export enum ContractLibErrors {
+    OrderUnfillable = 'ORDER_UNFILLABLE',
+    InvalidMaker = 'INVALID_MAKER',
+    InvalidTaker = 'INVALID_TAKER',
+    InvalidSender = 'INVALID_SENDER',
+    InvalidOrderSignature = 'INVALID_ORDER_SIGNATURE',
+    InvalidTakerAmount = 'INVALID_TAKER_AMOUNT',
+    RoundingError = 'ROUNDING_ERROR',
+    InvalidSignature = 'INVALID_SIGNATURE',
+    SignatureIllegal = 'SIGNATURE_ILLEGAL',
+    SignatureUnsupported = 'SIGNATURE_UNSUPPORTED',
+    InvalidNewOrderEpoch = 'INVALID_NEW_ORDER_EPOCH',
+    CompleteFillFailed = 'COMPLETE_FILL_FAILED',
+    NegativeSpreadRequired = 'NEGATIVE_SPREAD_REQUIRED',
+    ReentrancyIllegal = 'REENTRANCY_ILLEGAL',
+    InvalidTxHash = 'INVALID_TX_HASH',
+    InvalidTxSignature = 'INVALID_TX_SIGNATURE',
+    FailedExecution = 'FAILED_EXECUTION',
+    AssetProxyMismatch = 'ASSET_PROXY_MISMATCH',
+    AssetProxyIdMismatch = 'ASSET_PROXY_ID_MISMATCH',
+    LengthGreaterThan0Required = 'LENGTH_GREATER_THAN_0_REQUIRED',
+    Length1Required = 'LENGTH_1_REQUIRED',
+    Length66Required = 'LENGTH_66_REQUIRED',
+    InvalidAmount = 'INVALID_AMOUNT',
+    TransferFailed = 'TRANSFER_FAILED',
+    SenderNotAuthorized = 'SENDER_NOT_AUTHORIZED',
+    TargetNotAuthorized = 'TARGET_NOT_AUTHORIZED',
+    TargetAlreadyAuthorized = 'TARGET_ALREADY_AUTHORIZED',
+    IndexOutOfBounds = 'INDEX_OUT_OF_BOUNDS',
+    AuthorizedAddressMismatch = 'AUTHORIZED_ADDRESS_MISMATCH',
+}
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index ea37a1e99..e0a9d5b92 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -18,14 +18,17 @@ import {
     FillContractEventArgs,
 } from '../../src/generated_contract_wrappers/exchange';
 import { artifacts } from '../../src/utils/artifacts';
-import { expectRevertOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions';
+import {
+    expectRevertOrAlwaysFailingTransactionAsync,
+    expectRevertReasonOrAlwaysFailingTransactionAsync,
+} from '../../src/utils/assertions';
 import { chaiSetup } from '../../src/utils/chai_setup';
 import { constants } from '../../src/utils/constants';
 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 { ERC20BalancesByOwner } from '../../src/utils/types';
+import { ContractLibErrors, ERC20BalancesByOwner } from '../../src/utils/types';
 import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
 
 chaiSetup.configure();
@@ -126,7 +129,7 @@ describe('Exchange core', () => {
     afterEach(async () => {
         await blockchainLifecycle.revertAsync();
     });
-    describe('fillOrder', () => {
+    describe.only('fillOrder', () => {
         beforeEach(async () => {
             erc20Balances = await erc20Wrapper.getBalancesAsync();
             signedOrder = orderFactory.newSignedOrder();
@@ -415,8 +418,9 @@ describe('Exchange core', () => {
                 makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
                 takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18),
             });
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.InvalidTaker,
             );
         });
 
@@ -432,8 +436,9 @@ describe('Exchange core', () => {
             const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]);
             const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`;
             signedOrder.signature = invalidSigHex;
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.InvalidOrderSignature,
             );
         });
 
@@ -442,8 +447,9 @@ describe('Exchange core', () => {
                 makerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
@@ -452,18 +458,20 @@ describe('Exchange core', () => {
                 takerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
         it('should throw if takerAssetFillAmount is 0', async () => {
             signedOrder = orderFactory.newSignedOrder();
 
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
                     takerAssetFillAmount: new BigNumber(0),
                 }),
+                ContractLibErrors.InvalidTakerAmount,
             );
         });
 
@@ -472,8 +480,9 @@ describe('Exchange core', () => {
                 makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.TransferFailed,
             );
         });
 
@@ -481,8 +490,9 @@ describe('Exchange core', () => {
             signedOrder = orderFactory.newSignedOrder({
                 takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18),
             });
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.TransferFailed,
             );
         });
 
@@ -493,8 +503,9 @@ describe('Exchange core', () => {
                 }),
                 constants.AWAIT_TRANSACTION_MINED_MS,
             );
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.TransferFailed,
             );
         });
 
@@ -505,8 +516,9 @@ describe('Exchange core', () => {
                 }),
                 constants.AWAIT_TRANSACTION_MINED_MS,
             );
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.TransferFailed,
             );
         });
 
@@ -514,16 +526,18 @@ describe('Exchange core', () => {
             signedOrder = orderFactory.newSignedOrder({
                 expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)),
             });
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
         it('should throw if no value is filled', async () => {
             signedOrder = orderFactory.newSignedOrder();
             await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
     });
@@ -545,8 +559,9 @@ describe('Exchange core', () => {
                 makerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
@@ -555,17 +570,19 @@ describe('Exchange core', () => {
                 takerAssetAmount: new BigNumber(0),
             });
 
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
         it('should be able to cancel a full order', async () => {
             await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
                     takerAssetFillAmount: signedOrder.takerAssetAmount.div(2),
                 }),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
@@ -585,8 +602,9 @@ describe('Exchange core', () => {
 
         it('should throw if already cancelled', async () => {
             await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
@@ -594,8 +612,9 @@ describe('Exchange core', () => {
             signedOrder = orderFactory.newSignedOrder({
                 expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)),
             });
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
@@ -611,10 +630,11 @@ describe('Exchange core', () => {
             });
 
             const fillTakerAssetAmount2 = new BigNumber(1);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, {
                     takerAssetFillAmount: fillTakerAssetAmount2,
                 }),
+                ContractLibErrors.RoundingError,
             );
         });
     });
@@ -624,16 +644,18 @@ describe('Exchange core', () => {
             const orderEpoch = new BigNumber(1);
             await exchangeWrapper.cancelOrdersUpToAsync(orderEpoch, makerAddress);
             const lesserOrderEpoch = new BigNumber(0);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.cancelOrdersUpToAsync(lesserOrderEpoch, makerAddress),
+                ContractLibErrors.InvalidNewOrderEpoch,
             );
         });
 
         it('should fail to set orderEpoch equal to existing orderEpoch', async () => {
             const orderEpoch = new BigNumber(1);
             await exchangeWrapper.cancelOrdersUpToAsync(orderEpoch, makerAddress);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.cancelOrdersUpToAsync(orderEpoch, makerAddress),
+                ContractLibErrors.InvalidNewOrderEpoch,
             );
         });
 
@@ -791,8 +813,9 @@ describe('Exchange core', () => {
             expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress);
             // Call Exchange
             const takerAssetFillAmount = signedOrder.takerAssetAmount;
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
@@ -813,30 +836,26 @@ describe('Exchange core', () => {
             expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress);
             // Call Exchange
             const takerAssetFillAmount = signedOrder.takerAssetAmount;
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }),
+                ContractLibErrors.OrderUnfillable,
             );
         });
 
         it('should throw on partial fill', async () => {
             // Construct Exchange parameters
             const makerAssetId = erc721MakerAssetIds[0];
-            const takerAssetId = erc721TakerAssetIds[0];
             signedOrder = orderFactory.newSignedOrder({
                 makerAssetAmount: new BigNumber(1),
-                takerAssetAmount: new BigNumber(0),
+                takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18),
                 makerAssetData: assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerAssetId),
-                takerAssetData: assetProxyUtils.encodeERC721AssetData(erc721Token.address, takerAssetId),
+                takerAssetData: assetProxyUtils.encodeERC20AssetData(defaultTakerAssetAddress),
             });
-            // Verify pre-conditions
-            const initialOwnerMakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId);
-            expect(initialOwnerMakerAsset).to.be.bignumber.equal(makerAddress);
-            const initialOwnerTakerAsset = await erc721Token.ownerOf.callAsync(takerAssetId);
-            expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress);
             // Call Exchange
-            const takerAssetFillAmount = signedOrder.takerAssetAmount;
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            const takerAssetFillAmount = signedOrder.takerAssetAmount.div(2);
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }),
+                ContractLibErrors.RoundingError,
             );
         });