From 1cc9d9c0713a56b59717498fcae6dc2720ca4fb0 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Tue, 22 May 2018 12:47:37 -0700 Subject: Replace constant.REVERT test assertions with expectRevertOrAlwaysFailingTransaction --- packages/contracts/src/utils/assertions.ts | 4 +- .../contracts/test/asset_proxy/authorizable.ts | 17 ++--- packages/contracts/test/asset_proxy/proxies.ts | 33 ++++----- packages/contracts/test/exchange/core.ts | 80 ++++++++-------------- packages/contracts/test/exchange/dispatcher.ts | 17 ++--- packages/contracts/test/exchange/match_orders.ts | 21 +++--- packages/contracts/test/exchange/transactions.ts | 13 ++-- packages/contracts/test/exchange/wrapper.ts | 29 ++++---- packages/contracts/test/libraries/lib_bytes.ts | 25 +++---- .../contracts/test/multi_sig_with_time_lock.ts | 14 ++-- packages/contracts/test/token_registry.ts | 51 +++++++------- .../contracts/test/unlimited_allowance_token.ts | 9 +-- 12 files changed, 143 insertions(+), 170 deletions(-) (limited to 'packages/contracts') diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 72c2734d8..e3f31bf89 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -7,7 +7,7 @@ const expect = chai.expect; // throws if the given promise does not reject with one of two expected error // messages. -export const expectRevertOrAlwaysFailingTransaction = (p: Promise) => { +export function expectRevertOrAlwaysFailingTransaction(p: Promise): PromiseLike { return expect(p) .to.be.rejected() .then(e => { @@ -17,4 +17,4 @@ export const expectRevertOrAlwaysFailingTransaction = (p: Promise) => { _.includes(err.message, constants.ALWAYS_FAILING_TRANSACTION), ); }); -}; +} diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index e8274acb1..b09125284 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -6,6 +6,7 @@ import * as Web3 from 'web3'; import { MixinAuthorizableContract } from '../../src/contract_wrappers/generated/mixin_authorizable'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } 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'; @@ -44,9 +45,9 @@ describe('Authorizable', () => { }); describe('addAuthorizedAddress', () => { it('should throw if not called by owner', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( authorizable.addAuthorizedAddress.sendTransactionAsync(notOwner, { from: notOwner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should allow owner to add an authorized address', async () => { await web3Wrapper.awaitTransactionSuccessAsync( @@ -61,9 +62,9 @@ describe('Authorizable', () => { await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expect( + return expectRevertOrAlwaysFailingTransaction( authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -73,11 +74,11 @@ describe('Authorizable', () => { await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expect( + return expectRevertOrAlwaysFailingTransaction( authorizable.removeAuthorizedAddress.sendTransactionAsync(address, { from: notOwner, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should allow owner to remove an authorized address', async () => { @@ -96,11 +97,11 @@ describe('Authorizable', () => { }); it('should throw if owner attempts to remove an address that is not authorized', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( authorizable.removeAuthorizedAddress.sendTransactionAsync(address, { from: owner, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index faab39759..2bd4a36ec 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -10,6 +10,7 @@ import { DummyERC20TokenContract } from '../../src/contract_wrappers/generated/d import { DummyERC721TokenContract } from '../../src/contract_wrappers/generated/dummy_e_r_c721_token'; import { ERC20ProxyContract } from '../../src/contract_wrappers/generated/e_r_c20_proxy'; import { ERC721ProxyContract } from '../../src/contract_wrappers/generated/e_r_c721_proxy'; +import { expectRevertOrAlwaysFailingTransaction } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -144,7 +145,7 @@ describe('Asset Transfer Proxies', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); // Perform a transfer; expect this to fail. - return expect( + return expectRevertOrAlwaysFailingTransaction( erc20Proxy.transferFrom.sendTransactionAsync( encodedProxyMetadata, makerAddress, @@ -152,7 +153,7 @@ describe('Asset Transfer Proxies', () => { transferAmount, { from: notAuthorized }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if requesting address is not authorized', async () => { @@ -160,7 +161,7 @@ describe('Asset Transfer Proxies', () => { const encodedProxyMetadata = assetProxyUtils.encodeERC20ProxyData(zrxToken.address); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(10); - return expect( + return expectRevertOrAlwaysFailingTransaction( erc20Proxy.transferFrom.sendTransactionAsync( encodedProxyMetadata, makerAddress, @@ -170,7 +171,7 @@ describe('Asset Transfer Proxies', () => { from: notAuthorized, }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -217,7 +218,7 @@ describe('Asset Transfer Proxies', () => { const toAddresses = _.times(numTransfers, () => takerAddress); const amounts = _.times(numTransfers, () => amount); - return expect( + return expectRevertOrAlwaysFailingTransaction( erc20Proxy.batchTransferFrom.sendTransactionAsync( assetMetadata, fromAddresses, @@ -225,7 +226,7 @@ describe('Asset Transfer Proxies', () => { amounts, { from: notAuthorized }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -276,7 +277,7 @@ describe('Asset Transfer Proxies', () => { // Perform a transfer from makerAddress to takerAddress const erc20Balances = await erc20Wrapper.getBalancesAsync(); const amount = new BigNumber(0); - return expect( + return expectRevertOrAlwaysFailingTransaction( erc721Proxy.transferFrom.sendTransactionAsync( encodedProxyMetadata, makerAddress, @@ -284,7 +285,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if transferring > 1 amount of a token', async () => { @@ -299,7 +300,7 @@ describe('Asset Transfer Proxies', () => { // Perform a transfer from makerAddress to takerAddress const erc20Balances = await erc20Wrapper.getBalancesAsync(); const amount = new BigNumber(500); - return expect( + return expectRevertOrAlwaysFailingTransaction( erc721Proxy.transferFrom.sendTransactionAsync( encodedProxyMetadata, makerAddress, @@ -307,7 +308,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if allowances are too low', async () => { @@ -325,7 +326,7 @@ describe('Asset Transfer Proxies', () => { ); // Perform a transfer; expect this to fail. const amount = new BigNumber(1); - return expect( + return expectRevertOrAlwaysFailingTransaction( erc20Proxy.transferFrom.sendTransactionAsync( encodedProxyMetadata, makerAddress, @@ -335,7 +336,7 @@ describe('Asset Transfer Proxies', () => { from: notAuthorized, }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if requesting address is not authorized', async () => { @@ -346,7 +347,7 @@ describe('Asset Transfer Proxies', () => { ); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); - return expect( + return expectRevertOrAlwaysFailingTransaction( erc721Proxy.transferFrom.sendTransactionAsync( encodedProxyMetadata, makerAddress, @@ -354,7 +355,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: notAuthorized }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -404,7 +405,7 @@ describe('Asset Transfer Proxies', () => { const toAddresses = _.times(numTransfers, () => takerAddress); const amounts = _.times(numTransfers, () => new BigNumber(1)); - return expect( + return expectRevertOrAlwaysFailingTransaction( erc721Proxy.batchTransferFrom.sendTransactionAsync( assetMetadata, fromAddresses, @@ -412,7 +413,7 @@ describe('Asset Transfer Proxies', () => { amounts, { from: notAuthorized }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 91ead93f0..4f2fb80bd 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -18,6 +18,7 @@ import { FillContractEventArgs, } from '../../src/contract_wrappers/generated/exchange'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -415,9 +416,7 @@ describe('Exchange core', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(200), 18), }); - return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if signature is invalid', async () => { @@ -432,9 +431,7 @@ describe('Exchange core', () => { const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]); const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`; signedOrder.signature = invalidSigHex; - return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if makerAssetAmount is 0', async () => { @@ -442,9 +439,7 @@ describe('Exchange core', () => { makerAssetAmount: new BigNumber(0), }); - return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if takerAssetAmount is 0', async () => { @@ -452,19 +447,17 @@ describe('Exchange core', () => { takerAssetAmount: new BigNumber(0), }); - return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if takerAssetFillAmount is 0', async () => { signedOrder = orderFactory.newSignedOrder(); - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount: new BigNumber(0), }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if maker erc20Balances are too low to fill order', async () => { @@ -472,9 +465,7 @@ describe('Exchange core', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18), }); - return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if taker erc20Balances are too low to fill order', async () => { @@ -482,9 +473,7 @@ describe('Exchange core', () => { takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18), }); - return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if maker allowances are too low to fill order', async () => { @@ -497,9 +486,7 @@ describe('Exchange core', () => { // HACK: `rejectWith` returns a "promise-like" type, but not an actual "Promise", so TSLint // complains, even though we do need to `await` it. So we disable the TSLint error below. // tslint:disable-next-line:await-promise - await expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + await expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if taker allowances are too low to fill order', async () => { @@ -509,12 +496,7 @@ describe('Exchange core', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - // HACK: `rejectWith` returns a "promise-like" type, but not an actual "Promise", so TSLint - // complains, even though we do need to `await` it. So we disable the TSLint error below. - // tslint:disable-next-line:await-promise - await expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + await expectRevertOrAlwaysFailingTransaction(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)); }); it('should throw if an order is expired', async () => { @@ -542,9 +524,7 @@ describe('Exchange core', () => { }); it('should throw if not sent by maker', async () => { - return expect(exchangeWrapper.cancelOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.cancelOrderAsync(signedOrder, takerAddress)); }); it('should throw if makerAssetAmount is 0', async () => { @@ -552,9 +532,7 @@ describe('Exchange core', () => { makerAssetAmount: new BigNumber(0), }); - return expect(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress)); }); it('should throw if takerAssetAmount is 0', async () => { @@ -562,9 +540,7 @@ describe('Exchange core', () => { takerAssetAmount: new BigNumber(0), }); - return expect(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress)); }); it('should be able to cancel a full order', async () => { @@ -632,16 +608,16 @@ describe('Exchange core', () => { const makerEpoch = new BigNumber(1); await exchangeWrapper.cancelOrdersUpToAsync(makerEpoch, makerAddress); const lesserMakerEpoch = new BigNumber(0); - return expect(exchangeWrapper.cancelOrdersUpToAsync(lesserMakerEpoch, makerAddress)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + exchangeWrapper.cancelOrdersUpToAsync(lesserMakerEpoch, makerAddress), ); }); it('should fail to set makerEpoch equal to existing makerEpoch', async () => { const makerEpoch = new BigNumber(1); await exchangeWrapper.cancelOrdersUpToAsync(makerEpoch, makerAddress); - return expect(exchangeWrapper.cancelOrdersUpToAsync(makerEpoch, makerAddress)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + exchangeWrapper.cancelOrdersUpToAsync(makerEpoch, makerAddress), ); }); @@ -749,9 +725,9 @@ describe('Exchange core', () => { expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); // Call Exchange const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw when taker does not own the token with id takerAssetId', async () => { @@ -771,9 +747,9 @@ describe('Exchange core', () => { expect(initialOwnerTakerAsset).to.be.bignumber.not.equal(takerAddress); // Call Exchange const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw when makerAssetAmount is greater than 1', async () => { @@ -793,9 +769,9 @@ describe('Exchange core', () => { expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); // Call Exchange const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw when takerAssetAmount is greater than 1', async () => { @@ -815,9 +791,9 @@ describe('Exchange core', () => { expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); // Call Exchange const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw on partial fill', async () => { @@ -837,9 +813,9 @@ describe('Exchange core', () => { expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); // Call Exchange const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should successfully fill order when makerAsset is ERC721 and takerAsset is ERC20', async () => { diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts index 8bc66e3cf..3da5155ef 100644 --- a/packages/contracts/test/exchange/dispatcher.ts +++ b/packages/contracts/test/exchange/dispatcher.ts @@ -10,6 +10,7 @@ import { ERC20ProxyContract } from '../../src/contract_wrappers/generated/e_r_c2 import { ERC721ProxyContract } from '../../src/contract_wrappers/generated/e_r_c721_proxy'; import { TestAssetProxyDispatcherContract } from '../../src/contract_wrappers/generated/test_asset_proxy_dispatcher'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -177,14 +178,14 @@ describe('AssetProxyDispatcher', () => { const proxyAddress = await assetProxyDispatcher.getAssetProxy.callAsync(AssetProxyId.ERC20); expect(proxyAddress).to.be.equal(erc20Proxy.address); // The following transaction will throw because the currentAddress is no longer constants.NULL_ADDRESS - return expect( + return expectRevertOrAlwaysFailingTransaction( assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, constants.NULL_ADDRESS, { from: owner }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should be able to reset proxy address to NULL', async () => { @@ -218,26 +219,26 @@ describe('AssetProxyDispatcher', () => { it('should throw if requesting address is not owner', async () => { const prevProxyAddress = constants.NULL_ADDRESS; - return expect( + return expectRevertOrAlwaysFailingTransaction( assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, prevProxyAddress, { from: notOwner }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if attempting to register a proxy to the incorrect id', async () => { const prevProxyAddress = constants.NULL_ADDRESS; - return expect( + return expectRevertOrAlwaysFailingTransaction( assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( AssetProxyId.ERC721, erc20Proxy.address, prevProxyAddress, { from: owner }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -307,7 +308,7 @@ describe('AssetProxyDispatcher', () => { // Perform a transfer from makerAddress to takerAddress const erc20Balances = await erc20Wrapper.getBalancesAsync(); const amount = new BigNumber(10); - return expect( + return expectRevertOrAlwaysFailingTransaction( assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( encodedProxyMetadata, makerAddress, @@ -315,7 +316,7 @@ describe('AssetProxyDispatcher', () => { amount, { from: owner }, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); }); diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 24ee794bc..3becb6220 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -18,6 +18,7 @@ import { FillContractEventArgs, } from '../../src/contract_wrappers/generated/exchange'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -639,9 +640,9 @@ describe('matchOrders', () => { // Cancel left order await exchangeWrapper.cancelOrderAsync(signedOrderLeft, signedOrderLeft.makerAddress); // Match orders - return expect( + return exexpectRevertOrAlwaysFailingTransactionpect( exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('Should throw if right order is not fillable', async () => { @@ -665,9 +666,9 @@ describe('matchOrders', () => { // Cancel right order await exchangeWrapper.cancelOrderAsync(signedOrderRight, signedOrderRight.makerAddress); // Match orders - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if there is not a positive spread', async () => { @@ -689,7 +690,7 @@ describe('matchOrders', () => { feeRecipientAddress: feeRecipientAddressRight, }); // Match orders - return expect( + return expectRevertOrAlwaysFailingTransaction( matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -697,7 +698,7 @@ describe('matchOrders', () => { erc20BalancesByOwner, erc721TokenIdsByOwner, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if the left maker asset is not equal to the right taker asset ', async () => { @@ -719,7 +720,7 @@ describe('matchOrders', () => { feeRecipientAddress: feeRecipientAddressRight, }); // Match orders - return expect( + return expectRevertOrAlwaysFailingTransaction( matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -727,7 +728,7 @@ describe('matchOrders', () => { erc20BalancesByOwner, erc721TokenIdsByOwner, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if the right maker asset is not equal to the left taker asset', async () => { @@ -749,7 +750,7 @@ describe('matchOrders', () => { feeRecipientAddress: feeRecipientAddressRight, }); // Match orders - return expect( + return expectRevertOrAlwaysFailingTransaction( matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -757,7 +758,7 @@ describe('matchOrders', () => { erc20BalancesByOwner, erc721TokenIdsByOwner, ), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should transfer correct amounts when left order maker asset is an ERC721 token', async () => { diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts index f31053ad3..8cd800ee2 100644 --- a/packages/contracts/test/exchange/transactions.ts +++ b/packages/contracts/test/exchange/transactions.ts @@ -11,6 +11,7 @@ import { ERC20ProxyContract } from '../../src/contract_wrappers/generated/e_r_c2 import { ExchangeContract } from '../../src/contract_wrappers/generated/exchange'; import { WhitelistContract } from '../../src/contract_wrappers/generated/whitelist'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -126,8 +127,8 @@ describe('Exchange transactions', () => { }); it('should throw if not called by specified sender', async () => { - return expect(exchangeWrapper.executeTransactionAsync(signedTx, takerAddress)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + exchangeWrapper.executeTransactionAsync(signedTx, takerAddress), ); }); @@ -168,8 +169,8 @@ 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 expect(exchangeWrapper.executeTransactionAsync(signedTx, senderAddress)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + exchangeWrapper.executeTransactionAsync(signedTx, senderAddress), ); }); @@ -187,8 +188,8 @@ describe('Exchange transactions', () => { }); it('should throw if not called by specified sender', async () => { - return expect(exchangeWrapper.executeTransactionAsync(signedTx, makerAddress)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + exchangeWrapper.executeTransactionAsync(signedTx, makerAddress), ); }); diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index 583ec9f91..18d69a647 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -15,6 +15,7 @@ import { ERC721ProxyContract } from '../../src/contract_wrappers/generated/e_r_c import { ExchangeContract } from '../../src/contract_wrappers/generated/exchange'; import { TokenRegistryContract } from '../../src/contract_wrappers/generated/token_registry'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } from '../../src/utils/assertions'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { ERC20Wrapper } from '../../src/utils/erc20_wrapper'; @@ -172,8 +173,8 @@ describe('Exchange wrappers', () => { expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)), }); - return expect(exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), ); }); @@ -184,8 +185,8 @@ describe('Exchange wrappers', () => { takerAssetFillAmount: signedOrder.takerAssetAmount.div(2), }); - return expect(exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), ); }); }); @@ -485,11 +486,11 @@ describe('Exchange wrappers', () => { await exchangeWrapper.fillOrKillOrderAsync(signedOrders[0], takerAddress); - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.batchFillOrKillOrdersAsync(signedOrders, takerAddress, { takerAssetFillAmounts, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -679,11 +680,11 @@ describe('Exchange wrappers', () => { orderFactory.newSignedOrder(), ]; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.marketSellOrdersAsync(signedOrders, takerAddress, { takerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -768,11 +769,11 @@ describe('Exchange wrappers', () => { orderFactory.newSignedOrder(), ]; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, { takerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -857,11 +858,11 @@ describe('Exchange wrappers', () => { orderFactory.newSignedOrder(), ]; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.marketBuyOrdersAsync(signedOrders, takerAddress, { makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); @@ -946,11 +947,11 @@ describe('Exchange wrappers', () => { orderFactory.newSignedOrder(), ]; - return expect( + return expectRevertOrAlwaysFailingTransaction( exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, { makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 26cfa8291..e639e6db1 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -10,6 +10,7 @@ import * as Web3 from 'web3'; import { TestLibBytesContract } from '../../src/contract_wrappers/generated/test_lib_bytes'; import { artifacts } from '../../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } 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'; @@ -162,17 +163,13 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold an address)', async () => { const shortByteArray = '0xabcdef'; const offset = new BigNumber(0); - return expect(libBytes.publicReadAddress.callAsync(shortByteArray, offset)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(libBytes.publicReadAddress.callAsync(shortByteArray, offset)); }); it('should fail if the length between the offset and end of the byte array is too short to hold an address)', async () => { const byteArray = ethUtil.addHexPrefix(testAddress); const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); - return expect(libBytes.publicReadAddress.callAsync(byteArray, badOffset)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(libBytes.publicReadAddress.callAsync(byteArray, badOffset)); }); }); @@ -206,16 +203,14 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold a bytes32)', async () => { const offset = new BigNumber(0); - return expect(libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset), ); }); it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32)', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); - return expect(libBytes.publicReadBytes32.callAsync(testBytes32, badOffset)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(libBytes.publicReadBytes32.callAsync(testBytes32, badOffset)); }); }); @@ -253,8 +248,8 @@ describe('LibBytes', () => { it('should fail if the byte array is too short to hold a uint256)', async () => { const offset = new BigNumber(0); - return expect(libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset)).to.be.rejectedWith( - constants.REVERT, + return expectRevertOrAlwaysFailingTransaction( + libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset), ); }); @@ -263,9 +258,7 @@ describe('LibBytes', () => { const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256); const byteArray = ethUtil.bufferToHex(testUint256AsBuffer); const badOffset = new BigNumber(testUint256AsBuffer.byteLength); - return expect(libBytes.publicReadUint256.callAsync(byteArray, badOffset)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(libBytes.publicReadUint256.callAsync(byteArray, badOffset)); }); }); diff --git a/packages/contracts/test/multi_sig_with_time_lock.ts b/packages/contracts/test/multi_sig_with_time_lock.ts index 1302d0fa0..acd6fc8d2 100644 --- a/packages/contracts/test/multi_sig_with_time_lock.ts +++ b/packages/contracts/test/multi_sig_with_time_lock.ts @@ -12,6 +12,7 @@ import { SubmissionContractEventArgs, } from '../src/contract_wrappers/generated/multi_sig_wallet_with_time_lock'; import { artifacts } from '../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } from '../src/utils/assertions'; import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; import { MultiSigWrapper } from '../src/utils/multi_sig_wrapper'; @@ -69,9 +70,9 @@ describe('MultiSigWalletWithTimeLock', () => { }); it('should throw when not called by wallet', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( multiSig.changeTimeLock.sendTransactionAsync(SECONDS_TIME_LOCKED, { from: owners[0] }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw without enough confirmations', async () => { @@ -80,10 +81,9 @@ describe('MultiSigWalletWithTimeLock', () => { const res = await multiSigWrapper.submitTransactionAsync(destination, changeTimeLockData, owners[0]); const log = res.logs[0] as LogWithDecodedArgs; const txId = log.args.transactionId; - - return expect( + return expectRevertOrAlwaysFailingTransaction( multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should set confirmation time with enough confirmations', async () => { @@ -149,9 +149,9 @@ describe('MultiSigWalletWithTimeLock', () => { await multiSigWrapper.confirmTransactionAsync(txId, owners[1]); }); it('should throw if it has enough confirmations but is not past the time lock', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should execute if it has enough confirmations and is past the time lock', async () => { diff --git a/packages/contracts/test/token_registry.ts b/packages/contracts/test/token_registry.ts index 64caac387..4fbb77d43 100644 --- a/packages/contracts/test/token_registry.ts +++ b/packages/contracts/test/token_registry.ts @@ -9,6 +9,7 @@ import * as Web3 from 'web3'; import { TokenRegistryContract } from '../src/contract_wrappers/generated/token_registry'; import { artifacts } from '../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } from '../src/utils/assertions'; import { chaiSetup } from '../src/utils/chai_setup'; import { constants } from '../src/utils/constants'; import { TokenRegWrapper } from '../src/utils/token_registry_wrapper'; @@ -76,7 +77,7 @@ describe('TokenRegistry', () => { describe('addToken', () => { it('should throw when not called by owner', async () => { - return expect(tokenRegWrapper.addTokenAsync(token1, notOwner)).to.be.rejectedWith(constants.REVERT); + return expectRevertOrAlwaysFailingTransaction(tokenRegWrapper.addTokenAsync(token1, notOwner)); }); it('should add token metadata when called by owner', async () => { @@ -88,20 +89,18 @@ describe('TokenRegistry', () => { it('should throw if token already exists', async () => { await tokenRegWrapper.addTokenAsync(token1, owner); - return expect(tokenRegWrapper.addTokenAsync(token1, owner)).to.be.rejectedWith(constants.REVERT); + return expectRevertOrAlwaysFailingTransaction(tokenRegWrapper.addTokenAsync(token1, owner)); }); it('should throw if token address is null', async () => { - return expect(tokenRegWrapper.addTokenAsync(nullToken, owner)).to.be.rejectedWith(constants.REVERT); + return expectRevertOrAlwaysFailingTransaction(tokenRegWrapper.addTokenAsync(nullToken, owner)); }); it('should throw if name already exists', async () => { await tokenRegWrapper.addTokenAsync(token1, owner); const duplicateNameToken = _.assign({}, token2, { name: token1.name }); - return expect(tokenRegWrapper.addTokenAsync(duplicateNameToken, owner)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(tokenRegWrapper.addTokenAsync(duplicateNameToken, owner)); }); it('should throw if symbol already exists', async () => { @@ -110,9 +109,7 @@ describe('TokenRegistry', () => { symbol: token1.symbol, }); - return expect(tokenRegWrapper.addTokenAsync(duplicateSymbolToken, owner)).to.be.rejectedWith( - constants.REVERT, - ); + return expectRevertOrAlwaysFailingTransaction(tokenRegWrapper.addTokenAsync(duplicateSymbolToken, owner)); }); }); @@ -137,9 +134,9 @@ describe('TokenRegistry', () => { describe('setTokenName', () => { it('should throw when not called by owner', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.setTokenName.sendTransactionAsync(token1.address, token2.name, { from: notOwner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should change the token name when called by owner', async () => { @@ -163,25 +160,25 @@ describe('TokenRegistry', () => { it('should throw if the name already exists', async () => { await tokenRegWrapper.addTokenAsync(token2, owner); - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.setTokenName.sendTransactionAsync(token1.address, token2.name, { from: owner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if token does not exist', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.setTokenName.sendTransactionAsync(nullToken.address, token2.name, { from: owner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); describe('setTokenSymbol', () => { it('should throw when not called by owner', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.setTokenSymbol.sendTransactionAsync(token1.address, token2.symbol, { from: notOwner, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should change the token symbol when called by owner', async () => { @@ -203,28 +200,28 @@ describe('TokenRegistry', () => { it('should throw if the symbol already exists', async () => { await tokenRegWrapper.addTokenAsync(token2, owner); - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.setTokenSymbol.sendTransactionAsync(token1.address, token2.symbol, { from: owner, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if token does not exist', async () => { - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.setTokenSymbol.sendTransactionAsync(nullToken.address, token2.symbol, { from: owner, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); describe('removeToken', () => { it('should throw if not called by owner', async () => { const index = new BigNumber(0); - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.removeToken.sendTransactionAsync(token1.address, index, { from: notOwner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should remove token metadata when called by owner', async () => { @@ -241,17 +238,17 @@ describe('TokenRegistry', () => { it('should throw if token does not exist', async () => { const index = new BigNumber(0); - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.removeToken.sendTransactionAsync(nullToken.address, index, { from: owner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if token at given index does not match address', async () => { await tokenRegWrapper.addTokenAsync(token2, owner); const incorrectIndex = new BigNumber(0); - return expect( + return expectRevertOrAlwaysFailingTransaction( tokenReg.removeToken.sendTransactionAsync(token2.address, incorrectIndex, { from: owner }), - ).to.be.rejectedWith(constants.REVERT); + ); }); }); }); diff --git a/packages/contracts/test/unlimited_allowance_token.ts b/packages/contracts/test/unlimited_allowance_token.ts index b2acdebaa..baa616f59 100644 --- a/packages/contracts/test/unlimited_allowance_token.ts +++ b/packages/contracts/test/unlimited_allowance_token.ts @@ -7,6 +7,7 @@ import * as Web3 from 'web3'; import { DummyERC20TokenContract } from '../src/contract_wrappers/generated/dummy_e_r_c20_token'; import { artifacts } from '../src/utils/artifacts'; +import { expectRevertOrAlwaysFailingTransaction } 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'; @@ -93,11 +94,11 @@ describe('UnlimitedAllowanceToken', () => { await token.approve.sendTransactionAsync(spender, amountToTransfer, { from: owner }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expect( + return expectRevertOrAlwaysFailingTransaction( token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should throw if spender has insufficient allowance', async () => { @@ -108,11 +109,11 @@ describe('UnlimitedAllowanceToken', () => { const isSpenderAllowanceInsufficient = spenderAllowance.cmp(amountToTransfer) < 0; expect(isSpenderAllowanceInsufficient).to.be.true(); - return expect( + return expectRevertOrAlwaysFailingTransaction( token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), - ).to.be.rejectedWith(constants.REVERT); + ); }); it('should return true on a 0 value transfer', async () => { -- cgit