From ad67a6add54cc31f4b888dbbfe70fed75b743745 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Mon, 25 Jun 2018 13:51:00 +0200 Subject: Check revert reasons in declarative and combinatorial tests --- .../src/utils/core_combinatorial_utils.ts | 15 +-- packages/order-utils/src/order_validation_utils.ts | 113 +++++++++++---------- 2 files changed, 67 insertions(+), 61 deletions(-) diff --git a/packages/contracts/src/utils/core_combinatorial_utils.ts b/packages/contracts/src/utils/core_combinatorial_utils.ts index 7e9d18e46..aea9a65e7 100644 --- a/packages/contracts/src/utils/core_combinatorial_utils.ts +++ b/packages/contracts/src/utils/core_combinatorial_utils.ts @@ -16,7 +16,7 @@ import 'make-promises-safe'; import { ExchangeContract, FillContractEventArgs } from '../generated_contract_wrappers/exchange'; import { artifacts } from '../utils/artifacts'; -import { expectRevertOrAlwaysFailingTransactionAsync } from '../utils/assertions'; +import { expectRevertReasonOrAlwaysFailingTransactionAsync } from '../utils/assertions'; import { AssetWrapper } from '../utils/asset_wrapper'; import { chaiSetup } from '../utils/chai_setup'; import { constants } from '../utils/constants'; @@ -298,7 +298,7 @@ export class CoreCombinatorialUtils { const lazyStore = new BalanceAndProxyAllowanceLazyStore(balanceAndProxyAllowanceFetcher); const exchangeTransferSimulator = new ExchangeTransferSimulator(lazyStore); - let isFillFailureExpected = false; + let fillRevertReasonIfExists; try { await orderValidationUtils.validateFillOrderThrowIfInvalidAsync( exchangeTransferSimulator, @@ -312,7 +312,7 @@ export class CoreCombinatorialUtils { logUtils.log(`Expecting fillOrder to succeed.`); } } catch (err) { - isFillFailureExpected = true; + fillRevertReasonIfExists = err.message; if (isVerbose) { logUtils.log(`Expecting fillOrder to fail with:`); logUtils.log(err); @@ -324,7 +324,7 @@ export class CoreCombinatorialUtils { signedOrder, takerAssetFillAmount, lazyStore, - isFillFailureExpected, + fillRevertReasonIfExists, provider, ); } @@ -332,12 +332,13 @@ export class CoreCombinatorialUtils { signedOrder: SignedOrder, takerAssetFillAmount: BigNumber, lazyStore: BalanceAndProxyAllowanceLazyStore, - isFillFailureExpected: boolean, + fillRevertReasonIfExists: string | undefined, provider: Provider, ): Promise { - if (isFillFailureExpected) { - return expectRevertOrAlwaysFailingTransactionAsync( + if (!_.isUndefined(fillRevertReasonIfExists)) { + return expectRevertReasonOrAlwaysFailingTransactionAsync( this.exchangeWrapper.fillOrderAsync(signedOrder, this.takerAddress, { takerAssetFillAmount }), + fillRevertReasonIfExists, ); } diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index da5b37456..d2449b183 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -1,4 +1,5 @@ -import { ExchangeContractErrs, Order, SignedOrder } from '@0xproject/types'; +import { Order, SignedOrder } from '@0xproject/types'; +import { RevertReasons } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Provider } from 'ethereum-types'; import * as _ from 'lodash'; @@ -54,66 +55,70 @@ export class OrderValidationUtils { senderAddress: string, zrxAssetData: string, ): Promise { - const fillMakerTokenAmount = utils.getPartialAmount( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.makerAssetAmount, - ); - await exchangeTradeEmulator.transferFromAsync( - signedOrder.makerAssetData, - signedOrder.makerAddress, - senderAddress, - fillMakerTokenAmount, - TradeSide.Maker, - TransferType.Trade, - ); - await exchangeTradeEmulator.transferFromAsync( - signedOrder.takerAssetData, - senderAddress, - signedOrder.makerAddress, - fillTakerAssetAmount, - TradeSide.Taker, - TransferType.Trade, - ); - const makerFeeAmount = utils.getPartialAmount( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.makerFee, - ); - await exchangeTradeEmulator.transferFromAsync( - zrxAssetData, - signedOrder.makerAddress, - signedOrder.feeRecipientAddress, - makerFeeAmount, - TradeSide.Maker, - TransferType.Fee, - ); - const takerFeeAmount = utils.getPartialAmount( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.takerFee, - ); - await exchangeTradeEmulator.transferFromAsync( - zrxAssetData, - senderAddress, - signedOrder.feeRecipientAddress, - takerFeeAmount, - TradeSide.Taker, - TransferType.Fee, - ); + try { + const fillMakerTokenAmount = utils.getPartialAmount( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.makerAssetAmount, + ); + await exchangeTradeEmulator.transferFromAsync( + signedOrder.makerAssetData, + signedOrder.makerAddress, + senderAddress, + fillMakerTokenAmount, + TradeSide.Maker, + TransferType.Trade, + ); + await exchangeTradeEmulator.transferFromAsync( + signedOrder.takerAssetData, + senderAddress, + signedOrder.makerAddress, + fillTakerAssetAmount, + TradeSide.Taker, + TransferType.Trade, + ); + const makerFeeAmount = utils.getPartialAmount( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.makerFee, + ); + await exchangeTradeEmulator.transferFromAsync( + zrxAssetData, + signedOrder.makerAddress, + signedOrder.feeRecipientAddress, + makerFeeAmount, + TradeSide.Maker, + TransferType.Fee, + ); + const takerFeeAmount = utils.getPartialAmount( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.takerFee, + ); + await exchangeTradeEmulator.transferFromAsync( + zrxAssetData, + senderAddress, + signedOrder.feeRecipientAddress, + takerFeeAmount, + TradeSide.Taker, + TransferType.Fee, + ); + } catch (err) { + throw new Error(RevertReasons.TransferFailed); + } } private static _validateRemainingFillAmountNotZeroOrThrow( takerAssetAmount: BigNumber, filledTakerTokenAmount: BigNumber, ): void { if (takerAssetAmount.eq(filledTakerTokenAmount)) { - throw new Error(ExchangeContractErrs.OrderRemainingFillAmountZero); + throw new Error(RevertReasons.OrderUnfillable); } } private static _validateOrderNotExpiredOrThrow(expirationTimeSeconds: BigNumber): void { const currentUnixTimestampSec = utils.getCurrentUnixTimestampSec(); if (expirationTimeSeconds.lessThan(currentUnixTimestampSec)) { - throw new Error(ExchangeContractErrs.OrderFillExpired); + throw new Error(RevertReasons.OrderUnfillable); } } constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) { @@ -153,7 +158,7 @@ export class OrderValidationUtils { zrxAssetData: string, ): Promise { if (fillTakerAssetAmount.eq(0)) { - throw new Error(ExchangeContractErrs.OrderFillAmountZero); + throw new Error(RevertReasons.TakerAssetFillAmount); } const orderHash = orderHashUtils.getOrderHashHex(signedOrder); const isValid = await isValidSignatureAsync( @@ -171,7 +176,7 @@ export class OrderValidationUtils { filledTakerTokenAmount, ); if (signedOrder.takerAddress !== constants.NULL_ADDRESS && signedOrder.takerAddress !== takerAddress) { - throw new Error(ExchangeContractErrs.TransactionSenderIsNotFillOrderTaker); + throw new Error(RevertReasons.InvalidTaker); } OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds); const remainingTakerTokenAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount); @@ -192,7 +197,7 @@ export class OrderValidationUtils { signedOrder.makerAssetAmount, ); if (wouldRoundingErrorOccur) { - throw new Error(ExchangeContractErrs.OrderFillRoundingError); + throw new Error(RevertReasons.RoundingError); } return filledTakerTokenAmount; } @@ -213,7 +218,7 @@ export class OrderValidationUtils { zrxAssetData, ); if (filledTakerTokenAmount !== fillTakerAssetAmount) { - throw new Error(ExchangeContractErrs.InsufficientRemainingFillAmount); + throw new Error(RevertReasons.OrderUnfillable); } } } -- cgit v1.2.3