diff options
author | Fabio Berger <me@fabioberger.com> | 2018-06-27 01:20:19 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-06-27 01:20:19 +0800 |
commit | ec4fb70b805434fca66de432abb9dd989f4ca65f (patch) | |
tree | ce2edd713186dab60b313d50b1df64c8f4bec667 | |
parent | cba92a01b6dee208b497817445b7ae4048e299c0 (diff) | |
parent | 1bc742aed1e7f10b79b5ef23ebb57b6c93d64e3c (diff) | |
download | dexon-sol-tools-ec4fb70b805434fca66de432abb9dd989f4ca65f.tar dexon-sol-tools-ec4fb70b805434fca66de432abb9dd989f4ca65f.tar.gz dexon-sol-tools-ec4fb70b805434fca66de432abb9dd989f4ca65f.tar.bz2 dexon-sol-tools-ec4fb70b805434fca66de432abb9dd989f4ca65f.tar.lz dexon-sol-tools-ec4fb70b805434fca66de432abb9dd989f4ca65f.tar.xz dexon-sol-tools-ec4fb70b805434fca66de432abb9dd989f4ca65f.tar.zst dexon-sol-tools-ec4fb70b805434fca66de432abb9dd989f4ca65f.zip |
Merge pull request #760 from 0xProject/refactor/check-revert-reasons
Check Revert Reasons in Contract Tests
-rw-r--r-- | package.json | 1 | ||||
-rw-r--r-- | packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol | 10 | ||||
-rw-r--r-- | packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol | 5 | ||||
-rw-r--r-- | packages/contracts/src/utils/assertions.ts | 16 | ||||
-rw-r--r-- | packages/contracts/src/utils/constants.ts | 13 | ||||
-rw-r--r-- | packages/contracts/test/asset_proxy/authorizable.ts | 18 | ||||
-rw-r--r-- | packages/contracts/test/asset_proxy/proxies.ts | 34 | ||||
-rw-r--r-- | packages/contracts/test/exchange/core.ts | 88 | ||||
-rw-r--r-- | packages/contracts/test/exchange/dispatcher.ts | 16 | ||||
-rw-r--r-- | packages/contracts/test/exchange/match_orders.ts | 23 | ||||
-rw-r--r-- | packages/contracts/test/exchange/signature_validator.ts | 10 | ||||
-rw-r--r-- | packages/contracts/test/exchange/transactions.ts | 38 | ||||
-rw-r--r-- | packages/contracts/test/exchange/wrapper.ts | 27 | ||||
-rw-r--r-- | packages/contracts/test/libraries/lib_bytes.ts | 45 | ||||
-rw-r--r-- | packages/contracts/test/unlimited_allowance_token.ts | 7 | ||||
-rw-r--r-- | packages/types/src/index.ts | 44 |
16 files changed, 255 insertions, 140 deletions
diff --git a/package.json b/package.json index 11444d16d..3436f152b 100644 --- a/package.json +++ b/package.json @@ -25,6 +25,7 @@ "build": "wsrun build $PKG --fast-exit -r --stages", "build:monorepo_scripts": "PKG=@0xproject/monorepo-scripts yarn build", "clean": "wsrun clean $PKG --fast-exit -r --parallel", + "remove_node_modules": "lerna clean --yes; rm -rf node_modules", "rebuild": "run-s clean build", "test": "wsrun test $PKG --fast-exit --serial --exclude-missing", "stage_docs": "wsrun docs:stage $PKG --fast-exit --parallel --exclude-missing", 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/contracts/current/tokens/ERC20Token/ERC20Token.sol b/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol index f0bcdafef..b6961a6ec 100644 --- a/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol +++ b/packages/contracts/src/contracts/current/tokens/ERC20Token/ERC20Token.sol @@ -23,8 +23,8 @@ import "./IERC20Token.sol"; contract ERC20Token is IERC20Token { - string constant INSUFFICIENT_BALANCE = "Insufficient balance to complete transfer."; - string constant INSUFFICIENT_ALLOWANCE = "Insufficient allowance to complete transfer."; + string constant INSUFFICIENT_BALANCE = "ERC20_INSUFFICIENT_BALANCE"; + string constant INSUFFICIENT_ALLOWANCE = "ERC20_INSUFFICIENT_ALLOWANCE"; string constant OVERFLOW = "Transfer would result in an overflow."; mapping (address => uint256) balances; @@ -97,4 +97,3 @@ contract ERC20Token is IERC20Token { return allowed[_owner][_spender]; } } - diff --git a/packages/contracts/src/utils/assertions.ts b/packages/contracts/src/utils/assertions.ts index 615e648f3..e702a3200 100644 --- a/packages/contracts/src/utils/assertions.ts +++ b/packages/contracts/src/utils/assertions.ts @@ -1,3 +1,4 @@ +import { RevertReason } from '@0xproject/types'; import * as chai from 'chai'; import * as _ from 'lodash'; @@ -52,6 +53,21 @@ 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: RevertReason, +): 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/constants.ts b/packages/contracts/src/utils/constants.ts index f21b8c7a0..2b2bd2425 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -19,19 +19,6 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', - LIB_BYTES_GREATER_THAN_ZERO_LENGTH_REQUIRED: 'GREATER_THAN_ZERO_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', - LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', - ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', - ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', - EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED: 'LENGTH_GREATER_THAN_0_REQUIRED', - EXCHANGE_SIGNATURE_UNSUPPORTED: 'SIGNATURE_UNSUPPORTED', - EXCHANGE_SIGNATURE_ILLEGAL: 'SIGNATURE_ILLEGAL', - EXCHANGE_LENGTH_0_REQUIRED: 'LENGTH_0_REQUIRED', - EXCHANGE_LENGTH_65_REQUIRED: 'LENGTH_65_REQUIRED', TESTRPC_NETWORK_ID: 50, // Note(albrow): In practice V8 and most other engines limit the minimum // interval for setInterval to 10ms. We still set it to 0 here in order to diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts index c35dc7882..c2295dda6 100644 --- a/packages/contracts/test/asset_proxy/authorizable.ts +++ b/packages/contracts/test/asset_proxy/authorizable.ts @@ -1,10 +1,14 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { MixinAuthorizableContract } from '../../src/generated_contract_wrappers/mixin_authorizable'; 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 { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'; @@ -43,8 +47,9 @@ describe('Authorizable', () => { }); describe('addAuthorizedAddress', () => { it('should throw if not called by owner', async () => { - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.addAuthorizedAddress.sendTransactionAsync(notOwner, { from: notOwner }), + RevertReason.OnlyContractOwner, ); }); it('should allow owner to add an authorized address', async () => { @@ -60,8 +65,9 @@ describe('Authorizable', () => { await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), + RevertReason.TargetAlreadyAuthorized, ); }); }); @@ -72,10 +78,11 @@ describe('Authorizable', () => { await authorizable.addAuthorizedAddress.sendTransactionAsync(address, { from: owner }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddress.sendTransactionAsync(address, { from: notOwner, }), + RevertReason.OnlyContractOwner, ); }); @@ -95,10 +102,11 @@ describe('Authorizable', () => { }); it('should throw if owner attempts to remove an address that is not authorized', async () => { - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( authorizable.removeAuthorizedAddress.sendTransactionAsync(address, { from: owner, }), + RevertReason.TargetNotAuthorized, ); }); }); diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 2313c0b0e..7960e46d2 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -1,5 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; +import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; @@ -15,7 +16,10 @@ 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 } 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'; @@ -158,14 +162,15 @@ describe('Asset Transfer Proxies', () => { constants.AWAIT_TRANSACTION_MINED_MS, ); // Perform a transfer; expect this to fail. - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc20Proxy.transferFrom.sendTransactionAsync( encodedAssetData, makerAddress, takerAddress, transferAmount, - { from: notAuthorized }, + { from: exchangeAddress }, ), + RevertReason.TransferFailed, ); }); @@ -175,10 +180,11 @@ describe('Asset Transfer Proxies', () => { // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(10); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, { from: notAuthorized, }), + RevertReason.SenderNotAuthorized, ); }); }); @@ -226,10 +232,11 @@ describe('Asset Transfer Proxies', () => { const toAddresses = _.times(numTransfers, () => takerAddress); const amounts = _.times(numTransfers, () => amount); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc20Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, { from: notAuthorized, }), + RevertReason.SenderNotAuthorized, ); }); }); @@ -363,7 +370,7 @@ describe('Asset Transfer Proxies', () => { expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(0); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( encodedAssetData, makerAddress, @@ -371,6 +378,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ), + RevertReason.InvalidAmount, ); }); @@ -382,7 +390,7 @@ describe('Asset Transfer Proxies', () => { expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(500); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( encodedAssetData, makerAddress, @@ -390,6 +398,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ), + RevertReason.InvalidAmount, ); }); @@ -405,10 +414,11 @@ describe('Asset Transfer Proxies', () => { ); // Perform a transfer; expect this to fail. const amount = new BigNumber(1); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, { - from: notAuthorized, + from: exchangeAddress, }), + RevertReason.TransferFailed, ); }); @@ -417,7 +427,7 @@ describe('Asset Transfer Proxies', () => { const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( encodedAssetData, makerAddress, @@ -425,6 +435,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: notAuthorized }, ), + RevertReason.SenderNotAuthorized, ); }); }); @@ -475,10 +486,11 @@ describe('Asset Transfer Proxies', () => { const toAddresses = _.times(numTransfers, () => takerAddress); const amounts = _.times(numTransfers, () => new BigNumber(1)); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( erc721Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, { from: notAuthorized, }), + RevertReason.SenderNotAuthorized, ); }); }); diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 5b2d4590f..99d2bc157 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils, orderHashUtils } from '@0xproject/order-utils'; -import { AssetProxyId, SignedOrder } from '@0xproject/types'; +import { AssetProxyId, RevertReason, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; @@ -17,7 +17,10 @@ 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'; @@ -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), + RevertReason.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), + RevertReason.InvalidOrderSignature, ); }); @@ -442,8 +447,9 @@ describe('Exchange core', () => { makerAssetAmount: new BigNumber(0), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.OrderUnfillable, ); }); @@ -452,18 +458,20 @@ describe('Exchange core', () => { takerAssetAmount: new BigNumber(0), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.OrderUnfillable, ); }); it('should throw if takerAssetFillAmount is 0', async () => { signedOrder = orderFactory.newSignedOrder(); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount: new BigNumber(0), }), + RevertReason.InvalidTakerAmount, ); }); @@ -472,8 +480,9 @@ describe('Exchange core', () => { makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.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), + RevertReason.TransferFailed, ); }); @@ -493,8 +503,9 @@ describe('Exchange core', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.TransferFailed, ); }); @@ -505,8 +516,9 @@ describe('Exchange core', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress), + RevertReason.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), + RevertReason.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), + RevertReason.OrderUnfillable, ); }); }); @@ -535,8 +549,9 @@ describe('Exchange core', () => { }); it('should throw if not sent by maker', async () => { - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.cancelOrderAsync(signedOrder, takerAddress), + RevertReason.InvalidMaker, ); }); @@ -545,8 +560,9 @@ describe('Exchange core', () => { makerAssetAmount: new BigNumber(0), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress), + RevertReason.OrderUnfillable, ); }); @@ -555,17 +571,19 @@ describe('Exchange core', () => { takerAssetAmount: new BigNumber(0), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress), + RevertReason.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), }), + RevertReason.OrderUnfillable, ); }); @@ -586,8 +604,9 @@ describe('Exchange core', () => { it('should throw if already cancelled', async () => { await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress), + RevertReason.OrderUnfillable, ); }); @@ -595,8 +614,9 @@ describe('Exchange core', () => { signedOrder = orderFactory.newSignedOrder({ expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress), + RevertReason.OrderUnfillable, ); }); @@ -612,10 +632,11 @@ describe('Exchange core', () => { }); const fillTakerAssetAmount2 = new BigNumber(1); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount: fillTakerAssetAmount2, }), + RevertReason.RoundingError, ); }); }); @@ -625,16 +646,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), + RevertReason.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), + RevertReason.InvalidNewOrderEpoch, ); }); @@ -792,8 +815,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, ); }); @@ -814,30 +838,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 }), + RevertReason.TransferFailed, ); }); 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 }), + RevertReason.RoundingError, ); }); diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts index f96f15f16..8adbee6a3 100644 --- a/packages/contracts/test/exchange/dispatcher.ts +++ b/packages/contracts/test/exchange/dispatcher.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils } from '@0xproject/order-utils'; -import { AssetProxyId } from '@0xproject/types'; +import { AssetProxyId, RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; @@ -9,7 +9,7 @@ import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c2 import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy'; import { TestAssetProxyDispatcherContract } from '../../src/generated_contract_wrappers/test_asset_proxy_dispatcher'; 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'; @@ -175,13 +175,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 expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, constants.NULL_ADDRESS, { from: owner }, ), + RevertReason.AssetProxyMismatch, ); }); @@ -216,25 +217,27 @@ describe('AssetProxyDispatcher', () => { it('should throw if requesting address is not owner', async () => { const prevProxyAddress = constants.NULL_ADDRESS; - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( AssetProxyId.ERC20, erc20Proxy.address, prevProxyAddress, { from: notOwner }, ), + RevertReason.OnlyContractOwner, ); }); it('should throw if attempting to register a proxy to the incorrect id', async () => { const prevProxyAddress = constants.NULL_ADDRESS; - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( AssetProxyId.ERC721, erc20Proxy.address, prevProxyAddress, { from: owner }, ), + RevertReason.AssetProxyIdMismatch, ); }); }); @@ -305,7 +308,7 @@ describe('AssetProxyDispatcher', () => { const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(10); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( encodedAssetData, makerAddress, @@ -313,6 +316,7 @@ describe('AssetProxyDispatcher', () => { amount, { from: owner }, ), + RevertReason.AssetProxyDoesNotExist, ); }); }); diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 18a46187f..bcf691471 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils } from '@0xproject/order-utils'; -import { AssetProxyId } from '@0xproject/types'; +import { AssetProxyId, RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; @@ -12,7 +12,7 @@ import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c2 import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy'; import { ExchangeContract } from '../../src/generated_contract_wrappers/exchange'; 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'; @@ -598,8 +598,9 @@ describe('matchOrders', () => { // Cancel left order await exchangeWrapper.cancelOrderAsync(signedOrderLeft, signedOrderLeft.makerAddress); // Match orders - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress), + RevertReason.OrderUnfillable, ); }); @@ -622,8 +623,9 @@ describe('matchOrders', () => { // Cancel right order await exchangeWrapper.cancelOrderAsync(signedOrderRight, signedOrderRight.makerAddress); // Match orders - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress), + RevertReason.OrderUnfillable, ); }); @@ -644,7 +646,7 @@ describe('matchOrders', () => { feeRecipientAddress: feeRecipientAddressRight, }); // Match orders - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -652,6 +654,7 @@ describe('matchOrders', () => { erc20BalancesByOwner, erc721TokenIdsByOwner, ), + RevertReason.NegativeSpreadRequired, ); }); @@ -672,7 +675,7 @@ describe('matchOrders', () => { feeRecipientAddress: feeRecipientAddressRight, }); // Match orders - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -680,6 +683,11 @@ describe('matchOrders', () => { erc20BalancesByOwner, erc721TokenIdsByOwner, ), + // We are assuming assetData fields of the right order are the + // reverse of the left order, rather than checking equality. This + // saves a bunch of gas, but as a result if the assetData fields are + // off then the failure ends up happening at signature validation + RevertReason.InvalidOrderSignature, ); }); @@ -702,7 +710,7 @@ describe('matchOrders', () => { feeRecipientAddress: feeRecipientAddressRight, }); // Match orders - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( matchOrderTester.matchOrdersAndVerifyBalancesAsync( signedOrderLeft, signedOrderRight, @@ -710,6 +718,7 @@ describe('matchOrders', () => { erc20BalancesByOwner, erc721TokenIdsByOwner, ), + RevertReason.InvalidOrderSignature, ); }); diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts index 8e221e3f1..21cc343b3 100644 --- a/packages/contracts/test/exchange/signature_validator.ts +++ b/packages/contracts/test/exchange/signature_validator.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { addSignedMessagePrefix, assetProxyUtils, MessagePrefixType, orderHashUtils } from '@0xproject/order-utils'; -import { SignatureType, SignedOrder } from '@0xproject/types'; +import { RevertReason, SignatureType, SignedOrder } from '@0xproject/types'; import * as chai from 'chai'; import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); @@ -107,7 +107,7 @@ describe('MixinSignatureValidator', () => { signedOrder.makerAddress, emptySignature, ), - constants.EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED, + RevertReason.LengthGreaterThan0Required, ); }); @@ -121,7 +121,7 @@ describe('MixinSignatureValidator', () => { signedOrder.makerAddress, unsupportedSignatureHex, ), - constants.EXCHANGE_SIGNATURE_UNSUPPORTED, + RevertReason.SignatureUnsupported, ); }); @@ -134,7 +134,7 @@ describe('MixinSignatureValidator', () => { signedOrder.makerAddress, unsupportedSignatureHex, ), - constants.EXCHANGE_SIGNATURE_ILLEGAL, + RevertReason.SignatureIllegal, ); }); @@ -161,7 +161,7 @@ describe('MixinSignatureValidator', () => { signedOrder.makerAddress, signatureHex, ), - constants.EXCHANGE_LENGTH_0_REQUIRED, + RevertReason.Length0Required, ); }); diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts index 43b9f46b2..b0c08e46e 100644 --- a/packages/contracts/test/exchange/transactions.ts +++ b/packages/contracts/test/exchange/transactions.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; -import { AssetProxyId, OrderWithoutExchangeAddress, SignedOrder } from '@0xproject/types'; +import { AssetProxyId, OrderWithoutExchangeAddress, RevertReason, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; @@ -10,7 +10,7 @@ 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'; @@ -59,6 +59,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 +107,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 +125,9 @@ describe('Exchange transactions', () => { }); it('should throw if not called by specified sender', async () => { - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.executeTransactionAsync(signedTx, takerAddress), + RevertReason.FailedExecution, ); }); @@ -168,8 +168,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), + RevertReason.InvalidTxHash, ); }); @@ -187,15 +188,17 @@ describe('Exchange transactions', () => { }); it('should throw if not called by specified sender', async () => { - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.executeTransactionAsync(signedTx, makerAddress), + RevertReason.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), + RevertReason.OrderUnfillable, ); }); }); @@ -238,7 +241,7 @@ describe('Exchange transactions', () => { signedOrder.signature, ); const signedFillTx = takerTransactionFactory.newSignedTransaction(fillData); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapperContract.fillOrder.sendTransactionAsync( orderWithoutExchangeAddress, takerAssetFillAmount, @@ -247,6 +250,7 @@ describe('Exchange transactions', () => { signedFillTx.signature, { from: takerAddress }, ), + RevertReason.FailedExecution, ); }); @@ -357,7 +361,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 +369,7 @@ describe('Exchange transactions', () => { signedOrder.signature, { from: takerAddress }, ), + RevertReason.MakerNotWhitelisted, ); }); @@ -378,7 +383,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 +391,7 @@ describe('Exchange transactions', () => { signedOrder.signature, { from: takerAddress }, ), + RevertReason.TakerNotWhitelisted, ); }); diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index a39a81057..9ee68f94d 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -1,6 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils } from '@0xproject/order-utils'; -import { AssetProxyId, SignedOrder } from '@0xproject/types'; +import { AssetProxyId, RevertReason, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; @@ -12,7 +12,7 @@ import { ERC20ProxyContract } from '../../src/generated_contract_wrappers/e_r_c2 import { ERC721ProxyContract } from '../../src/generated_contract_wrappers/e_r_c721_proxy'; import { ExchangeContract } from '../../src/generated_contract_wrappers/exchange'; 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'; @@ -165,13 +165,14 @@ describe('Exchange wrappers', () => { ); }); - it('should throw if an signedOrder is expired', async () => { + it('should throw if a signedOrder is expired', async () => { const signedOrder = orderFactory.newSignedOrder({ expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), + RevertReason.OrderUnfillable, ); }); @@ -182,8 +183,9 @@ describe('Exchange wrappers', () => { takerAssetFillAmount: signedOrder.takerAssetAmount.div(2), }); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), + RevertReason.CompleteFillFailed, ); }); }); @@ -494,10 +496,11 @@ describe('Exchange wrappers', () => { await exchangeWrapper.fillOrKillOrderAsync(signedOrders[0], takerAddress); - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.batchFillOrKillOrdersAsync(signedOrders, takerAddress, { takerAssetFillAmounts, }), + RevertReason.OrderUnfillable, ); }); }); @@ -687,7 +690,7 @@ describe('Exchange wrappers', () => { expect(newBalances).to.be.deep.equal(erc20Balances); }); - it('should throw when an signedOrder does not use the same takerAssetAddress', async () => { + it('should throw when a signedOrder does not use the same takerAssetAddress', async () => { signedOrders = [ orderFactory.newSignedOrder(), orderFactory.newSignedOrder({ @@ -696,10 +699,13 @@ describe('Exchange wrappers', () => { orderFactory.newSignedOrder(), ]; - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.marketSellOrdersAsync(signedOrders, takerAddress, { takerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), }), + // We simply use the takerAssetData from the first order for all orders. + // If they are not the same, the contract throws when validating the order signature + RevertReason.InvalidOrderSignature, ); }); }); @@ -902,7 +908,7 @@ describe('Exchange wrappers', () => { expect(newBalances).to.be.deep.equal(erc20Balances); }); - it('should throw when an signedOrder does not use the same makerAssetAddress', async () => { + it('should throw when a signedOrder does not use the same makerAssetAddress', async () => { signedOrders = [ orderFactory.newSignedOrder(), orderFactory.newSignedOrder({ @@ -911,10 +917,11 @@ describe('Exchange wrappers', () => { orderFactory.newSignedOrder(), ]; - return expectRevertOrAlwaysFailingTransactionAsync( + return expectRevertReasonOrAlwaysFailingTransactionAsync( exchangeWrapper.marketBuyOrdersAsync(signedOrders, takerAddress, { makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), }), + RevertReason.InvalidOrderSignature, ); }); }); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 56f1dc2bc..3d4058cbc 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -1,5 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; +import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import BN = require('bn.js'); import * as chai from 'chai'; @@ -101,7 +102,7 @@ describe('LibBytes', () => { it('should revert if length is 0', async () => { return expectRevertOrOtherErrorAsync( libBytes.publicPopLastByte.callAsync(constants.NULL_BYTES), - constants.LIB_BYTES_GREATER_THAN_ZERO_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterThanZeroLengthRequired, ); }); it('should pop the last byte from the input and return it', async () => { @@ -117,7 +118,7 @@ describe('LibBytes', () => { it('should revert if length is less than 20', async () => { return expectRevertOrOtherErrorAsync( libBytes.publicPopLast20Bytes.callAsync(byteArrayShorterThan20Bytes), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); }); it('should pop the last 20 bytes from the input and return it', async () => { @@ -185,7 +186,7 @@ describe('LibBytes', () => { it('should revert if dest is shorter than source', async () => { return expectRevertOrOtherErrorAsync( libBytes.publicDeepCopyBytes.callAsync(byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualToSourceBytesLengthRequired, ); }); it('should overwrite dest with source if source and dest have equal length', async () => { @@ -238,7 +239,7 @@ describe('LibBytes', () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicReadAddress.callAsync(shortByteArray, offset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { @@ -246,7 +247,7 @@ describe('LibBytes', () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicReadAddress.callAsync(byteArray, badOffset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); }); }); @@ -282,7 +283,7 @@ describe('LibBytes', () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicWriteAddress.callAsync(byteArrayShorterThan20Bytes, offset, testAddress), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { @@ -290,7 +291,7 @@ describe('LibBytes', () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicWriteAddress.callAsync(byteArray, badOffset, testAddress), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo20LengthRequired, ); }); }); @@ -314,14 +315,14 @@ describe('LibBytes', () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); 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 expectRevertOrOtherErrorAsync( libBytes.publicReadBytes32.callAsync(testBytes32, badOffset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); }); @@ -357,7 +358,7 @@ describe('LibBytes', () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicWriteBytes32.callAsync(byteArrayShorterThan32Bytes, offset, testBytes32), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => { @@ -365,7 +366,7 @@ describe('LibBytes', () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicWriteBytes32.callAsync(byteArray, badOffset, testBytes32), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); }); @@ -393,7 +394,7 @@ describe('LibBytes', () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => { @@ -403,7 +404,7 @@ describe('LibBytes', () => { const badOffset = new BigNumber(testUint256AsBuffer.byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicReadUint256.callAsync(byteArray, badOffset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); }); @@ -443,7 +444,7 @@ describe('LibBytes', () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicWriteUint256.callAsync(byteArrayShorterThan32Bytes, offset, testUint256), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => { @@ -451,7 +452,7 @@ describe('LibBytes', () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicWriteUint256.callAsync(byteArray, badOffset, testUint256), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); }); @@ -462,7 +463,7 @@ describe('LibBytes', () => { const byteArrayLessThan4Bytes = '0x010101'; return expectRevertOrOtherErrorAsync( libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo4LengthRequired, ); }); it('should return the first 4 bytes of a byte array of arbitrary length', async () => { @@ -517,28 +518,28 @@ describe('LibBytes', () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, offset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if we store a nested byte array length, without a nested byte array', async () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicReadBytesWithLength.callAsync(testBytes32, offset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArrayShorterThan32Bytes).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, badOffset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicReadBytesWithLength.callAsync(testBytes32, badOffset), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualTo32LengthRequired, ); }); }); @@ -650,7 +651,7 @@ describe('LibBytes', () => { const emptyByteArray = ethUtil.bufferToHex(new Buffer(1)); return expectRevertOrOtherErrorAsync( libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, offset, longData), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); }); it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array)', async () => { @@ -658,7 +659,7 @@ describe('LibBytes', () => { const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, badOffset, shortData), - constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, + RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired, ); }); }); diff --git a/packages/contracts/test/unlimited_allowance_token.ts b/packages/contracts/test/unlimited_allowance_token.ts index 7132c57bf..0eb82b5fe 100644 --- a/packages/contracts/test/unlimited_allowance_token.ts +++ b/packages/contracts/test/unlimited_allowance_token.ts @@ -1,4 +1,5 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { RevertReason } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; @@ -55,7 +56,7 @@ describe('UnlimitedAllowanceToken', () => { const amountToTransfer = ownerBalance.plus(1); return expectRevertOrOtherErrorAsync( token.transfer.callAsync(spender, amountToTransfer, { from: owner }), - constants.ERC20_INSUFFICIENT_BALANCE, + RevertReason.Erc20InsufficientBalance, ); }); @@ -96,7 +97,7 @@ describe('UnlimitedAllowanceToken', () => { token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), - constants.ERC20_INSUFFICIENT_BALANCE, + RevertReason.Erc20InsufficientBalance, ); }); @@ -112,7 +113,7 @@ describe('UnlimitedAllowanceToken', () => { token.transferFrom.callAsync(owner, spender, amountToTransfer, { from: spender, }), - constants.ERC20_INSUFFICIENT_ALLOWANCE, + RevertReason.Erc20InsufficientAllowance, ); }); diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 88abdc1e7..423292d45 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -168,3 +168,47 @@ export interface ERC721AssetData { tokenId: BigNumber; receiverData: string; } + +export enum RevertReason { + 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', + Length0Required = 'LENGTH_0_REQUIRED', + Length65Required = 'LENGTH_65_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', + OnlyContractOwner = 'ONLY_CONTRACT_OWNER', + MakerNotWhitelisted = 'MAKER_NOT_WHITELISTED', + TakerNotWhitelisted = 'TAKER_NOT_WHITELISTED', + AssetProxyDoesNotExist = 'ASSET_PROXY_DOES_NOT_EXIST', + LibBytesGreaterThanZeroLengthRequired = 'GREATER_THAN_ZERO_LENGTH_REQUIRED', + LibBytesGreaterOrEqualTo4LengthRequired = 'GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED', + LibBytesGreaterOrEqualTo20LengthRequired = 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', + LibBytesGreaterOrEqualTo32LengthRequired = 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', + LibBytesGreaterOrEqualToNestedBytesLengthRequired = 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', + LibBytesGreaterOrEqualToSourceBytesLengthRequired = 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', + Erc20InsufficientBalance = 'ERC20_INSUFFICIENT_BALANCE', + Erc20InsufficientAllowance = 'ERC20_INSUFFICIENT_ALLOWANCE', +} |