Add more revert reasons to tests

4 files changed, 44 insertions, 19 deletions

diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts
index c2295dda6..8c9d0495d 100644
--- a/packages/contracts/test/asset_proxy/authorizable.ts
+++ b/packages/contracts/test/asset_proxy/authorizable.ts
@@ -5,10 +5,7 @@ import * as chai from 'chai';
 
 import { MixinAuthorizableContract } from '../../src/generated_contract_wrappers/mixin_authorizable';
 import { artifacts } from '../../src/utils/artifacts';
-import {
-    expectRevertOrAlwaysFailingTransactionAsync,
-    expectRevertReasonOrAlwaysFailingTransactionAsync,
-} from '../../src/utils/assertions';
+import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../../src/utils/assertions';
 import { chaiSetup } from '../../src/utils/chai_setup';
 import { constants } from '../../src/utils/constants';
 import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper';
@@ -118,10 +115,11 @@ describe('Authorizable', () => {
                 constants.AWAIT_TRANSACTION_MINED_MS,
             );
             const index = new BigNumber(0);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, {
                     from: notOwner,
                 }),
+                RevertReason.OnlyContractOwner,
             );
         });
         it('should throw if index is >= authorities.length', async () => {
@@ -130,18 +128,20 @@ describe('Authorizable', () => {
                 constants.AWAIT_TRANSACTION_MINED_MS,
             );
             const index = new BigNumber(1);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, {
                     from: owner,
                 }),
+                RevertReason.IndexOutOfBounds,
             );
         });
         it('should throw if owner attempts to remove an address that is not authorized', async () => {
             const index = new BigNumber(0);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address, index, {
                     from: owner,
                 }),
+                RevertReason.TargetNotAuthorized,
             );
         });
         it('should throw if address at index does not match target', async () => {
@@ -156,10 +156,11 @@ describe('Authorizable', () => {
                 constants.AWAIT_TRANSACTION_MINED_MS,
             );
             const address1Index = new BigNumber(0);
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 authorizable.removeAuthorizedAddressAtIndex.sendTransactionAsync(address2, address1Index, {
                     from: owner,
                 }),
+                RevertReason.AuthorizedAddressMismatch,
             );
         });
         it('should allow owner to remove an authorized address', async () => {
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 5a5289508..595839519 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -15,10 +15,7 @@ import { DummyERC721TokenContract } from '../../src/generated_contract_wrappers/
 import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c20_proxy';
 import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy';
 import { artifacts } from '../../src/utils/artifacts';
-import {
-    expectRevertOrAlwaysFailingTransactionAsync,
-    expectRevertReasonOrAlwaysFailingTransactionAsync,
-} 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';
@@ -298,7 +295,7 @@ describe('Asset Transfer Proxies', () => {
                 expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
                 // Perform a transfer from makerAddress to takerAddress
                 const amount = new BigNumber(1);
-                return expectRevertOrAlwaysFailingTransactionAsync(
+                return expectRevertReasonOrAlwaysFailingTransactionAsync(
                     erc721Proxy.transferFrom.sendTransactionAsync(
                         encodedAssetData,
                         makerAddress,
@@ -306,6 +303,7 @@ describe('Asset Transfer Proxies', () => {
                         amount,
                         { from: exchangeAddress },
                     ),
+                    RevertReason.TransferFailed,
                 );
             });
 
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 59756fe9a..36f6fa8d3 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -17,10 +17,7 @@ import {
     FillContractEventArgs,
 } from '../../src/generated_contract_wrappers/exchange';
 import { artifacts } from '../../src/utils/artifacts';
-import {
-    expectRevertOrAlwaysFailingTransactionAsync,
-    expectRevertReasonOrAlwaysFailingTransactionAsync,
-} 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';
@@ -771,8 +768,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 }),
+                RevertReason.TransferFailed,
             );
         });
 
@@ -793,8 +791,9 @@ describe('Exchange core', () => {
             expect(initialOwnerTakerAsset).to.be.bignumber.not.equal(takerAddress);
             // Call Exchange
             const takerAssetFillAmount = signedOrder.takerAssetAmount;
-            return expectRevertOrAlwaysFailingTransactionAsync(
+            return expectRevertReasonOrAlwaysFailingTransactionAsync(
                 exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }),
+                RevertReason.TransferFailed,
             );
         });
 
@@ -861,6 +860,32 @@ describe('Exchange core', () => {
             );
         });
 
+        it('should throw if assetData has a length < 132', async () => {
+            // Construct Exchange parameters
+            const makerAssetId = erc721MakerAssetIds[0];
+            const takerAssetId = erc721TakerAssetIds[0];
+            const makerAssetData = assetProxyUtils
+                .encodeERC721AssetData(erc721Token.address, makerAssetId)
+                .slice(0, -2);
+            signedOrder = orderFactory.newSignedOrder({
+                makerAssetAmount: new BigNumber(1),
+                takerAssetAmount: new BigNumber(1),
+                makerAssetData,
+                takerAssetData: assetProxyUtils.encodeERC721AssetData(erc721Token.address, takerAssetId),
+            });
+            // 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 expectRevertReasonOrAlwaysFailingTransactionAsync(
+                exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }),
+                RevertReason.LengthGreaterThan131Required,
+            );
+        });
+
         it('should successfully fill order when makerAsset is ERC721 and takerAsset is ERC20', async () => {
             // Construct Exchange parameters
             const makerAssetId = erc721MakerAssetIds[0];
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
index 423292d45..4b70c36f9 100644
--- a/packages/types/src/index.ts
+++ b/packages/types/src/index.ts
@@ -190,6 +190,7 @@ export enum RevertReason {
     AssetProxyMismatch = 'ASSET_PROXY_MISMATCH',
     AssetProxyIdMismatch = 'ASSET_PROXY_ID_MISMATCH',
     LengthGreaterThan0Required = 'LENGTH_GREATER_THAN_0_REQUIRED',
+    LengthGreaterThan131Required = 'LENGTH_GREATER_THAN_131_REQUIRED',
     Length0Required = 'LENGTH_0_REQUIRED',
     Length65Required = 'LENGTH_65_REQUIRED',
     InvalidAmount = 'INVALID_AMOUNT',