aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFabio Berger <me@fabioberger.com>2018-11-09 07:45:48 +0800
committerFabio Berger <me@fabioberger.com>2018-11-09 07:45:48 +0800
commit857a35d4f71c1c954033c3b0505250e18be21cfb (patch)
tree47dc81508fe8d71b0c0acfcd24b36fa8455c6ae6
parent8b06b3627426c3f9eda3262d0b6479d55cc4c9ad (diff)
downloaddexon-sol-tools-857a35d4f71c1c954033c3b0505250e18be21cfb.tar.gz
dexon-sol-tools-857a35d4f71c1c954033c3b0505250e18be21cfb.tar.zst
dexon-sol-tools-857a35d4f71c1c954033c3b0505250e18be21cfb.zip
Fix validateOrderFillableOrThrowAsync method so it also checks order signature, cancelled, cancelledUpTo, and throws helpful error messages
-rw-r--r--packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts17
-rw-r--r--packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts16
-rw-r--r--packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts7
-rw-r--r--packages/contract-wrappers/test/exchange_wrapper_test.ts12
-rw-r--r--packages/contracts/test/utils/exchange_wrapper.ts4
-rw-r--r--packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts13
-rw-r--r--packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts3
-rw-r--r--packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts3
-rw-r--r--packages/order-utils/src/order_state_utils.ts4
-rw-r--r--packages/order-utils/src/order_validation_utils.ts35
-rw-r--r--packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts7
-rw-r--r--packages/order-utils/test/order_state_utils_test.ts3
12 files changed, 94 insertions, 30 deletions
diff --git a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
index 2e978f35b..902fc755c 100644
--- a/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
+++ b/packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
@@ -5,7 +5,9 @@ import {
assetDataUtils,
BalanceAndProxyAllowanceLazyStore,
ExchangeTransferSimulator,
+ orderHashUtils,
OrderValidationUtils,
+ signatureUtils,
} from '@0x/order-utils';
import { AssetProxyId, Order, SignedOrder } from '@0x/types';
import { BigNumber } from '@0x/utils';
@@ -18,6 +20,7 @@ import { OrderFilledCancelledFetcher } from '../fetchers/order_filled_cancelled_
import { methodOptsSchema } from '../schemas/method_opts_schema';
import { orderTxOptsSchema } from '../schemas/order_tx_opts_schema';
import { txOptsSchema } from '../schemas/tx_opts_schema';
+import { validateOrderFillableOptsSchema } from '../schemas/validate_order_fillable_opts_schema';
import {
BlockRange,
EventCallback,
@@ -1114,6 +1117,20 @@ export class ExchangeWrapper extends ContractWrapper {
signedOrder: SignedOrder,
opts: ValidateOrderFillableOpts = {},
): Promise<void> {
+ assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema);
+ assert.doesConformToSchema('opts', opts, validateOrderFillableOptsSchema);
+
+ const orderHash = await orderHashUtils.getOrderHashHex(signedOrder);
+ const isValidSignature = await signatureUtils.isValidSignatureAsync(
+ this._web3Wrapper.getProvider(),
+ orderHash,
+ signedOrder.signature,
+ signedOrder.makerAddress,
+ );
+ if (!isValidSignature) {
+ throw new Error('INVALID_ORDER_SIGNATURE');
+ }
+
const balanceAllowanceFetcher = new AssetBalanceAndProxyAllowanceFetcher(
this._erc20TokenWrapper,
this._erc721TokenWrapper,
diff --git a/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts b/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts
index acf7038fa..5d350916c 100644
--- a/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts
+++ b/packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts
@@ -1,5 +1,6 @@
// tslint:disable:no-unnecessary-type-assertion
-import { AbstractOrderFilledCancelledFetcher } from '@0x/order-utils';
+import { AbstractOrderFilledCancelledFetcher, orderHashUtils } from '@0x/order-utils';
+import { SignedOrder } from '@0x/types';
import { BigNumber } from '@0x/utils';
import { BlockParamLiteral } from 'ethereum-types';
@@ -18,9 +19,18 @@ export class OrderFilledCancelledFetcher implements AbstractOrderFilledCancelled
});
return filledTakerAmount;
}
- public async isOrderCancelledAsync(orderHash: string): Promise<boolean> {
+ public async isOrderCancelledAsync(signedOrder: SignedOrder): Promise<boolean> {
+ const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
const isCancelled = await this._exchange.isCancelledAsync(orderHash);
- return isCancelled;
+ const orderEpoch = await this._exchange.getOrderEpochAsync(
+ signedOrder.makerAddress,
+ signedOrder.senderAddress,
+ {
+ defaultBlock: this._stateLayer,
+ },
+ );
+ const isCancelledByOrderEpoch = orderEpoch > signedOrder.salt;
+ return isCancelled || isCancelledByOrderEpoch;
}
public getZRXAssetData(): string {
const zrxAssetData = this._exchange.getZRXAssetData();
diff --git a/packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts b/packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts
new file mode 100644
index 000000000..2e111af04
--- /dev/null
+++ b/packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts
@@ -0,0 +1,7 @@
+export const validateOrderFillableOptsSchema = {
+ id: '/ValidateOrderFillableOpts',
+ properties: {
+ expectedFillTakerTokenAmount: { $ref: '/wholeNumberSchema' },
+ },
+ type: 'object',
+};
diff --git a/packages/contract-wrappers/test/exchange_wrapper_test.ts b/packages/contract-wrappers/test/exchange_wrapper_test.ts
index 0e537bd83..8058be873 100644
--- a/packages/contract-wrappers/test/exchange_wrapper_test.ts
+++ b/packages/contract-wrappers/test/exchange_wrapper_test.ts
@@ -282,6 +282,18 @@ describe('ExchangeWrapper', () => {
expect(ordersInfo[1].orderHash).to.be.equal(anotherOrderHash);
});
});
+ describe('#validateOrderFillableOrThrowAsync', () => {
+ it('should throw if signature is invalid', async () => {
+ const signedOrderWithInvalidSignature = {
+ ...signedOrder,
+ signature: '0xdeadbeef',
+ };
+
+ expect(
+ contractWrappers.exchange.validateOrderFillableOrThrowAsync(signedOrderWithInvalidSignature),
+ ).to.eventually.to.be.rejected();
+ });
+ });
describe('#isValidSignature', () => {
it('should check if the signature is valid', async () => {
const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
diff --git a/packages/contracts/test/utils/exchange_wrapper.ts b/packages/contracts/test/utils/exchange_wrapper.ts
index 29dba690a..cfff682e1 100644
--- a/packages/contracts/test/utils/exchange_wrapper.ts
+++ b/packages/contracts/test/utils/exchange_wrapper.ts
@@ -219,6 +219,10 @@ export class ExchangeWrapper {
const isCancelled = await this._exchange.cancelled.callAsync(orderHashHex);
return isCancelled;
}
+ public async getOrderEpochAsync(makerAddress: string, takerAddress: string): Promise<BigNumber> {
+ const orderEpoch = new BigNumber(await this._exchange.orderEpoch.callAsync(makerAddress, takerAddress));
+ return orderEpoch;
+ }
public async getOrderInfoAsync(signedOrder: SignedOrder): Promise<OrderInfo> {
const orderInfo = (await this._exchange.getOrderInfo.callAsync(signedOrder)) as OrderInfo;
return orderInfo;
diff --git a/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts b/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts
index 5f5575c7b..af959e00e 100644
--- a/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts
+++ b/packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts
@@ -1,4 +1,5 @@
-import { AbstractOrderFilledCancelledFetcher } from '@0x/order-utils';
+import { AbstractOrderFilledCancelledFetcher, orderHashUtils } from '@0x/order-utils';
+import { SignedOrder } from '@0x/types';
import { BigNumber } from '@0x/utils';
import { ExchangeWrapper } from './exchange_wrapper';
@@ -14,9 +15,15 @@ export class SimpleOrderFilledCancelledFetcher implements AbstractOrderFilledCan
const filledTakerAmount = new BigNumber(await this._exchangeWrapper.getTakerAssetFilledAmountAsync(orderHash));
return filledTakerAmount;
}
- public async isOrderCancelledAsync(orderHash: string): Promise<boolean> {
+ public async isOrderCancelledAsync(signedOrder: SignedOrder): Promise<boolean> {
+ const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
const isCancelled = await this._exchangeWrapper.isCancelledAsync(orderHash);
- return isCancelled;
+ const orderEpoch = await this._exchangeWrapper.getOrderEpochAsync(
+ signedOrder.makerAddress,
+ signedOrder.senderAddress,
+ );
+ const isCancelledByOrderEpoch = orderEpoch > signedOrder.salt;
+ return isCancelled || isCancelledByOrderEpoch;
}
public getZRXAssetData(): string {
return this._zrxAssetData;
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..766fbaf0f 100644
--- a/packages/order-utils/src/order_validation_utils.ts
+++ b/packages/order-utils/src/order_validation_utils.ts
@@ -109,14 +109,6 @@ export class OrderValidationUtils {
throw new Error(RevertReason.TransferFailed);
}
}
- private static _validateRemainingFillAmountNotZeroOrThrow(
- takerAssetAmount: BigNumber,
- filledTakerTokenAmount: BigNumber,
- ): void {
- if (takerAssetAmount.eq(filledTakerTokenAmount)) {
- throw new Error(RevertReason.OrderUnfillable);
- }
- }
private static _validateOrderNotExpiredOrThrow(expirationTimeSeconds: BigNumber): void {
const currentUnixTimestampSec = utils.getCurrentUnixTimestampSec();
if (expirationTimeSeconds.lessThan(currentUnixTimestampSec)) {
@@ -125,12 +117,15 @@ export class OrderValidationUtils {
}
/**
* Instantiate OrderValidationUtils
- * @param orderFilledCancelledFetcher A module that implements the AbstractOrderFilledCancelledFetcher
+ * @param orderFilledCancelledFetcher A module that implements the AbstractOrderInfoFetcher
* @return An instance of OrderValidationUtils
*/
constructor(orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher) {
this._orderFilledCancelledFetcher = orderFilledCancelledFetcher;
}
+ // 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 +141,19 @@ export class OrderValidationUtils {
expectedFillTakerTokenAmount?: BigNumber,
): Promise<void> {
const orderHash = orderHashUtils.getOrderHashHex(signedOrder);
+ const isCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(signedOrder);
+ if (isCancelled) {
+ throw new Error('CANCELLED');
+ }
const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash);
- OrderValidationUtils._validateRemainingFillAmountNotZeroOrThrow(
- signedOrder.takerAssetAmount,
- filledTakerTokenAmount,
- );
+ 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 +200,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);
}
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 {