From e30b8999d455503742dea3bbf1322365bcc474a1 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Fri, 5 Apr 2019 15:04:28 -0700 Subject: [PATCH] Update order utils to use new order schema --- packages/order-utils/src/constants.ts | 2 ++ packages/order-utils/src/order_factory.ts | 4 +++ .../src/remaining_fillable_calculator.ts | 18 +++++----- packages/order-utils/src/types.ts | 2 ++ packages/order-utils/test/order_hash_test.ts | 5 ++- .../remaining_fillable_calculator_test.ts | 36 ++++++++++--------- .../order-utils/test/signature_utils_test.ts | 8 +++-- 7 files changed, 46 insertions(+), 29 deletions(-) diff --git a/packages/order-utils/src/constants.ts b/packages/order-utils/src/constants.ts index b953b248e1..8d7272c146 100644 --- a/packages/order-utils/src/constants.ts +++ b/packages/order-utils/src/constants.ts @@ -125,6 +125,8 @@ export const constants = { { name: 'salt', type: 'uint256' }, { name: 'makerAssetData', type: 'bytes' }, { name: 'takerAssetData', type: 'bytes' }, + { name: 'makerFeeAssetData', type: 'bytes' }, + { name: 'takerFeeAssetData', type: 'bytes' }, ], }, EXCHANGE_ZEROEX_TRANSACTION_SCHEMA: { diff --git a/packages/order-utils/src/order_factory.ts b/packages/order-utils/src/order_factory.ts index 7e408ae826..6a41d3c4b3 100644 --- a/packages/order-utils/src/order_factory.ts +++ b/packages/order-utils/src/order_factory.ts @@ -42,6 +42,8 @@ export const orderFactory = { takerAssetAmount, makerAssetData, takerAssetData, + makerFeeAssetData: createOrderOpts.makerFeeAssetData || makerAssetData, + takerFeeAssetData: createOrderOpts.takerFeeAssetData || takerAssetData, takerAddress: createOrderOpts.takerAddress || defaultCreateOrderOpts.takerAddress, senderAddress: createOrderOpts.senderAddress || defaultCreateOrderOpts.senderAddress, makerFee: createOrderOpts.makerFee || defaultCreateOrderOpts.makerFee, @@ -110,6 +112,8 @@ function generateEmptyOrder(chainId: number): Order { takerAssetAmount: constants.ZERO_AMOUNT, makerAssetData: constants.NULL_BYTES, takerAssetData: constants.NULL_BYTES, + makerFeeAssetData: constants.NULL_BYTES, + takerFeeAssetData: constants.NULL_BYTES, salt: generatePseudoRandomSalt(), feeRecipientAddress: constants.NULL_ADDRESS, expirationTimeSeconds: constants.INFINITE_TIMESTAMP_SEC, diff --git a/packages/order-utils/src/remaining_fillable_calculator.ts b/packages/order-utils/src/remaining_fillable_calculator.ts index 92ffc8e80b..24a66ee76f 100644 --- a/packages/order-utils/src/remaining_fillable_calculator.ts +++ b/packages/order-utils/src/remaining_fillable_calculator.ts @@ -1,7 +1,7 @@ import { BigNumber } from '@0x/utils'; export class RemainingFillableCalculator { - private readonly _isTraderAssetZRX: boolean; + private readonly _isPercentageFee: boolean; // Transferrable Amount is the minimum of Approval and Balance private readonly _transferrableAssetAmount: BigNumber; private readonly _transferrableFeeAmount: BigNumber; @@ -12,14 +12,14 @@ export class RemainingFillableCalculator { constructor( orderFee: BigNumber, orderAssetAmount: BigNumber, - isTraderAssetZRX: boolean, + isPercentageFee: boolean, transferrableAssetAmount: BigNumber, transferrableFeeAmount: BigNumber, remainingOrderAssetAmount: BigNumber, ) { this._orderFee = orderFee; this._orderAssetAmount = orderAssetAmount; - this._isTraderAssetZRX = isTraderAssetZRX; + this._isPercentageFee = isPercentageFee; this._transferrableAssetAmount = transferrableAssetAmount; this._transferrableFeeAmount = transferrableFeeAmount; this._remainingOrderAssetAmount = remainingOrderAssetAmount; @@ -37,10 +37,10 @@ export class RemainingFillableCalculator { return this._calculatePartiallyFillableAssetAmount(); } private _hasSufficientFundsForFeeAndTransferAmount(): boolean { - if (this._isTraderAssetZRX) { - const totalZRXTransferAmountRequired = this._remainingOrderAssetAmount.plus(this._remainingOrderFeeAmount); + if (this._isPercentageFee) { + const totalTransferAmountRequired = this._remainingOrderAssetAmount.plus(this._remainingOrderFeeAmount); const hasSufficientFunds = this._transferrableAssetAmount.isGreaterThanOrEqualTo( - totalZRXTransferAmountRequired, + totalTransferAmountRequired, ); return hasSufficientFunds; } else { @@ -64,13 +64,13 @@ export class RemainingFillableCalculator { // 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 (this._isPercentageFee) { // 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._transferrableAssetAmount; + const totalAssetPooled = 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 - fillableTimesInAssetUnits = totalZRXTokenPooled.dividedBy(orderToFeeRatio.plus(new BigNumber(1))); + fillableTimesInAssetUnits = totalAssetPooled.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. diff --git a/packages/order-utils/src/types.ts b/packages/order-utils/src/types.ts index c58eafe036..ac0c103371 100644 --- a/packages/order-utils/src/types.ts +++ b/packages/order-utils/src/types.ts @@ -23,6 +23,8 @@ export interface CreateOrderOpts { feeRecipientAddress?: string; salt?: BigNumber; expirationTimeSeconds?: BigNumber; + makerFeeAssetData?: string; + takerFeeAssetData?: string; } /** diff --git a/packages/order-utils/test/order_hash_test.ts b/packages/order-utils/test/order_hash_test.ts index e9cc2b2a07..59c53bb5cd 100644 --- a/packages/order-utils/test/order_hash_test.ts +++ b/packages/order-utils/test/order_hash_test.ts @@ -14,7 +14,8 @@ const expect = chai.expect; describe('Order hashing', () => { describe('#getOrderHashHex', () => { - const expectedOrderHash = '0x507a818f74a0400444af08a1f58fa99b8df80ace57bfbdb5da304d9d1713e8ba'; + // const expectedOrderHash = '0x507a818f74a0400444af08a1f58fa99b8df80ace57bfbdb5da304d9d1713e8ba'; + const expectedOrderHash = '0x67f2637ce3098736611e86a1fd6fa25bef559d547c8751813f80e76507f344ab'; const fakeExchangeContractAddress = '0x1dc4c1cefef38a777b15aa20260a54e584b16c48'; const fakeChainID = 1337; const order: Order = { @@ -24,6 +25,8 @@ describe('Order hashing', () => { feeRecipientAddress: constants.NULL_ADDRESS, makerAssetData: constants.NULL_ADDRESS, takerAssetData: constants.NULL_ADDRESS, + makerFeeAssetData: constants.NULL_ADDRESS, + takerFeeAssetData: constants.NULL_ADDRESS, salt: new BigNumber(0), makerFee: new BigNumber(0), takerFee: new BigNumber(0), diff --git a/packages/order-utils/test/remaining_fillable_calculator_test.ts b/packages/order-utils/test/remaining_fillable_calculator_test.ts index 3d19088ea3..31a5c278bb 100644 --- a/packages/order-utils/test/remaining_fillable_calculator_test.ts +++ b/packages/order-utils/test/remaining_fillable_calculator_test.ts @@ -20,9 +20,11 @@ describe('RemainingFillableCalculator', () => { let makerAmount: BigNumber; let takerAmount: BigNumber; let makerFeeAmount: BigNumber; - let isMakeAssetZRX: boolean; + let isPercentageFee: boolean; const makerAssetData: string = '0x1'; const takerAssetData: string = '0x2'; + const makerFeeAssetData: string = '0x03'; + const takerFeeAssetData: string = '0x04'; const decimals: number = 4; const zero: BigNumber = new BigNumber(0); const chainId: number = 1337; @@ -53,6 +55,8 @@ describe('RemainingFillableCalculator', () => { takerAssetAmount: takerAmount, makerAssetData, takerAssetData, + makerFeeAssetData, + takerFeeAssetData, salt: zero, expirationTimeSeconds: zero, domain: { @@ -61,9 +65,9 @@ describe('RemainingFillableCalculator', () => { }, }; } - describe('Maker token is NOT ZRX', () => { + describe('Maker asset is not fee asset', () => { before(async () => { - isMakeAssetZRX = false; + isPercentageFee = false; }); it('calculates the correct amount when unfilled and funds available', () => { signedOrder = buildSignedOrder(); @@ -71,7 +75,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -84,7 +88,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -98,7 +102,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -113,7 +117,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -136,7 +140,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -144,7 +148,7 @@ describe('RemainingFillableCalculator', () => { expect(calculator.computeRemainingFillable()).to.be.bignumber.equal(transferrableMakeAssetAmount); }); }); - describe('Ratio is not evenly divisble', () => { + describe('Ratio is not evenly divisible', () => { beforeEach(async () => { [makerAmount, takerAmount, makerFeeAmount] = [ Web3Wrapper.toBaseUnitAmount(new BigNumber(3), decimals), @@ -160,7 +164,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -174,9 +178,9 @@ describe('RemainingFillableCalculator', () => { }); }); }); - describe('Maker Token is ZRX', () => { + describe('Maker asset is fee asset', () => { before(async () => { - isMakeAssetZRX = true; + isPercentageFee = true; }); it('calculates the correct amount when unfilled and funds available', () => { signedOrder = buildSignedOrder(); @@ -186,7 +190,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -199,7 +203,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -214,7 +218,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, @@ -233,7 +237,7 @@ describe('RemainingFillableCalculator', () => { calculator = new RemainingFillableCalculator( signedOrder.makerFee, signedOrder.makerAssetAmount, - isMakeAssetZRX, + isPercentageFee, transferrableMakeAssetAmount, transferrableMakerFeeTokenAmount, remainingMakeAssetAmount, diff --git a/packages/order-utils/test/signature_utils_test.ts b/packages/order-utils/test/signature_utils_test.ts index 0d82a11bdc..d9feaccc5b 100644 --- a/packages/order-utils/test/signature_utils_test.ts +++ b/packages/order-utils/test/signature_utils_test.ts @@ -33,6 +33,8 @@ describe('Signature utils', () => { feeRecipientAddress: constants.NULL_ADDRESS, makerAssetData: constants.NULL_ADDRESS, takerAssetData: constants.NULL_ADDRESS, + makerFeeAssetData: constants.NULL_ADDRESS, + takerFeeAssetData: constants.NULL_ADDRESS, salt: new BigNumber(0), makerFee: new BigNumber(0), takerFee: new BigNumber(0), @@ -164,7 +166,7 @@ describe('Signature utils', () => { describe('#ecSignOrderAsync', () => { it('should default to eth_sign if eth_signTypedData is unavailable', async () => { const expectedSignature = - '0x1c8bb89df0d0b537eced265159e75082086fa6f2c1422e743b36a095711edabf890531b8c37f0060bbd7d282a1a8a98b1497c85ac85a2eefc05854f090831fd9be03'; + '0x1c5e6c2a66037230b86a410b1d68108683ee8288d3668af91cab72dde1e1541ca86a50b638ccc743f6194a21dd313e71b4b155b2ad882f874d5d790d37d4623ca203'; const fakeProvider = { async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise { @@ -358,9 +360,9 @@ describe('Signature utils', () => { expect(signatureHex).to.eq(signedOrder.signature); expect(isValidSignature).to.eq(true); }); - it('should return the correct Signature for signatureHex concatenated as R + S + V', async () => { + it('should return the correct signature for signatureHex concatenated as R + S + V', async () => { const expectedSignature = - '0x1cdf371e6e60c9f2a0a3a1adcc1b27411e1d88bdb9c7dbae8c6faa38d37c5e330e2b467e08bbde8285d627cc5fc680746b9f97ec9d5e5be3b9fdf7abe37ed3279502'; + '0x1c686a55fa3f563115c0f2bc2b1c51964d0b6a05dd5ee862b702275600b7f634970f2275dccc1073f87a916dfde88574385b2d7a3eed0c2bdc08c76bad748d7d9002'; const fakeProvider = { async sendAsync(payload: JSONRPCRequestPayload, callback: JSONRPCErrorCallback): Promise { if (payload.method === 'eth_signTypedData') {