Change Whitelist error messages to conform to rest and added revert reason checks to transactions tests

3 files changed, 31 insertions, 21 deletions

diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index d35815474..8b52858b1 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -23,13 +23,13 @@ import "../../protocol/Exchange/interfaces/IExchange.sol";
 import "../../protocol/Exchange/libs/LibOrder.sol";
 import "../../utils/Ownable/Ownable.sol";
 
-contract Whitelist is 
+contract Whitelist is
     Ownable
 {
     // Revert reasons
-    string constant MAKER_NOT_WHITELISTED = "Maker address not whitelisted.";
-    string constant TAKER_NOT_WHITELISTED = "Taker address not whitelisted.";
-    string constant INVALID_SENDER = "Sender must equal transaction origin.";
+    string constant MAKER_NOT_WHITELISTED = "MAKER_NOT_WHITELISTED"; // Maker address not whitelisted.
+    string constant TAKER_NOT_WHITELISTED = "TAKER_NOT_WHITELISTED"; // Taker address not whitelisted.
+    string constant INVALID_SENDER = "INVALID_SENDER"; // Sender must equal transaction origin.
 
     // Mapping of address => whitelist status.
     mapping (address => bool) public isWhitelisted;
@@ -77,7 +77,7 @@ contract Whitelist is
         public
     {
         address takerAddress = msg.sender;
-    
+
         // This contract must be the entry point for the transaction.
         require(
             takerAddress == tx.origin,
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index 24183b549..c1eb72099 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -183,4 +183,6 @@ export enum ContractLibErrors {
     IndexOutOfBounds = 'INDEX_OUT_OF_BOUNDS',
     AuthorizedAddressMismatch = 'AUTHORIZED_ADDRESS_MISMATCH',
     OnlyContractOwner = 'ONLY_CONTRACT_OWNER',
+    MakerNotWhitelisted = 'MAKER_NOT_WHITELISTED',
+    TakerNotWhitelisted = 'TAKER_NOT_WHITELISTED',
 }
diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts
index 9a625880c..958a6dce8 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -10,7 +10,9 @@ import { ExchangeContract } from '../../src/generated_contract_wrappers/exchange
 import { ExchangeWrapperContract } from '../../src/generated_contract_wrappers/exchange_wrapper';
 import { WhitelistContract } from '../../src/generated_contract_wrappers/whitelist';
 import { artifacts } from '../../src/utils/artifacts';
-import { expectRevertOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions';
+import {
+    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';
@@ -18,7 +20,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, SignedTransaction } from '../../src/utils/types';
+import { ContractLibErrors, ERC20BalancesByOwner, SignedTransaction } from '../../src/utils/types';
 import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
 
 chaiSetup.configure();
@@ -59,6 +61,12 @@ describe('Exchange transactions', () => {
     after(async () => {
         await blockchainLifecycle.revertAsync();
     });
+    beforeEach(async () => {
+        await blockchainLifecycle.startAsync();
+    });
+    afterEach(async () => {
+        await blockchainLifecycle.revertAsync();
+    });
     before(async () => {
         const accounts = await web3Wrapper.getAvailableAddressesAsync();
         const usedAddresses = ([owner, senderAddress, makerAddress, takerAddress, feeRecipientAddress] = accounts);
@@ -101,13 +109,6 @@ describe('Exchange transactions', () => {
         makerTransactionFactory = new TransactionFactory(makerPrivateKey, exchange.address);
         takerTransactionFactory = new TransactionFactory(takerPrivateKey, exchange.address);
     });
-    beforeEach(async () => {
-        await blockchainLifecycle.startAsync();
-    });
-    afterEach(async () => {
-        await blockchainLifecycle.revertAsync();
-    });
-
     describe('executeTransaction', () => {
         describe('fillOrder', () => {
             let takerAssetFillAmount: BigNumber;
@@ -126,8 +127,9 @@ describe('Exchange transactions', () => {
             });
 
             it('should throw if not called by specified sender', async () => {
-                return expectRevertOrAlwaysFailingTransactionAsync(
+                return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     exchangeWrapper.executeTransactionAsync(signedTx, takerAddress),
+                    ContractLibErrors.FailedExecution,
                 );
             });
 
@@ -168,8 +170,9 @@ describe('Exchange transactions', () => {
 
             it('should throw if the a 0x transaction with the same transactionHash has already been executed', async () => {
                 await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress);
-                return expectRevertOrAlwaysFailingTransactionAsync(
+                return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     exchangeWrapper.executeTransactionAsync(signedTx, senderAddress),
+                    ContractLibErrors.InvalidTxHash,
                 );
             });
 
@@ -187,15 +190,17 @@ describe('Exchange transactions', () => {
             });
 
             it('should throw if not called by specified sender', async () => {
-                return expectRevertOrAlwaysFailingTransactionAsync(
+                return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     exchangeWrapper.executeTransactionAsync(signedTx, makerAddress),
+                    ContractLibErrors.FailedExecution,
                 );
             });
 
             it('should cancel the order when signed by maker and called by sender', async () => {
                 await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress);
-                return expectRevertOrAlwaysFailingTransactionAsync(
+                return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     exchangeWrapper.fillOrderAsync(signedOrder, senderAddress),
+                    ContractLibErrors.OrderUnfillable,
                 );
             });
         });
@@ -238,7 +243,7 @@ describe('Exchange transactions', () => {
                     signedOrder.signature,
                 );
                 const signedFillTx = takerTransactionFactory.newSignedTransaction(fillData);
-                return expectRevertOrAlwaysFailingTransactionAsync(
+                return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     exchangeWrapperContract.fillOrder.sendTransactionAsync(
                         orderWithoutExchangeAddress,
                         takerAssetFillAmount,
@@ -247,6 +252,7 @@ describe('Exchange transactions', () => {
                         signedFillTx.signature,
                         { from: takerAddress },
                     ),
+                    ContractLibErrors.FailedExecution,
                 );
             });
 
@@ -357,7 +363,7 @@ describe('Exchange transactions', () => {
             orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder);
             const takerAssetFillAmount = signedOrder.takerAssetAmount;
             const salt = generatePseudoRandomSalt();
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 whitelist.fillOrderIfWhitelisted.sendTransactionAsync(
                     orderWithoutExchangeAddress,
                     takerAssetFillAmount,
@@ -365,6 +371,7 @@ describe('Exchange transactions', () => {
                     signedOrder.signature,
                     { from: takerAddress },
                 ),
+                ContractLibErrors.MakerNotWhitelisted,
             );
         });
 
@@ -378,7 +385,7 @@ describe('Exchange transactions', () => {
             orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder);
             const takerAssetFillAmount = signedOrder.takerAssetAmount;
             const salt = generatePseudoRandomSalt();
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 whitelist.fillOrderIfWhitelisted.sendTransactionAsync(
                     orderWithoutExchangeAddress,
                     takerAssetFillAmount,
@@ -386,6 +393,7 @@ describe('Exchange transactions', () => {
                     signedOrder.signature,
                     { from: takerAddress },
                 ),
+                ContractLibErrors.TakerNotWhitelisted,
             );
         });