crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix validateOrderFillableOrThrowAsync method so it also checks order signature, cancelled, cancelledUpTo, and throws helpful error messages

Browse Source
This commit is contained in:
Fabio Berger 2018-11-09 00:45:48 +01:00
parent 8b06b36274
commit 857a35d4f7
12 changed files with 94 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
packages/contract-wrappers/src/contract_wrappers/exchange_wrapper.ts
View File

@ -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,

16
packages/contract-wrappers/src/fetchers/order_filled_cancelled_fetcher.ts
View File

@ -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();

7
packages/contract-wrappers/src/schemas/validate_order_fillable_opts_schema.ts Normal file
View File

@ -0,0 +1,7 @@
export const validateOrderFillableOptsSchema = {
id: '/ValidateOrderFillableOpts',
properties: {
expectedFillTakerTokenAmount: { $ref: '/wholeNumberSchema' },
},
type: 'object',
};

12
packages/contract-wrappers/test/exchange_wrapper_test.ts
View File

@ -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);

4
packages/contracts/test/utils/exchange_wrapper.ts
View File

@ -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;

13
packages/contracts/test/utils/simple_order_filled_cancelled_fetcher.ts
View File

@ -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;

3
packages/order-utils/src/abstract/abstract_order_filled_cancelled_fetcher.ts
View File

@ -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;
}

3
packages/order-utils/src/abstract/abstract_order_filled_cancelled_lazy_store.ts
View File

@ -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;

4
packages/order-utils/src/order_state_utils.ts
View File

@ -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);

35
packages/order-utils/src/order_validation_utils.ts
View File

@ -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);
}

7
packages/order-utils/src/store/order_filled_cancelled_lazy_store.ts
View File

@ -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

3
packages/order-utils/test/order_state_utils_test.ts
View File

@ -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 {