aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLeonid <logvinov.leon@gmail.com>2017-10-24 23:54:50 +0800
committerGitHub <noreply@github.com>2017-10-24 23:54:50 +0800
commit8330dabda829ab507c859cf72462351f4ede70b9 (patch)
treef7b8b42ae711535f2ba75e87428286b32697265d
parentcd1f0c74c1439302cd691449c0493ed7a1c14bf3 (diff)
parent1fd8c2a6e28150cb7825cf79b0d46c1db9286053 (diff)
downloaddexon-sol-tools-8330dabda829ab507c859cf72462351f4ede70b9.tar
dexon-sol-tools-8330dabda829ab507c859cf72462351f4ede70b9.tar.gz
dexon-sol-tools-8330dabda829ab507c859cf72462351f4ede70b9.tar.bz2
dexon-sol-tools-8330dabda829ab507c859cf72462351f4ede70b9.tar.lz
dexon-sol-tools-8330dabda829ab507c859cf72462351f4ede70b9.tar.xz
dexon-sol-tools-8330dabda829ab507c859cf72462351f4ede70b9.tar.zst
dexon-sol-tools-8330dabda829ab507c859cf72462351f4ede70b9.zip
Merge pull request #197 from 0xProject/fix/rounding
Fix rounding of maker fill amount and correctly validate partial fees
-rw-r--r--CHANGELOG.md4
-rw-r--r--src/utils/order_validation_utils.ts43
-rw-r--r--test/order_validation_test.ts36
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);
+ });
});
});