diff options
Diffstat (limited to 'packages/order-utils')
9 files changed, 156 insertions, 120 deletions
diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index abcf3d392..6ce3169d5 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -1,5 +1,31 @@ [ { + "version": "3.0.0", + "changes": [ + { + "note": + "Add signature validation, regular cancellation and `cancelledUpTo` checks to `validateOrderFillableOrThrowAsync`", + "pr": 1235 + }, + { + "note": + "Improved the errors thrown by `validateOrderFillableOrThrowAsync` by making them more descriptive", + "pr": 1235 + }, + { + "note": + "Throw previously swallowed network errors when calling `validateOrderFillableOrThrowAsync` (see issue: #1218)", + "pr": 1235 + }, + { + "note": + "Modified the `AbstractOrderFilledCancelledFetcher` interface slightly such that `isOrderCancelledAsync` accepts a `signedOrder` instead of an `orderHash` param", + "pr": 1235 + } + ], + "timestamp": 1542028948 + }, + { "version": "2.0.1", "changes": [ { diff --git a/packages/order-utils/CHANGELOG.md b/packages/order-utils/CHANGELOG.md index 31bd02595..eaa74d6ce 100644 --- a/packages/order-utils/CHANGELOG.md +++ b/packages/order-utils/CHANGELOG.md @@ -5,6 +5,13 @@ Edit the package's CHANGELOG.json file only. CHANGELOG +## v3.0.0 - _November 12, 2018_ + + * Add signature validation, regular cancellation and `cancelledUpTo` checks to `validateOrderFillableOrThrowAsync` (#1235) + * Improved the errors thrown by `validateOrderFillableOrThrowAsync` by making them more descriptive (#1235) + * Throw previously swallowed network errors when calling `validateOrderFillableOrThrowAsync` (see issue: #1218) (#1235) + * Modified the `AbstractOrderFilledCancelledFetcher` interface slightly such that `isOrderCancelledAsync` accepts a `signedOrder` instead of an `orderHash` param (#1235) + ## v2.0.1 - _November 9, 2018_ * Dependencies updated diff --git a/packages/order-utils/package.json b/packages/order-utils/package.json index d6fb9892e..060150f96 100644 --- a/packages/order-utils/package.json +++ b/packages/order-utils/package.json @@ -1,6 +1,6 @@ { "name": "@0x/order-utils", - "version": "2.0.1", + "version": "3.0.0", "engines": { "node": ">=6.12" }, @@ -35,7 +35,7 @@ }, "homepage": "https://github.com/0xProject/0x-monorepo/packages/order-utils/README.md", "devDependencies": { - "@0x/dev-utils": "^1.0.14", + "@0x/dev-utils": "^1.0.15", "@0x/tslint-config": "^1.0.10", "@types/bn.js": "^4.11.0", "@types/lodash": "4.14.104", @@ -53,15 +53,15 @@ "typescript": "3.0.1" }, "dependencies": { - "@0x/abi-gen-wrappers": "^1.0.2", - "@0x/assert": "^1.0.15", - "@0x/base-contract": "^3.0.3", + "@0x/abi-gen-wrappers": "^1.0.3", + "@0x/assert": "^1.0.16", + "@0x/base-contract": "^3.0.4", "@0x/contract-artifacts": "^1.1.0", - "@0x/json-schemas": "^2.0.1", + "@0x/json-schemas": "^2.1.0", "@0x/types": "^1.2.1", "@0x/typescript-typings": "^3.0.4", "@0x/utils": "^2.0.4", - "@0x/web3-wrapper": "^3.1.1", + "@0x/web3-wrapper": "^3.1.2", "@types/node": "*", "bn.js": "^4.11.8", "ethereum-types": "^1.1.2", diff --git a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts index de096b7d9..9e240f9ef 100644 --- a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts +++ b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts @@ -1,3 +1,4 @@ +import { SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; /** @@ -17,6 +18,6 @@ export abstract class AbstractOrderFilledCancelledFetcher { * @param orderHash OrderHash of order we are interested in * @return Whether or not the order is cancelled */ - public abstract async isOrderCancelledAsync(orderHash: string): Promise<boolean>; + public abstract async isOrderCancelledAsync(signedOrder: SignedOrder): Promise<boolean>; public abstract getZRXAssetData(): string; } diff --git a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts index d9e66db06..186521401 100644 --- a/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts +++ b/packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts @@ -1,8 +1,9 @@ +import { SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; export abstract class AbstractOrderFilledCancelledLazyStore { public abstract async getFilledTakerAmountAsync(orderHash: string): Promise<BigNumber>; - public abstract async getIsCancelledAsync(orderHash: string): Promise<boolean>; + public abstract async getIsCancelledAsync(signedOrder: SignedOrder): Promise<boolean>; public abstract setFilledTakerAmount(orderHash: string, balance: BigNumber): void; public abstract deleteFilledTakerAmount(orderHash: string): void; public abstract setIsCancelled(orderHash: string, isCancelled: boolean): void; diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index 159aeeb09..fe0d6c773 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -117,7 +117,7 @@ export class OrderStateUtils { public async getOpenOrderStateAsync(signedOrder: SignedOrder, transactionHash?: string): Promise<OrderState> { const orderRelevantState = await this.getOpenOrderRelevantStateAsync(signedOrder); const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash); + const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder); const sidedOrderRelevantState = { isMakerSide: true, traderBalance: orderRelevantState.makerBalance, @@ -256,7 +256,7 @@ export class OrderStateUtils { const filledTakerAssetAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); const totalMakerAssetAmount = signedOrder.makerAssetAmount; const totalTakerAssetAmount = signedOrder.takerAssetAmount; - const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash); + const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder); const remainingTakerAssetAmount = isOrderCancelled ? new BigNumber(0) : totalTakerAssetAmount.minus(filledTakerAssetAmount); diff --git a/packages/order-utils/src/order_validation_utils.ts b/packages/order-utils/src/order_validation_utils.ts index a40069f63..ae4291ea8 100644 --- a/packages/order-utils/src/order_validation_utils.ts +++ b/packages/order-utils/src/order_validation_utils.ts @@ -1,4 +1,4 @@ -import { RevertReason, SignedOrder } from '@0x/types'; +import { ExchangeContractErrs, RevertReason, SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; import { Provider } from 'ethereum-types'; import * as _ from 'lodash'; @@ -17,6 +17,7 @@ import { utils } from './utils'; */ export class OrderValidationUtils { private readonly _orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher; + private readonly _provider: Provider; /** * A Typescript implementation mirroring the implementation of isRoundingError in the * Exchange smart contract @@ -57,65 +58,53 @@ export class OrderValidationUtils { senderAddress: string, zrxAssetData: string, ): Promise<void> { - try { - const fillMakerTokenAmount = utils.getPartialAmountFloor( - 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.getPartialAmountFloor( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.makerFee, - ); - await exchangeTradeEmulator.transferFromAsync( - zrxAssetData, - signedOrder.makerAddress, - signedOrder.feeRecipientAddress, - makerFeeAmount, - TradeSide.Maker, - TransferType.Fee, - ); - const takerFeeAmount = utils.getPartialAmountFloor( - fillTakerAssetAmount, - signedOrder.takerAssetAmount, - signedOrder.takerFee, - ); - await exchangeTradeEmulator.transferFromAsync( - zrxAssetData, - senderAddress, - signedOrder.feeRecipientAddress, - takerFeeAmount, - TradeSide.Taker, - TransferType.Fee, - ); - } catch (err) { - throw new Error(RevertReason.TransferFailed); - } - } - private static _validateRemainingFillAmountNotZeroOrThrow( - takerAssetAmount: BigNumber, - filledTakerTokenAmount: BigNumber, - ): void { - if (takerAssetAmount.eq(filledTakerTokenAmount)) { - throw new Error(RevertReason.OrderUnfillable); - } + const fillMakerTokenAmount = utils.getPartialAmountFloor( + 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.getPartialAmountFloor( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.makerFee, + ); + await exchangeTradeEmulator.transferFromAsync( + zrxAssetData, + signedOrder.makerAddress, + signedOrder.feeRecipientAddress, + makerFeeAmount, + TradeSide.Maker, + TransferType.Fee, + ); + const takerFeeAmount = utils.getPartialAmountFloor( + fillTakerAssetAmount, + signedOrder.takerAssetAmount, + signedOrder.takerFee, + ); + await exchangeTradeEmulator.transferFromAsync( + zrxAssetData, + senderAddress, + signedOrder.feeRecipientAddress, + takerFeeAmount, + TradeSide.Taker, + TransferType.Fee, + ); } private static _validateOrderNotExpiredOrThrow(expirationTimeSeconds: BigNumber): void { const currentUnixTimestampSec = utils.getCurrentUnixTimestampSec(); @@ -128,9 +117,13 @@ export class OrderValidationUtils { * @param orderFilledCancelledFetcher A module that implements the AbstractOrderFilledCancelledFetcher * @return An instance of OrderValidationUtils */ - constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) { + constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher, provider: Provider) { this._orderFilledCancelledFetcher = orderFilledCancelledFetcher; + this._provider = provider; } + // TODO(fabio): remove this method once the smart contracts have been refactored + // to return helpful revert reasons instead of ORDER_UNFILLABLE. Instruct devs + // to make "calls" to validate order fillability + getOrderInfo for fillable amount. /** * Validate if the supplied order is fillable, and throw if it isn't * @param exchangeTradeEmulator ExchangeTradeEmulator instance @@ -146,12 +139,29 @@ export class OrderValidationUtils { expectedFillTakerTokenAmount?: BigNumber, ): Promise<void> { const orderHash = orderHashUtils.getOrderHashHex(signedOrder); - const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); - OrderValidationUtils._validateRemainingFillAmountNotZeroOrThrow( - signedOrder.takerAssetAmount, - filledTakerTokenAmount, + const isValidSignature = await signatureUtils.isValidSignatureAsync( + this._provider, + orderHash, + signedOrder.signature, + signedOrder.makerAddress, ); - OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds); + if (!isValidSignature) { + throw new Error(RevertReason.InvalidOrderSignature); + } + + const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder); + if (isCancelled) { + throw new Error('CANCELLED'); + } + const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); + if (signedOrder.takerAssetAmount.eq(filledTakerTokenAmount)) { + throw new Error('FULLY_FILLED'); + } + try { + OrderValidationUtils._validateOrderNotExpiredOrThrow(signedOrder.expirationTimeSeconds); + } catch (err) { + throw new Error('EXPIRED'); + } let fillTakerAssetAmount = signedOrder.takerAssetAmount.minus(filledTakerTokenAmount); if (!_.isUndefined(expectedFillTakerTokenAmount)) { fillTakerAssetAmount = expectedFillTakerTokenAmount; @@ -198,10 +208,9 @@ export class OrderValidationUtils { throw new Error(OrderError.InvalidSignature); } const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); - OrderValidationUtils._validateRemainingFillAmountNotZeroOrThrow( - signedOrder.takerAssetAmount, - filledTakerTokenAmount, - ); + if (signedOrder.takerAssetAmount.eq(filledTakerTokenAmount)) { + throw new Error(RevertReason.OrderUnfillable); + } if (signedOrder.takerAddress !== constants.NULL_ADDRESS && signedOrder.takerAddress !== takerAddress) { throw new Error(RevertReason.InvalidTaker); } @@ -210,13 +219,30 @@ export class OrderValidationUtils { const desiredFillTakerTokenAmount = remainingTakerTokenAmount.lessThan(fillTakerAssetAmount) ? remainingTakerTokenAmount : fillTakerAssetAmount; - await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( - exchangeTradeEmulator, - signedOrder, - desiredFillTakerTokenAmount, - takerAddress, - zrxAssetData, - ); + try { + await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( + exchangeTradeEmulator, + signedOrder, + desiredFillTakerTokenAmount, + takerAddress, + zrxAssetData, + ); + } catch (err) { + const transferFailedErrorMessages = [ + ExchangeContractErrs.InsufficientMakerBalance, + ExchangeContractErrs.InsufficientMakerFeeBalance, + ExchangeContractErrs.InsufficientTakerBalance, + ExchangeContractErrs.InsufficientTakerFeeBalance, + ExchangeContractErrs.InsufficientMakerAllowance, + ExchangeContractErrs.InsufficientMakerFeeAllowance, + ExchangeContractErrs.InsufficientTakerAllowance, + ExchangeContractErrs.InsufficientTakerFeeAllowance, + ]; + if (_.includes(transferFailedErrorMessages, err.message)) { + throw new Error(RevertReason.TransferFailed); + } + throw err; + } const wouldRoundingErrorOccur = OrderValidationUtils.isRoundingErrorFloor( desiredFillTakerTokenAmount, @@ -228,33 +254,4 @@ export class OrderValidationUtils { } return filledTakerTokenAmount; } - /** - * Validate a call to fillOrKillOrder and throw if it would fail - * @param exchangeTradeEmulator ExchangeTradeEmulator to use - * @param provider Web3 provider to use for JSON RPC requests - * @param signedOrder SignedOrder of interest - * @param fillTakerAssetAmount Amount we'd like to fill the order for - * @param takerAddress The taker of the order - * @param zrxAssetData ZRX asset data - */ - public async validateFillOrKillOrderThrowIfInvalidAsync( - exchangeTradeEmulator: ExchangeTransferSimulator, - provider: Provider, - signedOrder: SignedOrder, - fillTakerAssetAmount: BigNumber, - takerAddress: string, - zrxAssetData: string, - ): Promise<void> { - const filledTakerTokenAmount = await this.validateFillOrderThrowIfInvalidAsync( - exchangeTradeEmulator, - provider, - signedOrder, - fillTakerAssetAmount, - takerAddress, - zrxAssetData, - ); - if (filledTakerTokenAmount !== fillTakerAssetAmount) { - throw new Error(RevertReason.OrderUnfillable); - } - } } diff --git a/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts b/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts index 1d84ffdaa..afd6f1108 100644 --- a/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts +++ b/packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts @@ -1,8 +1,10 @@ +import { SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; import * as _ from 'lodash'; import { AbstractOrderFilledCancelledFetcher } from '../abstract/abstract_order_filled_cancelled_fetcher'; import { AbstractOrderFilledCancelledLazyStore } from '../abstract/abstract_order_filled_cancelled_lazy_store'; +import { orderHashUtils } from '../order_hash'; /** * Copy on read store for balances/proxyAllowances of tokens/accounts @@ -58,9 +60,10 @@ export class OrderFilledCancelledLazyStore implements AbstractOrderFilledCancell * @param orderHash OrderHash from order of interest * @return Whether the order has been cancelled */ - public async getIsCancelledAsync(orderHash: string): Promise<boolean> { + public async getIsCancelledAsync(signedOrder: SignedOrder): Promise<boolean> { + const orderHash = orderHashUtils.getOrderHashHex(signedOrder); if (_.isUndefined(this._isCancelled[orderHash])) { - const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash); + const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder); this.setIsCancelled(orderHash, isCancelled); } const cachedIsCancelled = this._isCancelled[orderHash]; // tslint:disable-line:boolean-naming diff --git a/packages/order-utils/test/order_state_utils_test.ts b/packages/order-utils/test/order_state_utils_test.ts index 39c4c362f..42acd54c6 100644 --- a/packages/order-utils/test/order_state_utils_test.ts +++ b/packages/order-utils/test/order_state_utils_test.ts @@ -1,3 +1,4 @@ +import { SignedOrder } from '@0x/types'; import { BigNumber } from '@0x/utils'; import * as chai from 'chai'; import 'mocha'; @@ -33,7 +34,7 @@ describe('OrderStateUtils', () => { async getFilledTakerAmountAsync(_orderHash: string): Promise<BigNumber> { return filledAmount; }, - async isOrderCancelledAsync(_orderHash: string): Promise<boolean> { + async isOrderCancelledAsync(_signedOrder: SignedOrder): Promise<boolean> { return cancelled; }, getZRXAssetData(): string { |