diff --git a/contracts/exchange/contracts/test/TestSignatureValidator.sol b/contracts/exchange/contracts/test/TestSignatureValidator.sol index 108974bdaa..4f68928ac4 100644 --- a/contracts/exchange/contracts/test/TestSignatureValidator.sol +++ b/contracts/exchange/contracts/test/TestSignatureValidator.sol @@ -22,12 +22,14 @@ pragma experimental ABIEncoderV2; import "@0x/contracts-exchange-libs/contracts/src/LibEIP712ExchangeDomain.sol"; import "../src/MixinSignatureValidator.sol"; import "../src/MixinTransactions.sol"; +import "../src/MixinExchangeRichErrors.sol"; contract TestSignatureValidator is LibEIP712ExchangeDomain, MixinSignatureValidator, - MixinTransactions + MixinTransactions, + MixinExchangeRichErrors { // solhint-disable no-empty-blocks diff --git a/contracts/exchange/test/core.ts b/contracts/exchange/test/core.ts index c1a8f30318..100acd2996 100644 --- a/contracts/exchange/test/core.ts +++ b/contracts/exchange/test/core.ts @@ -21,7 +21,6 @@ import { chaiSetup, constants, ERC20BalancesByOwner, - expectTransactionFailedAsync, getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync, OrderFactory, @@ -56,7 +55,6 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); // tslint:disable:no-unnecessary-type-assertion describe('Exchange core', () => { - const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'; let chainId: number; let makerAddress: string; let owner: string; @@ -266,7 +264,7 @@ describe('Exchange core', () => { signedOrder.signature = invalidSigHex; const expectedError = new ExchangeRevertErrors.SignatureError( orderHashHex, - ExchangeRevertErrors.SignatureErrorCodes.BadSignature, + ExchangeRevertErrors.SignatureErrorCode.BadSignature, ); const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress); return expect(tx).to.revertWith(expectedError); @@ -276,10 +274,7 @@ describe('Exchange core', () => { signedOrder = await orderFactory.newSignedOrderAsync(); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); await exchangeWrapper.fillOrderAsync(signedOrder, takerAddress); - const expectedError = new ExchangeRevertErrors.OrderStatusError( - orderHashHex, - OrderStatus.FullyFilled, - ); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHashHex, OrderStatus.FullyFilled); const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress); return expect(tx).to.revertWith(expectedError); }); @@ -303,7 +298,7 @@ describe('Exchange core', () => { signedOrder.signature = `0x0${SignatureType.Wallet}`; const expectedError = new ExchangeRevertErrors.SignatureError( orderHashHex, - ExchangeRevertErrors.SignatureErrorCodes.WalletError, + ExchangeRevertErrors.SignatureErrorCode.WalletError, ); const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress); return expect(tx).to.revertWith(expectedError); @@ -320,7 +315,7 @@ describe('Exchange core', () => { const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const expectedError = new ExchangeRevertErrors.SignatureError( orderHashHex, - ExchangeRevertErrors.SignatureErrorCodes.ValidatorError, + ExchangeRevertErrors.SignatureErrorCode.ValidatorError, ); const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress); return expect(tx).to.revertWith(expectedError); @@ -497,10 +492,7 @@ describe('Exchange core', () => { it('should throw if not sent by maker', async () => { const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const expectedError = new ExchangeRevertErrors.InvalidMakerError( - orderHash, - takerAddress, - ); + const expectedError = new ExchangeRevertErrors.InvalidMakerError(orderHash, takerAddress); const tx = exchangeWrapper.cancelOrderAsync(signedOrder, takerAddress); return expect(tx).to.revertWith(expectedError); }); @@ -534,10 +526,7 @@ describe('Exchange core', () => { it('should be able to cancel an order', async () => { await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress); const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const expectedError = new ExchangeRevertErrors.OrderStatusError( - orderHash, - OrderStatus.Cancelled, - ); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHash, OrderStatus.Cancelled); const tx = exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount: signedOrder.takerAssetAmount.div(2), }); @@ -562,10 +551,7 @@ describe('Exchange core', () => { it('should throw if already cancelled', async () => { await exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress); const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const expectedError = new ExchangeRevertErrors.OrderStatusError( - orderHash, - OrderStatus.Cancelled, - ); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHash, OrderStatus.Cancelled); const tx = exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress); return expect(tx).to.revertWith(expectedError); }); @@ -576,10 +562,7 @@ describe('Exchange core', () => { expirationTimeSeconds: new BigNumber(currentTimestamp).minus(10), }); const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const expectedError = new ExchangeRevertErrors.OrderStatusError( - orderHash, - OrderStatus.Expired, - ); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHash, OrderStatus.Expired); const tx = exchangeWrapper.cancelOrderAsync(signedOrder, makerAddress); return expect(tx).to.revertWith(expectedError); }); @@ -610,7 +593,7 @@ describe('Exchange core', () => { const lesserOrderEpoch = new BigNumber(0); const expectedError = new ExchangeRevertErrors.OrderEpochError( makerAddress, - ZERO_ADDRESS, + constants.NULL_ADDRESS, orderEpoch.plus(1), ); const tx = exchangeWrapper.cancelOrdersUpToAsync(lesserOrderEpoch, makerAddress); @@ -622,7 +605,7 @@ describe('Exchange core', () => { await exchangeWrapper.cancelOrdersUpToAsync(orderEpoch, makerAddress); const expectedError = new ExchangeRevertErrors.OrderEpochError( makerAddress, - ZERO_ADDRESS, + constants.NULL_ADDRESS, orderEpoch.plus(1), ); const tx = exchangeWrapper.cancelOrdersUpToAsync(orderEpoch, makerAddress); diff --git a/contracts/exchange/test/dispatcher.ts b/contracts/exchange/test/dispatcher.ts index 9d6d70f065..ab79b18cb5 100644 --- a/contracts/exchange/test/dispatcher.ts +++ b/contracts/exchange/test/dispatcher.ts @@ -9,7 +9,6 @@ import { DummyERC20TokenContract } from '@0x/contracts-erc20'; import { chaiSetup, constants, - expectTransactionFailedAsync, LogDecoder, orderUtils, provider, @@ -137,18 +136,16 @@ describe('AssetProxyDispatcher', () => { txDefaults, ); const expectedError = new ExchangeRevertErrors.AssetProxyExistsError(proxyAddress); - const tx = assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( - newErc20TransferProxy.address, - { from: owner }, - ); + const tx = assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(newErc20TransferProxy.address, { + from: owner, + }); return expect(tx).to.revertWith(expectedError); }); it('should throw if requesting address is not owner', async () => { - const tx = assetProxyDispatcher.registerAssetProxy.sendTransactionAsync( - erc20Proxy.address, - { from: notOwner }, - ); + const tx = assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { + from: notOwner, + }); return expect(tx).to.revertWith(RevertReason.OnlyContractOwner); }); @@ -279,7 +276,7 @@ describe('AssetProxyDispatcher', () => { const expectedError = new ExchangeRevertErrors.AssetProxyDispatchError( orderHash, encodedAssetData, - ExchangeRevertErrors.AssetProxyDispatchErrorCodes.UnknownAssetProxy, + ExchangeRevertErrors.AssetProxyDispatchErrorCode.UnknownAssetProxy, ); const tx = assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( orderHash, diff --git a/contracts/exchange/test/match_orders.ts b/contracts/exchange/test/match_orders.ts index bfffba50cb..70bcaf8f90 100644 --- a/contracts/exchange/test/match_orders.ts +++ b/contracts/exchange/test/match_orders.ts @@ -6,7 +6,6 @@ import { constants, ERC20BalancesByOwner, ERC721TokenIdsByOwner, - expectTransactionFailedAsync, OrderFactory, provider, txDefaults, @@ -588,10 +587,12 @@ describe('matchOrders', () => { await reentrantErc20Token.setReentrantFunction.sendTransactionAsync(functionId), constants.AWAIT_TRANSACTION_MINED_MS, ); - await expectTransactionFailedAsync( - exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress), - RevertReason.ReentrancyIllegal, + const tx = exchangeWrapper.matchOrdersAsync( + signedOrderLeft, + signedOrderRight, + takerAddress, ); + return expect(tx).to.revertWith(RevertReason.ReentrancyIllegal); }); }); }; @@ -1129,10 +1130,7 @@ describe('matchOrders', () => { // Cancel left order await exchangeWrapper.cancelOrderAsync(signedOrderLeft, signedOrderLeft.makerAddress); // Match orders - const expectedError = new ExchangeRevertErrors.OrderStatusError( - orderHashHexLeft, - OrderStatus.Cancelled, - ); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHashHexLeft, OrderStatus.Cancelled); const tx = exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); return expect(tx).to.revertWith(expectedError); }); @@ -1151,10 +1149,7 @@ describe('matchOrders', () => { // Cancel right order await exchangeWrapper.cancelOrderAsync(signedOrderRight, signedOrderRight.makerAddress); // Match orders - const expectedError = new ExchangeRevertErrors.OrderStatusError( - orderHashHexRight, - OrderStatus.Cancelled, - ); + const expectedError = new ExchangeRevertErrors.OrderStatusError(orderHashHexRight, OrderStatus.Cancelled); const tx = exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); return expect(tx).to.revertWith(expectedError); }); @@ -1172,10 +1167,7 @@ describe('matchOrders', () => { const orderHashHexLeft = orderHashUtils.getOrderHashHex(signedOrderLeft); const orderHashHexRight = orderHashUtils.getOrderHashHex(signedOrderRight); // Match orders - const expectedError = new ExchangeRevertErrors.NegativeSpreadError( - orderHashHexLeft, - orderHashHexRight, - ); + const expectedError = new ExchangeRevertErrors.NegativeSpreadError(orderHashHexLeft, orderHashHexRight); const tx = exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); return expect(tx).to.revertWith(expectedError); }); @@ -1202,7 +1194,7 @@ describe('matchOrders', () => { const orderHashHex = orderHashUtils.getOrderHashHex(reconstructedOrderRight); const expectedError = new ExchangeRevertErrors.SignatureError( orderHashHex, - ExchangeRevertErrors.SignatureErrorCodes.BadSignature, + ExchangeRevertErrors.SignatureErrorCode.BadSignature, ); // Match orders const tx = exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); @@ -1227,7 +1219,7 @@ describe('matchOrders', () => { const orderHashHex = orderHashUtils.getOrderHashHex(reconstructedOrderRight); const expectedError = new ExchangeRevertErrors.SignatureError( orderHashHex, - ExchangeRevertErrors.SignatureErrorCodes.BadSignature, + ExchangeRevertErrors.SignatureErrorCode.BadSignature, ); // Match orders const tx = exchangeWrapper.matchOrdersAsync(signedOrderLeft, signedOrderRight, takerAddress); diff --git a/contracts/exchange/test/signature_validator.ts b/contracts/exchange/test/signature_validator.ts index ef820d3e6c..24b7adcb73 100644 --- a/contracts/exchange/test/signature_validator.ts +++ b/contracts/exchange/test/signature_validator.ts @@ -2,7 +2,6 @@ import { addressUtils, chaiSetup, constants, - expectContractCallFailedAsync, LogDecoder, OrderFactory, provider, @@ -10,7 +9,12 @@ import { web3Wrapper, } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; -import { assetDataUtils, orderHashUtils, signatureUtils } from '@0x/order-utils'; +import { + assetDataUtils, + ExchangeRevertErrors, + orderHashUtils, + signatureUtils +} from '@0x/order-utils'; import { RevertReason, SignatureType, SignedOrder } from '@0x/types'; import { BigNumber, providerUtils } from '@0x/utils'; import * as chai from 'chai'; @@ -130,41 +134,47 @@ describe('MixinSignatureValidator', () => { it('should revert when signature is empty', async () => { const emptySignature = '0x'; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailedAsync( - signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signedOrder.makerAddress, - emptySignature, - ), - RevertReason.LengthGreaterThan0Required, + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.InvalidLength, ); + const tx = signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + emptySignature, + ); + return expect(tx).to.revertWith(expectedError); }); it('should revert when signature type is unsupported', async () => { const unsupportedSignatureType = SignatureType.NSignatureTypes; const unsupportedSignatureHex = `0x${Buffer.from([unsupportedSignatureType]).toString('hex')}`; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailedAsync( - signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signedOrder.makerAddress, - unsupportedSignatureHex, - ), - RevertReason.SignatureUnsupported, + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.Unsupported, ); + const tx = signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + unsupportedSignatureHex, + ); + return expect(tx).to.revertWith(expectedError); }); it('should revert when SignatureType=Illegal', async () => { const unsupportedSignatureHex = `0x${Buffer.from([SignatureType.Illegal]).toString('hex')}`; const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailedAsync( - signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signedOrder.makerAddress, - unsupportedSignatureHex, - ), - RevertReason.SignatureIllegal, + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.Illegal, ); + const tx = signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + unsupportedSignatureHex, + ); + return expect(tx).to.revertWith(expectedError); }); it('should return false when SignatureType=Invalid and signature has a length of zero', async () => { @@ -184,14 +194,16 @@ describe('MixinSignatureValidator', () => { const signatureBuffer = Buffer.concat([fillerData, signatureType]); const signatureHex = ethUtil.bufferToHex(signatureBuffer); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - return expectContractCallFailedAsync( - signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - signedOrder.makerAddress, - signatureHex, - ), - RevertReason.Length0Required, + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.InvalidLength, ); + const tx = signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signedOrder.makerAddress, + signatureHex, + ); + return expect(tx).to.revertWith(expectedError); }); it('should return true when SignatureType=EIP712 and signature is valid', async () => { @@ -344,14 +356,16 @@ describe('MixinSignatureValidator', () => { ethUtil.toBuffer(`0x${SignatureType.Wallet}`), ]); const signatureHex = ethUtil.bufferToHex(signature); - await expectContractCallFailedAsync( - signatureValidator.publicIsValidSignature.callAsync( - orderHashHex, - maliciousWallet.address, - signatureHex, - ), - RevertReason.WalletError, + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.WalletError, ); + const tx = signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + maliciousWallet.address, + signatureHex, + ); + return expect(tx).to.revertWith(expectedError); }); it('should return true when SignatureType=Validator, signature is valid and validator is approved', async () => { @@ -390,10 +404,16 @@ describe('MixinSignatureValidator', () => { const signature = Buffer.concat([validatorAddress, signatureType]); const signatureHex = ethUtil.bufferToHex(signature); const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); - await expectContractCallFailedAsync( - signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex), - RevertReason.ValidatorError, + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.ValidatorError, ); + const tx = signatureValidator.publicIsValidSignature.callAsync( + orderHashHex, + signerAddress, + signatureHex, + ); + return expect(tx).to.revertWith(expectedError); }); it('should return false when SignatureType=Validator, signature is valid and validator is not approved', async () => { // Set approval of signature validator to false diff --git a/contracts/exchange/test/transactions.ts b/contracts/exchange/test/transactions.ts index ee8f3945bf..a8f5290fa1 100644 --- a/contracts/exchange/test/transactions.ts +++ b/contracts/exchange/test/transactions.ts @@ -4,7 +4,6 @@ import { chaiSetup, constants, ERC20BalancesByOwner, - expectTransactionFailedAsync, OrderFactory, orderUtils, provider, @@ -17,10 +16,12 @@ import { assetDataUtils, ExchangeRevertErrors, generatePseudoRandomSalt, + orderHashUtils, transactionHashUtils, } from '@0x/order-utils'; import { EIP712DomainWithDefaultSchema, + OrderStatus, OrderWithoutDomain, RevertReason, SignedOrder, @@ -36,7 +37,7 @@ chaiSetup.configure(); const expect = chai.expect; const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper); -describe.only('Exchange transactions', () => { +describe('Exchange transactions', () => { let chainId: number; let senderAddress: string; let owner: string; @@ -153,9 +154,11 @@ describe.only('Exchange transactions', () => { }); it('should throw if not called by specified sender', async () => { + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTx); const expectedError = new ExchangeRevertErrors.TransactionExecutionError( transactionHashHex, + new ExchangeRevertErrors.InvalidSenderError(orderHashHex, takerAddress).encode(), ); const tx = exchangeWrapper.executeTransactionAsync(signedTx, takerAddress); return expect(tx).to.revertWith(expectedError); @@ -198,10 +201,13 @@ describe.only('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 expectTransactionFailedAsync( - exchangeWrapper.executeTransactionAsync(signedTx, senderAddress), - RevertReason.InvalidTxHash, + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTx); + const expectedError = new ExchangeRevertErrors.TransactionError( + transactionHashHex, + ExchangeRevertErrors.TransactionErrorCode.AlreadyExecuted, ); + const tx = exchangeWrapper.executeTransactionAsync(signedTx, senderAddress); + return expect(tx).to.revertWith(expectedError); }); it('should reset the currentContextAddress', async () => { @@ -218,18 +224,25 @@ describe.only('Exchange transactions', () => { }); it('should throw if not called by specified sender', async () => { - return expectTransactionFailedAsync( - exchangeWrapper.executeTransactionAsync(signedTx, makerAddress), - RevertReason.FailedExecution, + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedTx); + const expectedError = new ExchangeRevertErrors.TransactionExecutionError( + transactionHashHex, + new ExchangeRevertErrors.InvalidSenderError(orderHashHex, makerAddress).encode(), ); + const tx = exchangeWrapper.executeTransactionAsync(signedTx, makerAddress); + return expect(tx).to.revertWith(expectedError); }); it('should cancel the order when signed by maker and called by sender', async () => { await exchangeWrapper.executeTransactionAsync(signedTx, senderAddress); - return expectTransactionFailedAsync( - exchangeWrapper.fillOrderAsync(signedOrder, senderAddress), - RevertReason.OrderUnfillable, + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + orderHashHex, + OrderStatus.Cancelled, ); + const tx = exchangeWrapper.fillOrderAsync(signedOrder, senderAddress); + return expect(tx).to.revertWith(expectedError); }); }); @@ -271,17 +284,21 @@ describe.only('Exchange transactions', () => { signedOrder.signature, ); const signedFillTx = takerTransactionFactory.newSignedTransaction(fillData); - return expectTransactionFailedAsync( - exchangeWrapperContract.fillOrder.sendTransactionAsync( - orderWithoutDomain, - takerAssetFillAmount, - signedFillTx.salt, - signedOrder.signature, - signedFillTx.signature, - { from: takerAddress }, - ), - RevertReason.FailedExecution, + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const transactionHashHex = transactionHashUtils.getTransactionHashHex(signedFillTx); + const expectedError = new ExchangeRevertErrors.TransactionExecutionError( + transactionHashHex, + new ExchangeRevertErrors.OrderStatusError(orderHashHex, OrderStatus.Cancelled).encode(), ); + const tx = exchangeWrapperContract.fillOrder.sendTransactionAsync( + orderWithoutDomain, + takerAssetFillAmount, + signedFillTx.salt, + signedOrder.signature, + signedFillTx.signature, + { from: takerAddress }, + ); + return expect(tx).to.revertWith(expectedError); }); it("should not cancel an order if not called from the order's sender", async () => { @@ -391,16 +408,14 @@ describe.only('Exchange transactions', () => { orderWithoutDomain = orderUtils.getOrderWithoutDomain(signedOrder); const takerAssetFillAmount = signedOrder.takerAssetAmount; const salt = generatePseudoRandomSalt(); - return expectTransactionFailedAsync( - whitelist.fillOrderIfWhitelisted.sendTransactionAsync( - orderWithoutDomain, - takerAssetFillAmount, - salt, - signedOrder.signature, - { from: takerAddress }, - ), - RevertReason.MakerNotWhitelisted, + const tx = whitelist.fillOrderIfWhitelisted.sendTransactionAsync( + orderWithoutDomain, + takerAssetFillAmount, + salt, + signedOrder.signature, + { from: takerAddress }, ); + return expect(tx).to.revertWith(RevertReason.MakerNotWhitelisted); }); it('should revert if taker has not been whitelisted', async () => { @@ -413,16 +428,14 @@ describe.only('Exchange transactions', () => { orderWithoutDomain = orderUtils.getOrderWithoutDomain(signedOrder); const takerAssetFillAmount = signedOrder.takerAssetAmount; const salt = generatePseudoRandomSalt(); - return expectTransactionFailedAsync( - whitelist.fillOrderIfWhitelisted.sendTransactionAsync( - orderWithoutDomain, - takerAssetFillAmount, - salt, - signedOrder.signature, - { from: takerAddress }, - ), - RevertReason.TakerNotWhitelisted, + const tx = whitelist.fillOrderIfWhitelisted.sendTransactionAsync( + orderWithoutDomain, + takerAssetFillAmount, + salt, + signedOrder.signature, + { from: takerAddress }, ); + return expect(tx).to.revertWith(RevertReason.TakerNotWhitelisted); }); it('should fill the order if maker and taker have been whitelisted', async () => { diff --git a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts index 28aa65d196..8eb9e02825 100644 --- a/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts +++ b/contracts/exchange/test/utils/fill_order_combinatorial_utils.ts @@ -6,7 +6,6 @@ import { BalanceAmountScenario, chaiSetup, constants, - expectTransactionFailedAsync, ExpirationTimeSecondsScenario, FeeRecipientAddressScenario, FillScenario, @@ -441,7 +440,11 @@ export class FillOrderCombinatorialUtils { lazyStore: BalanceAndProxyAllowanceLazyStore, fillRevertReasonIfExists: RevertReason | RevertError | undefined, ): Promise { +<<<<<<< HEAD if (fillRevertReasonIfExists !== undefined) { +======= + if (!_.isUndefined(fillRevertReasonIfExists)) { +>>>>>>> In `@0x/contracts-exchange`: Update tests for rich reverts const tx = this.exchangeWrapper.fillOrderAsync(signedOrder, this.takerAddress, { takerAssetFillAmount }); return expect(tx).to.revertWith(fillRevertReasonIfExists); } @@ -945,14 +948,14 @@ function validationErrorToRevertError(order: Order, reason: RevertReason): Rever case RevertReason.InvalidTakerAmount: return new ExchangeRevertErrors.FillError( orderHash, - ExchangeRevertErrors.FillErrorCodes.InvalidTakerAmount, + ExchangeRevertErrors.FillErrorCode.InvalidTakerAmount, ); case RevertReason.TakerOverpay: - return new ExchangeRevertErrors.FillError(orderHash, ExchangeRevertErrors.FillErrorCodes.TakerOverpay); + return new ExchangeRevertErrors.FillError(orderHash, ExchangeRevertErrors.FillErrorCode.TakerOverpay); case RevertReason.OrderOverfill: - return new ExchangeRevertErrors.FillError(orderHash, ExchangeRevertErrors.FillErrorCodes.Overfill); + return new ExchangeRevertErrors.FillError(orderHash, ExchangeRevertErrors.FillErrorCode.Overfill); case RevertReason.InvalidFillPrice: - return new ExchangeRevertErrors.FillError(orderHash, ExchangeRevertErrors.FillErrorCodes.InvalidFillPrice); + return new ExchangeRevertErrors.FillError(orderHash, ExchangeRevertErrors.FillErrorCode.InvalidFillPrice); case RevertReason.TransferFailed: return new ExchangeRevertErrors.AssetProxyTransferError(orderHash, undefined, 'TRANSFER_FAILED'); default: diff --git a/contracts/exchange/test/wrapper.ts b/contracts/exchange/test/wrapper.ts index 6287416c0c..17998cf9b0 100644 --- a/contracts/exchange/test/wrapper.ts +++ b/contracts/exchange/test/wrapper.ts @@ -5,18 +5,16 @@ import { chaiSetup, constants, ERC20BalancesByOwner, - expectTransactionFailedAsync, getLatestBlockTimestampAsync, increaseTimeAndMineBlockAsync, OrderFactory, - OrderStatus, provider, txDefaults, web3Wrapper, } from '@0x/contracts-test-utils'; import { BlockchainLifecycle } from '@0x/dev-utils'; -import { assetDataUtils, orderHashUtils } from '@0x/order-utils'; -import { RevertReason, SignedOrder } from '@0x/types'; +import { assetDataUtils, ExchangeRevertErrors, orderHashUtils } from '@0x/order-utils'; +import { OrderStatus, RevertReason, SignedOrder } from '@0x/types'; import { BigNumber, providerUtils } from '@0x/utils'; import { Web3Wrapper } from '@0x/web3-wrapper'; import * as chai from 'chai'; @@ -152,10 +150,8 @@ describe('Exchange wrappers', () => { await reentrantErc20Token.setReentrantFunction.sendTransactionAsync(functionId), constants.AWAIT_TRANSACTION_MINED_MS, ); - await expectTransactionFailedAsync( - exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), - RevertReason.ReentrancyIllegal, - ); + const tx = exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress); + return expect(tx).to.revertWith(RevertReason.ReentrancyIllegal); }); }); }; @@ -210,11 +206,13 @@ describe('Exchange wrappers', () => { const signedOrder = await orderFactory.newSignedOrderAsync({ expirationTimeSeconds: new BigNumber(currentTimestamp).minus(10), }); - - return expectTransactionFailedAsync( - exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), - RevertReason.OrderUnfillable, + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + orderHashHex, + OrderStatus.Expired, ); + const tx = exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress); + return expect(tx).to.revertWith(expectedError); }); it('should throw if entire takerAssetFillAmount not filled', async () => { @@ -224,10 +222,12 @@ describe('Exchange wrappers', () => { takerAssetFillAmount: signedOrder.takerAssetAmount.div(2), }); - return expectTransactionFailedAsync( - exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress), - RevertReason.CompleteFillFailed, + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder); + const expectedError = new ExchangeRevertErrors.IncompleteFillError( + orderHashHex, ); + const tx = exchangeWrapper.fillOrKillOrderAsync(signedOrder, takerAddress); + return expect(tx).to.revertWith(expectedError); }); }); @@ -462,10 +462,8 @@ describe('Exchange wrappers', () => { await reentrantErc20Token.setReentrantFunction.sendTransactionAsync(functionId), constants.AWAIT_TRANSACTION_MINED_MS, ); - await expectTransactionFailedAsync( - exchangeWrapper.batchFillOrdersAsync([signedOrder], takerAddress), - RevertReason.ReentrancyIllegal, - ); + const tx = exchangeWrapper.batchFillOrdersAsync([signedOrder], takerAddress); + return expect(tx).to.revertWith(RevertReason.ReentrancyIllegal); }); }); }; @@ -531,10 +529,8 @@ describe('Exchange wrappers', () => { await reentrantErc20Token.setReentrantFunction.sendTransactionAsync(functionId), constants.AWAIT_TRANSACTION_MINED_MS, ); - await expectTransactionFailedAsync( - exchangeWrapper.batchFillOrKillOrdersAsync([signedOrder], takerAddress), - RevertReason.ReentrancyIllegal, - ); + const tx = exchangeWrapper.batchFillOrKillOrdersAsync([signedOrder], takerAddress); + return expect(tx).to.revertWith(RevertReason.ReentrancyIllegal); }); }); }; @@ -595,13 +591,17 @@ describe('Exchange wrappers', () => { }); await exchangeWrapper.fillOrKillOrderAsync(signedOrders[0], takerAddress); - - return expectTransactionFailedAsync( - exchangeWrapper.batchFillOrKillOrdersAsync(signedOrders, takerAddress, { - takerAssetFillAmounts, - }), - RevertReason.OrderUnfillable, + const orderHashHex = orderHashUtils.getOrderHashHex(signedOrders[0]); + const expectedError = new ExchangeRevertErrors.OrderStatusError( + orderHashHex, + OrderStatus.FullyFilled, ); + const tx = exchangeWrapper.batchFillOrKillOrdersAsync( + signedOrders, + takerAddress, + { takerAssetFillAmounts }, + ); + return expect(tx).to.revertWith(expectedError); }); }); @@ -749,12 +749,12 @@ describe('Exchange wrappers', () => { await reentrantErc20Token.setReentrantFunction.sendTransactionAsync(functionId), constants.AWAIT_TRANSACTION_MINED_MS, ); - await expectTransactionFailedAsync( - exchangeWrapper.marketSellOrdersAsync([signedOrder], takerAddress, { - takerAssetFillAmount: signedOrder.takerAssetAmount, - }), - RevertReason.ReentrancyIllegal, + const tx = exchangeWrapper.marketSellOrdersAsync( + [signedOrder], + takerAddress, + { takerAssetFillAmount: signedOrder.takerAssetAmount}, ); + return expect(tx).to.revertWith(RevertReason.ReentrancyIllegal); }); }); }; @@ -839,15 +839,21 @@ describe('Exchange wrappers', () => { }), await orderFactory.newSignedOrderAsync(), ]; - - return expectTransactionFailedAsync( - 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, + const reconstructedOrder = { + ...signedOrders[1], + takerAssetData: signedOrders[0].takerAssetData, + }; + const orderHashHex = orderHashUtils.getOrderHashHex(reconstructedOrder); + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.BadSignature, ); + const tx = exchangeWrapper.marketSellOrdersAsync( + signedOrders, + takerAddress, + { takerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18) }, + ); + return expect(tx).to.revertWith(expectedError); }); }); @@ -1010,12 +1016,12 @@ describe('Exchange wrappers', () => { await reentrantErc20Token.setReentrantFunction.sendTransactionAsync(functionId), constants.AWAIT_TRANSACTION_MINED_MS, ); - await expectTransactionFailedAsync( - exchangeWrapper.marketBuyOrdersAsync([signedOrder], takerAddress, { - makerAssetFillAmount: signedOrder.makerAssetAmount, - }), - RevertReason.ReentrancyIllegal, + const tx = exchangeWrapper.marketBuyOrdersAsync( + [signedOrder], + takerAddress, + { makerAssetFillAmount: signedOrder.makerAssetAmount }, ); + return expect(tx).to.revertWith(RevertReason.ReentrancyIllegal); }); }); }; @@ -1100,13 +1106,21 @@ describe('Exchange wrappers', () => { }), await orderFactory.newSignedOrderAsync(), ]; - - return expectTransactionFailedAsync( - exchangeWrapper.marketBuyOrdersAsync(signedOrders, takerAddress, { - makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), - }), - RevertReason.InvalidOrderSignature, + const reconstructedOrder = { + ...signedOrders[1], + makerAssetData: signedOrders[0].makerAssetData, + }; + const orderHashHex = orderHashUtils.getOrderHashHex(reconstructedOrder); + const expectedError = new ExchangeRevertErrors.SignatureError( + orderHashHex, + ExchangeRevertErrors.SignatureErrorCode.BadSignature, ); + const tx = exchangeWrapper.marketBuyOrdersAsync( + signedOrders, + takerAddress, + { makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18) }, + ); + return expect(tx).to.revertWith(expectedError); }); });