From 6058a74da54e916e970912d942954d55922cef95 Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 7 Jun 2018 18:02:48 +0200 Subject: Update naming in orderStateUtils to use asset over token, also removed cancelledAmount since in V2 it's binary. --- .../abstract_order_filled_cancelled_fetcher.ts | 3 +- packages/order-utils/src/order_state_utils.ts | 78 ++++++++++------------ 2 files changed, 37 insertions(+), 44 deletions(-) (limited to 'packages/order-utils') 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 f54bed6f1..ec398a11e 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 @@ -2,7 +2,6 @@ import { BigNumber } from '@0xproject/utils'; export abstract class AbstractOrderFilledCancelledFetcher { public abstract async getFilledTakerAmountAsync(orderHash: string): Promise; - public abstract async getCancelledTakerAmountAsync(orderHash: string): Promise; - public abstract async getUnavailableTakerAmountAsync(orderHash: string): Promise; + public abstract async isOrderCancelledAsync(orderHash: string): Promise; public abstract getZRXTokenAddress(): string; } diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index da33ea010..7636372f8 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -20,11 +20,8 @@ export class OrderStateUtils { private _balanceAndProxyAllowanceFetcher: AbstractBalanceAndProxyAllowanceFetcher; private _orderFilledCancelledFetcher: AbstractOrderFilledCancelledFetcher; private static _validateIfOrderIsValid(signedOrder: SignedOrder, orderRelevantState: OrderRelevantState): void { - const unavailableTakerTokenAmount = orderRelevantState.cancelledTakerTokenAmount.add( - orderRelevantState.filledTakerTokenAmount, - ); - const availableTakerTokenAmount = signedOrder.takerAssetAmount.minus(unavailableTakerTokenAmount); - if (availableTakerTokenAmount.eq(0)) { + const availableTakerAssetAmount = signedOrder.takerAssetAmount.minus(orderRelevantState.filledTakerAssetAmount); + if (availableTakerAssetAmount.eq(0)) { throw new Error(ExchangeContractErrs.OrderRemainingFillAmountZero); } @@ -42,12 +39,12 @@ export class OrderStateUtils { throw new Error(ExchangeContractErrs.InsufficientMakerFeeAllowance); } } - const minFillableTakerTokenAmountWithinNoRoundingErrorRange = signedOrder.takerAssetAmount + const minFillableTakerAssetAmountWithinNoRoundingErrorRange = signedOrder.takerAssetAmount .dividedBy(ACCEPTABLE_RELATIVE_ROUNDING_ERROR) .dividedBy(signedOrder.makerAssetAmount); if ( - orderRelevantState.remainingFillableTakerTokenAmount.lessThan( - minFillableTakerTokenAmountWithinNoRoundingErrorRange, + orderRelevantState.remainingFillableTakerAssetAmount.lessThan( + minFillableTakerAssetAmountWithinNoRoundingErrorRange, ) ) { throw new Error(ExchangeContractErrs.OrderFillRoundingError); @@ -72,23 +69,20 @@ export class OrderStateUtils { }; return orderState; } catch (err) { - const orderState: OrderStateInvalid = { - isValid: false, - orderHash, - error: err.message, - }; - return orderState; + throw err; } } public async getOrderRelevantStateAsync(signedOrder: SignedOrder): Promise { const zrxTokenAddress = this._orderFilledCancelledFetcher.getZRXTokenAddress(); + const makerProxyData = assetProxyUtils.decodeERC20ProxyData(signedOrder.makerAssetData); + const makerAssetAddress = makerProxyData.tokenAddress; const orderHash = orderHashUtils.getOrderHashHex(signedOrder); const makerBalance = await this._balanceAndProxyAllowanceFetcher.getBalanceAsync( - signedOrder.makerAssetData, + makerAssetAddress, signedOrder.makerAddress, ); const makerProxyAllowance = await this._balanceAndProxyAllowanceFetcher.getProxyAllowanceAsync( - signedOrder.makerAssetData, + makerAssetAddress, signedOrder.makerAddress, ); const makerFeeBalance = await this._balanceAndProxyAllowanceFetcher.getBalanceAsync( @@ -99,42 +93,42 @@ export class OrderStateUtils { zrxTokenAddress, signedOrder.makerAddress, ); - const filledTakerTokenAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); - const cancelledTakerTokenAmount = await this._orderFilledCancelledFetcher.getCancelledTakerAmountAsync( - orderHash, - ); - const unavailableTakerTokenAmount = await this._orderFilledCancelledFetcher.getUnavailableTakerAmountAsync( - orderHash, - ); - const totalMakerTokenAmount = signedOrder.makerAssetAmount; - const totalTakerTokenAmount = signedOrder.takerAssetAmount; - const remainingTakerTokenAmount = totalTakerTokenAmount.minus(unavailableTakerTokenAmount); - const remainingMakerTokenAmount = remainingTakerTokenAmount - .times(totalMakerTokenAmount) - .dividedToIntegerBy(totalTakerTokenAmount); - const transferrableMakerTokenAmount = BigNumber.min([makerProxyAllowance, makerBalance]); - const transferrableFeeTokenAmount = BigNumber.min([makerFeeProxyAllowance, makerFeeBalance]); + const filledTakerAssetAmount = await this._orderFilledCancelledFetcher.getFilledTakerAmountAsync(orderHash); + const isOrderCancelled = await this._orderFilledCancelledFetcher.isOrderCancelledAsync(orderHash); + const totalMakerAssetAmount = signedOrder.makerAssetAmount; + const totalTakerAssetAmount = signedOrder.takerAssetAmount; + const remainingTakerAssetAmount = isOrderCancelled + ? new BigNumber(0) + : totalTakerAssetAmount.minus(filledTakerAssetAmount); + const remainingMakerAssetAmount = remainingTakerAssetAmount + .times(totalMakerAssetAmount) + .dividedToIntegerBy(totalTakerAssetAmount); + const transferrableMakerAssetAmount = BigNumber.min([makerProxyAllowance, makerBalance]); + const transferrableFeeAssetAmount = BigNumber.min([makerFeeProxyAllowance, makerFeeBalance]); const zrxAssetData = assetProxyUtils.encodeERC20ProxyData(zrxTokenAddress); - const isMakerTokenZRX = signedOrder.makerAssetData === zrxAssetData; + const isMakerAssetZRX = signedOrder.makerAssetData === zrxAssetData; + const isTraderMaker = true; const remainingFillableCalculator = new RemainingFillableCalculator( + isTraderMaker, signedOrder, - isMakerTokenZRX, - transferrableMakerTokenAmount, - transferrableFeeTokenAmount, - remainingMakerTokenAmount, + isMakerAssetZRX, + transferrableMakerAssetAmount, + transferrableFeeAssetAmount, + remainingMakerAssetAmount, ); - const remainingFillableMakerTokenAmount = remainingFillableCalculator.computeRemainingMakerFillable(); - const remainingFillableTakerTokenAmount = remainingFillableCalculator.computeRemainingTakerFillable(); + const remainingFillableMakerAssetAmount = remainingFillableCalculator.computeRemainingFillable(); + const remainingFillableTakerAssetAmount = remainingFillableMakerAssetAmount + .times(signedOrder.takerAssetAmount) + .dividedToIntegerBy(signedOrder.makerAssetAmount); const orderRelevantState = { makerBalance, makerProxyAllowance, makerFeeBalance, makerFeeProxyAllowance, - filledTakerTokenAmount, - cancelledTakerTokenAmount, - remainingFillableMakerTokenAmount, - remainingFillableTakerTokenAmount, + filledTakerAssetAmount, + remainingFillableMakerAssetAmount, + remainingFillableTakerAssetAmount, }; return orderRelevantState; } -- cgit v1.2.3 From 73cc2a140c778a5396c2766562a3fc648337e18a Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 7 Jun 2018 18:03:50 +0200 Subject: - Refactor remainingFillableCalculator so it can be used for the maker and taker side - Moved the tests over from order-watcher - Did some token -> asset renaming --- .../src/remaining_fillable_calculator.ts | 115 +++++---- .../test/remaining_fillable_calculator_test.ts | 257 +++++++++++++++++++++ 2 files changed, 314 insertions(+), 58 deletions(-) create mode 100644 packages/order-utils/test/remaining_fillable_calculator_test.ts (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/remaining_fillable_calculator.ts b/packages/order-utils/src/remaining_fillable_calculator.ts index b291d8ea9..acab2f41b 100644 --- a/packages/order-utils/src/remaining_fillable_calculator.ts +++ b/packages/order-utils/src/remaining_fillable_calculator.ts @@ -3,93 +3,92 @@ import { BigNumber } from '@0xproject/utils'; export class RemainingFillableCalculator { private _signedOrder: SignedOrder; - private _isMakerTokenZRX: boolean; + private _isTraderAssetZRX: boolean; // Transferrable Amount is the minimum of Approval and Balance - private _transferrableMakerTokenAmount: BigNumber; - private _transferrableMakerFeeTokenAmount: BigNumber; - private _remainingMakerTokenAmount: BigNumber; - private _remainingMakerFeeAmount: BigNumber; + private _transferrableAssetAmount: BigNumber; + private _transferrableFeeAmount: BigNumber; + private _remainingOrderAssetAmount: BigNumber; + private _remainingOrderFeeAmount: BigNumber; + private _orderFee: BigNumber; + private _orderAssetAmount: BigNumber; constructor( + isTraderMaker: boolean, signedOrder: SignedOrder, - isMakerTokenZRX: boolean, - transferrableMakerTokenAmount: BigNumber, - transferrableMakerFeeTokenAmount: BigNumber, - remainingMakerTokenAmount: BigNumber, + isTraderAssetZRX: boolean, + transferrableAssetAmount: BigNumber, + transferrableFeeAmount: BigNumber, + remainingOrderAssetAmount: BigNumber, ) { + if (isTraderMaker) { + this._orderFee = signedOrder.makerFee; + this._orderAssetAmount = signedOrder.makerAssetAmount; + } else { + this._orderFee = signedOrder.takerFee; + this._orderAssetAmount = signedOrder.takerAssetAmount; + } this._signedOrder = signedOrder; - this._isMakerTokenZRX = isMakerTokenZRX; - this._transferrableMakerTokenAmount = transferrableMakerTokenAmount; - this._transferrableMakerFeeTokenAmount = transferrableMakerFeeTokenAmount; - this._remainingMakerTokenAmount = remainingMakerTokenAmount; - this._remainingMakerFeeAmount = remainingMakerTokenAmount - .times(signedOrder.makerFee) - .dividedToIntegerBy(signedOrder.makerAssetAmount); + this._isTraderAssetZRX = isTraderAssetZRX; + this._transferrableAssetAmount = transferrableAssetAmount; + this._transferrableFeeAmount = transferrableFeeAmount; + this._remainingOrderAssetAmount = remainingOrderAssetAmount; + this._remainingOrderFeeAmount = remainingOrderAssetAmount + .times(this._orderFee) + .dividedToIntegerBy(this._orderAssetAmount); } - public computeRemainingMakerFillable(): BigNumber { + public computeRemainingFillable(): BigNumber { if (this._hasSufficientFundsForFeeAndTransferAmount()) { - return this._remainingMakerTokenAmount; + return this._remainingOrderAssetAmount; } - if (this._signedOrder.makerFee.isZero()) { - return BigNumber.min(this._remainingMakerTokenAmount, this._transferrableMakerTokenAmount); + if (this._orderFee.isZero()) { + return BigNumber.min(this._remainingOrderAssetAmount, this._transferrableAssetAmount); } - return this._calculatePartiallyFillableMakerTokenAmount(); - } - public computeRemainingTakerFillable(): BigNumber { - return this.computeRemainingMakerFillable() - .times(this._signedOrder.takerAssetAmount) - .dividedToIntegerBy(this._signedOrder.makerAssetAmount); + return this._calculatePartiallyFillableAssetAmount(); } private _hasSufficientFundsForFeeAndTransferAmount(): boolean { - if (this._isMakerTokenZRX) { - const totalZRXTransferAmountRequired = this._remainingMakerTokenAmount.plus(this._remainingMakerFeeAmount); - const hasSufficientFunds = this._transferrableMakerTokenAmount.greaterThanOrEqualTo( + if (this._isTraderAssetZRX) { + const totalZRXTransferAmountRequired = this._remainingOrderAssetAmount.plus(this._remainingOrderFeeAmount); + const hasSufficientFunds = this._transferrableAssetAmount.greaterThanOrEqualTo( totalZRXTransferAmountRequired, ); return hasSufficientFunds; } else { - const hasSufficientFundsForTransferAmount = this._transferrableMakerTokenAmount.greaterThanOrEqualTo( - this._remainingMakerTokenAmount, + const hasSufficientFundsForTransferAmount = this._transferrableAssetAmount.greaterThanOrEqualTo( + this._remainingOrderAssetAmount, ); - const hasSufficientFundsForFeeAmount = this._transferrableMakerFeeTokenAmount.greaterThanOrEqualTo( - this._remainingMakerFeeAmount, + const hasSufficientFundsForFeeAmount = this._transferrableFeeAmount.greaterThanOrEqualTo( + this._remainingOrderFeeAmount, ); const hasSufficientFunds = hasSufficientFundsForTransferAmount && hasSufficientFundsForFeeAmount; return hasSufficientFunds; } } - private _calculatePartiallyFillableMakerTokenAmount(): BigNumber { + private _calculatePartiallyFillableAssetAmount(): BigNumber { // Given an order for 200 wei for 2 ZRXwei fee, find 100 wei for 1 ZRXwei. Order ratio is then 100:1 - const orderToFeeRatio = this._signedOrder.makerAssetAmount.dividedBy(this._signedOrder.makerFee); - // The number of times the maker can fill the order, if each fill only required the transfer of a single + const orderToFeeRatio = this._orderAssetAmount.dividedBy(this._orderFee); + // The number of times the trader (maker or taker) can fill the order, if each fill only required the transfer of a single // baseUnit of fee tokens. - // Given 2 ZRXwei, the maximum amount of times Maker can fill this order, in terms of fees, is 2 - const fillableTimesInFeeTokenBaseUnits = BigNumber.min( - this._transferrableMakerFeeTokenAmount, - this._remainingMakerFeeAmount, - ); - // The number of times the Maker can fill the order, given the Maker Token Balance - // Assuming a balance of 150 wei, and an orderToFeeRatio of 100:1, maker can fill this order 1 time. - let fillableTimesInMakerTokenUnits = this._transferrableMakerTokenAmount.dividedBy(orderToFeeRatio); - if (this._isMakerTokenZRX) { - // If ZRX is the maker token, the Fee and the Maker amount need to be removed from the same pool; + // Given 2 ZRXwei, the maximum amount of times trader can fill this order, in terms of fees, is 2 + const fillableTimesInFeeBaseUnits = BigNumber.min(this._transferrableFeeAmount, this._remainingOrderFeeAmount); + // The number of times the trader can fill the order, given the traders asset Balance + // Assuming a balance of 150 wei, and an orderToFeeRatio of 100:1, trader can fill this order 1 time. + let fillableTimesInAssetUnits = this._transferrableAssetAmount.dividedBy(orderToFeeRatio); + if (this._isTraderAssetZRX) { + // If ZRX is the trader asset, the Fee and the trader fill amount need to be removed from the same pool; // 200 ZRXwei for 2ZRXwei fee can only be filled once (need 202 ZRXwei) - const totalZRXTokenPooled = this._transferrableMakerTokenAmount; + const totalZRXTokenPooled = this._transferrableAssetAmount; // The purchasing power here is less as the tokens are taken from the same Pool // For every one number of fills, we have to take an extra ZRX out of the pool - fillableTimesInMakerTokenUnits = totalZRXTokenPooled.dividedBy(orderToFeeRatio.plus(new BigNumber(1))); + fillableTimesInAssetUnits = totalZRXTokenPooled.dividedBy(orderToFeeRatio.plus(new BigNumber(1))); } // When Ratio is not fully divisible there can be remainders which cannot be represented, so they are floored. // This can result in a RoundingError being thrown by the Exchange Contract. - const partiallyFillableMakerTokenAmount = fillableTimesInMakerTokenUnits - .times(this._signedOrder.makerAssetAmount) - .dividedToIntegerBy(this._signedOrder.makerFee); - const partiallyFillableFeeTokenAmount = fillableTimesInFeeTokenBaseUnits - .times(this._signedOrder.makerAssetAmount) - .dividedToIntegerBy(this._signedOrder.makerFee); - const partiallyFillableAmount = BigNumber.min( - partiallyFillableMakerTokenAmount, - partiallyFillableFeeTokenAmount, - ); + const partiallyFillableAssetAmount = fillableTimesInAssetUnits + .times(this._orderAssetAmount) + .dividedToIntegerBy(this._orderFee); + const partiallyFillableFeeAmount = fillableTimesInFeeBaseUnits + .times(this._orderAssetAmount) + .dividedToIntegerBy(this._orderFee); + const partiallyFillableAmount = BigNumber.min(partiallyFillableAssetAmount, partiallyFillableFeeAmount); return partiallyFillableAmount; } } diff --git a/packages/order-utils/test/remaining_fillable_calculator_test.ts b/packages/order-utils/test/remaining_fillable_calculator_test.ts new file mode 100644 index 000000000..81f8132b2 --- /dev/null +++ b/packages/order-utils/test/remaining_fillable_calculator_test.ts @@ -0,0 +1,257 @@ +import { ECSignature, SignedOrder } from '@0xproject/types'; +import { BigNumber } from '@0xproject/utils'; +import { Web3Wrapper } from '@0xproject/web3-wrapper'; +import * as chai from 'chai'; +import 'make-promises-safe'; +import 'mocha'; + +import { RemainingFillableCalculator } from '../src/remaining_fillable_calculator'; + +import { chaiSetup } from './utils/chai_setup'; + +chaiSetup.configure(); +const expect = chai.expect; + +describe('RemainingFillableCalculator', () => { + let calculator: RemainingFillableCalculator; + let signedOrder: SignedOrder; + let transferrableMakeAssetAmount: BigNumber; + let transferrableMakerFeeTokenAmount: BigNumber; + let remainingMakeAssetAmount: BigNumber; + let makerAmount: BigNumber; + let takerAmount: BigNumber; + let makerFeeAmount: BigNumber; + let isMakeAssetZRX: boolean; + const makerAssetData: string = '0x1'; + const takerAssetData: string = '0x2'; + const decimals: number = 4; + const zero: BigNumber = new BigNumber(0); + const zeroAddress = '0x0'; + const signature: string = + '0x1B61a3ed31b43c8780e905a260a35faefcc527be7516aa11c0256729b5b351bc3340349190569279751135161d22529dc25add4f6069af05be04cacbda2ace225403'; + beforeEach(async () => { + [makerAmount, takerAmount, makerFeeAmount] = [ + Web3Wrapper.toBaseUnitAmount(new BigNumber(50), decimals), + Web3Wrapper.toBaseUnitAmount(new BigNumber(5), decimals), + Web3Wrapper.toBaseUnitAmount(new BigNumber(1), decimals), + ]; + [transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount] = [ + Web3Wrapper.toBaseUnitAmount(new BigNumber(50), decimals), + Web3Wrapper.toBaseUnitAmount(new BigNumber(5), decimals), + ]; + }); + function buildSignedOrder(): SignedOrder { + return { + signature, + exchangeAddress: zeroAddress, + feeRecipientAddress: zeroAddress, + senderAddress: zeroAddress, + makerAddress: zeroAddress, + takerAddress: zeroAddress, + makerFee: makerFeeAmount, + takerFee: zero, + makerAssetAmount: makerAmount, + takerAssetAmount: takerAmount, + makerAssetData, + takerAssetData, + salt: zero, + expirationTimeSeconds: zero, + }; + } + describe('Maker token is NOT ZRX', () => { + before(async () => { + isMakeAssetZRX = false; + }); + it('calculates the correct amount when unfilled and funds available', () => { + signedOrder = buildSignedOrder(); + remainingMakeAssetAmount = signedOrder.makerAssetAmount; + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(remainingMakeAssetAmount); + }); + it('calculates the correct amount when partially filled and funds available', () => { + signedOrder = buildSignedOrder(); + remainingMakeAssetAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1), decimals); + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(remainingMakeAssetAmount); + }); + it('calculates the amount to be 0 when all fee funds are transferred', () => { + signedOrder = buildSignedOrder(); + transferrableMakerFeeTokenAmount = zero; + remainingMakeAssetAmount = signedOrder.makerAssetAmount; + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(zero); + }); + it('calculates the correct amount when balance is less than remaining fillable', () => { + signedOrder = buildSignedOrder(); + const partiallyFilledAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(2), decimals); + remainingMakeAssetAmount = signedOrder.makerAssetAmount.minus(partiallyFilledAmount); + transferrableMakeAssetAmount = remainingMakeAssetAmount.minus(partiallyFilledAmount); + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(transferrableMakeAssetAmount); + }); + describe('Order to Fee Ratio is < 1', () => { + beforeEach(async () => { + [makerAmount, takerAmount, makerFeeAmount] = [ + Web3Wrapper.toBaseUnitAmount(new BigNumber(3), decimals), + Web3Wrapper.toBaseUnitAmount(new BigNumber(6), decimals), + Web3Wrapper.toBaseUnitAmount(new BigNumber(6), decimals), + ]; + }); + it('calculates the correct amount when funds unavailable', () => { + signedOrder = buildSignedOrder(); + remainingMakeAssetAmount = signedOrder.makerAssetAmount; + const transferredAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(2), decimals); + transferrableMakeAssetAmount = remainingMakeAssetAmount.minus(transferredAmount); + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(transferrableMakeAssetAmount); + }); + }); + describe('Ratio is not evenly divisble', () => { + beforeEach(async () => { + [makerAmount, takerAmount, makerFeeAmount] = [ + Web3Wrapper.toBaseUnitAmount(new BigNumber(3), decimals), + Web3Wrapper.toBaseUnitAmount(new BigNumber(7), decimals), + Web3Wrapper.toBaseUnitAmount(new BigNumber(7), decimals), + ]; + }); + it('calculates the correct amount when funds unavailable', () => { + signedOrder = buildSignedOrder(); + remainingMakeAssetAmount = signedOrder.makerAssetAmount; + const transferredAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(2), decimals); + transferrableMakeAssetAmount = remainingMakeAssetAmount.minus(transferredAmount); + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + const calculatedFillableAmount = calculator.computeRemainingFillable(); + expect(calculatedFillableAmount.lessThanOrEqualTo(transferrableMakeAssetAmount)).to.be.true(); + expect(calculatedFillableAmount).to.be.bignumber.greaterThan(new BigNumber(0)); + const orderToFeeRatio = signedOrder.makerAssetAmount.dividedBy(signedOrder.makerFee); + const calculatedFeeAmount = calculatedFillableAmount.dividedBy(orderToFeeRatio); + expect(calculatedFeeAmount).to.be.bignumber.lessThan(transferrableMakerFeeTokenAmount); + }); + }); + }); + describe('Maker Token is ZRX', () => { + before(async () => { + isMakeAssetZRX = true; + }); + it('calculates the correct amount when unfilled and funds available', () => { + signedOrder = buildSignedOrder(); + transferrableMakeAssetAmount = makerAmount.plus(makerFeeAmount); + transferrableMakerFeeTokenAmount = transferrableMakeAssetAmount; + remainingMakeAssetAmount = signedOrder.makerAssetAmount; + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(remainingMakeAssetAmount); + }); + it('calculates the correct amount when partially filled and funds available', () => { + signedOrder = buildSignedOrder(); + remainingMakeAssetAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1), decimals); + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(remainingMakeAssetAmount); + }); + it('calculates the amount to be 0 when all fee funds are transferred', () => { + signedOrder = buildSignedOrder(); + transferrableMakeAssetAmount = zero; + transferrableMakerFeeTokenAmount = zero; + remainingMakeAssetAmount = signedOrder.makerAssetAmount; + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(zero); + }); + it('calculates the correct amount when balance is less than remaining fillable', () => { + signedOrder = buildSignedOrder(); + const partiallyFilledAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(2), decimals); + remainingMakeAssetAmount = signedOrder.makerAssetAmount.minus(partiallyFilledAmount); + transferrableMakeAssetAmount = remainingMakeAssetAmount.minus(partiallyFilledAmount); + transferrableMakerFeeTokenAmount = transferrableMakeAssetAmount; + + const orderToFeeRatio = signedOrder.makerAssetAmount.dividedToIntegerBy(signedOrder.makerFee); + const expectedFillableAmount = new BigNumber(450980); + const isTraderMaker = true; + calculator = new RemainingFillableCalculator( + isTraderMaker, + signedOrder, + isMakeAssetZRX, + transferrableMakeAssetAmount, + transferrableMakerFeeTokenAmount, + remainingMakeAssetAmount, + ); + const calculatedFillableAmount = calculator.computeRemainingFillable(); + const numberOfFillsInRatio = calculatedFillableAmount.dividedToIntegerBy(orderToFeeRatio); + const calculatedFillableAmountPlusFees = calculatedFillableAmount.plus(numberOfFillsInRatio); + expect(calculatedFillableAmountPlusFees).to.be.bignumber.lessThan(transferrableMakeAssetAmount); + expect(calculatedFillableAmountPlusFees).to.be.bignumber.lessThan(remainingMakeAssetAmount); + expect(calculatedFillableAmount).to.be.bignumber.equal(expectedFillableAmount); + expect(numberOfFillsInRatio.decimalPlaces()).to.be.equal(0); + }); + }); +}); -- cgit v1.2.3 From dee0fec9e9521712d11f1f040ae1264c41badaba Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 7 Jun 2018 18:10:56 +0200 Subject: Revert to returning orderState on invalid order --- packages/order-utils/src/order_state_utils.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index 7636372f8..ef868e60f 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -69,7 +69,12 @@ export class OrderStateUtils { }; return orderState; } catch (err) { - throw err; + const orderState: OrderStateInvalid = { + isValid: false, + orderHash, + error: err.message, + }; + return orderState; } } public async getOrderRelevantStateAsync(signedOrder: SignedOrder): Promise { -- cgit v1.2.3 From f5ad553be35bcc35fdac9620ae3d1e5546fd902c Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 7 Jun 2018 18:33:42 +0200 Subject: Pass in fee and assetAmount rather then the whole signedOrder --- packages/order-utils/src/order_state_utils.ts | 5 +-- .../src/remaining_fillable_calculator.ts | 16 ++----- .../test/remaining_fillable_calculator_test.ts | 50 +++++++++------------- 3 files changed, 26 insertions(+), 45 deletions(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/src/order_state_utils.ts b/packages/order-utils/src/order_state_utils.ts index ef868e60f..ca18097c9 100644 --- a/packages/order-utils/src/order_state_utils.ts +++ b/packages/order-utils/src/order_state_utils.ts @@ -113,10 +113,9 @@ export class OrderStateUtils { const zrxAssetData = assetProxyUtils.encodeERC20ProxyData(zrxTokenAddress); const isMakerAssetZRX = signedOrder.makerAssetData === zrxAssetData; - const isTraderMaker = true; const remainingFillableCalculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakerAssetZRX, transferrableMakerAssetAmount, transferrableFeeAssetAmount, diff --git a/packages/order-utils/src/remaining_fillable_calculator.ts b/packages/order-utils/src/remaining_fillable_calculator.ts index acab2f41b..bc146e931 100644 --- a/packages/order-utils/src/remaining_fillable_calculator.ts +++ b/packages/order-utils/src/remaining_fillable_calculator.ts @@ -1,8 +1,6 @@ -import { SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; export class RemainingFillableCalculator { - private _signedOrder: SignedOrder; private _isTraderAssetZRX: boolean; // Transferrable Amount is the minimum of Approval and Balance private _transferrableAssetAmount: BigNumber; @@ -12,21 +10,15 @@ export class RemainingFillableCalculator { private _orderFee: BigNumber; private _orderAssetAmount: BigNumber; constructor( - isTraderMaker: boolean, - signedOrder: SignedOrder, + orderFee: BigNumber, + orderAssetAmount: BigNumber, isTraderAssetZRX: boolean, transferrableAssetAmount: BigNumber, transferrableFeeAmount: BigNumber, remainingOrderAssetAmount: BigNumber, ) { - if (isTraderMaker) { - this._orderFee = signedOrder.makerFee; - this._orderAssetAmount = signedOrder.makerAssetAmount; - } else { - this._orderFee = signedOrder.takerFee; - this._orderAssetAmount = signedOrder.takerAssetAmount; - } - this._signedOrder = signedOrder; + this._orderFee = orderFee; + this._orderAssetAmount = orderAssetAmount; this._isTraderAssetZRX = isTraderAssetZRX; this._transferrableAssetAmount = transferrableAssetAmount; this._transferrableFeeAmount = transferrableFeeAmount; diff --git a/packages/order-utils/test/remaining_fillable_calculator_test.ts b/packages/order-utils/test/remaining_fillable_calculator_test.ts index 81f8132b2..45ad90626 100644 --- a/packages/order-utils/test/remaining_fillable_calculator_test.ts +++ b/packages/order-utils/test/remaining_fillable_calculator_test.ts @@ -65,10 +65,9 @@ describe('RemainingFillableCalculator', () => { it('calculates the correct amount when unfilled and funds available', () => { signedOrder = buildSignedOrder(); remainingMakeAssetAmount = signedOrder.makerAssetAmount; - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -79,10 +78,9 @@ describe('RemainingFillableCalculator', () => { it('calculates the correct amount when partially filled and funds available', () => { signedOrder = buildSignedOrder(); remainingMakeAssetAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1), decimals); - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -94,10 +92,9 @@ describe('RemainingFillableCalculator', () => { signedOrder = buildSignedOrder(); transferrableMakerFeeTokenAmount = zero; remainingMakeAssetAmount = signedOrder.makerAssetAmount; - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -110,10 +107,9 @@ describe('RemainingFillableCalculator', () => { const partiallyFilledAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(2), decimals); remainingMakeAssetAmount = signedOrder.makerAssetAmount.minus(partiallyFilledAmount); transferrableMakeAssetAmount = remainingMakeAssetAmount.minus(partiallyFilledAmount); - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -134,10 +130,9 @@ describe('RemainingFillableCalculator', () => { remainingMakeAssetAmount = signedOrder.makerAssetAmount; const transferredAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(2), decimals); transferrableMakeAssetAmount = remainingMakeAssetAmount.minus(transferredAmount); - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -159,10 +154,9 @@ describe('RemainingFillableCalculator', () => { remainingMakeAssetAmount = signedOrder.makerAssetAmount; const transferredAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(2), decimals); transferrableMakeAssetAmount = remainingMakeAssetAmount.minus(transferredAmount); - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -186,10 +180,9 @@ describe('RemainingFillableCalculator', () => { transferrableMakeAssetAmount = makerAmount.plus(makerFeeAmount); transferrableMakerFeeTokenAmount = transferrableMakeAssetAmount; remainingMakeAssetAmount = signedOrder.makerAssetAmount; - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -200,10 +193,9 @@ describe('RemainingFillableCalculator', () => { it('calculates the correct amount when partially filled and funds available', () => { signedOrder = buildSignedOrder(); remainingMakeAssetAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(1), decimals); - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -216,10 +208,9 @@ describe('RemainingFillableCalculator', () => { transferrableMakeAssetAmount = zero; transferrableMakerFeeTokenAmount = zero; remainingMakeAssetAmount = signedOrder.makerAssetAmount; - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, @@ -236,10 +227,9 @@ describe('RemainingFillableCalculator', () => { const orderToFeeRatio = signedOrder.makerAssetAmount.dividedToIntegerBy(signedOrder.makerFee); const expectedFillableAmount = new BigNumber(450980); - const isTraderMaker = true; calculator = new RemainingFillableCalculator( - isTraderMaker, - signedOrder, + signedOrder.makerFee, + signedOrder.makerAssetAmount, isMakeAssetZRX, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, -- cgit v1.2.3 From a200eaacaa3975b63f24d8be6cdfc7b0921d91ef Mon Sep 17 00:00:00 2001 From: Fabio Berger Date: Thu, 7 Jun 2018 18:51:52 +0200 Subject: Fix tslint failure --- packages/order-utils/test/remaining_fillable_calculator_test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/order-utils') diff --git a/packages/order-utils/test/remaining_fillable_calculator_test.ts b/packages/order-utils/test/remaining_fillable_calculator_test.ts index 45ad90626..c99d10f3f 100644 --- a/packages/order-utils/test/remaining_fillable_calculator_test.ts +++ b/packages/order-utils/test/remaining_fillable_calculator_test.ts @@ -1,4 +1,4 @@ -import { ECSignature, SignedOrder } from '@0xproject/types'; +import { SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import * as chai from 'chai'; -- cgit v1.2.3