From 7f233dcb15f897a4c7de29987e26143c9d192c01 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 26 Jun 2018 17:34:43 -0700 Subject: Add more revert reasons to tests --- .../contracts/test/asset_proxy/authorizable.ts | 17 +++++----- packages/contracts/test/asset_proxy/proxies.ts | 8 ++--- packages/contracts/test/exchange/core.ts | 37 ++++++++++++++++++---- packages/types/src/index.ts | 1 + 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', -- cgit v1.2.3