diff options
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | src/utils/order_validation_utils.ts | 43 | ||||
| -rw-r--r-- | test/order_validation_test.ts | 36 |
3 files changed, 72 insertions, 11 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a60daca4..1148929e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # CHANGELOG +v0.22.2 - _October 24, 2017_ +------------------------ + * Fixed rounding of maker fill amount and incorrect validation of partial fees (#197) + v0.22.0 - _October 16, 2017_ ------------------------ * Started using `OrderFillRequest` interface instead of `OrderFillOrKillRequest` interface for `zeroEx.exchange.batchFillOrKill` (#187) diff --git a/src/utils/order_validation_utils.ts b/src/utils/order_validation_utils.ts index 1d9aac884..b7eae7e2f 100644 --- a/src/utils/order_validation_utils.ts +++ b/src/utils/order_validation_utils.ts @@ -27,13 +27,22 @@ export class OrderValidationUtils { if (!_.isUndefined(expectedFillTakerTokenAmount)) { fillTakerTokenAmount = expectedFillTakerTokenAmount; } - const fillMakerTokenAmount = this.getFillMakerTokenAmount(signedOrder, fillTakerTokenAmount); + const fillMakerTokenAmount = this.getPartialAmount( + fillTakerTokenAmount, + signedOrder.takerTokenAmount, + signedOrder.makerTokenAmount, + ); await exchangeTradeEmulator.transferFromAsync( signedOrder.makerTokenAddress, signedOrder.maker, signedOrder.taker, fillMakerTokenAmount, TradeSide.Maker, TransferType.Trade, ); + const makerFeeAmount = this.getPartialAmount( + fillTakerTokenAmount, + signedOrder.takerTokenAmount, + signedOrder.makerFee, + ); await exchangeTradeEmulator.transferFromAsync( - zrxTokenAddress, signedOrder.maker, signedOrder.feeRecipient, signedOrder.makerFee, + zrxTokenAddress, signedOrder.maker, signedOrder.feeRecipient, makerFeeAmount, TradeSide.Maker, TransferType.Fee, ); } @@ -100,7 +109,11 @@ export class OrderValidationUtils { public async validateFillOrderBalancesAllowancesThrowIfInvalidAsync( exchangeTradeEmulator: ExchangeTransferSimulator, signedOrder: SignedOrder, fillTakerTokenAmount: BigNumber.BigNumber, senderAddress: string, zrxTokenAddress: string): Promise<void> { - const fillMakerTokenAmount = this.getFillMakerTokenAmount(signedOrder, fillTakerTokenAmount); + const fillMakerTokenAmount = this.getPartialAmount( + fillTakerTokenAmount, + signedOrder.takerTokenAmount, + signedOrder.makerTokenAmount, + ); await exchangeTradeEmulator.transferFromAsync( signedOrder.makerTokenAddress, signedOrder.maker, senderAddress, fillMakerTokenAmount, TradeSide.Maker, TransferType.Trade, @@ -109,12 +122,22 @@ export class OrderValidationUtils { signedOrder.takerTokenAddress, senderAddress, signedOrder.maker, fillTakerTokenAmount, TradeSide.Taker, TransferType.Trade, ); + const makerFeeAmount = this.getPartialAmount( + fillTakerTokenAmount, + signedOrder.takerTokenAmount, + signedOrder.makerFee, + ); await exchangeTradeEmulator.transferFromAsync( - zrxTokenAddress, signedOrder.maker, signedOrder.feeRecipient, signedOrder.makerFee, TradeSide.Maker, + zrxTokenAddress, signedOrder.maker, signedOrder.feeRecipient, makerFeeAmount, TradeSide.Maker, TransferType.Fee, ); + const takerFeeAmount = this.getPartialAmount( + fillTakerTokenAmount, + signedOrder.takerTokenAmount, + signedOrder.takerFee, + ); await exchangeTradeEmulator.transferFromAsync( - zrxTokenAddress, senderAddress, signedOrder.feeRecipient, signedOrder.takerFee, TradeSide.Taker, + zrxTokenAddress, senderAddress, signedOrder.feeRecipient, takerFeeAmount, TradeSide.Taker, TransferType.Fee, ); } @@ -131,10 +154,12 @@ export class OrderValidationUtils { throw new Error(ExchangeContractErrs.OrderFillExpired); } } - private getFillMakerTokenAmount(signedOrder: Order, - fillTakerTokenAmount: BigNumber.BigNumber): BigNumber.BigNumber { - const exchangeRate = signedOrder.takerTokenAmount.div(signedOrder.makerTokenAmount); - const fillMakerTokenAmount = fillTakerTokenAmount.div(exchangeRate); + private getPartialAmount(numerator: BigNumber.BigNumber, denominator: BigNumber.BigNumber, + target: BigNumber.BigNumber): BigNumber.BigNumber { + const fillMakerTokenAmount = numerator + .mul(target) + .div(denominator) + .round(0); return fillMakerTokenAmount; } } diff --git a/test/order_validation_test.ts b/test/order_validation_test.ts index 784fa9ec4..dfcf1d43d 100644 --- a/test/order_validation_test.ts +++ b/test/order_validation_test.ts @@ -210,14 +210,14 @@ describe('OrderValidation', () => { }); describe('#validateFillOrderBalancesAllowancesThrowIfInvalidAsync', () => { let exchangeTransferSimulator: ExchangeTransferSimulator; - let transferFromAsync: any; + let transferFromAsync: Sinon.SinonSpy; const bigNumberMatch = (expected: BigNumber.BigNumber) => { return Sinon.match((value: BigNumber.BigNumber) => value.eq(expected)); }; beforeEach('create exchangeTransferSimulator', async () => { exchangeTransferSimulator = new ExchangeTransferSimulator(zeroEx.token); transferFromAsync = Sinon.spy(); - exchangeTransferSimulator.transferFromAsync = transferFromAsync; + exchangeTransferSimulator.transferFromAsync = transferFromAsync as any; }); it('should call exchangeTransferSimulator.transferFrom in a correct order', async () => { const makerFee = new BigNumber(2); @@ -291,5 +291,37 @@ describe('OrderValidation', () => { ), ).to.be.true(); }); + it('should correctly round the fillMakerTokenAmount', async () => { + const makerTokenAmount = new BigNumber(3); + const takerTokenAmount = new BigNumber(1); + const signedOrder = await fillScenarios.createAsymmetricFillableSignedOrderAsync( + makerTokenAddress, takerTokenAddress, makerAddress, takerAddress, makerTokenAmount, takerTokenAmount, + ); + await orderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( + exchangeTransferSimulator, signedOrder, takerTokenAmount, takerAddress, zrxTokenAddress, + ); + expect(transferFromAsync.callCount).to.be.equal(4); + const makerFillAmount = transferFromAsync.getCall(0).args[3]; + expect(makerFillAmount).to.be.bignumber.equal(makerTokenAmount); + }); + it('should correctly round the makerFeeAmount', async () => { + const makerFee = new BigNumber(2); + const takerFee = new BigNumber(4); + const signedOrder = await fillScenarios.createFillableSignedOrderWithFeesAsync( + makerTokenAddress, takerTokenAddress, makerFee, takerFee, makerAddress, takerAddress, + fillableAmount, ZeroEx.NULL_ADDRESS, + ); + const fillTakerTokenAmount = fillableAmount.div(2).round(0); + await orderValidationUtils.validateFillOrderBalancesAllowancesThrowIfInvalidAsync( + exchangeTransferSimulator, signedOrder, fillTakerTokenAmount, takerAddress, zrxTokenAddress, + ); + const makerPartialFee = makerFee.div(2); + const takerPartialFee = takerFee.div(2); + expect(transferFromAsync.callCount).to.be.equal(4); + const partialMakerFee = transferFromAsync.getCall(2).args[3]; + expect(partialMakerFee).to.be.bignumber.equal(makerPartialFee); + const partialTakerFee = transferFromAsync.getCall(3).args[3]; + expect(partialTakerFee).to.be.bignumber.equal(takerPartialFee); + }); }); }); |