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

Merge pull request #1235 from 0xProject/fixOrderValidation

Browse Source 
[order-utils] Fix order validation method
This commit is contained in:
Fabio B 2018-11-12 12:17:27 +01:00 committed by GitHub
commit c41622c20a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 217 additions and 124 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
packages/contract-wrappers/CHANGELOG.json
View File

@ -1,4 +1,24 @@
[
{
"version": "4.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
}
]
},
{
"version": "3.0.1",
"changes": [

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

@ -18,6 +18,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 +1115,9 @@ export class ExchangeWrapper extends ContractWrapper {
signedOrder: SignedOrder,
opts: ValidateOrderFillableOpts = {},
): Promise<void> {
assert.doesConformToSchema('signedOrder', signedOrder, schemas.signedOrderSchema);
assert.doesConformToSchema('opts', opts, validateOrderFillableOptsSchema);
const balanceAllowanceFetcher = new AssetBalanceAndProxyAllowanceFetcher(
this._erc20TokenWrapper,
this._erc721TokenWrapper,
@ -1124,7 +1128,7 @@ export class ExchangeWrapper extends ContractWrapper {
const expectedFillTakerTokenAmountIfExists = opts.expectedFillTakerTokenAmount;
const filledCancelledFetcher = new OrderFilledCancelledFetcher(this, BlockParamLiteral.Latest);
const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher);
const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher, this._web3Wrapper.getProvider());
await orderValidationUtils.validateOrderFillableOrThrowAsync(
exchangeTradeSimulator,
signedOrder,
@ -1152,7 +1156,7 @@ export class ExchangeWrapper extends ContractWrapper {
const exchangeTradeSimulator = new ExchangeTransferSimulator(balanceAllowanceStore);
const filledCancelledFetcher = new OrderFilledCancelledFetcher(this, BlockParamLiteral.Latest);
const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher);
const orderValidationUtils = new OrderValidationUtils(filledCancelledFetcher, this._web3Wrapper.getProvider());
await orderValidationUtils.validateFillOrderThrowIfInvalidAsync(
exchangeTradeSimulator,
this._web3Wrapper.getProvider(),

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',
};

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

@ -1,7 +1,7 @@
import { BlockchainLifecycle, callbackErrorReporter } from '@0x/dev-utils';
import { FillScenarios } from '@0x/fill-scenarios';
import { assetDataUtils, orderHashUtils } from '@0x/order-utils';
import { DoneCallback, SignedOrder } from '@0x/types';
import { DoneCallback, RevertReason, SignedOrder } from '@0x/types';
import { BigNumber } from '@0x/utils';
import * as chai from 'chai';
import { BlockParamLiteral } from 'ethereum-types';
@ -282,6 +282,19 @@ describe('ExchangeWrapper', () => {
expect(ordersInfo[1].orderHash).to.be.equal(anotherOrderHash);
});
});
describe('#validateOrderFillableOrThrowAsync', () => {
it('should throw if signature is invalid', async () => {
const signedOrderWithInvalidSignature = {
...signedOrder,
signature:
'0x1b61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc3340349190569279751135161d22529dc25add4f6069af05be04cacbda2ace225403',
};
expect(
contractWrappers.exchange.validateOrderFillableOrThrowAsync(signedOrderWithInvalidSignature),
).to.eventually.to.be.rejectedWith(RevertReason.InvalidOrderSignature);
});
});
describe('#isValidSignature', () => {
it('should check if the signature is valid', async () => {
const orderHash = orderHashUtils.getOrderHashHex(signedOrder);

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

@ -212,13 +212,17 @@ export class ExchangeWrapper {
return tx;
}
public async getTakerAssetFilledAmountAsync(orderHashHex: string): Promise<BigNumber> {
const filledAmount = new BigNumber(await this._exchange.filled.callAsync(orderHashHex));
const filledAmount = await this._exchange.filled.callAsync(orderHashHex);
return filledAmount;
}
public async isCancelledAsync(orderHashHex: string): Promise<boolean> {
const isCancelled = await this._exchange.cancelled.callAsync(orderHashHex);
return isCancelled;
}
public async getOrderEpochAsync(makerAddress: string, senderAddress: string): Promise<BigNumber> {
const orderEpoch = await this._exchange.orderEpoch.callAsync(makerAddress, senderAddress);
return orderEpoch;
}
public async getOrderInfoAsync(signedOrder: SignedOrder): Promise<OrderInfo> {
const orderInfo = (await this._exchange.getOrderInfo.callAsync(signedOrder)) as OrderInfo;
return orderInfo;

2
packages/contracts/test/utils/fill_order_combinatorial_utils.ts
View File

@ -392,7 +392,7 @@ export class FillOrderCombinatorialUtils {
);
// 5. If I fill it by X, what are the resulting balances/allowances/filled amounts expected?
const orderValidationUtils = new OrderValidationUtils(orderFilledCancelledFetcher);
const orderValidationUtils = new OrderValidationUtils(orderFilledCancelledFetcher, provider);
const lazyStore = new BalanceAndProxyAllowanceLazyStore(balanceAndProxyAllowanceFetcher);
const exchangeTransferSimulator = new ExchangeTransferSimulator(lazyStore);

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;

25
packages/order-utils/CHANGELOG.json
View File

@ -1,4 +1,29 @@
[
{
"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
}
]
},
{
"version": "2.0.1",
"changes": [

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

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

@ -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,7 +58,6 @@ export class OrderValidationUtils {
senderAddress: string,
zrxAssetData: string,
): Promise<void> {
try {
const fillMakerTokenAmount = utils.getPartialAmountFloor(
fillTakerAssetAmount,
signedOrder.takerAssetAmount,
@ -105,17 +105,6 @@ export class OrderValidationUtils {
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);
}
}
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,
);
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,6 +219,7 @@ export class OrderValidationUtils {
const desiredFillTakerTokenAmount = remainingTakerTokenAmount.lessThan(fillTakerAssetAmount)
? remainingTakerTokenAmount
: fillTakerAssetAmount;
try {
await OrderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync(
exchangeTradeEmulator,
signedOrder,
@ -217,6 +227,22 @@ export class OrderValidationUtils {
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);
}
}
}

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 {